| Message ID | 20230320163823.886-5-clin@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pinctrl: s32: driver improvements and generic struct use | expand |
On Mon, Mar 20, 2023 at 6:39 PM Chester Lin <clin@suse.com> wrote: > > Use generic data structure to describe pin control functions and groups in > S32 SoC family and drop duplicated struct members. Not sure about the need of the casting, see below, otherwise LGTM. Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Signed-off-by: Chester Lin <clin@suse.com> > --- > Changes in v2: > - Simply use generic 'struct pinfunction' rather than having extra 'struct > s32_pmx_func'. > > drivers/pinctrl/nxp/pinctrl-s32.h | 26 ++-------- > drivers/pinctrl/nxp/pinctrl-s32cc.c | 76 +++++++++++++++-------------- > 2 files changed, 45 insertions(+), 57 deletions(-) > > diff --git a/drivers/pinctrl/nxp/pinctrl-s32.h b/drivers/pinctrl/nxp/pinctrl-s32.h > index 545bf16b988d..2f7aecd462e4 100644 > --- a/drivers/pinctrl/nxp/pinctrl-s32.h > +++ b/drivers/pinctrl/nxp/pinctrl-s32.h > @@ -15,31 +15,15 @@ struct platform_device; > > /** > * struct s32_pin_group - describes an S32 pin group > - * @name: the name of this specific pin group > - * @npins: the number of pins in this group array, i.e. the number of > - * elements in pin_ids and pin_sss so we can iterate over that array > - * @pin_ids: an array of pin IDs in this group > - * @pin_sss: an array of source signal select configs paired with pin_ids > + * @data: generic data describes group name, number of pins, and a pin array in > + this group. > + * @pin_sss: an array of source signal select configs paired with pin array. > */ > struct s32_pin_group { > - const char *name; > - unsigned int npins; > - unsigned int *pin_ids; > + struct pingroup data; > unsigned int *pin_sss; > }; > > -/** > - * struct s32_pmx_func - describes S32 pinmux functions > - * @name: the name of this specific function > - * @groups: corresponding pin groups > - * @num_groups: the number of groups > - */ > -struct s32_pmx_func { > - const char *name; > - const char **groups; > - unsigned int num_groups; > -}; > - > /** > * struct s32_pin_range - pin ID range for each memory region. > * @start: start pin ID > @@ -56,7 +40,7 @@ struct s32_pinctrl_soc_info { > unsigned int npins; > struct s32_pin_group *groups; > unsigned int ngroups; > - struct s32_pmx_func *functions; > + struct pinfunction *functions; > unsigned int nfunctions; > unsigned int grp_index; > const struct s32_pin_range *mem_pin_ranges; > diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c > index cb8a0844c0fa..4ed0cc905232 100644 > --- a/drivers/pinctrl/nxp/pinctrl-s32cc.c > +++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c > @@ -188,7 +188,7 @@ static const char *s32_get_group_name(struct pinctrl_dev *pctldev, > struct s32_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev); > const struct s32_pinctrl_soc_info *info = ipctl->info; > > - return info->groups[selector].name; > + return info->groups[selector].data.name; > } > > static int s32_get_group_pins(struct pinctrl_dev *pctldev, > @@ -198,8 +198,8 @@ static int s32_get_group_pins(struct pinctrl_dev *pctldev, > struct s32_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev); > const struct s32_pinctrl_soc_info *info = ipctl->info; > > - *pins = info->groups[selector].pin_ids; > - *npins = info->groups[selector].npins; > + *pins = info->groups[selector].data.pins; > + *npins = info->groups[selector].data.npins; > > return 0; > } > @@ -314,23 +314,23 @@ static int s32_pmx_set(struct pinctrl_dev *pctldev, unsigned int selector, > grp = &info->groups[group]; > > dev_dbg(ipctl->dev, "set mux for function %s group %s\n", > - info->functions[selector].name, grp->name); > + info->functions[selector].name, grp->data.name); > > /* Check beforehand so we don't have a partial config. */ > - for (i = 0; i < grp->npins; i++) { > - if (s32_check_pin(pctldev, grp->pin_ids[i]) != 0) { > + for (i = 0; i < grp->data.npins; i++) { > + if (s32_check_pin(pctldev, grp->data.pins[i]) != 0) { > dev_err(info->dev, "invalid pin: %u in group: %u\n", > - grp->pin_ids[i], group); > + grp->data.pins[i], group); > return -EINVAL; > } > } > > - for (i = 0, ret = 0; i < grp->npins && !ret; i++) { > - ret = s32_regmap_update(pctldev, grp->pin_ids[i], > + for (i = 0, ret = 0; i < grp->data.npins && !ret; i++) { > + ret = s32_regmap_update(pctldev, grp->data.pins[i], > S32_MSCR_SSS_MASK, grp->pin_sss[i]); > if (ret) { > dev_err(info->dev, "Failed to set pin %u\n", > - grp->pin_ids[i]); > + grp->data.pins[i]); > return ret; > } > } > @@ -364,7 +364,7 @@ static int s32_pmx_get_groups(struct pinctrl_dev *pctldev, > const struct s32_pinctrl_soc_info *info = ipctl->info; > > *groups = info->functions[selector].groups; > - *num_groups = info->functions[selector].num_groups; > + *num_groups = info->functions[selector].ngroups; > > return 0; > } > @@ -602,8 +602,8 @@ static int s32_pconf_group_set(struct pinctrl_dev *pctldev, unsigned int selecto > int i, ret; > > grp = &info->groups[selector]; > - for (i = 0; i < grp->npins; i++) { > - ret = s32_pinconf_mscr_update(pctldev, grp->pin_ids[i], > + for (i = 0; i < grp->data.npins; i++) { > + ret = s32_pinconf_mscr_update(pctldev, grp->data.pins[i], > configs, num_configs); > if (ret) > return ret; > @@ -637,9 +637,9 @@ static void s32_pinconf_group_dbg_show(struct pinctrl_dev *pctldev, > > seq_puts(s, "\n"); > grp = &info->groups[selector]; > - for (i = 0; i < grp->npins; i++) { > - name = pin_get_name(pctldev, grp->pin_ids[i]); > - ret = s32_regmap_read(pctldev, grp->pin_ids[i], &config); > + for (i = 0; i < grp->data.npins; i++) { > + name = pin_get_name(pctldev, grp->data.pins[i]); > + ret = s32_regmap_read(pctldev, grp->data.pins[i], &config); > if (ret) > return; > seq_printf(s, "%s: 0x%x\n", name, config); > @@ -732,6 +732,7 @@ static int s32_pinctrl_parse_groups(struct device_node *np, > const __be32 *p; > struct device *dev; > struct property *prop; > + unsigned int *pins, *sss; > int i, npins; > u32 pinmux; > > @@ -740,38 +741,38 @@ static int s32_pinctrl_parse_groups(struct device_node *np, > dev_dbg(dev, "group: %pOFn\n", np); > > /* Initialise group */ > - grp->name = np->name; > + grp->data.name = np->name; > > npins = of_property_count_elems_of_size(np, "pinmux", sizeof(u32)); > if (npins < 0) { > dev_err(dev, "Failed to read 'pinmux' property in node %s.\n", > - grp->name); > + grp->data.name); > return -EINVAL; > } > if (!npins) { > - dev_err(dev, "The group %s has no pins.\n", grp->name); > + dev_err(dev, "The group %s has no pins.\n", grp->data.name); > return -EINVAL; > } > > - grp->npins = npins; > + grp->data.npins = npins; > > - grp->pin_ids = devm_kcalloc(info->dev, grp->npins, > - sizeof(unsigned int), GFP_KERNEL); > - grp->pin_sss = devm_kcalloc(info->dev, grp->npins, > - sizeof(unsigned int), GFP_KERNEL); > - if (!grp->pin_ids || !grp->pin_sss) > + pins = devm_kcalloc(info->dev, npins, sizeof(*pins), GFP_KERNEL); > + sss = devm_kcalloc(info->dev, npins, sizeof(*sss), GFP_KERNEL); > + if (!pins || !sss) > return -ENOMEM; > > i = 0; > of_property_for_each_u32(np, "pinmux", prop, p, pinmux) { > - grp->pin_ids[i] = get_pin_no(pinmux); > - grp->pin_sss[i] = get_pin_func(pinmux); > + pins[i] = get_pin_no(pinmux); > + sss[i] = get_pin_func(pinmux); > > - dev_dbg(info->dev, "pin-id: 0x%x, sss: 0x%x", > - grp->pin_ids[i], grp->pin_sss[i]); > + dev_dbg(info->dev, "pin: 0x%x, sss: 0x%x", pins[i], sss[i]); > i++; > } > > + grp->data.pins = pins; > + grp->pin_sss = sss; > + > return 0; > } > > @@ -780,8 +781,9 @@ static int s32_pinctrl_parse_functions(struct device_node *np, > u32 index) > { > struct device_node *child; > - struct s32_pmx_func *func; > + struct pinfunction *func; > struct s32_pin_group *grp; > + char **groups; > u32 i = 0; > int ret = 0; > > @@ -791,18 +793,18 @@ static int s32_pinctrl_parse_functions(struct device_node *np, > > /* Initialise function */ > func->name = np->name; > - func->num_groups = of_get_child_count(np); > - if (func->num_groups == 0) { > + func->ngroups = of_get_child_count(np); > + if (func->ngroups == 0) { > dev_err(info->dev, "no groups defined in %pOF\n", np); > return -EINVAL; > } > - func->groups = devm_kcalloc(info->dev, func->num_groups, > - sizeof(*func->groups), GFP_KERNEL); > - if (!func->groups) > + groups = devm_kcalloc(info->dev, func->ngroups, > + sizeof(*func->groups), GFP_KERNEL); > + if (!groups) > return -ENOMEM; > > for_each_child_of_node(np, child) { > - func->groups[i] = child->name; > + groups[i] = (char *)child->name; > grp = &info->groups[info->grp_index++]; > ret = s32_pinctrl_parse_groups(child, grp, info); > if (ret) > @@ -810,6 +812,8 @@ static int s32_pinctrl_parse_functions(struct device_node *np, > i++; > } > > + func->groups = (const char **)groups; Hmm... Why is casting needed? > return 0; > }
On Mon, Mar 20, 2023 at 07:10:40PM +0200, Andy Shevchenko wrote: > On Mon, Mar 20, 2023 at 6:39 PM Chester Lin <clin@suse.com> wrote: > > > > Use generic data structure to describe pin control functions and groups in > > S32 SoC family and drop duplicated struct members. > > Not sure about the need of the casting, see below, otherwise LGTM. > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > > Signed-off-by: Chester Lin <clin@suse.com> > > --- > > Changes in v2: > > - Simply use generic 'struct pinfunction' rather than having extra 'struct > > s32_pmx_func'. > > > > drivers/pinctrl/nxp/pinctrl-s32.h | 26 ++-------- > > drivers/pinctrl/nxp/pinctrl-s32cc.c | 76 +++++++++++++++-------------- > > 2 files changed, 45 insertions(+), 57 deletions(-) > > > > diff --git a/drivers/pinctrl/nxp/pinctrl-s32.h b/drivers/pinctrl/nxp/pinctrl-s32.h > > index 545bf16b988d..2f7aecd462e4 100644 > > --- a/drivers/pinctrl/nxp/pinctrl-s32.h > > +++ b/drivers/pinctrl/nxp/pinctrl-s32.h > > @@ -15,31 +15,15 @@ struct platform_device; > > > > /** > > * struct s32_pin_group - describes an S32 pin group > > - * @name: the name of this specific pin group > > - * @npins: the number of pins in this group array, i.e. the number of > > - * elements in pin_ids and pin_sss so we can iterate over that array > > - * @pin_ids: an array of pin IDs in this group > > - * @pin_sss: an array of source signal select configs paired with pin_ids > > + * @data: generic data describes group name, number of pins, and a pin array in > > + this group. > > + * @pin_sss: an array of source signal select configs paired with pin array. > > */ > > struct s32_pin_group { > > - const char *name; > > - unsigned int npins; > > - unsigned int *pin_ids; > > + struct pingroup data; > > unsigned int *pin_sss; > > }; > > > > -/** > > - * struct s32_pmx_func - describes S32 pinmux functions > > - * @name: the name of this specific function > > - * @groups: corresponding pin groups > > - * @num_groups: the number of groups > > - */ > > -struct s32_pmx_func { > > - const char *name; > > - const char **groups; > > - unsigned int num_groups; > > -}; > > - > > /** > > * struct s32_pin_range - pin ID range for each memory region. > > * @start: start pin ID > > @@ -56,7 +40,7 @@ struct s32_pinctrl_soc_info { > > unsigned int npins; > > struct s32_pin_group *groups; > > unsigned int ngroups; > > - struct s32_pmx_func *functions; > > + struct pinfunction *functions; > > unsigned int nfunctions; > > unsigned int grp_index; > > const struct s32_pin_range *mem_pin_ranges; > > diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c > > index cb8a0844c0fa..4ed0cc905232 100644 > > --- a/drivers/pinctrl/nxp/pinctrl-s32cc.c > > +++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c > > @@ -188,7 +188,7 @@ static const char *s32_get_group_name(struct pinctrl_dev *pctldev, > > struct s32_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev); > > const struct s32_pinctrl_soc_info *info = ipctl->info; > > > > - return info->groups[selector].name; > > + return info->groups[selector].data.name; > > } > > > > static int s32_get_group_pins(struct pinctrl_dev *pctldev, > > @@ -198,8 +198,8 @@ static int s32_get_group_pins(struct pinctrl_dev *pctldev, > > struct s32_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev); > > const struct s32_pinctrl_soc_info *info = ipctl->info; > > > > - *pins = info->groups[selector].pin_ids; > > - *npins = info->groups[selector].npins; > > + *pins = info->groups[selector].data.pins; > > + *npins = info->groups[selector].data.npins; > > > > return 0; > > } > > @@ -314,23 +314,23 @@ static int s32_pmx_set(struct pinctrl_dev *pctldev, unsigned int selector, > > grp = &info->groups[group]; > > > > dev_dbg(ipctl->dev, "set mux for function %s group %s\n", > > - info->functions[selector].name, grp->name); > > + info->functions[selector].name, grp->data.name); > > > > /* Check beforehand so we don't have a partial config. */ > > - for (i = 0; i < grp->npins; i++) { > > - if (s32_check_pin(pctldev, grp->pin_ids[i]) != 0) { > > + for (i = 0; i < grp->data.npins; i++) { > > + if (s32_check_pin(pctldev, grp->data.pins[i]) != 0) { > > dev_err(info->dev, "invalid pin: %u in group: %u\n", > > - grp->pin_ids[i], group); > > + grp->data.pins[i], group); > > return -EINVAL; > > } > > } > > > > - for (i = 0, ret = 0; i < grp->npins && !ret; i++) { > > - ret = s32_regmap_update(pctldev, grp->pin_ids[i], > > + for (i = 0, ret = 0; i < grp->data.npins && !ret; i++) { > > + ret = s32_regmap_update(pctldev, grp->data.pins[i], > > S32_MSCR_SSS_MASK, grp->pin_sss[i]); > > if (ret) { > > dev_err(info->dev, "Failed to set pin %u\n", > > - grp->pin_ids[i]); > > + grp->data.pins[i]); > > return ret; > > } > > } > > @@ -364,7 +364,7 @@ static int s32_pmx_get_groups(struct pinctrl_dev *pctldev, > > const struct s32_pinctrl_soc_info *info = ipctl->info; > > > > *groups = info->functions[selector].groups; > > - *num_groups = info->functions[selector].num_groups; > > + *num_groups = info->functions[selector].ngroups; > > > > return 0; > > } > > @@ -602,8 +602,8 @@ static int s32_pconf_group_set(struct pinctrl_dev *pctldev, unsigned int selecto > > int i, ret; > > > > grp = &info->groups[selector]; > > - for (i = 0; i < grp->npins; i++) { > > - ret = s32_pinconf_mscr_update(pctldev, grp->pin_ids[i], > > + for (i = 0; i < grp->data.npins; i++) { > > + ret = s32_pinconf_mscr_update(pctldev, grp->data.pins[i], > > configs, num_configs); > > if (ret) > > return ret; > > @@ -637,9 +637,9 @@ static void s32_pinconf_group_dbg_show(struct pinctrl_dev *pctldev, > > > > seq_puts(s, "\n"); > > grp = &info->groups[selector]; > > - for (i = 0; i < grp->npins; i++) { > > - name = pin_get_name(pctldev, grp->pin_ids[i]); > > - ret = s32_regmap_read(pctldev, grp->pin_ids[i], &config); > > + for (i = 0; i < grp->data.npins; i++) { > > + name = pin_get_name(pctldev, grp->data.pins[i]); > > + ret = s32_regmap_read(pctldev, grp->data.pins[i], &config); > > if (ret) > > return; > > seq_printf(s, "%s: 0x%x\n", name, config); > > @@ -732,6 +732,7 @@ static int s32_pinctrl_parse_groups(struct device_node *np, > > const __be32 *p; > > struct device *dev; > > struct property *prop; > > + unsigned int *pins, *sss; > > int i, npins; > > u32 pinmux; > > > > @@ -740,38 +741,38 @@ static int s32_pinctrl_parse_groups(struct device_node *np, > > dev_dbg(dev, "group: %pOFn\n", np); > > > > /* Initialise group */ > > - grp->name = np->name; > > + grp->data.name = np->name; > > > > npins = of_property_count_elems_of_size(np, "pinmux", sizeof(u32)); > > if (npins < 0) { > > dev_err(dev, "Failed to read 'pinmux' property in node %s.\n", > > - grp->name); > > + grp->data.name); > > return -EINVAL; > > } > > if (!npins) { > > - dev_err(dev, "The group %s has no pins.\n", grp->name); > > + dev_err(dev, "The group %s has no pins.\n", grp->data.name); > > return -EINVAL; > > } > > > > - grp->npins = npins; > > + grp->data.npins = npins; > > > > - grp->pin_ids = devm_kcalloc(info->dev, grp->npins, > > - sizeof(unsigned int), GFP_KERNEL); > > - grp->pin_sss = devm_kcalloc(info->dev, grp->npins, > > - sizeof(unsigned int), GFP_KERNEL); > > - if (!grp->pin_ids || !grp->pin_sss) > > + pins = devm_kcalloc(info->dev, npins, sizeof(*pins), GFP_KERNEL); > > + sss = devm_kcalloc(info->dev, npins, sizeof(*sss), GFP_KERNEL); > > + if (!pins || !sss) > > return -ENOMEM; > > > > i = 0; > > of_property_for_each_u32(np, "pinmux", prop, p, pinmux) { > > - grp->pin_ids[i] = get_pin_no(pinmux); > > - grp->pin_sss[i] = get_pin_func(pinmux); > > + pins[i] = get_pin_no(pinmux); > > + sss[i] = get_pin_func(pinmux); > > > > - dev_dbg(info->dev, "pin-id: 0x%x, sss: 0x%x", > > - grp->pin_ids[i], grp->pin_sss[i]); > > + dev_dbg(info->dev, "pin: 0x%x, sss: 0x%x", pins[i], sss[i]); > > i++; > > } > > > > + grp->data.pins = pins; > > + grp->pin_sss = sss; > > + > > return 0; > > } > > > > @@ -780,8 +781,9 @@ static int s32_pinctrl_parse_functions(struct device_node *np, > > u32 index) > > { > > struct device_node *child; > > - struct s32_pmx_func *func; > > + struct pinfunction *func; > > struct s32_pin_group *grp; > > + char **groups; > > u32 i = 0; > > int ret = 0; > > > > @@ -791,18 +793,18 @@ static int s32_pinctrl_parse_functions(struct device_node *np, > > > > /* Initialise function */ > > func->name = np->name; > > - func->num_groups = of_get_child_count(np); > > - if (func->num_groups == 0) { > > + func->ngroups = of_get_child_count(np); > > + if (func->ngroups == 0) { > > dev_err(info->dev, "no groups defined in %pOF\n", np); > > return -EINVAL; > > } > > - func->groups = devm_kcalloc(info->dev, func->num_groups, > > - sizeof(*func->groups), GFP_KERNEL); > > - if (!func->groups) > > + groups = devm_kcalloc(info->dev, func->ngroups, > > + sizeof(*func->groups), GFP_KERNEL); > > + if (!groups) > > return -ENOMEM; > > > > for_each_child_of_node(np, child) { > > - func->groups[i] = child->name; > > + groups[i] = (char *)child->name; > > grp = &info->groups[info->grp_index++]; > > ret = s32_pinctrl_parse_groups(child, grp, info); > > if (ret) > > @@ -810,6 +812,8 @@ static int s32_pinctrl_parse_functions(struct device_node *np, > > i++; > > } > > > > + func->groups = (const char **)groups; > > Hmm... Why is casting needed? It's used for fulfilling the type checking done by kbuild otherwise an error will occur: drivers/pinctrl/nxp/pinctrl-s32cc.c:815:22: error: assignment to 'const char * const*' from incompatible pointer type 'char **' [-Werror=incompatible-pointer-types] In 'struct pinfunction', the member 'groups' is declared as (const char * const *). Regards, Chester > > > return 0; > > } > > -- > With Best Regards, > Andy Shevchenko
On Tue, Mar 21, 2023 at 7:09 AM Chester Lin <clin@suse.com> wrote: > On Mon, Mar 20, 2023 at 07:10:40PM +0200, Andy Shevchenko wrote: > > On Mon, Mar 20, 2023 at 6:39 PM Chester Lin <clin@suse.com> wrote: ... > > > for_each_child_of_node(np, child) { > > > - func->groups[i] = child->name; > > > + groups[i] = (char *)child->name; Here is also questionable casting. ... > > > + func->groups = (const char **)groups; > > > > Hmm... Why is casting needed? > > It's used for fulfilling the type checking done by kbuild otherwise an error will occur: > > drivers/pinctrl/nxp/pinctrl-s32cc.c:815:22: error: assignment to 'const char * const*' from incompatible pointer type 'char **' [-Werror=incompatible-pointer-types] > > In 'struct pinfunction', the member 'groups' is declared as (const char * const *). So, please decouple `struct pingroup` change to a separate patch and hence `struct pinfunction` on its own. After, consider changing types elsewhere that are following the types in that data structures.
diff --git a/drivers/pinctrl/nxp/pinctrl-s32.h b/drivers/pinctrl/nxp/pinctrl-s32.h index 545bf16b988d..2f7aecd462e4 100644 --- a/drivers/pinctrl/nxp/pinctrl-s32.h +++ b/drivers/pinctrl/nxp/pinctrl-s32.h @@ -15,31 +15,15 @@ struct platform_device; /** * struct s32_pin_group - describes an S32 pin group - * @name: the name of this specific pin group - * @npins: the number of pins in this group array, i.e. the number of - * elements in pin_ids and pin_sss so we can iterate over that array - * @pin_ids: an array of pin IDs in this group - * @pin_sss: an array of source signal select configs paired with pin_ids + * @data: generic data describes group name, number of pins, and a pin array in + this group. + * @pin_sss: an array of source signal select configs paired with pin array. */ struct s32_pin_group { - const char *name; - unsigned int npins; - unsigned int *pin_ids; + struct pingroup data; unsigned int *pin_sss; }; -/** - * struct s32_pmx_func - describes S32 pinmux functions - * @name: the name of this specific function - * @groups: corresponding pin groups - * @num_groups: the number of groups - */ -struct s32_pmx_func { - const char *name; - const char **groups; - unsigned int num_groups; -}; - /** * struct s32_pin_range - pin ID range for each memory region. * @start: start pin ID @@ -56,7 +40,7 @@ struct s32_pinctrl_soc_info { unsigned int npins; struct s32_pin_group *groups; unsigned int ngroups; - struct s32_pmx_func *functions; + struct pinfunction *functions; unsigned int nfunctions; unsigned int grp_index; const struct s32_pin_range *mem_pin_ranges; diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c index cb8a0844c0fa..4ed0cc905232 100644 --- a/drivers/pinctrl/nxp/pinctrl-s32cc.c +++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c @@ -188,7 +188,7 @@ static const char *s32_get_group_name(struct pinctrl_dev *pctldev, struct s32_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev); const struct s32_pinctrl_soc_info *info = ipctl->info; - return info->groups[selector].name; + return info->groups[selector].data.name; } static int s32_get_group_pins(struct pinctrl_dev *pctldev, @@ -198,8 +198,8 @@ static int s32_get_group_pins(struct pinctrl_dev *pctldev, struct s32_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev); const struct s32_pinctrl_soc_info *info = ipctl->info; - *pins = info->groups[selector].pin_ids; - *npins = info->groups[selector].npins; + *pins = info->groups[selector].data.pins; + *npins = info->groups[selector].data.npins; return 0; } @@ -314,23 +314,23 @@ static int s32_pmx_set(struct pinctrl_dev *pctldev, unsigned int selector, grp = &info->groups[group]; dev_dbg(ipctl->dev, "set mux for function %s group %s\n", - info->functions[selector].name, grp->name); + info->functions[selector].name, grp->data.name); /* Check beforehand so we don't have a partial config. */ - for (i = 0; i < grp->npins; i++) { - if (s32_check_pin(pctldev, grp->pin_ids[i]) != 0) { + for (i = 0; i < grp->data.npins; i++) { + if (s32_check_pin(pctldev, grp->data.pins[i]) != 0) { dev_err(info->dev, "invalid pin: %u in group: %u\n", - grp->pin_ids[i], group); + grp->data.pins[i], group); return -EINVAL; } } - for (i = 0, ret = 0; i < grp->npins && !ret; i++) { - ret = s32_regmap_update(pctldev, grp->pin_ids[i], + for (i = 0, ret = 0; i < grp->data.npins && !ret; i++) { + ret = s32_regmap_update(pctldev, grp->data.pins[i], S32_MSCR_SSS_MASK, grp->pin_sss[i]); if (ret) { dev_err(info->dev, "Failed to set pin %u\n", - grp->pin_ids[i]); + grp->data.pins[i]); return ret; } } @@ -364,7 +364,7 @@ static int s32_pmx_get_groups(struct pinctrl_dev *pctldev, const struct s32_pinctrl_soc_info *info = ipctl->info; *groups = info->functions[selector].groups; - *num_groups = info->functions[selector].num_groups; + *num_groups = info->functions[selector].ngroups; return 0; } @@ -602,8 +602,8 @@ static int s32_pconf_group_set(struct pinctrl_dev *pctldev, unsigned int selecto int i, ret; grp = &info->groups[selector]; - for (i = 0; i < grp->npins; i++) { - ret = s32_pinconf_mscr_update(pctldev, grp->pin_ids[i], + for (i = 0; i < grp->data.npins; i++) { + ret = s32_pinconf_mscr_update(pctldev, grp->data.pins[i], configs, num_configs); if (ret) return ret; @@ -637,9 +637,9 @@ static void s32_pinconf_group_dbg_show(struct pinctrl_dev *pctldev, seq_puts(s, "\n"); grp = &info->groups[selector]; - for (i = 0; i < grp->npins; i++) { - name = pin_get_name(pctldev, grp->pin_ids[i]); - ret = s32_regmap_read(pctldev, grp->pin_ids[i], &config); + for (i = 0; i < grp->data.npins; i++) { + name = pin_get_name(pctldev, grp->data.pins[i]); + ret = s32_regmap_read(pctldev, grp->data.pins[i], &config); if (ret) return; seq_printf(s, "%s: 0x%x\n", name, config); @@ -732,6 +732,7 @@ static int s32_pinctrl_parse_groups(struct device_node *np, const __be32 *p; struct device *dev; struct property *prop; + unsigned int *pins, *sss; int i, npins; u32 pinmux; @@ -740,38 +741,38 @@ static int s32_pinctrl_parse_groups(struct device_node *np, dev_dbg(dev, "group: %pOFn\n", np); /* Initialise group */ - grp->name = np->name; + grp->data.name = np->name; npins = of_property_count_elems_of_size(np, "pinmux", sizeof(u32)); if (npins < 0) { dev_err(dev, "Failed to read 'pinmux' property in node %s.\n", - grp->name); + grp->data.name); return -EINVAL; } if (!npins) { - dev_err(dev, "The group %s has no pins.\n", grp->name); + dev_err(dev, "The group %s has no pins.\n", grp->data.name); return -EINVAL; } - grp->npins = npins; + grp->data.npins = npins; - grp->pin_ids = devm_kcalloc(info->dev, grp->npins, - sizeof(unsigned int), GFP_KERNEL); - grp->pin_sss = devm_kcalloc(info->dev, grp->npins, - sizeof(unsigned int), GFP_KERNEL); - if (!grp->pin_ids || !grp->pin_sss) + pins = devm_kcalloc(info->dev, npins, sizeof(*pins), GFP_KERNEL); + sss = devm_kcalloc(info->dev, npins, sizeof(*sss), GFP_KERNEL); + if (!pins || !sss) return -ENOMEM; i = 0; of_property_for_each_u32(np, "pinmux", prop, p, pinmux) { - grp->pin_ids[i] = get_pin_no(pinmux); - grp->pin_sss[i] = get_pin_func(pinmux); + pins[i] = get_pin_no(pinmux); + sss[i] = get_pin_func(pinmux); - dev_dbg(info->dev, "pin-id: 0x%x, sss: 0x%x", - grp->pin_ids[i], grp->pin_sss[i]); + dev_dbg(info->dev, "pin: 0x%x, sss: 0x%x", pins[i], sss[i]); i++; } + grp->data.pins = pins; + grp->pin_sss = sss; + return 0; } @@ -780,8 +781,9 @@ static int s32_pinctrl_parse_functions(struct device_node *np, u32 index) { struct device_node *child; - struct s32_pmx_func *func; + struct pinfunction *func; struct s32_pin_group *grp; + char **groups; u32 i = 0; int ret = 0; @@ -791,18 +793,18 @@ static int s32_pinctrl_parse_functions(struct device_node *np, /* Initialise function */ func->name = np->name; - func->num_groups = of_get_child_count(np); - if (func->num_groups == 0) { + func->ngroups = of_get_child_count(np); + if (func->ngroups == 0) { dev_err(info->dev, "no groups defined in %pOF\n", np); return -EINVAL; } - func->groups = devm_kcalloc(info->dev, func->num_groups, - sizeof(*func->groups), GFP_KERNEL); - if (!func->groups) + groups = devm_kcalloc(info->dev, func->ngroups, + sizeof(*func->groups), GFP_KERNEL); + if (!groups) return -ENOMEM; for_each_child_of_node(np, child) { - func->groups[i] = child->name; + groups[i] = (char *)child->name; grp = &info->groups[info->grp_index++]; ret = s32_pinctrl_parse_groups(child, grp, info); if (ret) @@ -810,6 +812,8 @@ static int s32_pinctrl_parse_functions(struct device_node *np, i++; } + func->groups = (const char **)groups; + return 0; }
Use generic data structure to describe pin control functions and groups in S32 SoC family and drop duplicated struct members. Signed-off-by: Chester Lin <clin@suse.com> --- Changes in v2: - Simply use generic 'struct pinfunction' rather than having extra 'struct s32_pmx_func'. drivers/pinctrl/nxp/pinctrl-s32.h | 26 ++-------- drivers/pinctrl/nxp/pinctrl-s32cc.c | 76 +++++++++++++++-------------- 2 files changed, 45 insertions(+), 57 deletions(-)