[v4,1/4] pinctrl: sh-pfc: Add optional arg to VIN_DATA_PIN_GROUP

VIN data groups may appear on different sets of pins, usually named
"vinX_data_[a|b]". The existing VIN_DATA_PIN_GROUP() does not support
appending the '_a' or '_b' suffix, leading to the definition of groups
names not consistent with the ones defined using SH_PFC_PIN_GROUP() macro.

Fix this by adding making the VIN_DATA_PIN_GROUP macro a variadic one,
which accepts an optional 'version' argument.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/pinctrl/sh-pfc/sh_pfc.h | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

On Tue, Nov 06, 2018 at 11:35:30AM +0100, Jacopo Mondi wrote:
> VIN data groups may appear on different sets of pins, usually named
> "vinX_data_[a|b]". The existing VIN_DATA_PIN_GROUP() does not support
> appending the '_a' or '_b' suffix, leading to the definition of groups
> names not consistent with the ones defined using SH_PFC_PIN_GROUP() macro.
> 
> Fix this by adding making the VIN_DATA_PIN_GROUP macro a variadic one,
> which accepts an optional 'version' argument.

FWIW I prefered the VIN_DATA_PIN_GROUP_VER() approach as it pinned down
what the parameters to the macro should be, albeit with parameter naming
conundrum.  While we could have any number f varargs present.

But I don't think we need to debate the colour of the bike shed at this
point.

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/pinctrl/sh-pfc/sh_pfc.h | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
> index 458ae0a..0e0b4cc 100644
> --- a/drivers/pinctrl/sh-pfc/sh_pfc.h
> +++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
> @@ -54,15 +54,16 @@ struct sh_pfc_pin_group {
>  
>  /*
>   * Using union vin_data saves memory occupied by the VIN data pins.
> - * VIN_DATA_PIN_GROUP() is  a macro  used  to describe the VIN pin groups
> - * in this case.
> + * VIN_DATA_PIN_GROUP() is a macro used to describe the VIN pin groups
> + * in this case. It accepts an optional 'version' argument used when the
> + * same group can appear on a different set of pins.
>   */
> -#define VIN_DATA_PIN_GROUP(n, s)				\
> -	{							\
> -		.name = #n#s,					\
> -		.pins = n##_pins.data##s,			\
> -		.mux = n##_mux.data##s,				\
> -		.nr_pins = ARRAY_SIZE(n##_pins.data##s),	\
> +#define VIN_DATA_PIN_GROUP(n, s, ...)					\
> +	{								\
> +		.name = #n#s#__VA_ARGS__,				\
> +		.pins = n##__VA_ARGS__##_pins.data##s,			\
> +		.mux = n##__VA_ARGS__##_mux.data##s,			\
> +		.nr_pins = ARRAY_SIZE(n##__VA_ARGS__##_pins.data##s),	\
>  	}
>  
>  union vin_data {
> -- 
> 2.7.4
>

Hi Jacopo,

Thanks for your patch!

On Tue, Nov 6, 2018 at 11:35 AM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> VIN data groups may appear on different sets of pins, usually named
> "vinX_data_[a|b]". The existing VIN_DATA_PIN_GROUP() does not support
> appending the '_a' or '_b' suffix, leading to the definition of groups

group

> names not consistent with the ones defined using SH_PFC_PIN_GROUP() macro.

the SH_PFC_PIN_GROUP() macro.

>
> Fix this by adding making the VIN_DATA_PIN_GROUP macro a variadic one,

Please drop "adding".
VIN_DATA_PIN_GROUP()

> which accepts an optional 'version' argument.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

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

> --- a/drivers/pinctrl/sh-pfc/sh_pfc.h
> +++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
> @@ -54,15 +54,16 @@ struct sh_pfc_pin_group {
>
>  /*
>   * Using union vin_data saves memory occupied by the VIN data pins.
> - * VIN_DATA_PIN_GROUP() is  a macro  used  to describe the VIN pin groups
> - * in this case.
> + * VIN_DATA_PIN_GROUP() is a macro used to describe the VIN pin groups
> + * in this case. It accepts an optional 'version' argument used when the
> + * same group can appear on a different set of pins.

Please rebase against sh-pfc-for-v4.21.

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

Hi Jacopo,

On Thu, Nov 8, 2018 at 11:58 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, Nov 6, 2018 at 11:35 AM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> > VIN data groups may appear on different sets of pins, usually named
> > "vinX_data_[a|b]". The existing VIN_DATA_PIN_GROUP() does not support
> > appending the '_a' or '_b' suffix, leading to the definition of groups
>
> group
>
> > names not consistent with the ones defined using SH_PFC_PIN_GROUP() macro.
>
> the SH_PFC_PIN_GROUP() macro.
>
> >
> > Fix this by adding making the VIN_DATA_PIN_GROUP macro a variadic one,
>
> Please drop "adding".
> VIN_DATA_PIN_GROUP()
>
> > which accepts an optional 'version' argument.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Perhaps it makes sense to add

    Fixes: 423caa52534ff15a ("pinctrl: sh-pfc: r8a779[01]: Move 'union
vin_data' to shared header file")

to make sure it gets backported with the SoC-specific fixes that depend on it?

Gr{oetje,eeting}s,

                        Geert