Joel's suggestion to explore the previously proposed generic solution makes sense, especially considering the recent changes for PMBus devices that have added delays. Thank you for your review and input. I will modify the code to incorporate the necessary delay directly in the max31785 driver. Thanks, Lakshmi On 10/10/23, 10:54 PM, "Andrew Jeffery" <andrew@xxxxxxxx <mailto:andrew@xxxxxxxx>> wrote: On Wed, 11 Oct 2023, at 09:29, Guenter Roeck wrote: > On Tue, Oct 10, 2023 at 08:58:06PM +0200, Wolfram Sang wrote: >> Hi Guenter, >> >> > > > Reference to Andrew's previous proposal: >> > > > https://lore.kernel.org/all/20200914122811.3295678-1-andrew@xxxxxxxx/ <https://lore.kernel.org/all/20200914122811.3295678-1-andrew@xxxxxxxx/> >> > > >> > > I do totally agree with Guenter's comment[1], though. This just affects >> > > a few drivers and this patch is way too intrusive for the I2C core. The >> > > later suggested prepare_device() callback[2] sounds better to me. I >> > > still haven't fully understood why this all cannot be handled in the >> > > driver's probe. Could someone give me a small summary about that? >> > > >> > >> > Lots of PMBus devices have the same problem, we have always handled >> > it in PMBus drivers by implementing local wait code, and your references >> > point that out. >> >> I am confused now. Reading your reply: >> >> "I am not sure if an implementation in the i2c core is desirable. It >> looks quite invasive to me, and it won't solve the problem for all >> devices since it isn't always a simple "wait <n> microseconds between >> accesses". For example, some devices may require a wait after a write >> but not after a read, or a wait only after certain commands (such as >> commands writing to an EEPROM)." >> >> I get the impression you don't prefer to have a generic mechanism in the >> I2C core. This I share. Your response now sounds like you do support >> that idea now? >> > > I didn't (want to) say that. I am perfectly happy with driver specific > code, and I would personally still very much prefer it. I only wanted to > suggest that _if_ a generic solution is implemented, it should cover all > existing use cases and not just this one. But, really, I'd rather leave > that alone and not risk introducing regressions to existing drivers. We had an out-of-tree patch for the max31785[1] that I wrote a little after the initial discussion on this generic throttling and possibly somewhat before the other drivers had their delays added. Recently Joel pointed out the addition of the delays in the other drivers and I raised the idea that we could get rid of that out-of-tree patch by doing the same. Guenter's point about the work-arounds being very particular to the device is good justification for not trying to fix drivers that we can't immediately test - not that the series did that, but arguably if we're shooting for the generic solution then it should. So I agree with Guenter that we probably want to do down the path of adding the delays directly into the max31785 driver and not trying to over-generalise. Lakshmi: Apologies for misleading you in some way there - unfortunately I can't go back to understand exactly what I suggested as I've changed jobs in the mean time. Andrew [1]: https://github.com/openbmc/linux/commit/44e1397368a70ffe9cdad1f9212ffdef8c16b9be <https://github.com/openbmc/linux/commit/44e1397368a70ffe9cdad1f9212ffdef8c16b9be>
