Re: [RFC 2/3] pinctrl: sh-pfc: r8a7795: add support for voltage switching

[Geert Uytterhoeven]

Hi Wolfram,

On Mon, Jun 6, 2016 at 1:03 PM, Wolfram Sang  wrote:
> On Mon, Jun 06, 2016 at 09:23:35AM +0200, Geert Uytterhoeven wrote:
>> On Mon, Jun 6, 2016 at 8:50 AM, Wolfram Sang  wrote:
>> > From: Wolfram Sang 
>> >
>> > Signed-off-by: Wolfram Sang 
>> > ---
>> >  drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 24 ++--
>> >  1 file changed, 22 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c 
>> > b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
>> > index 44632b1a5c978c..8e068d8534de00 100644
>> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
>> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
>> > @@ -17,8 +17,8 @@
>> > PORT_GP_CFG_16(0, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
>> > PORT_GP_CFG_28(1, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
>> > PORT_GP_CFG_15(2, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
>> > -   PORT_GP_CFG_16(3, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
>> > -   PORT_GP_CFG_18(4, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
>> > +   PORT_GP_CFG_16(3, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH | 
>> > SH_PFC_PIN_CFG_IO_VOLTAGE),  \
>>
>> Shouldn't this be split in PORT_GP_CFG_12() with SH_PFC_PIN_CFG_IO_VOLTAGE,
>> and PORT_GP_CFG_4() without?
>
> Right. However, PORT_GP_CFG_4 doesn't allow to set an offset for the pin
> numbers. Options I see:

> a) keep it as is and rely on the checks in pin_to_pocctrl()
> b) use PORT_GP_CFG_12 and 4 times PORT_GP_CFG_1 which allow setting the
>pin number
> c) introduce (yet another) macro like PORT_GP_CFG_4_OFS
>
> So far, I thought a) was good enough. Now I tend to option b) because it
> is indeed more precise. We still can do c) if demand for such a macro
> increases.
>
> What do you think?

Option b is fine for me.

Thanks!

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: [RFC 2/3] pinctrl: sh-pfc: r8a7795: add support for voltage switching

[Wolfram Sang]

On Mon, Jun 06, 2016 at 09:23:35AM +0200, Geert Uytterhoeven wrote:
> Hi Wolfram,
> 
> On Mon, Jun 6, 2016 at 8:50 AM, Wolfram Sang  wrote:
> > From: Wolfram Sang 
> >
> > Signed-off-by: Wolfram Sang 
> > ---
> >  drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 24 ++--
> >  1 file changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c 
> > b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> > index 44632b1a5c978c..8e068d8534de00 100644
> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> > @@ -17,8 +17,8 @@
> > PORT_GP_CFG_16(0, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
> > PORT_GP_CFG_28(1, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
> > PORT_GP_CFG_15(2, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
> > -   PORT_GP_CFG_16(3, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
> > -   PORT_GP_CFG_18(4, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
> > +   PORT_GP_CFG_16(3, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH | 
> > SH_PFC_PIN_CFG_IO_VOLTAGE),  \
> 
> Shouldn't this be split in PORT_GP_CFG_12() with SH_PFC_PIN_CFG_IO_VOLTAGE,
> and PORT_GP_CFG_4() without?

Right. However, PORT_GP_CFG_4 doesn't allow to set an offset for the pin
numbers. Options I see:

a) keep it as is and rely on the checks in pin_to_pocctrl()
b) use PORT_GP_CFG_12 and 4 times PORT_GP_CFG_1 which allow setting the
   pin number
c) introduce (yet another) macro like PORT_GP_CFG_4_OFS

So far, I thought a) was good enough. Now I tend to option b) because it
is indeed more precise. We still can do c) if demand for such a macro
increases.

What do you think?

Thanks,

   Wolfram

Re: [RFC 2/3] pinctrl: sh-pfc: r8a7795: add support for voltage switching

[Geert Uytterhoeven]

Hi Wolfram,

On Mon, Jun 6, 2016 at 8:50 AM, Wolfram Sang  wrote:
> From: Wolfram Sang 
>
> Signed-off-by: Wolfram Sang 
> ---
>  drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 24 ++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c 
> b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> index 44632b1a5c978c..8e068d8534de00 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> @@ -17,8 +17,8 @@
> PORT_GP_CFG_16(0, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
> PORT_GP_CFG_28(1, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
> PORT_GP_CFG_15(2, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
> -   PORT_GP_CFG_16(3, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
> -   PORT_GP_CFG_18(4, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
> +   PORT_GP_CFG_16(3, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH | 
> SH_PFC_PIN_CFG_IO_VOLTAGE),  \

Shouldn't this be split in PORT_GP_CFG_12() with SH_PFC_PIN_CFG_IO_VOLTAGE,
and PORT_GP_CFG_4() without?

> +   PORT_GP_CFG_18(4, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH | 
> SH_PFC_PIN_CFG_IO_VOLTAGE),  \

Apart from that:
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

[RFC 2/3] pinctrl: sh-pfc: r8a7795: add support for voltage switching

[Wolfram Sang]

From: Wolfram Sang 

Signed-off-by: Wolfram Sang 
---
 drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
index 44632b1a5c978c..8e068d8534de00 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
@@ -17,8 +17,8 @@
PORT_GP_CFG_16(0, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
PORT_GP_CFG_28(1, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
PORT_GP_CFG_15(2, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
-   PORT_GP_CFG_16(3, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
-   PORT_GP_CFG_18(4, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
+   PORT_GP_CFG_16(3, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH | 
SH_PFC_PIN_CFG_IO_VOLTAGE),  \
+   PORT_GP_CFG_18(4, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH | 
SH_PFC_PIN_CFG_IO_VOLTAGE),  \
PORT_GP_CFG_26(5, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
PORT_GP_CFG_32(6, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH),  \
PORT_GP_CFG_4(7, fn, sfx, SH_PFC_PIN_CFG_DRIVE_STRENGTH)
@@ -4765,8 +4765,28 @@ static const struct pinmux_drive_reg pinmux_drive_regs[] 
= {
{ },
 };
 
+static int r8a7795_pin_to_pocctrl(struct sh_pfc *pfc, unsigned int pin, u32 
*pocctrl)
+{
+   int bit = -EINVAL;
+
+   *pocctrl = 0xe6060380;
+
+   if (pin >= RCAR_GP_PIN(3, 0) && pin <= RCAR_GP_PIN(3, 11))
+   bit = pin & 0x1f;
+
+   if (pin >= RCAR_GP_PIN(4, 0) && pin <= RCAR_GP_PIN(4, 17))
+   bit = (pin & 0x1f) + 12;
+
+   return bit;
+}
+
+static const struct sh_pfc_soc_operations r8a7795_pinmux_ops = {
+   .pin_to_pocctrl = r8a7795_pin_to_pocctrl,
+};
+
 const struct sh_pfc_soc_info r8a7795_pinmux_info = {
.name = "r8a77950_pfc",
+   .ops = _pinmux_ops,
.unlock_reg = 0xe606, /* PMMR */
 
.function = { PINMUX_FUNCTION_BEGIN, PINMUX_FUNCTION_END },
-- 
2.8.1