PCI/pwrctl: pwrseq: abandon QCom WCN probe on pre-pwrseq device-trees

From: Bartosz Golaszewski <[email protected]>

Old device trees for some platforms already define wifi nodes for the WCN family of chips since before power sequencing was added upstream.

These nodes don't consume the regulator outputs from the PMU and if we allow this driver to bind to one of such "incomplete" nodes, we'll see a kernel log error about the infinite probe deferral.

Let's extend the driver by adding a platform data struct matched against the compatible. This struct will now contain the pwrseq target string as well as a validation function called right after entering probe(). For Qualcomm WCN models, we'll check the existence of the regulator supply property that indicates the DT is already using power sequencing and return -ENODEV if it's not there, indicating to the driver model that the device should not be bound to the pwrctl driver.

Fixes 6140d185a43d ("PCI/pwrctl: Add a PCI power control driver for power sequenced devices")

Reported-by Johan Hovold <[email protected]>

Closes https://lore.kernel.org/all/[email protected]/ open here

Signed-off-by Bartosz Golaszewski <[email protected]>

---

From: Bartosz Golaszewski <[email protected]>

Old device trees for some platforms already define wifi nodes for the WCN family of chips since before power sequencing was added upstream.

These nodes don't consume the regulator outputs from the PMU and if we allow this driver to bind to one of such "incomplete" nodes, we'll see a kernel log error about the infinite probe deferral.

Let's extend the driver by adding a platform data struct matched against the compatible. This struct will now contain the pwrseq target string as well as a validation function called right after entering probe(). For Qualcomm WCN models, we'll check the existence of the regulator supply property that indicates the DT is already using power sequencing and return -ENODEV if it's not there, indicating to the driver model that the device should not be bound to the pwrctl driver.

Fixes 6140d185a43d ("PCI/pwrctl: Add a PCI power control driver for power sequenced devices")

Reported-by Johan Hovold <[email protected]>

Closes https://lore.kernel.org/all/[email protected]/ open here

Signed-off-by Bartosz Golaszewski <[email protected]>

---

From: Bartosz Golaszewski <[email protected]>

Old device trees for some platforms already define wifi nodes for the WCN family of chips since before power sequencing was added upstream.

These nodes don't consume the regulator outputs from the PMU and if we allow this driver to bind to one of such "incomplete" nodes, we'll see a kernel log error about the infinite probe deferral.

Let's extend the driver by adding a platform data struct matched against the compatible. This struct will now contain the pwrseq target string as well as a validation function called right after entering probe(). For Qualcomm WCN models, we'll check the existence of the regulator supply property that indicates the DT is already using power sequencing and return -ENODEV if it's not there, indicating to the driver model that the device should not be bound to the pwrctl driver.

Fixes 6140d185a43d ("PCI/pwrctl: Add a PCI power control driver for power sequenced devices")

Reported-by Johan Hovold <[email protected]>

Closes https://lore.kernel.org/all/[email protected]/ open here

Signed-off-by Bartosz Golaszewski <[email protected]>

---
v1 -> v2:
- make the fix more generic, add a platform data struct matched against the compatible and use it to store a device-specific validation callback

 drivers/pci/pwrctl/pci-pwrctl-pwrseq.c | 55 +++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 5 deletions(-)

drivers/pci/pwrctl/pci-pwrctl-pwrseq.c +50 -5

index a23a4312574b..e331a73a1b0c 100644
@@ -6,9 +6,9 @@
 #include <linux/device.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
-#include <linux/of.h>
 #include <linux/pci-pwrctl.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/pwrseq/consumer.h>
 #include <linux/slab.h>
 #include <linux/types.h>

@@ -18,6 +18,40 @@ struct pci_pwrctl_pwrseq_data {
 	struct pwrseq_desc *pwrseq;
 };
 
+struct pci_pwrctl_pwrseq_pdata {
+	const char *target;
+	/*
+	 * Called before doing anything else to perform device-specific
+	 * verification between requesting the power sequencing handle.
+	 */
+	int (*validate_device)(struct device *dev);
+};
+
+static int pci_pwrctl_pwrseq_qcm_wcn_validate_device(struct device *dev)
+{
+	/*
+	 * Old device trees for some platforms already define wifi nodes for
+	 * the WCN family of chips since before power sequencing was added
+	 * upstream.
+	 *
+	 * These nodes don't consume the regulator outputs from the PMU and
+	 * if we allow this driver to bind to one of such "incomplete" nodes,
+	 * we'll see a kernel log error about the indefinite probe deferral.
+	 *
+	 * Let's check the existence of the regulator supply that exists on all
+	 * WCN models before moving forward.
+	 */
+	if (!device_property_present(dev, "vddaon-supply"))
+		return -ENODEV;
+
+	return 0;
+}
+
+static const struct pci_pwrctl_pwrseq_pdata pci_pwrctl_pwrseq_qcom_wcn_pdata = {
+	.target = "wlan",
+	.validate_device = pci_pwrctl_pwrseq_qcm_wcn_validate_device,
+};
+
 static void devm_pci_pwrctl_pwrseq_power_off(void *data)
 {
 	struct pwrseq_desc *pwrseq = data;

@@ -27,15 +61,26 @@ static void devm_pci_pwrctl_pwrseq_power_off(void *data)
 
 static int pci_pwrctl_pwrseq_probe(struct platform_device *pdev)
 {
+	const struct pci_pwrctl_pwrseq_pdata *pdata;
 	struct pci_pwrctl_pwrseq_data *data;
 	struct device *dev = &pdev->dev;
 	int ret;
 
+	pdata = device_get_match_data(dev);
+	if (!pdata || !pdata->target)
+		return -EINVAL;
+
+	if (pdata->validate_device) {
+		ret = pdata->validate_device(dev);
+		if (ret)
+			return ret;
+	}
+
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
-	data->pwrseq = devm_pwrseq_get(dev, of_device_get_match_data(dev));
+	data->pwrseq = devm_pwrseq_get(dev, pdata->target);
 	if (IS_ERR(data->pwrseq))
 		return dev_err_probe(dev, PTR_ERR(data->pwrseq),
 				     "Failed to get the power sequencer\n");

@@ -64,17 +109,17 @@ static const struct of_device_id pci_pwrctl_pwrseq_of_match[] = {
 	{
 		/* ATH11K in QCA6390 package. */
 		.compatible = "pci17cb,1101",
-		.data = "wlan",
+		.data = &pci_pwrctl_pwrseq_qcom_wcn_pdata,
 	},
 	{
 		/* ATH11K in WCN6855 package. */
 		.compatible = "pci17cb,1103",
-		.data = "wlan",
+		.data = &pci_pwrctl_pwrseq_qcom_wcn_pdata,
 	},
 	{
 		/* ATH12K in WCN7850 package. */
 		.compatible = "pci17cb,1107",
-		.data = "wlan",
+		.data = &pci_pwrctl_pwrseq_qcom_wcn_pdata,
 	},
 	{ }
 };

Signature · 1 line

2.30.2

Quoted 17 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Old device trees for some platforms already define wifi nodes for the WCN family of chips since before power sequencing was added upstream.

These nodes don't consume the regulator outputs from the PMU and if we allow this driver to bind to one of such "incomplete" nodes, we'll see a kernel log error about the infinite probe deferral.

Let's extend the driver by adding a platform data struct matched against the compatible. This struct will now contain the pwrseq target string as well as a validation function called right after entering probe(). For Qualcomm WCN models, we'll check the existence of the regulator supply property that indicates the DT is already using power sequencing and return -ENODEV if it's not there, indicating to the driver model that the device should not be bound to the pwrctl driver.

Quoted 4 lines

Fixes 6140d185a43d ("PCI/pwrctl: Add a PCI power control driver for power sequenced devices")

Reported-by Johan Hovold <[email protected]>

Closes https://lore.kernel.org/all/[email protected]/ open here

Signed-off-by Bartosz Golaszewski <[email protected]>

Applied to pci/for-linus for v6.12, thanks.

It would help me out to have a hint in the posting that this is intended for the current release, since by default everything is for the "next" release.