[RFC PATCH 06/13] pinctrl: renesas: pinctrl-rzg2l: Make cfg to u64 in struct rzg2l_variable_pin_cfg

Prabhakar posted 13 patches 1 year, 10 months ago
There is a newer version of this series
[RFC PATCH 06/13] pinctrl: renesas: pinctrl-rzg2l: Make cfg to u64 in struct rzg2l_variable_pin_cfg
Posted by Prabhakar 1 year, 10 months ago
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Now that we have updated the macro PIN_CFG_MASK to allow for the maximum
configuration bits, update the size of 'cfg' to 'u64' in the
'struct rzg2l_variable_pin_cfg'.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index da3a54b7b06a..348fdccaff72 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -241,7 +241,7 @@ struct rzg2l_dedicated_configs {
  * @pin: port pin
  */
 struct rzg2l_variable_pin_cfg {
-	u32 cfg:20;
+	u64 cfg:46;
 	u32 port:5;
 	u32 pin:3;
 };
-- 
2.34.1
Re: [RFC PATCH 06/13] pinctrl: renesas: pinctrl-rzg2l: Make cfg to u64 in struct rzg2l_variable_pin_cfg
Posted by Geert Uytterhoeven 1 year, 10 months ago
Hi Prabhakar,

On Tue, Mar 26, 2024 at 11:30 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Now that we have updated the macro PIN_CFG_MASK to allow for the maximum
> configuration bits, update the size of 'cfg' to 'u64' in the
> 'struct rzg2l_variable_pin_cfg'.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -241,7 +241,7 @@ struct rzg2l_dedicated_configs {
>   * @pin: port pin
>   */
>  struct rzg2l_variable_pin_cfg {
> -       u32 cfg:20;
> +       u64 cfg:46;
>         u32 port:5;
>         u32 pin:3;

Doesn't this store the 46 cfg bits in a 64-bit word, and the 8 port
and pin bits in a different 32-bit word?  Worse, you'll get 4 bytes
of padding at the end of the structure.
Changing the port and pin to u64 should make sure everything is
stored together in a single 64-bit word.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [RFC PATCH 06/13] pinctrl: renesas: pinctrl-rzg2l: Make cfg to u64 in struct rzg2l_variable_pin_cfg
Posted by Lad, Prabhakar 1 year, 10 months ago
Hi Geert,

Thank you for the review.

On Thu, Mar 28, 2024 at 2:14 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Tue, Mar 26, 2024 at 11:30 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Now that we have updated the macro PIN_CFG_MASK to allow for the maximum
> > configuration bits, update the size of 'cfg' to 'u64' in the
> > 'struct rzg2l_variable_pin_cfg'.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -241,7 +241,7 @@ struct rzg2l_dedicated_configs {
> >   * @pin: port pin
> >   */
> >  struct rzg2l_variable_pin_cfg {
> > -       u32 cfg:20;
> > +       u64 cfg:46;
> >         u32 port:5;
> >         u32 pin:3;
>
> Doesn't this store the 46 cfg bits in a 64-bit word, and the 8 port
> and pin bits in a different 32-bit word?  Worse, you'll get 4 bytes
> of padding at the end of the structure.
Agreed.

> Changing the port and pin to u64 should make sure everything is
> stored together in a single 64-bit word.
>
I'll change the port and pin to u64 .

Cheers,
Prabhakar