Re: [PATCH v4 1/4] pinctrl: sh-pfc: Add optional arg to VIN_DATA_PIN_GROUP
Hi Jacopo,
On Thu, Nov 8, 2018 at 11:58 AM Geert Uytterhoeven wrote:
> On Tue, Nov 6, 2018 at 11:35 AM 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
>
> 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
>
> Reviewed-by: Geert Uytterhoeven
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
--
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 v4 1/4] pinctrl: sh-pfc: Add optional arg to VIN_DATA_PIN_GROUP
Hi Jacopo,
Thanks for your patch!
On Tue, Nov 6, 2018 at 11:35 AM 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
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
Reviewed-by: Geert Uytterhoeven
> --- 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 -- 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 v4 1/4] pinctrl: sh-pfc: Add optional arg to VIN_DATA_PIN_GROUP
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
> Signed-off-by: Jacopo Mondi
> ---
> 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
>
[PATCH 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
---
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
