[3/3] pinctrl: sh-pfc: r8a7794: Implement voltage switching for SDHI
diff mbox

Message ID 1473334608-24638-4-git-send-email-horms+renesas@verge.net.au
State New
Headers show

Commit Message

Simon Horman Sept. 8, 2016, 11:36 a.m. UTC
All the SHDIs can operate with either 3.3V or 1.8V signals, depending
on negotiation with the card.

Based on work by Wolfram Sang for the r8a7790.

Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 drivers/pinctrl/sh-pfc/pfc-r8a7794.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

Comments

Geert Uytterhoeven Sept. 9, 2016, 8:08 a.m. UTC | #1
Hi Simon,

On Thu, Sep 8, 2016 at 1:36 PM, Simon Horman <horms+renesas@verge.net.au> wrote:
> All the SHDIs can operate with either 3.3V or 1.8V signals, depending
> on negotiation with the card.
>
> Based on work by Wolfram Sang for the r8a7790.
>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> ---
>  drivers/pinctrl/sh-pfc/pfc-r8a7794.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
> index 8bc2cf0c594e..c02b367837b7 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
> @@ -5160,8 +5162,38 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
>         { },
>  };
>
> +static int r8a7794_pin_to_pocctrl(struct sh_pfc *pfc, unsigned int pin, u32 *pocctrl)
> +{
> +       if (pin < RCAR_GP_PIN(6, 0) || pin > RCAR_GP_PIN(6, 23))
> +               return -EINVAL;
> +
> +       *pocctrl = 0xe606006c;
> +
> +       /* GP6_16-23 -> bits 31-24 of pocctrl
> +        * GP6_06    -> bit  23    of pocctrl
> +        * GP6_00-05 -> bits 22-17 of pocctrl
> +        * GP6_07    -> bit  16    of pocctrl
> +        * GP6_14    -> bit  15    of pocctrl
> +        * GP6_08-13 -> bits 14-09 of pocctrl
> +        * GP6_15    -> bit  08    of pocctrl
> +        */
> +       if (pin == RCAR_GP_PIN(6, 6) || pin == RCAR_GP_PIN(6, 14))
> +               return 31 - 2 - (pin & 0x1f);
> +       else if (pin == RCAR_GP_PIN(6, 7) || pin == RCAR_GP_PIN(6, 15))
> +               return 31 - 8 - (pin & 0x1f);
> +       else if (pin < RCAR_GP_PIN(6, 14))
> +               return 31 - 9 - (pin & 0x1f);
> +       else
> +               return 31 + 16 - (pin & 0x1f);

While your code is correct, I think it's easier for the casual reader to use
a plain switch () statement, and let the optimizer handle the rest.

> +}

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
--
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
Wolfram Sang Sept. 9, 2016, 12:30 p.m. UTC | #2
> > +static int r8a7794_pin_to_pocctrl(struct sh_pfc *pfc, unsigned int pin, u32 *pocctrl)
> > +{
> > +       if (pin < RCAR_GP_PIN(6, 0) || pin > RCAR_GP_PIN(6, 23))
> > +               return -EINVAL;
> > +
> > +       *pocctrl = 0xe606006c;
> > +
> > +       /* GP6_16-23 -> bits 31-24 of pocctrl
> > +        * GP6_06    -> bit  23    of pocctrl
> > +        * GP6_00-05 -> bits 22-17 of pocctrl
> > +        * GP6_07    -> bit  16    of pocctrl
> > +        * GP6_14    -> bit  15    of pocctrl
> > +        * GP6_08-13 -> bits 14-09 of pocctrl
> > +        * GP6_15    -> bit  08    of pocctrl
> > +        */
> > +       if (pin == RCAR_GP_PIN(6, 6) || pin == RCAR_GP_PIN(6, 14))
> > +               return 31 - 2 - (pin & 0x1f);
> > +       else if (pin == RCAR_GP_PIN(6, 7) || pin == RCAR_GP_PIN(6, 15))
> > +               return 31 - 8 - (pin & 0x1f);
> > +       else if (pin < RCAR_GP_PIN(6, 14))
> > +               return 31 - 9 - (pin & 0x1f);
> > +       else
> > +               return 31 + 16 - (pin & 0x1f);
> 
> While your code is correct, I think it's easier for the casual reader to use
> a plain switch () statement, and let the optimizer handle the rest.

Both is fine with me:

Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Simon Horman Sept. 9, 2016, 3:26 p.m. UTC | #3
On Fri, Sep 09, 2016 at 10:08:03AM +0200, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Thu, Sep 8, 2016 at 1:36 PM, Simon Horman <horms+renesas@verge.net.au> wrote:
> > All the SHDIs can operate with either 3.3V or 1.8V signals, depending
> > on negotiation with the card.
> >
> > Based on work by Wolfram Sang for the r8a7790.
> >
> > Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > ---
> >  drivers/pinctrl/sh-pfc/pfc-r8a7794.c | 34 +++++++++++++++++++++++++++++++++-
> >  1 file changed, 33 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
> > index 8bc2cf0c594e..c02b367837b7 100644
> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
> > @@ -5160,8 +5162,38 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
> >         { },
> >  };
> >
> > +static int r8a7794_pin_to_pocctrl(struct sh_pfc *pfc, unsigned int pin, u32 *pocctrl)
> > +{
> > +       if (pin < RCAR_GP_PIN(6, 0) || pin > RCAR_GP_PIN(6, 23))
> > +               return -EINVAL;
> > +
> > +       *pocctrl = 0xe606006c;
> > +
> > +       /* GP6_16-23 -> bits 31-24 of pocctrl
> > +        * GP6_06    -> bit  23    of pocctrl
> > +        * GP6_00-05 -> bits 22-17 of pocctrl
> > +        * GP6_07    -> bit  16    of pocctrl
> > +        * GP6_14    -> bit  15    of pocctrl
> > +        * GP6_08-13 -> bits 14-09 of pocctrl
> > +        * GP6_15    -> bit  08    of pocctrl
> > +        */
> > +       if (pin == RCAR_GP_PIN(6, 6) || pin == RCAR_GP_PIN(6, 14))
> > +               return 31 - 2 - (pin & 0x1f);
> > +       else if (pin == RCAR_GP_PIN(6, 7) || pin == RCAR_GP_PIN(6, 15))
> > +               return 31 - 8 - (pin & 0x1f);
> > +       else if (pin < RCAR_GP_PIN(6, 14))
> > +               return 31 - 9 - (pin & 0x1f);
> > +       else
> > +               return 31 + 16 - (pin & 0x1f);
> 
> While your code is correct, I think it's easier for the casual reader to use
> a plain switch () statement, and let the optimizer handle the rest.

Like this? If so I don't particularly mind but it doesn't seem clearer to
me.

+static int r8a7794_pin_to_pocctrl(struct sh_pfc *pfc, unsigned int pin, u32 *pocctrl)
+{
+	*pocctrl = 0xe606006c;
+
+	switch (pin) {
+	case RCAR_GP_PIN(6, 0):
+		return 22;
+	case RCAR_GP_PIN(6, 1):
+		return 21;
+	case RCAR_GP_PIN(6, 2):
+		return 20;
+	case RCAR_GP_PIN(6, 3):
+		return 19;
+	case RCAR_GP_PIN(6, 4):
+		return 18;
+	case RCAR_GP_PIN(6, 5):
+		return 17;
+	case RCAR_GP_PIN(6, 6):
+		return 23;
+	case RCAR_GP_PIN(6, 7):
+		return 16;
+	case RCAR_GP_PIN(6, 8):
+		return 14;
+	case RCAR_GP_PIN(6, 9):
+		return 13;
+	case RCAR_GP_PIN(6, 10):
+		return 12;
+	case RCAR_GP_PIN(6, 11):
+		return 11;
+	case RCAR_GP_PIN(6, 12):
+		return 10;
+	case RCAR_GP_PIN(6, 13):
+		return 9;
+	case RCAR_GP_PIN(6, 14):
+		return 15;
+	case RCAR_GP_PIN(6, 15):
+		return 8;
+	case RCAR_GP_PIN(6, 16):
+		return 31;
+	case RCAR_GP_PIN(6, 17):
+		return 30;
+	case RCAR_GP_PIN(6, 18):
+		return 29;
+	case RCAR_GP_PIN(6, 19):
+		return 28;
+	case RCAR_GP_PIN(6, 20):
+		return 27;
+	case RCAR_GP_PIN(6, 21):
+		return 26;
+	case RCAR_GP_PIN(6, 22):
+		return 25;
+	case RCAR_GP_PIN(6, 23):
+		return 24;
+	}
+
+	return -EINVAL;
+}
+
+static const struct sh_pfc_soc_operations r8a7794_pinmux_ops = {
+	.pin_to_pocctrl = r8a7794_pin_to_pocctrl,
+};
+
 const struct sh_pfc_soc_info r8a7794_pinmux_info = {
 	.name = "r8a77940_pfc",
+	.ops = &r8a7794_pinmux_ops,
 	.unlock_reg = 0xe6060000, /* PMMR */
 
 	.function = { PINMUX_FUNCTION_BEGIN, PINMUX_FUNCTION_END },
--
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
Geert Uytterhoeven Sept. 9, 2016, 4:57 p.m. UTC | #4
Hi Simon,

On Fri, Sep 9, 2016 at 5:26 PM, Simon Horman <horms@verge.net.au> wrote:
>> > +       /* GP6_16-23 -> bits 31-24 of pocctrl
>> > +        * GP6_06    -> bit  23    of pocctrl
>> > +        * GP6_00-05 -> bits 22-17 of pocctrl
>> > +        * GP6_07    -> bit  16    of pocctrl
>> > +        * GP6_14    -> bit  15    of pocctrl
>> > +        * GP6_08-13 -> bits 14-09 of pocctrl
>> > +        * GP6_15    -> bit  08    of pocctrl
>> > +        */
>> > +       if (pin == RCAR_GP_PIN(6, 6) || pin == RCAR_GP_PIN(6, 14))
>> > +               return 31 - 2 - (pin & 0x1f);
>> > +       else if (pin == RCAR_GP_PIN(6, 7) || pin == RCAR_GP_PIN(6, 15))
>> > +               return 31 - 8 - (pin & 0x1f);
>> > +       else if (pin < RCAR_GP_PIN(6, 14))
>> > +               return 31 - 9 - (pin & 0x1f);
>> > +       else
>> > +               return 31 + 16 - (pin & 0x1f);
>>
>> While your code is correct, I think it's easier for the casual reader to use
>> a plain switch () statement, and let the optimizer handle the rest.
>
> Like this? If so I don't particularly mind but it doesn't seem clearer to
> me.
>
> +static int r8a7794_pin_to_pocctrl(struct sh_pfc *pfc, unsigned int pin, u32 *pocctrl)
> +{
> +       *pocctrl = 0xe606006c;
> +
> +       switch (pin) {
> +       case RCAR_GP_PIN(6, 0):
> +               return 22;
> +       case RCAR_GP_PIN(6, 1):
> +               return 21;

Right, a full list of cases indeed doesn't look that much better.

Note that you can use "switch (pin & 0x1f)", and have ranges in case
statements:

        switch (pin & 0x1f) {
        case 6: return 23:
        case 7: return 16;
        case 14: return 15;
        case 15: return 8;
        case 0...5:
        case 7...13:
                return 22 - (pin & 0x1f);
        case 16..23:
                return 47 - (pin & 0x1f);
        }

Does that look better?

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
--
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
Simon Horman Sept. 10, 2016, 6:14 a.m. UTC | #5
On Fri, Sep 09, 2016 at 06:57:54PM +0200, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Fri, Sep 9, 2016 at 5:26 PM, Simon Horman <horms@verge.net.au> wrote:
> >> > +       /* GP6_16-23 -> bits 31-24 of pocctrl
> >> > +        * GP6_06    -> bit  23    of pocctrl
> >> > +        * GP6_00-05 -> bits 22-17 of pocctrl
> >> > +        * GP6_07    -> bit  16    of pocctrl
> >> > +        * GP6_14    -> bit  15    of pocctrl
> >> > +        * GP6_08-13 -> bits 14-09 of pocctrl
> >> > +        * GP6_15    -> bit  08    of pocctrl
> >> > +        */
> >> > +       if (pin == RCAR_GP_PIN(6, 6) || pin == RCAR_GP_PIN(6, 14))
> >> > +               return 31 - 2 - (pin & 0x1f);
> >> > +       else if (pin == RCAR_GP_PIN(6, 7) || pin == RCAR_GP_PIN(6, 15))
> >> > +               return 31 - 8 - (pin & 0x1f);
> >> > +       else if (pin < RCAR_GP_PIN(6, 14))
> >> > +               return 31 - 9 - (pin & 0x1f);
> >> > +       else
> >> > +               return 31 + 16 - (pin & 0x1f);
> >>
> >> While your code is correct, I think it's easier for the casual reader to use
> >> a plain switch () statement, and let the optimizer handle the rest.
> >
> > Like this? If so I don't particularly mind but it doesn't seem clearer to
> > me.
> >
> > +static int r8a7794_pin_to_pocctrl(struct sh_pfc *pfc, unsigned int pin, u32 *pocctrl)
> > +{
> > +       *pocctrl = 0xe606006c;
> > +
> > +       switch (pin) {
> > +       case RCAR_GP_PIN(6, 0):
> > +               return 22;
> > +       case RCAR_GP_PIN(6, 1):
> > +               return 21;
> 
> Right, a full list of cases indeed doesn't look that much better.
> 
> Note that you can use "switch (pin & 0x1f)", and have ranges in case
> statements:
> 
>         switch (pin & 0x1f) {
>         case 6: return 23:
>         case 7: return 16;
>         case 14: return 15;
>         case 15: return 8;
>         case 0...5:
>         case 7...13:
>                 return 22 - (pin & 0x1f);
>         case 16..23:
>                 return 47 - (pin & 0x1f);
>         }
> 
> Does that look better?

Yes, I think that could be a winning combination.
--
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

Patch
diff mbox

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
index 8bc2cf0c594e..c02b367837b7 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
@@ -22,7 +22,9 @@ 
 	PORT_GP_32(3, fn, sfx),						\
 	PORT_GP_32(4, fn, sfx),						\
 	PORT_GP_28(5, fn, sfx),						\
-	PORT_GP_26(6, fn, sfx)
+	PORT_GP_CFG_24(6, fn, sfx, SH_PFC_PIN_CFG_IO_VOLTAGE),		\
+	PORT_GP_1(6, 24, fn, sfx),					\
+	PORT_GP_1(6, 25, fn, sfx)
 
 enum {
 	PINMUX_RESERVED = 0,
@@ -5160,8 +5162,38 @@  static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 	{ },
 };
 
+static int r8a7794_pin_to_pocctrl(struct sh_pfc *pfc, unsigned int pin, u32 *pocctrl)
+{
+	if (pin < RCAR_GP_PIN(6, 0) || pin > RCAR_GP_PIN(6, 23))
+		return -EINVAL;
+
+	*pocctrl = 0xe606006c;
+
+	/* GP6_16-23 -> bits 31-24 of pocctrl
+	 * GP6_06    -> bit  23    of pocctrl
+	 * GP6_00-05 -> bits 22-17 of pocctrl
+	 * GP6_07    -> bit  16    of pocctrl
+	 * GP6_14    -> bit  15    of pocctrl
+	 * GP6_08-13 -> bits 14-09 of pocctrl
+	 * GP6_15    -> bit  08    of pocctrl
+	 */
+	if (pin == RCAR_GP_PIN(6, 6) || pin == RCAR_GP_PIN(6, 14))
+		return 31 - 2 - (pin & 0x1f);
+	else if (pin == RCAR_GP_PIN(6, 7) || pin == RCAR_GP_PIN(6, 15))
+		return 31 - 8 - (pin & 0x1f);
+	else if (pin < RCAR_GP_PIN(6, 14))
+		return 31 - 9 - (pin & 0x1f);
+	else
+		return 31 + 16 - (pin & 0x1f);
+}
+
+static const struct sh_pfc_soc_operations r8a7794_pinmux_ops = {
+	.pin_to_pocctrl = r8a7794_pin_to_pocctrl,
+};
+
 const struct sh_pfc_soc_info r8a7794_pinmux_info = {
 	.name = "r8a77940_pfc",
+	.ops = &r8a7794_pinmux_ops,
 	.unlock_reg = 0xe6060000, /* PMMR */
 
 	.function = { PINMUX_FUNCTION_BEGIN, PINMUX_FUNCTION_END },