Re: pinctrl: sx150x bug
From: Linus Walleij
Date: Thu Aug 06 2020 - 05:43:39 EST
Hi Martin,
thanks for your report, let's check with Peter, Neil and Andrey who also
use this expander if they also see this problem (CC).
On Wed, Aug 5, 2020 at 11:28 AM Martin DEVERA <devik@xxxxxxxxxx> wrote:
> I encountered bug in SX1502 expander driver in 5.7.7. Here is relevant
> DTS part:
>
> compatible = "semtech,sx1502q";
> gpio4_cfg_pins: gpio2-cfg {
> pins = "gpio5";
> output-high;
> };
>
> And part of OOPS:
>
> [ 0.673996] [<c023cfa6>] (gpiochip_get_data) from [<c023b235>]
> (sx150x_gpio_direction_output+0xd)
> [ 0.683259] [<c023b235>] (sx150x_gpio_direction_output) from
> [<c023b363>] (sx150x_pinconf_set+0x)
> [ 0.692796] [<c023b363>] (sx150x_pinconf_set) from [<c0238fef>]
> (pinconf_apply_setting+0x39/0x7e)
> [ 0.701635] [<c0238fef>] (pinconf_apply_setting) from [<c0236c77>]
> (pinctrl_commit_state+0xa5/0x)
> [ 0.710648] [<c0236c77>] (pinctrl_commit_state) from [<c0237e03>]
> (pinctrl_enable+0xff/0x1d4)
> [ 0.719139] [<c0237e03>] (pinctrl_enable) from [<c023b791>]
> (sx150x_probe+0x1a3/0x358)
> [ 0.727027] [<c023b791>] (sx150x_probe) from [<c02c38bf>]
> (i2c_device_probe+0x1bb/0x1dc)
>
> The problem is that sx150x_pinconf_set uses sx150x_gpio_direction_output
> but gpio is not
> setup yet. Patch below fixes it but I'm not sure whether is it correct:
>
> diff --git a/drivers/pinctrl/pinctrl-sx150x.c
> b/drivers/pinctrl/pinctrl-sx150x.c
> index 6e74bd87d959..3f5651edd336 100644
> --- a/drivers/pinctrl/pinctrl-sx150x.c
> +++ b/drivers/pinctrl/pinctrl-sx150x.c
> @@ -1154,12 +1154,6 @@ static int sx150x_probe(struct i2c_client *client,
> return ret;
> }
>
> - ret = pinctrl_enable(pctl->pctldev);
> - if (ret) {
> - dev_err(dev, "Failed to enable pinctrl device\n");
> - return ret;
> - }
> -
> /* Register GPIO controller */
> pctl->gpio.base = -1;
> pctl->gpio.ngpio = pctl->data->npins;
> @@ -1191,6 +1185,12 @@ static int sx150x_probe(struct i2c_client *client,
> if (ret)
> return ret;
>
> + ret = pinctrl_enable(pctl->pctldev);
> + if (ret) {
> + dev_err(dev, "Failed to enable pinctrl device\n");
> + return ret;
> + }
> +
> ret = gpiochip_add_pin_range(&pctl->gpio, dev_name(dev),
> 0, 0, pctl->data->npins);
> if (ret)
I don't see any problem with the patch, can you send a proper patch
with git-send-email so we can test it and apply it if it works for the
other users? Include the mentioned people on CC.
Yours,
Linus Walleij