[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<BY5PR12MB4258577B994214AA6A509288DBC72@BY5PR12MB4258.namprd12.prod.outlook.com>
Date: Tue, 11 Jun 2024 07:03:51 +0000
From: "Potthuri, Sai Krishna" <sai.krishna.potthuri@....com>
To: Sean Anderson <sean.anderson@...ux.dev>
CC: Linus Walleij <linus.walleij@...aro.org>, "Simek, Michal"
<michal.simek@....com>, "linux-gpio@...r.kernel.org"
<linux-gpio@...r.kernel.org>, Andy Shevchenko <andy.shevchenko@...il.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v3 2/2] pinctrl: zynqmp: Support muxing individual pins
> -----Original Message-----
> From: Sean Anderson <sean.anderson@...ux.dev>
> Sent: Tuesday, June 11, 2024 4:06 AM
> To: Linus Walleij <linus.walleij@...aro.org>; Simek, Michal
> <michal.simek@....com>; linux-gpio@...r.kernel.org
> Cc: Andy Shevchenko <andy.shevchenko@...il.com>; Potthuri, Sai Krishna
> <sai.krishna.potthuri@....com>; linux-arm-kernel@...ts.infradead.org;
> linux-kernel@...r.kernel.org; Sean Anderson <sean.anderson@...ux.dev>
> Subject: [PATCH v3 2/2] pinctrl: zynqmp: Support muxing individual pins
>
> 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.
>
> There is not too much point in pin "groups" when there are not actual pin
> groups at the hardware level. The pins can all be muxed individually, so
> there's no point in adding artificial groups on top.
> Just mux the pins like the hardware allows.
>
> To this effect, 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 <sean.anderson@...ux.dev>
Reviewed-by: Sai Krishna Potthuri <sai.krishna.potthuri@....com>
Regards
Sai Krishna
> ---
>
> Changes in v3:
> - Fix zynqmp_pinmux_set_mux and zynqmp_pinconf_group_set not
> handling
> "pin" groups (thanks Sai Krishna).
>
> Changes in v2:
> - Use __set_bit instead of set_bit
> - Use size_add when calculating the number of kcalloc members
> - Expand commit message with some more motivation
>
> drivers/pinctrl/pinctrl-zynqmp.c | 83 +++++++++++++++++++++-----------
> 1 file changed, 54 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-zynqmp.c b/drivers/pinctrl/pinctrl-
> zynqmp.c
> index 5c46b7d7ebcb..e4ff3c55b87d 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;
> }
> @@ -197,17 +206,16 @@ static int zynqmp_pinmux_set_mux(struct
> pinctrl_dev *pctldev,
> unsigned int function,
> unsigned int group)
> {
> - struct zynqmp_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> - const struct zynqmp_pctrl_group *pgrp = &pctrl->groups[group];
> + const unsigned int *pins;
> + unsigned int npins;
> int ret, i;
>
> - for (i = 0; i < pgrp->npins; i++) {
> - unsigned int pin = pgrp->pins[i];
> -
> - ret = zynqmp_pm_pinctrl_set_function(pin, function);
> + zynqmp_pctrl_get_group_pins(pctldev, group, &pins, &npins);
> + for (i = 0; i < npins; i++) {
> + ret = zynqmp_pm_pinctrl_set_function(pins[i], function);
> if (ret) {
> dev_err(pctldev->dev, "set mux failed for pin %u\n",
> - pin);
> + pins[i]);
> return ret;
> }
> }
> @@ -467,12 +475,13 @@ static int zynqmp_pinconf_group_set(struct
> pinctrl_dev *pctldev,
> unsigned long *configs,
> unsigned int num_configs)
> {
> + const unsigned int *pins;
> + unsigned int npins;
> int i, ret;
> - struct zynqmp_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> - const struct zynqmp_pctrl_group *pgrp = &pctrl->groups[selector];
>
> - for (i = 0; i < pgrp->npins; i++) {
> - ret = zynqmp_pinconf_cfg_set(pctldev, pgrp->pins[i], configs,
> + zynqmp_pctrl_get_group_pins(pctldev, selector, &pins, &npins);
> + for (i = 0; i < npins; i++) {
> + ret = zynqmp_pinconf_cfg_set(pctldev, pins[i], configs,
> num_configs);
> if (ret)
> return ret;
> @@ -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, size_add(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
Powered by blists - more mailing lists