diff mbox series

[11/15] pinctrl: sh-pfc: Print actual field width for variable-width fields

Message ID 20181213182714.26094-12-geert+renesas@glider.be (mailing list archive)
State New, archived
Headers show
Series pinctrl: sh-pfc: Fix config register descriptions | expand

Commit Message

Geert Uytterhoeven Dec. 13, 2018, 6:27 p.m. UTC
The debug code in sh_pfc_write_config_reg() prints the width of the
field being modified.

However, registers with a variable-width field layout are identified by
pinmux_cfg_reg.field_width being zero, hence zeroes are printed instead
of the actual field widths.

Fix this by printing the Hamming height of the field mask instead, which
is correct for both fixed-width and variable-width fields.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pinctrl/sh-pfc/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Simon Horman Dec. 17, 2018, 2:46 p.m. UTC | #1
On Thu, Dec 13, 2018 at 07:27:10PM +0100, Geert Uytterhoeven wrote:
> The debug code in sh_pfc_write_config_reg() prints the width of the
> field being modified.
> 
> However, registers with a variable-width field layout are identified by
> pinmux_cfg_reg.field_width being zero, hence zeroes are printed instead
> of the actual field widths.
> 
> Fix this by printing the Hamming height of the field mask instead, which
> is correct for both fixed-width and variable-width fields.

s/height/width ?

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> ---
>  drivers/pinctrl/sh-pfc/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
> index a10f7050a74f35ff..f1cfcc8c65446662 100644
> --- a/drivers/pinctrl/sh-pfc/core.c
> +++ b/drivers/pinctrl/sh-pfc/core.c
> @@ -221,7 +221,7 @@ static void sh_pfc_write_config_reg(struct sh_pfc *pfc,
>  
>  	dev_dbg(pfc->dev, "write_reg addr = %x, value = 0x%x, field = %u, "
>  		"r_width = %u, f_width = %u\n",
> -		crp->reg, value, field, crp->reg_width, crp->field_width);
> +		crp->reg, value, field, crp->reg_width, hweight32(mask));
>  
>  	mask = ~(mask << pos);
>  	value = value << pos;
> -- 
> 2.17.1
>
Geert Uytterhoeven Dec. 17, 2018, 2:58 p.m. UTC | #2
Hi Simon,

On Mon, Dec 17, 2018 at 3:46 PM Simon Horman <horms@verge.net.au> wrote:
> On Thu, Dec 13, 2018 at 07:27:10PM +0100, Geert Uytterhoeven wrote:
> > The debug code in sh_pfc_write_config_reg() prints the width of the
> > field being modified.
> >
> > However, registers with a variable-width field layout are identified by
> > pinmux_cfg_reg.field_width being zero, hence zeroes are printed instead
> > of the actual field widths.
> >
> > Fix this by printing the Hamming height of the field mask instead, which
> > is correct for both fixed-width and variable-width fields.
>
> s/height/width ?

Nah, weight. Will fix.
https://en.wikipedia.org/wiki/Hamming_weight

> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Thanks!

Gr{oetje,eeting}s,

                        Geert
Simon Horman Dec. 17, 2018, 3:17 p.m. UTC | #3
On Mon, Dec 17, 2018 at 03:58:52PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Mon, Dec 17, 2018 at 3:46 PM Simon Horman <horms@verge.net.au> wrote:
> > On Thu, Dec 13, 2018 at 07:27:10PM +0100, Geert Uytterhoeven wrote:
> > > The debug code in sh_pfc_write_config_reg() prints the width of the
> > > field being modified.
> > >
> > > However, registers with a variable-width field layout are identified by
> > > pinmux_cfg_reg.field_width being zero, hence zeroes are printed instead
> > > of the actual field widths.
> > >
> > > Fix this by printing the Hamming height of the field mask instead, which
> > > is correct for both fixed-width and variable-width fields.
> >
> > s/height/width ?
> 
> Nah, weight. Will fix.
> https://en.wikipedia.org/wiki/Hamming_weight

I was thinking weight :)

> 
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> 
> Thanks!
> 
> 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
>
diff mbox series

Patch

diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
index a10f7050a74f35ff..f1cfcc8c65446662 100644
--- a/drivers/pinctrl/sh-pfc/core.c
+++ b/drivers/pinctrl/sh-pfc/core.c
@@ -221,7 +221,7 @@  static void sh_pfc_write_config_reg(struct sh_pfc *pfc,
 
 	dev_dbg(pfc->dev, "write_reg addr = %x, value = 0x%x, field = %u, "
 		"r_width = %u, f_width = %u\n",
-		crp->reg, value, field, crp->reg_width, crp->field_width);
+		crp->reg, value, field, crp->reg_width, hweight32(mask));
 
 	mask = ~(mask << pos);
 	value = value << pos;