Re: [PATCH v6 4/7] pinctrl: s32: convert the driver into an mfd cell

Krzysztof Kozlowski
2024-11-19T09:26:17Z

On 13/11/2024 11:10, Andrei Stefanescu wrote:
>
> -static inline void s32_pin_set_pull(enum pin_config_param param,
> - unsigned int *mask, unsigned int *config)
> +static void s32_pin_set_pull(enum pin_config_param param,
> + unsigned int *mask, unsigned int *config)
> {
> switch (param) {
> case PIN_CONFIG_BIAS_DISABLE:
> @@ -762,15 +757,15 @@ static int s32_pinctrl_parse_groups(struct device_node *np,
> 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->data.name);
> - return -EINVAL;
> - }
> - if (!npins) {
> - dev_err(dev, "The group %s has no pins.\n", grp->data.name);
> - return -EINVAL;
> - }
> + if (npins < 0)
> + return dev_err_probe(dev, -EINVAL,
> + "Failed to read 'pinmux' in node %s\n",
> + grp->data.name);

I do not see how this change is related. Looks you are mixing cleanups
with refactoring into MFD cell. These are two different things.

> +
> + if (!npins)
> + return dev_err_probe(dev, -EINVAL,
> + "The group %s has no pins\n",
> + grp->data.name);
>
> grp->data.npins = npins;
>
> @@ -811,11 +806,9 @@ static int s32_pinctrl_parse_functions(struct device_node *np,
> /* Initialise function */
> func->name = np->name;
> func->ngroups = of_get_child_count(np);
> - if (func->ngroups == 0) {
> - dev_err(info->dev, "no groups defined in %pOF\n", np);
> - return -EINVAL;
> - }
> -
> + if (func->ngroups == 0)
> + return dev_err_probe(info->dev, -EINVAL,
> + "No groups defined in %pOF\n", np);
> groups = devm_kcalloc(info->dev, func->ngroups,
> sizeof(*func->groups), GFP_KERNEL);
> if (!groups)
> @@ -838,57 +831,42 @@ static int s32_pinctrl_parse_functions(struct device_node *np,
> static int s32_pinctrl_probe_dt(struct platform_device *pdev,
> struct s32_pinctrl *ipctl)
> {
> + struct nxp_siul2_mfd *mfd = dev_get_drvdata(pdev->dev.parent);
> struct s32_pinctrl_soc_info *info = ipctl->info;
> - struct device_node *np = pdev->dev.of_node;
> - struct resource *res;
> - struct regmap *map;
> - void __iomem *base;
> - unsigned int mem_regions = info->soc_data->mem_regions;
> + unsigned int mem_regions;
> + struct device_node *np;
> + u32 nfuncs = 0, i = 0, j;
> + u8 regmap_type;
> int ret;
> - u32 nfuncs = 0;
> - u32 i = 0;
>
> + np = pdev->dev.parent->of_node;
> if (!np)
> return -ENODEV;
>
> - if (mem_regions == 0 || mem_regions >= 10000) {
> - dev_err(&pdev->dev, "mem_regions is invalid: %u\n", mem_regions);
> - return -EINVAL;
> - }
> + /* one MSCR and one IMCR region per SIUL2 module */

How is this related to converion into MFD cell?

Still looks like an ABI break.

Best regards,
Krzysztof