* [PATCH v7 01/16] pinctrl: check the return value of pinmux_ops::get_function_name()
2025-09-02 11:59 [PATCH v7 00/16] pinctrl: introduce the concept of a GPIO pin function category Bartosz Golaszewski
@ 2025-09-02 11:59 ` Bartosz Golaszewski
2025-09-02 13:06 ` Andy Shevchenko
2025-09-02 14:46 ` [PATCH v7 00/16] pinctrl: introduce the concept of a GPIO pin function category Andy Shevchenko
2025-09-02 22:18 ` Linus Walleij
2 siblings, 1 reply; 11+ messages in thread
From: Bartosz Golaszewski @ 2025-09-02 11:59 UTC (permalink / raw)
To: Linus Walleij, Bjorn Andersson, Konrad Dybcio, Alexey Klimov,
Lorenzo Bianconi, Sean Wang, Matthias Brugger,
AngeloGioacchino Del Regno, Paul Cercueil, Kees Cook,
Andy Shevchenko, Andrew Morton, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Dong Aisheng, Fabio Estevam,
Shawn Guo, Jacky Bai, Pengutronix Kernel Team, NXP S32 Linux Team,
Sascha Hauer, Tony Lindgren, Haojian Zhuang, Geert Uytterhoeven,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Neil Armstrong, Mark Brown
Cc: linux-gpio, linux-kernel, linux-arm-msm, linux-mediatek,
linux-arm-kernel, linux-mips, linux-hardening, linux-mm, imx,
linux-omap, linux-renesas-soc, Bartosz Golaszewski, stable
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
While the API contract in docs doesn't specify it explicitly, the
generic implementation of the get_function_name() callback from struct
pinmux_ops - pinmux_generic_get_function_name() - can fail and return
NULL. This is already checked in pinmux_check_ops() so add a similar
check in pinmux_func_name_to_selector() instead of passing the returned
pointer right down to strcmp() where the NULL can get dereferenced. This
is normal operation when adding new pinfunctions.
Cc: stable@vger.kernel.org
Tested-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/pinctrl/pinmux.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 79814758a084570adea0ea1a3151d186f65d1d1f..07a478b2c48740c24a32e6ac8f10df4876e718e3 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -337,7 +337,7 @@ static int pinmux_func_name_to_selector(struct pinctrl_dev *pctldev,
while (selector < nfuncs) {
const char *fname = ops->get_function_name(pctldev, selector);
- if (!strcmp(function, fname))
+ if (fname && !strcmp(function, fname))
return selector;
selector++;
--
2.48.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v7 01/16] pinctrl: check the return value of pinmux_ops::get_function_name()
2025-09-02 11:59 ` [PATCH v7 01/16] pinctrl: check the return value of pinmux_ops::get_function_name() Bartosz Golaszewski
@ 2025-09-02 13:06 ` Andy Shevchenko
2025-09-02 13:29 ` Bartosz Golaszewski
0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2025-09-02 13:06 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Bjorn Andersson, Konrad Dybcio, Alexey Klimov,
Lorenzo Bianconi, Sean Wang, Matthias Brugger,
AngeloGioacchino Del Regno, Paul Cercueil, Kees Cook,
Andy Shevchenko, Andrew Morton, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Dong Aisheng, Fabio Estevam,
Shawn Guo, Jacky Bai, Pengutronix Kernel Team, NXP S32 Linux Team,
Sascha Hauer, Tony Lindgren, Haojian Zhuang, Geert Uytterhoeven,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Neil Armstrong, Mark Brown, linux-gpio, linux-kernel,
linux-arm-msm, linux-mediatek, linux-arm-kernel, linux-mips,
linux-hardening, linux-mm, imx, linux-omap, linux-renesas-soc,
Bartosz Golaszewski, stable
On Tue, Sep 02, 2025 at 01:59:10PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> While the API contract in docs doesn't specify it explicitly,
So, why not to amend the doc at the same time?
> the generic implementation of the get_function_name() callback from struct
> pinmux_ops - pinmux_generic_get_function_name() - can fail and return
> NULL. This is already checked in pinmux_check_ops() so add a similar
> check in pinmux_func_name_to_selector() instead of passing the returned
> pointer right down to strcmp() where the NULL can get dereferenced. This
> is normal operation when adding new pinfunctions.
Fixes?
Reported?
Closes?
...
> while (selector < nfuncs) {
> const char *fname = ops->get_function_name(pctldev, selector);
>
> - if (!strcmp(function, fname))
> + if (fname && !strcmp(function, fname))
> return selector;
I would slightly refactor this:
const char *fname;
fname = ops->get_function_name(pctldev, selector);
if (fname && !strcmp(function, fname))
return selector;
> selector++;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v7 01/16] pinctrl: check the return value of pinmux_ops::get_function_name()
2025-09-02 13:06 ` Andy Shevchenko
@ 2025-09-02 13:29 ` Bartosz Golaszewski
2025-09-02 13:50 ` Andy Shevchenko
0 siblings, 1 reply; 11+ messages in thread
From: Bartosz Golaszewski @ 2025-09-02 13:29 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, Bjorn Andersson, Konrad Dybcio, Alexey Klimov,
Lorenzo Bianconi, Sean Wang, Matthias Brugger,
AngeloGioacchino Del Regno, Paul Cercueil, Kees Cook,
Andy Shevchenko, Andrew Morton, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Dong Aisheng, Fabio Estevam,
Shawn Guo, Jacky Bai, Pengutronix Kernel Team, NXP S32 Linux Team,
Sascha Hauer, Tony Lindgren, Haojian Zhuang, Geert Uytterhoeven,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Neil Armstrong, Mark Brown, linux-gpio, linux-kernel,
linux-arm-msm, linux-mediatek, linux-arm-kernel, linux-mips,
linux-hardening, linux-mm, imx, linux-omap, linux-renesas-soc,
Bartosz Golaszewski, stable
On Tue, Sep 2, 2025 at 3:06 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Tue, Sep 02, 2025 at 01:59:10PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > While the API contract in docs doesn't specify it explicitly,
>
> So, why not to amend the doc at the same time?
>
Because this series is already big as is. That would be another commit
that can be separate.
> > the generic implementation of the get_function_name() callback from struct
> > pinmux_ops - pinmux_generic_get_function_name() - can fail and return
> > NULL. This is already checked in pinmux_check_ops() so add a similar
> > check in pinmux_func_name_to_selector() instead of passing the returned
> > pointer right down to strcmp() where the NULL can get dereferenced. This
> > is normal operation when adding new pinfunctions.
>
> Fixes?
This has always been like that.
> Reported?
I mean, technically Mark Brown reported my previous patch failing but
I don't think we do this if we're still within the same series just
another iteration?
> Closes?
Ditto.
>
> ...
>
> > while (selector < nfuncs) {
> > const char *fname = ops->get_function_name(pctldev, selector);
> >
> > - if (!strcmp(function, fname))
> > + if (fname && !strcmp(function, fname))
> > return selector;
>
> I would slightly refactor this:
>
> const char *fname;
>
> fname = ops->get_function_name(pctldev, selector);
> if (fname && !strcmp(function, fname))
> return selector;
>
> > selector++;
>
You can do this in a subsequent patch, I prefer a smaller diff personally.
Bartosz
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v7 01/16] pinctrl: check the return value of pinmux_ops::get_function_name()
2025-09-02 13:29 ` Bartosz Golaszewski
@ 2025-09-02 13:50 ` Andy Shevchenko
2025-09-02 14:02 ` Bartosz Golaszewski
0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2025-09-02 13:50 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Bjorn Andersson, Konrad Dybcio, Alexey Klimov,
Lorenzo Bianconi, Sean Wang, Matthias Brugger,
AngeloGioacchino Del Regno, Paul Cercueil, Kees Cook,
Andy Shevchenko, Andrew Morton, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Dong Aisheng, Fabio Estevam,
Shawn Guo, Jacky Bai, Pengutronix Kernel Team, NXP S32 Linux Team,
Sascha Hauer, Tony Lindgren, Haojian Zhuang, Geert Uytterhoeven,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Neil Armstrong, Mark Brown, linux-gpio, linux-kernel,
linux-arm-msm, linux-mediatek, linux-arm-kernel, linux-mips,
linux-hardening, linux-mm, imx, linux-omap, linux-renesas-soc,
Bartosz Golaszewski, stable
On Tue, Sep 02, 2025 at 03:29:31PM +0200, Bartosz Golaszewski wrote:
> On Tue, Sep 2, 2025 at 3:06 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Tue, Sep 02, 2025 at 01:59:10PM +0200, Bartosz Golaszewski wrote:
> > >
> > > While the API contract in docs doesn't specify it explicitly,
> >
> > So, why not to amend the doc at the same time?
>
> Because this series is already big as is. That would be another commit
> that can be separate.
I meant _in the same_ patch.
> > > the generic implementation of the get_function_name() callback from struct
> > > pinmux_ops - pinmux_generic_get_function_name() - can fail and return
> > > NULL. This is already checked in pinmux_check_ops() so add a similar
> > > check in pinmux_func_name_to_selector() instead of passing the returned
> > > pointer right down to strcmp() where the NULL can get dereferenced. This
> > > is normal operation when adding new pinfunctions.
> > Fixes?
>
> This has always been like that.
>
> > Reported?
>
> I mean, technically Mark Brown reported my previous patch failing but
> I don't think we do this if we're still within the same series just
> another iteration?
>
> > Closes?
>
> Ditto.
I meant that this fixes a potential issue disregard to your series, right?
...
> > > while (selector < nfuncs) {
> > > const char *fname = ops->get_function_name(pctldev, selector);
> > >
> > > - if (!strcmp(function, fname))
> > > + if (fname && !strcmp(function, fname))
> > > return selector;
> >
> > I would slightly refactor this:
> >
> > const char *fname;
> >
> > fname = ops->get_function_name(pctldev, selector);
> > if (fname && !strcmp(function, fname))
> > return selector;
> >
> > > selector++;
> >
>
> You can do this in a subsequent patch, I prefer a smaller diff personally.
Sure.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v7 01/16] pinctrl: check the return value of pinmux_ops::get_function_name()
2025-09-02 13:50 ` Andy Shevchenko
@ 2025-09-02 14:02 ` Bartosz Golaszewski
2025-09-02 14:20 ` Andy Shevchenko
0 siblings, 1 reply; 11+ messages in thread
From: Bartosz Golaszewski @ 2025-09-02 14:02 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, Bjorn Andersson, Konrad Dybcio, Alexey Klimov,
Lorenzo Bianconi, Sean Wang, Matthias Brugger,
AngeloGioacchino Del Regno, Paul Cercueil, Kees Cook,
Andy Shevchenko, Andrew Morton, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Dong Aisheng, Fabio Estevam,
Shawn Guo, Jacky Bai, Pengutronix Kernel Team, NXP S32 Linux Team,
Sascha Hauer, Tony Lindgren, Haojian Zhuang, Geert Uytterhoeven,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Neil Armstrong, Mark Brown, linux-gpio, linux-kernel,
linux-arm-msm, linux-mediatek, linux-arm-kernel, linux-mips,
linux-hardening, linux-mm, imx, linux-omap, linux-renesas-soc,
Bartosz Golaszewski, stable
On Tue, Sep 2, 2025 at 3:50 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Tue, Sep 02, 2025 at 03:29:31PM +0200, Bartosz Golaszewski wrote:
> > On Tue, Sep 2, 2025 at 3:06 PM Andy Shevchenko
> > <andriy.shevchenko@intel.com> wrote:
> > > On Tue, Sep 02, 2025 at 01:59:10PM +0200, Bartosz Golaszewski wrote:
> > > >
> > > > While the API contract in docs doesn't specify it explicitly,
> > >
> > > So, why not to amend the doc at the same time?
> >
> > Because this series is already big as is. That would be another commit
> > that can be separate.
>
> I meant _in the same_ patch.
>
> > > > the generic implementation of the get_function_name() callback from struct
> > > > pinmux_ops - pinmux_generic_get_function_name() - can fail and return
> > > > NULL. This is already checked in pinmux_check_ops() so add a similar
> > > > check in pinmux_func_name_to_selector() instead of passing the returned
> > > > pointer right down to strcmp() where the NULL can get dereferenced. This
> > > > is normal operation when adding new pinfunctions.
>
> > > Fixes?
> >
> > This has always been like that.
> >
> > > Reported?
> >
> > I mean, technically Mark Brown reported my previous patch failing but
> > I don't think we do this if we're still within the same series just
> > another iteration?
> >
> > > Closes?
> >
> > Ditto.
>
> I meant that this fixes a potential issue disregard to your series, right?
>
No, as long as the imx driver keeps putting stuff into the pin
function radix tree directly, this cannot happen. The issue was
triggered by the discrepancy between the number of added selectors and
the hardcoded number of functions (we started at 0 which was not in
the radix tree and crashed before we got to 1).
Bart
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7 01/16] pinctrl: check the return value of pinmux_ops::get_function_name()
2025-09-02 14:02 ` Bartosz Golaszewski
@ 2025-09-02 14:20 ` Andy Shevchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2025-09-02 14:20 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Bjorn Andersson, Konrad Dybcio, Alexey Klimov,
Lorenzo Bianconi, Sean Wang, Matthias Brugger,
AngeloGioacchino Del Regno, Paul Cercueil, Kees Cook,
Andy Shevchenko, Andrew Morton, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Dong Aisheng, Fabio Estevam,
Shawn Guo, Jacky Bai, Pengutronix Kernel Team, NXP S32 Linux Team,
Sascha Hauer, Tony Lindgren, Haojian Zhuang, Geert Uytterhoeven,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Neil Armstrong, Mark Brown, linux-gpio, linux-kernel,
linux-arm-msm, linux-mediatek, linux-arm-kernel, linux-mips,
linux-hardening, linux-mm, imx, linux-omap, linux-renesas-soc,
Bartosz Golaszewski, stable
On Tue, Sep 02, 2025 at 04:02:27PM +0200, Bartosz Golaszewski wrote:
> On Tue, Sep 2, 2025 at 3:50 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Tue, Sep 02, 2025 at 03:29:31PM +0200, Bartosz Golaszewski wrote:
> > > On Tue, Sep 2, 2025 at 3:06 PM Andy Shevchenko
> > > <andriy.shevchenko@intel.com> wrote:
> > > > On Tue, Sep 02, 2025 at 01:59:10PM +0200, Bartosz Golaszewski wrote:
...
> > > > Fixes?
> > >
> > > This has always been like that.
> > >
> > > > Reported?
> > >
> > > I mean, technically Mark Brown reported my previous patch failing but
> > > I don't think we do this if we're still within the same series just
> > > another iteration?
> > >
> > > > Closes?
> > >
> > > Ditto.
> >
> > I meant that this fixes a potential issue disregard to your series, right?
>
> No, as long as the imx driver keeps putting stuff into the pin
> function radix tree directly, this cannot happen. The issue was
> triggered by the discrepancy between the number of added selectors and
> the hardcoded number of functions (we started at 0 which was not in
> the radix tree and crashed before we got to 1).
Ah, thanks for the explanation. The problem is that current commit message
implies a (potential) but lurking somewhere (regardless IMX case). Can you
amend it to make more explicit that there is no bug right now.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7 00/16] pinctrl: introduce the concept of a GPIO pin function category
2025-09-02 11:59 [PATCH v7 00/16] pinctrl: introduce the concept of a GPIO pin function category Bartosz Golaszewski
2025-09-02 11:59 ` [PATCH v7 01/16] pinctrl: check the return value of pinmux_ops::get_function_name() Bartosz Golaszewski
@ 2025-09-02 14:46 ` Andy Shevchenko
2025-09-02 17:37 ` Bartosz Golaszewski
2025-09-02 22:18 ` Linus Walleij
2 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2025-09-02 14:46 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Bjorn Andersson, Konrad Dybcio, Alexey Klimov,
Lorenzo Bianconi, Sean Wang, Matthias Brugger,
AngeloGioacchino Del Regno, Paul Cercueil, Kees Cook,
Andy Shevchenko, Andrew Morton, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Dong Aisheng, Fabio Estevam,
Shawn Guo, Jacky Bai, Pengutronix Kernel Team, NXP S32 Linux Team,
Sascha Hauer, Tony Lindgren, Haojian Zhuang, Geert Uytterhoeven,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Neil Armstrong, Mark Brown, linux-gpio, linux-kernel,
linux-arm-msm, linux-mediatek, linux-arm-kernel, linux-mips,
linux-hardening, linux-mm, imx, linux-omap, linux-renesas-soc,
Bartosz Golaszewski, stable, Chen-Yu Tsai, Konrad Dybcio
On Tue, Sep 02, 2025 at 01:59:09PM +0200, Bartosz Golaszewski wrote:
> Problem: when pinctrl core binds pins to a consumer device and the
> pinmux ops of the underlying driver are marked as strict, the pin in
> question can no longer be requested as a GPIO using the GPIO descriptor
> API. It will result in the following error:
>
> [ 5.095688] sc8280xp-tlmm f100000.pinctrl: pin GPIO_25 already requested by regulator-edp-3p3; cannot claim for f100000.pinctrl:570
> [ 5.107822] sc8280xp-tlmm f100000.pinctrl: error -EINVAL: pin-25 (f100000.pinctrl:570)
>
> This typically makes sense except when the pins are muxed to a function
> that actually says "GPIO". Of course, the function name is just a string
> so it has no meaning to the pinctrl subsystem.
>
> We have many Qualcomm SoCs (and I can imagine it's a common pattern in
> other platforms as well) where we mux a pin to "gpio" function using the
> `pinctrl-X` property in order to configure bias or drive-strength and
> then access it using the gpiod API. This makes it impossible to mark the
> pin controller module as "strict".
>
> This series proposes to introduce a concept of a sub-category of
> pinfunctions: GPIO functions where the above is not true and the pin
> muxed as a GPIO can still be accessed via the GPIO consumer API even for
> strict pinmuxers.
>
> To that end: we first clean up the drivers that use struct function_desc
> and make them use the smaller struct pinfunction instead - which is the
> correct structure for drivers to describe their pin functions with. We
> also rework pinmux core to not duplicate memory used to store the
> pinfunctions unless they're allocated dynamically.
>
> First: provide the kmemdup_const() helper which only duplicates memory
> if it's not in the .rodata section. Then rework all pinctrl drivers that
> instantiate objects of type struct function_desc as they should only be
> created by pinmux core. Next constify the return value of the accessor
> used to expose these structures to users and finally convert the
> pinfunction object within struct function_desc to a pointer and use
> kmemdup_const() to assign it. With this done proceed to add
> infrastructure for the GPIO pin function category and use it in Qualcomm
> drivers. At the very end: make the Qualcomm pinmuxer strict.
I read all this and do not understand why we take all this way,
Esp. see my Q in patch 16. Can we rather limit this to the controller
driver to decide and have it handle all the possible configurations,
muxing, etc?
I think what we are trying to do here is to delegate part of the
driver's work pin mux / pin control core. While it sounds like right
direction the implementation (design wise) seems to me unscalable.
In any case first 12 patch (in case they are not regressing) are good
to go as soon as they can. I like the part of constification.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7 00/16] pinctrl: introduce the concept of a GPIO pin function category
2025-09-02 14:46 ` [PATCH v7 00/16] pinctrl: introduce the concept of a GPIO pin function category Andy Shevchenko
@ 2025-09-02 17:37 ` Bartosz Golaszewski
0 siblings, 0 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2025-09-02 17:37 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, Bjorn Andersson, Konrad Dybcio, Alexey Klimov,
Lorenzo Bianconi, Sean Wang, Matthias Brugger,
AngeloGioacchino Del Regno, Paul Cercueil, Kees Cook,
Andy Shevchenko, Andrew Morton, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Dong Aisheng, Fabio Estevam,
Shawn Guo, Jacky Bai, Pengutronix Kernel Team, NXP S32 Linux Team,
Sascha Hauer, Tony Lindgren, Haojian Zhuang, Geert Uytterhoeven,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Neil Armstrong, Mark Brown, linux-gpio, linux-kernel,
linux-arm-msm, linux-mediatek, linux-arm-kernel, linux-mips,
linux-hardening, linux-mm, imx, linux-omap, linux-renesas-soc,
Bartosz Golaszewski, stable, Chen-Yu Tsai, Konrad Dybcio
On Tue, Sep 2, 2025 at 4:46 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Tue, Sep 02, 2025 at 01:59:09PM +0200, Bartosz Golaszewski wrote:
> > Problem: when pinctrl core binds pins to a consumer device and the
> > pinmux ops of the underlying driver are marked as strict, the pin in
> > question can no longer be requested as a GPIO using the GPIO descriptor
> > API. It will result in the following error:
> >
> > [ 5.095688] sc8280xp-tlmm f100000.pinctrl: pin GPIO_25 already requested by regulator-edp-3p3; cannot claim for f100000.pinctrl:570
> > [ 5.107822] sc8280xp-tlmm f100000.pinctrl: error -EINVAL: pin-25 (f100000.pinctrl:570)
> >
> > This typically makes sense except when the pins are muxed to a function
> > that actually says "GPIO". Of course, the function name is just a string
> > so it has no meaning to the pinctrl subsystem.
> >
> > We have many Qualcomm SoCs (and I can imagine it's a common pattern in
> > other platforms as well) where we mux a pin to "gpio" function using the
> > `pinctrl-X` property in order to configure bias or drive-strength and
> > then access it using the gpiod API. This makes it impossible to mark the
> > pin controller module as "strict".
> >
> > This series proposes to introduce a concept of a sub-category of
> > pinfunctions: GPIO functions where the above is not true and the pin
> > muxed as a GPIO can still be accessed via the GPIO consumer API even for
> > strict pinmuxers.
> >
> > To that end: we first clean up the drivers that use struct function_desc
> > and make them use the smaller struct pinfunction instead - which is the
> > correct structure for drivers to describe their pin functions with. We
> > also rework pinmux core to not duplicate memory used to store the
> > pinfunctions unless they're allocated dynamically.
> >
> > First: provide the kmemdup_const() helper which only duplicates memory
> > if it's not in the .rodata section. Then rework all pinctrl drivers that
> > instantiate objects of type struct function_desc as they should only be
> > created by pinmux core. Next constify the return value of the accessor
> > used to expose these structures to users and finally convert the
> > pinfunction object within struct function_desc to a pointer and use
> > kmemdup_const() to assign it. With this done proceed to add
> > infrastructure for the GPIO pin function category and use it in Qualcomm
> > drivers. At the very end: make the Qualcomm pinmuxer strict.
>
> I read all this and do not understand why we take all this way,
> Esp. see my Q in patch 16. Can we rather limit this to the controller
> driver to decide and have it handle all the possible configurations,
> muxing, etc?
>
> I think what we are trying to do here is to delegate part of the
> driver's work pin mux / pin control core. While it sounds like right
> direction the implementation (design wise) seems to me unscalable.
>
> In any case first 12 patch (in case they are not regressing) are good
> to go as soon as they can. I like the part of constification.
>
I'm not sure how to rephrase it. Strict pinmuxers are already a thing,
but on many platforms it's impossible to use them BECAUSE pinctrl
doesn't care about what a function does semantically. It just so
happens that some functions are GPIOs and as such can also be used by
GPIOLIB. Except that if the pinmuxer is "strict", any gpiod_get() call
will fail BECAUSE pinctrl does not know that a function called "gpio"
is actually a GPIO and will say NO if anything tries to request a
muxed pin. This (the function name) is just a string, it could as well
be called "andy" for all pinctrl cares. This is why we're doing it at
the pinctrl core level - because it will benefit many other platforms
as Linus mentioned elsewhere - he has some other platforms lined up
for a similar conversion. And also because it cannot be done at the
driver level at the moment, it's the pinctrl core that says "NO" to
GPIOLIB. I think you missed the entire point of this series.
Bartosz
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7 00/16] pinctrl: introduce the concept of a GPIO pin function category
2025-09-02 11:59 [PATCH v7 00/16] pinctrl: introduce the concept of a GPIO pin function category Bartosz Golaszewski
2025-09-02 11:59 ` [PATCH v7 01/16] pinctrl: check the return value of pinmux_ops::get_function_name() Bartosz Golaszewski
2025-09-02 14:46 ` [PATCH v7 00/16] pinctrl: introduce the concept of a GPIO pin function category Andy Shevchenko
@ 2025-09-02 22:18 ` Linus Walleij
2025-09-03 11:51 ` Andy Shevchenko
2 siblings, 1 reply; 11+ messages in thread
From: Linus Walleij @ 2025-09-02 22:18 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Bjorn Andersson, Konrad Dybcio, Alexey Klimov, Lorenzo Bianconi,
Sean Wang, Matthias Brugger, AngeloGioacchino Del Regno,
Paul Cercueil, Kees Cook, Andy Shevchenko, Andrew Morton,
David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Dong Aisheng, Fabio Estevam, Shawn Guo, Jacky Bai,
Pengutronix Kernel Team, NXP S32 Linux Team, Sascha Hauer,
Tony Lindgren, Haojian Zhuang, Geert Uytterhoeven,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Neil Armstrong, Mark Brown, linux-gpio, linux-kernel,
linux-arm-msm, linux-mediatek, linux-arm-kernel, linux-mips,
linux-hardening, linux-mm, imx, linux-omap, linux-renesas-soc,
Bartosz Golaszewski, stable, Chen-Yu Tsai, Konrad Dybcio
On Tue, Sep 2, 2025 at 1:59 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> We have many Qualcomm SoCs (and I can imagine it's a common pattern in
> other platforms as well) where we mux a pin to "gpio" function using the
> `pinctrl-X` property in order to configure bias or drive-strength and
> then access it using the gpiod API. This makes it impossible to mark the
> pin controller module as "strict".
>
> This series proposes to introduce a concept of a sub-category of
> pinfunctions: GPIO functions where the above is not true and the pin
> muxed as a GPIO can still be accessed via the GPIO consumer API even for
> strict pinmuxers.
This is what I want for pin control, and fixes an ages old issue
that pin control has no intrinsic awareness of if a pin is muxed
to a function providing GPIO.
So patches applied!
Any remaining code nitpicks can be fixed in-tree, I need this
to be able to apply the much desired Broadcom STB driver,
so this needs to go into -next now for cooking.
I also want to strictify some drivers using this, bringing GPIO
function awareness into them, which is a good thing!
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7 00/16] pinctrl: introduce the concept of a GPIO pin function category
2025-09-02 22:18 ` Linus Walleij
@ 2025-09-03 11:51 ` Andy Shevchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2025-09-03 11:51 UTC (permalink / raw)
To: Linus Walleij
Cc: Bartosz Golaszewski, Bjorn Andersson, Konrad Dybcio,
Alexey Klimov, Lorenzo Bianconi, Sean Wang, Matthias Brugger,
AngeloGioacchino Del Regno, Paul Cercueil, Kees Cook,
Andy Shevchenko, Andrew Morton, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Dong Aisheng, Fabio Estevam,
Shawn Guo, Jacky Bai, Pengutronix Kernel Team, NXP S32 Linux Team,
Sascha Hauer, Tony Lindgren, Haojian Zhuang, Geert Uytterhoeven,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Neil Armstrong, Mark Brown, linux-gpio, linux-kernel,
linux-arm-msm, linux-mediatek, linux-arm-kernel, linux-mips,
linux-hardening, linux-mm, imx, linux-omap, linux-renesas-soc,
Bartosz Golaszewski, stable, Chen-Yu Tsai, Konrad Dybcio
On Wed, Sep 03, 2025 at 12:18:18AM +0200, Linus Walleij wrote:
> On Tue, Sep 2, 2025 at 1:59 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > We have many Qualcomm SoCs (and I can imagine it's a common pattern in
> > other platforms as well) where we mux a pin to "gpio" function using the
> > `pinctrl-X` property in order to configure bias or drive-strength and
> > then access it using the gpiod API. This makes it impossible to mark the
> > pin controller module as "strict".
> >
> > This series proposes to introduce a concept of a sub-category of
> > pinfunctions: GPIO functions where the above is not true and the pin
> > muxed as a GPIO can still be accessed via the GPIO consumer API even for
> > strict pinmuxers.
>
> This is what I want for pin control, and fixes an ages old issue
> that pin control has no intrinsic awareness of if a pin is muxed
> to a function providing GPIO.
> So patches applied!
No objections, let's move on.
> Any remaining code nitpicks can be fixed in-tree, I need this
> to be able to apply the much desired Broadcom STB driver,
> so this needs to go into -next now for cooking.
>
> I also want to strictify some drivers using this, bringing GPIO
> function awareness into them, which is a good thing!
Well said!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread