[PATCH v1 1/3] pinctrl: intel: Extract intel_pinctrl_get_soc_data() helper for wider use

[PATCH v1 1/3] pinctrl: intel: Extract intel_pinctrl_get_soc_data() helper for wider use

[Andy Shevchenko]

intel_pinctrl_get_soc_data() helper can be used in few driver instead of
open-coded variants. Thus, extract it as a standalone API.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 24 ++++++++++++++++++------
 drivers/pinctrl/intel/pinctrl-intel.h |  2 ++
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 5a39e6ce6786..6e11d16a8684 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1424,9 +1424,6 @@ static int intel_pinctrl_probe(struct platform_device *pdev,
 	struct intel_pinctrl *pctrl;
 	int i, ret, irq;
 
-	if (!soc_data)
-		return -EINVAL;
-
 	pctrl = devm_kzalloc(&pdev->dev, sizeof(*pctrl), GFP_KERNEL);
 	if (!pctrl)
 		return -ENOMEM;
@@ -1534,11 +1531,26 @@ int intel_pinctrl_probe_by_hid(struct platform_device *pdev)
 	const struct intel_pinctrl_soc_data *data;
 
 	data = device_get_match_data(&pdev->dev);
+	if (!data)
+		return -ENODATA;
+
 	return intel_pinctrl_probe(pdev, data);
 }
 EXPORT_SYMBOL_GPL(intel_pinctrl_probe_by_hid);
 
 int intel_pinctrl_probe_by_uid(struct platform_device *pdev)
+{
+	const struct intel_pinctrl_soc_data *data;
+
+	data = intel_pinctrl_get_soc_data(pdev);
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+
+	return intel_pinctrl_probe(pdev, data);
+}
+EXPORT_SYMBOL_GPL(intel_pinctrl_probe_by_uid);
+
+const struct intel_pinctrl_soc_data *intel_pinctrl_get_soc_data(struct platform_device *pdev)
 {
 	const struct intel_pinctrl_soc_data *data = NULL;
 	const struct intel_pinctrl_soc_data **table;
@@ -1561,15 +1573,15 @@ int intel_pinctrl_probe_by_uid(struct platform_device *pdev)
 
 		id = platform_get_device_id(pdev);
 		if (!id)
-			return -ENODEV;
+			return ERR_PTR(-ENODEV);
 
 		table = (const struct intel_pinctrl_soc_data **)id->driver_data;
 		data = table[pdev->id];
 	}
 
-	return intel_pinctrl_probe(pdev, data);
+	return data ?: ERR_PTR(-ENODATA);
 }
-EXPORT_SYMBOL_GPL(intel_pinctrl_probe_by_uid);
+EXPORT_SYMBOL_GPL(intel_pinctrl_get_soc_data);
 
 #ifdef CONFIG_PM_SLEEP
 static bool intel_pinctrl_should_save(struct intel_pinctrl *pctrl, unsigned int pin)
diff --git a/drivers/pinctrl/intel/pinctrl-intel.h b/drivers/pinctrl/intel/pinctrl-intel.h
index 2e283247f3ba..38a1d6af7427 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.h
+++ b/drivers/pinctrl/intel/pinctrl-intel.h
@@ -198,6 +198,8 @@ struct intel_pinctrl_soc_data {
 	size_t ncommunities;
 };
 
+const struct intel_pinctrl_soc_data *intel_pinctrl_get_soc_data(struct platform_device *pdev);
+
 struct intel_pad_context;
 struct intel_community_context;
 
-- 
2.27.0

[PATCH v1 2/3] pinctrl: baytrail: Switch to use intel_pinctrl_get_soc_data()

[Andy Shevchenko]

Since we have common helper to retrieve SoC data from driver data
we may switch to use it.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-baytrail.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c
index d6e35cba3065..d49aab3cfbaa 100644
--- a/drivers/pinctrl/intel/pinctrl-baytrail.c
+++ b/drivers/pinctrl/intel/pinctrl-baytrail.c
@@ -1635,28 +1635,14 @@ static const struct acpi_device_id byt_gpio_acpi_match[] = {
 
 static int byt_pinctrl_probe(struct platform_device *pdev)
 {
-	const struct intel_pinctrl_soc_data *soc_data = NULL;
-	const struct intel_pinctrl_soc_data **soc_table;
+	const struct intel_pinctrl_soc_data *soc_data;
 	struct device *dev = &pdev->dev;
-	struct acpi_device *acpi_dev;
 	struct intel_pinctrl *vg;
-	int i, ret;
-
-	acpi_dev = ACPI_COMPANION(dev);
-	if (!acpi_dev)
-		return -ENODEV;
-
-	soc_table = (const struct intel_pinctrl_soc_data **)device_get_match_data(dev);
-
-	for (i = 0; soc_table[i]; i++) {
-		if (!strcmp(acpi_dev->pnp.unique_id, soc_table[i]->uid)) {
-			soc_data = soc_table[i];
-			break;
-		}
-	}
+	int ret;
 
-	if (!soc_data)
-		return -ENODEV;
+	soc_data = intel_pinctrl_get_soc_data(pdev);
+	if (IS_ERR(soc_data))
+		return PTR_ERR(soc_data);
 
 	vg = devm_kzalloc(dev, sizeof(*vg), GFP_KERNEL);
 	if (!vg)
-- 
2.27.0

[PATCH v1 3/3] pinctrl: cherryview: Switch to use intel_pinctrl_get_soc_data()

[Andy Shevchenko]

Since we have common helper to retrieve SoC data from driver data
we may switch to use it.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 23 ++++++----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index c18abc1a0d04..595686e6a238 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -1605,28 +1605,17 @@ static acpi_status chv_pinctrl_mmio_access_handler(u32 function,
 
 static int chv_pinctrl_probe(struct platform_device *pdev)
 {
-	const struct intel_pinctrl_soc_data *soc_data = NULL;
-	const struct intel_pinctrl_soc_data **soc_table;
+	const struct intel_pinctrl_soc_data *soc_data;
 	struct intel_community *community;
 	struct device *dev = &pdev->dev;
+	struct acpi_device *adev = ACPI_COMPANION(dev);
 	struct intel_pinctrl *pctrl;
-	struct acpi_device *adev;
 	acpi_status status;
-	int ret, irq, i;
+	int ret, irq;
 
-	adev = ACPI_COMPANION(&pdev->dev);
-	if (!adev)
-		return -ENODEV;
-
-	soc_table = (const struct intel_pinctrl_soc_data **)device_get_match_data(dev);
-	for (i = 0; soc_table[i]; i++) {
-		if (!strcmp(adev->pnp.unique_id, soc_table[i]->uid)) {
-			soc_data = soc_table[i];
-			break;
-		}
-	}
-	if (!soc_data)
-		return -ENODEV;
+	soc_data = intel_pinctrl_get_soc_data(pdev);
+	if (IS_ERR(soc_data))
+		return PTR_ERR(soc_data);
 
 	pctrl = devm_kzalloc(dev, sizeof(*pctrl), GFP_KERNEL);
 	if (!pctrl)
-- 
2.27.0

Re: [PATCH v1 1/3] pinctrl: intel: Extract intel_pinctrl_get_soc_data() helper for wider use

[Mika Westerberg]

On Wed, Jul 29, 2020 at 02:57:06PM +0300, Andy Shevchenko wrote:
> intel_pinctrl_get_soc_data() helper can be used in few driver instead of
> open-coded variants. Thus, extract it as a standalone API.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pinctrl/intel/pinctrl-intel.c | 24 ++++++++++++++++++------
>  drivers/pinctrl/intel/pinctrl-intel.h |  2 ++
>  2 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
> index 5a39e6ce6786..6e11d16a8684 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -1424,9 +1424,6 @@ static int intel_pinctrl_probe(struct platform_device *pdev,
>  	struct intel_pinctrl *pctrl;
>  	int i, ret, irq;
>  
> -	if (!soc_data)
> -		return -EINVAL;
> -
>  	pctrl = devm_kzalloc(&pdev->dev, sizeof(*pctrl), GFP_KERNEL);
>  	if (!pctrl)
>  		return -ENOMEM;
> @@ -1534,11 +1531,26 @@ int intel_pinctrl_probe_by_hid(struct platform_device *pdev)
>  	const struct intel_pinctrl_soc_data *data;
>  
>  	data = device_get_match_data(&pdev->dev);
> +	if (!data)
> +		return -ENODATA;
> +
>  	return intel_pinctrl_probe(pdev, data);
>  }
>  EXPORT_SYMBOL_GPL(intel_pinctrl_probe_by_hid);
>  
>  int intel_pinctrl_probe_by_uid(struct platform_device *pdev)
> +{
> +	const struct intel_pinctrl_soc_data *data;
> +
> +	data = intel_pinctrl_get_soc_data(pdev);
> +	if (IS_ERR(data))
> +		return PTR_ERR(data);
> +
> +	return intel_pinctrl_probe(pdev, data);
> +}
> +EXPORT_SYMBOL_GPL(intel_pinctrl_probe_by_uid);
> +
> +const struct intel_pinctrl_soc_data *intel_pinctrl_get_soc_data(struct platform_device *pdev)

Can we make this take const parameter as well?

>  {
>  	const struct intel_pinctrl_soc_data *data = NULL;
>  	const struct intel_pinctrl_soc_data **table;
> @@ -1561,15 +1573,15 @@ int intel_pinctrl_probe_by_uid(struct platform_device *pdev)
>  
>  		id = platform_get_device_id(pdev);
>  		if (!id)
> -			return -ENODEV;
> +			return ERR_PTR(-ENODEV);
>  
>  		table = (const struct intel_pinctrl_soc_data **)id->driver_data;
>  		data = table[pdev->id];
>  	}
>  
> -	return intel_pinctrl_probe(pdev, data);
> +	return data ?: ERR_PTR(-ENODATA);
>  }
> -EXPORT_SYMBOL_GPL(intel_pinctrl_probe_by_uid);
> +EXPORT_SYMBOL_GPL(intel_pinctrl_get_soc_data);
>  
>  #ifdef CONFIG_PM_SLEEP
>  static bool intel_pinctrl_should_save(struct intel_pinctrl *pctrl, unsigned int pin)
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.h b/drivers/pinctrl/intel/pinctrl-intel.h
> index 2e283247f3ba..38a1d6af7427 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.h
> +++ b/drivers/pinctrl/intel/pinctrl-intel.h
> @@ -198,6 +198,8 @@ struct intel_pinctrl_soc_data {
>  	size_t ncommunities;
>  };
>  
> +const struct intel_pinctrl_soc_data *intel_pinctrl_get_soc_data(struct platform_device *pdev);
> +
>  struct intel_pad_context;
>  struct intel_community_context;
>  
> -- 
> 2.27.0

Re: [PATCH v1 1/3] pinctrl: intel: Extract intel_pinctrl_get_soc_data() helper for wider use

[Andy Shevchenko]

On Mon, Aug 03, 2020 at 03:33:18PM +0300, Mika Westerberg wrote:
> On Wed, Jul 29, 2020 at 02:57:06PM +0300, Andy Shevchenko wrote:
> > intel_pinctrl_get_soc_data() helper can be used in few driver instead of
> > open-coded variants. Thus, extract it as a standalone API.

...

> > +const struct intel_pinctrl_soc_data *intel_pinctrl_get_soc_data(struct platform_device *pdev)
> 
> Can we make this take const parameter as well?

You mean

const struct intel_pinctrl_soc_data *
intel_pinctrl_get_soc_data(const struct platform_device *pdev)

?

Sure, I can do it for v2.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/3] pinctrl: intel: Extract intel_pinctrl_get_soc_data() helper for wider use

[Mika Westerberg]

On Mon, Aug 03, 2020 at 04:50:31PM +0300, Andy Shevchenko wrote:
> On Mon, Aug 03, 2020 at 03:33:18PM +0300, Mika Westerberg wrote:
> > On Wed, Jul 29, 2020 at 02:57:06PM +0300, Andy Shevchenko wrote:
> > > intel_pinctrl_get_soc_data() helper can be used in few driver instead of
> > > open-coded variants. Thus, extract it as a standalone API.
> 
> ...
> 
> > > +const struct intel_pinctrl_soc_data *intel_pinctrl_get_soc_data(struct platform_device *pdev)
> > 
> > Can we make this take const parameter as well?
> 
> You mean
> 
> const struct intel_pinctrl_soc_data *
> intel_pinctrl_get_soc_data(const struct platform_device *pdev)
> 
> ?

Yes, exactly.

> Sure, I can do it for v2.

Thanks!

Re: [PATCH v1 1/3] pinctrl: intel: Extract intel_pinctrl_get_soc_data() helper for wider use

[Andy Shevchenko]

On Mon, Aug 03, 2020 at 04:50:31PM +0300, Andy Shevchenko wrote:
> On Mon, Aug 03, 2020 at 03:33:18PM +0300, Mika Westerberg wrote:
> > On Wed, Jul 29, 2020 at 02:57:06PM +0300, Andy Shevchenko wrote:
> > > intel_pinctrl_get_soc_data() helper can be used in few driver instead of
> > > open-coded variants. Thus, extract it as a standalone API.
> 
> ...
> 
> > > +const struct intel_pinctrl_soc_data *intel_pinctrl_get_soc_data(struct platform_device *pdev)
> > 
> > Can we make this take const parameter as well?
> 
> You mean
> 
> const struct intel_pinctrl_soc_data *
> intel_pinctrl_get_soc_data(const struct platform_device *pdev)
> 
> ?
> 
> Sure, I can do it for v2.

Actually I can't. device_match_data() requires struct device * pointer and
compiler is unable to compile with const qualifier.

Are you good with current version then?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/3] pinctrl: intel: Extract intel_pinctrl_get_soc_data() helper for wider use

[Mika Westerberg]

On Tue, Aug 18, 2020 at 03:34:59PM +0300, Andy Shevchenko wrote:
> On Mon, Aug 03, 2020 at 04:50:31PM +0300, Andy Shevchenko wrote:
> > On Mon, Aug 03, 2020 at 03:33:18PM +0300, Mika Westerberg wrote:
> > > On Wed, Jul 29, 2020 at 02:57:06PM +0300, Andy Shevchenko wrote:
> > > > intel_pinctrl_get_soc_data() helper can be used in few driver instead of
> > > > open-coded variants. Thus, extract it as a standalone API.
> > 
> > ...
> > 
> > > > +const struct intel_pinctrl_soc_data *intel_pinctrl_get_soc_data(struct platform_device *pdev)
> > > 
> > > Can we make this take const parameter as well?
> > 
> > You mean
> > 
> > const struct intel_pinctrl_soc_data *
> > intel_pinctrl_get_soc_data(const struct platform_device *pdev)
> > 
> > ?
> > 
> > Sure, I can do it for v2.
> 
> Actually I can't. device_match_data() requires struct device * pointer and
> compiler is unable to compile with const qualifier.

OK

> Are you good with current version then?

Yes :)

Re: [PATCH v1 1/3] pinctrl: intel: Extract intel_pinctrl_get_soc_data() helper for wider use

[Andy Shevchenko]

On Tue, Aug 18, 2020 at 03:36:02PM +0300, Mika Westerberg wrote:
> On Tue, Aug 18, 2020 at 03:34:59PM +0300, Andy Shevchenko wrote:
> > On Mon, Aug 03, 2020 at 04:50:31PM +0300, Andy Shevchenko wrote:
> > > On Mon, Aug 03, 2020 at 03:33:18PM +0300, Mika Westerberg wrote:
> > > > On Wed, Jul 29, 2020 at 02:57:06PM +0300, Andy Shevchenko wrote:
> > > > > intel_pinctrl_get_soc_data() helper can be used in few driver instead of
> > > > > open-coded variants. Thus, extract it as a standalone API.
> > > 
> > > ...
> > > 
> > > > > +const struct intel_pinctrl_soc_data *intel_pinctrl_get_soc_data(struct platform_device *pdev)
> > > > 
> > > > Can we make this take const parameter as well?
> > > 
> > > You mean
> > > 
> > > const struct intel_pinctrl_soc_data *
> > > intel_pinctrl_get_soc_data(const struct platform_device *pdev)
> > > 
> > > ?
> > > 
> > > Sure, I can do it for v2.
> > 
> > Actually I can't. device_match_data() requires struct device * pointer and
> > compiler is unable to compile with const qualifier.
> 
> OK
> 
> > Are you good with current version then?
> 
> Yes :)

Pushed to my review and testing queue, thanks!

P.S. I have added your Ack as discussed offline.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/3] pinctrl: intel: Extract intel_pinctrl_get_soc_data() helper for wider use

[Andy Shevchenko]

On Tue, Aug 18, 2020 at 03:34:59PM +0300, Andy Shevchenko wrote:
> On Mon, Aug 03, 2020 at 04:50:31PM +0300, Andy Shevchenko wrote:
> > On Mon, Aug 03, 2020 at 03:33:18PM +0300, Mika Westerberg wrote:
> > > On Wed, Jul 29, 2020 at 02:57:06PM +0300, Andy Shevchenko wrote:

...

> > > > +const struct intel_pinctrl_soc_data *intel_pinctrl_get_soc_data(struct platform_device *pdev)
> > > 
> > > Can we make this take const parameter as well?
> > 
> > You mean
> > 
> > const struct intel_pinctrl_soc_data *
> > intel_pinctrl_get_soc_data(const struct platform_device *pdev)
> > 
> > ?
> > 
> > Sure, I can do it for v2.
> 
> Actually I can't. device_match_data() requires struct device * pointer and
> compiler is unable to compile with const qualifier.

pinctrl-intel.c: In function ‘intel_pinctrl_get_soc_data’:
pinctrl-intel.c:1533:45: warning: passing argument 1 of ‘device_get_match_data’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]


> Are you good with current version then?

-- 
With Best Regards,
Andy Shevchenko