Hi,
While looking into a report by Florian Vaussard [1] I have noticed couple of most
likely unrelated issues:
- all boards using twl6040 configures the i2c bus to 400KHz while twl6040 is set
to 100KHz as default.
- if I set the audpwron GPIO high [2] in the bootloader the i2c communication towards
twl6040 will be broken
The solution or these are:
set the twl6040 to i2c fast mode with regmap patch
Clear the INTID register right after we request the audpwron GPIO and set it to
low.
Generated on top of:
git://git.linaro.org/people/lee.jones/mfd.git for-mfd-next
Tested on PandaBoard, PandaBoardES, OMAP4-blaze (SDP)
[1] http://www.spinics.net/lists/arm-kernel/msg310725.html
[2] Command in u-boot to enable the audpwron on PandaBoards: gpio set 127
Regards,
Peter
---
Peter Ujfalusi (3):
mfd: twl6040: Select i2c fast mode as default with regmap patch
mfd: twl6040: Move register patching earlier in probe
mfd: twl6040: Clear the interrupt ID register before requesting IRQ
drivers/mfd/twl6040.c | 20 ++++++++++++++------
include/linux/mfd/twl6040.h | 1 +
2 files changed, 15 insertions(+), 6 deletions(-)
--
1.9.0
Make sure that we patch the ACCCTL register as the first thing when the
driver loads, thus configuring I2C fast mode and i2c access for dual access
registers.
Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/mfd/twl6040.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
index 2c308750f40f..693d547db1de 100644
--- a/drivers/mfd/twl6040.c
+++ b/drivers/mfd/twl6040.c
@@ -665,6 +665,10 @@ static int twl6040_probe(struct i2c_client *client,
mutex_init(&twl6040->mutex);
init_completion(&twl6040->ready);
+ regmap_register_patch(twl6040->regmap, twl6040_patch,
+ ARRAY_SIZE(twl6040_patch));
+
+
twl6040->rev = twl6040_reg_read(twl6040, TWL6040_REG_ASICREV);
if (twl6040->rev < 0) {
dev_err(&client->dev, "Failed to read revision register: %d\n",
@@ -712,10 +716,6 @@ static int twl6040_probe(struct i2c_client *client,
goto readyirq_err;
}
- /* dual-access registers controlled by I2C only */
- regmap_register_patch(twl6040->regmap, twl6040_patch,
- ARRAY_SIZE(twl6040_patch));
-
/*
* The main functionality of twl6040 to provide audio on OMAP4+ systems.
* We can add the ASoC codec child whenever this driver has been loaded.
--
1.9.0
If for some reason the boot loader enabled the audpwron GPIO we will have
pending IRQs to be handled. This seams to break twl6040 for some reason
leading to non working i2c communication (i2c timeouts). Clearing the INTID
register after we requested the audpwron GPIO (and set it to low) will
ensure that the chip will operate normally in this case as well.
Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/mfd/twl6040.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
index 693d547db1de..ea8acfb029df 100644
--- a/drivers/mfd/twl6040.c
+++ b/drivers/mfd/twl6040.c
@@ -688,6 +688,9 @@ static int twl6040_probe(struct i2c_client *client,
GPIOF_OUT_INIT_LOW, "audpwron");
if (ret)
goto gpio_err;
+
+ /* Clear any pending interrupt */
+ twl6040_reg_read(twl6040, TWL6040_REG_INTID);
}
ret = regmap_add_irq_chip(twl6040->regmap, twl6040->irq, IRQF_ONESHOT,
--
1.9.0
All boards using twl6040 configures the i2c bus to 400KHz. While twl6040's
defaults to normal mode (100KHz). So far twl6040 has no problem with i2c
communication in this configuration it is safer to select fast i2c mode.
Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/mfd/twl6040.c | 9 +++++++--
include/linux/mfd/twl6040.h | 1 +
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
index 6e88f25832fb..2c308750f40f 100644
--- a/drivers/mfd/twl6040.c
+++ b/drivers/mfd/twl6040.c
@@ -87,8 +87,13 @@ static struct reg_default twl6040_defaults[] = {
};
static struct reg_default twl6040_patch[] = {
- /* Select I2C bus access to dual access registers */
- { TWL6040_REG_ACCCTL, 0x09 },
+ /*
+ * Select I2C bus access to dual access registers
+ * Interrupt register is cleared on read
+ * Select fast mode for i2c (400KHz)
+ */
+ { TWL6040_REG_ACCCTL,
+ TWL6040_I2CSEL | TWL6040_INTCLRMODE | TWL6040_I2CMODE(1) },
};
diff --git a/include/linux/mfd/twl6040.h b/include/linux/mfd/twl6040.h
index 81f639bc1ae6..a69d16b30c18 100644
--- a/include/linux/mfd/twl6040.h
+++ b/include/linux/mfd/twl6040.h
@@ -157,6 +157,7 @@
#define TWL6040_I2CSEL 0x01
#define TWL6040_RESETSPLIT 0x04
#define TWL6040_INTCLRMODE 0x08
+#define TWL6040_I2CMODE(x) ((x & 0x3) << 4)
/* STATUS (0x2E) fields */
--
1.9.0
On 02/27/2014 07:56 AM, Peter Ujfalusi wrote:
> All boards using twl6040 configures the i2c bus to 400KHz. While twl6040's
> defaults to normal mode (100KHz). So far twl6040 has no problem with i2c
> communication in this configuration it is safer to select fast i2c mode.
>
> Signed-off-by: Peter Ujfalusi <[email protected]>
> ---
> drivers/mfd/twl6040.c | 9 +++++++--
> include/linux/mfd/twl6040.h | 1 +
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
> index 6e88f25832fb..2c308750f40f 100644
> --- a/drivers/mfd/twl6040.c
> +++ b/drivers/mfd/twl6040.c
> @@ -87,8 +87,13 @@ static struct reg_default twl6040_defaults[] = {
> };
>
> static struct reg_default twl6040_patch[] = {
> - /* Select I2C bus access to dual access registers */
> - { TWL6040_REG_ACCCTL, 0x09 },
> + /*
> + * Select I2C bus access to dual access registers
> + * Interrupt register is cleared on read
> + * Select fast mode for i2c (400KHz)
> + */
> + { TWL6040_REG_ACCCTL,
> + TWL6040_I2CSEL | TWL6040_INTCLRMODE | TWL6040_I2CMODE(1) },
> };
>
>
> diff --git a/include/linux/mfd/twl6040.h b/include/linux/mfd/twl6040.h
> index 81f639bc1ae6..a69d16b30c18 100644
> --- a/include/linux/mfd/twl6040.h
> +++ b/include/linux/mfd/twl6040.h
> @@ -157,6 +157,7 @@
> #define TWL6040_I2CSEL 0x01
> #define TWL6040_RESETSPLIT 0x04
> #define TWL6040_INTCLRMODE 0x08
> +#define TWL6040_I2CMODE(x) ((x & 0x3) << 4)
>
> /* STATUS (0x2E) fields */
>
>
we should ideally have been using highspeed for i2c bus.
is'nt it better if i2c_check_functionality (and adding required flags
for func) be used to check the adapter speed and decide this in the
driver instead of hardcoding the bus speed within TWL6040 -
considering that 6040 can infact do max high speed as well?
--
Regards,
Nishanth Menon
On 02/27/2014 04:24 PM, Nishanth Menon wrote:
>
> we should ideally have been using highspeed for i2c bus.
>
> is'nt it better if i2c_check_functionality (and adding required flags
> for func) be used to check the adapter speed and decide this in the
> driver instead of hardcoding the bus speed within TWL6040 -
> considering that 6040 can infact do max high speed as well?
Yeah, I was thinking of something similar. Just did not found the API to get
the speed the i2c bus has been configured to.
I'll check this.
--
P?ter
On 02/27/2014 08:33 AM, Peter Ujfalusi wrote:
> On 02/27/2014 04:24 PM, Nishanth Menon wrote:
>>
>> we should ideally have been using highspeed for i2c bus.
>>
>> is'nt it better if i2c_check_functionality (and adding required flags
>> for func) be used to check the adapter speed and decide this in the
>> driver instead of hardcoding the bus speed within TWL6040 -
>> considering that 6040 can infact do max high speed as well?
>
> Yeah, I was thinking of something similar. Just did not found the API to get
> the speed the i2c bus has been configured to.
> I'll check this.
The other option might be to blindly configure 6040 to max speed ->
but then you do have an issue with that single register write
operation to configure the speed?
TWL6040 is by default at 100KHz, bus_speed is configured (via dts or
otherwise) is 400KHz/3.4MHz, the register write for configuring 6040
to max speed will occur at 400KHz/3.4MHz, which implies 6040 register
write operation might not actually take place (as 6040 still expects
to talk 100KHz till the mentioned register write takes place).
Ideally, the behavior you need is as follows:
talk at 100KHz for the first register write(of configuring speed),
followed by 400KHz/3.4 MHz for the subsequent operations on the bus
(assuming 400KHz/3.4 is the least common denominator speed on the bus).
I am not sure if the i2c framework has ability to do that.
--
Regards,
Nishanth Menon
Hi Peter,
On 02/27/2014 02:56 PM, Peter Ujfalusi wrote:
> Hi,
>
> While looking into a report by Florian Vaussard [1] I have noticed couple of most
> likely unrelated issues:
> - all boards using twl6040 configures the i2c bus to 400KHz while twl6040 is set
> to 100KHz as default.
> - if I set the audpwron GPIO high [2] in the bootloader the i2c communication towards
> twl6040 will be broken
>
> The solution or these are:
> set the twl6040 to i2c fast mode with regmap patch
> Clear the INTID register right after we request the audpwron GPIO and set it to
> low.
>
> Generated on top of:
> git://git.linaro.org/people/lee.jones/mfd.git for-mfd-next
>
> Tested on PandaBoard, PandaBoardES, OMAP4-blaze (SDP)
>
> [1] http://www.spinics.net/lists/arm-kernel/msg310725.html
> [2] Command in u-boot to enable the audpwron on PandaBoards: gpio set 127
>
> Regards,
> Peter
> ---
> Peter Ujfalusi (3):
> mfd: twl6040: Select i2c fast mode as default with regmap patch
> mfd: twl6040: Move register patching earlier in probe
> mfd: twl6040: Clear the interrupt ID register before requesting IRQ
Patch 3 is making my system to boot normally.
Now, it is hard to devise if this is the root cause of the problem that
I was experiencing, since adding a sleep at the exact same place was
fixing my issue. Your twl6040_reg_read() could have the same effect,
without solving directly the issue.
Anyway, I tested your series on DuoVero.
Tested-by: Florian Vaussard <[email protected]>
Regards,
Florian
On 02/27/2014 05:00 PM, Nishanth Menon wrote:
> The other option might be to blindly configure 6040 to max speed ->
> but then you do have an issue with that single register write
> operation to configure the speed?
Yes, exactly. It is unfortunate that twl6040's i2c speed is configured via i2c
write. It would have been better if we would have had pins to configure this.
> TWL6040 is by default at 100KHz, bus_speed is configured (via dts or
> otherwise) is 400KHz/3.4MHz, the register write for configuring 6040
> to max speed will occur at 400KHz/3.4MHz, which implies 6040 register
> write operation might not actually take place (as 6040 still expects
> to talk 100KHz till the mentioned register write takes place).
>
> Ideally, the behavior you need is as follows:
> talk at 100KHz for the first register write(of configuring speed),
> followed by 400KHz/3.4 MHz for the subsequent operations on the bus
> (assuming 400KHz/3.4 is the least common denominator speed on the bus).
Yes. Or the bootloader should have been configuring the twl6040 to 3.4MHz with
a single write. When the driver comes up the i2c controller had been already
probed and it would know that the bus can be used in 3.4MHz and we would not
have issues with the speed.
Yous see: twl6030 is also on the same bus (usually). Not sure how twl6030's
i2c speed is selected but the fact that twl6040 is not alone on the bus makes
things complicated.
Also you could have more devices on the bus, wired for 3.4MHz. In that case we
would need to make sure that the first access happens with 100KHz to twl6040
to select 3.4MHz mode, switch the controller speed and allow the communication
to other chips. It is another question how the 3.4MHz clients will interpret
the 100KHz communication on the bus, I guess it is ignored by them.
> I am not sure if the i2c framework has ability to do that.
AFAIK we do not have such a thing in i2c framework. We would need client
drivers reporting what they support, notification mechanism to notify clients
on speed change and couple of other things might be needed as well. So there
are quite a number of things missing to get dynamic/adaptive i2c working where
the master configuration depends on the attached devices and not the other way
around.
I'll do some experiments with the twl6040 and i2c speeds today, but so far I
think the 400KHz change is a safe thing to do.
--
P?ter
On 02/28/2014 01:39 AM, Peter Ujfalusi wrote:
> On 02/27/2014 05:00 PM, Nishanth Menon wrote:
>> The other option might be to blindly configure 6040 to max speed ->
>> but then you do have an issue with that single register write
>> operation to configure the speed?
>
> Yes, exactly. It is unfortunate that twl6040's i2c speed is configured via i2c
> write. It would have been better if we would have had pins to configure this.
>
>> TWL6040 is by default at 100KHz, bus_speed is configured (via dts or
>> otherwise) is 400KHz/3.4MHz, the register write for configuring 6040
>> to max speed will occur at 400KHz/3.4MHz, which implies 6040 register
>> write operation might not actually take place (as 6040 still expects
>> to talk 100KHz till the mentioned register write takes place).
>>
>> Ideally, the behavior you need is as follows:
>> talk at 100KHz for the first register write(of configuring speed),
>> followed by 400KHz/3.4 MHz for the subsequent operations on the bus
>> (assuming 400KHz/3.4 is the least common denominator speed on the bus).
>
> Yes. Or the bootloader should have been configuring the twl6040 to 3.4MHz with
> a single write. When the driver comes up the i2c controller had been already
you would not be able to do that unless you have powered on TWL6040
regulators - and the moment you power them down (example on module
removal: regulator_bulk_disable(TWL6040_NUM_SUPPLIES,
twl6040->supplies);), you are back to square 1.
> probed and it would know that the bus can be used in 3.4MHz and we would not
> have issues with the speed.
> Yous see: twl6030 is also on the same bus (usually). Not sure how twl6030's
> i2c speed is selected but the fact that twl6040 is not alone on the bus makes
> things complicated.
TWL6030 can do 3.3MHz by default and there are no speed registers to
configure.
> Also you could have more devices on the bus, wired for 3.4MHz. In that case we
> would need to make sure that the first access happens with 100KHz to twl6040
> to select 3.4MHz mode, switch the controller speed and allow the communication
> to other chips. It is another question how the 3.4MHz clients will interpret
> the 100KHz communication on the bus, I guess it is ignored by them.
As far as the i2c spec 2.1 says:
"Fast-mode devices are downward-compatible and can communicate with
Standard-mode devices in a 0 to 100 kbit/s I2C-bus system"
...
"Hs-mode devices can transfer information at bit rates of up to 3.4
Mbit/s, yet they remain fully downward compatible with Fast- or
Standard-mode (F/S-mode) devices for bi-directional communication in a
mixed-speed bus system."
So, they are backward compatible.
>
>> I am not sure if the i2c framework has ability to do that.
>
> AFAIK we do not have such a thing in i2c framework. We would need client
> drivers reporting what they support, notification mechanism to notify clients
> on speed change and couple of other things might be needed as well. So there
> are quite a number of things missing to get dynamic/adaptive i2c working where
> the master configuration depends on the attached devices and not the other way
> around.
>
> I'll do some experiments with the twl6040 and i2c speeds today, but so far I
> think the 400KHz change is a safe thing to do.
>
I dont think that is correct. The safest thing to do is to switch the
i2c bus speed to 100KHz. you are depending on emperical results and a
specification violation by expecting to let a chip default set for
100KHz to always allow for the first communication at 400KHz.
Unless ofcourse, you can confirm that the default speed register
writes are allowed at 400KHz contrary to the data sheet info you
mentioned in the patch.
--
Regards,
Nishanth Menon
On 02/28/2014 03:30 PM, Nishanth Menon wrote:
> TWL6030 can do 3.3MHz by default and there are no speed registers to
> configure.
According to the datasheet the speed of twl6030 is limited to 2.4MHz. I have
not seen registers or pins to select the speed. As the documentation puts
this: High-speed mode (limited to 2.4Mbit/s maximum)
>> Also you could have more devices on the bus, wired for 3.4MHz. In that case we
>> would need to make sure that the first access happens with 100KHz to twl6040
>> to select 3.4MHz mode, switch the controller speed and allow the communication
>> to other chips. It is another question how the 3.4MHz clients will interpret
>> the 100KHz communication on the bus, I guess it is ignored by them.
> As far as the i2c spec 2.1 says:
> "Fast-mode devices are downward-compatible and can communicate with
> Standard-mode devices in a 0 to 100 kbit/s I2C-bus system"
> ...
> "Hs-mode devices can transfer information at bit rates of up to 3.4
> Mbit/s, yet they remain fully downward compatible with Fast- or
> Standard-mode (F/S-mode) devices for bi-directional communication in a
> mixed-speed bus system."
>
> So, they are backward compatible.
I just tried it on PandaBoard to set the i2c speed to 24MHz. If I do not touch
the twl6040 ACCCTL register's i2cmode I can not access to twl6040 later on.
However if I select the high-speed mode as the first write everything is fine
afterwards.
Hrm, it is possible that I can write with high-speed to twl6040 but the
twl6040 is sending back the data in normal-mode?
So in theory if I have the ACCCTL write as a first I2C access towards twl6040
we might be safe if the bus is in high-speed mode?
If I have fast mode configured to the controller, I can still communicate with
twl6040 even if it is set to normal mode.
I still think that this patch is safe for now. We could try to figure out how
to increase the i2c speed on the bus for twl6030 and twl6040. This has to be
done in the same series.
Now after several boots:
It seams if I set the i2c to 2.4MHz and I can not communicate with twl6040
right after cold power on.
So if the i2c bus is already set to 2.4GHz I can not set the twl6040 ACCCTL
register. But the content of ACCCTL register seams to be preserved between
reboots and this is why I saw that the 2.4MHz bus speed might be even possible.
--
P?ter
On 02/28/2014 08:26 AM, Peter Ujfalusi wrote:
> On 02/28/2014 03:30 PM, Nishanth Menon wrote:
>> TWL6030 can do 3.3MHz by default and there are no speed registers to
>> configure.
>
> According to the datasheet the speed of twl6030 is limited to 2.4MHz. I have
> not seen registers or pins to select the speed. As the documentation puts
> this: High-speed mode (limited to 2.4Mbit/s maximum)
Aye, you are correct, the latest data sheet does show this - i made
the mistake of opening an older spec doc :(..
>>> Also you could have more devices on the bus, wired for 3.4MHz. In that case we
>>> would need to make sure that the first access happens with 100KHz to twl6040
>>> to select 3.4MHz mode, switch the controller speed and allow the communication
>>> to other chips. It is another question how the 3.4MHz clients will interpret
>>> the 100KHz communication on the bus, I guess it is ignored by them.
>> As far as the i2c spec 2.1 says:
>> "Fast-mode devices are downward-compatible and can communicate with
>> Standard-mode devices in a 0 to 100 kbit/s I2C-bus system"
>> ...
>> "Hs-mode devices can transfer information at bit rates of up to 3.4
>> Mbit/s, yet they remain fully downward compatible with Fast- or
>> Standard-mode (F/S-mode) devices for bi-directional communication in a
>> mixed-speed bus system."
>>
>> So, they are backward compatible.
>
> I just tried it on PandaBoard to set the i2c speed to 24MHz. If I do not touch
> the twl6040 ACCCTL register's i2cmode I can not access to twl6040 later on.
> However if I select the high-speed mode as the first write everything is fine
> afterwards.
> Hrm, it is possible that I can write with high-speed to twl6040 but the
> twl6040 is sending back the data in normal-mode?
I doubt it would do that as it would be a violation of i2c high speed
specification.
> So in theory if I have the ACCCTL write as a first I2C access towards twl6040
> we might be safe if the bus is in high-speed mode?
I think you did catch the real behavior in the email below.
I do however suggest that you put one of those i2c analyser on the
rail to see what is happening. else, we'd be doing a lot of guessing here.
>
> If I have fast mode configured to the controller, I can still communicate with
> twl6040 even if it is set to normal mode.
>
> I still think that this patch is safe for now. We could try to figure out how
> to increase the i2c speed on the bus for twl6030 and twl6040. This has to be
> done in the same series.
?? you mean set i2c bus speed to full speed on PandaBoard? it is
already full speed[1]!
>
> Now after several boots:
> It seams if I set the i2c to 2.4MHz and I can not communicate with twl6040
> right after cold power on.
> So if the i2c bus is already set to 2.4GHz I can not set the twl6040 ACCCTL
> register. But the content of ACCCTL register seams to be preserved between
> reboots and this is why I saw that the 2.4MHz bus speed might be even possible.
This observation is the key.
Are you power cycling the regulators for TWL6040 as part of your
shutdown handler? Did you read the ACCTL register after a reboot to
see if the register contents are still the same as before?
Based on this observation, I suggest switching [1] to 100KHz.
[1]
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/omap4-panda-common.dtsi#n291
--
Regards,
Nishanth Menon