Re: [PATCH RTF 1/2] gpiolib: of: Use local variables

Alex Elder
2025-02-24T14:43:04Z

On 2/18/25 4:28 AM, Linus Walleij wrote:
> Instead of modifying the contents of the array of valued read

s/valued/values/

> in from a phandle, use local variables to store the values.
> This makes the code easier to read and the array immutable.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

In addition to preparing for the next patch, this is much more
readable.

Reviewed-by: Alex Elder <elder@riscstar.com>

> ---
> drivers/gpio/gpiolib-of.c | 39 +++++++++++++++++++++++----------------
> 1 file changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index 2e537ee979f3e2b6e8d5f86f3e121a66f2a8e083..86405218f4e2ddc951a1a9d168e886400652bf60 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -1057,6 +1057,9 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
> const char *name;
> static const char group_names_propname[] = "gpio-ranges-group-names";
> bool has_group_names;
> + int offset; /* Offset of the first GPIO line on the chip */
> + int pin; /* Pin base number in the range */
> + int count; /* Number of pins/GPIO lines to map */
>
> np = dev_of_node(&chip->gpiodev->dev);
> if (!np)
> @@ -1075,13 +1078,17 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
> if (!pctldev)
> return -EPROBE_DEFER;
>
> + offset = pinspec.args[0];
> + pin = pinspec.args[1];
> + count = pinspec.args[2];
> +
> /* Ignore ranges outside of this GPIO chip */
> - if (pinspec.args[0] >= (chip->offset + chip->ngpio))
> + if (offset >= (chip->offset + chip->ngpio))
> continue;
> - if (pinspec.args[0] + pinspec.args[2] <= chip->offset)
> + if (offset + count <= chip->offset)
> continue;
>
> - if (pinspec.args[2]) {
> + if (count) {
> /* npins != 0: linear range */
> if (has_group_names) {
> of_property_read_string_index(np,
> @@ -1095,27 +1102,27 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
> }
>
> /* Trim the range to fit this GPIO chip */
> - if (chip->offset > pinspec.args[0]) {
> - trim = chip->offset - pinspec.args[0];
> - pinspec.args[2] -= trim;
> - pinspec.args[1] += trim;
> - pinspec.args[0] = 0;
> + if (chip->offset > offset) {
> + trim = chip->offset - offset;
> + count -= trim;
> + pin += trim;
> + offset = 0;
> } else {
> - pinspec.args[0] -= chip->offset;
> + offset -= chip->offset;
> }
> - if ((pinspec.args[0] + pinspec.args[2]) > chip->ngpio)
> - pinspec.args[2] = chip->ngpio - pinspec.args[0];
> + if ((offset + count) > chip->ngpio)
> + count = chip->ngpio - offset;
>
> ret = gpiochip_add_pin_range(chip,
> pinctrl_dev_get_devname(pctldev),
> - pinspec.args[0],
> - pinspec.args[1],
> - pinspec.args[2]);
> + offset,
> + pin,
> + count);
> if (ret)
> return ret;
> } else {
> /* npins == 0: special range */
> - if (pinspec.args[1]) {
> + if (pin) {
> pr_err("%pOF: Illegal gpio-range format.\n",
> np);
> break;
> @@ -1140,7 +1147,7 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
> }
>
> ret = gpiochip_add_pingroup_range(chip, pctldev,
> - pinspec.args[0], name);
> + offset, name);
> if (ret)
> return ret;
> }
>