| Message ID | 20170511151647.GT3489@atomide.com |
|---|---|
| State | New |
| Headers | show |
> Hmm maybe yeah. I don't quite follow the above the "pinctrl-0 property > of sx150x device tree node, is misinterpreted as hog" part though. sx150x is i2c-gpio device. It has 16 GPIO lines that are communicated with via i2c bus, and an interrupt line. Interrupt line is typically connected to SoC's pin. This pin has to be configured. This is done by providing appropriate subnode in SoC's pinmux node, with information with pin configuration, and pinctrl-0 property in sx150x's node with phandle to that subnode: ... &i2c0 { sx1503@20 { compatible = "semtech,sx1503q"; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_sx1503_20>; ... }; }; ... &iomuxc { pinctrl_sx1503_20: pinctrl-sx1503-20 { fsl,pins = < VF610_PAD_PTB1__GPIO_23 0x219d >; }; }; This pin configuration is handled by driver core, i.e. before probe() for sx150x is called, core applies pin configuration. However sx150x driver is currently implemented as a pinctrl driver. When it initializes, pinctrl searches for "hog", i.e. pin config that should be applied at driver registration time. While doing so, core searches for any registered pinctrl_map for device being register. Search loop is in create_pinctrl(). In this case, this loop finds map that is defined above. This is *not* hog. This is pin setting already applied in SoC's pinmux controller for sx1503 device. However code in create_pinctrl() tries to apply it, and use sx1503's methods to do so. Which is plain wrong and errors out. > But at least with updating the probe to use pinctrl_register_and_init() > and pinctrl_enable() the driver can do something before the hogs are > claimed. I just don't know what the driver would here as I don't > understand the "misinterpreted as hog" part :) Tried to explain above :) I don't know whar can be done in sx1503 driver to avoid that scenario... perhaps port it back from pinctrl subsystem to gpio subsystem. However I guess it was ported to pinctrl subsystem for a reason (that I don't know). > Anyways, care to test if the following patch fixes the issue somehow? No that patch does not help here. Nikita -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Nikita Yushchenko <nikita.yoush@cogentembedded.com> [170511 09:27]: > > Hmm maybe yeah. I don't quite follow the above the "pinctrl-0 property > > of sx150x device tree node, is misinterpreted as hog" part though. > > sx150x is i2c-gpio device. It has 16 GPIO lines that are communicated > with via i2c bus, and an interrupt line. > > Interrupt line is typically connected to SoC's pin. > This pin has to be configured. > This is done by providing appropriate subnode in SoC's pinmux node, with > information with pin configuration, and pinctrl-0 property in sx150x's > node with phandle to that subnode: > > ... > &i2c0 { > sx1503@20 { > compatible = "semtech,sx1503q"; > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_sx1503_20>; > ... > }; > }; > ... > &iomuxc { > pinctrl_sx1503_20: pinctrl-sx1503-20 { > fsl,pins = < > VF610_PAD_PTB1__GPIO_23 0x219d > >; > }; > }; > > This pin configuration is handled by driver core, i.e. before probe() > for sx150x is called, core applies pin configuration. > > However sx150x driver is currently implemented as a pinctrl driver. > > When it initializes, pinctrl searches for "hog", i.e. pin config that > should be applied at driver registration time. > > While doing so, core searches for any registered pinctrl_map for device > being register. Search loop is in create_pinctrl(). > > In this case, this loop finds map that is defined above. > > This is *not* hog. This is pin setting already applied in SoC's pinmux > controller for sx1503 device. > > However code in create_pinctrl() tries to apply it, and use sx1503's > methods to do so. Which is plain wrong and errors out. Maybe create_pinctrl() could check if the pin controller device for a potential hog points to the device itself and bail out if that's not the case? > > But at least with updating the probe to use pinctrl_register_and_init() > > and pinctrl_enable() the driver can do something before the hogs are > > claimed. I just don't know what the driver would here as I don't > > understand the "misinterpreted as hog" part :) > > Tried to explain above :) Yup OK based on that this seems like a pinctrl core issue. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Hmm maybe yeah. I don't quite follow the above the "pinctrl-0 property >>> of sx150x device tree node, is misinterpreted as hog" part though. >> >> sx150x is i2c-gpio device. It has 16 GPIO lines that are communicated >> with via i2c bus, and an interrupt line. >> >> Interrupt line is typically connected to SoC's pin. >> This pin has to be configured. >> This is done by providing appropriate subnode in SoC's pinmux node, with >> information with pin configuration, and pinctrl-0 property in sx150x's >> node with phandle to that subnode: >> >> ... >> &i2c0 { >> sx1503@20 { >> compatible = "semtech,sx1503q"; >> pinctrl-names = "default"; >> pinctrl-0 = <&pinctrl_sx1503_20>; >> ... >> }; >> }; >> ... >> &iomuxc { >> pinctrl_sx1503_20: pinctrl-sx1503-20 { >> fsl,pins = < >> VF610_PAD_PTB1__GPIO_23 0x219d >> >; >> }; >> }; >> >> This pin configuration is handled by driver core, i.e. before probe() >> for sx150x is called, core applies pin configuration. >> >> However sx150x driver is currently implemented as a pinctrl driver. >> >> When it initializes, pinctrl searches for "hog", i.e. pin config that >> should be applied at driver registration time. >> >> While doing so, core searches for any registered pinctrl_map for device >> being register. Search loop is in create_pinctrl(). >> >> In this case, this loop finds map that is defined above. >> >> This is *not* hog. This is pin setting already applied in SoC's pinmux >> controller for sx1503 device. >> >> However code in create_pinctrl() tries to apply it, and use sx1503's >> methods to do so. Which is plain wrong and errors out. > > Maybe create_pinctrl() could check if the pin controller device > for a potential hog points to the device itself and bail out > if that's not the case? Well that's exactly what patch from my first mail in this thread does. This indeed fixes my case, but I don't know if it is correct in generic case. Should I submit it? Do you ack? Nikita -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Nikita Yushchenko <nikita.yoush@cogentembedded.com> [170511 10:01]: > >>> Hmm maybe yeah. I don't quite follow the above the "pinctrl-0 property > >>> of sx150x device tree node, is misinterpreted as hog" part though. > >> > >> sx150x is i2c-gpio device. It has 16 GPIO lines that are communicated > >> with via i2c bus, and an interrupt line. > >> > >> Interrupt line is typically connected to SoC's pin. > >> This pin has to be configured. > >> This is done by providing appropriate subnode in SoC's pinmux node, with > >> information with pin configuration, and pinctrl-0 property in sx150x's > >> node with phandle to that subnode: > >> > >> ... > >> &i2c0 { > >> sx1503@20 { > >> compatible = "semtech,sx1503q"; > >> pinctrl-names = "default"; > >> pinctrl-0 = <&pinctrl_sx1503_20>; > >> ... > >> }; > >> }; > >> ... > >> &iomuxc { > >> pinctrl_sx1503_20: pinctrl-sx1503-20 { > >> fsl,pins = < > >> VF610_PAD_PTB1__GPIO_23 0x219d > >> >; > >> }; > >> }; > >> > >> This pin configuration is handled by driver core, i.e. before probe() > >> for sx150x is called, core applies pin configuration. > >> > >> However sx150x driver is currently implemented as a pinctrl driver. > >> > >> When it initializes, pinctrl searches for "hog", i.e. pin config that > >> should be applied at driver registration time. > >> > >> While doing so, core searches for any registered pinctrl_map for device > >> being register. Search loop is in create_pinctrl(). > >> > >> In this case, this loop finds map that is defined above. > >> > >> This is *not* hog. This is pin setting already applied in SoC's pinmux > >> controller for sx1503 device. > >> > >> However code in create_pinctrl() tries to apply it, and use sx1503's > >> methods to do so. Which is plain wrong and errors out. > > > > Maybe create_pinctrl() could check if the pin controller device > > for a potential hog points to the device itself and bail out > > if that's not the case? > > Well that's exactly what patch from my first mail in this thread does. > This indeed fixes my case, but I don't know if it is correct in generic > case. OK, yeah I just looked it up as I was not in cc. > Should I submit it? Do you ack? Yeah please submit a proper patch. I assume you already checked that this change only affects the pinctrl hogs only, not regular device pins? I'd assume so as it's in create_pinctrl().. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Maybe create_pinctrl() could check if the pin controller device >>> for a potential hog points to the device itself and bail out >>> if that's not the case? >> >> Well that's exactly what patch from my first mail in this thread does. >> This indeed fixes my case, but I don't know if it is correct in generic >> case. > > OK, yeah I just looked it up as I was not in cc. > >> Should I submit it? Do you ack? > > Yeah please submit a proper patch. I assume you already checked > that this change only affects the pinctrl hogs only, not regular > device pins? I'd assume so as it's in create_pinctrl().. Regular device pins go via pinctrl_get() which calls create_pinctrl() with second argument set to NULL. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 11, 2017 at 7:51 PM, Tony Lindgren <tony@atomide.com> wrote: > * Nikita Yushchenko <nikita.yoush@cogentembedded.com> [170511 10:01]: >> Well that's exactly what patch from my first mail in this thread does. >> This indeed fixes my case, but I don't know if it is correct in generic >> case. > > OK, yeah I just looked it up as I was not in cc. > >> Should I submit it? Do you ack? > > Yeah please submit a proper patch. I assume you already checked > that this change only affects the pinctrl hogs only, not regular > device pins? I'd assume so as it's in create_pinctrl().. Thanks for sorting this out, both of you. Much appreciated! Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pinctrl/pinctrl-sx150x.c b/drivers/pinctrl/pinctrl-sx150x.c --- a/drivers/pinctrl/pinctrl-sx150x.c +++ b/drivers/pinctrl/pinctrl-sx150x.c @@ -1225,13 +1225,16 @@ static int sx150x_probe(struct i2c_client *client, pctl->pinctrl_desc.npins = pctl->data->npins; pctl->pinctrl_desc.owner = THIS_MODULE; - pctl->pctldev = pinctrl_register(&pctl->pinctrl_desc, dev, pctl); - if (IS_ERR(pctl->pctldev)) { - dev_err(dev, "Failed to register pinctrl device\n"); - return PTR_ERR(pctl->pctldev); + ret = pinctrl_register_and_init(&pctl->pinctrl_desc, dev, + pctl, &pctl->pctldev); + if (ret) { + dev_err(dev, "Failed to register pinctrl\n"); + return ret; } - return 0; + /* REVISIT: Do something with pinctrl-0 misinterpreted as hog? */ + + return pinctrl_enable(pctl->pctldev); } static struct i2c_driver sx150x_driver = {