On Fr, 2015-07-17 at 02:26 +0000, Gao Pandy wrote: > From: Jan Lübbe <mailto:jlu@xxxxxxxxxxxxxx> Sent: Thursday, July 16, 2015 8:58 PM > > To: Gao Pan-B54642 > > Cc: Uwe Kleine-König; wsa@xxxxxxxxxxxxx; linux-i2c@xxxxxxxxxxxxxxx; 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@xxxxxxxxxxxxxxx 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 | -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
