Re: [PATCH v4] pinctrl: qcom: sc8180x: add ACPI probe support

[Shawn Guo <shawn.guo@xxxxxxxxxx>]

On Wed, Mar 10, 2021 at 01:24:49PM -0600, Bjorn Andersson wrote:
> > @@ -1588,13 +1610,105 @@ static struct msm_pinctrl_soc_data sc8180x_pinctrl = {
> >  	.nwakeirq_map = ARRAY_SIZE(sc8180x_pdc_map),
> >  };
> >  
> > +static const struct msm_pinctrl_soc_data sc8180x_acpi_pinctrl = {
> > +	.tiles = sc8180x_tiles,
> > +	.ntiles = ARRAY_SIZE(sc8180x_tiles),
> > +	.pins = sc8180x_pins,
> > +	.npins = ARRAY_SIZE(sc8180x_pins),
> > +	.groups = sc8180x_groups,
> > +	.ngroups = ARRAY_SIZE(sc8180x_groups),
> > +	.reserved_gpios = sc8180x_acpi_reserved_gpios,
> > +	.ngpios = 191,
> 
> As I said before, there really only is 190 things called "GPIO" on this
> platform - the 191 thing is a hack of mine, but it's inclusion here
> should depend on how the ACPI system would reset the UFS device - if it
> does so at all.

No, so far I do not see reset is required for ACPI boot.  I will keep it
as 190 to match hardware spec.

> 
> > +};
> > +
> 
> A comment describing the purpose of this function (to split the
> IORESOURCE_MEM into three) would be in its place.

Okay.

> 
> > +static int sc8180x_pinctrl_add_tile_resources(struct platform_device *pdev)
> > +{
> > +	int nres_num = pdev->num_resources + ARRAY_SIZE(sc8180x_tiles) - 1;
> > +	struct resource *mres, *nres, *res;
> > +	int i, ret;
> > +
> > +	/*
> > +	 * DT already has tiles defined properly, so nothing needs to be done
> > +	 * for DT probe.
> > +	 */
> > +	if (pdev->dev.of_node)
> > +		return 0;
> > +
> > +	/* Allocate for new resources */
> > +	nres = devm_kzalloc(&pdev->dev, sizeof(*nres) * nres_num, GFP_KERNEL);
> > +	if (!nres)
> > +		return -ENOMEM;
> > +
> > +	res = nres;
> > +
> > +	for (i = 0; i < pdev->num_resources; i++) {
> > +		struct resource *r = &pdev->resource[i];
> > +
> > +		/* Save memory resource and copy others */
> > +		if (resource_type(r) == IORESOURCE_MEM)
> > +			mres = r;
> > +		else
> > +			*res++ = *r;
> > +	}
> > +
> > +	/* Append tile memory resources */
> > +	for (i = 0; i < ARRAY_SIZE(sc8180x_tiles); i++, res++) {
> > +		const struct tile_info *info = &sc8180x_tile_info[i];
> > +
> > +		res->start = mres->start + info->offset;
> > +		res->end = mres->start + info->offset + info->size - 1;
> > +		res->flags = mres->flags;
> > +		res->name = sc8180x_tiles[i];
> > +
> > +		/* Add new MEM to resource tree */
> > +		insert_resource(mres->parent, res);
> > +	}
> > +
> > +	/* Remove old MEM from resource tree */
> > +	remove_resource(mres);
> > +
> > +	/* Free old resources and install new ones */
> > +	ret = platform_device_add_resources(pdev, nres, nres_num);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "failed to add new resources: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int sc8180x_pinctrl_probe(struct platform_device *pdev)
> >  {
> > -	return msm_pinctrl_probe(pdev, &sc8180x_pinctrl);
> > +	const struct msm_pinctrl_soc_data *soc_data;
> > +	int ret;
> > +
> > +	soc_data = device_get_match_data(&pdev->dev);
> > +	if (!soc_data)
> > +		return -EINVAL;
> > +
> > +	ret = sc8180x_pinctrl_add_tile_resources(pdev);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "failed to add tile resources: %d\n", ret);
> 
> sc8180x_pinctrl_add_tile_resources() already printed an error when we
> get here, so no need to print another one.

Sure.

Shawn