From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Robert Marko <robert.marko@sartura.hr>
Cc: wsa@kernel.org, codrin.ciubotariu@microchip.com,
linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
luka.perkov@sartura.hr
Subject: Re: [RFC PATCH] i2c: core: dont change pinmux state to GPIO during recovery setup
Date: Wed, 6 Sep 2023 17:55:56 +0100 [thread overview]
Message-ID: <ZPivHKd0LWWnhPr/@shell.armlinux.org.uk> (raw)
In-Reply-To: <CA+HBbNEM6AfwX87DLRNAuJSWPKboGuuJJDRK_E+G3sJDF73oZA@mail.gmail.com>
On Wed, Sep 06, 2023 at 04:41:33PM +0200, Robert Marko wrote:
> On Fri, Sep 1, 2023 at 1:49 PM Robert Marko <robert.marko@sartura.hr> wrote:
> >
> > Ever since PXA I2C driver was moved to the generic I2C recovery, I2C has
> > stopped working completely on Armada 3720 if the pins are specified in DTS.
> >
> > After a while it was traced down to the only difference being that PXA
> > driver did not change the pinmux state to GPIO before trying to acquire the
> > GPIO pins.
> > And indeed as soon as this call is removed I2C starts working.
> >
> > To me it seems that this call is not required at all as devm_gpiod_get()
> > will result in the pinmux state being changed to GPIO via the pinmux
> > set_mux() op.
> >
> > Fixes: 0b01392c18b9 ("i2c: pxa: move to generic GPIO recovery")
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > ---
> > I am aware this probably isnt the correct fix, so I am sending it as RFC
> > cause I have ran out of ideas.
>
> CC-ing Russel as well since I forgot him.
So the generic recovery decided to set the pinmux state before calling
devm_gpiod_get(), where as the driver (and my code) originally did this
after calling devm_gpiod_get():
- /*
- * Claiming GPIOs can change the pinmux state, which confuses the
- * pinctrl since pinctrl's idea of the current setting is unaffected
- * by the pinmux change caused by claiming the GPIO. Work around that
- * by switching pinctrl to the GPIO state here. We do it this way to
- * avoid glitching the I2C bus.
- */
- pinctrl_select_state(i2c->pinctrl, i2c->pinctrl_recovery);
-
- return pinctrl_select_state(i2c->pinctrl, i2c->pinctrl_default);
I'd suggest re-implementing my original scheme in the generic code
because this _does_ work on Armada 3720 hardware.
Removing the pinmux frobbing is likely to break stuff.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
next prev parent reply other threads:[~2023-09-06 16:56 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-01 11:49 [RFC PATCH] i2c: core: dont change pinmux state to GPIO during recovery setup Robert Marko
2023-09-06 14:41 ` Robert Marko
2023-09-06 16:55 ` Russell King (Oracle) [this message]
2023-09-13 18:49 ` 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=ZPivHKd0LWWnhPr/@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=codrin.ciubotariu@microchip.com \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luka.perkov@sartura.hr \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox