| Message ID | 20231211190321.307330-4-andriy.shevchenko@linux.intel.com |
|---|---|
| State | Accepted |
| Commit | b0f24e021d58d74b83857b9d5c468bcda3785572 |
| Headers | show |
| Series | pinctrl: Convert struct group_desc to use struct pingroup | expand |
Hi Andy, Le lundi 11 décembre 2023 à 20:57 +0200, Andy Shevchenko a écrit : > For the better flexibility use C99 initializers in > PINCTRL_PIN_GROUP(). > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/pinctrl/pinctrl-ingenic.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-ingenic.c > b/drivers/pinctrl/pinctrl-ingenic.c > index ee718f6e2556..f5661dcdedf5 100644 > --- a/drivers/pinctrl/pinctrl-ingenic.c > +++ b/drivers/pinctrl/pinctrl-ingenic.c > @@ -82,16 +82,21 @@ > #define PINS_PER_GPIO_CHIP 32 > #define JZ4730_PINS_PER_PAIRED_REG 16 > > -#define INGENIC_PIN_GROUP_FUNCS(name, id, funcs) \ > - { \ > - name, \ > - id##_pins, \ > - ARRAY_SIZE(id##_pins), \ > - funcs, \ > +#define INGENIC_PIN_GROUP_FUNCS(_name_, id, > funcs) \ > + { > \ > + .name = > _name_, \ > + .pins = > id##_pins, \ > + .num_pins = > ARRAY_SIZE(id##_pins), \ > + .data = > funcs, \ > } > > -#define INGENIC_PIN_GROUP(name, id, func) \ > - INGENIC_PIN_GROUP_FUNCS(name, id, (void *)(func)) > +#define INGENIC_PIN_GROUP(_name_, id, > func) \ > + { > \ > + .name = > _name_, \ > + .pins = > id##_pins, \ > + .num_pins = > ARRAY_SIZE(id##_pins), \ > + .data = (void > *)func, \ > + } This INGENIC_PIN_GROUP() macro doesn't need to be modified, does it? Cheers, -Paul > > enum jz_version { > ID_JZ4730,
Hi Andy, Le mercredi 13 décembre 2023 à 15:21 +0200, Andy Shevchenko a écrit : > On Wed, Dec 13, 2023 at 10:55:46AM +0100, Paul Cercueil wrote: > > Le lundi 11 décembre 2023 à 20:57 +0200, Andy Shevchenko a écrit : > > ... > > > > -#define INGENIC_PIN_GROUP(name, id, func) \ > > > - INGENIC_PIN_GROUP_FUNCS(name, id, (void *)(func)) > > > +#define INGENIC_PIN_GROUP(_name_, id, > > > func) \ > > > + { > > > \ > > > + .name = > > > _name_, \ > > > + .pins = > > > id##_pins, \ > > > + .num_pins = > > > ARRAY_SIZE(id##_pins), \ > > > + .data = (void > > > *)func, \ > > > + } > > > > This INGENIC_PIN_GROUP() macro doesn't need to be modified, does > > it? > > We can go either way. I prefer to go this way as it reduces level of > indirections in the macros. It makes code easier to read and > understand. > But if you insist, I can drop that change in next version. > I like the patches to be minimal. But I understand your point of view as well. If you have to issue a v6, maybe state the reason why you also modify INGENIC_PIN_GROUP() then. But I don't care enough to request a v6 just for that. So: Acked-by: Paul Cercueil <paul@crapouillou.net> Cheers, -Paul
Hi Andy, Le lundi 18 décembre 2023 à 12:41 +0200, Andy Shevchenko a écrit : > On Sun, Dec 17, 2023 at 03:43:24PM +0100, Paul Cercueil wrote: > > Le mercredi 13 décembre 2023 à 15:21 +0200, Andy Shevchenko a > > écrit : > > > On Wed, Dec 13, 2023 at 10:55:46AM +0100, Paul Cercueil wrote: > > > > Le lundi 11 décembre 2023 à 20:57 +0200, Andy Shevchenko a > > > > écrit : > > ... > > > > > > -#define INGENIC_PIN_GROUP(name, id, func) \ > > > > > - INGENIC_PIN_GROUP_FUNCS(name, id, (void *)(func)) > > > > > +#define INGENIC_PIN_GROUP(_name_, id, > > > > > func) \ > > > > > + { > > > > > > > > > > \ > > > > > + .name = > > > > > _name_, > > > > > \ > > > > > + .pins = > > > > > id##_pins, > > > > > \ > > > > > + .num_pins = > > > > > ARRAY_SIZE(id##_pins), \ > > > > > + .data = (void > > > > > *)func, \ > > > > > + } > > > > > > > > This INGENIC_PIN_GROUP() macro doesn't need to be modified, > > > > does > > > > it? > > > > > > We can go either way. I prefer to go this way as it reduces level > > > of > > > indirections in the macros. It makes code easier to read and > > > understand. > > > But if you insist, I can drop that change in next version. > > > > I like the patches to be minimal. But I understand your point of > > view > > as well. > > > > If you have to issue a v6, maybe state the reason why you also > > modify > > INGENIC_PIN_GROUP() then. But I don't care enough to request a v6 > > just > > for that. > > > > So: > > Acked-by: Paul Cercueil <paul@crapouillou.net> > > Thank you! > > But as I already noted, the series had been applied (by Linus W.) > and this does not seem to be a critical to fix, do you agree? > Well I only suggested to change the commit message - so no, it is not critical to fix. Cheers, -Paul
diff --git a/drivers/pinctrl/pinctrl-ingenic.c b/drivers/pinctrl/pinctrl-ingenic.c index ee718f6e2556..f5661dcdedf5 100644 --- a/drivers/pinctrl/pinctrl-ingenic.c +++ b/drivers/pinctrl/pinctrl-ingenic.c @@ -82,16 +82,21 @@ #define PINS_PER_GPIO_CHIP 32 #define JZ4730_PINS_PER_PAIRED_REG 16 -#define INGENIC_PIN_GROUP_FUNCS(name, id, funcs) \ - { \ - name, \ - id##_pins, \ - ARRAY_SIZE(id##_pins), \ - funcs, \ +#define INGENIC_PIN_GROUP_FUNCS(_name_, id, funcs) \ + { \ + .name = _name_, \ + .pins = id##_pins, \ + .num_pins = ARRAY_SIZE(id##_pins), \ + .data = funcs, \ } -#define INGENIC_PIN_GROUP(name, id, func) \ - INGENIC_PIN_GROUP_FUNCS(name, id, (void *)(func)) +#define INGENIC_PIN_GROUP(_name_, id, func) \ + { \ + .name = _name_, \ + .pins = id##_pins, \ + .num_pins = ARRAY_SIZE(id##_pins), \ + .data = (void *)func, \ + } enum jz_version { ID_JZ4730,
For the better flexibility use C99 initializers in PINCTRL_PIN_GROUP(). Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/pinctrl/pinctrl-ingenic.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)