[PATCH 0/3] pinctrl: bcm: Fix fsel_get() and use generic dt_free_map

[PATCH 0/3] pinctrl: bcm: Fix fsel_get() and use generic dt_free_map

[Wang Xiaolei]

This series contains one fix and two cleanups for the Broadcom pin
controllers.

Patch 1 fixes an off-by-one and out-of-bounds read in the STB pin
controller driver that was added in v6.18. The FSEL register field is
a 1-based function number -- 0 means GPIO, 1..n_funcs select
funcs[0..n_funcs - 1] -- but brcmstb_pinctrl_fsel_get() uses the raw
field value as a 0-based index into funcs[]. Every readback is
therefore shifted by one entry, and the value 8, which
brcmstb_pinctrl_fsel_set() accepts as the eighth alternate function,
reads funcs[8] and trips KASAN when the pin state is printed through
debugfs.

Patches 2 and 3 replace hand-written dt_free_map callbacks with
pinconf_generic_dt_free_map(). Both drivers already call
pinconf_generic_dt_node_to_map_*() for dt_node_to_map and already
select GENERIC_PINCONF, so the generic helper is the matching
counterpart, and the open-coded free loop just duplicates what
pinctrl_utils_free_map() already does. No functional change is
intended for the map types the existing device trees use.

Patch 1 was tested on a Raspberry Pi 5 with KASAN enabled. Before the
change, writing "gpio45 alt8" to pinmux-select and then reading pins
triggers a global-out-of-bounds splat in brcmstb_pctl_pin_dbg_show();
after the change the same sequence reports "function spi_m" without
a warning, and a register field value of 0 reports "gpio".

The series is based on v7.4-rc1 and has no inter-patch dependencies.


Wang Xiaolei (3):
  pinctrl: bcm: Fix off-by-one and out-of-bounds access in fsel_get()
  pinctrl: bcm: Use pinconf_generic_dt_free_map in bcm2835
  pinctrl: bcm: Use pinconf_generic_dt_free_map in brcmstb

 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 16 ++--------------
 drivers/pinctrl/bcm/pinctrl-brcmstb.c | 26 +++++++++++---------------
 2 files changed, 13 insertions(+), 29 deletions(-)

-- 
2.34.1

[PATCH 1/3] pinctrl: bcm: Fix off-by-one and out-of-bounds access in fsel_get()

[Wang Xiaolei]

brcmstb_pinctrl_fsel_get() takes the value read from the pin's FSEL
register and uses it directly as an index into
pc->pin_funcs[pin].funcs[]:

        fsel = (val >> BIT_TO_SHIFT(bit)) & pc->pin_funcs[pin].func_mask;
        func = pc->pin_funcs[pin].funcs[fsel];

That is not how the register field is encoded. The setter,
brcmstb_pinctrl_fsel_set(), treats 0 as GPIO, 1..n_funcs as
funcs[0..n_funcs - 1], rejects n_funcs + 1 and above, and n_funcs is
always BRCMSTB_FSEL_COUNT (8). The field is therefore a 1-based function
number, while funcs[] is a 0-based array of eight entries.

Every readback is consequently shifted by one slot, and a field value of
8 -- which the setter accepts, being the eighth and last alternate
function -- indexes funcs[8], one element past the end of the array.

This is reachable from userspace without supplying any invalid input:

        # echo "gpio45 alt8" > .../pinmux-select
        # cat .../pins

The write is an ordinary mux request that fsel_set() accepts; the read
that follows goes through pinctrl_pins_show() ->
brcmstb_pctl_pin_dbg_show() -> fsel_get() and triggers KASAN on
Raspberry Pi 5:

        BUG: KASAN: global-out-of-bounds in brcmstb_pctl_pin_dbg_show
        Read of size 1 at addr ffffa07c577ab2e8 by task cat/839
        The buggy address belongs to the variable:
         __compound_literal.62+0x8/0x40

A field value of 0 has the same root cause in a milder form: it is
reported as funcs[0] instead of GPIO. The mux itself is programmed
correctly in both cases, so this affects the debugfs readback and KASAN
rather than the hardware configuration.

Handle 0 and out-of-range values explicitly, and index funcs[] with
fsel - 1, so that the getter uses the same numbering as the setter and
the index can no longer go past funcs[n_funcs - 1]. The existing
fallback for func__ placeholder entries is kept:

        if (!fsel)
                return pc->func_gpio;

        if (fsel > pc->pin_funcs[pin].n_funcs) {
                dev_warn(pc->dev,
			 "invalid fsel %d for pin %u\n", fsel, pin);
                return pc->func_gpio;
        }

        func = pc->pin_funcs[pin].funcs[fsel - 1];

Tested on Raspberry Pi 5 with KASAN enabled. Before the fix, writing
"gpio45 alt8" to .../pinmux-select and then reading .../pins triggers
the splat above. After the fix the same sequence reports
"function spi_m" with no warning and no splat, and a register field
value of 0 reports "gpio". The register is restored to 0 afterwards.

Fixes: 657cbf9b24ba ("pinctrl: bcm: Add STB family pin controller driver")
Cc: Jonathan Bell <jonathan@raspberrypi.com>
Cc: Phil Elwell <phil@raspberrypi.com>
Signed-off-by: Wang Xiaolei <wangxiaolei01@kaihong.com>
---
 drivers/pinctrl/bcm/pinctrl-brcmstb.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-brcmstb.c b/drivers/pinctrl/bcm/pinctrl-brcmstb.c
index f46b27155c3c..2b942ca7196b 100644
--- a/drivers/pinctrl/bcm/pinctrl-brcmstb.c
+++ b/drivers/pinctrl/bcm/pinctrl-brcmstb.c
@@ -66,7 +66,16 @@ static unsigned int brcmstb_pinctrl_fsel_get(struct brcmstb_pinctrl *pc,
 
 	val = readl(pc->base + BIT_TO_REG(bit));
 	fsel = (val >> BIT_TO_SHIFT(bit)) & pc->pin_funcs[pin].func_mask;
-	func = pc->pin_funcs[pin].funcs[fsel];
+
+	if (!fsel)
+		return pc->func_gpio;
+
+	if (fsel > pc->pin_funcs[pin].n_funcs) {
+		dev_warn(pc->dev, "invalid fsel %d for pin %u\n", fsel, pin);
+		return pc->func_gpio;
+	}
+
+	func = pc->pin_funcs[pin].funcs[fsel - 1];
 
 	if (func >= pc->func_count)
 		func = fsel;
-- 
2.34.1

[PATCH 2/3] pinctrl: bcm: Use pinconf_generic_dt_free_map in bcm2835

[Wang Xiaolei]

Replace the hand-written dt_free_map callback with the generic
pinconf_generic_dt_free_map() helper, which wraps
pinctrl_utils_free_map(). This drops the duplicated free loop and
handles both CONFIGS_PIN and CONFIGS_GROUP map types.

The error path in bcm2835_pctl_dt_node_to_map() uses the same helper.
The driver already selects GENERIC_PINCONF, so no Kconfig change is
needed.

Signed-off-by: Wang Xiaolei <wangxiaolei01@kaihong.com>
---
 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index 725e880ae086..69a92cd7bfe0 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -760,18 +760,6 @@ static void bcm2835_pctl_pin_dbg_show(struct pinctrl_dev *pctldev,
 		irq, irq_type_names[pc->irq_type[offset]]);
 }
 
-static void bcm2835_pctl_dt_free_map(struct pinctrl_dev *pctldev,
-		struct pinctrl_map *maps, unsigned num_maps)
-{
-	int i;
-
-	for (i = 0; i < num_maps; i++)
-		if (maps[i].type == PIN_MAP_TYPE_CONFIGS_PIN)
-			kfree(maps[i].data.configs.configs);
-
-	kfree(maps);
-}
-
 static int bcm2835_pctl_dt_node_to_map_func(struct bcm2835_pinctrl *pc,
 		struct device_node *np, u32 pin, u32 fnum,
 		struct pinctrl_map **maps)
@@ -916,7 +904,7 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev *pctldev,
 	return 0;
 
 out:
-	bcm2835_pctl_dt_free_map(pctldev, maps, num_pins * maps_per_pin);
+	pinconf_generic_dt_free_map(pctldev, maps, num_pins * maps_per_pin);
 	return err;
 }
 
@@ -926,7 +914,7 @@ static const struct pinctrl_ops bcm2835_pctl_ops = {
 	.get_group_pins = bcm2835_pctl_get_group_pins,
 	.pin_dbg_show = bcm2835_pctl_pin_dbg_show,
 	.dt_node_to_map = bcm2835_pctl_dt_node_to_map,
-	.dt_free_map = bcm2835_pctl_dt_free_map,
+	.dt_free_map = pinconf_generic_dt_free_map,
 };
 
 static int bcm2835_pmx_free(struct pinctrl_dev *pctldev,
-- 
2.34.1

[PATCH 3/3] pinctrl: bcm: Use pinconf_generic_dt_free_map in brcmstb

[Wang Xiaolei]

Replace the hand-written dt_free_map callback with the generic
pinconf_generic_dt_free_map() helper, which wraps
pinctrl_utils_free_map(). This drops the duplicated free loop and
pairs the callback with the pinconf_generic_dt_node_to_map_all()
already used for dt_node_to_map.

The driver already selects GENERIC_PINCONF, so no Kconfig change is
needed.

Cc: Jonathan Bell <jonathan@raspberrypi.com>
Cc: Phil Elwell <phil@raspberrypi.com>
Signed-off-by: Wang Xiaolei <wangxiaolei01@kaihong.com>
---
 drivers/pinctrl/bcm/pinctrl-brcmstb.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-brcmstb.c b/drivers/pinctrl/bcm/pinctrl-brcmstb.c
index 2b942ca7196b..e81c5b295a05 100644
--- a/drivers/pinctrl/bcm/pinctrl-brcmstb.c
+++ b/drivers/pinctrl/bcm/pinctrl-brcmstb.c
@@ -180,26 +180,13 @@ static void brcmstb_pctl_pin_dbg_show(struct pinctrl_dev *pctldev,
 	seq_printf(s, "function %s", fname);
 }
 
-static void brcmstb_pctl_dt_free_map(struct pinctrl_dev *pctldev,
-				     struct pinctrl_map *maps,
-				     unsigned int num_maps)
-{
-	int i;
-
-	for (i = 0; i < num_maps; i++)
-		if (maps[i].type == PIN_MAP_TYPE_CONFIGS_PIN)
-			kfree(maps[i].data.configs.configs);
-
-	kfree(maps);
-}
-
 static const struct pinctrl_ops brcmstb_pctl_ops = {
 	.get_groups_count = brcmstb_pctl_get_groups_count,
 	.get_group_name = brcmstb_pctl_get_group_name,
 	.get_group_pins = brcmstb_pctl_get_group_pins,
 	.pin_dbg_show = brcmstb_pctl_pin_dbg_show,
 	.dt_node_to_map = pinconf_generic_dt_node_to_map_all,
-	.dt_free_map = brcmstb_pctl_dt_free_map,
+	.dt_free_map = pinconf_generic_dt_free_map,
 };
 
 static int brcmstb_pmx_free(struct pinctrl_dev *pctldev, unsigned int offset)
-- 
2.34.1

Re: [PATCH 1/3] pinctrl: bcm: Fix off-by-one and out-of-bounds access in fsel_get()

[Florian Fainelli]

On 11/5/26 23:06, Wang Xiaolei wrote:
> brcmstb_pinctrl_fsel_get() takes the value read from the pin's FSEL
> register and uses it directly as an index into
> pc->pin_funcs[pin].funcs[]:
> 
>          fsel = (val >> BIT_TO_SHIFT(bit)) & pc->pin_funcs[pin].func_mask;
>          func = pc->pin_funcs[pin].funcs[fsel];
> 
> That is not how the register field is encoded. The setter,
> brcmstb_pinctrl_fsel_set(), treats 0 as GPIO, 1..n_funcs as
> funcs[0..n_funcs - 1], rejects n_funcs + 1 and above, and n_funcs is
> always BRCMSTB_FSEL_COUNT (8). The field is therefore a 1-based function
> number, while funcs[] is a 0-based array of eight entries.
> 
> Every readback is consequently shifted by one slot, and a field value of
> 8 -- which the setter accepts, being the eighth and last alternate
> function -- indexes funcs[8], one element past the end of the array.
> 
> This is reachable from userspace without supplying any invalid input:
> 
>          # echo "gpio45 alt8" > .../pinmux-select
>          # cat .../pins
> 
> The write is an ordinary mux request that fsel_set() accepts; the read
> that follows goes through pinctrl_pins_show() ->
> brcmstb_pctl_pin_dbg_show() -> fsel_get() and triggers KASAN on
> Raspberry Pi 5:
> 
>          BUG: KASAN: global-out-of-bounds in brcmstb_pctl_pin_dbg_show
>          Read of size 1 at addr ffffa07c577ab2e8 by task cat/839
>          The buggy address belongs to the variable:
>           __compound_literal.62+0x8/0x40
> 
> A field value of 0 has the same root cause in a milder form: it is
> reported as funcs[0] instead of GPIO. The mux itself is programmed
> correctly in both cases, so this affects the debugfs readback and KASAN
> rather than the hardware configuration.
> 
> Handle 0 and out-of-range values explicitly, and index funcs[] with
> fsel - 1, so that the getter uses the same numbering as the setter and
> the index can no longer go past funcs[n_funcs - 1]. The existing
> fallback for func__ placeholder entries is kept:
> 
>          if (!fsel)
>                  return pc->func_gpio;
> 
>          if (fsel > pc->pin_funcs[pin].n_funcs) {
>                  dev_warn(pc->dev,
> 			 "invalid fsel %d for pin %u\n", fsel, pin);
>                  return pc->func_gpio;
>          }
> 
>          func = pc->pin_funcs[pin].funcs[fsel - 1];
> 
> Tested on Raspberry Pi 5 with KASAN enabled. Before the fix, writing
> "gpio45 alt8" to .../pinmux-select and then reading .../pins triggers
> the splat above. After the fix the same sequence reports
> "function spi_m" with no warning and no splat, and a register field
> value of 0 reports "gpio". The register is restored to 0 afterwards.
> 
> Fixes: 657cbf9b24ba ("pinctrl: bcm: Add STB family pin controller driver")
> Cc: Jonathan Bell <jonathan@raspberrypi.com>
> Cc: Phil Elwell <phil@raspberrypi.com>
> Signed-off-by: Wang Xiaolei <wangxiaolei01@kaihong.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH 2/3] pinctrl: bcm: Use pinconf_generic_dt_free_map in bcm2835

[Florian Fainelli]

On 11/5/26 23:06, Wang Xiaolei wrote:
> Replace the hand-written dt_free_map callback with the generic
> pinconf_generic_dt_free_map() helper, which wraps
> pinctrl_utils_free_map(). This drops the duplicated free loop and
> handles both CONFIGS_PIN and CONFIGS_GROUP map types.
> 
> The error path in bcm2835_pctl_dt_node_to_map() uses the same helper.
> The driver already selects GENERIC_PINCONF, so no Kconfig change is
> needed.
> 
> Signed-off-by: Wang Xiaolei <wangxiaolei01@kaihong.com>

I think this deserves a Fixes: tag because it fixes a memory leak, 
generic allocation can allocate PIN_MAP_TYPE_CONFIGS_GROUP, but the 
custom callback does not free that allocation type:

Fixes: 0de704955ee4 ("pinctrl: bcm2835: Add support for generic pinctrl 
binding")
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH 3/3] pinctrl: bcm: Use pinconf_generic_dt_free_map in brcmstb

[Florian Fainelli]

On 11/5/26 23:06, Wang Xiaolei wrote:
> Replace the hand-written dt_free_map callback with the generic
> pinconf_generic_dt_free_map() helper, which wraps
> pinctrl_utils_free_map(). This drops the duplicated free loop and
> pairs the callback with the pinconf_generic_dt_node_to_map_all()
> already used for dt_node_to_map.
> 
> The driver already selects GENERIC_PINCONF, so no Kconfig change is
> needed.
> 
> Cc: Jonathan Bell <jonathan@raspberrypi.com>
> Cc: Phil Elwell <phil@raspberrypi.com>
> Signed-off-by: Wang Xiaolei <wangxiaolei01@kaihong.com>

Similar thing to patch #2, this should have:

Fixes: 657cbf9b24ba ("pinctrl: bcm: Add STB family pin controller driver")
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH 1/3] pinctrl: bcm: Fix off-by-one and out-of-bounds access in fsel_get()

[Andrea della Porta]

Hi Wang,

On 07:06 Fri 06 Nov     , Wang Xiaolei wrote:
> brcmstb_pinctrl_fsel_get() takes the value read from the pin's FSEL
> register and uses it directly as an index into
> pc->pin_funcs[pin].funcs[]:
> 
>         fsel = (val >> BIT_TO_SHIFT(bit)) & pc->pin_funcs[pin].func_mask;
>         func = pc->pin_funcs[pin].funcs[fsel];
> 
> That is not how the register field is encoded. The setter,
> brcmstb_pinctrl_fsel_set(), treats 0 as GPIO, 1..n_funcs as
> funcs[0..n_funcs - 1], rejects n_funcs + 1 and above, and n_funcs is
> always BRCMSTB_FSEL_COUNT (8). The field is therefore a 1-based function
> number, while funcs[] is a 0-based array of eight entries.
> 
> Every readback is consequently shifted by one slot, and a field value of
> 8 -- which the setter accepts, being the eighth and last alternate
> function -- indexes funcs[8], one element past the end of the array.
> 
> This is reachable from userspace without supplying any invalid input:
> 
>         # echo "gpio45 alt8" > .../pinmux-select
>         # cat .../pins
> 
> The write is an ordinary mux request that fsel_set() accepts; the read
> that follows goes through pinctrl_pins_show() ->
> brcmstb_pctl_pin_dbg_show() -> fsel_get() and triggers KASAN on
> Raspberry Pi 5:
> 
>         BUG: KASAN: global-out-of-bounds in brcmstb_pctl_pin_dbg_show
>         Read of size 1 at addr ffffa07c577ab2e8 by task cat/839
>         The buggy address belongs to the variable:
>          __compound_literal.62+0x8/0x40
> 
> A field value of 0 has the same root cause in a milder form: it is
> reported as funcs[0] instead of GPIO. The mux itself is programmed
> correctly in both cases, so this affects the debugfs readback and KASAN
> rather than the hardware configuration.
> 
> Handle 0 and out-of-range values explicitly, and index funcs[] with
> fsel - 1, so that the getter uses the same numbering as the setter and
> the index can no longer go past funcs[n_funcs - 1]. The existing
> fallback for func__ placeholder entries is kept:
> 
>         if (!fsel)
>                 return pc->func_gpio;
> 
>         if (fsel > pc->pin_funcs[pin].n_funcs) {
>                 dev_warn(pc->dev,
> 			 "invalid fsel %d for pin %u\n", fsel, pin);
>                 return pc->func_gpio;
>         }
> 
>         func = pc->pin_funcs[pin].funcs[fsel - 1];

Non-blocking nit-pick: I think that pasting the code in the comment is
too verbose, especially when the same code consists in the entire patch
body here below. A succint explanation by words would be more than enough.

> 
> Tested on Raspberry Pi 5 with KASAN enabled. Before the fix, writing
> "gpio45 alt8" to .../pinmux-select and then reading .../pins triggers
> the splat above. After the fix the same sequence reports
> "function spi_m" with no warning and no splat, and a register field
> value of 0 reports "gpio". The register is restored to 0 afterwards.

Since you're at it, would you mind fixing the SGPIO AON related OOB access too?
Please take a look at bcm2712_[cd]0_aon_gpio_pin_regs struct: they both have
a bunch of AON_SGPIO_REGS macro describing pins at the end of the struct. That
macro puts them at [(n) + 32] in the struct so there's a hole between 'normal'
gpios and SGPIO ones.
Accessing bcm2712_[cd]0_aon_gpio_pin_funcs struct via this +32 offset as the
'pin' index in pc->pin_funcs[pin].funcs[] will result in OOB access. KASAN should
report that if you 'cat pins' in aon pin controller.

One way would be to refactor the code in order to avoid this hole, but I
won't pretend this.
The other way is just add the missing SGPIO entries in bcm2712_[cd]0_aon_gpio_pin_funcs
(mind the +32 offset, a new macro is probably needed).
You can find the functions for SGPIO pins declared in downstream driver, see file
drivers/pinctrl/bcm/pinctrl-bcm2712.c from rpi-6.12.y branch (later branches
already have the upstream driver which is missing the SGPIO functions entirely).
I've posted here the relevant data copied from downstream for your convenience,
please rearrange accordingly:

static const struct bcm2712_pin_funcs bcm2712_c0_aon_sgpio_pin_funcs[] = {
        PIN(0, hdmi_tx0_bsc, hdmi_tx0_auto_i2c, bsc_m0, vc_i2c0, _, _, _, _),
        PIN(1, hdmi_tx0_bsc, hdmi_tx0_auto_i2c, bsc_m0, vc_i2c0, _, _, _, _),
        PIN(2, hdmi_tx1_bsc, hdmi_tx1_auto_i2c, bsc_m1, vc_i2c4, ctl_hdmi_5v, _, _, _),
        PIN(3, hdmi_tx1_bsc, hdmi_tx1_auto_i2c, bsc_m1, vc_i2c4, _, _, _, _),
        PIN(4, avs_pmu_bsc, bsc_m2, vc_i2c5, ctl_hdmi_5v, _, _, _, _),
        PIN(5, avs_pmu_bsc, bsc_m2, vc_i2c5, _, _, _, _, _),
};

static const struct bcm2712_pin_funcs bcm2712_d0_aon_sgpio_pin_funcs[] = {
        PIN(0, hdmi_tx0_bsc, hdmi_tx0_auto_i2c, bsc_m0, vc_i2c0, _, _, _, _),
        PIN(1, hdmi_tx0_bsc, hdmi_tx0_auto_i2c, bsc_m0, vc_i2c0, _, _, _, _),
        PIN(2, hdmi_tx1_bsc, hdmi_tx1_auto_i2c, bsc_m1, vc_i2c0, ctl_hdmi_5v, _, _, _),
        PIN(3, hdmi_tx1_bsc, hdmi_tx1_auto_i2c, bsc_m1, vc_i2c0, _, _, _, _),
        PIN(4, avs_pmu_bsc, bsc_m2, vc_i2c3, ctl_hdmi_5v, _, _, _, _),
        PIN(5, avs_pmu_bsc, bsc_m2, vc_i2c3, _, _, _, _, _),
};

In case you don't want to fix this, you will still have a Reviewed-by from me,
and I'll take care of uploading the fix in a subsequent patch.

Many thanks,
Andrea

> 
> Fixes: 657cbf9b24ba ("pinctrl: bcm: Add STB family pin controller driver")
> Cc: Jonathan Bell <jonathan@raspberrypi.com>
> Cc: Phil Elwell <phil@raspberrypi.com>
> Signed-off-by: Wang Xiaolei <wangxiaolei01@kaihong.com>
> ---
>  drivers/pinctrl/bcm/pinctrl-brcmstb.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/bcm/pinctrl-brcmstb.c b/drivers/pinctrl/bcm/pinctrl-brcmstb.c
> index f46b27155c3c..2b942ca7196b 100644
> --- a/drivers/pinctrl/bcm/pinctrl-brcmstb.c
> +++ b/drivers/pinctrl/bcm/pinctrl-brcmstb.c
> @@ -66,7 +66,16 @@ static unsigned int brcmstb_pinctrl_fsel_get(struct brcmstb_pinctrl *pc,
>  
>  	val = readl(pc->base + BIT_TO_REG(bit));
>  	fsel = (val >> BIT_TO_SHIFT(bit)) & pc->pin_funcs[pin].func_mask;
> -	func = pc->pin_funcs[pin].funcs[fsel];
> +
> +	if (!fsel)
> +		return pc->func_gpio;
> +
> +	if (fsel > pc->pin_funcs[pin].n_funcs) {
> +		dev_warn(pc->dev, "invalid fsel %d for pin %u\n", fsel, pin);
> +		return pc->func_gpio;
> +	}
> +
> +	func = pc->pin_funcs[pin].funcs[fsel - 1];
>  
>  	if (func >= pc->func_count)
>  		func = fsel;
> -- 
> 2.34.1
> 

Re: [PATCH 2/3] pinctrl: bcm: Use pinconf_generic_dt_free_map in bcm2835

[Andrea della Porta]

Hi Wang,

On 07:06 Fri 06 Nov     , Wang Xiaolei wrote:
> Replace the hand-written dt_free_map callback with the generic
> pinconf_generic_dt_free_map() helper, which wraps
> pinctrl_utils_free_map(). This drops the duplicated free loop and
> handles both CONFIGS_PIN and CONFIGS_GROUP map types.
> 
> The error path in bcm2835_pctl_dt_node_to_map() uses the same helper.
> The driver already selects GENERIC_PINCONF, so no Kconfig change is
> needed.
> 
> Signed-off-by: Wang Xiaolei <wangxiaolei01@kaihong.com>

Please CC:stable.

Reviewed-by: Andrea della Porta <andrea.porta@suse.com>

Many thanks,
Andrea

> ---
>  drivers/pinctrl/bcm/pinctrl-bcm2835.c | 16 ++--------------
>  1 file changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> index 725e880ae086..69a92cd7bfe0 100644
> --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> @@ -760,18 +760,6 @@ static void bcm2835_pctl_pin_dbg_show(struct pinctrl_dev *pctldev,
>  		irq, irq_type_names[pc->irq_type[offset]]);
>  }
>  
> -static void bcm2835_pctl_dt_free_map(struct pinctrl_dev *pctldev,
> -		struct pinctrl_map *maps, unsigned num_maps)
> -{
> -	int i;
> -
> -	for (i = 0; i < num_maps; i++)
> -		if (maps[i].type == PIN_MAP_TYPE_CONFIGS_PIN)
> -			kfree(maps[i].data.configs.configs);
> -
> -	kfree(maps);
> -}
> -
>  static int bcm2835_pctl_dt_node_to_map_func(struct bcm2835_pinctrl *pc,
>  		struct device_node *np, u32 pin, u32 fnum,
>  		struct pinctrl_map **maps)
> @@ -916,7 +904,7 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev *pctldev,
>  	return 0;
>  
>  out:
> -	bcm2835_pctl_dt_free_map(pctldev, maps, num_pins * maps_per_pin);
> +	pinconf_generic_dt_free_map(pctldev, maps, num_pins * maps_per_pin);
>  	return err;
>  }
>  
> @@ -926,7 +914,7 @@ static const struct pinctrl_ops bcm2835_pctl_ops = {
>  	.get_group_pins = bcm2835_pctl_get_group_pins,
>  	.pin_dbg_show = bcm2835_pctl_pin_dbg_show,
>  	.dt_node_to_map = bcm2835_pctl_dt_node_to_map,
> -	.dt_free_map = bcm2835_pctl_dt_free_map,
> +	.dt_free_map = pinconf_generic_dt_free_map,
>  };
>  
>  static int bcm2835_pmx_free(struct pinctrl_dev *pctldev,
> -- 
> 2.34.1
> 

Re: [PATCH 3/3] pinctrl: bcm: Use pinconf_generic_dt_free_map in brcmstb

[Andrea della Porta]

Hi Wang,

On 07:06 Fri 06 Nov     , Wang Xiaolei wrote:
> Replace the hand-written dt_free_map callback with the generic
> pinconf_generic_dt_free_map() helper, which wraps
> pinctrl_utils_free_map(). This drops the duplicated free loop and
> pairs the callback with the pinconf_generic_dt_node_to_map_all()
> already used for dt_node_to_map.
> 
> The driver already selects GENERIC_PINCONF, so no Kconfig change is
> needed.
> 
> Cc: Jonathan Bell <jonathan@raspberrypi.com>
> Cc: Phil Elwell <phil@raspberrypi.com>
> Signed-off-by: Wang Xiaolei <wangxiaolei01@kaihong.com>

Please CC:stable.

Reviewed-by: Andrea della Porta <andrea.porta@suse.com>

Many thanks,
Andrea

> ---
>  drivers/pinctrl/bcm/pinctrl-brcmstb.c | 15 +--------------
>  1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/drivers/pinctrl/bcm/pinctrl-brcmstb.c b/drivers/pinctrl/bcm/pinctrl-brcmstb.c
> index 2b942ca7196b..e81c5b295a05 100644
> --- a/drivers/pinctrl/bcm/pinctrl-brcmstb.c
> +++ b/drivers/pinctrl/bcm/pinctrl-brcmstb.c
> @@ -180,26 +180,13 @@ static void brcmstb_pctl_pin_dbg_show(struct pinctrl_dev *pctldev,
>  	seq_printf(s, "function %s", fname);
>  }
>  
> -static void brcmstb_pctl_dt_free_map(struct pinctrl_dev *pctldev,
> -				     struct pinctrl_map *maps,
> -				     unsigned int num_maps)
> -{
> -	int i;
> -
> -	for (i = 0; i < num_maps; i++)
> -		if (maps[i].type == PIN_MAP_TYPE_CONFIGS_PIN)
> -			kfree(maps[i].data.configs.configs);
> -
> -	kfree(maps);
> -}
> -
>  static const struct pinctrl_ops brcmstb_pctl_ops = {
>  	.get_groups_count = brcmstb_pctl_get_groups_count,
>  	.get_group_name = brcmstb_pctl_get_group_name,
>  	.get_group_pins = brcmstb_pctl_get_group_pins,
>  	.pin_dbg_show = brcmstb_pctl_pin_dbg_show,
>  	.dt_node_to_map = pinconf_generic_dt_node_to_map_all,
> -	.dt_free_map = brcmstb_pctl_dt_free_map,
> +	.dt_free_map = pinconf_generic_dt_free_map,
>  };
>  
>  static int brcmstb_pmx_free(struct pinctrl_dev *pctldev, unsigned int offset)
> -- 
> 2.34.1
>