On 03.08.2020 17:16, Russell King - ARM Linux admin wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Thu, Jul 30, 2020 at 09:00:36AM +0000, Codrin.Ciubotariu@xxxxxxxxxxxxx wrote: >> On 27.07.2020 13:50, Russell King - ARM Linux admin wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> On Mon, Jul 27, 2020 at 10:44:57AM +0000, Codrin.Ciubotariu@xxxxxxxxxxxxx wrote: >>>> On 24.07.2020 23:52, Russell King - ARM Linux admin wrote: >>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>>>> >>>>> On Fri, Jul 24, 2020 at 09:39:13PM +0200, Wolfram Sang wrote: >>>>>> On Sun, Jul 05, 2020 at 11:19:18PM +0200, Wolfram Sang wrote: >>>>>>> >>>>>>>> +- pinctrl >>>>>>>> + add extra pinctrl to configure SCL/SDA pins to GPIO function for bus >>>>>>>> + recovery, call it "gpio" or "recovery" state >>>>>>> >>>>>>> I think we should stick with "gpio" only. That is what at91 and imx have >>>>>>> in their bindings. pxa uses "recovery" as a pinctrl state name but I >>>>>>> can't find any further use or documentation of that. PXA is not fully >>>>>>> converted to the best of my knowledge, so maybe it is no problem for PXA >>>>>>> to switch to "gpio", too? We should ask Russell King (cced). >>>>> >>>>> Fully converted to what? The generic handling where the i2c core layer >>>>> handles everything to do with recovery, including the switch between >>>>> modes? >>>>> >>>>> i2c-pxa _intentionally_ carefully handles the switch between i2c mode and >>>>> GPIO mode, and I don't see a generic driver doing that to avoid causing >>>>> any additional glitches on the bus. Given the use case that this recovery >>>>> is targetted at, avoiding glitches is very important to keep. >>>> >>>> Why is it not possbile to handle glitches in a generic way? I guess it >>>> depends on the pinctl, but we could treat a worst-case scenario to >>>> assure the switch between states is done properly. >>> >>> Please look at how i2c-pxa switches between the two, and decide whether >>> the generic implementation can do the same. >> >> The handling of glitches from initialization looks generic to me. I see >> that there are specific clear/reset routines that are in the >> (un)prepare_recovery() callbacks, but these callbacks are not replaced >> by the generic i2c recovery and will still be used if given by the >> driver. The only thing the generic recovery does is to switch the pinmux >> state. We can discuss whether we want to change the pinmux state first >> or call the (un)preapre_recovery(). > > Right, the key point i2c-pxa does is that on prepare: > - read the current state of the SCL and SDA lines and set the GPIO to > reflect those values. > - then switch the pinmux state. > > That must be preserved, otherwise if SCL is being held low by the I2C > master, and we switch to GPIO mode, SCL will be released. So the > driver needs to be involved before the pinmux state is changed. I understand, and I admit that I didn't see this case. In my mind, the master would always be in (almost) a reset state before calling for SDA recovery, so it won't hold any lines. These steps can't be generic, of course. Also, not all I2C masters have a way to show the state of its lines. For these masters, one idea would be to reset them before calling i2c_recover_bus() > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! >
