[RFC,2/2] pinctrl: sh-pfc: r8a7791: Add can_clk function

This patch adds can_clk function to r8a7743/r8a7791 which is cleaner,
as it reduces duplication, and allows for independent configuration.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
Reviewed-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
---
 drivers/pinctrl/sh-pfc/pfc-r8a7791.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Hi Fabrizio,

On Fri, Nov 10, 2017 at 6:16 PM, Fabrizio Castro
<fabrizio.castro@bp.renesas.com> wrote:
> This patch adds can_clk function to r8a7743/r8a7791 which is cleaner,
> as it reduces duplication, and allows for independent configuration.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Reviewed-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
> @@ -4826,10 +4826,6 @@ static const char * const can0_groups[] = {
>         "can0_data_d",
>         "can0_data_e",
>         "can0_data_f",
> -       "can_clk",
> -       "can_clk_b",
> -       "can_clk_c",
> -       "can_clk_d",

As the pin groups and functions are part of the stable DT ABI, you cannot just
remove the can_clk* pins from can0_groups[], as that would break existing users.

I would keep them, but add a comment "/* retained for backwards
compatibility */".

>  };
>
>  static const char * const can1_groups[] = {
> @@ -4837,6 +4833,9 @@ static const char * const can1_groups[] = {
>         "can1_data_b",
>         "can1_data_c",
>         "can1_data_d",

Likewise.

> +};
> +
> +static const char * const can_clk_groups[] = {
>         "can_clk",
>         "can_clk_b",
>         "can_clk_c",

Adding a new can_clk_groups[] ...

> @@ -5308,7 +5307,7 @@ static const char * const vin2_groups[] = {
>  };
>
>  static const struct {
> -       struct sh_pfc_function common[56];
> +       struct sh_pfc_function common[57];
>         struct sh_pfc_function r8a779x[2];
>  } pinmux_functions = {
>         .common = {
> @@ -5316,6 +5315,7 @@ static const struct {
>                 SH_PFC_FUNCTION(avb),
>                 SH_PFC_FUNCTION(can0),
>                 SH_PFC_FUNCTION(can1),
> +               SH_PFC_FUNCTION(can_clk),

and can_clk function is OK, though.

>                 SH_PFC_FUNCTION(du),
>                 SH_PFC_FUNCTION(du0),
>                 SH_PFC_FUNCTION(du1),

My comments apply to your r8a7794 patch, too. Except that on r8a7794 we
didn't have CAN support in pfc-r8a7794.c before, but I would like to treat all
R-Car Gen2 and RZ/G1 SoCs in the same way.

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

Hello Geert,

thank you for your comments.

>

> As the pin groups and functions are part of the stable DT ABI, you cannot just

> remove the can_clk* pins from can0_groups[], as that would break existing users.

>

> I would keep them, but add a comment "/* retained for backwards

> compatibility */".

>

> Adding a new can_clk_groups[] ...

>

> and can_clk function is OK, though.

>

> My comments apply to your r8a7794 patch, too. Except that on r8a7794 we

> didn't have CAN support in pfc-r8a7794.c before, but I would like to treat all

> R-Car Gen2 and RZ/G1 SoCs in the same way.


I'll send a v2 to fix this.

Thanks,
Fab

>

> 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




Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.