Messages in this thread | | | Date | Wed, 5 Aug 2020 10:36:29 +0200 | From | <> | Subject | Re: [PATCH 2/4] i2c: core: add generic I2C GPIO recovery |
| |
On Tue, Aug 04, 2020 at 12:59:24PM +0300, Codrin Ciubotariu wrote: > Multiple I2C bus drivers use similar bindings to obtain information needed > for I2C recovery. For example, for platforms using device-tree, the > properties look something like this: > > &i2c { > ... > pinctrl-names = "default", "gpio"; > pinctrl-0 = <&pinctrl_i2c_default>; > pinctrl-1 = <&pinctrl_i2c_gpio>; > sda-gpios = <&pio 0 GPIO_ACTIVE_HIGH>; > scl-gpios = <&pio 1 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; > ... > } > > For this reason, we can add this common initialization in the core. This > way, other I2C bus drivers will be able to support GPIO recovery just by > providing a pointer to platform's pinctrl and calling i2c_recover_bus() > when SDA is stuck low. > > Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com> > --- Applied to for-next, thanks! Two minor change:
> + /* for pinctrl state changes, we need all the information */ > + if (!bri->pins_default || !bri->pins_gpio) { > + bri->pinctrl = NULL; > + bri->pins_default = NULL; > + bri->pins_gpio = NULL; > + } else { > + dev_info(dev, "using pinctrl states for GPIO recovery"); > + }
I inverted the logic here:
294 /* for pinctrl state changes, we need all the information */ 295 if (bri->pins_default && bri->pins_gpio) { 296 dev_info(dev, "using pinctrl states for GPIO recovery"); 297 } else { 298 bri->pinctrl = NULL; 299 bri->pins_default = NULL; 300 bri->pins_gpio = NULL; 301 }
I think it is a bit easier to read if the desired path is not in the else case.
> + * @pins_default: default state of SCL/SDA lines, when they are assigned to the > + * I2C bus. Optional. Populated internally for GPIO recovery, if a state with > + * the name PINCTRL_STATE_DEFAULT is found and pinctrl is valid. > + * @pins_gpio: recovery state of SCL/SDA lines, when they are used as GPIOs. > + * Optional. Populated internally for GPIO recovery, if this state is called > + * "gpio" or "recovery" and pinctrl is valid.
Added "pinctrl" to "state of SCL/SDA" to make it clear.
[unhandled content-type:application/pgp-signature] | |