Hello,
On Mon, Sep 21, 2015 at 04:29:20AM +0000, Gao Pandy wrote:
> > On Fri, Sep 11, 2015 at 06:42:34PM +0800, Gao Pan wrote:
> > > V5:
> > > -introduce a dedicated gpio state for bus recovery.
> > > -assign adapter.bus_recovery_info after the two gpios were aquired
> > > successfully.
> >
> > You also do this if the gpios were not acquired successfully.
>
> Thanks. If the gpios are not acquired successfully, the context goes
> to label clk_disable. So the assignment is skipped.
> Please see the following code.
>
> ......
>
> if (IS_ERR(i2c_imx->pins.sda)) {
> ret = PTR_ERR(i2c_imx->pins.sda);
> goto clk_disable;
> }
> if (IS_ERR(i2c_imx->pins.scl)) {
> ret = PTR_ERR(i2c_imx->pins.scl);
> goto clk_disable;
> }
>
> i2c_imx->adapter.bus_recovery_info = &i2c_imx_bus_recovery_info;
>
> ......
Right, if devm_gpiod_get_optional returns an error probing fails and
everything is right. If however devm_gpiod_get_optional returns NULL
(i.e. the dt doesn't contain an scl-gpios property) you happily use them
even though gpio_set_value are stubs in this case.
> > IMHO pinctrl_lookup_state returning an error is enough to not try a
> > recovery.
>
> Thanks, you are right. How about the following logic.
>
> if (!(IS_ERR(i2c_imx->pinctrl_pins_default)) && !(IS_ERR(i2c_imx->pinctrl_pins_gpio))) {
After the ! you don't need a (, but apart from this the logic is fine.
> i2c_imx->pins.sda =
> devm_gpiod_get_optional(&pdev->dev, "sda-gpios", GPIOD_IN);
> i2c_imx->pins.scl =
> devm_gpiod_get_optional(&pdev->dev, "scl-gpios", GPIOD_IN);
>
> if (IS_ERR(i2c_imx->pins.sda)) {
> ret = PTR_ERR(i2c_imx->pins.sda);
> goto clk_disable;
> }
>
> if (IS_ERR(i2c_imx->pins.scl)) {
> ret = PTR_ERR(i2c_imx->pins.scl);
> goto clk_disable;
> }
As said above, here you need an
if (i2c_imx->pins.scl && i2c_imx->pins.sda)
> i2c_imx->adapter.bus_recovery_info = &i2c_imx_bus_recovery_info;
> }
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
