Hello,
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)
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 <[email protected]> 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
On Thu, Aug 6, 2020 at 2:41 AM Linus Walleij <[email protected]> wrote:
>
> 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).
>
Looks reasonable. I haven't used that HW in a while, so I added Chris
Healy to this thread, since he might be in a better position to
comment on this and maybe even provide a Tested-by
> On Wed, Aug 5, 2020 at 11:28 AM Martin DEVERA <[email protected]> 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
On Wed, Aug 5, 2020 at 11:28 AM Martin DEVERA <[email protected]> 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:
I simply rebased and applied this patch now.
Yours,
Linus Walleij