While muxing groups of pins at once can be convenient for large
interfaces, it can also be rigid. This is because the group is set to
all pins which support a particular function, even though not all pins
may be used. For example, the sdhci0 function may be used with a 8-bit
eMMC, 4-bit SD card, or even a 1-bit SD card. In these cases, the extra
pins may be repurposed for other uses, but this is not currently
allowed.
Add a new group for each pin which can be muxed. These groups are part
of each function the pin can be muxed to. We treat group selectors
beyond the number of groups as "pin" groups. To set this up, we
initialize groups before functions, and then create a bitmap of used
pins for each function. These used pins are appended to the function's
list of groups.
Signed-off-by: Sean Anderson <[email protected]>
---
drivers/pinctrl/pinctrl-zynqmp.c | 61 ++++++++++++++++++++++----------
1 file changed, 43 insertions(+), 18 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-zynqmp.c b/drivers/pinctrl/pinctrl-zynqmp.c
index 5c46b7d7ebcb..4829150ab069 100644
--- a/drivers/pinctrl/pinctrl-zynqmp.c
+++ b/drivers/pinctrl/pinctrl-zynqmp.c
@@ -10,6 +10,7 @@
#include <dt-bindings/pinctrl/pinctrl-zynqmp.h>
+#include <linux/bitmap.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/of_address.h>
@@ -97,7 +98,7 @@ static int zynqmp_pctrl_get_groups_count(struct pinctrl_dev *pctldev)
{
struct zynqmp_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
- return pctrl->ngroups;
+ return pctrl->ngroups + zynqmp_desc.npins;
}
static const char *zynqmp_pctrl_get_group_name(struct pinctrl_dev *pctldev,
@@ -105,7 +106,10 @@ static const char *zynqmp_pctrl_get_group_name(struct pinctrl_dev *pctldev,
{
struct zynqmp_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
- return pctrl->groups[selector].name;
+ if (selector < pctrl->ngroups)
+ return pctrl->groups[selector].name;
+
+ return zynqmp_desc.pins[selector - pctrl->ngroups].name;
}
static int zynqmp_pctrl_get_group_pins(struct pinctrl_dev *pctldev,
@@ -115,8 +119,13 @@ static int zynqmp_pctrl_get_group_pins(struct pinctrl_dev *pctldev,
{
struct zynqmp_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
- *pins = pctrl->groups[selector].pins;
- *npins = pctrl->groups[selector].npins;
+ if (selector < pctrl->ngroups) {
+ *pins = pctrl->groups[selector].pins;
+ *npins = pctrl->groups[selector].npins;
+ } else {
+ *pins = &zynqmp_desc.pins[selector - pctrl->ngroups].number;
+ *npins = 1;
+ }
return 0;
}
@@ -560,10 +569,12 @@ static int zynqmp_pinctrl_prepare_func_groups(struct device *dev, u32 fid,
{
u16 resp[NUM_GROUPS_PER_RESP] = {0};
const char **fgroups;
- int ret, index, i;
+ int ret, index, i, pin;
+ unsigned int npins;
+ unsigned long *used_pins __free(bitmap) =
+ bitmap_zalloc(zynqmp_desc.npins, GFP_KERNEL);
- fgroups = devm_kcalloc(dev, func->ngroups, sizeof(*fgroups), GFP_KERNEL);
- if (!fgroups)
+ if (!used_pins)
return -ENOMEM;
for (index = 0; index < func->ngroups; index += NUM_GROUPS_PER_RESP) {
@@ -578,23 +589,37 @@ static int zynqmp_pinctrl_prepare_func_groups(struct device *dev, u32 fid,
if (resp[i] == RESERVED_GROUP)
continue;
- fgroups[index + i] = devm_kasprintf(dev, GFP_KERNEL,
- "%s_%d_grp",
- func->name,
- index + i);
- if (!fgroups[index + i])
- return -ENOMEM;
-
groups[resp[i]].name = devm_kasprintf(dev, GFP_KERNEL,
"%s_%d_grp",
func->name,
index + i);
if (!groups[resp[i]].name)
return -ENOMEM;
+
+ for (pin = 0; pin < groups[resp[i]].npins; pin++)
+ set_bit(groups[resp[i]].pins[pin], used_pins);
}
}
done:
+ npins = bitmap_weight(used_pins, zynqmp_desc.npins);
+ fgroups = devm_kcalloc(dev, func->ngroups + npins, sizeof(*fgroups),
+ GFP_KERNEL);
+ if (!fgroups)
+ return -ENOMEM;
+
+ for (i = 0; i < func->ngroups; i++) {
+ fgroups[i] = devm_kasprintf(dev, GFP_KERNEL, "%s_%d_grp",
+ func->name, i);
+ if (!fgroups[i])
+ return -ENOMEM;
+ }
+
+ pin = 0;
+ for_each_set_bit(pin, used_pins, zynqmp_desc.npins)
+ fgroups[i++] = zynqmp_desc.pins[pin].name;
+
func->groups = fgroups;
+ func->ngroups += npins;
return 0;
}
@@ -772,6 +797,10 @@ static int zynqmp_pinctrl_prepare_function_info(struct device *dev,
if (!groups)
return -ENOMEM;
+ ret = zynqmp_pinctrl_prepare_group_pins(dev, groups, pctrl->ngroups);
+ if (ret)
+ return ret;
+
for (i = 0; i < pctrl->nfuncs; i++) {
ret = zynqmp_pinctrl_prepare_func_groups(dev, i, &funcs[i],
groups);
@@ -779,10 +808,6 @@ static int zynqmp_pinctrl_prepare_function_info(struct device *dev,
return ret;
}
- ret = zynqmp_pinctrl_prepare_group_pins(dev, groups, pctrl->ngroups);
- if (ret)
- return ret;
-
pctrl->funcs = funcs;
pctrl->groups = groups;
--
2.35.1.1320.gc452695387.dirty
Fri, May 03, 2024 at 12:22:17PM -0400, Sean Anderson kirjoitti:
> While muxing groups of pins at once can be convenient for large
> interfaces, it can also be rigid. This is because the group is set to
> all pins which support a particular function, even though not all pins
> may be used. For example, the sdhci0 function may be used with a 8-bit
> eMMC, 4-bit SD card, or even a 1-bit SD card. In these cases, the extra
> pins may be repurposed for other uses, but this is not currently
> allowed.
>
> Add a new group for each pin which can be muxed. These groups are part
> of each function the pin can be muxed to. We treat group selectors
> beyond the number of groups as "pin" groups. To set this up, we
> initialize groups before functions, and then create a bitmap of used
> pins for each function. These used pins are appended to the function's
> list of groups.
..
> + for (pin = 0; pin < groups[resp[i]].npins; pin++)
> + set_bit(groups[resp[i]].pins[pin], used_pins);
Why atomic bit operation?
..
> + fgroups = devm_kcalloc(dev, func->ngroups + npins, sizeof(*fgroups),
size_add() from overflow.h.
> + GFP_KERNEL);
> + if (!fgroups)
> + return -ENOMEM;
..
> + for (i = 0; i < func->ngroups; i++) {
> + fgroups[i] = devm_kasprintf(dev, GFP_KERNEL, "%s_%d_grp",
> + func->name, i);
> + if (!fgroups[i])
> + return -ENOMEM;
> + }
Hmm... Can this benefit from devm_kasprintf_strarray()?
--
With Best Regards,
Andy Shevchenko
On 5/6/24 15:26, Andy Shevchenko wrote:
> Fri, May 03, 2024 at 12:22:17PM -0400, Sean Anderson kirjoitti:
>> While muxing groups of pins at once can be convenient for large
>> interfaces, it can also be rigid. This is because the group is set to
>> all pins which support a particular function, even though not all pins
>> may be used. For example, the sdhci0 function may be used with a 8-bit
>> eMMC, 4-bit SD card, or even a 1-bit SD card. In these cases, the extra
>> pins may be repurposed for other uses, but this is not currently
>> allowed.
>>
>> Add a new group for each pin which can be muxed. These groups are part
>> of each function the pin can be muxed to. We treat group selectors
>> beyond the number of groups as "pin" groups. To set this up, we
>> initialize groups before functions, and then create a bitmap of used
>> pins for each function. These used pins are appended to the function's
>> list of groups.
>
> ...
>
>> + for (pin = 0; pin < groups[resp[i]].npins; pin++)
>> + set_bit(groups[resp[i]].pins[pin], used_pins);
>
> Why atomic bit operation?
The name was easier to remember. I can make it non-atomic.
> ...
>
>> + fgroups = devm_kcalloc(dev, func->ngroups + npins, sizeof(*fgroups),
>
> size_add() from overflow.h.
OK
>> + GFP_KERNEL);
>> + if (!fgroups)
>> + return -ENOMEM;
>
> ...
>
>> + for (i = 0; i < func->ngroups; i++) {
>> + fgroups[i] = devm_kasprintf(dev, GFP_KERNEL, "%s_%d_grp",
>> + func->name, i);
>> + if (!fgroups[i])
>> + return -ENOMEM;
>> + }
>
> Hmm... Can this benefit from devm_kasprintf_strarray()?
>
I don't think so, since the prefix is different for each group.
Thanks for the suggestions.
--Sean
On 5/6/24 15:38, Sean Anderson wrote:
> On 5/6/24 15:26, Andy Shevchenko wrote:
>> Fri, May 03, 2024 at 12:22:17PM -0400, Sean Anderson kirjoitti:
>>> While muxing groups of pins at once can be convenient for large
>>> interfaces, it can also be rigid. This is because the group is set to
>>> all pins which support a particular function, even though not all pins
>>> may be used. For example, the sdhci0 function may be used with a 8-bit
>>> eMMC, 4-bit SD card, or even a 1-bit SD card. In these cases, the extra
>>> pins may be repurposed for other uses, but this is not currently
>>> allowed.
>>>
>>> Add a new group for each pin which can be muxed. These groups are part
>>> of each function the pin can be muxed to. We treat group selectors
>>> beyond the number of groups as "pin" groups. To set this up, we
>>> initialize groups before functions, and then create a bitmap of used
>>> pins for each function. These used pins are appended to the function's
>>> list of groups.
>>
>> ...
>>
>>> + for (pin = 0; pin < groups[resp[i]].npins; pin++)
>>> + set_bit(groups[resp[i]].pins[pin], used_pins);
>>
>> Why atomic bit operation?
>
> The name was easier to remember. I can make it non-atomic.
>
>> ...
>>
>>> + fgroups = devm_kcalloc(dev, func->ngroups + npins, sizeof(*fgroups),
>>
>> size_add() from overflow.h.
>
> OK
>
>>> + GFP_KERNEL);
>>> + if (!fgroups)
>>> + return -ENOMEM;
>>
>> ...
>>
>>> + for (i = 0; i < func->ngroups; i++) {
>>> + fgroups[i] = devm_kasprintf(dev, GFP_KERNEL, "%s_%d_grp",
>>> + func->name, i);
>>> + if (!fgroups[i])
>>> + return -ENOMEM;
>>> + }
>>
>> Hmm... Can this benefit from devm_kasprintf_strarray()?
>>
>
> I don't think so, since the prefix is different for each group.
Sorry, the prefix is the same, but we have to use this format as to not
break the devicetree ABI.
--Sean