Skip to contentSkip to conversation

i2c: davinci: Fix problems discovered on Keystone CPU

PATCH In review Alexander Sverdlin 3 participants 4 messages Mar 11, 2015 – Apr 3, 2015 3 messages not in the archive I2C subsystem host drivers

lore
Share & tools
lore
#0 lore raw

cover i2c: davinci: Fix problems discovered on Keystone CPU

During intensive I2C tests on Keystone CPU we've discovered several problems with the i2c-davinci driver. Maybe Keystone is the first relatively powerful CPU utilizing davinci controller and therefore the races in the driver became more obvious, but it's clear that many users had problems with the driver and even though driver has received many band aids over the years, nobody addressed the original racy design.

The series address the follwing problems:
"i2c: davinci: Rework racy ISR" -- stop the parallel activities in the driver including concurrect registers access. Also simplifies the design and removes some bad aids.
"i2c: davinci: Refactor i2c_davinci_wait_bus_not_busy()" -- the routine is over-designed and instead of "waiting" will trigger the bus recovering immediately.
"i2c: davinci: Avoid sending to own address" -- works around one davinci controller issue when it unexpectedly switches to some sort of slave mode trying to send to its own address. The controller remains in this locked state until reset, so it's better to avoid this situation than to deal with transfer timeouts.

#2 lore raw
In reply to a message not in the archive
Quoted 7 lines · Alexander Sverdlin

During intensive I2C tests on Keystone CPU we've discovered several problems with the i2c-davinci driver. Maybe Keystone is the first relatively powerful CPU utilizing davinci controller and therefore the races in the driver became more obvious, but it's clear that many users had problems with the driver and even though driver has received many band aids over the years, nobody addressed the original racy design.

Quoted 13 lines

The series address the follwing problems:
"i2c: davinci: Rework racy ISR" -- stop the parallel activities in the driver including concurrect registers access. Also simplifies the design and removes some bad aids.
"i2c: davinci: Refactor i2c_davinci_wait_bus_not_busy()" -- the routine is over-designed and instead of "waiting" will trigger the bus recovering immediately.
"i2c: davinci: Avoid sending to own address" -- works around one davinci controller issue when it unexpectedly switches to some sort of slave mode trying to send to its own address. The controller remains in this locked state until reset, so it's better to avoid this situation than to deal with transfer timeouts.

Have you used git format-patch --cover-letter?
Overall stats and list of patches are missing.

Also I think It'll be better to send patches 2/3 first, as they have more chances to be merged.

regards,
-grygorii

#4 lore raw
In reply to a message not in the archive

Hi!

Quoted 14 lines · On 11/03/15 19:35, ext [email protected]…
Quoted 13 lines

The series address the follwing problems:

Quoted 12 lines

"i2c: davinci: Rework racy ISR" -- stop the parallel activities in the driver including concurrect registers access. Also simplifies the design and removes some bad aids.
"i2c: davinci: Refactor i2c_davinci_wait_bus_not_busy()" -- the routine is over-designed and instead of "waiting" will trigger the bus recovering immediately.
"i2c: davinci: Avoid sending to own address" -- works around one davinci controller issue when it unexpectedly switches to some sort of slave mode trying to send to its own address. The controller remains in this locked state until reset, so it's better to avoid this situation than to deal with transfer timeouts.

Quoted 2 lines

Have you used git format-patch --cover-letter?
Overall stats and list of patches are missing.

Will do in v2...

Quoted 2 lines

Also I think It'll be better to send patches 2/3 first, as they have more chances to be merged.

First patch is an important fix for blocking problem and therefore a candidate for -stable backports. So, it should be less dependent of the less-important 2 and 3.

Signature · 2 lines
Best regards,
Alexander Sverdlin.
#6 lore raw
In reply to a message not in the archive
Quoted 17 lines · Alexander Sverdlin

Hi!

Quoted 14 lines · On 11/03/15 19:35, ext [email protected]…
Quoted 13 lines

The series address the follwing problems:

Quoted 12 lines

"i2c: davinci: Rework racy ISR" -- stop the parallel activities in the driver including concurrect registers access. Also simplifies the design and removes some bad aids.
"i2c: davinci: Refactor i2c_davinci_wait_bus_not_busy()" -- the routine is over-designed and instead of "waiting" will trigger the bus recovering immediately.
"i2c: davinci: Avoid sending to own address" -- works around one davinci controller issue when it unexpectedly switches to some sort of slave mode trying to send to its own address. The controller remains in this locked state until reset, so it's better to avoid this situation than to deal with transfer timeouts.

Quoted 4 lines
Quoted 2 lines

Have you used git format-patch --cover-letter?
Overall stats and list of patches are missing.

Will do in v2...

And please make sure the patches are chained to the cover-letter. That makes reviewing much easier.

Quoted 2 lines

First patch is an important fix for blocking problem and therefore a candidate for -stable backports. So, it should be less dependent of the less-important 2 and 3.

IMO this patch is too intrusive for stable, so I won't add the tag. You may request that individually to stable maintainers, though, if you disagree.