Re: [PATCH] pinctrl: sh-pfc: r8a77970: add pin I/O voltage control

2018-04-19 Thread Geert Uytterhoeven
Hi Sergei,

On Fri, Apr 13, 2018 at 8: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 
> Signed-off-by: Sergei Shtylyov 

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 

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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


Re: [PATCH] pinctrl: sh-pfc: r8a77970: add pin I/O voltage control

2018-04-17 Thread Geert Uytterhoeven
Hi Sergei,

On Mon, Apr 16, 2018 at 5:06 PM, Sergei Shtylyov
 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 
>>> Signed-off-by: Sergei Shtylyov 
>>>
>>> ---
>>>  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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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


Re: [PATCH] pinctrl: sh-pfc: r8a77970: add pin I/O voltage control

2018-04-16 Thread Sergei Shtylyov
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 
>> Signed-off-by: Sergei Shtylyov 
>>
>> ---
>>  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


Re: [PATCH] pinctrl: sh-pfc: r8a77970: add pin I/O voltage control

2018-04-16 Thread Geert Uytterhoeven
Hi Sergei,

Thanks for your patch!

On Fri, Apr 13, 2018 at 8:29 PM, Sergei Shtylyov
 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 
> Signed-off-by: Sergei Shtylyov 
>
> ---
>  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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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


Re: [PATCH] pinctrl: sh-pfc: r8a77970: add pin I/O voltage control

2018-04-13 Thread Sergei Shtylyov
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 
>> Signed-off-by: Sergei Shtylyov 
> 
>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




Re: [PATCH] pinctrl: sh-pfc: r8a77970: add pin I/O voltage control

2018-04-13 Thread Sergei Shtylyov
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 
> Signed-off-by: Sergei Shtylyov 

   Oops, forgot to mention that the patch was against the 'sj-pfc' branch
of Geert's 'renesas-drivers.git' repo...

[...]

MBR, Sergei