diff mbox series

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

Message ID 20240326222844.1422948-7-prabhakar.mahadev-lad.rj@bp.renesas.com
State New
Headers show
Series Add PFC support for Renesas RZ/V2H(P) SoC | expand

Commit Message

Lad, Prabhakar March 26, 2024, 10:28 p.m. UTC 
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(-)

Comments

Geert Uytterhoeven March 28, 2024, 2:13 p.m. UTC | #1 
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
Lad, Prabhakar March 28, 2024, 7:53 p.m. UTC | #2 
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
diff mbox series

Patch

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;
 };