Re: [Patch v1] i2c: imx: implement bus recovery

["Jan Lübbe" <jlu-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]

On Fr, 2015-07-17 at 02:26 +0000, Gao Pandy wrote:
> From: Jan Lübbe <mailto:jlu-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> Sent: Thursday, July 16, 2015 8:58 PM
> > To: Gao Pan-B54642
> > Cc: Uwe Kleine-König; wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org; linux-i2c-u79uwXL29TZUIDd8j+nm9g@public.gmane.org.org; Li
> > Frank-B20596; Duan Fugang-B38611
> > Subject: Re: [Patch v1] i2c: imx: implement bus recovery
> > 
> > On Di, 2015-07-14 at 08:12 +0000, Gao Pandy wrote:
> > > > > --- a/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> > > > > +++ b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> > > > > @@ -14,6 +14,8 @@ Optional properties:
> > > > >    The absence of the propoerty indicates the default frequency
> > > 100 kHz.
> > > > >  - dmas: A list of two dma specifiers, one for each entry in
> > > dma-names.
> > > > >  - dma-names: should contain "tx" and "rx".
> > > > > +- recover-scl: specify the gpio related to SCL pin
> > > > > +- recover-sda: specify the gpio related to SDA pin
> > > > I don't like the naming here. That the gpios are used for recovery
> > > isn't
> > > > a hardware description. What about "scl-gpio" and "sda-gpio"?
> > >
> > > Thanks. I will change it in the next version.
> > 
> > There is already a binding for i2c-gpio:
> > Documentation/devicetree/bindings/i2c/i2c-gpio.txt
> > 
> > We should be able to reuse those properties as-is, instead of defining
> > something for each i2c controller binding.
> 
> Thanks. The hardware description in i2c-gpio.txt describes the case of 
> i2c gpio. However, in our patch, we just want the sda and scl pins change
> to gpio mode for bus recovery, after recovery, we set the two pins to i2c function,
> i2c host controller will take over the transfer. So it's a different case.

Please note that the devicetree binding should describe the hardware,
not what Linux is doing with the hardware. And on the HW-level I2C
recovery with GPIOs is basically the same as using the GPIOs for normal
I2C.

We don't want to have different DT bindings for every I2C controller
which needs to use GPIOs for recovery. We already know that the existing
i2c-gpio bindings are enough to do normal I2C transfers, so they should
also be good for the recovery case.

You also need to add devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org to CC for patches
changing DT bindings.

> Once we use the hardware description in i2c-gpio.txt, additional gpios should
> be applied. Correspondingly, it also need plentiful code support.

Which additional GPIOs do you mean?

By having a common binding for recovery (based on or identical to
i2c-gpios), we can have common setup code for GPIO recovery instead of
duplicating the DT parsing and setup code in each driver. We probably
should define the pinmux state name which is used for GPIO mode as
well. 

Regards,
Jan
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |