On Wed, May 06, 2020 at 04:00:54PM +0200, Andrew Lunn wrote:
> On Wed, May 06, 2020 at 10:40:31AM +0100, Russell King wrote:
> > Implement generic GPIO-based I2C bus recovery for the PXA I2C driver.
> >
> > Signed-off-by: Russell King <rmk+kernel@xxxxxxxxxxxxxxx>
> > ---
> > drivers/i2c/busses/i2c-pxa.c | 176 +++++++++++++++++++++++++++++++----
> > 1 file changed, 159 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
> > index 0e194d6cd1b5..da15ab2a519e 100644
> > --- a/drivers/i2c/busses/i2c-pxa.c
> > +++ b/drivers/i2c/busses/i2c-pxa.c
> > @@ -20,6 +20,7 @@
> > #include <linux/delay.h>
> > #include <linux/err.h>
> > #include <linux/errno.h>
> > +#include <linux/gpio/consumer.h>
> > #include <linux/i2c.h>
> > #include <linux/init.h>
> > #include <linux/interrupt.h>
> > @@ -28,6 +29,7 @@
> > #include <linux/module.h>
> > #include <linux/of.h>
> > #include <linux/of_device.h>
> > +#include <linux/pinctrl/consumer.h>
> > #include <linux/platform_device.h>
> > #include <linux/platform_data/i2c-pxa.h>
> > #include <linux/slab.h>
> > @@ -260,6 +262,11 @@ struct pxa_i2c {
> > bool highmode_enter;
> > u32 fm_mask;
> > u32 hs_mask;
> > +
> > + struct i2c_bus_recovery_info recovery;
> > + struct pinctrl *pinctrl;
> > + struct pinctrl_state *pinctrl_default;
> > + struct pinctrl_state *pinctrl_recovery;
> > };
> >
> > #define _IBMR(i2c) ((i2c)->reg_ibmr)
> > @@ -559,13 +566,8 @@ static void i2c_pxa_set_slave(struct pxa_i2c *i2c, int errcode)
> > #define i2c_pxa_set_slave(i2c, err) do { } while (0)
> > #endif
> >
> > -static void i2c_pxa_reset(struct pxa_i2c *i2c)
> > +static void i2c_pxa_do_reset(struct pxa_i2c *i2c)
> > {
> > - pr_debug("Resetting I2C Controller Unit\n");
> > -
> > - /* abort any transfer currently under way */
> > - i2c_pxa_abort(i2c);
> > -
> > /* reset according to 9.8 */
> > writel(ICR_UR, _ICR(i2c));
> > writel(I2C_ISR_INIT, _ISR(i2c));
> > @@ -584,12 +586,25 @@ static void i2c_pxa_reset(struct pxa_i2c *i2c)
> > #endif
> >
> > i2c_pxa_set_slave(i2c, 0);
> > +}
> >
> > +static void i2c_pxa_enable(struct pxa_i2c *i2c)
> > +{
> > /* enable unit */
> > writel(readl(_ICR(i2c)) | ICR_IUE, _ICR(i2c));
> > udelay(100);
> > }
> >
> > +static void i2c_pxa_reset(struct pxa_i2c *i2c)
> > +{
> > + pr_debug("Resetting I2C Controller Unit\n");
>
> Hi Russell
>
> I know you are just moving code around, but maybe pr_debug() could be
> cleaned up to dev_debug()?
Yes, but I don't want to add irrelevant changes into this; changing
the way the driver prints stuff wouldn't be part of adding recovery
support, and would likely get review comments suggesting it should
be a separate patch.
> > + /*
> > + * Claiming GPIOs can change the pinmux state, which confuses the
> > + * pinctrl since since pinctrl's idea of the current setting is
>
> since since
>
> I don't know too much about this hardware, but for what it is worth,
>
> Reviewed-by: Andrew Lunn <andrew@xxxxxxx>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
