On 08/03/2018 03:48 PM, Wolfram Sang wrote: > On Thu, Aug 02, 2018 at 09:38:58PM +0300, Tero Kristo wrote: >> On 02/08/18 10:15, Wolfram Sang wrote: >>> >>>> Dealing with the new API sounds pretty cumbersome, as this would mean that >>>> we need to change everything from the MFD / regmap level down to the i2c >>>> platform drivers (the poweroff handler uses regmap to write to the PMIC.) I >>> >>> Yes, that's the problem. That's where letting the I2C core decide will >>> save some hazzle. Some kind of whitelist for such transfer would be >>> nice, though, to still find buggy drivers. I haven't looked into that >>> yet if there is some information we can use (instead of passing yet >>> another flag around). >>> >>>> guess we could use a shortcut from the poweroff handler to write directly to >>>> the i2c bus though. >>> >>> It is not all about poweroff / reset. IIRC someone needed the irqless >>> transfer very early, too. >>> >>> My personal use case would be "reset", too. There are some R-Car Gen2 >>> boards which we need to reset via PMIC because of HW issues. >>> >> >> Grygorii proposed to use shutdown handler for omap I2C to fix the issue, and >> so it does. I added a simple shutdown handler to the driver which switches >> all following I2C traffic to use polling mode. Inlined patch below, what do >> you think of this approach? > > Hmm... > > * This is a driver specific solution, I don't see much we can move into > core. shutdown flag can be made part of the core. (or it can be named irq_poll_en) Maybe the shutdown handler, but then we would still need a > master_xfer_irqless callback. Most probably not only master_xfer_irqless callback is required, but i2c_transfer() need to be modified also to be lockless, because it is doing bus locking based on rt_mutex's as of now. As result scheduling may happen and irq might be enabled unconditionally (see __rt_mutex_slowlock()). Actually, master_xfer_irqless() callback might not be required at all if driver can auto switch to polling mode by checking shutdown flag. This will need to be indicated to i2c core somehow. > > * It will only work for 'very late' and not for 'very early'. it's the intention. Right? IRQ will be disabled after all .shutdown() callbacks are executed, so during this time regular I2C transactions will work. Or I misunderstood 'very early'? > > * Also, there is no white-listing, all transfers will be executed, so > buggy drivers will go unnoticed. yeah. That's probably a problem :( I though that some flag I2C_CLIENT_POWER(or smth) can be introduced for i2c client devices, so those devices can do i2c transfers during poweroff/reset with IRQs disabled (if i2c master driver support this) and I2C transfers for all other devices should fail. But ... PM operation can involve many devices :( pmic, gpio, i2c-muxes, regulators, so such flag will just spread all over the kernel. Another, options is to mark i2c devices in DT, but that might not go as SW specific. > > Or? > -- regards, -grygorii
