Re: [PATCH v2 2/3] i2c: pxa: prevent calling of the generic recovery init code

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Sun, Aug 17, 2025 at 04:59:22PM +0200, Gabor Juhos wrote:
> 2025. 08. 13. 17:28 keltezéssel, Russell King (Oracle) írta:
> > On Wed, Aug 13, 2025 at 05:17:28PM +0200, Gabor Juhos wrote:
> >> 2025. 08. 13. 15:10 keltezéssel, Andy Shevchenko írta:
> >>> On Wed, Aug 13, 2025 at 12:36:45PM +0200, Gabor Juhos wrote:
> >>>> 2025. 08. 11. 22:26 keltezéssel, Andy Shevchenko írta:
> >>>>> On Mon, Aug 11, 2025 at 09:49:56PM +0200, Gabor Juhos wrote:
> >>>
> >>> ...
> >>>
> >>>>> TBH this sounds to me like trying to hack the solution and as you pointed out
> >>>>> the problem is in pinctrl state changes. I think it may affect not only I2C case.
> >>>>
> >>>> It is not an error in the pinctrl code. I have checked and even fixed a few bugs
> >>>> in that.
> >>>>
> >>>>> And I didn't get how recovery code affects the initialisation (enumeration).
> >>>>
> >>>> Without the fix, it is not possible to initiate a transaction on the bus, which
> >>>> in turn prevents enumeration.
> >>>
> >>> But why? As you said below the first pin control state is changed during the
> >>> probe, which is fine, and the culprit one happens on the recovery.
> >>
> >> Erm, no. Both happens during probe, before the I2C core tries to enumerate the
> >> devices on the bus.
> >>
> >>> Why is recovery involved in probe? This is quite confusing...
> >> Let me try to explain it differently. Here is the simplified call chain:
> >>
> >>   i2c_pxa_probe()
> >>      ...
> >>      i2c_pxa_init_recovery()
> >>         pinctrl_select_state()                  <- selects GPIO state
> >>         pinctrl_select_state()                  <- selects default (I2C) state
> >>      ...
> >>      i2c_add_numbered_adapter()
> >>          i2c_register_adapter()
> >>              ...
> >>              i2c_init_recovery()
> >>                  i2c_gpio_init_recovery()
> >>                      i2c_gpio_init_generic_recovery()
> >>                          pinctrl_select_state() <- selects GPIO state***
> >>                          ...
> >>                          pinctrl_select_state() <- selects default (I2C) state
> >>              ...
> >>              bus_for_each_drv()
> >>                  __process_new_adapter()
> >>                      i2c_do_add_adapter()
> >>                          i2c_detect()           <- enumerates the devices
> >>
> >> The culprit is the first pinctrl_select_state() call in
> >> i2c_gpio_init_generic_recovery() marked with '***'.
> >>
> >> That call causes the controller to go stuck, which makes it impossible to
> >> transfer anything on the bus.
> > 
> > Probably because when GPIO state is selected, the I2C bus pins end up
> > being set low, which the I2C controller sees, so it thinks there's
> > another device communicating on the bus.
> 
> Yes, it seems so.
> 
> When GPIO state is selected, the bits in the Bus Monitor register which are
> continuously reflecting the value of the SCL and SDA pins contains zeros.
> 
> Additionally, the Status register indicates an 'Early Bus Busy' condition, which
> means that 'The SCL or SDA line is low, without a Start condition'.
> 
> 
> > I could be wrong, as I don't have the hardware to hand to research
> > the issue again.
> > 
> > I have a vague memory that the GPIO state must _always_ reflect the
> > actual pin state before switching to it to avoid glitches and avoid
> > inadvertently changing the I2C controller state.
> 
> Unfortunately, it only helps to avoid glitches on the external lines. At least,
> in the current case the controller hungs no matter which value combination is
> being set on the GPIO pins before switching to GPIO state.

Note that my original i2c-pxa recovery implementation was proven
functional on the uDPU, both by myself and Telus.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!