| Message ID | 1c6cc202-5f20-4304-11ae-79ea0c559457@cogentembedded.com |
|---|---|
| State | New |
| Headers | show |
| Series | pinctrl: sh-pfc: r8a77970: add pin I/O voltage control | expand |
On 04/13/2018 09:29 PM, Sergei Shtylyov wrote: > Add the pin I/O voltage level control to the R8A77980 PFC driver. > > Loosely based on the original (and large) patch by Vladimir Barinov. > > Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com> > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Oops, forgot to mention that the patch was against the 'sj-pfc' branch of Geert's 'renesas-drivers.git' repo... [...] MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/13/2018 09:31 PM, Sergei Shtylyov wrote: >> Add the pin I/O voltage level control to the R8A77980 PFC driver. >> >> Loosely based on the original (and large) patch by Vladimir Barinov. >> >> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com> >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > Oops, forgot to mention that the patch was against the 'sj-pfc' branch The branch is called 'sh-pfc', of course. Sigh... > of Geert's 'renesas-drivers.git' repo... > > [...] MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sergei, Thanks for your patch! On Fri, Apr 13, 2018 at 8:29 PM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > Add the pin I/O voltage level control to the R8A77980 PFC driver. Subject says r8a77970? > Loosely based on the original (and large) patch by Vladimir Barinov. > > Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com> > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > --- > drivers/pinctrl/sh-pfc/pfc-r8a77980.c | 50 +++++++++++++++++++++++++++++++--- > 1 file changed, 47 insertions(+), 3 deletions(-) > > Index: renesas-drivers/drivers/pinctrl/sh-pfc/pfc-r8a77980.c > =================================================================== > --- renesas-drivers.orig/drivers/pinctrl/sh-pfc/pfc-r8a77980.c > +++ renesas-drivers/drivers/pinctrl/sh-pfc/pfc-r8a77980.c Ah, pfc-r8a77980.c it is. > @@ -2779,8 +2779,51 @@ static const struct pinmux_cfg_reg pinmu > { }, > }; > > +enum ioctrl_regs { > + IOCTRL30, > + IOCTRL31, > + IOCTRL32, > +}; > + > +static const struct pinmux_ioctrl_reg pinmux_ioctrl_regs[] = { > + [IOCTRL30] = { 0xe6060380, }, > + [IOCTRL31] = { 0xe6060384, }, > + [IOCTRL32] = { 0xe6060388, }, > + { /* sentinel */ }, However, r8a77980 has 4 IOCTRL3x registers (r8a77970 has three)? Something is wrong: which SoC source file is this patch for? Gr{oetje,eeting}s, Geert
Hello! On 04/16/2018 04:02 PM, Geert Uytterhoeven wrote: >> Add the pin I/O voltage level control to the R8A77980 PFC driver. > > Subject says r8a77970? Typo, I guess. :-) >> Loosely based on the original (and large) patch by Vladimir Barinov. >> >> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com> >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >> >> --- >> drivers/pinctrl/sh-pfc/pfc-r8a77980.c | 50 +++++++++++++++++++++++++++++++--- >> 1 file changed, 47 insertions(+), 3 deletions(-) >> >> Index: renesas-drivers/drivers/pinctrl/sh-pfc/pfc-r8a77980.c >> =================================================================== >> --- renesas-drivers.orig/drivers/pinctrl/sh-pfc/pfc-r8a77980.c >> +++ renesas-drivers/drivers/pinctrl/sh-pfc/pfc-r8a77980.c > > Ah, pfc-r8a77980.c it is. > >> @@ -2779,8 +2779,51 @@ static const struct pinmux_cfg_reg pinmu >> { }, >> }; >> >> +enum ioctrl_regs { >> + IOCTRL30, >> + IOCTRL31, >> + IOCTRL32, >> +}; >> + >> +static const struct pinmux_ioctrl_reg pinmux_ioctrl_regs[] = { >> + [IOCTRL30] = { 0xe6060380, }, >> + [IOCTRL31] = { 0xe6060384, }, >> + [IOCTRL32] = { 0xe6060388, }, >> + { /* sentinel */ }, > > However, r8a77980 has 4 IOCTRL3x registers (r8a77970 has three)? I thought that since we don't change it, no need to list it for save/restore either. I was wrong? > Something is wrong: which SoC source file is this patch for? R8A77980. > Gr{oetje,eeting}s, > > Geert MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sergei, On Mon, Apr 16, 2018 at 5:06 PM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > On 04/16/2018 04:02 PM, Geert Uytterhoeven wrote: >>> Add the pin I/O voltage level control to the R8A77980 PFC driver. >> >> Subject says r8a77970? > > Typo, I guess. :-) > >>> Loosely based on the original (and large) patch by Vladimir Barinov. >>> >>> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com> >>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >>> >>> --- >>> drivers/pinctrl/sh-pfc/pfc-r8a77980.c | 50 +++++++++++++++++++++++++++++++--- >>> 1 file changed, 47 insertions(+), 3 deletions(-) >>> >>> Index: renesas-drivers/drivers/pinctrl/sh-pfc/pfc-r8a77980.c >>> =================================================================== >>> --- renesas-drivers.orig/drivers/pinctrl/sh-pfc/pfc-r8a77980.c >>> +++ renesas-drivers/drivers/pinctrl/sh-pfc/pfc-r8a77980.c >> >> Ah, pfc-r8a77980.c it is. >> >>> @@ -2779,8 +2779,51 @@ static const struct pinmux_cfg_reg pinmu >>> { }, >>> }; >>> >>> +enum ioctrl_regs { >>> + IOCTRL30, >>> + IOCTRL31, >>> + IOCTRL32, >>> +}; >>> + >>> +static const struct pinmux_ioctrl_reg pinmux_ioctrl_regs[] = { >>> + [IOCTRL30] = { 0xe6060380, }, >>> + [IOCTRL31] = { 0xe6060384, }, >>> + [IOCTRL32] = { 0xe6060388, }, >>> + { /* sentinel */ }, >> >> However, r8a77980 has 4 IOCTRL3x registers (r8a77970 has three)? > > I thought that since we don't change it, no need to list it for save/restore > either. I was wrong? Given the system resume path differs from the boot path (resume skips the boot loader, and thus all PFC initialization it performs), I think it is safest to list it anyway, so it is saved and restored. >> Something is wrong: which SoC source file is this patch for? > > R8A77980. Thanks for confirming, will resume reviewing (either this version, or v2 ;-) Gr{oetje,eeting}s, Geert
Hi Sergei, On Fri, Apr 13, 2018 at 8:29 PM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > Add the pin I/O voltage level control to the R8A77980 PFC driver. > > Loosely based on the original (and large) patch by Vladimir Barinov. > > Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com> > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Thanks for your patch! > --- renesas-drivers.orig/drivers/pinctrl/sh-pfc/pfc-r8a77980.c > +++ renesas-drivers/drivers/pinctrl/sh-pfc/pfc-r8a77980.c > @@ -2779,8 +2779,51 @@ static const struct pinmux_cfg_reg pinmu > { }, > }; > > +enum ioctrl_regs { > + IOCTRL30, > + IOCTRL31, > + IOCTRL32, > +}; > + > +static const struct pinmux_ioctrl_reg pinmux_ioctrl_regs[] = { > + [IOCTRL30] = { 0xe6060380, }, > + [IOCTRL31] = { 0xe6060384, }, > + [IOCTRL32] = { 0xe6060388, }, I'd add IOCTRL33, so it is saved/restored during system suspend/resume. You never know what U-Boot wrote to it, which is skipped on system resume. Note for the future: unlike all other current pocctrl handling, IOCTRL33 is used to select between 2.5V and 3.3V (instead of 1.8V and 3.3V. Handling that will require changes to the r8a77980_pin_to_pocctrl() interface (add two output parameters returning supported voltages?). With IOCTRL33 added: Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
Index: renesas-drivers/drivers/pinctrl/sh-pfc/pfc-r8a77980.c =================================================================== --- renesas-drivers.orig/drivers/pinctrl/sh-pfc/pfc-r8a77980.c +++ renesas-drivers/drivers/pinctrl/sh-pfc/pfc-r8a77980.c @@ -19,10 +19,10 @@ #include "sh_pfc.h" #define CPU_ALL_PORT(fn, sfx) \ - PORT_GP_22(0, fn, sfx), \ + PORT_GP_CFG_22(0, fn, sfx, SH_PFC_PIN_CFG_IO_VOLTAGE), \ PORT_GP_28(1, fn, sfx), \ - PORT_GP_30(2, fn, sfx), \ - PORT_GP_17(3, fn, sfx), \ + PORT_GP_CFG_30(2, fn, sfx, SH_PFC_PIN_CFG_IO_VOLTAGE), \ + PORT_GP_CFG_17(3, fn, sfx, SH_PFC_PIN_CFG_IO_VOLTAGE), \ PORT_GP_25(4, fn, sfx), \ PORT_GP_15(5, fn, sfx) @@ -2779,8 +2779,51 @@ static const struct pinmux_cfg_reg pinmu { }, }; +enum ioctrl_regs { + IOCTRL30, + IOCTRL31, + IOCTRL32, +}; + +static const struct pinmux_ioctrl_reg pinmux_ioctrl_regs[] = { + [IOCTRL30] = { 0xe6060380, }, + [IOCTRL31] = { 0xe6060384, }, + [IOCTRL32] = { 0xe6060388, }, + { /* sentinel */ }, +}; + +static int r8a77980_pin_to_pocctrl(struct sh_pfc *pfc, unsigned int pin, + u32 *pocctrl) +{ + int bit = pin & 0x1f; + + *pocctrl = pinmux_ioctrl_regs[IOCTRL30].reg; + if (pin >= RCAR_GP_PIN(0, 0) && pin <= RCAR_GP_PIN(0, 21)) + return bit; + else if (pin >= RCAR_GP_PIN(2, 0) && pin <= RCAR_GP_PIN(2, 9)) + return bit + 22; + + *pocctrl = pinmux_ioctrl_regs[IOCTRL31].reg; + if (pin >= RCAR_GP_PIN(2, 10) && pin <= RCAR_GP_PIN(2, 16)) + return bit - 10; + if ((pin >= RCAR_GP_PIN(2, 17) && pin <= RCAR_GP_PIN(2, 24)) || + (pin >= RCAR_GP_PIN(3, 0) && pin <= RCAR_GP_PIN(3, 16))) + return bit + 7; + + *pocctrl = pinmux_ioctrl_regs[IOCTRL32].reg; + if (pin >= RCAR_GP_PIN(2, 25) && pin <= RCAR_GP_PIN(2, 29)) + return pin - 25; + + return -EINVAL; +} + +static const struct sh_pfc_soc_operations pinmux_ops = { + .pin_to_pocctrl = r8a77980_pin_to_pocctrl, +}; + const struct sh_pfc_soc_info r8a77980_pinmux_info = { .name = "r8a77980_pfc", + .ops = &pinmux_ops, .unlock_reg = 0xe6060000, /* PMMR */ .function = { PINMUX_FUNCTION_BEGIN, PINMUX_FUNCTION_END }, @@ -2793,6 +2836,7 @@ const struct sh_pfc_soc_info r8a77980_pi .nr_functions = ARRAY_SIZE(pinmux_functions), .cfg_regs = pinmux_config_regs, + .ioctrl_regs = pinmux_ioctrl_regs, .pinmux_data = pinmux_data, .pinmux_data_size = ARRAY_SIZE(pinmux_data),