All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Robert Marko <robert.marko@sartura.hr>,
	wsa@kernel.org, codrin.ciubotariu@microchip.com,
	linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-gpio@vger.kernel.org
Subject: Re: [PATCH] i2c: core: dont change pinmux state to GPIO during recovery setup
Date: Thu, 9 Nov 2023 20:30:02 +0000	[thread overview]
Message-ID: <ZU1BSmyD931BRwSD@shell.armlinux.org.uk> (raw)
In-Reply-To: <CACRpkdYUW-mO6vhh-zkZAuqQOHpwMeJsNw=jSLzbgoEtoCTtNQ@mail.gmail.com>

On Thu, Nov 09, 2023 at 09:04:29PM +0100, Linus Walleij wrote:
> Hi Robert!
> 
> Thanks for getting back on this issue.
> 
> On Thu, Nov 9, 2023 at 8:10 PM Robert Marko <robert.marko@sartura.hr> wrote:
> 
> > Yes, I2C recovery is required on this board as otherwise the I2C bus will
> > get stuck after a certain number of SFP module plug/unplug events or
> > sometimes even just randomly, I2C recovery allows the bus to recover
> > and continue working.
> 
> OK makes sense.
> 
> > Maybe my commit message was confusing, so I will try and explain further.
> > I2C recovery did work on Armada 3720 just fine until the driver was converted
> > to use the generic I2C recovery which is now part of the I2C core.
> >
> > After it was converted to it, the I2C bus completely stopped working
> > on Armada 3720
> > if I2C recovery is enabled by making the recovery pinctrl available in DTS.
> 
> Shouldn't we just revert that patch until we can figure this out then?

Note that when I wrote the i2c-pxa recovery code (which was developed
and tested on Armada 3720 - the uDPU) it had to work... when the
suggestion came up to implement generic recovery, I stated:

http://archive.lwn.net:8080/linux-kernel/20200705210942.GA1055@kunai/T/#mf7f862fcd53245f14fb650d33c29cf139d41039d

> > I then spent quite a while trying to bisect the exact change that
> > causes this issue
> > in the conversion as code is almost identical to what the driver was
> > doing previously,
> > and have bisected it down to pinctrl_select_state(bri->pinctrl,
> > bri->pins_gpio) being
> > called before SDA and SCL pins are obtained via devm_gpiod_get().

Yes, indeed. That's because the pinctrl internals get confused. I sent
you an email about it on 6th December 2019

"pinctrl states vs pinmux vs gpio (i2c bus recovery)"

which is why i2c-pxa did things the way it did in my commit
"i2c: pxa: implement generic i2c bus recovery".

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

  reply	other threads:[~2023-11-09 20:31 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-26 16:01 [PATCH] i2c: core: dont change pinmux state to GPIO during recovery setup Robert Marko
2023-09-28 21:11 ` Linus Walleij
2023-11-09 19:09   ` Robert Marko
2023-11-09 20:04     ` Linus Walleij
2023-11-09 20:30       ` Russell King (Oracle) [this message]
2023-11-09 21:02         ` Linus Walleij
2023-11-09 21:15           ` Robert Marko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZU1BSmyD931BRwSD@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=codrin.ciubotariu@microchip.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robert.marko@sartura.hr \
    --cc=wsa@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.