On 04/19/2018 04:06 PM, Geert Uytterhoeven wrote:
>> I've included the pin I/O voltage control into the R8A77970 PFC driver but
>> it was incomplete because:
>> - SH_PFC_PIN_CFG_IO_VOLTAGE pin flags weren't set properly;
>> - sh_pfc_soc_info::ioctrl_regs wasn't set at all...
>
> Thanks for your patch!
>
>> Fixes: b92ac66a1819 ("pinctrl: sh-pfc: Add R8A77970 PFC support")
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
>
> Minor nit below...
Not sure which of your multiple comments was this nit... :-)
>> --- renesas-drivers.orig/drivers/pinctrl/sh-pfc/pfc-r8a77970.c
>> +++ renesas-drivers/drivers/pinctrl/sh-pfc/pfc-r8a77970.c
>
>> @@ -2382,18 +2384,33 @@ static const struct pinmux_cfg_reg pinmu
>> { },
>> };
>>
>> +enum ioctrl_regs {
>> + IOCTRL30,
>> + IOCTRL31,
>> + IOCTRL32,
>> + IOCTRL40,
>
> Note for the future: unlike all other current pocctrl handling, IOCTRL32
> is used to select between 2.5V and 3.3V (instead of 1.8V and 3.3V).
It also controls a group of pins instead of the individual pins, doesn't it?
> Handling that will require changes to the r8a77980_pin_to_pocctrl()
I'm afraid we'd need a new method, other than pin_to_pocctrl...
> interface (add two output parameters returning supported voltages?).
>
>> + IOCTRL40,
>
> That's TDSEL, and thus not related to I/O voltage.
It's still an IOCTRL<n> register, no?
> But I agree it should be saved during suspend/resume. ;-)
You mean we shouldn't use the sh_pfc_soc_info::ioctrl_regs mechanism for it?
> Gr{oetje,eeting}s,
>
> Geert
MBR, Sergei
