| Message ID | 20231128200155.438722-7-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | pinctrl: Convert struct group_desc to use struct pingroup | expand |
Hi Andy, On Tue, Nov 28, 2023 at 9:04 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > It's unclear why it's not a const from day 1. Make the pins member > const in struct group_desc. Update necessary APIs. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Thanks for your patch! > --- a/drivers/pinctrl/core.c > +++ b/drivers/pinctrl/core.c > @@ -642,7 +642,7 @@ static int pinctrl_generic_group_name_to_selector(struct pinctrl_dev *pctldev, > * Note that the caller must take care of locking. > */ > int pinctrl_generic_add_group(struct pinctrl_dev *pctldev, const char *name, > - int *pins, int num_pins, void *data) > + const int *pins, int num_pins, void *data) > { > struct group_desc *group; > int selector, error; > diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h > index 530370443c19..01ea1ce99fe8 100644 > --- a/drivers/pinctrl/core.h > +++ b/drivers/pinctrl/core.h > @@ -203,7 +203,7 @@ struct pinctrl_maps { > */ > struct group_desc { > const char *name; > - int *pins; > + const int *pins; > int num_pins; > void *data; > }; > @@ -222,7 +222,7 @@ struct group_desc *pinctrl_generic_get_group(struct pinctrl_dev *pctldev, > unsigned int group_selector); > > int pinctrl_generic_add_group(struct pinctrl_dev *pctldev, const char *name, > - int *gpins, int ngpins, void *data); > + const int *pins, int num_pins, void *data); > > int pinctrl_generic_remove_group(struct pinctrl_dev *pctldev, > unsigned int group_selector); Probably this is also the right moment to change all of these to arrays of unsigned ints? Else you will have mixed int/unsigned int after "[PATCH v3 13/22] pinctrl: core: Embed struct pingroup into struct group_desc", and purely unsigned int after "[PATCH v3 22/22] pinctrl: core: Remove unused members from struct group_desc". The Renesas pinctrl drivers already pass arrays of unsigned ints. Some other drivers still use arrays of ints, though. Gr{oetje,eeting}s, Geert
On Wed, Nov 29, 2023 at 12:21:45PM +0100, Geert Uytterhoeven wrote: > On Tue, Nov 28, 2023 at 9:04 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > It's unclear why it's not a const from day 1. Make the pins member > > const in struct group_desc. Update necessary APIs. ... > > int pinctrl_generic_add_group(struct pinctrl_dev *pctldev, const char *name, > > - int *gpins, int ngpins, void *data); > > + const int *pins, int num_pins, void *data); > > > > int pinctrl_generic_remove_group(struct pinctrl_dev *pctldev, > > unsigned int group_selector); > > Probably this is also the right moment to change all of these to arrays > of unsigned ints? Else you will have mixed int/unsigned int after > "[PATCH v3 13/22] pinctrl: core: Embed struct pingroup into struct > group_desc", and purely unsigned int after "[PATCH v3 22/22] pinctrl: > core: Remove unused members from struct group_desc". Hmm... Can it be done later? I can, of course try to change the parameter here to be unsigned, but it most likely fail the build for those drivers means need more patches, more delay to this series. Linus?
On Wed, Nov 29, 2023 at 03:41:55PM +0200, Andy Shevchenko wrote: > On Wed, Nov 29, 2023 at 12:21:45PM +0100, Geert Uytterhoeven wrote: > > On Tue, Nov 28, 2023 at 9:04 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > It's unclear why it's not a const from day 1. Make the pins member > > > const in struct group_desc. Update necessary APIs. ... > > > int pinctrl_generic_add_group(struct pinctrl_dev *pctldev, const char *name, > > > - int *gpins, int ngpins, void *data); > > > + const int *pins, int num_pins, void *data); > > > > > > int pinctrl_generic_remove_group(struct pinctrl_dev *pctldev, > > > unsigned int group_selector); > > > > Probably this is also the right moment to change all of these to arrays > > of unsigned ints? Else you will have mixed int/unsigned int after > > "[PATCH v3 13/22] pinctrl: core: Embed struct pingroup into struct > > group_desc", and purely unsigned int after "[PATCH v3 22/22] pinctrl: > > core: Remove unused members from struct group_desc". > > Hmm... Can it be done later? > > I can, of course try to change the parameter here to be unsigned, but it most > likely fail the build for those drivers means need more patches, more delay to > this series. > > Linus? On the first glance updating API here does not fail the build. Lemme incorporate this into v4. Meanwhile the drivers I left untouched, it might be separate changes to convert from int to const unsigned int.
Hi Andy, On Wed, Nov 29, 2023 at 3:23 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Wed, Nov 29, 2023 at 03:41:55PM +0200, Andy Shevchenko wrote: > > On Wed, Nov 29, 2023 at 12:21:45PM +0100, Geert Uytterhoeven wrote: > > > On Tue, Nov 28, 2023 at 9:04 PM Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > It's unclear why it's not a const from day 1. Make the pins member > > > > const in struct group_desc. Update necessary APIs. > > ... > > > > > int pinctrl_generic_add_group(struct pinctrl_dev *pctldev, const char *name, > > > > - int *gpins, int ngpins, void *data); > > > > + const int *pins, int num_pins, void *data); > > > > > > > > int pinctrl_generic_remove_group(struct pinctrl_dev *pctldev, > > > > unsigned int group_selector); > > > > > > Probably this is also the right moment to change all of these to arrays > > > of unsigned ints? Else you will have mixed int/unsigned int after > > > "[PATCH v3 13/22] pinctrl: core: Embed struct pingroup into struct > > > group_desc", and purely unsigned int after "[PATCH v3 22/22] pinctrl: > > > core: Remove unused members from struct group_desc". > > > > Hmm... Can it be done later? > > > > I can, of course try to change the parameter here to be unsigned, but it most > > likely fail the build for those drivers means need more patches, more delay to > > this series. > > > > Linus? > > On the first glance updating API here does not fail the build. That's what I had expected, as drivers already pass int or unsigned int arrays anyway. > Lemme incorporate this into v4. Thanks! > Meanwhile the drivers I left untouched, it might be separate changes > to convert from int to const unsigned int. Sounds fine to me. Gr{oetje,eeting}s, Geert
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c index f2977eb65522..d20e3aad923e 100644 --- a/drivers/pinctrl/core.c +++ b/drivers/pinctrl/core.c @@ -642,7 +642,7 @@ static int pinctrl_generic_group_name_to_selector(struct pinctrl_dev *pctldev, * Note that the caller must take care of locking. */ int pinctrl_generic_add_group(struct pinctrl_dev *pctldev, const char *name, - int *pins, int num_pins, void *data) + const int *pins, int num_pins, void *data) { struct group_desc *group; int selector, error; diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h index 530370443c19..01ea1ce99fe8 100644 --- a/drivers/pinctrl/core.h +++ b/drivers/pinctrl/core.h @@ -203,7 +203,7 @@ struct pinctrl_maps { */ struct group_desc { const char *name; - int *pins; + const int *pins; int num_pins; void *data; }; @@ -222,7 +222,7 @@ struct group_desc *pinctrl_generic_get_group(struct pinctrl_dev *pctldev, unsigned int group_selector); int pinctrl_generic_add_group(struct pinctrl_dev *pctldev, const char *name, - int *gpins, int ngpins, void *data); + const int *pins, int num_pins, void *data); int pinctrl_generic_remove_group(struct pinctrl_dev *pctldev, unsigned int group_selector);
It's unclear why it's not a const from day 1. Make the pins member const in struct group_desc. Update necessary APIs. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/pinctrl/core.c | 2 +- drivers/pinctrl/core.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)