[PATCH v2] gpio: mmio: fix calculation of bgpio_bits

[PATCH v2] gpio: mmio: fix calculation of bgpio_bits

[Asmaa Mnebhi]

If the "ngpios" property is specified, bgpio_bits is calculated
as the round up value of ngpio. At the moment, the only requirement
specified is that the round up value must be a multiple of 8 but
it should also be a power of 2 because we provide accessors based
on the bank size in bgpio_setup_accessors().

Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/gpio-mmio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
index b52a3dd511ca..74fdf0d87b2c 100644
--- a/drivers/gpio/gpio-mmio.c
+++ b/drivers/gpio/gpio-mmio.c
@@ -623,7 +623,7 @@ int bgpio_init(struct gpio_chip *gc, struct device *dev,
 	if (ret)
 		gc->ngpio = gc->bgpio_bits;
 	else
-		gc->bgpio_bits = round_up(gc->ngpio, 8);
+		gc->bgpio_bits = roundup_pow_of_two(round_up(gc->ngpio, 8));
 
 	ret = bgpio_setup_io(gc, dat, set, clr, flags);
 	if (ret)
-- 
2.30.1

Re: [PATCH v2] gpio: mmio: fix calculation of bgpio_bits

[Andy Shevchenko]

On Tue, Jul 11, 2023 at 12:50 AM Asmaa Mnebhi <asmaa@nvidia.com> wrote:
>
> If the "ngpios" property is specified, bgpio_bits is calculated
> as the round up value of ngpio. At the moment, the only requirement
> specified is that the round up value must be a multiple of 8 but
> it should also be a power of 2 because we provide accessors based
> on the bank size in bgpio_setup_accessors().

Is this a fixup for the other patch? If so, then why did you split them again?

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v2] gpio: mmio: fix calculation of bgpio_bits

[Asmaa Mnebhi]

> > If the "ngpios" property is specified, bgpio_bits is calculated as the
> > round up value of ngpio. At the moment, the only requirement specified
> > is that the round up value must be a multiple of 8 but it should also
> > be a power of 2 because we provide accessors based on the bank size in
> > bgpio_setup_accessors().
> 
> Is this a fixup for the other patch? If so, then why did you split them again?
> 
Apologies, I might have misunderstood your previous comment. I thought you asked me to split it again so that it is a rebased continuation of the approved old patches.
To avoid any further misunderstandings on my part , could you please confirm the following:
1) I will create one patch combining both
2) The tag for this one patch will be "[PATCH v2] gpio: mmio: handle "ngpios" properly in bgpio_init()"
And Reviewed-By Andy and Linus.
3) I will add in the commit message , a changelog section as shown below:
Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
---
V1->v2:
- rebase + combination of the 2 patches.

Re: [PATCH v2] gpio: mmio: fix calculation of bgpio_bits

[Andy Shevchenko]

On Tue, Jul 11, 2023 at 3:23 PM Asmaa Mnebhi <asmaa@nvidia.com> wrote:
>
> > > If the "ngpios" property is specified, bgpio_bits is calculated as the
> > > round up value of ngpio. At the moment, the only requirement specified
> > > is that the round up value must be a multiple of 8 but it should also
> > > be a power of 2 because we provide accessors based on the bank size in
> > > bgpio_setup_accessors().
> >
> > Is this a fixup for the other patch? If so, then why did you split them again?
> >
> Apologies, I might have misunderstood your previous comment. I thought you asked me to split it again so that it is a rebased continuation of the approved old patches.

No, I asked to move the changelog section to be not a part of the
commit message and asked what happened to the tags, because you
dropped them.

> To avoid any further misunderstandings on my part , could you please confirm the following:
> 1) I will create one patch combining both

_If_ I understood the patch flow correctly, and it means that one is
the fix to the other one, then yes, otherwise no, keep them split with
appropriate Fixes tag added.

> 2) The tag for this one patch will be "[PATCH v2] gpio: mmio: handle "ngpios" properly in bgpio_init()"
> And Reviewed-By Andy and Linus.

What do you mean by "tag" in this context? Do you mean "Subject" or
title? Otherwise it's very confusing.
For the Rb tag by me, yes, please keep it. As for Linus' one I think
you should drop it and add a changelog entry to explain why, because
that tag is only for the "fix" part. Do not forget to Cc Linus.

> 3) I will add in the commit message , a changelog section as shown below:
> Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>

It's mandatory according to the Submitting Patches documentation. Read
it for clarification on how to use and when the tags.

> ---
> V1->v2:
> - rebase + combination of the 2 patches.

I believe this needs more entries. Since it's not part of the commit
message, you may use free wording to explain what exactly had happened
to the patches, with links, references, etc.

-- 
With Best Regards,
Andy Shevchenko