On Fri, 30 Jul 2021 17:18:30 +0200, Vitaly Rodionov wrote:
From: Lucas Tanure tanureal@opensource.cirrus.com
Only disable I2C clock 25 ms after not being used.
The current implementation enables and disables the I2C clock for each I2C transaction. Each enable/disable call requires two verb transactions. This means each I2C transaction requires a total of four verb transactions to enable and disable the clock. However, if there are multiple consecutive I2C transactions, it is not necessary to enable and disable the clock each time, instead it is more efficient to enable the clock for the first transaction, and disable it after the final transaction, which would improve performance. This is achieved by using a timeout which disables the clock if no request to enable the clock has occurred for 25 ms.
Signed-off-by: Lucas Tanure tanureal@opensource.cirrus.com Signed-off-by: Vitaly Rodionov vitalyr@opensource.cirrus.com Signed-off-by: Stefan Binding sbinding@opensource.cirrus.com
Changes in v2:
- Improved delayed work start/cancel implementation, and re-worked commit message
adding more explanation why this was required.
Changes in v3:
- Cancel the disable timer, but do not wait for any running disable functions to finish.
If the disable timer runs out before cancel, the delayed work thread will be blocked, waiting for the mutex to become unlocked. This mutex will be locked for the duration of any i2c transaction, so the disable function will run to completion immediately afterwards in the scenario. The next enable call will re-enable the clock, regardless.
This looks almost fine, but just a couple of thoughts:
- cancel_delayed_work_sync() means to it might keep the i2c enabled after that point (just cancel the pending work). Would it cause a inconsistency afterwards?
- A similar procedure is needed for suspend callback to cancel / flush the work. The shutdown is another question, but usually it's fine to without any special handling as long as the resource is kept.
thanks,
Takashi