On Sat, Oct 28, 2023 at 03:31:00AM -0600, James Hilliard wrote: > On Fri, Oct 27, 2023 at 1:59 PM Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> > wrote: > > > Hi James, > > > > On Tue, Oct 24, 2023 at 07:39:38PM -0600, James Hilliard wrote: > > > The vendor cyttsp5 driver does not use regmap for i2c support, it > > > would appear this is due to regmap not providing sufficient levels > > > of control to handle various error conditions that may be present > > > under some configuration/firmware variants. > > > > > > To improve reliability lets refactor the cyttsp5 i2c interface to > > > function more like the vendor driver and implement some of the error > > > handling retry/recovery techniques present there. > > > > Sorry but you need to elaborate more on what is missing in regmap and > > how vendor code is better. In my experience vendors rarely follow kernel > > development and either are not aware of the latest kernel APIs, or they > > simply have the driver written to what we had in 3.x kernels and have > > not really updated it since then. > > > > I'm unaware of a way to do essentially raw reads when using regmap, for > example I don't know of a way to implement the cyttsp5_deassert_read > function using regmap, maybe there's a way I'm not aware of however? What is wrong with current way of reading from the input register? It should clear the interrupt line. > > In general the issue with regmap seems to be that regmap always does > operations against specific registers and prevents doing raw i2c operations > needed to handle some hardware/firmware issues for some variants. What are those issues and why do they need raw access. > > Note that I'm not exactly doing things the same way the vendor driver does, > I have simplified the error recovery/retry code paths in the startup > function. > > > > > > > > > > As part of this rather than assuming the device is in bootloader mode > > > we should first check that the device is in bootloader and only > > > attempt to launch the app if it actually is in the bootloader. > > > > I would prefer if this was split into a separate patch. > > > > I think this change is somewhat intertwined with the probe retry/recovery > logic > changes and is a bit tricky to split out without breaking the startup > sequence > from my testing at least. I understand that it might be tricky but each logical change should stand on its own. Thanks. -- Dmitry
