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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aKH6_1MRqD24QTq5@shell.armlinux.org.uk>
Date: Sun, 17 Aug 2025 16:53:35 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Gabor Juhos <j4g8y7@...il.com>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Wolfram Sang <wsa@...nel.org>,
	Wolfram Sang <wsa+renesas@...g-engineering.com>,
	Andi Shyti <andi.shyti@...nel.org>, Andrew Lunn <andrew@...n.ch>,
	Hanna Hawa <hhhawa@...zon.com>,
	Robert Marko <robert.marko@...tura.hr>,
	Linus Walleij <linus.walleij@...aro.org>, linux-i2c@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Imre Kaloz <kaloz@...nwrt.org>, stable@...r.kernel.org
Subject: Re: [PATCH v2 2/3] i2c: pxa: prevent calling of the generic recovery
 init code

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!