diff mbox series

[10/15] pinctrl: sh-pfc: sh7734: Fix shifted values in IPSR10

Message ID 20181213182714.26094-11-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 
Some values in the Peripheral Function Select Register 10 descriptor are
shifted by one position, which may cause a peripheral function to be
programmed incorrectly.

Fixing this makes all HSCIF0 pins use Function 4 (value 3), like was
already the case for the HSCK0 pin in field IP10[5:3].

Fixes: ac1ebc2190f575fc ("sh-pfc: Add sh7734 pinmux support")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Based on the SH7734 Hardware User's Manual Rev. 1.00.
Compile-tested only.

Noticed when investigating a different bug in the same register
description.
---
 drivers/pinctrl/sh-pfc/pfc-sh7734.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Simon Horman Dec. 17, 2018, 2:37 p.m. UTC | #1 
On Thu, Dec 13, 2018 at 07:27:09PM +0100, Geert Uytterhoeven wrote:
> Some values in the Peripheral Function Select Register 10 descriptor are
> shifted by one position, which may cause a peripheral function to be
> programmed incorrectly.
> 
> Fixing this makes all HSCIF0 pins use Function 4 (value 3), like was
> already the case for the HSCK0 pin in field IP10[5:3].
> 
> Fixes: ac1ebc2190f575fc ("sh-pfc: Add sh7734 pinmux support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Based on the SH7734 Hardware User's Manual Rev. 1.00.
> Compile-tested only.
> 
> Noticed when investigating a different bug in the same register
> description.
> ---
>  drivers/pinctrl/sh-pfc/pfc-sh7734.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/pfc-sh7734.c b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> index cad70f9cf5699f0c..748a32a3af82d368 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> @@ -2210,22 +2210,22 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
>  	    /* IP10_22 [1] */
>  		FN_CAN_CLK_A, FN_RX4_D,
>  	    /* IP10_21_19 [3] */
> -		FN_AUDIO_CLKOUT, FN_TX1_E, FN_HRTS0_C, FN_FSE_B,
> -		FN_LCD_M_DISP_B, 0, 0, 0,
> +		FN_AUDIO_CLKOUT, FN_TX1_E, 0, FN_HRTS0_C, FN_FSE_B,

FN_FSE_B does no appear to be documented for this field in Users' Manual
Hardware v1.00 (Jun 12, 2012). Should it be 0?

Otherwise this patch looks good.


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


> +		FN_LCD_M_DISP_B, 0, 0,
>  	    /* IP10_18_16 [3] */
> -		FN_AUDIO_CLKC, FN_SCK1_E, FN_HCTS0_C, FN_FRB_B,
> -		FN_LCD_VEPWC_B, 0, 0, 0,
> +		FN_AUDIO_CLKC, FN_SCK1_E, 0, FN_HCTS0_C, FN_FRB_B,
> +		FN_LCD_VEPWC_B, 0, 0,
>  	    /* IP10_15 [1] */
>  		FN_AUDIO_CLKB_A, FN_LCD_CLK_B,
>  	    /* IP10_14_12 [3] */
>  		FN_AUDIO_CLKA_A, FN_VI1_CLK_B, FN_SCK1_D, FN_IECLK_B,
>  		FN_LCD_FLM_B, 0, 0, 0,
>  	    /* IP10_11_9 [3] */
> -		FN_SSI_SDATA3, FN_VI1_7_B, FN_HTX0_C, FN_FWE_B,
> -		FN_LCD_CL2_B, 0, 0, 0,
> +		FN_SSI_SDATA3, FN_VI1_7_B, 0, FN_HTX0_C, FN_FWE_B,
> +		FN_LCD_CL2_B, 0, 0,
>  	    /* IP10_8_6 [3] */
> -		FN_SSI_SDATA2, FN_VI1_6_B, FN_HRX0_C, FN_FRE_B,
> -		FN_LCD_CL1_B, 0, 0, 0,
> +		FN_SSI_SDATA2, FN_VI1_6_B, 0, FN_HRX0_C, FN_FRE_B,
> +		FN_LCD_CL1_B, 0, 0,
>  	    /* IP10_5_3 [3] */
>  		FN_SSI_WS23, FN_VI1_5_B, FN_TX1_D, FN_HSCK0_C, FN_FALE_B,
>  		FN_LCD_DON_B, 0, 0,
> -- 
> 2.17.1
>
Geert Uytterhoeven Dec. 17, 2018, 2:45 p.m. UTC | #2 
Hi Simon,

On Mon, Dec 17, 2018 at 3:37 PM Simon Horman <horms@verge.net.au> wrote:
> On Thu, Dec 13, 2018 at 07:27:09PM +0100, Geert Uytterhoeven wrote:
> > Some values in the Peripheral Function Select Register 10 descriptor are
> > shifted by one position, which may cause a peripheral function to be
> > programmed incorrectly.
> >
> > Fixing this makes all HSCIF0 pins use Function 4 (value 3), like was
> > already the case for the HSCK0 pin in field IP10[5:3].
> >
> > Fixes: ac1ebc2190f575fc ("sh-pfc: Add sh7734 pinmux support")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > Based on the SH7734 Hardware User's Manual Rev. 1.00.
> > Compile-tested only.
> >
> > Noticed when investigating a different bug in the same register
> > description.
> > ---
> >  drivers/pinctrl/sh-pfc/pfc-sh7734.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/pinctrl/sh-pfc/pfc-sh7734.c b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> > index cad70f9cf5699f0c..748a32a3af82d368 100644
> > --- a/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> > +++ b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> > @@ -2210,22 +2210,22 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
> >           /* IP10_22 [1] */
> >               FN_CAN_CLK_A, FN_RX4_D,
> >           /* IP10_21_19 [3] */
> > -             FN_AUDIO_CLKOUT, FN_TX1_E, FN_HRTS0_C, FN_FSE_B,
> > -             FN_LCD_M_DISP_B, 0, 0, 0,
> > +             FN_AUDIO_CLKOUT, FN_TX1_E, 0, FN_HRTS0_C, FN_FSE_B,
>
> FN_FSE_B does no appear to be documented for this field in Users' Manual
> Hardware v1.00 (Jun 12, 2012). Should it be 0?

None of the FSE bits seem to be documented in the public datasheet, so I
just retained it, maintaining consistency with the surrounding fields.

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

Thanks!

Gr{oetje,eeting}s,

                        Geert
Simon Horman Dec. 17, 2018, 3:18 p.m. UTC | #3 
On Mon, Dec 17, 2018 at 03:45:26PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Mon, Dec 17, 2018 at 3:37 PM Simon Horman <horms@verge.net.au> wrote:
> > On Thu, Dec 13, 2018 at 07:27:09PM +0100, Geert Uytterhoeven wrote:
> > > Some values in the Peripheral Function Select Register 10 descriptor are
> > > shifted by one position, which may cause a peripheral function to be
> > > programmed incorrectly.
> > >
> > > Fixing this makes all HSCIF0 pins use Function 4 (value 3), like was
> > > already the case for the HSCK0 pin in field IP10[5:3].
> > >
> > > Fixes: ac1ebc2190f575fc ("sh-pfc: Add sh7734 pinmux support")
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > ---
> > > Based on the SH7734 Hardware User's Manual Rev. 1.00.
> > > Compile-tested only.
> > >
> > > Noticed when investigating a different bug in the same register
> > > description.
> > > ---
> > >  drivers/pinctrl/sh-pfc/pfc-sh7734.c | 16 ++++++++--------
> > >  1 file changed, 8 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/sh-pfc/pfc-sh7734.c b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> > > index cad70f9cf5699f0c..748a32a3af82d368 100644
> > > --- a/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> > > +++ b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> > > @@ -2210,22 +2210,22 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
> > >           /* IP10_22 [1] */
> > >               FN_CAN_CLK_A, FN_RX4_D,
> > >           /* IP10_21_19 [3] */
> > > -             FN_AUDIO_CLKOUT, FN_TX1_E, FN_HRTS0_C, FN_FSE_B,
> > > -             FN_LCD_M_DISP_B, 0, 0, 0,
> > > +             FN_AUDIO_CLKOUT, FN_TX1_E, 0, FN_HRTS0_C, FN_FSE_B,
> >
> > FN_FSE_B does no appear to be documented for this field in Users' Manual
> > Hardware v1.00 (Jun 12, 2012). Should it be 0?
> 
> None of the FSE bits seem to be documented in the public datasheet, so I
> just retained it, maintaining consistency with the surrounding fields.

That is fine by me.

> > 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/pfc-sh7734.c b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
index cad70f9cf5699f0c..748a32a3af82d368 100644
--- a/drivers/pinctrl/sh-pfc/pfc-sh7734.c
+++ b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
@@ -2210,22 +2210,22 @@  static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 	    /* IP10_22 [1] */
 		FN_CAN_CLK_A, FN_RX4_D,
 	    /* IP10_21_19 [3] */
-		FN_AUDIO_CLKOUT, FN_TX1_E, FN_HRTS0_C, FN_FSE_B,
-		FN_LCD_M_DISP_B, 0, 0, 0,
+		FN_AUDIO_CLKOUT, FN_TX1_E, 0, FN_HRTS0_C, FN_FSE_B,
+		FN_LCD_M_DISP_B, 0, 0,
 	    /* IP10_18_16 [3] */
-		FN_AUDIO_CLKC, FN_SCK1_E, FN_HCTS0_C, FN_FRB_B,
-		FN_LCD_VEPWC_B, 0, 0, 0,
+		FN_AUDIO_CLKC, FN_SCK1_E, 0, FN_HCTS0_C, FN_FRB_B,
+		FN_LCD_VEPWC_B, 0, 0,
 	    /* IP10_15 [1] */
 		FN_AUDIO_CLKB_A, FN_LCD_CLK_B,
 	    /* IP10_14_12 [3] */
 		FN_AUDIO_CLKA_A, FN_VI1_CLK_B, FN_SCK1_D, FN_IECLK_B,
 		FN_LCD_FLM_B, 0, 0, 0,
 	    /* IP10_11_9 [3] */
-		FN_SSI_SDATA3, FN_VI1_7_B, FN_HTX0_C, FN_FWE_B,
-		FN_LCD_CL2_B, 0, 0, 0,
+		FN_SSI_SDATA3, FN_VI1_7_B, 0, FN_HTX0_C, FN_FWE_B,
+		FN_LCD_CL2_B, 0, 0,
 	    /* IP10_8_6 [3] */
-		FN_SSI_SDATA2, FN_VI1_6_B, FN_HRX0_C, FN_FRE_B,
-		FN_LCD_CL1_B, 0, 0, 0,
+		FN_SSI_SDATA2, FN_VI1_6_B, 0, FN_HRX0_C, FN_FRE_B,
+		FN_LCD_CL1_B, 0, 0,
 	    /* IP10_5_3 [3] */
 		FN_SSI_WS23, FN_VI1_5_B, FN_TX1_D, FN_HSCK0_C, FN_FALE_B,
 		FN_LCD_DON_B, 0, 0,