On Fri, 30 Jul 2021 17:18:30 +0200, Vitaly Rodionov wrote: > > From: Lucas Tanure <tanureal@xxxxxxxxxxxxxxxxxxxxx> > > 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@xxxxxxxxxxxxxxxxxxxxx> > Signed-off-by: Vitaly Rodionov <vitalyr@xxxxxxxxxxxxxxxxxxxxx> > Signed-off-by: Stefan Binding <sbinding@xxxxxxxxxxxxxxxxxxxxx> > --- > > 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
