On Wed, Apr 15, 2020 at 10:06:43AM +0300, Codrin Ciubotariu wrote:
> devm_gpiod_get() usually calls gpio_request_enable() for non-strict pinmux
> drivers. These puts the pins in GPIO mode, whithout notifying the pinctrl
> driver. At this point, the I2C bus no longer owns the pins. To mux the
> pins back to the I2C bus, we use the pinctrl driver to change the state
> of the pins to GPIO, before using devm_gpiod_get(). After the pins are
> received as GPIOs, we switch theer pinctrl state back to the default
> one,
>
> Fixes: d3d3fdcc4c90 ("i2c: at91: implement i2c bus recovery")
> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@xxxxxxxxxxxxx>
At the moment, I don't see another way to deal with this issue.
Link to the discussion:
https://lore.kernel.org/linux-arm-kernel/20191206173343.GX25745@xxxxxxxxxxxxxxxxxxxxx/
Acked-by: Ludovic Desroches <ludovic.desroches@xxxxxxxxxxxxx>
> ---
> drivers/i2c/busses/i2c-at91-master.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
> index 0aba51a7df32..43d85845c897 100644
> --- a/drivers/i2c/busses/i2c-at91-master.c
> +++ b/drivers/i2c/busses/i2c-at91-master.c
> @@ -845,6 +845,18 @@ static int at91_init_twi_recovery_info(struct platform_device *pdev,
> PINCTRL_STATE_DEFAULT);
> dev->pinctrl_pins_gpio = pinctrl_lookup_state(dev->pinctrl,
> "gpio");
> + if (IS_ERR(dev->pinctrl_pins_default) ||
> + IS_ERR(dev->pinctrl_pins_gpio)) {
> + dev_info(&pdev->dev, "pinctrl states incomplete for recovery\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * pins will be taken as GPIO, so we might as well inform pinctrl about
> + * this and move the state to GPIO
> + */
> + pinctrl_select_state(dev->pinctrl, dev->pinctrl_pins_gpio);
> +
> rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_IN);
> if (PTR_ERR(rinfo->sda_gpiod) == -EPROBE_DEFER)
> return -EPROBE_DEFER;
> @@ -855,9 +867,7 @@ static int at91_init_twi_recovery_info(struct platform_device *pdev,
> return -EPROBE_DEFER;
>
> if (IS_ERR(rinfo->sda_gpiod) ||
> - IS_ERR(rinfo->scl_gpiod) ||
> - IS_ERR(dev->pinctrl_pins_default) ||
> - IS_ERR(dev->pinctrl_pins_gpio)) {
> + IS_ERR(rinfo->scl_gpiod)) {
> dev_info(&pdev->dev, "recovery information incomplete\n");
> if (!IS_ERR(rinfo->sda_gpiod)) {
> gpiod_put(rinfo->sda_gpiod);
> @@ -870,6 +880,9 @@ static int at91_init_twi_recovery_info(struct platform_device *pdev,
> return -EINVAL;
> }
>
> + /* change the state of the pins back to their default state */
> + pinctrl_select_state(dev->pinctrl, dev->pinctrl_pins_default);
> +
> dev_info(&pdev->dev, "using scl, sda for recovery\n");
>
> rinfo->prepare_recovery = at91_prepare_twi_recovery;
> --
> 2.20.1
>
