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

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

quoted

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:

quoted

quoted

quoted

--- 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.

quoted

quoted

 - dmas: A list of two dma specifiers, one for each entry in

dma-names.

quoted

quoted

 - 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

quoted

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 |