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

[Krzysztof Kozlowski <krzk@xxxxxxxxxx>]

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