pinctrl: intel: Extract intel_pinctrl_get_soc_data() helper for wider use

PATCH In review Andy Shevchenko 2 participants 10 messages Jul 29, 2020 – Aug 18, 2020

Subsystems (3)

GPIO Pin control Pin controller - Intel

lore

Share & tools

lore

Andy Shevchenko

author

2020-07-29 11:57 UTC #0 lore raw

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

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 <[email protected]>

---
 drivers/pinctrl/intel/pinctrl-intel.c | 24 ++++++++++++++++++------
 drivers/pinctrl/intel/pinctrl-intel.h |  2 ++
 2 files changed, 20 insertions(+), 6 deletions(-)

drivers/pinctrl/intel/pinctrl-intel.c +18 -6

index 5a39e6ce6786..6e11d16a8684 100644
@@ -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)

drivers/pinctrl/intel/pinctrl-intel.h +2 -0

index 2e283247f3ba..38a1d6af7427 100644
@@ -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;

Signature · 1 line

2.27.0

Andy Shevchenko

author

2020-07-29 11:57 UTC #1 lore raw

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

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

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/pinctrl/intel/pinctrl-baytrail.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

drivers/pinctrl/intel/pinctrl-baytrail.c +5 ~~-19~~

index d6e35cba3065..d49aab3cfbaa 100644
@@ -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)

Signature · 1 line

2.27.0

Andy Shevchenko

author

2020-07-29 11:57 UTC #2 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 23 ++++++----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

drivers/pinctrl/intel/pinctrl-cherryview.c +6 ~~-17~~

index c18abc1a0d04..595686e6a238 100644
@@ -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)

Signature · 1 line

2.27.0

Mika Westerberg

maintainer

2020-08-03 12:39 UTC #3 lore raw

In reply to Andy Shevchenko

Quoted 37 lines · 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 <[email protected]>

---
 drivers/pinctrl/intel/pinctrl-intel.c | 24 ++++++++++++++++++------
 drivers/pinctrl/intel/pinctrl-intel.h |  2 ++
 2 files changed, 20 insertions(+), 6 deletions(-)

drivers/pinctrl/intel/pinctrl-intel.c +5 -3

index 5a39e6ce6786..6e11d16a8684 100644
@@ -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;

drivers/pinctrl/intel/pinctrl-intel.c int intel_pinctrl_probe_by_hid(struct platform_device *pdev) · line 1553

Quoted 10 lines

drivers/pinctrl/intel/pinctrl-intel.c +10 -0

+
+	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?

Quoted 37 lines

drivers/pinctrl/intel/pinctrl-intel.c +3 -3

 {
 	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)

drivers/pinctrl/intel/pinctrl-intel.h +2 -0

index 2e283247f3ba..38a1d6af7427 100644
@@ -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;

Signature · 1 line

2.27.0

Andy Shevchenko

author

2020-08-03 13:50 UTC #4 lore raw

Quoted 3 lines · Mika Westerberg

Quoted 2 lines · 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.

...

Quoted 3 lines

Quoted 1 line
+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.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Mika Westerberg

maintainer

2020-08-03 14:23 UTC #5 lore raw

Quoted 9 lines · Andy Shevchenko

Quoted 3 lines · Mika Westerberg
Quoted 2 lines · 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.
...
Quoted 2 lines
Quoted 1 line
+const struct intel_pinctrl_soc_data *intel_pinctrl_get_soc_data(struct platform_device *pdev)

Quoted 8 lines

Quoted 1 line
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.

Quoted 1 line

Sure, I can do it for v2.

Thanks!

Andy Shevchenko

author

2020-08-18 12:35 UTC #6 lore raw

In reply to Andy Shevchenko

Quoted 11 lines · Andy Shevchenko

Quoted 3 lines · Mika Westerberg
Quoted 2 lines · 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.
...
Quoted 3 lines
Quoted 1 line
+const struct intel_pinctrl_soc_data *intel_pinctrl_get_soc_data(struct platform_device *pdev)
Can we make this take const parameter as well?

Quoted 8 lines

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?

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Mika Westerberg

maintainer

2020-08-18 12:36 UTC #7 lore raw

Quoted 17 lines · Andy Shevchenko

Quoted 16 lines · Andy Shevchenko
Quoted 3 lines · Mika Westerberg
Quoted 2 lines · 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.
...
Quoted 3 lines
Quoted 1 line
+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)

Quoted 6 lines

Quoted 3 lines
?
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.

Quoted 1 line

Are you good with current version then?

Yes :)

Andy Shevchenko

author

2020-08-18 13:47 UTC #8 lore raw

Quoted 22 lines · Mika Westerberg

Quoted 21 lines · Andy Shevchenko
Quoted 19 lines · Andy Shevchenko
Quoted 3 lines · Mika Westerberg
Quoted 2 lines · 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.
...
Quoted 3 lines
Quoted 1 line
+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.

Quoted 8 lines

Quoted 2 lines
Actually I can't. device_match_data() requires struct device * pointer and compiler is unable to compile with const qualifier.
OK
Quoted 1 line
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.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Andy Shevchenko

author

2020-08-18 12:37 UTC #9 lore raw

In reply to Andy Shevchenko

Quoted 3 lines · Andy Shevchenko

Quoted 2 lines · Andy Shevchenko
Quoted 1 line · Mika Westerberg
On Wed, Jul 29, 2020 at 02:57:06PM +0300, Andy Shevchenko wrote:

...

Quoted 9 lines

Quoted 9 lines
Quoted 3 lines
Quoted 1 line
+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)

Quoted 6 lines

Quoted 3 lines
?
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]

Quoted 1 line

Are you good with current version then?

Signature · 2 lines

With Best Regards,
Andy Shevchenko