[PATCH 1/3] PCI: of: Match pci devices or drivers against OF DT nodes

[PATCH 1/3] PCI: of: Match pci devices or drivers against OF DT nodes

[Edward Chow]

Currently, whether a compatibility string within an OF DT node for a
PCI device (whose spec is at
https://www.devicetree.org/open-firmware/bindings/pci/ ) matches the
vendor and device id of either the PCI device installed on the
corresponding location or the driver suggested by the compatibility
string is not supported.

This patch introduces a function to decode a compatibility string into
a struct pci_device_id, which could further be matched against PCI
devices or drivers, as well as functions to match a compatibility
string or OF DT node against PCI devices or drivers.

Signed-off-by: Edward Chow <equu@openmail.cc>
Cc: Bjorn Helgaas <helgaas@kernel.org>
---
 drivers/pci/of.c         | 299 +++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci-driver.c |   5 -
 drivers/pci/pci.h        |  56 ++++++++
 include/linux/of_pci.h   |  25 ++++
 include/linux/pci.h      |   6 +
 5 files changed, 386 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 196834ed44fe..edb61195ea53 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -13,6 +13,8 @@
 #include <linux/of_irq.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
+#include <linux/string.h>
+#include <linux/kstrtox.h>
 #include "pci.h"
 
 #ifdef CONFIG_PCI
@@ -251,6 +253,303 @@ void of_pci_check_probe_only(void)
 }
 EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
 
+/**
+ * of_pci_compat_to_device_id() - Decode an OF compatibility string into a
+ * pci_device_id structure.
+ * @compat: the compatibility string to decode, could be NULL
+ * @id: pointer to a struct pci_device_id, to store the result
+ * @rev: pointer to output revision info, PCI_ANY_ID if no revision in @compat
+ * @req_pcie: pointer to output whether @compat mandates PCIe compatibility
+ *
+ * returns 0 when success, -EINVAL when failed.
+ */
+int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       u32 *rev, u32 *req_pcie)
+{
+	union {
+		u8 u8;
+		u16 u16;
+		u32 u32;
+	} res = {0};
+	*req_pcie = 0;
+	*rev = PCI_ANY_ID;
+	if (!compat || strncasecmp(compat, "pci", 3) != 0)
+		return -EINVAL;
+	compat += 3;
+
+	if (strncasecmp(compat, "class,", 6) == 0) {
+		/* pciclass,CCSSPP */
+		compat += 6;
+		if ((strlen(compat) < 4)
+		   || kstrtouint(compat, 16, &id->class))
+			return -EINVAL;
+		if (id->class < 0x10000) {
+			id->class <<= 8;
+			id->class_mask = 0xFFFF00;
+		} else {
+			id->class_mask = PCI_ANY_ID;
+		}
+		id->vendor = PCI_ANY_ID;
+		id->device = PCI_ANY_ID;
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0
+	}
+
+	if (strncasecmp(compat, "ex", 2) == 0) {
+		/* pciex...  */
+		*req_pcie = 1;
+		compat += 2;
+	}
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->vendor = res.u16;
+	compat = strchr(compat, ',');
+	if (!compat)
+		return -EINVAL;
+	compat++;
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->device = res.u16;
+	compat = strchr(compat, '.');
+	if (compat == NULL) {
+		/* pciVVVV,DDDD */
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0
+	}
+
+	compat++;
+	if (strlen(compat) == 2) {
+		/* pciVVVV,DDDD.RR */
+		if (kstrtou8(compat, 16, &res.u8))
+			return -EINVAL;
+		*rev = res.u8;
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0
+	}
+
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->subvendor = res.u16;
+	compat = strchr(compat, '.');
+	if (!compat)
+		return -EINVAL;
+	compat++;
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->subdevice = res.u16;
+	compat = strchr(compat, '.');
+	if (compat == NULL)
+		/* pciVVVV,DDDD.SSSS.ssss */
+		return 0
+
+	compat++;
+	if (strlen(compat) == 2) {
+		/* pciVVVV,DDDD.SSSS.ssss.RR */
+		if (kstrtou8(compat, 16, &res.u8))
+			return -EINVAL;
+		*rev = res.u8;
+	}
+	return 0;
+}
+
+/**
+ * of_pci_compat_match_device() - Tell whether a PCI device structure matches
+ * a given OF compatibility string
+ * @compat: single OF compatibility string to match, could be NULL
+ * @dev the PCI device structure to match against
+ *
+ * Returns whether they match.
+ */
+bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev)
+{
+	__u32 rev = PCI_ANY_ID;
+	__u32 req_pcie = 0;
+	struct pci_device_id id = {0};
+
+	if (of_pci_compat_to_device_id(compat, &id, &rev, &req_pcie))
+		return false;
+	return pci_match_one_device(&id, dev) &&
+		(rev == PCI_ANY_ID || rev == dev->revision) &&
+		req_pcie ? dev->pcie_cap : true;
+}
+
+/**
+ * of_pci_node_match_device() - Tell whether an OF device tree node
+ * matches the given pci device
+ * @node: single OF device tree node to match, could be NULL
+ * @dev: the PCI device structure to match against, could be NULL
+ *
+ * Returns whether they match.
+ */
+bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev)
+{
+	struct property *prop;
+	const char *cp;
+
+	if (!node || !dev)
+		return false;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		if (of_pci_compat_match_device(cp, dev))
+			return true;
+	}
+	return false;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_device);
+
+/**
+ * of_pci_compat_match_one_id() - Tell whether a PCI device ID structure matches
+ * a given OF compatibility string, note that there is no revision nor PCIe
+ * capability info in PCI device ID structures
+ *
+ * @compat: single OF compatibility string to match, could be NULL
+ * @id the PCI device ID structure to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure pointed by ID
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id)
+{
+	__u32 rev = PCI_ANY_ID;
+	__u32 req_pcie = 0;
+	struct pci_device_id pr = {0};
+
+	if (!compat || !id ||
+	    of_pci_compat_to_device_id(compat, &pr, &rev, &req_pcie))
+		return NULL;
+	return pci_match_one_id(id, &pr);
+}
+
+/**
+ * of_pci_compat_match_id_table() - Tell whether a given OF compatibility string
+ * matches a given pci_id table
+ *
+ * @compat: single OF compatibility string to match, could be NULL
+ * @table the PCI device ID table to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table)
+{
+	if (compat && table) {
+		while (table->vendor || table->subvendor || table->class_mask) {
+			if (of_pci_compat_match_one_id(compat, table))
+				return table;
+			table++;
+		}
+	}
+	return NULL;
+}
+
+/**
+ * of_pci_node_match_id_table() - Tell whether an OF device tree node
+ * matches the given pci_id table
+ * @node: single OF device tree node to match, could be NULL
+ * @table: the PCI device ID table to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table)
+{
+	struct property *prop;
+	const char *cp;
+	const struct pci_device_id *id;
+
+	if (!node || !table)
+		return NULL;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		id = of_pci_compat_match_id_table(cp, table);
+		if (id)
+			return id;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_id_table);
+
+/**
+ * of_pci_compat_match_driver - See if a given OF compatibility string matches
+ * a driver's list of IDs
+ * @compat: single OF compatibility string to match, could be NULL
+ * @drv: the PCI driver to match against, could be NULL
+ *
+ * Used by a driver to check whether an OF compatibility string matches one of
+ * (dynamically) supported devices, which may have been augmented
+ * via the sysfs "new_id" file.  Returns the matching pci_device_id
+ * structure or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv)
+{
+	struct pci_dynid *dynid;
+	const struct pci_device_id *found_id = NULL, *ids;
+
+	if (!compat || !drv)
+		return NULL;
+	/* Look at the dynamic ids first, before the static ones */
+	spin_lock(&drv->dynids.lock);
+	list_for_each_entry(dynid, &drv->dynids.list, node) {
+		if (of_pci_compat_match_one_id(compat, &dynid->id)) {
+			found_id = &dynid->id;
+			break;
+		}
+	}
+	spin_unlock(&drv->dynids.lock);
+
+	if (found_id)
+		return found_id;
+
+	for (ids = drv->id_table; (found_id = of_pci_compat_match_one_id(compat, ids));
+	     ids = found_id + 1) {
+		/* exclude ids in id_table with override_only */
+		if (!found_id->override_only)
+			return found_id;
+	}
+
+	return NULL;
+}
+
+/**
+ * of_pci_node_match_driver() - Tell whether an OF device tree node
+ * matches the given pci driver
+ * @node: single OF device tree node to match, could be NULL
+ * @drv: the PCI driver structure to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv)
+{
+	struct property *prop;
+	const char *cp;
+	const struct pci_device_id *id;
+
+	if (!node || !drv)
+		return NULL;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		id = of_pci_compat_match_driver(cp, drv);
+		if (id)
+			return id;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_driver);
+
 /**
  * devm_of_pci_get_host_bridge_resources() - Resource-managed parsing of PCI
  *                                           host bridge resources from DT
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index a2ceeacc33eb..aa212d12353f 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -24,11 +24,6 @@
 #include "pci.h"
 #include "pcie/portdrv.h"
 
-struct pci_dynid {
-	struct list_head node;
-	struct pci_device_id id;
-};
-
 /**
  * pci_add_dynid - add a new PCI device ID to this driver and re-probe devices
  * @drv: target pci driver
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9ed3b5550043..e30652021a63 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -204,6 +204,29 @@ pci_match_one_device(const struct pci_device_id *id, const struct pci_dev *dev)
 	return NULL;
 }
 
+/**
+ * pci_match_one_id - Tell if a PCI device id structure matches another
+ *			  PCI device id structure
+ * @id: single PCI device id structure to match, usually in a list or array
+ * @pr: the probing PCI device id structure to match against, usually converted from
+ *      other format
+ *
+ * Returns the matching pci_device_id structure pointed by id
+ * or %NULL if there is no match.
+ */
+static inline const struct pci_device_id *
+pci_match_one_id(const struct pci_device_id *id, const struct pci_device_id *pr)
+{
+	if ((id->vendor == pr->vendor) &&
+	    (id->device == pr->device) &&
+	    (id->subvendor == pr->subvendor) &&
+	    (id->subdevice == pr->subdevice) &&
+	    (id->class == pr->class) &&
+	    (id->class_mask == pr->class_mask))
+		return id;
+	return NULL;
+}
+
 /* PCI slot sysfs helper code */
 #define to_pci_slot(s) container_of(s, struct pci_slot, kobj)
 
@@ -638,6 +661,15 @@ void pci_release_bus_of_node(struct pci_bus *bus);
 
 int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge);
 
+int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       __u32 *rev, __u32 *req_pcie);
+bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev);
+const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id);
+const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table);
+const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv);
 #else
 static inline int
 of_pci_parse_bus_range(struct device_node *node, struct resource *res)
@@ -679,6 +711,30 @@ static inline int devm_of_pci_bridge_init(struct device *dev, struct pci_host_br
 	return 0;
 }
 
+static inline int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       __u32 *rev, __u32 *req_pcie)
+{
+	return -EINVAL;
+}
+static inline bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev)
+{
+	return false;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv)
+{
+	return NULL;
+}
 #endif /* CONFIG_OF */
 
 #ifdef CONFIG_PCIEAER
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 29658c0ee71f..eef1eaafc03d 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -13,6 +13,14 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn);
 int of_pci_get_devfn(struct device_node *np);
 void of_pci_check_probe_only(void);
+bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev);
+const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table);
+const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv);
 #else
 static inline struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn)
@@ -26,6 +34,23 @@ static inline int of_pci_get_devfn(struct device_node *np)
 }
 
 static inline void of_pci_check_probe_only(void) { }
+static inline bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev)
+{
+	return false;
+}
+static inline const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv)
+{
+	return NULL;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_OF_IRQ)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index adffd65e84b4..04c908d84b90 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1513,6 +1513,12 @@ void pci_unregister_driver(struct pci_driver *dev);
 	builtin_driver(__pci_driver, pci_register_driver)
 
 struct pci_driver *pci_dev_driver(const struct pci_dev *dev);
+
+struct pci_dynid {
+	struct list_head node;
+	struct pci_device_id id;
+};
+
 int pci_add_dynid(struct pci_driver *drv,
 		  unsigned int vendor, unsigned int device,
 		  unsigned int subvendor, unsigned int subdevice,
-- 
2.39.0

[PATCH 2/3] wifi: ath9k: stop loading incompatible DT cal data

[Edward Chow]

Loading calibration data from an OF device tree node not declared
compatible with the device (e.g. a PCI device with calibration data
from corresponding DT node gets replaced, so the newly installed
device become incompatible with the node) or driver may lead to fatal
result, e.g. kernel panic.

The driver should check whether the DT node corresponding to the
device compatible with it, and load calibration data only from
compatible node.

Signed-off-by: Edward Chow <equu@openmail.cc>
---
 drivers/net/wireless/ath/ath9k/ath9k.h |  1 +
 drivers/net/wireless/ath/ath9k/init.c  | 26 ++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/pci.c   |  2 +-
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 2cc23605c9fc..4f6f0383a5f8 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -35,6 +35,7 @@ struct ath_node;
 struct ath_vif;
 
 extern struct ieee80211_ops ath9k_ops;
+extern struct pci_driver ath_pci_driver;
 extern int ath9k_modparam_nohwcrypt;
 extern int ath9k_led_blink;
 extern bool is_ath9k_unloaded;
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 4f00400c7ffb..f88a48e8456b 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -22,6 +22,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_net.h>
+#include <linux/pci.h>
 #include <linux/nvmem-consumer.h>
 #include <linux/relay.h>
 #include <linux/dmi.h>
@@ -577,6 +578,31 @@ static int ath9k_nvmem_request_eeprom(struct ath_softc *sc)
 	size_t len;
 	int err;
 
+	/* devm_nvmem_cell_get() will get a cell first from the OF
+	 * DT node representing the given device with nvmem-cell-name
+	 * "calibration", and from the global lookup table as a fallback,
+	 * and an ath9k device could be either a pci one or a platform one.
+	 *
+	 * If the OF DT node is not compatible with the real device, the
+	 * calibration data got from the node should not be applied.
+	 *
+	 * dev_is_pci(sc->dev) && ( no OF node || caldata not from node
+	 * || not compatible ) -> do not use caldata .
+	 *
+	 * !dev_is_pci(sc->dev) -> always use caldata .
+	 */
+	if (dev_is_pci(sc->dev) &&
+	    (!sc->dev->of_node ||
+	     !of_property_match_string(sc->dev->of_node,
+				       "nvmem-cell-names",
+				       "calibration") ||
+	     !of_pci_node_match_driver(sc->dev->of_node,
+				       &ath_pci_driver)))
+		/* follow the "just return 0;" convention as
+		 * noted below.
+		 */
+		return 0;
+
 	cell = devm_nvmem_cell_get(sc->dev, "calibration");
 	if (IS_ERR(cell)) {
 		err = PTR_ERR(cell);
diff --git a/drivers/net/wireless/ath/ath9k/pci.c b/drivers/net/wireless/ath/ath9k/pci.c
index a074e23013c5..fcb19761e60d 100644
--- a/drivers/net/wireless/ath/ath9k/pci.c
+++ b/drivers/net/wireless/ath/ath9k/pci.c
@@ -1074,7 +1074,7 @@ static SIMPLE_DEV_PM_OPS(ath9k_pm_ops, ath_pci_suspend, ath_pci_resume);
 
 MODULE_DEVICE_TABLE(pci, ath_pci_id_table);
 
-static struct pci_driver ath_pci_driver = {
+struct pci_driver ath_pci_driver = {
 	.name       = "ath9k",
 	.id_table   = ath_pci_id_table,
 	.probe      = ath_pci_probe,
-- 
2.39.0

Re: [PATCH 2/3] wifi: ath9k: stop loading incompatible DT cal data

[Bjorn Helgaas]

On Tue, Jan 17, 2023 at 05:27:46PM +0800, Edward Chow wrote:
> Loading calibration data from an OF device tree node not declared
> compatible with the device (e.g. a PCI device with calibration data
> from corresponding DT node gets replaced, so the newly installed
> device become incompatible with the node) or driver may lead to fatal
> result, e.g. kernel panic.

Please include a link to a bug report and include a few lines of the
oops or panic directly in the commit log so when users see this
problem, they can search for the text and possibly find this fix.

> The driver should check whether the DT node corresponding to the
> device compatible with it, and load calibration data only from
> compatible node.

If you read this commit log carefully, it doesn't actually say what
this patch *does*.  It has some background and this assertion about
what drivers *should* do, but it doesn't say what this patch does.

Suggest structure like this (flesh out with the relevant DT property
names, etc):

  For PCI ath9k devices, load calibration data only if there is a DT
  node corresponding to the device with XXX ...

More details: https://chris.beams.io/posts/git-commit/

> Signed-off-by: Edward Chow <equu@openmail.cc>
> ---
>  drivers/net/wireless/ath/ath9k/ath9k.h |  1 +
>  drivers/net/wireless/ath/ath9k/init.c  | 26 ++++++++++++++++++++++++++
>  drivers/net/wireless/ath/ath9k/pci.c   |  2 +-
>  3 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> index 2cc23605c9fc..4f6f0383a5f8 100644
> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
> @@ -35,6 +35,7 @@ struct ath_node;
>  struct ath_vif;
>  
>  extern struct ieee80211_ops ath9k_ops;
> +extern struct pci_driver ath_pci_driver;
>  extern int ath9k_modparam_nohwcrypt;
>  extern int ath9k_led_blink;
>  extern bool is_ath9k_unloaded;
> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
> index 4f00400c7ffb..f88a48e8456b 100644
> --- a/drivers/net/wireless/ath/ath9k/init.c
> +++ b/drivers/net/wireless/ath/ath9k/init.c
> @@ -22,6 +22,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_net.h>
> +#include <linux/pci.h>
>  #include <linux/nvmem-consumer.h>
>  #include <linux/relay.h>
>  #include <linux/dmi.h>
> @@ -577,6 +578,31 @@ static int ath9k_nvmem_request_eeprom(struct ath_softc *sc)
>  	size_t len;
>  	int err;
>  
> +	/* devm_nvmem_cell_get() will get a cell first from the OF
> +	 * DT node representing the given device with nvmem-cell-name
> +	 * "calibration", and from the global lookup table as a fallback,
> +	 * and an ath9k device could be either a pci one or a platform one.
> +	 *
> +	 * If the OF DT node is not compatible with the real device, the
> +	 * calibration data got from the node should not be applied.
> +	 *
> +	 * dev_is_pci(sc->dev) && ( no OF node || caldata not from node
> +	 * || not compatible ) -> do not use caldata .
> +	 *
> +	 * !dev_is_pci(sc->dev) -> always use caldata .
> +	 */
> +	if (dev_is_pci(sc->dev) &&
> +	    (!sc->dev->of_node ||
> +	     !of_property_match_string(sc->dev->of_node,
> +				       "nvmem-cell-names",
> +				       "calibration") ||
> +	     !of_pci_node_match_driver(sc->dev->of_node,
> +				       &ath_pci_driver)))
> +		/* follow the "just return 0;" convention as
> +		 * noted below.
> +		 */
> +		return 0;
> +
>  	cell = devm_nvmem_cell_get(sc->dev, "calibration");
>  	if (IS_ERR(cell)) {
>  		err = PTR_ERR(cell);
> diff --git a/drivers/net/wireless/ath/ath9k/pci.c b/drivers/net/wireless/ath/ath9k/pci.c
> index a074e23013c5..fcb19761e60d 100644
> --- a/drivers/net/wireless/ath/ath9k/pci.c
> +++ b/drivers/net/wireless/ath/ath9k/pci.c
> @@ -1074,7 +1074,7 @@ static SIMPLE_DEV_PM_OPS(ath9k_pm_ops, ath_pci_suspend, ath_pci_resume);
>  
>  MODULE_DEVICE_TABLE(pci, ath_pci_id_table);
>  
> -static struct pci_driver ath_pci_driver = {
> +struct pci_driver ath_pci_driver = {
>  	.name       = "ath9k",
>  	.id_table   = ath_pci_id_table,
>  	.probe      = ath_pci_probe,
> -- 
> 2.39.0
> 

Re: [PATCH 2/3] wifi: ath9k: stop loading incompatible DT cal data

[Mad Horse]

在 2023/1/18 03:46, Bjorn Helgaas 写道:
> On Tue, Jan 17, 2023 at 05:27:46PM +0800, Edward Chow wrote:
>> Loading calibration data from an OF device tree node not declared
>> compatible with the device (e.g. a PCI device with calibration data
>> from corresponding DT node gets replaced, so the newly installed
>> device become incompatible with the node) or driver may lead to fatal
>> result, e.g. kernel panic.
> Please include a link to a bug report and include a few lines of the
> oops or panic directly in the commit log so when users see this
> problem, they can search for the text and possibly find this fix.
>
>> The driver should check whether the DT node corresponding to the
>> device compatible with it, and load calibration data only from
>> compatible node.
> If you read this commit log carefully, it doesn't actually say what
> this patch *does*.  It has some background and this assertion about
> what drivers *should* do, but it doesn't say what this patch does.
>
> Suggest structure like this (flesh out with the relevant DT property
> names, etc):
>
>   For PCI ath9k devices, load calibration data only if there is a DT
>   node corresponding to the device with XXX ...
>
> More details: https://chris.beams.io/posts/git-commit/
>
Thanks for pointing it out. The commit log of the last two patch has been reworded with required content, and sent to the current thread earlier.

Re: [PATCH 2/3] wifi: ath9k: stop loading incompatible DT cal data

[Bjorn Helgaas]

On Wed, Feb 01, 2023 at 11:02:59AM +0800, Mad Horse wrote:
> 在 2023/1/18 03:46, Bjorn Helgaas 写道:

> > Suggest structure like this (flesh out with the relevant DT property
> > names, etc):
> >
> >   For PCI ath9k devices, load calibration data only if there is a DT
> >   node corresponding to the device with XXX ...
> >
> > More details: https://chris.beams.io/posts/git-commit/
> >
> Thanks for pointing it out. The commit log of the last two patch has
> been reworded with required content, and sent to the current thread
> earlier.

Thanks.  Sending updated version as replies to the original series
posting makes it hard to keep track of what's happening.  Better if
you send a "v2" series of *all* the patches, even if some of them
haven't changed.  There are a few hints here, although it's not very
explicit:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=v6.1#n633

Also, you use multiple email addresses, and if you can figure out how
to at least make the display names the same, it will make the
conversation easier to follow.  Here's what it looks like in mutt:

  - Jan  7  9:14 Mad Horse       [PATCH] pci: Add functions ...
  - Jan 10  9:24 To Mad Horse    ├─>
  - Jan 17  4:01 Mad Horse       ├─>[PATCH 1/3] PCI: of: Match ...
  - Jan 17  4:02 Mad Horse       ├─>[PATCH 2/3] wifi: ath9k: stop ...
  - Jan 17  4:02 Mad Horse       ├─>[PATCH 3/3] wifi: ath10k: only ...
  - Jan 17  4:29 Mad Horse       ├─>[PATCH 1/3] PCI: of: Match pci ...
  - Jan 17  3:27 Edward Chow     ├─>[PATCH 1/3] PCI: of: Match pci ...
  - Jan 17  3:28 Edward Chow     ├─>[PATCH 3/3] wifi: ath10k: only ...
  - Jan 21  4:00 persmule@gmail. ├─>[PATCH 2/3] wifi: ath9k: stop ...
  - Jan 21  4:06 persmule@gmail. ├─>[PATCH 3/3] wifi: ath10k: only ...
  - Jan 17  3:27 Edward Chow     └─>[PATCH 2/3] wifi: ath9k: stop ...
  - Jan 17 13:46 To Edward Chow    └─>
  - Jan 31 21:02 Mad Horse           └─>

This also shows the problem with updated versions in the same thread.
I can work out that "Jan 17 1/3", "Jan 21 2/3", and "Jan 21 3/3" are
supposed to be the newest versions, but tools like patchwork and b4
are not smart enough to do that, so it's really hard to apply this.

If you just post an updated series, e.g.,

  [PATCH v2 0/3] PCI: ...
  |-> [PATCH v2 1/3] PCI: of: Match ...
  |-> [PATCH v2 1/3] PCI: of: Match ...
  |-> [PATCH v2 1/3] PCI: of: Match ...

then everything will be nice and clean and easy to review and apply.

Bjorn

[PATCH v2 0/3] PCI: of: Load extra data only from compatible DT nodes

[equu]

From: Edward Chow <equu@openmail.cc>

In order to solve the issue reported in
https://github.com/openwrt/openwrt/pull/11345 , this patchset attempt
to add mechanisms to ckeck whether an OF DT node is compatible to the
PCI device installed on the corresponding location or the driver for
it, and make ath9k and ath10k only load extra data from compatible
nodes.

V1 -> V2:
1, Update commit messages and comments, minor improvement on code

2, Update commit messages and comments.

3, Update commit messages and comments.

Signed-off-by: Edward Chow <equu@openmail.cc>

Edward Chow (3):
  PCI: of: Match pci devices or drivers against OF DT nodes
  wifi: ath9k: stop loading incompatible DT cal data
  wifi: ath10k: only load compatible DT cal data

 drivers/net/wireless/ath/ath10k/core.c |  28 +++
 drivers/net/wireless/ath/ath10k/pci.c  |   2 +-
 drivers/net/wireless/ath/ath10k/pci.h  |   2 +
 drivers/net/wireless/ath/ath9k/ath9k.h |   1 +
 drivers/net/wireless/ath/ath9k/init.c  |  26 +++
 drivers/net/wireless/ath/ath9k/pci.c   |   2 +-
 drivers/pci/of.c                       | 299 +++++++++++++++++++++++++
 drivers/pci/pci-driver.c               |   5 -
 drivers/pci/pci.h                      |  56 +++++
 include/linux/of_pci.h                 |  25 +++
 include/linux/pci.h                    |   6 +
 11 files changed, 445 insertions(+), 7 deletions(-)

-- 
2.39.0

[PATCH v2 1/3] PCI: of: Match pci devices or drivers against OF DT nodes

[equu]

From: Edward Chow <equu@openmail.cc>

Currently, whether a compatibility string within an OF DT node for a
PCI device (whose spec is at
https://www.devicetree.org/open-firmware/bindings/pci/ ) matches the
vendor and device id of either the PCI device installed on the
corresponding location or the driver suggested by the compatibility
string is not supported.

This patch introduces a function to decode a compatibility string into
a struct pci_device_id, which could further be matched against PCI
devices or drivers, as well as functions to match a compatibility
string or OF DT node against PCI devices or drivers.

Signed-off-by: Edward Chow <equu@openmail.cc>
Cc: Bjorn Helgaas <helgaas@kernel.org>
---
 drivers/pci/of.c         | 299 +++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci-driver.c |   5 -
 drivers/pci/pci.h        |  56 ++++++++
 include/linux/of_pci.h   |  25 ++++
 include/linux/pci.h      |   6 +
 5 files changed, 386 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 196834ed44fe..edb61195ea53 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -13,6 +13,8 @@
 #include <linux/of_irq.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
+#include <linux/string.h>
+#include <linux/kstrtox.h>
 #include "pci.h"
 
 #ifdef CONFIG_PCI
@@ -251,6 +253,303 @@ void of_pci_check_probe_only(void)
 }
 EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
 
+/**
+ * of_pci_compat_to_device_id() - Decode an OF compatibility string into a
+ * pci_device_id structure.
+ * @compat: the compatibility string to decode, could be NULL
+ * @id: pointer to a struct pci_device_id, to store the result
+ * @rev: pointer to output revision info, PCI_ANY_ID if no revision in @compat
+ * @req_pcie: pointer to output whether @compat mandates PCIe compatibility
+ *
+ * returns 0 when success, -EINVAL when failed.
+ */
+int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       u32 *rev, u32 *req_pcie)
+{
+	union {
+		u8 u8;
+		u16 u16;
+		u32 u32;
+	} res = {0};
+	*req_pcie = 0;
+	*rev = PCI_ANY_ID;
+	if (!compat || strncasecmp(compat, "pci", 3) != 0)
+		return -EINVAL;
+	compat += 3;
+
+	if (strncasecmp(compat, "class,", 6) == 0) {
+		/* pciclass,CCSSPP */
+		compat += 6;
+		if ((strlen(compat) < 4)
+		   || kstrtouint(compat, 16, &id->class))
+			return -EINVAL;
+		if (id->class < 0x10000) {
+			id->class <<= 8;
+			id->class_mask = 0xFFFF00;
+		} else {
+			id->class_mask = PCI_ANY_ID;
+		}
+		id->vendor = PCI_ANY_ID;
+		id->device = PCI_ANY_ID;
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0
+	}
+
+	if (strncasecmp(compat, "ex", 2) == 0) {
+		/* pciex...  */
+		*req_pcie = 1;
+		compat += 2;
+	}
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->vendor = res.u16;
+	compat = strchr(compat, ',');
+	if (!compat)
+		return -EINVAL;
+	compat++;
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->device = res.u16;
+	compat = strchr(compat, '.');
+	if (compat == NULL) {
+		/* pciVVVV,DDDD */
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0
+	}
+
+	compat++;
+	if (strlen(compat) == 2) {
+		/* pciVVVV,DDDD.RR */
+		if (kstrtou8(compat, 16, &res.u8))
+			return -EINVAL;
+		*rev = res.u8;
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0
+	}
+
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->subvendor = res.u16;
+	compat = strchr(compat, '.');
+	if (!compat)
+		return -EINVAL;
+	compat++;
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->subdevice = res.u16;
+	compat = strchr(compat, '.');
+	if (compat == NULL)
+		/* pciVVVV,DDDD.SSSS.ssss */
+		return 0
+
+	compat++;
+	if (strlen(compat) == 2) {
+		/* pciVVVV,DDDD.SSSS.ssss.RR */
+		if (kstrtou8(compat, 16, &res.u8))
+			return -EINVAL;
+		*rev = res.u8;
+	}
+	return 0;
+}
+
+/**
+ * of_pci_compat_match_device() - Tell whether a PCI device structure matches
+ * a given OF compatibility string
+ * @compat: single OF compatibility string to match, could be NULL
+ * @dev the PCI device structure to match against
+ *
+ * Returns whether they match.
+ */
+bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev)
+{
+	__u32 rev = PCI_ANY_ID;
+	__u32 req_pcie = 0;
+	struct pci_device_id id = {0};
+
+	if (of_pci_compat_to_device_id(compat, &id, &rev, &req_pcie))
+		return false;
+	return pci_match_one_device(&id, dev) &&
+		(rev == PCI_ANY_ID || rev == dev->revision) &&
+		req_pcie ? dev->pcie_cap : true;
+}
+
+/**
+ * of_pci_node_match_device() - Tell whether an OF device tree node
+ * matches the given pci device
+ * @node: single OF device tree node to match, could be NULL
+ * @dev: the PCI device structure to match against, could be NULL
+ *
+ * Returns whether they match.
+ */
+bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev)
+{
+	struct property *prop;
+	const char *cp;
+
+	if (!node || !dev)
+		return false;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		if (of_pci_compat_match_device(cp, dev))
+			return true;
+	}
+	return false;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_device);
+
+/**
+ * of_pci_compat_match_one_id() - Tell whether a PCI device ID structure matches
+ * a given OF compatibility string, note that there is no revision nor PCIe
+ * capability info in PCI device ID structures
+ *
+ * @compat: single OF compatibility string to match, could be NULL
+ * @id the PCI device ID structure to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure pointed by ID
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id)
+{
+	__u32 rev = PCI_ANY_ID;
+	__u32 req_pcie = 0;
+	struct pci_device_id pr = {0};
+
+	if (!compat || !id ||
+	    of_pci_compat_to_device_id(compat, &pr, &rev, &req_pcie))
+		return NULL;
+	return pci_match_one_id(id, &pr);
+}
+
+/**
+ * of_pci_compat_match_id_table() - Tell whether a given OF compatibility string
+ * matches a given pci_id table
+ *
+ * @compat: single OF compatibility string to match, could be NULL
+ * @table the PCI device ID table to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table)
+{
+	if (compat && table) {
+		while (table->vendor || table->subvendor || table->class_mask) {
+			if (of_pci_compat_match_one_id(compat, table))
+				return table;
+			table++;
+		}
+	}
+	return NULL;
+}
+
+/**
+ * of_pci_node_match_id_table() - Tell whether an OF device tree node
+ * matches the given pci_id table
+ * @node: single OF device tree node to match, could be NULL
+ * @table: the PCI device ID table to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table)
+{
+	struct property *prop;
+	const char *cp;
+	const struct pci_device_id *id;
+
+	if (!node || !table)
+		return NULL;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		id = of_pci_compat_match_id_table(cp, table);
+		if (id)
+			return id;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_id_table);
+
+/**
+ * of_pci_compat_match_driver - See if a given OF compatibility string matches
+ * a driver's list of IDs
+ * @compat: single OF compatibility string to match, could be NULL
+ * @drv: the PCI driver to match against, could be NULL
+ *
+ * Used by a driver to check whether an OF compatibility string matches one of
+ * (dynamically) supported devices, which may have been augmented
+ * via the sysfs "new_id" file.  Returns the matching pci_device_id
+ * structure or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv)
+{
+	struct pci_dynid *dynid;
+	const struct pci_device_id *found_id = NULL, *ids;
+
+	if (!compat || !drv)
+		return NULL;
+	/* Look at the dynamic ids first, before the static ones */
+	spin_lock(&drv->dynids.lock);
+	list_for_each_entry(dynid, &drv->dynids.list, node) {
+		if (of_pci_compat_match_one_id(compat, &dynid->id)) {
+			found_id = &dynid->id;
+			break;
+		}
+	}
+	spin_unlock(&drv->dynids.lock);
+
+	if (found_id)
+		return found_id;
+
+	for (ids = drv->id_table; (found_id = of_pci_compat_match_one_id(compat, ids));
+	     ids = found_id + 1) {
+		/* exclude ids in id_table with override_only */
+		if (!found_id->override_only)
+			return found_id;
+	}
+
+	return NULL;
+}
+
+/**
+ * of_pci_node_match_driver() - Tell whether an OF device tree node
+ * matches the given pci driver
+ * @node: single OF device tree node to match, could be NULL
+ * @drv: the PCI driver structure to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv)
+{
+	struct property *prop;
+	const char *cp;
+	const struct pci_device_id *id;
+
+	if (!node || !drv)
+		return NULL;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		id = of_pci_compat_match_driver(cp, drv);
+		if (id)
+			return id;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_driver);
+
 /**
  * devm_of_pci_get_host_bridge_resources() - Resource-managed parsing of PCI
  *                                           host bridge resources from DT
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index a2ceeacc33eb..aa212d12353f 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -24,11 +24,6 @@
 #include "pci.h"
 #include "pcie/portdrv.h"
 
-struct pci_dynid {
-	struct list_head node;
-	struct pci_device_id id;
-};
-
 /**
  * pci_add_dynid - add a new PCI device ID to this driver and re-probe devices
  * @drv: target pci driver
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9ed3b5550043..e30652021a63 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -204,6 +204,29 @@ pci_match_one_device(const struct pci_device_id *id, const struct pci_dev *dev)
 	return NULL;
 }
 
+/**
+ * pci_match_one_id - Tell if a PCI device id structure matches another
+ *			  PCI device id structure
+ * @id: single PCI device id structure to match, usually in a list or array
+ * @pr: the probing PCI device id structure to match against, usually converted from
+ *      other format
+ *
+ * Returns the matching pci_device_id structure pointed by id
+ * or %NULL if there is no match.
+ */
+static inline const struct pci_device_id *
+pci_match_one_id(const struct pci_device_id *id, const struct pci_device_id *pr)
+{
+	if ((id->vendor == pr->vendor) &&
+	    (id->device == pr->device) &&
+	    (id->subvendor == pr->subvendor) &&
+	    (id->subdevice == pr->subdevice) &&
+	    (id->class == pr->class) &&
+	    (id->class_mask == pr->class_mask))
+		return id;
+	return NULL;
+}
+
 /* PCI slot sysfs helper code */
 #define to_pci_slot(s) container_of(s, struct pci_slot, kobj)
 
@@ -638,6 +661,15 @@ void pci_release_bus_of_node(struct pci_bus *bus);
 
 int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge);
 
+int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       __u32 *rev, __u32 *req_pcie);
+bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev);
+const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id);
+const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table);
+const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv);
 #else
 static inline int
 of_pci_parse_bus_range(struct device_node *node, struct resource *res)
@@ -679,6 +711,30 @@ static inline int devm_of_pci_bridge_init(struct device *dev, struct pci_host_br
 	return 0;
 }
 
+static inline int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       __u32 *rev, __u32 *req_pcie)
+{
+	return -EINVAL;
+}
+static inline bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev)
+{
+	return false;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv)
+{
+	return NULL;
+}
 #endif /* CONFIG_OF */
 
 #ifdef CONFIG_PCIEAER
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 29658c0ee71f..eef1eaafc03d 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -13,6 +13,14 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn);
 int of_pci_get_devfn(struct device_node *np);
 void of_pci_check_probe_only(void);
+bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev);
+const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table);
+const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv);
 #else
 static inline struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn)
@@ -26,6 +34,23 @@ static inline int of_pci_get_devfn(struct device_node *np)
 }
 
 static inline void of_pci_check_probe_only(void) { }
+static inline bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev)
+{
+	return false;
+}
+static inline const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv)
+{
+	return NULL;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_OF_IRQ)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index adffd65e84b4..04c908d84b90 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1513,6 +1513,12 @@ void pci_unregister_driver(struct pci_driver *dev);
 	builtin_driver(__pci_driver, pci_register_driver)
 
 struct pci_driver *pci_dev_driver(const struct pci_dev *dev);
+
+struct pci_dynid {
+	struct list_head node;
+	struct pci_device_id id;
+};
+
 int pci_add_dynid(struct pci_driver *drv,
 		  unsigned int vendor, unsigned int device,
 		  unsigned int subvendor, unsigned int subdevice,
-- 
2.39.0

[PATCH v2 2/3] wifi: ath9k: stop loading incompatible DT cal data

[equu]

From: Edward Chow <equu@openmail.cc>

As reported in https://github.com/openwrt/openwrt/pull/11345 , ath9k
would load calibration data from a device tree node declared
incompatible.

Now, ath9k will first check whether the device tree node is compatible
with it, using the functionality introduced with the first patch of
this series, ("PCI: of: Match pci devices or drivers against OF DT
nodes") and only proceed loading calibration data from compatible node.

Signed-off-by: Edward Chow <equu@openmail.cc>
---
 drivers/net/wireless/ath/ath9k/ath9k.h |  1 +
 drivers/net/wireless/ath/ath9k/init.c  | 26 ++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/pci.c   |  2 +-
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 2cc23605c9fc..4f6f0383a5f8 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -35,6 +35,7 @@ struct ath_node;
 struct ath_vif;
 
 extern struct ieee80211_ops ath9k_ops;
+extern struct pci_driver ath_pci_driver;
 extern int ath9k_modparam_nohwcrypt;
 extern int ath9k_led_blink;
 extern bool is_ath9k_unloaded;
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 4f00400c7ffb..f88a48e8456b 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -22,6 +22,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_net.h>
+#include <linux/pci.h>
 #include <linux/nvmem-consumer.h>
 #include <linux/relay.h>
 #include <linux/dmi.h>
@@ -577,6 +578,31 @@ static int ath9k_nvmem_request_eeprom(struct ath_softc *sc)
 	size_t len;
 	int err;
 
+	/* devm_nvmem_cell_get() will get a cell first from the OF
+	 * DT node representing the given device with nvmem-cell-name
+	 * "calibration", and from the global lookup table as a fallback,
+	 * and an ath9k device could be either a pci one or a platform one.
+	 *
+	 * If the OF DT node is not compatible with the real device, the
+	 * calibration data got from the node should not be applied.
+	 *
+	 * dev_is_pci(sc->dev) && ( no OF node || caldata not from node
+	 * || not compatible ) -> do not use caldata .
+	 *
+	 * !dev_is_pci(sc->dev) -> always use caldata .
+	 */
+	if (dev_is_pci(sc->dev) &&
+	    (!sc->dev->of_node ||
+	     !of_property_match_string(sc->dev->of_node,
+				       "nvmem-cell-names",
+				       "calibration") ||
+	     !of_pci_node_match_driver(sc->dev->of_node,
+				       &ath_pci_driver)))
+		/* follow the "just return 0;" convention as
+		 * noted below.
+		 */
+		return 0;
+
 	cell = devm_nvmem_cell_get(sc->dev, "calibration");
 	if (IS_ERR(cell)) {
 		err = PTR_ERR(cell);
diff --git a/drivers/net/wireless/ath/ath9k/pci.c b/drivers/net/wireless/ath/ath9k/pci.c
index a074e23013c5..fcb19761e60d 100644
--- a/drivers/net/wireless/ath/ath9k/pci.c
+++ b/drivers/net/wireless/ath/ath9k/pci.c
@@ -1074,7 +1074,7 @@ static SIMPLE_DEV_PM_OPS(ath9k_pm_ops, ath_pci_suspend, ath_pci_resume);
 
 MODULE_DEVICE_TABLE(pci, ath_pci_id_table);
 
-static struct pci_driver ath_pci_driver = {
+struct pci_driver ath_pci_driver = {
 	.name       = "ath9k",
 	.id_table   = ath_pci_id_table,
 	.probe      = ath_pci_probe,
-- 
2.39.0

[PATCH v2 3/3] wifi: ath10k: only load compatible DT cal data

[equu]

From: Edward Chow <equu@openmail.cc>

ath10k might also be sensitive to the issue reported on
https://github.com/openwrt/openwrt/pull/11345 , loading calibration
data from a device tree node declared incompatible.

ath10k will first check whether the device tree node is compatible
with it, using the functionality introduced with the first patch of
this series, ("PCI: of: Match pci devices or drivers against OF DT
nodes") and only proceed loading calibration data from compatible node.

Signed-off-by: Edward Chow <equu@openmail.cc>
---
 drivers/net/wireless/ath/ath10k/core.c | 28 ++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/pci.c  |  2 +-
 drivers/net/wireless/ath/ath10k/pci.h  |  2 ++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 5eb131ab916f..e4d7ec7f3b59 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -26,6 +26,7 @@
 #include "testmode.h"
 #include "wmi-ops.h"
 #include "coredump.h"
+#include "pci.h"
 
 unsigned int ath10k_debug_mask;
 EXPORT_SYMBOL(ath10k_debug_mask);
@@ -1958,6 +1959,33 @@ static int ath10k_download_cal_nvmem(struct ath10k *ar, const char *cell_name)
 	size_t len;
 	int ret;
 
+	/* devm_nvmem_cell_get() will get a cell first from the OF
+	 * DT node representing the given device with nvmem-cell-name
+	 * "calibration", and from the global lookup table as a fallback,
+	 * and an ath9k device could be either a pci one or a platform one.
+	 *
+	 * If the OF DT node is not compatible with the real device, the
+	 * calibration data got from the node should not be applied.
+	 *
+	 * dev_is_pci(ar->dev) && ( no OF node || caldata not from node
+	 * || not compatible ) -> do not use caldata .
+	 *
+	 * !dev_is_pci(ar->dev) -> always use caldata .
+	 *
+	 * The judgement for compatibility differs with ath9k for many
+	 * DT using "qcom,ath10k" as compatibility string.
+	 */
+	if (dev_is_pci(ar->dev) &&
+	    (!ar->dev->of_node ||
+	     (of_property_match_string(ar->dev->of_node,
+				       "nvmem-cell-names",
+				       cell_name) < 0) ||
+	     !of_device_is_compatible(ar->dev->of_node,
+				      "qcom,ath10k") ||
+	     !of_pci_node_match_device(ar->dev->of_node,
+				       &ath10k_pci_driver)))
+		return ERR_PTR(-ENOENT);
+
 	cell = devm_nvmem_cell_get(ar->dev, cell_name);
 	if (IS_ERR(cell)) {
 		ret = PTR_ERR(cell);
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 728d607289c3..5d9f6046f8cf 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -3780,7 +3780,7 @@ static SIMPLE_DEV_PM_OPS(ath10k_pci_pm_ops,
 			 ath10k_pci_pm_suspend,
 			 ath10k_pci_pm_resume);
 
-static struct pci_driver ath10k_pci_driver = {
+struct pci_driver ath10k_pci_driver = {
 	.name = "ath10k_pci",
 	.id_table = ath10k_pci_id_table,
 	.probe = ath10k_pci_probe,
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 480cd97ab739..de676797b736 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -209,6 +209,8 @@ static inline struct ath10k_pci *ath10k_pci_priv(struct ath10k *ar)
 #define DIAG_ACCESS_CE_TIMEOUT_US 10000 /* 10 ms */
 #define DIAG_ACCESS_CE_WAIT_US	50
 
+extern struct pci_driver ath10k_pci_driver;
+
 void ath10k_pci_write32(struct ath10k *ar, u32 offset, u32 value);
 void ath10k_pci_soc_write32(struct ath10k *ar, u32 addr, u32 val);
 void ath10k_pci_reg_write32(struct ath10k *ar, u32 addr, u32 val);
-- 
2.39.0

Re: [PATCH v2 3/3] wifi: ath10k: only load compatible DT cal data

[kernel test robot]

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on helgaas-pci/for-linus wireless-next/main wireless/main linus/master v6.2-rc6 next-20230202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/equu-openmail-cc/wifi-ath9k-stop-loading-incompatible-DT-cal-data/20230202-131914
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
patch link:    https://lore.kernel.org/r/20230202041823.2879262-4-equu%40openmail.cc
patch subject: [PATCH v2 3/3] wifi: ath10k: only load compatible DT cal data
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230202/202302021511.hIXEXftH-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/d3f328c8809b6349090ad2a2ee78250d657edb58
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review equu-openmail-cc/wifi-ath9k-stop-loading-incompatible-DT-cal-data/20230202-131914
        git checkout d3f328c8809b6349090ad2a2ee78250d657edb58
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/net/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/net/wireless/ath/ath10k/core.c: In function 'ath10k_download_cal_nvmem':
   drivers/net/wireless/ath/ath10k/core.c:1985:15: error: implicit declaration of function 'of_pci_node_match_device' [-Werror=implicit-function-declaration]
    1985 |              !of_pci_node_match_device(ar->dev->of_node,
         |               ^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/wireless/ath/ath10k/core.c:1987:24: warning: returning 'void *' from a function with return type 'int' makes integer from pointer without a cast [-Wint-conversion]
    1987 |                 return ERR_PTR(-ENOENT);
         |                        ^~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +1987 drivers/net/wireless/ath/ath10k/core.c

  1954	
  1955	static int ath10k_download_cal_nvmem(struct ath10k *ar, const char *cell_name)
  1956	{
  1957		struct nvmem_cell *cell;
  1958		void *buf;
  1959		size_t len;
  1960		int ret;
  1961	
  1962		/* devm_nvmem_cell_get() will get a cell first from the OF
  1963		 * DT node representing the given device with nvmem-cell-name
  1964		 * "calibration", and from the global lookup table as a fallback,
  1965		 * and an ath9k device could be either a pci one or a platform one.
  1966		 *
  1967		 * If the OF DT node is not compatible with the real device, the
  1968		 * calibration data got from the node should not be applied.
  1969		 *
  1970		 * dev_is_pci(ar->dev) && ( no OF node || caldata not from node
  1971		 * || not compatible ) -> do not use caldata .
  1972		 *
  1973		 * !dev_is_pci(ar->dev) -> always use caldata .
  1974		 *
  1975		 * The judgement for compatibility differs with ath9k for many
  1976		 * DT using "qcom,ath10k" as compatibility string.
  1977		 */
  1978		if (dev_is_pci(ar->dev) &&
  1979		    (!ar->dev->of_node ||
  1980		     (of_property_match_string(ar->dev->of_node,
  1981					       "nvmem-cell-names",
  1982					       cell_name) < 0) ||
  1983		     !of_device_is_compatible(ar->dev->of_node,
  1984					      "qcom,ath10k") ||
  1985		     !of_pci_node_match_device(ar->dev->of_node,
  1986					       &ath10k_pci_driver)))
> 1987			return ERR_PTR(-ENOENT);
  1988	
  1989		cell = devm_nvmem_cell_get(ar->dev, cell_name);
  1990		if (IS_ERR(cell)) {
  1991			ret = PTR_ERR(cell);
  1992			return ret;
  1993		}
  1994	
  1995		buf = nvmem_cell_read(cell, &len);
  1996		if (IS_ERR(buf))
  1997			return PTR_ERR(buf);
  1998	
  1999		if (ar->hw_params.cal_data_len != len) {
  2000			kfree(buf);
  2001			ath10k_warn(ar, "invalid calibration data length in nvmem-cell '%s': %zu != %u\n",
  2002				    cell_name, len, ar->hw_params.cal_data_len);
  2003			return -EMSGSIZE;
  2004		}
  2005	
  2006		ret = ath10k_download_board_data(ar, buf, len);
  2007		kfree(buf);
  2008		if (ret)
  2009			ath10k_warn(ar, "failed to download calibration data from nvmem-cell '%s': %d\n",
  2010				    cell_name, ret);
  2011	
  2012		return ret;
  2013	}
  2014	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

[PATCH v3 0/3] PCI: of: Load extra data only from compatible DT nodes

[equu]

From: Edward Chow <equu@openmail.cc>

In order to solve the issue reported in
https://github.com/openwrt/openwrt/pull/11345 , this patchset attempt
to add mechanisms to ckeck whether an OF DT node is compatible to the
PCI device installed on the corresponding location or the driver for
it, and make ath9k and ath10k only load extra data from compatible
nodes.

V2 -> V3:
2, include <linux/of_pci.h> instead for necessary functions.

3, include <linux/of_pci.h> instead for necessary functions,
   return error as int variable.

Signed-off-by: Edward Chow <equu@openmail.cc>
Reported-by: kernel test robot <lkp@intel.com>

Edward Chow (3):
  PCI: of: Match pci devices or drivers against OF DT nodes
  wifi: ath9k: stop loading incompatible DT cal data
  wifi: ath10k: only load compatible DT cal data

 drivers/net/wireless/ath/ath10k/core.c |  29 +++
 drivers/net/wireless/ath/ath10k/pci.c  |   2 +-
 drivers/net/wireless/ath/ath10k/pci.h  |   2 +
 drivers/net/wireless/ath/ath9k/ath9k.h |   1 +
 drivers/net/wireless/ath/ath9k/init.c  |  26 +++
 drivers/net/wireless/ath/ath9k/pci.c   |   2 +-
 drivers/pci/of.c                       | 299 +++++++++++++++++++++++++
 drivers/pci/pci-driver.c               |   5 -
 drivers/pci/pci.h                      |  56 +++++
 include/linux/of_pci.h                 |  25 +++
 include/linux/pci.h                    |   6 +
 11 files changed, 446 insertions(+), 7 deletions(-)

-- 
2.39.1

[PATCH v3 1/3] PCI: of: Match pci devices or drivers against OF DT nodes

[equu]

From: Edward Chow <equu@openmail.cc>

Currently, whether a compatibility string within an OF DT node for a
PCI device (whose spec is at
https://www.devicetree.org/open-firmware/bindings/pci/ ) matches the
vendor and device id of either the PCI device installed on the
corresponding location or the driver suggested by the compatibility
string is not supported.

This patch introduces a function to decode a compatibility string into
a struct pci_device_id, which could further be matched against PCI
devices or drivers, as well as functions to match a compatibility
string or OF DT node against PCI devices or drivers.

Signed-off-by: Edward Chow <equu@openmail.cc>
Cc: Bjorn Helgaas <helgaas@kernel.org>
---
 drivers/pci/of.c         | 299 +++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci-driver.c |   5 -
 drivers/pci/pci.h        |  56 ++++++++
 include/linux/of_pci.h   |  25 ++++
 include/linux/pci.h      |   6 +
 5 files changed, 386 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 196834ed44fe..edb61195ea53 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -13,6 +13,8 @@
 #include <linux/of_irq.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
+#include <linux/string.h>
+#include <linux/kstrtox.h>
 #include "pci.h"
 
 #ifdef CONFIG_PCI
@@ -251,6 +253,303 @@ void of_pci_check_probe_only(void)
 }
 EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
 
+/**
+ * of_pci_compat_to_device_id() - Decode an OF compatibility string into a
+ * pci_device_id structure.
+ * @compat: the compatibility string to decode, could be NULL
+ * @id: pointer to a struct pci_device_id, to store the result
+ * @rev: pointer to output revision info, PCI_ANY_ID if no revision in @compat
+ * @req_pcie: pointer to output whether @compat mandates PCIe compatibility
+ *
+ * returns 0 when success, -EINVAL when failed.
+ */
+int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       u32 *rev, u32 *req_pcie)
+{
+	union {
+		u8 u8;
+		u16 u16;
+		u32 u32;
+	} res = {0};
+	*req_pcie = 0;
+	*rev = PCI_ANY_ID;
+	if (!compat || strncasecmp(compat, "pci", 3) != 0)
+		return -EINVAL;
+	compat += 3;
+
+	if (strncasecmp(compat, "class,", 6) == 0) {
+		/* pciclass,CCSSPP */
+		compat += 6;
+		if ((strlen(compat) < 4)
+		   || kstrtouint(compat, 16, &id->class))
+			return -EINVAL;
+		if (id->class < 0x10000) {
+			id->class <<= 8;
+			id->class_mask = 0xFFFF00;
+		} else {
+			id->class_mask = PCI_ANY_ID;
+		}
+		id->vendor = PCI_ANY_ID;
+		id->device = PCI_ANY_ID;
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0
+	}
+
+	if (strncasecmp(compat, "ex", 2) == 0) {
+		/* pciex...  */
+		*req_pcie = 1;
+		compat += 2;
+	}
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->vendor = res.u16;
+	compat = strchr(compat, ',');
+	if (!compat)
+		return -EINVAL;
+	compat++;
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->device = res.u16;
+	compat = strchr(compat, '.');
+	if (compat == NULL) {
+		/* pciVVVV,DDDD */
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0
+	}
+
+	compat++;
+	if (strlen(compat) == 2) {
+		/* pciVVVV,DDDD.RR */
+		if (kstrtou8(compat, 16, &res.u8))
+			return -EINVAL;
+		*rev = res.u8;
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0
+	}
+
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->subvendor = res.u16;
+	compat = strchr(compat, '.');
+	if (!compat)
+		return -EINVAL;
+	compat++;
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->subdevice = res.u16;
+	compat = strchr(compat, '.');
+	if (compat == NULL)
+		/* pciVVVV,DDDD.SSSS.ssss */
+		return 0
+
+	compat++;
+	if (strlen(compat) == 2) {
+		/* pciVVVV,DDDD.SSSS.ssss.RR */
+		if (kstrtou8(compat, 16, &res.u8))
+			return -EINVAL;
+		*rev = res.u8;
+	}
+	return 0;
+}
+
+/**
+ * of_pci_compat_match_device() - Tell whether a PCI device structure matches
+ * a given OF compatibility string
+ * @compat: single OF compatibility string to match, could be NULL
+ * @dev the PCI device structure to match against
+ *
+ * Returns whether they match.
+ */
+bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev)
+{
+	__u32 rev = PCI_ANY_ID;
+	__u32 req_pcie = 0;
+	struct pci_device_id id = {0};
+
+	if (of_pci_compat_to_device_id(compat, &id, &rev, &req_pcie))
+		return false;
+	return pci_match_one_device(&id, dev) &&
+		(rev == PCI_ANY_ID || rev == dev->revision) &&
+		req_pcie ? dev->pcie_cap : true;
+}
+
+/**
+ * of_pci_node_match_device() - Tell whether an OF device tree node
+ * matches the given pci device
+ * @node: single OF device tree node to match, could be NULL
+ * @dev: the PCI device structure to match against, could be NULL
+ *
+ * Returns whether they match.
+ */
+bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev)
+{
+	struct property *prop;
+	const char *cp;
+
+	if (!node || !dev)
+		return false;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		if (of_pci_compat_match_device(cp, dev))
+			return true;
+	}
+	return false;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_device);
+
+/**
+ * of_pci_compat_match_one_id() - Tell whether a PCI device ID structure matches
+ * a given OF compatibility string, note that there is no revision nor PCIe
+ * capability info in PCI device ID structures
+ *
+ * @compat: single OF compatibility string to match, could be NULL
+ * @id the PCI device ID structure to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure pointed by ID
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id)
+{
+	__u32 rev = PCI_ANY_ID;
+	__u32 req_pcie = 0;
+	struct pci_device_id pr = {0};
+
+	if (!compat || !id ||
+	    of_pci_compat_to_device_id(compat, &pr, &rev, &req_pcie))
+		return NULL;
+	return pci_match_one_id(id, &pr);
+}
+
+/**
+ * of_pci_compat_match_id_table() - Tell whether a given OF compatibility string
+ * matches a given pci_id table
+ *
+ * @compat: single OF compatibility string to match, could be NULL
+ * @table the PCI device ID table to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table)
+{
+	if (compat && table) {
+		while (table->vendor || table->subvendor || table->class_mask) {
+			if (of_pci_compat_match_one_id(compat, table))
+				return table;
+			table++;
+		}
+	}
+	return NULL;
+}
+
+/**
+ * of_pci_node_match_id_table() - Tell whether an OF device tree node
+ * matches the given pci_id table
+ * @node: single OF device tree node to match, could be NULL
+ * @table: the PCI device ID table to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table)
+{
+	struct property *prop;
+	const char *cp;
+	const struct pci_device_id *id;
+
+	if (!node || !table)
+		return NULL;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		id = of_pci_compat_match_id_table(cp, table);
+		if (id)
+			return id;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_id_table);
+
+/**
+ * of_pci_compat_match_driver - See if a given OF compatibility string matches
+ * a driver's list of IDs
+ * @compat: single OF compatibility string to match, could be NULL
+ * @drv: the PCI driver to match against, could be NULL
+ *
+ * Used by a driver to check whether an OF compatibility string matches one of
+ * (dynamically) supported devices, which may have been augmented
+ * via the sysfs "new_id" file.  Returns the matching pci_device_id
+ * structure or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv)
+{
+	struct pci_dynid *dynid;
+	const struct pci_device_id *found_id = NULL, *ids;
+
+	if (!compat || !drv)
+		return NULL;
+	/* Look at the dynamic ids first, before the static ones */
+	spin_lock(&drv->dynids.lock);
+	list_for_each_entry(dynid, &drv->dynids.list, node) {
+		if (of_pci_compat_match_one_id(compat, &dynid->id)) {
+			found_id = &dynid->id;
+			break;
+		}
+	}
+	spin_unlock(&drv->dynids.lock);
+
+	if (found_id)
+		return found_id;
+
+	for (ids = drv->id_table; (found_id = of_pci_compat_match_one_id(compat, ids));
+	     ids = found_id + 1) {
+		/* exclude ids in id_table with override_only */
+		if (!found_id->override_only)
+			return found_id;
+	}
+
+	return NULL;
+}
+
+/**
+ * of_pci_node_match_driver() - Tell whether an OF device tree node
+ * matches the given pci driver
+ * @node: single OF device tree node to match, could be NULL
+ * @drv: the PCI driver structure to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv)
+{
+	struct property *prop;
+	const char *cp;
+	const struct pci_device_id *id;
+
+	if (!node || !drv)
+		return NULL;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		id = of_pci_compat_match_driver(cp, drv);
+		if (id)
+			return id;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_driver);
+
 /**
  * devm_of_pci_get_host_bridge_resources() - Resource-managed parsing of PCI
  *                                           host bridge resources from DT
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index a2ceeacc33eb..aa212d12353f 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -24,11 +24,6 @@
 #include "pci.h"
 #include "pcie/portdrv.h"
 
-struct pci_dynid {
-	struct list_head node;
-	struct pci_device_id id;
-};
-
 /**
  * pci_add_dynid - add a new PCI device ID to this driver and re-probe devices
  * @drv: target pci driver
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9ed3b5550043..e30652021a63 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -204,6 +204,29 @@ pci_match_one_device(const struct pci_device_id *id, const struct pci_dev *dev)
 	return NULL;
 }
 
+/**
+ * pci_match_one_id - Tell if a PCI device id structure matches another
+ *			  PCI device id structure
+ * @id: single PCI device id structure to match, usually in a list or array
+ * @pr: the probing PCI device id structure to match against, usually converted from
+ *      other format
+ *
+ * Returns the matching pci_device_id structure pointed by id
+ * or %NULL if there is no match.
+ */
+static inline const struct pci_device_id *
+pci_match_one_id(const struct pci_device_id *id, const struct pci_device_id *pr)
+{
+	if ((id->vendor == pr->vendor) &&
+	    (id->device == pr->device) &&
+	    (id->subvendor == pr->subvendor) &&
+	    (id->subdevice == pr->subdevice) &&
+	    (id->class == pr->class) &&
+	    (id->class_mask == pr->class_mask))
+		return id;
+	return NULL;
+}
+
 /* PCI slot sysfs helper code */
 #define to_pci_slot(s) container_of(s, struct pci_slot, kobj)
 
@@ -638,6 +661,15 @@ void pci_release_bus_of_node(struct pci_bus *bus);
 
 int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge);
 
+int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       __u32 *rev, __u32 *req_pcie);
+bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev);
+const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id);
+const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table);
+const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv);
 #else
 static inline int
 of_pci_parse_bus_range(struct device_node *node, struct resource *res)
@@ -679,6 +711,30 @@ static inline int devm_of_pci_bridge_init(struct device *dev, struct pci_host_br
 	return 0;
 }
 
+static inline int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       __u32 *rev, __u32 *req_pcie)
+{
+	return -EINVAL;
+}
+static inline bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev)
+{
+	return false;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv)
+{
+	return NULL;
+}
 #endif /* CONFIG_OF */
 
 #ifdef CONFIG_PCIEAER
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 29658c0ee71f..eef1eaafc03d 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -13,6 +13,14 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn);
 int of_pci_get_devfn(struct device_node *np);
 void of_pci_check_probe_only(void);
+bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev);
+const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table);
+const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv);
 #else
 static inline struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn)
@@ -26,6 +34,23 @@ static inline int of_pci_get_devfn(struct device_node *np)
 }
 
 static inline void of_pci_check_probe_only(void) { }
+static inline bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev)
+{
+	return false;
+}
+static inline const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv)
+{
+	return NULL;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_OF_IRQ)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index adffd65e84b4..04c908d84b90 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1513,6 +1513,12 @@ void pci_unregister_driver(struct pci_driver *dev);
 	builtin_driver(__pci_driver, pci_register_driver)
 
 struct pci_driver *pci_dev_driver(const struct pci_dev *dev);
+
+struct pci_dynid {
+	struct list_head node;
+	struct pci_device_id id;
+};
+
 int pci_add_dynid(struct pci_driver *drv,
 		  unsigned int vendor, unsigned int device,
 		  unsigned int subvendor, unsigned int subdevice,
-- 
2.39.1

Re: [PATCH v3 1/3] PCI: of: Match pci devices or drivers against OF DT nodes

[kernel test robot]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on helgaas-pci/next]
[also build test ERROR on helgaas-pci/for-linus wireless-next/main wireless/main linus/master v6.2-rc6 next-20230203]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/equu-openmail-cc/wifi-ath9k-stop-loading-incompatible-DT-cal-data/20230202-165536
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
patch link:    https://lore.kernel.org/r/20230202075524.2911058-2-equu%40openmail.cc
patch subject: [PATCH v3 1/3] PCI: of: Match pci devices or drivers against OF DT nodes
config: i386-randconfig-a005 (https://download.01.org/0day-ci/archive/20230203/202302031611.JepssFdR-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/1c1bc10b9cc6f7a71ba1fb7bdc505195a2c3e759
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review equu-openmail-cc/wifi-ath9k-stop-loading-incompatible-DT-cal-data/20230202-165536
        git checkout 1c1bc10b9cc6f7a71ba1fb7bdc505195a2c3e759
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 olddefconfig
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/pci/of.c: In function 'of_pci_compat_to_device_id':
>> drivers/pci/of.c:296:25: error: expected ';' before '}' token
     296 |                 return 0
         |                         ^
         |                         ;
     297 |         }
         |         ~                
   drivers/pci/of.c:319:25: error: expected ';' before '}' token
     319 |                 return 0
         |                         ^
         |                         ;
     320 |         }
         |         ~                
   drivers/pci/of.c:330:25: error: expected ';' before '}' token
     330 |                 return 0
         |                         ^
         |                         ;
     331 |         }
         |         ~                
>> drivers/pci/of.c:346:25: error: expected ';' before 'compat'
     346 |                 return 0
         |                         ^
         |                         ;
     347 | 
     348 |         compat++;
         |         ~~~~~~           


vim +296 drivers/pci/of.c

   255	
   256	/**
   257	 * of_pci_compat_to_device_id() - Decode an OF compatibility string into a
   258	 * pci_device_id structure.
   259	 * @compat: the compatibility string to decode, could be NULL
   260	 * @id: pointer to a struct pci_device_id, to store the result
   261	 * @rev: pointer to output revision info, PCI_ANY_ID if no revision in @compat
   262	 * @req_pcie: pointer to output whether @compat mandates PCIe compatibility
   263	 *
   264	 * returns 0 when success, -EINVAL when failed.
   265	 */
   266	int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
   267				       u32 *rev, u32 *req_pcie)
   268	{
   269		union {
   270			u8 u8;
   271			u16 u16;
   272			u32 u32;
   273		} res = {0};
   274		*req_pcie = 0;
   275		*rev = PCI_ANY_ID;
   276		if (!compat || strncasecmp(compat, "pci", 3) != 0)
   277			return -EINVAL;
   278		compat += 3;
   279	
   280		if (strncasecmp(compat, "class,", 6) == 0) {
   281			/* pciclass,CCSSPP */
   282			compat += 6;
   283			if ((strlen(compat) < 4)
   284			   || kstrtouint(compat, 16, &id->class))
   285				return -EINVAL;
   286			if (id->class < 0x10000) {
   287				id->class <<= 8;
   288				id->class_mask = 0xFFFF00;
   289			} else {
   290				id->class_mask = PCI_ANY_ID;
   291			}
   292			id->vendor = PCI_ANY_ID;
   293			id->device = PCI_ANY_ID;
   294			id->subvendor = PCI_ANY_ID;
   295			id->subdevice = PCI_ANY_ID;
 > 296			return 0
   297		}
   298	
   299		if (strncasecmp(compat, "ex", 2) == 0) {
   300			/* pciex...  */
   301			*req_pcie = 1;
   302			compat += 2;
   303		}
   304		if (kstrtou16(compat, 16, &res.u16))
   305			return -EINVAL;
   306		id->vendor = res.u16;
   307		compat = strchr(compat, ',');
   308		if (!compat)
   309			return -EINVAL;
   310		compat++;
   311		if (kstrtou16(compat, 16, &res.u16))
   312			return -EINVAL;
   313		id->device = res.u16;
   314		compat = strchr(compat, '.');
   315		if (compat == NULL) {
   316			/* pciVVVV,DDDD */
   317			id->subvendor = PCI_ANY_ID;
   318			id->subdevice = PCI_ANY_ID;
   319			return 0
   320		}
   321	
   322		compat++;
   323		if (strlen(compat) == 2) {
   324			/* pciVVVV,DDDD.RR */
   325			if (kstrtou8(compat, 16, &res.u8))
   326				return -EINVAL;
   327			*rev = res.u8;
   328			id->subvendor = PCI_ANY_ID;
   329			id->subdevice = PCI_ANY_ID;
   330			return 0
   331		}
   332	
   333		if (kstrtou16(compat, 16, &res.u16))
   334			return -EINVAL;
   335		id->subvendor = res.u16;
   336		compat = strchr(compat, '.');
   337		if (!compat)
   338			return -EINVAL;
   339		compat++;
   340		if (kstrtou16(compat, 16, &res.u16))
   341			return -EINVAL;
   342		id->subdevice = res.u16;
   343		compat = strchr(compat, '.');
   344		if (compat == NULL)
   345			/* pciVVVV,DDDD.SSSS.ssss */
 > 346			return 0
   347	
   348		compat++;
   349		if (strlen(compat) == 2) {
   350			/* pciVVVV,DDDD.SSSS.ssss.RR */
   351			if (kstrtou8(compat, 16, &res.u8))
   352				return -EINVAL;
   353			*rev = res.u8;
   354		}
   355		return 0;
   356	}
   357	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

[PATCH v3 2/3] wifi: ath9k: stop loading incompatible DT cal data

[equu]

From: Edward Chow <equu@openmail.cc>

As reported in https://github.com/openwrt/openwrt/pull/11345 , ath9k
would load calibration data from a device tree node declared
incompatible.

Now, ath9k will first check whether the device tree node is compatible
with it, using the functionality introduced with the first patch of
this series, ("PCI: of: Match pci devices or drivers against OF DT
nodes") and only proceed loading calibration data from compatible node.

Signed-off-by: Edward Chow <equu@openmail.cc>
---
 drivers/net/wireless/ath/ath9k/ath9k.h |  1 +
 drivers/net/wireless/ath/ath9k/init.c  | 26 ++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/pci.c   |  2 +-
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 2cc23605c9fc..4f6f0383a5f8 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -35,6 +35,7 @@ struct ath_node;
 struct ath_vif;
 
 extern struct ieee80211_ops ath9k_ops;
+extern struct pci_driver ath_pci_driver;
 extern int ath9k_modparam_nohwcrypt;
 extern int ath9k_led_blink;
 extern bool is_ath9k_unloaded;
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 4f00400c7ffb..6c0296b4d366 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -22,6 +22,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_net.h>
+#include <linux/of_pci.h>
 #include <linux/nvmem-consumer.h>
 #include <linux/relay.h>
 #include <linux/dmi.h>
@@ -577,6 +578,31 @@ static int ath9k_nvmem_request_eeprom(struct ath_softc *sc)
 	size_t len;
 	int err;
 
+	/* devm_nvmem_cell_get() will get a cell first from the OF
+	 * DT node representing the given device with nvmem-cell-name
+	 * "calibration", and from the global lookup table as a fallback,
+	 * and an ath9k device could be either a pci one or a platform one.
+	 *
+	 * If the OF DT node is not compatible with the real device, the
+	 * calibration data got from the node should not be applied.
+	 *
+	 * dev_is_pci(sc->dev) && ( no OF node || caldata not from node
+	 * || not compatible ) -> do not use caldata .
+	 *
+	 * !dev_is_pci(sc->dev) -> always use caldata .
+	 */
+	if (dev_is_pci(sc->dev) &&
+	    (!sc->dev->of_node ||
+	     !of_property_match_string(sc->dev->of_node,
+				       "nvmem-cell-names",
+				       "calibration") ||
+	     !of_pci_node_match_driver(sc->dev->of_node,
+				       &ath_pci_driver)))
+		/* follow the "just return 0;" convention as
+		 * noted below.
+		 */
+		return 0;
+
 	cell = devm_nvmem_cell_get(sc->dev, "calibration");
 	if (IS_ERR(cell)) {
 		err = PTR_ERR(cell);
diff --git a/drivers/net/wireless/ath/ath9k/pci.c b/drivers/net/wireless/ath/ath9k/pci.c
index a074e23013c5..fcb19761e60d 100644
--- a/drivers/net/wireless/ath/ath9k/pci.c
+++ b/drivers/net/wireless/ath/ath9k/pci.c
@@ -1074,7 +1074,7 @@ static SIMPLE_DEV_PM_OPS(ath9k_pm_ops, ath_pci_suspend, ath_pci_resume);
 
 MODULE_DEVICE_TABLE(pci, ath_pci_id_table);
 
-static struct pci_driver ath_pci_driver = {
+struct pci_driver ath_pci_driver = {
 	.name       = "ath9k",
 	.id_table   = ath_pci_id_table,
 	.probe      = ath_pci_probe,
-- 
2.39.1

Re: [PATCH v3 2/3] wifi: ath9k: stop loading incompatible DT cal data

[kernel test robot]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on helgaas-pci/next]
[also build test ERROR on helgaas-pci/for-linus wireless-next/main wireless/main linus/master v6.2-rc6 next-20230203]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/equu-openmail-cc/wifi-ath9k-stop-loading-incompatible-DT-cal-data/20230202-165536
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
patch link:    https://lore.kernel.org/r/20230202075524.2911058-3-equu%40openmail.cc
patch subject: [PATCH v3 2/3] wifi: ath9k: stop loading incompatible DT cal data
config: i386-randconfig-a005 (https://download.01.org/0day-ci/archive/20230203/202302031700.zxbfZuRh-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/b8656d7cf0ddf9edc732511d6d959c1b3a8b4ea8
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review equu-openmail-cc/wifi-ath9k-stop-loading-incompatible-DT-cal-data/20230202-165536
        git checkout b8656d7cf0ddf9edc732511d6d959c1b3a8b4ea8
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 olddefconfig
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/net/wireless/ath/ath10k/ drivers/net/wireless/ath/ath9k/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   In file included from drivers/net/wireless/ath/ath9k/init.c:25:
>> include/linux/of_pci.h:23:33: warning: 'struct pci_driver' declared inside parameter list will not be visible outside of this definition or declaration
      23 |                          struct pci_driver *drv);
         |                                 ^~~~~~~~~~
   drivers/net/wireless/ath/ath9k/init.c: In function 'ath9k_nvmem_request_eeprom':
>> drivers/net/wireless/ath/ath9k/init.c:594:13: error: implicit declaration of function 'dev_is_pci' [-Werror=implicit-function-declaration]
     594 |         if (dev_is_pci(sc->dev) &&
         |             ^~~~~~~~~~
>> drivers/net/wireless/ath/ath9k/init.c:600:40: error: passing argument 2 of 'of_pci_node_match_driver' from incompatible pointer type [-Werror=incompatible-pointer-types]
     600 |                                        &ath_pci_driver)))
         |                                        ^~~~~~~~~~~~~~~
         |                                        |
         |                                        struct pci_driver *
   In file included from drivers/net/wireless/ath/ath9k/init.c:25:
   include/linux/of_pci.h:23:45: note: expected 'struct pci_driver *' but argument is of type 'struct pci_driver *'
      23 |                          struct pci_driver *drv);
         |                          ~~~~~~~~~~~~~~~~~~~^~~
   cc1: some warnings being treated as errors


vim +/dev_is_pci +594 drivers/net/wireless/ath/ath9k/init.c

   572	
   573	static int ath9k_nvmem_request_eeprom(struct ath_softc *sc)
   574	{
   575		struct ath_hw *ah = sc->sc_ah;
   576		struct nvmem_cell *cell;
   577		void *buf;
   578		size_t len;
   579		int err;
   580	
   581		/* devm_nvmem_cell_get() will get a cell first from the OF
   582		 * DT node representing the given device with nvmem-cell-name
   583		 * "calibration", and from the global lookup table as a fallback,
   584		 * and an ath9k device could be either a pci one or a platform one.
   585		 *
   586		 * If the OF DT node is not compatible with the real device, the
   587		 * calibration data got from the node should not be applied.
   588		 *
   589		 * dev_is_pci(sc->dev) && ( no OF node || caldata not from node
   590		 * || not compatible ) -> do not use caldata .
   591		 *
   592		 * !dev_is_pci(sc->dev) -> always use caldata .
   593		 */
 > 594		if (dev_is_pci(sc->dev) &&
   595		    (!sc->dev->of_node ||
   596		     !of_property_match_string(sc->dev->of_node,
   597					       "nvmem-cell-names",
   598					       "calibration") ||
   599		     !of_pci_node_match_driver(sc->dev->of_node,
 > 600					       &ath_pci_driver)))
   601			/* follow the "just return 0;" convention as
   602			 * noted below.
   603			 */
   604			return 0;
   605	
   606		cell = devm_nvmem_cell_get(sc->dev, "calibration");
   607		if (IS_ERR(cell)) {
   608			err = PTR_ERR(cell);
   609	
   610			/* nvmem cell might not be defined, or the nvmem
   611			 * subsystem isn't included. In this case, follow
   612			 * the established "just return 0;" convention of
   613			 * ath9k_init_platform to say:
   614			 * "All good. Nothing to see here. Please go on."
   615			 */
   616			if (err == -ENOENT || err == -EOPNOTSUPP)
   617				return 0;
   618	
   619			return err;
   620		}
   621	
   622		buf = nvmem_cell_read(cell, &len);
   623		if (IS_ERR(buf))
   624			return PTR_ERR(buf);
   625	
   626		/* run basic sanity checks on the returned nvram cell length.
   627		 * That length has to be a multiple of a "u16" (i.e.: & 1).
   628		 * Furthermore, it has to be more than "let's say" 512 bytes
   629		 * but less than the maximum of AR9300_EEPROM_SIZE (16kb).
   630		 */
   631		if ((len & 1) == 1 || len < 512 || len >= AR9300_EEPROM_SIZE) {
   632			kfree(buf);
   633			return -EINVAL;
   634		}
   635	
   636		/* devres manages the calibration values release on shutdown */
   637		ah->nvmem_blob = (u16 *)devm_kmemdup(sc->dev, buf, len, GFP_KERNEL);
   638		kfree(buf);
   639		if (!ah->nvmem_blob)
   640			return -ENOMEM;
   641	
   642		ah->nvmem_blob_len = len;
   643		ah->ah_flags &= ~AH_USE_EEPROM;
   644		ah->ah_flags |= AH_NO_EEP_SWAP;
   645	
   646		return 0;
   647	}
   648	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

[PATCH v3 3/3] wifi: ath10k: only load compatible DT cal data

[equu]

From: Edward Chow <equu@openmail.cc>

ath10k might also be sensitive to the issue reported on
https://github.com/openwrt/openwrt/pull/11345 , loading calibration
data from a device tree node declared incompatible.

ath10k will first check whether the device tree node is compatible
with it, using the functionality introduced with the first patch of
this series, ("PCI: of: Match pci devices or drivers against OF DT
nodes") and only proceed loading calibration data from compatible node.

Signed-off-by: Edward Chow <equu@openmail.cc>
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/net/wireless/ath/ath10k/core.c | 29 ++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/pci.c  |  2 +-
 drivers/net/wireless/ath/ath10k/pci.h  |  2 ++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 5eb131ab916f..a1cf65679046 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -13,6 +13,7 @@
 #include <linux/ctype.h>
 #include <linux/pm_qos.h>
 #include <linux/nvmem-consumer.h>
+#include <linux/of_pci.h>
 #include <asm/byteorder.h>
 
 #include "core.h"
@@ -26,6 +27,7 @@
 #include "testmode.h"
 #include "wmi-ops.h"
 #include "coredump.h"
+#include "pci.h"
 
 unsigned int ath10k_debug_mask;
 EXPORT_SYMBOL(ath10k_debug_mask);
@@ -1958,6 +1960,33 @@ static int ath10k_download_cal_nvmem(struct ath10k *ar, const char *cell_name)
 	size_t len;
 	int ret;
 
+	/* devm_nvmem_cell_get() will get a cell first from the OF
+	 * DT node representing the given device with nvmem-cell-name
+	 * "calibration", and from the global lookup table as a fallback,
+	 * and an ath10k device could be either a pci one or a platform one.
+	 *
+	 * If the OF DT node is not compatible with the real device, the
+	 * calibration data got from the node should not be applied.
+	 *
+	 * dev_is_pci(ar->dev) && ( no OF node || caldata not from node
+	 * || not compatible ) -> do not use caldata .
+	 *
+	 * !dev_is_pci(ar->dev) -> always use caldata .
+	 *
+	 * The judgement for compatibility differs with ath9k for many
+	 * DT using "qcom,ath10k" as compatibility string.
+	 */
+	if (dev_is_pci(ar->dev) &&
+	    (!ar->dev->of_node ||
+	     (of_property_match_string(ar->dev->of_node,
+				       "nvmem-cell-names",
+				       cell_name) < 0) ||
+	     !of_device_is_compatible(ar->dev->of_node,
+				      "qcom,ath10k") ||
+	     !of_pci_node_match_driver(ar->dev->of_node,
+				       &ath10k_pci_driver)))
+		return -ENOENT;
+
 	cell = devm_nvmem_cell_get(ar->dev, cell_name);
 	if (IS_ERR(cell)) {
 		ret = PTR_ERR(cell);
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 728d607289c3..5d9f6046f8cf 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -3780,7 +3780,7 @@ static SIMPLE_DEV_PM_OPS(ath10k_pci_pm_ops,
 			 ath10k_pci_pm_suspend,
 			 ath10k_pci_pm_resume);
 
-static struct pci_driver ath10k_pci_driver = {
+struct pci_driver ath10k_pci_driver = {
 	.name = "ath10k_pci",
 	.id_table = ath10k_pci_id_table,
 	.probe = ath10k_pci_probe,
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 480cd97ab739..de676797b736 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -209,6 +209,8 @@ static inline struct ath10k_pci *ath10k_pci_priv(struct ath10k *ar)
 #define DIAG_ACCESS_CE_TIMEOUT_US 10000 /* 10 ms */
 #define DIAG_ACCESS_CE_WAIT_US	50
 
+extern struct pci_driver ath10k_pci_driver;
+
 void ath10k_pci_write32(struct ath10k *ar, u32 offset, u32 value);
 void ath10k_pci_soc_write32(struct ath10k *ar, u32 addr, u32 val);
 void ath10k_pci_reg_write32(struct ath10k *ar, u32 addr, u32 val);
-- 
2.39.1

Re: [PATCH v3 3/3] wifi: ath10k: only load compatible DT cal data

[kernel test robot]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on helgaas-pci/next]
[also build test ERROR on helgaas-pci/for-linus wireless-next/main wireless/main linus/master v6.2-rc6 next-20230203]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/equu-openmail-cc/wifi-ath9k-stop-loading-incompatible-DT-cal-data/20230202-165536
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
patch link:    https://lore.kernel.org/r/20230202075524.2911058-4-equu%40openmail.cc
patch subject: [PATCH v3 3/3] wifi: ath10k: only load compatible DT cal data
config: i386-randconfig-a005 (https://download.01.org/0day-ci/archive/20230203/202302031943.aLVT61nB-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/d2ef9bc195ade55c010e64762b135a46a62f3fdd
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review equu-openmail-cc/wifi-ath9k-stop-loading-incompatible-DT-cal-data/20230202-165536
        git checkout d2ef9bc195ade55c010e64762b135a46a62f3fdd
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 olddefconfig
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/net/wireless/ath/ath10k/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from drivers/net/wireless/ath/ath10k/core.c:16:
   include/linux/of_pci.h:23:33: warning: 'struct pci_driver' declared inside parameter list will not be visible outside of this definition or declaration
      23 |                          struct pci_driver *drv);
         |                                 ^~~~~~~~~~
   drivers/net/wireless/ath/ath10k/core.c: In function 'ath10k_download_cal_nvmem':
>> drivers/net/wireless/ath/ath10k/core.c:1987:40: error: passing argument 2 of 'of_pci_node_match_driver' from incompatible pointer type [-Werror=incompatible-pointer-types]
    1987 |                                        &ath10k_pci_driver)))
         |                                        ^~~~~~~~~~~~~~~~~~
         |                                        |
         |                                        struct pci_driver *
   In file included from drivers/net/wireless/ath/ath10k/core.c:16:
   include/linux/of_pci.h:23:45: note: expected 'struct pci_driver *' but argument is of type 'struct pci_driver *'
      23 |                          struct pci_driver *drv);
         |                          ~~~~~~~~~~~~~~~~~~~^~~
   cc1: some warnings being treated as errors


vim +/of_pci_node_match_driver +1987 drivers/net/wireless/ath/ath10k/core.c

  1955	
  1956	static int ath10k_download_cal_nvmem(struct ath10k *ar, const char *cell_name)
  1957	{
  1958		struct nvmem_cell *cell;
  1959		void *buf;
  1960		size_t len;
  1961		int ret;
  1962	
  1963		/* devm_nvmem_cell_get() will get a cell first from the OF
  1964		 * DT node representing the given device with nvmem-cell-name
  1965		 * "calibration", and from the global lookup table as a fallback,
  1966		 * and an ath10k device could be either a pci one or a platform one.
  1967		 *
  1968		 * If the OF DT node is not compatible with the real device, the
  1969		 * calibration data got from the node should not be applied.
  1970		 *
  1971		 * dev_is_pci(ar->dev) && ( no OF node || caldata not from node
  1972		 * || not compatible ) -> do not use caldata .
  1973		 *
  1974		 * !dev_is_pci(ar->dev) -> always use caldata .
  1975		 *
  1976		 * The judgement for compatibility differs with ath9k for many
  1977		 * DT using "qcom,ath10k" as compatibility string.
  1978		 */
  1979		if (dev_is_pci(ar->dev) &&
  1980		    (!ar->dev->of_node ||
  1981		     (of_property_match_string(ar->dev->of_node,
  1982					       "nvmem-cell-names",
  1983					       cell_name) < 0) ||
  1984		     !of_device_is_compatible(ar->dev->of_node,
  1985					      "qcom,ath10k") ||
  1986		     !of_pci_node_match_driver(ar->dev->of_node,
> 1987					       &ath10k_pci_driver)))
  1988			return -ENOENT;
  1989	
  1990		cell = devm_nvmem_cell_get(ar->dev, cell_name);
  1991		if (IS_ERR(cell)) {
  1992			ret = PTR_ERR(cell);
  1993			return ret;
  1994		}
  1995	
  1996		buf = nvmem_cell_read(cell, &len);
  1997		if (IS_ERR(buf))
  1998			return PTR_ERR(buf);
  1999	
  2000		if (ar->hw_params.cal_data_len != len) {
  2001			kfree(buf);
  2002			ath10k_warn(ar, "invalid calibration data length in nvmem-cell '%s': %zu != %u\n",
  2003				    cell_name, len, ar->hw_params.cal_data_len);
  2004			return -EMSGSIZE;
  2005		}
  2006	
  2007		ret = ath10k_download_board_data(ar, buf, len);
  2008		kfree(buf);
  2009		if (ret)
  2010			ath10k_warn(ar, "failed to download calibration data from nvmem-cell '%s': %d\n",
  2011				    cell_name, ret);
  2012	
  2013		return ret;
  2014	}
  2015	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

[PATCH v4 0/3] PCI: of: Load extra data only from compatible DT nodes

[equu]

From: Edward Chow <equu@openmail.cc>

In order to solve the issue reported in
https://github.com/openwrt/openwrt/pull/11345 , this patchset attempt
to add mechanisms to ckeck whether an OF DT node is compatible to the
PCI device installed on the corresponding location or the driver for
it, and make ath9k and ath10k only load extra data from compatible
nodes.

V3 -> V4:
1, Fix some syntax errors.

Signed-off-by: Edward Chow <equu@openmail.cc>
Reported-by: kernel test robot <lkp@intel.com>

Edward Chow (3):
  PCI: of: Match pci devices or drivers against OF DT nodes
  wifi: ath9k: stop loading incompatible DT cal data
  wifi: ath10k: only load compatible DT cal data

 drivers/net/wireless/ath/ath10k/core.c |  29 +++
 drivers/net/wireless/ath/ath10k/pci.c  |   2 +-
 drivers/net/wireless/ath/ath10k/pci.h  |   2 +
 drivers/net/wireless/ath/ath9k/ath9k.h |   1 +
 drivers/net/wireless/ath/ath9k/init.c  |  26 +++
 drivers/net/wireless/ath/ath9k/pci.c   |   2 +-
 drivers/pci/of.c                       | 299 +++++++++++++++++++++++++
 drivers/pci/pci-driver.c               |   5 -
 drivers/pci/pci.h                      |  56 +++++
 include/linux/of_pci.h                 |  25 +++
 include/linux/pci.h                    |   6 +
 11 files changed, 446 insertions(+), 7 deletions(-)

-- 
2.39.1

[PATCH v4 1/3] PCI: of: Match pci devices or drivers against OF DT nodes

[equu]

From: Edward Chow <equu@openmail.cc>

Currently, whether a compatibility string within an OF DT node for a
PCI device (whose spec is at
https://www.devicetree.org/open-firmware/bindings/pci/ ) matches the
vendor and device id of either the PCI device installed on the
corresponding location or the driver suggested by the compatibility
string is not supported.

This patch introduces a function to decode a compatibility string into
a struct pci_device_id, which could further be matched against PCI
devices or drivers, as well as functions to match a compatibility
string or OF DT node against PCI devices or drivers.

Signed-off-by: Edward Chow <equu@openmail.cc>
Cc: Bjorn Helgaas <helgaas@kernel.org>
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/pci/of.c         | 299 +++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci-driver.c |   5 -
 drivers/pci/pci.h        |  56 ++++++++
 include/linux/of_pci.h   |  25 ++++
 include/linux/pci.h      |   6 +
 5 files changed, 386 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 196834ed44fe..a9c3648cd9e9 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -13,6 +13,8 @@
 #include <linux/of_irq.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
+#include <linux/string.h>
+#include <linux/kstrtox.h>
 #include "pci.h"
 
 #ifdef CONFIG_PCI
@@ -251,6 +253,303 @@ void of_pci_check_probe_only(void)
 }
 EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
 
+/**
+ * of_pci_compat_to_device_id() - Decode an OF compatibility string into a
+ * pci_device_id structure.
+ * @compat: the compatibility string to decode, could be NULL
+ * @id: pointer to a struct pci_device_id, to store the result
+ * @rev: pointer to output revision info, PCI_ANY_ID if no revision in @compat
+ * @req_pcie: pointer to output whether @compat mandates PCIe compatibility
+ *
+ * returns 0 when success, -EINVAL when failed.
+ */
+int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       u32 *rev, u32 *req_pcie)
+{
+	union {
+		u8 u8;
+		u16 u16;
+		u32 u32;
+	} res = {0};
+	*req_pcie = 0;
+	*rev = PCI_ANY_ID;
+	if (!compat || strncasecmp(compat, "pci", 3) != 0)
+		return -EINVAL;
+	compat += 3;
+
+	if (strncasecmp(compat, "class,", 6) == 0) {
+		/* pciclass,CCSSPP */
+		compat += 6;
+		if ((strlen(compat) < 4)
+		   || kstrtouint(compat, 16, &id->class))
+			return -EINVAL;
+		if (id->class < 0x10000) {
+			id->class <<= 8;
+			id->class_mask = 0xFFFF00;
+		} else {
+			id->class_mask = PCI_ANY_ID;
+		}
+		id->vendor = PCI_ANY_ID;
+		id->device = PCI_ANY_ID;
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0;
+	}
+
+	if (strncasecmp(compat, "ex", 2) == 0) {
+		/* pciex...  */
+		*req_pcie = 1;
+		compat += 2;
+	}
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->vendor = res.u16;
+	compat = strchr(compat, ',');
+	if (!compat)
+		return -EINVAL;
+	compat++;
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->device = res.u16;
+	compat = strchr(compat, '.');
+	if (compat == NULL) {
+		/* pciVVVV,DDDD */
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0;
+	}
+
+	compat++;
+	if (strlen(compat) == 2) {
+		/* pciVVVV,DDDD.RR */
+		if (kstrtou8(compat, 16, &res.u8))
+			return -EINVAL;
+		*rev = res.u8;
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0;
+	}
+
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->subvendor = res.u16;
+	compat = strchr(compat, '.');
+	if (!compat)
+		return -EINVAL;
+	compat++;
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->subdevice = res.u16;
+	compat = strchr(compat, '.');
+	if (compat == NULL)
+		/* pciVVVV,DDDD.SSSS.ssss */
+		return 0;
+
+	compat++;
+	if (strlen(compat) == 2) {
+		/* pciVVVV,DDDD.SSSS.ssss.RR */
+		if (kstrtou8(compat, 16, &res.u8))
+			return -EINVAL;
+		*rev = res.u8;
+	}
+	return 0;
+}
+
+/**
+ * of_pci_compat_match_device() - Tell whether a PCI device structure matches
+ * a given OF compatibility string
+ * @compat: single OF compatibility string to match, could be NULL
+ * @dev the PCI device structure to match against
+ *
+ * Returns whether they match.
+ */
+bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev)
+{
+	__u32 rev = PCI_ANY_ID;
+	__u32 req_pcie = 0;
+	struct pci_device_id id = {0};
+
+	if (of_pci_compat_to_device_id(compat, &id, &rev, &req_pcie))
+		return false;
+	return pci_match_one_device(&id, dev) &&
+		(rev == PCI_ANY_ID || rev == dev->revision) &&
+		req_pcie ? dev->pcie_cap : true;
+}
+
+/**
+ * of_pci_node_match_device() - Tell whether an OF device tree node
+ * matches the given pci device
+ * @node: single OF device tree node to match, could be NULL
+ * @dev: the PCI device structure to match against, could be NULL
+ *
+ * Returns whether they match.
+ */
+bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev)
+{
+	struct property *prop;
+	const char *cp;
+
+	if (!node || !dev)
+		return false;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		if (of_pci_compat_match_device(cp, dev))
+			return true;
+	}
+	return false;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_device);
+
+/**
+ * of_pci_compat_match_one_id() - Tell whether a PCI device ID structure matches
+ * a given OF compatibility string, note that there is no revision nor PCIe
+ * capability info in PCI device ID structures
+ *
+ * @compat: single OF compatibility string to match, could be NULL
+ * @id the PCI device ID structure to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure pointed by ID
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id)
+{
+	__u32 rev = PCI_ANY_ID;
+	__u32 req_pcie = 0;
+	struct pci_device_id pr = {0};
+
+	if (!compat || !id ||
+	    of_pci_compat_to_device_id(compat, &pr, &rev, &req_pcie))
+		return NULL;
+	return pci_match_one_id(id, &pr);
+}
+
+/**
+ * of_pci_compat_match_id_table() - Tell whether a given OF compatibility string
+ * matches a given pci_id table
+ *
+ * @compat: single OF compatibility string to match, could be NULL
+ * @table the PCI device ID table to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table)
+{
+	if (compat && table) {
+		while (table->vendor || table->subvendor || table->class_mask) {
+			if (of_pci_compat_match_one_id(compat, table))
+				return table;
+			table++;
+		}
+	}
+	return NULL;
+}
+
+/**
+ * of_pci_node_match_id_table() - Tell whether an OF device tree node
+ * matches the given pci_id table
+ * @node: single OF device tree node to match, could be NULL
+ * @table: the PCI device ID table to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table)
+{
+	struct property *prop;
+	const char *cp;
+	const struct pci_device_id *id;
+
+	if (!node || !table)
+		return NULL;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		id = of_pci_compat_match_id_table(cp, table);
+		if (id)
+			return id;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_id_table);
+
+/**
+ * of_pci_compat_match_driver - See if a given OF compatibility string matches
+ * a driver's list of IDs
+ * @compat: single OF compatibility string to match, could be NULL
+ * @drv: the PCI driver to match against, could be NULL
+ *
+ * Used by a driver to check whether an OF compatibility string matches one of
+ * (dynamically) supported devices, which may have been augmented
+ * via the sysfs "new_id" file.  Returns the matching pci_device_id
+ * structure or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv)
+{
+	struct pci_dynid *dynid;
+	const struct pci_device_id *found_id = NULL, *ids;
+
+	if (!compat || !drv)
+		return NULL;
+	/* Look at the dynamic ids first, before the static ones */
+	spin_lock(&drv->dynids.lock);
+	list_for_each_entry(dynid, &drv->dynids.list, node) {
+		if (of_pci_compat_match_one_id(compat, &dynid->id)) {
+			found_id = &dynid->id;
+			break;
+		}
+	}
+	spin_unlock(&drv->dynids.lock);
+
+	if (found_id)
+		return found_id;
+
+	for (ids = drv->id_table; (found_id = of_pci_compat_match_one_id(compat, ids));
+	     ids = found_id + 1) {
+		/* exclude ids in id_table with override_only */
+		if (!found_id->override_only)
+			return found_id;
+	}
+
+	return NULL;
+}
+
+/**
+ * of_pci_node_match_driver() - Tell whether an OF device tree node
+ * matches the given pci driver
+ * @node: single OF device tree node to match, could be NULL
+ * @drv: the PCI driver structure to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv)
+{
+	struct property *prop;
+	const char *cp;
+	const struct pci_device_id *id;
+
+	if (!node || !drv)
+		return NULL;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		id = of_pci_compat_match_driver(cp, drv);
+		if (id)
+			return id;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_driver);
+
 /**
  * devm_of_pci_get_host_bridge_resources() - Resource-managed parsing of PCI
  *                                           host bridge resources from DT
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index a2ceeacc33eb..aa212d12353f 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -24,11 +24,6 @@
 #include "pci.h"
 #include "pcie/portdrv.h"
 
-struct pci_dynid {
-	struct list_head node;
-	struct pci_device_id id;
-};
-
 /**
  * pci_add_dynid - add a new PCI device ID to this driver and re-probe devices
  * @drv: target pci driver
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9ed3b5550043..e30652021a63 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -204,6 +204,29 @@ pci_match_one_device(const struct pci_device_id *id, const struct pci_dev *dev)
 	return NULL;
 }
 
+/**
+ * pci_match_one_id - Tell if a PCI device id structure matches another
+ *			  PCI device id structure
+ * @id: single PCI device id structure to match, usually in a list or array
+ * @pr: the probing PCI device id structure to match against, usually converted from
+ *      other format
+ *
+ * Returns the matching pci_device_id structure pointed by id
+ * or %NULL if there is no match.
+ */
+static inline const struct pci_device_id *
+pci_match_one_id(const struct pci_device_id *id, const struct pci_device_id *pr)
+{
+	if ((id->vendor == pr->vendor) &&
+	    (id->device == pr->device) &&
+	    (id->subvendor == pr->subvendor) &&
+	    (id->subdevice == pr->subdevice) &&
+	    (id->class == pr->class) &&
+	    (id->class_mask == pr->class_mask))
+		return id;
+	return NULL;
+}
+
 /* PCI slot sysfs helper code */
 #define to_pci_slot(s) container_of(s, struct pci_slot, kobj)
 
@@ -638,6 +661,15 @@ void pci_release_bus_of_node(struct pci_bus *bus);
 
 int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge);
 
+int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       __u32 *rev, __u32 *req_pcie);
+bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev);
+const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id);
+const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table);
+const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv);
 #else
 static inline int
 of_pci_parse_bus_range(struct device_node *node, struct resource *res)
@@ -679,6 +711,30 @@ static inline int devm_of_pci_bridge_init(struct device *dev, struct pci_host_br
 	return 0;
 }
 
+static inline int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       __u32 *rev, __u32 *req_pcie)
+{
+	return -EINVAL;
+}
+static inline bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev)
+{
+	return false;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv)
+{
+	return NULL;
+}
 #endif /* CONFIG_OF */
 
 #ifdef CONFIG_PCIEAER
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 29658c0ee71f..eef1eaafc03d 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -13,6 +13,14 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn);
 int of_pci_get_devfn(struct device_node *np);
 void of_pci_check_probe_only(void);
+bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev);
+const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table);
+const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv);
 #else
 static inline struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn)
@@ -26,6 +34,23 @@ static inline int of_pci_get_devfn(struct device_node *np)
 }
 
 static inline void of_pci_check_probe_only(void) { }
+static inline bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev)
+{
+	return false;
+}
+static inline const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv)
+{
+	return NULL;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_OF_IRQ)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index adffd65e84b4..04c908d84b90 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1513,6 +1513,12 @@ void pci_unregister_driver(struct pci_driver *dev);
 	builtin_driver(__pci_driver, pci_register_driver)
 
 struct pci_driver *pci_dev_driver(const struct pci_dev *dev);
+
+struct pci_dynid {
+	struct list_head node;
+	struct pci_device_id id;
+};
+
 int pci_add_dynid(struct pci_driver *drv,
 		  unsigned int vendor, unsigned int device,
 		  unsigned int subvendor, unsigned int subdevice,
-- 
2.39.1

[PATCH v4 2/3] wifi: ath9k: stop loading incompatible DT cal data

[equu]

From: Edward Chow <equu@openmail.cc>

As reported in https://github.com/openwrt/openwrt/pull/11345 , ath9k
would load calibration data from a device tree node declared
incompatible.

Now, ath9k will first check whether the device tree node is compatible
with it, using the functionality introduced with the first patch of
this series, ("PCI: of: Match pci devices or drivers against OF DT
nodes") and only proceed loading calibration data from compatible node.

Signed-off-by: Edward Chow <equu@openmail.cc>
---
 drivers/net/wireless/ath/ath9k/ath9k.h |  1 +
 drivers/net/wireless/ath/ath9k/init.c  | 26 ++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/pci.c   |  2 +-
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 2cc23605c9fc..4f6f0383a5f8 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -35,6 +35,7 @@ struct ath_node;
 struct ath_vif;
 
 extern struct ieee80211_ops ath9k_ops;
+extern struct pci_driver ath_pci_driver;
 extern int ath9k_modparam_nohwcrypt;
 extern int ath9k_led_blink;
 extern bool is_ath9k_unloaded;
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 4f00400c7ffb..6c0296b4d366 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -22,6 +22,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_net.h>
+#include <linux/of_pci.h>
 #include <linux/nvmem-consumer.h>
 #include <linux/relay.h>
 #include <linux/dmi.h>
@@ -577,6 +578,31 @@ static int ath9k_nvmem_request_eeprom(struct ath_softc *sc)
 	size_t len;
 	int err;
 
+	/* devm_nvmem_cell_get() will get a cell first from the OF
+	 * DT node representing the given device with nvmem-cell-name
+	 * "calibration", and from the global lookup table as a fallback,
+	 * and an ath9k device could be either a pci one or a platform one.
+	 *
+	 * If the OF DT node is not compatible with the real device, the
+	 * calibration data got from the node should not be applied.
+	 *
+	 * dev_is_pci(sc->dev) && ( no OF node || caldata not from node
+	 * || not compatible ) -> do not use caldata .
+	 *
+	 * !dev_is_pci(sc->dev) -> always use caldata .
+	 */
+	if (dev_is_pci(sc->dev) &&
+	    (!sc->dev->of_node ||
+	     !of_property_match_string(sc->dev->of_node,
+				       "nvmem-cell-names",
+				       "calibration") ||
+	     !of_pci_node_match_driver(sc->dev->of_node,
+				       &ath_pci_driver)))
+		/* follow the "just return 0;" convention as
+		 * noted below.
+		 */
+		return 0;
+
 	cell = devm_nvmem_cell_get(sc->dev, "calibration");
 	if (IS_ERR(cell)) {
 		err = PTR_ERR(cell);
diff --git a/drivers/net/wireless/ath/ath9k/pci.c b/drivers/net/wireless/ath/ath9k/pci.c
index a074e23013c5..fcb19761e60d 100644
--- a/drivers/net/wireless/ath/ath9k/pci.c
+++ b/drivers/net/wireless/ath/ath9k/pci.c
@@ -1074,7 +1074,7 @@ static SIMPLE_DEV_PM_OPS(ath9k_pm_ops, ath_pci_suspend, ath_pci_resume);
 
 MODULE_DEVICE_TABLE(pci, ath_pci_id_table);
 
-static struct pci_driver ath_pci_driver = {
+struct pci_driver ath_pci_driver = {
 	.name       = "ath9k",
 	.id_table   = ath_pci_id_table,
 	.probe      = ath_pci_probe,
-- 
2.39.1

[PATCH v4 3/3] wifi: ath10k: only load compatible DT cal data

[equu]

From: Edward Chow <equu@openmail.cc>

ath10k might also be sensitive to the issue reported on
https://github.com/openwrt/openwrt/pull/11345 , loading calibration
data from a device tree node declared incompatible.

ath10k will first check whether the device tree node is compatible
with it, using the functionality introduced with the first patch of
this series, ("PCI: of: Match pci devices or drivers against OF DT
nodes") and only proceed loading calibration data from compatible node.

Signed-off-by: Edward Chow <equu@openmail.cc>
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/net/wireless/ath/ath10k/core.c | 29 ++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/pci.c  |  2 +-
 drivers/net/wireless/ath/ath10k/pci.h  |  2 ++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 5eb131ab916f..a1cf65679046 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -13,6 +13,7 @@
 #include <linux/ctype.h>
 #include <linux/pm_qos.h>
 #include <linux/nvmem-consumer.h>
+#include <linux/of_pci.h>
 #include <asm/byteorder.h>
 
 #include "core.h"
@@ -26,6 +27,7 @@
 #include "testmode.h"
 #include "wmi-ops.h"
 #include "coredump.h"
+#include "pci.h"
 
 unsigned int ath10k_debug_mask;
 EXPORT_SYMBOL(ath10k_debug_mask);
@@ -1958,6 +1960,33 @@ static int ath10k_download_cal_nvmem(struct ath10k *ar, const char *cell_name)
 	size_t len;
 	int ret;
 
+	/* devm_nvmem_cell_get() will get a cell first from the OF
+	 * DT node representing the given device with nvmem-cell-name
+	 * "calibration", and from the global lookup table as a fallback,
+	 * and an ath10k device could be either a pci one or a platform one.
+	 *
+	 * If the OF DT node is not compatible with the real device, the
+	 * calibration data got from the node should not be applied.
+	 *
+	 * dev_is_pci(ar->dev) && ( no OF node || caldata not from node
+	 * || not compatible ) -> do not use caldata .
+	 *
+	 * !dev_is_pci(ar->dev) -> always use caldata .
+	 *
+	 * The judgement for compatibility differs with ath9k for many
+	 * DT using "qcom,ath10k" as compatibility string.
+	 */
+	if (dev_is_pci(ar->dev) &&
+	    (!ar->dev->of_node ||
+	     (of_property_match_string(ar->dev->of_node,
+				       "nvmem-cell-names",
+				       cell_name) < 0) ||
+	     !of_device_is_compatible(ar->dev->of_node,
+				      "qcom,ath10k") ||
+	     !of_pci_node_match_driver(ar->dev->of_node,
+				       &ath10k_pci_driver)))
+		return -ENOENT;
+
 	cell = devm_nvmem_cell_get(ar->dev, cell_name);
 	if (IS_ERR(cell)) {
 		ret = PTR_ERR(cell);
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 728d607289c3..5d9f6046f8cf 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -3780,7 +3780,7 @@ static SIMPLE_DEV_PM_OPS(ath10k_pci_pm_ops,
 			 ath10k_pci_pm_suspend,
 			 ath10k_pci_pm_resume);
 
-static struct pci_driver ath10k_pci_driver = {
+struct pci_driver ath10k_pci_driver = {
 	.name = "ath10k_pci",
 	.id_table = ath10k_pci_id_table,
 	.probe = ath10k_pci_probe,
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 480cd97ab739..de676797b736 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -209,6 +209,8 @@ static inline struct ath10k_pci *ath10k_pci_priv(struct ath10k *ar)
 #define DIAG_ACCESS_CE_TIMEOUT_US 10000 /* 10 ms */
 #define DIAG_ACCESS_CE_WAIT_US	50
 
+extern struct pci_driver ath10k_pci_driver;
+
 void ath10k_pci_write32(struct ath10k *ar, u32 offset, u32 value);
 void ath10k_pci_soc_write32(struct ath10k *ar, u32 addr, u32 val);
 void ath10k_pci_reg_write32(struct ath10k *ar, u32 addr, u32 val);
-- 
2.39.1

[PATCH v5 0/3] PCI: of: Load extra data only from compatible DT nodes

[equu]

From: Edward Chow <equu@openmail.cc>

In order to solve the issue reported in
https://github.com/openwrt/openwrt/pull/11345 , this patchset attempt
to add mechanisms to ckeck whether an OF DT node is compatible to the
PCI device installed on the corresponding location or the driver for
it, and make ath9k and ath10k only load extra data from compatible
nodes.

V4 -> V5:
2, include <linux/pci.h> in ath9k/pci.c.

3, include <linux/pci.h> in ath10k/core.c.

Signed-off-by: Edward Chow <equu@openmail.cc>
Reported-by: kernel test robot <lkp@intel.com>

Edward Chow (3):
  PCI: of: Match pci devices or drivers against OF DT nodes
  wifi: ath9k: stop loading incompatible DT cal data
  wifi: ath10k: only load compatible DT cal data

 drivers/net/wireless/ath/ath10k/core.c |  30 +++
 drivers/net/wireless/ath/ath10k/pci.c  |   2 +-
 drivers/net/wireless/ath/ath10k/pci.h  |   2 +
 drivers/net/wireless/ath/ath9k/ath9k.h |   1 +
 drivers/net/wireless/ath/ath9k/init.c  |  27 +++
 drivers/net/wireless/ath/ath9k/pci.c   |   2 +-
 drivers/pci/of.c                       | 299 +++++++++++++++++++++++++
 drivers/pci/pci-driver.c               |   5 -
 drivers/pci/pci.h                      |  56 +++++
 include/linux/of_pci.h                 |  25 +++
 include/linux/pci.h                    |   6 +
 11 files changed, 448 insertions(+), 7 deletions(-)

-- 
2.39.1

[PATCH v5 1/3] PCI: of: Match pci devices or drivers against OF DT nodes

[equu]

From: Edward Chow <equu@openmail.cc>

Currently, whether a compatibility string within an OF DT node for a
PCI device (whose spec is at
https://www.devicetree.org/open-firmware/bindings/pci/ ) matches the
vendor and device id of either the PCI device installed on the
corresponding location or the driver suggested by the compatibility
string is not supported.

This patch introduces a function to decode a compatibility string into
a struct pci_device_id, which could further be matched against PCI
devices or drivers, as well as functions to match a compatibility
string or OF DT node against PCI devices or drivers.

Signed-off-by: Edward Chow <equu@openmail.cc>
Cc: Bjorn Helgaas <helgaas@kernel.org>
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/pci/of.c         | 299 +++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci-driver.c |   5 -
 drivers/pci/pci.h        |  56 ++++++++
 include/linux/of_pci.h   |  25 ++++
 include/linux/pci.h      |   6 +
 5 files changed, 386 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 196834ed44fe..a9c3648cd9e9 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -13,6 +13,8 @@
 #include <linux/of_irq.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
+#include <linux/string.h>
+#include <linux/kstrtox.h>
 #include "pci.h"
 
 #ifdef CONFIG_PCI
@@ -251,6 +253,303 @@ void of_pci_check_probe_only(void)
 }
 EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
 
+/**
+ * of_pci_compat_to_device_id() - Decode an OF compatibility string into a
+ * pci_device_id structure.
+ * @compat: the compatibility string to decode, could be NULL
+ * @id: pointer to a struct pci_device_id, to store the result
+ * @rev: pointer to output revision info, PCI_ANY_ID if no revision in @compat
+ * @req_pcie: pointer to output whether @compat mandates PCIe compatibility
+ *
+ * returns 0 when success, -EINVAL when failed.
+ */
+int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       u32 *rev, u32 *req_pcie)
+{
+	union {
+		u8 u8;
+		u16 u16;
+		u32 u32;
+	} res = {0};
+	*req_pcie = 0;
+	*rev = PCI_ANY_ID;
+	if (!compat || strncasecmp(compat, "pci", 3) != 0)
+		return -EINVAL;
+	compat += 3;
+
+	if (strncasecmp(compat, "class,", 6) == 0) {
+		/* pciclass,CCSSPP */
+		compat += 6;
+		if ((strlen(compat) < 4)
+		   || kstrtouint(compat, 16, &id->class))
+			return -EINVAL;
+		if (id->class < 0x10000) {
+			id->class <<= 8;
+			id->class_mask = 0xFFFF00;
+		} else {
+			id->class_mask = PCI_ANY_ID;
+		}
+		id->vendor = PCI_ANY_ID;
+		id->device = PCI_ANY_ID;
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0;
+	}
+
+	if (strncasecmp(compat, "ex", 2) == 0) {
+		/* pciex...  */
+		*req_pcie = 1;
+		compat += 2;
+	}
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->vendor = res.u16;
+	compat = strchr(compat, ',');
+	if (!compat)
+		return -EINVAL;
+	compat++;
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->device = res.u16;
+	compat = strchr(compat, '.');
+	if (compat == NULL) {
+		/* pciVVVV,DDDD */
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0;
+	}
+
+	compat++;
+	if (strlen(compat) == 2) {
+		/* pciVVVV,DDDD.RR */
+		if (kstrtou8(compat, 16, &res.u8))
+			return -EINVAL;
+		*rev = res.u8;
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0;
+	}
+
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->subvendor = res.u16;
+	compat = strchr(compat, '.');
+	if (!compat)
+		return -EINVAL;
+	compat++;
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->subdevice = res.u16;
+	compat = strchr(compat, '.');
+	if (compat == NULL)
+		/* pciVVVV,DDDD.SSSS.ssss */
+		return 0;
+
+	compat++;
+	if (strlen(compat) == 2) {
+		/* pciVVVV,DDDD.SSSS.ssss.RR */
+		if (kstrtou8(compat, 16, &res.u8))
+			return -EINVAL;
+		*rev = res.u8;
+	}
+	return 0;
+}
+
+/**
+ * of_pci_compat_match_device() - Tell whether a PCI device structure matches
+ * a given OF compatibility string
+ * @compat: single OF compatibility string to match, could be NULL
+ * @dev the PCI device structure to match against
+ *
+ * Returns whether they match.
+ */
+bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev)
+{
+	__u32 rev = PCI_ANY_ID;
+	__u32 req_pcie = 0;
+	struct pci_device_id id = {0};
+
+	if (of_pci_compat_to_device_id(compat, &id, &rev, &req_pcie))
+		return false;
+	return pci_match_one_device(&id, dev) &&
+		(rev == PCI_ANY_ID || rev == dev->revision) &&
+		req_pcie ? dev->pcie_cap : true;
+}
+
+/**
+ * of_pci_node_match_device() - Tell whether an OF device tree node
+ * matches the given pci device
+ * @node: single OF device tree node to match, could be NULL
+ * @dev: the PCI device structure to match against, could be NULL
+ *
+ * Returns whether they match.
+ */
+bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev)
+{
+	struct property *prop;
+	const char *cp;
+
+	if (!node || !dev)
+		return false;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		if (of_pci_compat_match_device(cp, dev))
+			return true;
+	}
+	return false;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_device);
+
+/**
+ * of_pci_compat_match_one_id() - Tell whether a PCI device ID structure matches
+ * a given OF compatibility string, note that there is no revision nor PCIe
+ * capability info in PCI device ID structures
+ *
+ * @compat: single OF compatibility string to match, could be NULL
+ * @id the PCI device ID structure to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure pointed by ID
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id)
+{
+	__u32 rev = PCI_ANY_ID;
+	__u32 req_pcie = 0;
+	struct pci_device_id pr = {0};
+
+	if (!compat || !id ||
+	    of_pci_compat_to_device_id(compat, &pr, &rev, &req_pcie))
+		return NULL;
+	return pci_match_one_id(id, &pr);
+}
+
+/**
+ * of_pci_compat_match_id_table() - Tell whether a given OF compatibility string
+ * matches a given pci_id table
+ *
+ * @compat: single OF compatibility string to match, could be NULL
+ * @table the PCI device ID table to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table)
+{
+	if (compat && table) {
+		while (table->vendor || table->subvendor || table->class_mask) {
+			if (of_pci_compat_match_one_id(compat, table))
+				return table;
+			table++;
+		}
+	}
+	return NULL;
+}
+
+/**
+ * of_pci_node_match_id_table() - Tell whether an OF device tree node
+ * matches the given pci_id table
+ * @node: single OF device tree node to match, could be NULL
+ * @table: the PCI device ID table to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table)
+{
+	struct property *prop;
+	const char *cp;
+	const struct pci_device_id *id;
+
+	if (!node || !table)
+		return NULL;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		id = of_pci_compat_match_id_table(cp, table);
+		if (id)
+			return id;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_id_table);
+
+/**
+ * of_pci_compat_match_driver - See if a given OF compatibility string matches
+ * a driver's list of IDs
+ * @compat: single OF compatibility string to match, could be NULL
+ * @drv: the PCI driver to match against, could be NULL
+ *
+ * Used by a driver to check whether an OF compatibility string matches one of
+ * (dynamically) supported devices, which may have been augmented
+ * via the sysfs "new_id" file.  Returns the matching pci_device_id
+ * structure or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv)
+{
+	struct pci_dynid *dynid;
+	const struct pci_device_id *found_id = NULL, *ids;
+
+	if (!compat || !drv)
+		return NULL;
+	/* Look at the dynamic ids first, before the static ones */
+	spin_lock(&drv->dynids.lock);
+	list_for_each_entry(dynid, &drv->dynids.list, node) {
+		if (of_pci_compat_match_one_id(compat, &dynid->id)) {
+			found_id = &dynid->id;
+			break;
+		}
+	}
+	spin_unlock(&drv->dynids.lock);
+
+	if (found_id)
+		return found_id;
+
+	for (ids = drv->id_table; (found_id = of_pci_compat_match_one_id(compat, ids));
+	     ids = found_id + 1) {
+		/* exclude ids in id_table with override_only */
+		if (!found_id->override_only)
+			return found_id;
+	}
+
+	return NULL;
+}
+
+/**
+ * of_pci_node_match_driver() - Tell whether an OF device tree node
+ * matches the given pci driver
+ * @node: single OF device tree node to match, could be NULL
+ * @drv: the PCI driver structure to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv)
+{
+	struct property *prop;
+	const char *cp;
+	const struct pci_device_id *id;
+
+	if (!node || !drv)
+		return NULL;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		id = of_pci_compat_match_driver(cp, drv);
+		if (id)
+			return id;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_driver);
+
 /**
  * devm_of_pci_get_host_bridge_resources() - Resource-managed parsing of PCI
  *                                           host bridge resources from DT
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index a2ceeacc33eb..aa212d12353f 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -24,11 +24,6 @@
 #include "pci.h"
 #include "pcie/portdrv.h"
 
-struct pci_dynid {
-	struct list_head node;
-	struct pci_device_id id;
-};
-
 /**
  * pci_add_dynid - add a new PCI device ID to this driver and re-probe devices
  * @drv: target pci driver
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9ed3b5550043..e30652021a63 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -204,6 +204,29 @@ pci_match_one_device(const struct pci_device_id *id, const struct pci_dev *dev)
 	return NULL;
 }
 
+/**
+ * pci_match_one_id - Tell if a PCI device id structure matches another
+ *			  PCI device id structure
+ * @id: single PCI device id structure to match, usually in a list or array
+ * @pr: the probing PCI device id structure to match against, usually converted from
+ *      other format
+ *
+ * Returns the matching pci_device_id structure pointed by id
+ * or %NULL if there is no match.
+ */
+static inline const struct pci_device_id *
+pci_match_one_id(const struct pci_device_id *id, const struct pci_device_id *pr)
+{
+	if ((id->vendor == pr->vendor) &&
+	    (id->device == pr->device) &&
+	    (id->subvendor == pr->subvendor) &&
+	    (id->subdevice == pr->subdevice) &&
+	    (id->class == pr->class) &&
+	    (id->class_mask == pr->class_mask))
+		return id;
+	return NULL;
+}
+
 /* PCI slot sysfs helper code */
 #define to_pci_slot(s) container_of(s, struct pci_slot, kobj)
 
@@ -638,6 +661,15 @@ void pci_release_bus_of_node(struct pci_bus *bus);
 
 int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge);
 
+int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       __u32 *rev, __u32 *req_pcie);
+bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev);
+const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id);
+const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table);
+const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv);
 #else
 static inline int
 of_pci_parse_bus_range(struct device_node *node, struct resource *res)
@@ -679,6 +711,30 @@ static inline int devm_of_pci_bridge_init(struct device *dev, struct pci_host_br
 	return 0;
 }
 
+static inline int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       __u32 *rev, __u32 *req_pcie)
+{
+	return -EINVAL;
+}
+static inline bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev)
+{
+	return false;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv)
+{
+	return NULL;
+}
 #endif /* CONFIG_OF */
 
 #ifdef CONFIG_PCIEAER
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 29658c0ee71f..eef1eaafc03d 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -13,6 +13,14 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn);
 int of_pci_get_devfn(struct device_node *np);
 void of_pci_check_probe_only(void);
+bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev);
+const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table);
+const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv);
 #else
 static inline struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn)
@@ -26,6 +34,23 @@ static inline int of_pci_get_devfn(struct device_node *np)
 }
 
 static inline void of_pci_check_probe_only(void) { }
+static inline bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev)
+{
+	return false;
+}
+static inline const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv)
+{
+	return NULL;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_OF_IRQ)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index adffd65e84b4..04c908d84b90 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1513,6 +1513,12 @@ void pci_unregister_driver(struct pci_driver *dev);
 	builtin_driver(__pci_driver, pci_register_driver)
 
 struct pci_driver *pci_dev_driver(const struct pci_dev *dev);
+
+struct pci_dynid {
+	struct list_head node;
+	struct pci_device_id id;
+};
+
 int pci_add_dynid(struct pci_driver *drv,
 		  unsigned int vendor, unsigned int device,
 		  unsigned int subvendor, unsigned int subdevice,
-- 
2.39.1

[PATCH v5 2/3] wifi: ath9k: stop loading incompatible DT cal data

[equu]

From: Edward Chow <equu@openmail.cc>

As reported in https://github.com/openwrt/openwrt/pull/11345 , ath9k
would load calibration data from a device tree node declared
incompatible.

Now, ath9k will first check whether the device tree node is compatible
with it, using the functionality introduced with the first patch of
this series, ("PCI: of: Match pci devices or drivers against OF DT
nodes") and only proceed loading calibration data from compatible node.

Signed-off-by: Edward Chow <equu@openmail.cc>
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/net/wireless/ath/ath9k/ath9k.h |  1 +
 drivers/net/wireless/ath/ath9k/init.c  | 27 ++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/pci.c   |  2 +-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 2cc23605c9fc..4f6f0383a5f8 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -35,6 +35,7 @@ struct ath_node;
 struct ath_vif;
 
 extern struct ieee80211_ops ath9k_ops;
+extern struct pci_driver ath_pci_driver;
 extern int ath9k_modparam_nohwcrypt;
 extern int ath9k_led_blink;
 extern bool is_ath9k_unloaded;
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 4f00400c7ffb..fbfb5b5d32c3 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -22,6 +22,8 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_net.h>
+#include <linux/of_pci.h>
+#include <linux/pci.h>
 #include <linux/nvmem-consumer.h>
 #include <linux/relay.h>
 #include <linux/dmi.h>
@@ -577,6 +579,31 @@ static int ath9k_nvmem_request_eeprom(struct ath_softc *sc)
 	size_t len;
 	int err;
 
+	/* devm_nvmem_cell_get() will get a cell first from the OF
+	 * DT node representing the given device with nvmem-cell-name
+	 * "calibration", and from the global lookup table as a fallback,
+	 * and an ath9k device could be either a pci one or a platform one.
+	 *
+	 * If the OF DT node is not compatible with the real device, the
+	 * calibration data got from the node should not be applied.
+	 *
+	 * dev_is_pci(sc->dev) && ( no OF node || caldata not from node
+	 * || not compatible ) -> do not use caldata .
+	 *
+	 * !dev_is_pci(sc->dev) -> always use caldata .
+	 */
+	if (dev_is_pci(sc->dev) &&
+	    (!sc->dev->of_node ||
+	     !of_property_match_string(sc->dev->of_node,
+				       "nvmem-cell-names",
+				       "calibration") ||
+	     !of_pci_node_match_driver(sc->dev->of_node,
+				       &ath_pci_driver)))
+		/* follow the "just return 0;" convention as
+		 * noted below.
+		 */
+		return 0;
+
 	cell = devm_nvmem_cell_get(sc->dev, "calibration");
 	if (IS_ERR(cell)) {
 		err = PTR_ERR(cell);
diff --git a/drivers/net/wireless/ath/ath9k/pci.c b/drivers/net/wireless/ath/ath9k/pci.c
index a074e23013c5..fcb19761e60d 100644
--- a/drivers/net/wireless/ath/ath9k/pci.c
+++ b/drivers/net/wireless/ath/ath9k/pci.c
@@ -1074,7 +1074,7 @@ static SIMPLE_DEV_PM_OPS(ath9k_pm_ops, ath_pci_suspend, ath_pci_resume);
 
 MODULE_DEVICE_TABLE(pci, ath_pci_id_table);
 
-static struct pci_driver ath_pci_driver = {
+struct pci_driver ath_pci_driver = {
 	.name       = "ath9k",
 	.id_table   = ath_pci_id_table,
 	.probe      = ath_pci_probe,
-- 
2.39.1

[PATCH v5 3/3] wifi: ath10k: only load compatible DT cal data

[equu]

From: Edward Chow <equu@openmail.cc>

ath10k might also be sensitive to the issue reported on
https://github.com/openwrt/openwrt/pull/11345 , loading calibration
data from a device tree node declared incompatible.

ath10k will first check whether the device tree node is compatible
with it, using the functionality introduced with the first patch of
this series, ("PCI: of: Match pci devices or drivers against OF DT
nodes") and only proceed loading calibration data from compatible node.

Signed-off-by: Edward Chow <equu@openmail.cc>
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/net/wireless/ath/ath10k/core.c | 30 ++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/pci.c  |  2 +-
 drivers/net/wireless/ath/ath10k/pci.h  |  2 ++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 5eb131ab916f..a776b06f49b5 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -13,6 +13,8 @@
 #include <linux/ctype.h>
 #include <linux/pm_qos.h>
 #include <linux/nvmem-consumer.h>
+#include <linux/of_pci.h>
+#include <linux/pci.h>
 #include <asm/byteorder.h>
 
 #include "core.h"
@@ -26,6 +28,7 @@
 #include "testmode.h"
 #include "wmi-ops.h"
 #include "coredump.h"
+#include "pci.h"
 
 unsigned int ath10k_debug_mask;
 EXPORT_SYMBOL(ath10k_debug_mask);
@@ -1958,6 +1961,33 @@ static int ath10k_download_cal_nvmem(struct ath10k *ar, const char *cell_name)
 	size_t len;
 	int ret;
 
+	/* devm_nvmem_cell_get() will get a cell first from the OF
+	 * DT node representing the given device with nvmem-cell-name
+	 * "calibration", and from the global lookup table as a fallback,
+	 * and an ath10k device could be either a pci one or a platform one.
+	 *
+	 * If the OF DT node is not compatible with the real device, the
+	 * calibration data got from the node should not be applied.
+	 *
+	 * dev_is_pci(ar->dev) && ( no OF node || caldata not from node
+	 * || not compatible ) -> do not use caldata .
+	 *
+	 * !dev_is_pci(ar->dev) -> always use caldata .
+	 *
+	 * The judgement for compatibility differs with ath9k for many
+	 * DT using "qcom,ath10k" as compatibility string.
+	 */
+	if (dev_is_pci(ar->dev) &&
+	    (!ar->dev->of_node ||
+	     (of_property_match_string(ar->dev->of_node,
+				       "nvmem-cell-names",
+				       cell_name) < 0) ||
+	     !of_device_is_compatible(ar->dev->of_node,
+				      "qcom,ath10k") ||
+	     !of_pci_node_match_driver(ar->dev->of_node,
+				       &ath10k_pci_driver)))
+		return -ENOENT;
+
 	cell = devm_nvmem_cell_get(ar->dev, cell_name);
 	if (IS_ERR(cell)) {
 		ret = PTR_ERR(cell);
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 728d607289c3..5d9f6046f8cf 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -3780,7 +3780,7 @@ static SIMPLE_DEV_PM_OPS(ath10k_pci_pm_ops,
 			 ath10k_pci_pm_suspend,
 			 ath10k_pci_pm_resume);
 
-static struct pci_driver ath10k_pci_driver = {
+struct pci_driver ath10k_pci_driver = {
 	.name = "ath10k_pci",
 	.id_table = ath10k_pci_id_table,
 	.probe = ath10k_pci_probe,
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 480cd97ab739..de676797b736 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -209,6 +209,8 @@ static inline struct ath10k_pci *ath10k_pci_priv(struct ath10k *ar)
 #define DIAG_ACCESS_CE_TIMEOUT_US 10000 /* 10 ms */
 #define DIAG_ACCESS_CE_WAIT_US	50
 
+extern struct pci_driver ath10k_pci_driver;
+
 void ath10k_pci_write32(struct ath10k *ar, u32 offset, u32 value);
 void ath10k_pci_soc_write32(struct ath10k *ar, u32 addr, u32 val);
 void ath10k_pci_reg_write32(struct ath10k *ar, u32 addr, u32 val);
-- 
2.39.1

Re: [PATCH v5 3/3] wifi: ath10k: only load compatible DT cal data

[Rob Herring]

On Fri, Feb 3, 2023 at 4:48 AM <equu@openmail.cc> wrote:
>
> From: Edward Chow <equu@openmail.cc>
>
> ath10k might also be sensitive to the issue reported on
> https://github.com/openwrt/openwrt/pull/11345 , loading calibration
> data from a device tree node declared incompatible.
>
> ath10k will first check whether the device tree node is compatible
> with it, using the functionality introduced with the first patch of
> this series, ("PCI: of: Match pci devices or drivers against OF DT
> nodes") and only proceed loading calibration data from compatible node.
>
> Signed-off-by: Edward Chow <equu@openmail.cc>
> Reported-by: kernel test robot <lkp@intel.com>
> ---
>  drivers/net/wireless/ath/ath10k/core.c | 30 ++++++++++++++++++++++++++
>  drivers/net/wireless/ath/ath10k/pci.c  |  2 +-
>  drivers/net/wireless/ath/ath10k/pci.h  |  2 ++
>  3 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
> index 5eb131ab916f..a776b06f49b5 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -13,6 +13,8 @@
>  #include <linux/ctype.h>
>  #include <linux/pm_qos.h>
>  #include <linux/nvmem-consumer.h>
> +#include <linux/of_pci.h>
> +#include <linux/pci.h>
>  #include <asm/byteorder.h>
>
>  #include "core.h"
> @@ -26,6 +28,7 @@
>  #include "testmode.h"
>  #include "wmi-ops.h"
>  #include "coredump.h"
> +#include "pci.h"
>
>  unsigned int ath10k_debug_mask;
>  EXPORT_SYMBOL(ath10k_debug_mask);
> @@ -1958,6 +1961,33 @@ static int ath10k_download_cal_nvmem(struct ath10k *ar, const char *cell_name)
>         size_t len;
>         int ret;
>
> +       /* devm_nvmem_cell_get() will get a cell first from the OF
> +        * DT node representing the given device with nvmem-cell-name
> +        * "calibration", and from the global lookup table as a fallback,
> +        * and an ath10k device could be either a pci one or a platform one.
> +        *
> +        * If the OF DT node is not compatible with the real device, the
> +        * calibration data got from the node should not be applied.
> +        *
> +        * dev_is_pci(ar->dev) && ( no OF node || caldata not from node
> +        * || not compatible ) -> do not use caldata .
> +        *
> +        * !dev_is_pci(ar->dev) -> always use caldata .
> +        *
> +        * The judgement for compatibility differs with ath9k for many
> +        * DT using "qcom,ath10k" as compatibility string.
> +        */
> +       if (dev_is_pci(ar->dev) &&
> +           (!ar->dev->of_node ||
> +            (of_property_match_string(ar->dev->of_node,
> +                                      "nvmem-cell-names",
> +                                      cell_name) < 0) ||
> +            !of_device_is_compatible(ar->dev->of_node,
> +                                     "qcom,ath10k") ||
> +            !of_pci_node_match_driver(ar->dev->of_node,
> +                                      &ath10k_pci_driver)))
> +               return -ENOENT;

I think this can be done a bit cleaner and like other drivers. I see 2 options.

The first way is use VID/PID compatible strings and don't set the
of_node pointer if there is a mismatch.

If you must use "qcom,ath10k" (and 9k) only, then we should make
of_device_get_match_data() work on PCI drivers. This should just
require adding of_match_table ptr and it needs a data struct with a
flag saying use cal data or not.

Upon further thought, why can't you decide all this just on PCI
VID/PID? The giant switch statement in ath10k_pci_probe() could all
just be struct of driver_data from the PCI match table.

Rob

Re: [PATCH v5 3/3] wifi: ath10k: only load compatible DT cal data

[equu]

> I think this can be done a bit cleaner and like other drivers. I see 2 options.
> The first way is use VID/PID compatible strings and don't set the
> of_node pointer if there is a mismatch.
Where should I do this? In pci_set_of_node() from drivers/pci/of.c?
> Upon further thought, why can't you decide all this just on PCI
> VID/PID? The giant switch statement in ath10k_pci_probe() could all
> just be struct of driver_data from the PCI match table.

I cannot decide all this just on PCI VID/PID because PCI VID/PID cannot tell whether calibration data are stored in the device (like most expansion cards) or not (for example, in an NVRAM cell referenced by the device tree).

Re: [PATCH v5 3/3] wifi: ath10k: only load compatible DT cal data

[Rob Herring]

On Fri, Feb 3, 2023 at 11:15 AM <equu@openmail.cc> wrote:
>
> > I think this can be done a bit cleaner and like other drivers. I see 2 options.
> > The first way is use VID/PID compatible strings and don't set the
> > of_node pointer if there is a mismatch.
> Where should I do this? In pci_set_of_node() from drivers/pci/of.c?

Off the top of my head, I think so.

> > Upon further thought, why can't you decide all this just on PCI
> > VID/PID? The giant switch statement in ath10k_pci_probe() could all
> > just be struct of driver_data from the PCI match table.
>
> I cannot decide all this just on PCI VID/PID because PCI VID/PID cannot tell whether calibration data are stored in the device (like most expansion cards) or not (for example, in an NVRAM cell referenced by the device tree).
>

For a given VID/PID, you could have calibration data in DT that you
want to ignore sometimes and not other times (because the compatible
is wrong)?

Rob

Re: [PATCH v5 3/3] wifi: ath10k: only load compatible DT cal data

[equu]

>
>>> Upon further thought, why can't you decide all this just on PCI
>>> VID/PID? The giant switch statement in ath10k_pci_probe() could all
>>> just be struct of driver_data from the PCI match table.
>> I cannot decide all this just on PCI VID/PID because PCI VID/PID cannot tell whether calibration data are stored in the device (like most expansion cards) or not (for example, in an NVRAM cell referenced by the device tree).
>>
> For a given VID/PID, you could have calibration data in DT that you
> want to ignore sometimes and not other times (because the compatible
> is wrong)?

Some devices will change their VID/PID after applied with calibration data (e.g. AR922X will do 168c:ff1d -> 168c:0029), but most device trees only record their post-calibration VID/PID in their compatibility string.

Should we match such device against their pre-calibration VID/PID only, and break all current device trees for them?

I think we could add these pre-calibration VID/PIDs to the ID-list of the PCI driver, but had better match compatibility strings against drivers, not devices.

[PATCH v6 0/3] PCI: of: Load extra data only from compatible DT nodes

[equu]

From: Edward Chow <equu@openmail.cc>

In order to solve the issue reported in
https://github.com/openwrt/openwrt/pull/11345 , this patchset attempt
to add mechanisms to ckeck whether an OF DT node is compatible to the
PCI device installed on the corresponding location or the driver for
it, and make ath9k and ath10k only load extra data from compatible
nodes.

V5 -> V6:
3, handle generic "qcom,ath10k" via of_device_get_match_data()

Signed-off-by: Edward Chow <equu@openmail.cc>
Reported-by: kernel test robot <lkp@intel.com>

Edward Chow (3):
  PCI: of: Match pci devices or drivers against OF DT nodes
  wifi: ath9k: stop loading incompatible DT cal data
  wifi: ath10k: only load compatible DT cal data

 drivers/net/wireless/ath/ath10k/core.c |  31 +++
 drivers/net/wireless/ath/ath10k/hw.h   |   4 +
 drivers/net/wireless/ath/ath10k/pci.c  |  18 +-
 drivers/net/wireless/ath/ath10k/pci.h  |   2 +
 drivers/net/wireless/ath/ath9k/ath9k.h |   1 +
 drivers/net/wireless/ath/ath9k/init.c  |  27 +++
 drivers/net/wireless/ath/ath9k/pci.c   |   2 +-
 drivers/pci/of.c                       | 299 +++++++++++++++++++++++++
 drivers/pci/pci-driver.c               |   5 -
 drivers/pci/pci.h                      |  56 +++++
 include/linux/of_pci.h                 |  25 +++
 include/linux/pci.h                    |   6 +
 12 files changed, 469 insertions(+), 7 deletions(-)

-- 
2.39.1

[PATCH v6 1/3] PCI: of: Match pci devices or drivers against OF DT nodes

[equu]

From: Edward Chow <equu@openmail.cc>

Currently, whether a compatibility string within an OF DT node for a
PCI device (whose spec is at
https://www.devicetree.org/open-firmware/bindings/pci/ ) matches the
vendor and device id of either the PCI device installed on the
corresponding location or the driver suggested by the compatibility
string is not supported.

This patch introduces a function to decode a compatibility string into
a struct pci_device_id, which could further be matched against PCI
devices or drivers, as well as functions to match a compatibility
string or OF DT node against PCI devices or drivers.

Signed-off-by: Edward Chow <equu@openmail.cc>
Cc: Bjorn Helgaas <helgaas@kernel.org>
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/pci/of.c         | 299 +++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci-driver.c |   5 -
 drivers/pci/pci.h        |  56 ++++++++
 include/linux/of_pci.h   |  25 ++++
 include/linux/pci.h      |   6 +
 5 files changed, 386 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 196834ed44fe..a9c3648cd9e9 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -13,6 +13,8 @@
 #include <linux/of_irq.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
+#include <linux/string.h>
+#include <linux/kstrtox.h>
 #include "pci.h"
 
 #ifdef CONFIG_PCI
@@ -251,6 +253,303 @@ void of_pci_check_probe_only(void)
 }
 EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
 
+/**
+ * of_pci_compat_to_device_id() - Decode an OF compatibility string into a
+ * pci_device_id structure.
+ * @compat: the compatibility string to decode, could be NULL
+ * @id: pointer to a struct pci_device_id, to store the result
+ * @rev: pointer to output revision info, PCI_ANY_ID if no revision in @compat
+ * @req_pcie: pointer to output whether @compat mandates PCIe compatibility
+ *
+ * returns 0 when success, -EINVAL when failed.
+ */
+int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       u32 *rev, u32 *req_pcie)
+{
+	union {
+		u8 u8;
+		u16 u16;
+		u32 u32;
+	} res = {0};
+	*req_pcie = 0;
+	*rev = PCI_ANY_ID;
+	if (!compat || strncasecmp(compat, "pci", 3) != 0)
+		return -EINVAL;
+	compat += 3;
+
+	if (strncasecmp(compat, "class,", 6) == 0) {
+		/* pciclass,CCSSPP */
+		compat += 6;
+		if ((strlen(compat) < 4)
+		   || kstrtouint(compat, 16, &id->class))
+			return -EINVAL;
+		if (id->class < 0x10000) {
+			id->class <<= 8;
+			id->class_mask = 0xFFFF00;
+		} else {
+			id->class_mask = PCI_ANY_ID;
+		}
+		id->vendor = PCI_ANY_ID;
+		id->device = PCI_ANY_ID;
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0;
+	}
+
+	if (strncasecmp(compat, "ex", 2) == 0) {
+		/* pciex...  */
+		*req_pcie = 1;
+		compat += 2;
+	}
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->vendor = res.u16;
+	compat = strchr(compat, ',');
+	if (!compat)
+		return -EINVAL;
+	compat++;
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->device = res.u16;
+	compat = strchr(compat, '.');
+	if (compat == NULL) {
+		/* pciVVVV,DDDD */
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0;
+	}
+
+	compat++;
+	if (strlen(compat) == 2) {
+		/* pciVVVV,DDDD.RR */
+		if (kstrtou8(compat, 16, &res.u8))
+			return -EINVAL;
+		*rev = res.u8;
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0;
+	}
+
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->subvendor = res.u16;
+	compat = strchr(compat, '.');
+	if (!compat)
+		return -EINVAL;
+	compat++;
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->subdevice = res.u16;
+	compat = strchr(compat, '.');
+	if (compat == NULL)
+		/* pciVVVV,DDDD.SSSS.ssss */
+		return 0;
+
+	compat++;
+	if (strlen(compat) == 2) {
+		/* pciVVVV,DDDD.SSSS.ssss.RR */
+		if (kstrtou8(compat, 16, &res.u8))
+			return -EINVAL;
+		*rev = res.u8;
+	}
+	return 0;
+}
+
+/**
+ * of_pci_compat_match_device() - Tell whether a PCI device structure matches
+ * a given OF compatibility string
+ * @compat: single OF compatibility string to match, could be NULL
+ * @dev the PCI device structure to match against
+ *
+ * Returns whether they match.
+ */
+bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev)
+{
+	__u32 rev = PCI_ANY_ID;
+	__u32 req_pcie = 0;
+	struct pci_device_id id = {0};
+
+	if (of_pci_compat_to_device_id(compat, &id, &rev, &req_pcie))
+		return false;
+	return pci_match_one_device(&id, dev) &&
+		(rev == PCI_ANY_ID || rev == dev->revision) &&
+		req_pcie ? dev->pcie_cap : true;
+}
+
+/**
+ * of_pci_node_match_device() - Tell whether an OF device tree node
+ * matches the given pci device
+ * @node: single OF device tree node to match, could be NULL
+ * @dev: the PCI device structure to match against, could be NULL
+ *
+ * Returns whether they match.
+ */
+bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev)
+{
+	struct property *prop;
+	const char *cp;
+
+	if (!node || !dev)
+		return false;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		if (of_pci_compat_match_device(cp, dev))
+			return true;
+	}
+	return false;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_device);
+
+/**
+ * of_pci_compat_match_one_id() - Tell whether a PCI device ID structure matches
+ * a given OF compatibility string, note that there is no revision nor PCIe
+ * capability info in PCI device ID structures
+ *
+ * @compat: single OF compatibility string to match, could be NULL
+ * @id the PCI device ID structure to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure pointed by ID
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id)
+{
+	__u32 rev = PCI_ANY_ID;
+	__u32 req_pcie = 0;
+	struct pci_device_id pr = {0};
+
+	if (!compat || !id ||
+	    of_pci_compat_to_device_id(compat, &pr, &rev, &req_pcie))
+		return NULL;
+	return pci_match_one_id(id, &pr);
+}
+
+/**
+ * of_pci_compat_match_id_table() - Tell whether a given OF compatibility string
+ * matches a given pci_id table
+ *
+ * @compat: single OF compatibility string to match, could be NULL
+ * @table the PCI device ID table to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table)
+{
+	if (compat && table) {
+		while (table->vendor || table->subvendor || table->class_mask) {
+			if (of_pci_compat_match_one_id(compat, table))
+				return table;
+			table++;
+		}
+	}
+	return NULL;
+}
+
+/**
+ * of_pci_node_match_id_table() - Tell whether an OF device tree node
+ * matches the given pci_id table
+ * @node: single OF device tree node to match, could be NULL
+ * @table: the PCI device ID table to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table)
+{
+	struct property *prop;
+	const char *cp;
+	const struct pci_device_id *id;
+
+	if (!node || !table)
+		return NULL;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		id = of_pci_compat_match_id_table(cp, table);
+		if (id)
+			return id;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_id_table);
+
+/**
+ * of_pci_compat_match_driver - See if a given OF compatibility string matches
+ * a driver's list of IDs
+ * @compat: single OF compatibility string to match, could be NULL
+ * @drv: the PCI driver to match against, could be NULL
+ *
+ * Used by a driver to check whether an OF compatibility string matches one of
+ * (dynamically) supported devices, which may have been augmented
+ * via the sysfs "new_id" file.  Returns the matching pci_device_id
+ * structure or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv)
+{
+	struct pci_dynid *dynid;
+	const struct pci_device_id *found_id = NULL, *ids;
+
+	if (!compat || !drv)
+		return NULL;
+	/* Look at the dynamic ids first, before the static ones */
+	spin_lock(&drv->dynids.lock);
+	list_for_each_entry(dynid, &drv->dynids.list, node) {
+		if (of_pci_compat_match_one_id(compat, &dynid->id)) {
+			found_id = &dynid->id;
+			break;
+		}
+	}
+	spin_unlock(&drv->dynids.lock);
+
+	if (found_id)
+		return found_id;
+
+	for (ids = drv->id_table; (found_id = of_pci_compat_match_one_id(compat, ids));
+	     ids = found_id + 1) {
+		/* exclude ids in id_table with override_only */
+		if (!found_id->override_only)
+			return found_id;
+	}
+
+	return NULL;
+}
+
+/**
+ * of_pci_node_match_driver() - Tell whether an OF device tree node
+ * matches the given pci driver
+ * @node: single OF device tree node to match, could be NULL
+ * @drv: the PCI driver structure to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv)
+{
+	struct property *prop;
+	const char *cp;
+	const struct pci_device_id *id;
+
+	if (!node || !drv)
+		return NULL;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		id = of_pci_compat_match_driver(cp, drv);
+		if (id)
+			return id;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_driver);
+
 /**
  * devm_of_pci_get_host_bridge_resources() - Resource-managed parsing of PCI
  *                                           host bridge resources from DT
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index a2ceeacc33eb..aa212d12353f 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -24,11 +24,6 @@
 #include "pci.h"
 #include "pcie/portdrv.h"
 
-struct pci_dynid {
-	struct list_head node;
-	struct pci_device_id id;
-};
-
 /**
  * pci_add_dynid - add a new PCI device ID to this driver and re-probe devices
  * @drv: target pci driver
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9ed3b5550043..e30652021a63 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -204,6 +204,29 @@ pci_match_one_device(const struct pci_device_id *id, const struct pci_dev *dev)
 	return NULL;
 }
 
+/**
+ * pci_match_one_id - Tell if a PCI device id structure matches another
+ *			  PCI device id structure
+ * @id: single PCI device id structure to match, usually in a list or array
+ * @pr: the probing PCI device id structure to match against, usually converted from
+ *      other format
+ *
+ * Returns the matching pci_device_id structure pointed by id
+ * or %NULL if there is no match.
+ */
+static inline const struct pci_device_id *
+pci_match_one_id(const struct pci_device_id *id, const struct pci_device_id *pr)
+{
+	if ((id->vendor == pr->vendor) &&
+	    (id->device == pr->device) &&
+	    (id->subvendor == pr->subvendor) &&
+	    (id->subdevice == pr->subdevice) &&
+	    (id->class == pr->class) &&
+	    (id->class_mask == pr->class_mask))
+		return id;
+	return NULL;
+}
+
 /* PCI slot sysfs helper code */
 #define to_pci_slot(s) container_of(s, struct pci_slot, kobj)
 
@@ -638,6 +661,15 @@ void pci_release_bus_of_node(struct pci_bus *bus);
 
 int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge);
 
+int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       __u32 *rev, __u32 *req_pcie);
+bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev);
+const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id);
+const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table);
+const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv);
 #else
 static inline int
 of_pci_parse_bus_range(struct device_node *node, struct resource *res)
@@ -679,6 +711,30 @@ static inline int devm_of_pci_bridge_init(struct device *dev, struct pci_host_br
 	return 0;
 }
 
+static inline int of_pci_compat_to_device_id(const char *compat, struct pci_device_id *id,
+			       __u32 *rev, __u32 *req_pcie)
+{
+	return -EINVAL;
+}
+static inline bool of_pci_compat_match_device(const char *compat, const struct pci_dev *dev)
+{
+	return false;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct pci_device_id *id)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct pci_device_id *table)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv)
+{
+	return NULL;
+}
 #endif /* CONFIG_OF */
 
 #ifdef CONFIG_PCIEAER
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 29658c0ee71f..eef1eaafc03d 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -13,6 +13,14 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn);
 int of_pci_get_devfn(struct device_node *np);
 void of_pci_check_probe_only(void);
+bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev);
+const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table);
+const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv);
 #else
 static inline struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn)
@@ -26,6 +34,23 @@ static inline int of_pci_get_devfn(struct device_node *np)
 }
 
 static inline void of_pci_check_probe_only(void) { }
+static inline bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev)
+{
+	return false;
+}
+static inline const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv)
+{
+	return NULL;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_OF_IRQ)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index adffd65e84b4..04c908d84b90 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1513,6 +1513,12 @@ void pci_unregister_driver(struct pci_driver *dev);
 	builtin_driver(__pci_driver, pci_register_driver)
 
 struct pci_driver *pci_dev_driver(const struct pci_dev *dev);
+
+struct pci_dynid {
+	struct list_head node;
+	struct pci_device_id id;
+};
+
 int pci_add_dynid(struct pci_driver *drv,
 		  unsigned int vendor, unsigned int device,
 		  unsigned int subvendor, unsigned int subdevice,
-- 
2.39.1

[PATCH v6 2/3] wifi: ath9k: stop loading incompatible DT cal data

[equu]

From: Edward Chow <equu@openmail.cc>

As reported in https://github.com/openwrt/openwrt/pull/11345 , ath9k
would load calibration data from a device tree node declared
incompatible.

Now, ath9k will first check whether the device tree node is compatible
with it, using the functionality introduced with the first patch of
this series, ("PCI: of: Match pci devices or drivers against OF DT
nodes") and only proceed loading calibration data from compatible node.

Signed-off-by: Edward Chow <equu@openmail.cc>
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/net/wireless/ath/ath9k/ath9k.h |  1 +
 drivers/net/wireless/ath/ath9k/init.c  | 27 ++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/pci.c   |  2 +-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 2cc23605c9fc..4f6f0383a5f8 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -35,6 +35,7 @@ struct ath_node;
 struct ath_vif;
 
 extern struct ieee80211_ops ath9k_ops;
+extern struct pci_driver ath_pci_driver;
 extern int ath9k_modparam_nohwcrypt;
 extern int ath9k_led_blink;
 extern bool is_ath9k_unloaded;
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 4f00400c7ffb..fbfb5b5d32c3 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -22,6 +22,8 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_net.h>
+#include <linux/of_pci.h>
+#include <linux/pci.h>
 #include <linux/nvmem-consumer.h>
 #include <linux/relay.h>
 #include <linux/dmi.h>
@@ -577,6 +579,31 @@ static int ath9k_nvmem_request_eeprom(struct ath_softc *sc)
 	size_t len;
 	int err;
 
+	/* devm_nvmem_cell_get() will get a cell first from the OF
+	 * DT node representing the given device with nvmem-cell-name
+	 * "calibration", and from the global lookup table as a fallback,
+	 * and an ath9k device could be either a pci one or a platform one.
+	 *
+	 * If the OF DT node is not compatible with the real device, the
+	 * calibration data got from the node should not be applied.
+	 *
+	 * dev_is_pci(sc->dev) && ( no OF node || caldata not from node
+	 * || not compatible ) -> do not use caldata .
+	 *
+	 * !dev_is_pci(sc->dev) -> always use caldata .
+	 */
+	if (dev_is_pci(sc->dev) &&
+	    (!sc->dev->of_node ||
+	     !of_property_match_string(sc->dev->of_node,
+				       "nvmem-cell-names",
+				       "calibration") ||
+	     !of_pci_node_match_driver(sc->dev->of_node,
+				       &ath_pci_driver)))
+		/* follow the "just return 0;" convention as
+		 * noted below.
+		 */
+		return 0;
+
 	cell = devm_nvmem_cell_get(sc->dev, "calibration");
 	if (IS_ERR(cell)) {
 		err = PTR_ERR(cell);
diff --git a/drivers/net/wireless/ath/ath9k/pci.c b/drivers/net/wireless/ath/ath9k/pci.c
index a074e23013c5..fcb19761e60d 100644
--- a/drivers/net/wireless/ath/ath9k/pci.c
+++ b/drivers/net/wireless/ath/ath9k/pci.c
@@ -1074,7 +1074,7 @@ static SIMPLE_DEV_PM_OPS(ath9k_pm_ops, ath_pci_suspend, ath_pci_resume);
 
 MODULE_DEVICE_TABLE(pci, ath_pci_id_table);
 
-static struct pci_driver ath_pci_driver = {
+struct pci_driver ath_pci_driver = {
 	.name       = "ath9k",
 	.id_table   = ath_pci_id_table,
 	.probe      = ath_pci_probe,
-- 
2.39.1

[PATCH v6 3/3] wifi: ath10k: only load compatible DT cal data

[equu]

From: Edward Chow <equu@openmail.cc>

ath10k might also be sensitive to the issue reported on
https://github.com/openwrt/openwrt/pull/11345 , loading calibration
data from a device tree node declared incompatible.

ath10k will first check whether the device tree node is compatible
with it, using the functionality introduced with the first patch of
this series, ("PCI: of: Match pci devices or drivers against OF DT
nodes") and only proceed loading calibration data from compatible node.

Signed-off-by: Edward Chow <equu@openmail.cc>
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/net/wireless/ath/ath10k/core.c | 31 ++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/hw.h   |  4 ++++
 drivers/net/wireless/ath/ath10k/pci.c  | 18 ++++++++++++++-
 drivers/net/wireless/ath/ath10k/pci.h  |  2 ++
 4 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 5eb131ab916f..4c9e8140aeff 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -13,6 +13,8 @@
 #include <linux/ctype.h>
 #include <linux/pm_qos.h>
 #include <linux/nvmem-consumer.h>
+#include <linux/of_pci.h>
+#include <linux/pci.h>
 #include <asm/byteorder.h>
 
 #include "core.h"
@@ -26,6 +28,7 @@
 #include "testmode.h"
 #include "wmi-ops.h"
 #include "coredump.h"
+#include "pci.h"
 
 unsigned int ath10k_debug_mask;
 EXPORT_SYMBOL(ath10k_debug_mask);
@@ -1958,6 +1961,34 @@ static int ath10k_download_cal_nvmem(struct ath10k *ar, const char *cell_name)
 	size_t len;
 	int ret;
 
+	/* devm_nvmem_cell_get() will get a cell first from the OF
+	 * DT node representing the given device with nvmem-cell-name
+	 * "calibration", and from the global lookup table as a fallback,
+	 * and an ath10k device could be either a pci one or a platform one.
+	 *
+	 * If the OF DT node is not compatible with the real device, the
+	 * calibration data got from the node should not be applied.
+	 *
+	 * dev_is_pci(ar->dev) && ( no OF node || caldata not from node
+	 * || not compatible ) -> do not use caldata .
+	 *
+	 * !dev_is_pci(ar->dev) -> always use caldata .
+	 *
+	 * The judgement for compatibility differs with ath9k for many
+	 * DT using "qcom,ath10k" as compatibility string.
+	 */
+	if (dev_is_pci(ar->dev) &&
+	    (!ar->dev->of_node ||
+	     (of_property_match_string(ar->dev->of_node,
+				       "nvmem-cell-names",
+				       cell_name) < 0) ||
+	     !of_device_get_match_data(ar->dev) ||
+	     !(((const struct ath10k_hw_misc_flags *)
+		of_device_get_match_data(ar->dev))->need_calibration) ||
+	     !of_pci_node_match_driver(ar->dev->of_node,
+				       &ath10k_pci_driver)))
+		return -ENOENT;
+
 	cell = devm_nvmem_cell_get(ar->dev, cell_name);
 	if (IS_ERR(cell)) {
 		ret = PTR_ERR(cell);
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 9643031a4427..8e19fa637905 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -384,6 +384,10 @@ struct ath10k_hw_values {
 	bool rfkill_on_level;
 };
 
+struct ath10k_hw_misc_flags {
+	bool need_calibration;
+};
+
 extern const struct ath10k_hw_values qca988x_values;
 extern const struct ath10k_hw_values qca6174_values;
 extern const struct ath10k_hw_values qca99x0_values;
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 728d607289c3..b6ea40631e1b 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -97,6 +97,22 @@ static const struct ath10k_pci_supp_chip ath10k_pci_supp_chips[] = {
 	{ QCA9887_1_0_DEVICE_ID, QCA9887_HW_1_0_CHIP_ID_REV },
 };
 
+static const struct ath10k_hw_misc_flags ath10k_pci_of_flag = {
+	.need_calibration = true,
+}
+
+static const struct of_device_id ath10k_pci_of_match[] = {
+	/* OF DT nodes for a generic ath10k pci devices
+	 * usually use this compatibility string.
+	 */
+	{ .compatible = "qcom,ath10k",
+	  .data = &ath10k_pci_of_flag
+	},
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, ath10k_pci_of_match);
+
 static void ath10k_pci_buffer_cleanup(struct ath10k *ar);
 static int ath10k_pci_cold_reset(struct ath10k *ar);
 static int ath10k_pci_safe_chip_reset(struct ath10k *ar);
@@ -3780,7 +3796,7 @@ static SIMPLE_DEV_PM_OPS(ath10k_pci_pm_ops,
 			 ath10k_pci_pm_suspend,
 			 ath10k_pci_pm_resume);
 
-static struct pci_driver ath10k_pci_driver = {
+struct pci_driver ath10k_pci_driver = {
 	.name = "ath10k_pci",
 	.id_table = ath10k_pci_id_table,
 	.probe = ath10k_pci_probe,
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 480cd97ab739..de676797b736 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -209,6 +209,8 @@ static inline struct ath10k_pci *ath10k_pci_priv(struct ath10k *ar)
 #define DIAG_ACCESS_CE_TIMEOUT_US 10000 /* 10 ms */
 #define DIAG_ACCESS_CE_WAIT_US	50
 
+extern struct pci_driver ath10k_pci_driver;
+
 void ath10k_pci_write32(struct ath10k *ar, u32 offset, u32 value);
 void ath10k_pci_soc_write32(struct ath10k *ar, u32 addr, u32 val);
 void ath10k_pci_reg_write32(struct ath10k *ar, u32 addr, u32 val);
-- 
2.39.1

Re: [PATCH v6 3/3] wifi: ath10k: only load compatible DT cal data

[Rob Herring]

On Wed, Feb 8, 2023 at 10:51 PM <equu@openmail.cc> wrote:
>
> From: Edward Chow <equu@openmail.cc>
>
> ath10k might also be sensitive to the issue reported on
> https://github.com/openwrt/openwrt/pull/11345 , loading calibration
> data from a device tree node declared incompatible.
>
> ath10k will first check whether the device tree node is compatible
> with it, using the functionality introduced with the first patch of
> this series, ("PCI: of: Match pci devices or drivers against OF DT
> nodes") and only proceed loading calibration data from compatible node.
>
> Signed-off-by: Edward Chow <equu@openmail.cc>
> Reported-by: kernel test robot <lkp@intel.com>

This is for fixes created as a result of kernel test robot report.
Reports on your broken patches should not have this.

> ---
>  drivers/net/wireless/ath/ath10k/core.c | 31 ++++++++++++++++++++++++++
>  drivers/net/wireless/ath/ath10k/hw.h   |  4 ++++
>  drivers/net/wireless/ath/ath10k/pci.c  | 18 ++++++++++++++-
>  drivers/net/wireless/ath/ath10k/pci.h  |  2 ++
>  4 files changed, 54 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
> index 5eb131ab916f..4c9e8140aeff 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -13,6 +13,8 @@
>  #include <linux/ctype.h>
>  #include <linux/pm_qos.h>
>  #include <linux/nvmem-consumer.h>
> +#include <linux/of_pci.h>
> +#include <linux/pci.h>
>  #include <asm/byteorder.h>
>
>  #include "core.h"
> @@ -26,6 +28,7 @@
>  #include "testmode.h"
>  #include "wmi-ops.h"
>  #include "coredump.h"
> +#include "pci.h"
>
>  unsigned int ath10k_debug_mask;
>  EXPORT_SYMBOL(ath10k_debug_mask);
> @@ -1958,6 +1961,34 @@ static int ath10k_download_cal_nvmem(struct ath10k *ar, const char *cell_name)
>         size_t len;
>         int ret;
>
> +       /* devm_nvmem_cell_get() will get a cell first from the OF
> +        * DT node representing the given device with nvmem-cell-name
> +        * "calibration", and from the global lookup table as a fallback,
> +        * and an ath10k device could be either a pci one or a platform one.
> +        *
> +        * If the OF DT node is not compatible with the real device, the
> +        * calibration data got from the node should not be applied.
> +        *
> +        * dev_is_pci(ar->dev) && ( no OF node || caldata not from node
> +        * || not compatible ) -> do not use caldata .
> +        *
> +        * !dev_is_pci(ar->dev) -> always use caldata .
> +        *
> +        * The judgement for compatibility differs with ath9k for many
> +        * DT using "qcom,ath10k" as compatibility string.
> +        */
> +       if (dev_is_pci(ar->dev) &&
> +           (!ar->dev->of_node ||
> +            (of_property_match_string(ar->dev->of_node,
> +                                      "nvmem-cell-names",
> +                                      cell_name) < 0) ||
> +            !of_device_get_match_data(ar->dev) ||
> +            !(((const struct ath10k_hw_misc_flags *)
> +               of_device_get_match_data(ar->dev))->need_calibration) ||
> +            !of_pci_node_match_driver(ar->dev->of_node,
> +                                      &ath10k_pci_driver)))

That is just plain ugly and not understandable. Why do you still need
of_pci_node_match_driver()? If compatible using VID/PID doesn't match
the actual VID/PID, then you should never probe.

The prior explanations didn't really clear things up either. I'm
really at a loss as to what are the scenarios you need to work. Please
enumerate what are the different scenarios of what's in the DTs and
how you need the kernel/driver to respond.

Rob

[PATCH 3/3] wifi: ath10k: only load compatible DT cal data

[Edward Chow]

Loading calibration data from an OF device tree node not declared
compatible with the device (e.g. a PCI device with calibration data
from corresponding DT node gets replaced, so the newly installed
device become incompatible with the node) or driver may lead to fatal
result, e.g. kernel panic.

The driver should check whether the DT node corresponding to the
device compatible with it, and load calibration data only from
compatible node.

Signed-off-by: Edward Chow <equu@openmail.cc>
---
 drivers/net/wireless/ath/ath10k/core.c | 28 ++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/pci.c  |  2 +-
 drivers/net/wireless/ath/ath10k/pci.h  |  2 ++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 5eb131ab916f..e4d7ec7f3b59 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -26,6 +26,7 @@
 #include "testmode.h"
 #include "wmi-ops.h"
 #include "coredump.h"
+#include "pci.h"
 
 unsigned int ath10k_debug_mask;
 EXPORT_SYMBOL(ath10k_debug_mask);
@@ -1958,6 +1959,33 @@ static int ath10k_download_cal_nvmem(struct ath10k *ar, const char *cell_name)
 	size_t len;
 	int ret;
 
+	/* devm_nvmem_cell_get() will get a cell first from the OF
+	 * DT node representing the given device with nvmem-cell-name
+	 * "calibration", and from the global lookup table as a fallback,
+	 * and an ath9k device could be either a pci one or a platform one.
+	 *
+	 * If the OF DT node is not compatible with the real device, the
+	 * calibration data got from the node should not be applied.
+	 *
+	 * dev_is_pci(ar->dev) && ( no OF node || caldata not from node
+	 * || not compatible ) -> do not use caldata .
+	 *
+	 * !dev_is_pci(ar->dev) -> always use caldata .
+	 *
+	 * The judgement for compatibility differs with ath9k for many
+	 * DT using "qcom,ath10k" as compatibility string.
+	 */
+	if (dev_is_pci(ar->dev) &&
+	    (!ar->dev->of_node ||
+	     (of_property_match_string(ar->dev->of_node,
+				       "nvmem-cell-names",
+				       cell_name) < 0) ||
+	     !of_device_is_compatible(ar->dev->of_node,
+				      "qcom,ath10k") ||
+	     !of_pci_node_match_device(ar->dev->of_node,
+				       &ath10k_pci_driver)))
+		return ERR_PTR(-ENOENT);
+
 	cell = devm_nvmem_cell_get(ar->dev, cell_name);
 	if (IS_ERR(cell)) {
 		ret = PTR_ERR(cell);
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 728d607289c3..5d9f6046f8cf 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -3780,7 +3780,7 @@ static SIMPLE_DEV_PM_OPS(ath10k_pci_pm_ops,
 			 ath10k_pci_pm_suspend,
 			 ath10k_pci_pm_resume);
 
-static struct pci_driver ath10k_pci_driver = {
+struct pci_driver ath10k_pci_driver = {
 	.name = "ath10k_pci",
 	.id_table = ath10k_pci_id_table,
 	.probe = ath10k_pci_probe,
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 480cd97ab739..de676797b736 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -209,6 +209,8 @@ static inline struct ath10k_pci *ath10k_pci_priv(struct ath10k *ar)
 #define DIAG_ACCESS_CE_TIMEOUT_US 10000 /* 10 ms */
 #define DIAG_ACCESS_CE_WAIT_US	50
 
+extern struct pci_driver ath10k_pci_driver;
+
 void ath10k_pci_write32(struct ath10k *ar, u32 offset, u32 value);
 void ath10k_pci_soc_write32(struct ath10k *ar, u32 addr, u32 val);
 void ath10k_pci_reg_write32(struct ath10k *ar, u32 addr, u32 val);
-- 
2.39.0

[PATCH 1/3] PCI: of: Match pci devices or drivers against OF DT nodes

[Mad Horse]

Currently, whether a compatibility string within an OF DT node for a
PCI device (whose spec is at
https://www.devicetree.org/open-firmware/bindings/pci/ ) matches the
vendor and device id of either the PCI device installed on the
corresponding location or the driver suggested by the compatibility
string is not supported.

This patch introduces a function to decode a compatibility string into
a struct pci_device_id, which could further be matched against PCI
devices or drivers, as well as functions to match a compatibility
string or OF DT node against PCI devices or drivers.

Signed-off-by: Edward Chow <equu@openmail.cc>
Cc: Bjorn Helgaas <helgaas@kernel.org>
---
drivers/pci/of.c | 299 +++++++++++++++++++++++++++++++++++++++
drivers/pci/pci-driver.c | 5 -
drivers/pci/pci.h | 56 ++++++++
include/linux/of_pci.h | 25 ++++
include/linux/pci.h | 6 +
5 files changed, 386 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 196834ed44fe..edb61195ea53 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -13,6 +13,8 @@
#include <linux/of_irq.h>
#include <linux/of_address.h>
#include <linux/of_pci.h>
+#include <linux/string.h>
+#include <linux/kstrtox.h>
#include "pci.h"
#ifdef CONFIG_PCI
@@ -251,6 +253,303 @@ void of_pci_check_probe_only(void)
}
EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
+/**
+ * of_pci_compat_to_device_id() - Decode an OF compatibility string into a
+ * pci_device_id structure.
+ * @compat: the compatibility string to decode, could be NULL
+ * @id: pointer to a struct pci_device_id, to store the result
+ * @rev: pointer to output revision info, PCI_ANY_ID if no revision in 
@compat
+ * @req_pcie: pointer to output whether @compat mandates PCIe compatibility
+ *
+ * returns 0 when success, -EINVAL when failed.
+ */
+int of_pci_compat_to_device_id(const char *compat, struct pci_device_id 
*id,
+ u32 *rev, u32 *req_pcie)
+{
+ union {
+ u8 u8;
+ u16 u16;
+ u32 u32;
+ } res = {0};
+ *req_pcie = 0;
+ *rev = PCI_ANY_ID;
+ if (!compat || strncasecmp(compat, "pci", 3) != 0)
+ return -EINVAL;
+ compat += 3;
+
+ if (strncasecmp(compat, "class,", 6) == 0) {
+ /* pciclass,CCSSPP */
+ compat += 6;
+ if ((strlen(compat) < 4)
+ || kstrtouint(compat, 16, &id->class))
+ return -EINVAL;
+ if (id->class < 0x10000) {
+ id->class <<= 8;
+ id->class_mask = 0xFFFF00;
+ } else {
+ id->class_mask = PCI_ANY_ID;
+ }
+ id->vendor = PCI_ANY_ID;
+ id->device = PCI_ANY_ID;
+ id->subvendor = PCI_ANY_ID;
+ id->subdevice = PCI_ANY_ID;
+ return 0
+ }
+
+ if (strncasecmp(compat, "ex", 2) == 0) {
+ /* pciex... */
+ *req_pcie = 1;
+ compat += 2;
+ }
+ if (kstrtou16(compat, 16, &res.u16))
+ return -EINVAL;
+ id->vendor = res.u16;
+ compat = strchr(compat, ',');
+ if (!compat)
+ return -EINVAL;
+ compat++;
+ if (kstrtou16(compat, 16, &res.u16))
+ return -EINVAL;
+ id->device = res.u16;
+ compat = strchr(compat, '.');
+ if (compat == NULL) {
+ /* pciVVVV,DDDD */
+ id->subvendor = PCI_ANY_ID;
+ id->subdevice = PCI_ANY_ID;
+ return 0
+ }
+
+ compat++;
+ if (strlen(compat) == 2) {
+ /* pciVVVV,DDDD.RR */
+ if (kstrtou8(compat, 16, &res.u8))
+ return -EINVAL;
+ *rev = res.u8;
+ id->subvendor = PCI_ANY_ID;
+ id->subdevice = PCI_ANY_ID;
+ return 0
+ }
+
+ if (kstrtou16(compat, 16, &res.u16))
+ return -EINVAL;
+ id->subvendor = res.u16;
+ compat = strchr(compat, '.');
+ if (!compat)
+ return -EINVAL;
+ compat++;
+ if (kstrtou16(compat, 16, &res.u16))
+ return -EINVAL;
+ id->subdevice = res.u16;
+ compat = strchr(compat, '.');
+ if (compat == NULL)
+ /* pciVVVV,DDDD.SSSS.ssss */
+ return 0
+
+ compat++;
+ if (strlen(compat) == 2) {
+ /* pciVVVV,DDDD.SSSS.ssss.RR */
+ if (kstrtou8(compat, 16, &res.u8))
+ return -EINVAL;
+ *rev = res.u8;
+ }
+ return 0;
+}
+
+/**
+ * of_pci_compat_match_device() - Tell whether a PCI device structure 
matches
+ * a given OF compatibility string
+ * @compat: single OF compatibility string to match, could be NULL
+ * @dev the PCI device structure to match against
+ *
+ * Returns whether they match.
+ */
+bool of_pci_compat_match_device(const char *compat, const struct 
pci_dev *dev)
+{
+ __u32 rev = PCI_ANY_ID;
+ __u32 req_pcie = 0;
+ struct pci_device_id id = {0};
+
+ if (of_pci_compat_to_device_id(compat, &id, &rev, &req_pcie))
+ return false;
+ return pci_match_one_device(&id, dev) &&
+ (rev == PCI_ANY_ID || rev == dev->revision) &&
+ req_pcie ? dev->pcie_cap : true;
+}
+
+/**
+ * of_pci_node_match_device() - Tell whether an OF device tree node
+ * matches the given pci device
+ * @node: single OF device tree node to match, could be NULL
+ * @dev: the PCI device structure to match against, could be NULL
+ *
+ * Returns whether they match.
+ */
+bool of_pci_node_match_device(const struct device_node *node,
+ const struct pci_dev *dev)
+{
+ struct property *prop;
+ const char *cp;
+
+ if (!node || !dev)
+ return false;
+ prop = of_find_property(node, "compatible", NULL);
+ for (cp = of_prop_next_string(prop, NULL); cp;
+ cp = of_prop_next_string(prop, cp)) {
+ if (of_pci_compat_match_device(cp, dev))
+ return true;
+ }
+ return false;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_device);
+
+/**
+ * of_pci_compat_match_one_id() - Tell whether a PCI device ID 
structure matches
+ * a given OF compatibility string, note that there is no revision nor PCIe
+ * capability info in PCI device ID structures
+ *
+ * @compat: single OF compatibility string to match, could be NULL
+ * @id the PCI device ID structure to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure pointed by ID
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct 
pci_device_id *id)
+{
+ __u32 rev = PCI_ANY_ID;
+ __u32 req_pcie = 0;
+ struct pci_device_id pr = {0};
+
+ if (!compat || !id ||
+ of_pci_compat_to_device_id(compat, &pr, &rev, &req_pcie))
+ return NULL;
+ return pci_match_one_id(id, &pr);
+}
+
+/**
+ * of_pci_compat_match_id_table() - Tell whether a given OF 
compatibility string
+ * matches a given pci_id table
+ *
+ * @compat: single OF compatibility string to match, could be NULL
+ * @table the PCI device ID table to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure or %NULL if there is no 
match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct 
pci_device_id *table)
+{
+ if (compat && table) {
+ while (table->vendor || table->subvendor || table->class_mask) {
+ if (of_pci_compat_match_one_id(compat, table))
+ return table;
+ table++;
+ }
+ }
+ return NULL;
+}
+
+/**
+ * of_pci_node_match_id_table() - Tell whether an OF device tree node
+ * matches the given pci_id table
+ * @node: single OF device tree node to match, could be NULL
+ * @table: the PCI device ID table to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+ const struct pci_device_id *table)
+{
+ struct property *prop;
+ const char *cp;
+ const struct pci_device_id *id;
+
+ if (!node || !table)
+ return NULL;
+ prop = of_find_property(node, "compatible", NULL);
+ for (cp = of_prop_next_string(prop, NULL); cp;
+ cp = of_prop_next_string(prop, cp)) {
+ id = of_pci_compat_match_id_table(cp, table);
+ if (id)
+ return id;
+ }
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_id_table);
+
+/**
+ * of_pci_compat_match_driver - See if a given OF compatibility string 
matches
+ * a driver's list of IDs
+ * @compat: single OF compatibility string to match, could be NULL
+ * @drv: the PCI driver to match against, could be NULL
+ *
+ * Used by a driver to check whether an OF compatibility string matches 
one of
+ * (dynamically) supported devices, which may have been augmented
+ * via the sysfs "new_id" file. Returns the matching pci_device_id
+ * structure or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv)
+{
+ struct pci_dynid *dynid;
+ const struct pci_device_id *found_id = NULL, *ids;
+
+ if (!compat || !drv)
+ return NULL;
+ /* Look at the dynamic ids first, before the static ones */
+ spin_lock(&drv->dynids.lock);
+ list_for_each_entry(dynid, &drv->dynids.list, node) {
+ if (of_pci_compat_match_one_id(compat, &dynid->id)) {
+ found_id = &dynid->id;
+ break;
+ }
+ }
+ spin_unlock(&drv->dynids.lock);
+
+ if (found_id)
+ return found_id;
+
+ for (ids = drv->id_table; (found_id = 
of_pci_compat_match_one_id(compat, ids));
+ ids = found_id + 1) {
+ /* exclude ids in id_table with override_only */
+ if (!found_id->override_only)
+ return found_id;
+ }
+
+ return NULL;
+}
+
+/**
+ * of_pci_node_match_driver() - Tell whether an OF device tree node
+ * matches the given pci driver
+ * @node: single OF device tree node to match, could be NULL
+ * @drv: the PCI driver structure to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+ struct pci_driver *drv)
+{
+ struct property *prop;
+ const char *cp;
+ const struct pci_device_id *id;
+
+ if (!node || !drv)
+ return NULL;
+ prop = of_find_property(node, "compatible", NULL);
+ for (cp = of_prop_next_string(prop, NULL); cp;
+ cp = of_prop_next_string(prop, cp)) {
+ id = of_pci_compat_match_driver(cp, drv);
+ if (id)
+ return id;
+ }
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_driver);
+
/**
* devm_of_pci_get_host_bridge_resources() - Resource-managed parsing of PCI
* host bridge resources from DT
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index a2ceeacc33eb..aa212d12353f 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -24,11 +24,6 @@
#include "pci.h"
#include "pcie/portdrv.h"
-struct pci_dynid {
- struct list_head node;
- struct pci_device_id id;
-};
-
/**
* pci_add_dynid - add a new PCI device ID to this driver and re-probe 
devices
* @drv: target pci driver
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9ed3b5550043..e30652021a63 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -204,6 +204,29 @@ pci_match_one_device(const struct pci_device_id 
*id, const struct pci_dev *dev)
return NULL;
}
+/**
+ * pci_match_one_id - Tell if a PCI device id structure matches another
+ * PCI device id structure
+ * @id: single PCI device id structure to match, usually in a list or array
+ * @pr: the probing PCI device id structure to match against, usually 
converted from
+ * other format
+ *
+ * Returns the matching pci_device_id structure pointed by id
+ * or %NULL if there is no match.
+ */
+static inline const struct pci_device_id *
+pci_match_one_id(const struct pci_device_id *id, const struct 
pci_device_id *pr)
+{
+ if ((id->vendor == pr->vendor) &&
+ (id->device == pr->device) &&
+ (id->subvendor == pr->subvendor) &&
+ (id->subdevice == pr->subdevice) &&
+ (id->class == pr->class) &&
+ (id->class_mask == pr->class_mask))
+ return id;
+ return NULL;
+}
+
/* PCI slot sysfs helper code */
#define to_pci_slot(s) container_of(s, struct pci_slot, kobj)
@@ -638,6 +661,15 @@ void pci_release_bus_of_node(struct pci_bus *bus);
int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge 
*bridge);
+int of_pci_compat_to_device_id(const char *compat, struct pci_device_id 
*id,
+ __u32 *rev, __u32 *req_pcie);
+bool of_pci_compat_match_device(const char *compat, const struct 
pci_dev *dev);
+const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct 
pci_device_id *id);
+const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct 
pci_device_id *table);
+const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv);
#else
static inline int
of_pci_parse_bus_range(struct device_node *node, struct resource *res)
@@ -679,6 +711,30 @@ static inline int devm_of_pci_bridge_init(struct 
device *dev, struct pci_host_br
return 0;
}
+static inline int of_pci_compat_to_device_id(const char *compat, struct 
pci_device_id *id,
+ __u32 *rev, __u32 *req_pcie)
+{
+ return -EINVAL;
+}
+static inline bool of_pci_compat_match_device(const char *compat, const 
struct pci_dev *dev)
+{
+ return false;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct 
pci_device_id *id)
+{
+ return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct 
pci_device_id *table)
+{
+ return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv)
+{
+ return NULL;
+}
#endif /* CONFIG_OF */
#ifdef CONFIG_PCIEAER
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 29658c0ee71f..eef1eaafc03d 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -13,6 +13,14 @@ struct device_node *of_pci_find_child_device(struct 
device_node *parent,
unsigned int devfn);
int of_pci_get_devfn(struct device_node *np);
void of_pci_check_probe_only(void);
+bool of_pci_node_match_device(const struct device_node *node,
+ const struct pci_dev *dev);
+const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+ const struct pci_device_id *table);
+const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+ struct pci_driver *drv);
#else
static inline struct device_node *of_pci_find_child_device(struct 
device_node *parent,
unsigned int devfn)
@@ -26,6 +34,23 @@ static inline int of_pci_get_devfn(struct device_node 
*np)
}
static inline void of_pci_check_probe_only(void) { }
+static inline bool of_pci_node_match_device(const struct device_node *node,
+ const struct pci_dev *dev)
+{
+ return false;
+}
+static inline const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+ const struct pci_device_id *table)
+{
+ return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+ struct pci_driver *drv)
+{
+ return NULL;
+}
#endif
#if IS_ENABLED(CONFIG_OF_IRQ)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index adffd65e84b4..04c908d84b90 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1513,6 +1513,12 @@ void pci_unregister_driver(struct pci_driver *dev);
builtin_driver(__pci_driver, pci_register_driver)
struct pci_driver *pci_dev_driver(const struct pci_dev *dev);
+
+struct pci_dynid {
+ struct list_head node;
+ struct pci_device_id id;
+};
+
int pci_add_dynid(struct pci_driver *drv,
unsigned int vendor, unsigned int device,
unsigned int subvendor, unsigned int subdevice,

-- 
2.39.0

[PATCH 2/3] wifi: ath9k: stop loading incompatible DT cal data

[Mad Horse]

Loading calibration data from an OF device tree node not declared
compatible with the device (e.g. a PCI device with calibration data
from corresponding DT node gets replaced, so the newly installed
device become incompatible with the node) or driver may lead to fatal
result, e.g. kernel panic.

The driver should check whether the DT node corresponding to the
device compatible with it, and load calibration data only from
compatible node.

Signed-off-by: Edward Chow <equu@openmail.cc>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 1 +
drivers/net/wireless/ath/ath9k/init.c | 26 ++++++++++++++++++++++++++
drivers/net/wireless/ath/ath9k/pci.c | 2 +-
3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h 
b/drivers/net/wireless/ath/ath9k/ath9k.h
index 2cc23605c9fc..4f6f0383a5f8 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -35,6 +35,7 @@ struct ath_node;
struct ath_vif;
extern struct ieee80211_ops ath9k_ops;
+extern struct pci_driver ath_pci_driver;
extern int ath9k_modparam_nohwcrypt;
extern int ath9k_led_blink;
extern bool is_ath9k_unloaded;
diff --git a/drivers/net/wireless/ath/ath9k/init.c 
b/drivers/net/wireless/ath/ath9k/init.c
index 4f00400c7ffb..f88a48e8456b 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -22,6 +22,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_net.h>
+#include <linux/pci.h>
#include <linux/nvmem-consumer.h>
#include <linux/relay.h>
#include <linux/dmi.h>
@@ -577,6 +578,31 @@ static int ath9k_nvmem_request_eeprom(struct 
ath_softc *sc)
size_t len;
int err;
+ /* devm_nvmem_cell_get() will get a cell first from the OF
+ * DT node representing the given device with nvmem-cell-name
+ * "calibration", and from the global lookup table as a fallback,
+ * and an ath9k device could be either a pci one or a platform one.
+ *
+ * If the OF DT node is not compatible with the real device, the
+ * calibration data got from the node should not be applied.
+ *
+ * dev_is_pci(sc->dev) && ( no OF node || caldata not from node
+ * || not compatible ) -> do not use caldata .
+ *
+ * !dev_is_pci(sc->dev) -> always use caldata .
+ */
+ if (dev_is_pci(sc->dev) &&
+ (!sc->dev->of_node ||
+ !of_property_match_string(sc->dev->of_node,
+ "nvmem-cell-names",
+ "calibration") ||
+ !of_pci_node_match_driver(sc->dev->of_node,
+ &ath_pci_driver)))
+ /* follow the "just return 0;" convention as
+ * noted below.
+ */
+ return 0;
+
cell = devm_nvmem_cell_get(sc->dev, "calibration");
if (IS_ERR(cell)) {
err = PTR_ERR(cell);
diff --git a/drivers/net/wireless/ath/ath9k/pci.c 
b/drivers/net/wireless/ath/ath9k/pci.c
index a074e23013c5..fcb19761e60d 100644
--- a/drivers/net/wireless/ath/ath9k/pci.c
+++ b/drivers/net/wireless/ath/ath9k/pci.c
@@ -1074,7 +1074,7 @@ static SIMPLE_DEV_PM_OPS(ath9k_pm_ops, 
ath_pci_suspend, ath_pci_resume);
MODULE_DEVICE_TABLE(pci, ath_pci_id_table);
-static struct pci_driver ath_pci_driver = {
+struct pci_driver ath_pci_driver = {
.name = "ath9k",
.id_table = ath_pci_id_table,
.probe = ath_pci_probe,

-- 
2.39.0

[PATCH 3/3] wifi: ath10k: only load compatible DT cal data

[Mad Horse]

Loading calibration data from an OF device tree node not declared
compatible with the device (e.g. a PCI device with calibration data
from corresponding DT node gets replaced, so the newly installed
device become incompatible with the node) or driver may lead to fatal
result, e.g. kernel panic.

The driver should check whether the DT node corresponding to the
device compatible with it, and load calibration data only from
compatible node.

Signed-off-by: Edward Chow <equu@openmail.cc>
---
drivers/net/wireless/ath/ath10k/core.c | 28 ++++++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/pci.c | 2 +-
drivers/net/wireless/ath/ath10k/pci.h | 2 ++
3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c 
b/drivers/net/wireless/ath/ath10k/core.c
index 5eb131ab916f..e4d7ec7f3b59 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -26,6 +26,7 @@
#include "testmode.h"
#include "wmi-ops.h"
#include "coredump.h"
+#include "pci.h"
unsigned int ath10k_debug_mask;
EXPORT_SYMBOL(ath10k_debug_mask);
@@ -1958,6 +1959,33 @@ static int ath10k_download_cal_nvmem(struct 
ath10k *ar, const char *cell_name)
size_t len;
int ret;
+ /* devm_nvmem_cell_get() will get a cell first from the OF
+ * DT node representing the given device with nvmem-cell-name
+ * "calibration", and from the global lookup table as a fallback,
+ * and an ath9k device could be either a pci one or a platform one.
+ *
+ * If the OF DT node is not compatible with the real device, the
+ * calibration data got from the node should not be applied.
+ *
+ * dev_is_pci(ar->dev) && ( no OF node || caldata not from node
+ * || not compatible ) -> do not use caldata .
+ *
+ * !dev_is_pci(ar->dev) -> always use caldata .
+ *
+ * The judgement for compatibility differs with ath9k for many
+ * DT using "qcom,ath10k" as compatibility string.
+ */
+ if (dev_is_pci(ar->dev) &&
+ (!ar->dev->of_node ||
+ (of_property_match_string(ar->dev->of_node,
+ "nvmem-cell-names",
+ cell_name) < 0) ||
+ !of_device_is_compatible(ar->dev->of_node,
+ "qcom,ath10k") ||
+ !of_pci_node_match_device(ar->dev->of_node,
+ &ath10k_pci_driver)))
+ return ERR_PTR(-ENOENT);
+
cell = devm_nvmem_cell_get(ar->dev, cell_name);
if (IS_ERR(cell)) {
ret = PTR_ERR(cell);
diff --git a/drivers/net/wireless/ath/ath10k/pci.c 
b/drivers/net/wireless/ath/ath10k/pci.c
index 728d607289c3..5d9f6046f8cf 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -3780,7 +3780,7 @@ static SIMPLE_DEV_PM_OPS(ath10k_pci_pm_ops,
ath10k_pci_pm_suspend,
ath10k_pci_pm_resume);
-static struct pci_driver ath10k_pci_driver = {
+struct pci_driver ath10k_pci_driver = {
.name = "ath10k_pci",
.id_table = ath10k_pci_id_table,
.probe = ath10k_pci_probe,
diff --git a/drivers/net/wireless/ath/ath10k/pci.h 
b/drivers/net/wireless/ath/ath10k/pci.h
index 480cd97ab739..de676797b736 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -209,6 +209,8 @@ static inline struct ath10k_pci 
*ath10k_pci_priv(struct ath10k *ar)
#define DIAG_ACCESS_CE_TIMEOUT_US 10000 /* 10 ms */
#define DIAG_ACCESS_CE_WAIT_US 50
+extern struct pci_driver ath10k_pci_driver;
+
void ath10k_pci_write32(struct ath10k *ar, u32 offset, u32 value);
void ath10k_pci_soc_write32(struct ath10k *ar, u32 addr, u32 val);
void ath10k_pci_reg_write32(struct ath10k *ar, u32 addr, u32 val);

-- 
2.39.0

[PATCH 1/3] PCI: of: Match pci devices or drivers against OF DT nodes

[Mad Horse]

Currently, whether a compatibility string within an OF DT node for a
PCI device (whose spec is at
https://www.devicetree.org/open-firmware/bindings/pci/ ) matches the
vendor and device id of either the PCI device installed on the
corresponding location or the driver suggested by the compatibility
string is not supported.

This patch introduces a function to decode a compatibility string into
a struct pci_device_id, which could further be matched against PCI
devices or drivers, as well as functions to match a compatibility
string or OF DT node against PCI devices or drivers.

Signed-off-by: Edward Chow <equu@openmail.cc>
Cc: Bjorn Helgaas <helgaas@kernel.org>
---
 drivers/pci/of.c         | 299 +++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci-driver.c |   5 -
 drivers/pci/pci.h        |  56 ++++++++
 include/linux/of_pci.h   |  25 ++++
 include/linux/pci.h      |   6 +
 5 files changed, 386 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 196834ed44fe..edb61195ea53 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -13,6 +13,8 @@
 #include <linux/of_irq.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
+#include <linux/string.h>
+#include <linux/kstrtox.h>
 #include "pci.h"
  #ifdef CONFIG_PCI
@@ -251,6 +253,303 @@ void of_pci_check_probe_only(void)
 }
 EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
 +/**
+ * of_pci_compat_to_device_id() - Decode an OF compatibility string into a
+ * pci_device_id structure.
+ * @compat: the compatibility string to decode, could be NULL
+ * @id: pointer to a struct pci_device_id, to store the result
+ * @rev: pointer to output revision info, PCI_ANY_ID if no revision in
@compat
+ * @req_pcie: pointer to output whether @compat mandates PCIe compatibility
+ *
+ * returns 0 when success, -EINVAL when failed.
+ */
+int of_pci_compat_to_device_id(const char *compat, struct pci_device_id
*id,
+			       u32 *rev, u32 *req_pcie)
+{
+	union {
+		u8 u8;
+		u16 u16;
+		u32 u32;
+	} res = {0};
+	*req_pcie = 0;
+	*rev = PCI_ANY_ID;
+	if (!compat || strncasecmp(compat, "pci", 3) != 0)
+		return -EINVAL;
+	compat += 3;
+
+	if (strncasecmp(compat, "class,", 6) == 0) {
+		/* pciclass,CCSSPP */
+		compat += 6;
+		if ((strlen(compat) < 4)
+		   || kstrtouint(compat, 16, &id->class))
+			return -EINVAL;
+		if (id->class < 0x10000) {
+			id->class <<= 8;
+			id->class_mask = 0xFFFF00;
+		} else {
+			id->class_mask = PCI_ANY_ID;
+		}
+		id->vendor = PCI_ANY_ID;
+		id->device = PCI_ANY_ID;
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0
+	}
+
+	if (strncasecmp(compat, "ex", 2) == 0) {
+		/* pciex...  */
+		*req_pcie = 1;
+		compat += 2;
+	}
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->vendor = res.u16;
+	compat = strchr(compat, ',');
+	if (!compat)
+		return -EINVAL;
+	compat++;
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->device = res.u16;
+	compat = strchr(compat, '.');
+	if (compat == NULL) {
+		/* pciVVVV,DDDD */
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0
+	}
+
+	compat++;
+	if (strlen(compat) == 2) {
+		/* pciVVVV,DDDD.RR */
+		if (kstrtou8(compat, 16, &res.u8))
+			return -EINVAL;
+		*rev = res.u8;
+		id->subvendor = PCI_ANY_ID;
+		id->subdevice = PCI_ANY_ID;
+		return 0
+	}
+
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->subvendor = res.u16;
+	compat = strchr(compat, '.');
+	if (!compat)
+		return -EINVAL;
+	compat++;
+	if (kstrtou16(compat, 16, &res.u16))
+		return -EINVAL;
+	id->subdevice = res.u16;
+	compat = strchr(compat, '.');
+	if (compat == NULL)
+		/* pciVVVV,DDDD.SSSS.ssss */
+		return 0
+
+	compat++;
+	if (strlen(compat) == 2) {
+		/* pciVVVV,DDDD.SSSS.ssss.RR */
+		if (kstrtou8(compat, 16, &res.u8))
+			return -EINVAL;
+		*rev = res.u8;
+	}
+	return 0;
+}
+
+/**
+ * of_pci_compat_match_device() - Tell whether a PCI device structure
matches
+ * a given OF compatibility string
+ * @compat: single OF compatibility string to match, could be NULL
+ * @dev the PCI device structure to match against
+ *
+ * Returns whether they match.
+ */
+bool of_pci_compat_match_device(const char *compat, const struct
pci_dev *dev)
+{
+	__u32 rev = PCI_ANY_ID;
+	__u32 req_pcie = 0;
+	struct pci_device_id id = {0};
+
+	if (of_pci_compat_to_device_id(compat, &id, &rev, &req_pcie))
+		return false;
+	return pci_match_one_device(&id, dev) &&
+		(rev == PCI_ANY_ID || rev == dev->revision) &&
+		req_pcie ? dev->pcie_cap : true;
+}
+
+/**
+ * of_pci_node_match_device() - Tell whether an OF device tree node
+ * matches the given pci device
+ * @node: single OF device tree node to match, could be NULL
+ * @dev: the PCI device structure to match against, could be NULL
+ *
+ * Returns whether they match.
+ */
+bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev)
+{
+	struct property *prop;
+	const char *cp;
+
+	if (!node || !dev)
+		return false;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		if (of_pci_compat_match_device(cp, dev))
+			return true;
+	}
+	return false;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_device);
+
+/**
+ * of_pci_compat_match_one_id() - Tell whether a PCI device ID
structure matches
+ * a given OF compatibility string, note that there is no revision nor PCIe
+ * capability info in PCI device ID structures
+ *
+ * @compat: single OF compatibility string to match, could be NULL
+ * @id the PCI device ID structure to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure pointed by ID
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct
pci_device_id *id)
+{
+	__u32 rev = PCI_ANY_ID;
+	__u32 req_pcie = 0;
+	struct pci_device_id pr = {0};
+
+	if (!compat || !id ||
+	    of_pci_compat_to_device_id(compat, &pr, &rev, &req_pcie))
+		return NULL;
+	return pci_match_one_id(id, &pr);
+}
+
+/**
+ * of_pci_compat_match_id_table() - Tell whether a given OF
compatibility string
+ * matches a given pci_id table
+ *
+ * @compat: single OF compatibility string to match, could be NULL
+ * @table the PCI device ID table to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure or %NULL if there is no
match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct
pci_device_id *table)
+{
+	if (compat && table) {
+		while (table->vendor || table->subvendor || table->class_mask) {
+			if (of_pci_compat_match_one_id(compat, table))
+				return table;
+			table++;
+		}
+	}
+	return NULL;
+}
+
+/**
+ * of_pci_node_match_id_table() - Tell whether an OF device tree node
+ * matches the given pci_id table
+ * @node: single OF device tree node to match, could be NULL
+ * @table: the PCI device ID table to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table)
+{
+	struct property *prop;
+	const char *cp;
+	const struct pci_device_id *id;
+
+	if (!node || !table)
+		return NULL;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		id = of_pci_compat_match_id_table(cp, table);
+		if (id)
+			return id;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_id_table);
+
+/**
+ * of_pci_compat_match_driver - See if a given OF compatibility string
matches
+ * a driver's list of IDs
+ * @compat: single OF compatibility string to match, could be NULL
+ * @drv: the PCI driver to match against, could be NULL
+ *
+ * Used by a driver to check whether an OF compatibility string matches
one of
+ * (dynamically) supported devices, which may have been augmented
+ * via the sysfs "new_id" file.  Returns the matching pci_device_id
+ * structure or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv)
+{
+	struct pci_dynid *dynid;
+	const struct pci_device_id *found_id = NULL, *ids;
+
+	if (!compat || !drv)
+		return NULL;
+	/* Look at the dynamic ids first, before the static ones */
+	spin_lock(&drv->dynids.lock);
+	list_for_each_entry(dynid, &drv->dynids.list, node) {
+		if (of_pci_compat_match_one_id(compat, &dynid->id)) {
+			found_id = &dynid->id;
+			break;
+		}
+	}
+	spin_unlock(&drv->dynids.lock);
+
+	if (found_id)
+		return found_id;
+
+	for (ids = drv->id_table; (found_id =
of_pci_compat_match_one_id(compat, ids));
+	     ids = found_id + 1) {
+		/* exclude ids in id_table with override_only */
+		if (!found_id->override_only)
+			return found_id;
+	}
+
+	return NULL;
+}
+
+/**
+ * of_pci_node_match_driver() - Tell whether an OF device tree node
+ * matches the given pci driver
+ * @node: single OF device tree node to match, could be NULL
+ * @drv: the PCI driver structure to match against, could be NULL
+ *
+ * Returns the matching pci_device_id structure
+ * or %NULL if there is no match.
+ */
+const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv)
+{
+	struct property *prop;
+	const char *cp;
+	const struct pci_device_id *id;
+
+	if (!node || !drv)
+		return NULL;
+	prop = of_find_property(node, "compatible", NULL);
+	for (cp = of_prop_next_string(prop, NULL); cp;
+	     cp = of_prop_next_string(prop, cp)) {
+		id = of_pci_compat_match_driver(cp, drv);
+		if (id)
+			return id;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_node_match_driver);
+
 /**
  * devm_of_pci_get_host_bridge_resources() - Resource-managed parsing
of PCI
  *                                           host bridge resources from DT
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index a2ceeacc33eb..aa212d12353f 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -24,11 +24,6 @@
 #include "pci.h"
 #include "pcie/portdrv.h"
 -struct pci_dynid {
-	struct list_head node;
-	struct pci_device_id id;
-};
-
 /**
  * pci_add_dynid - add a new PCI device ID to this driver and re-probe
devices
  * @drv: target pci driver
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9ed3b5550043..e30652021a63 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -204,6 +204,29 @@ pci_match_one_device(const struct pci_device_id
*id, const struct pci_dev *dev)
 	return NULL;
 }
 +/**
+ * pci_match_one_id - Tell if a PCI device id structure matches another
+ *			  PCI device id structure
+ * @id: single PCI device id structure to match, usually in a list or array
+ * @pr: the probing PCI device id structure to match against, usually
converted from
+ *      other format
+ *
+ * Returns the matching pci_device_id structure pointed by id
+ * or %NULL if there is no match.
+ */
+static inline const struct pci_device_id *
+pci_match_one_id(const struct pci_device_id *id, const struct
pci_device_id *pr)
+{
+	if ((id->vendor == pr->vendor) &&
+	    (id->device == pr->device) &&
+	    (id->subvendor == pr->subvendor) &&
+	    (id->subdevice == pr->subdevice) &&
+	    (id->class == pr->class) &&
+	    (id->class_mask == pr->class_mask))
+		return id;
+	return NULL;
+}
+
 /* PCI slot sysfs helper code */
 #define to_pci_slot(s) container_of(s, struct pci_slot, kobj)
 @@ -638,6 +661,15 @@ void pci_release_bus_of_node(struct pci_bus *bus);
  int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge
*bridge);
 +int of_pci_compat_to_device_id(const char *compat, struct
pci_device_id *id,
+			       __u32 *rev, __u32 *req_pcie);
+bool of_pci_compat_match_device(const char *compat, const struct
pci_dev *dev);
+const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct
pci_device_id *id);
+const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct
pci_device_id *table);
+const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv);
 #else
 static inline int
 of_pci_parse_bus_range(struct device_node *node, struct resource *res)
@@ -679,6 +711,30 @@ static inline int devm_of_pci_bridge_init(struct
device *dev, struct pci_host_br
 	return 0;
 }
 +static inline int of_pci_compat_to_device_id(const char *compat,
struct pci_device_id *id,
+			       __u32 *rev, __u32 *req_pcie)
+{
+	return -EINVAL;
+}
+static inline bool of_pci_compat_match_device(const char *compat, const
struct pci_dev *dev)
+{
+	return false;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_one_id(const char *compat, const struct
pci_device_id *id)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_id_table(const char *compat, const struct
pci_device_id *table)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_compat_match_driver(const char *compat, struct pci_driver *drv)
+{
+	return NULL;
+}
 #endif /* CONFIG_OF */
  #ifdef CONFIG_PCIEAER
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 29658c0ee71f..eef1eaafc03d 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -13,6 +13,14 @@ struct device_node *of_pci_find_child_device(struct
device_node *parent,
 					     unsigned int devfn);
 int of_pci_get_devfn(struct device_node *np);
 void of_pci_check_probe_only(void);
+bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev);
+const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table);
+const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv);
 #else
 static inline struct device_node *of_pci_find_child_device(struct
device_node *parent,
 					     unsigned int devfn)
@@ -26,6 +34,23 @@ static inline int of_pci_get_devfn(struct device_node
*np)
 }
  static inline void of_pci_check_probe_only(void) { }
+static inline bool of_pci_node_match_device(const struct device_node *node,
+			      const struct pci_dev *dev)
+{
+	return false;
+}
+static inline const struct pci_device_id *
+of_pci_node_match_id_table(const struct device_node *node,
+			   const struct pci_device_id *table)
+{
+	return NULL;
+}
+static inline const struct pci_device_id *
+of_pci_node_match_driver(const struct device_node *node,
+			 struct pci_driver *drv)
+{
+	return NULL;
+}
 #endif
  #if IS_ENABLED(CONFIG_OF_IRQ)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index adffd65e84b4..04c908d84b90 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1513,6 +1513,12 @@ void pci_unregister_driver(struct pci_driver *dev);
 	builtin_driver(__pci_driver, pci_register_driver)
  struct pci_driver *pci_dev_driver(const struct pci_dev *dev);
+
+struct pci_dynid {
+	struct list_head node;
+	struct pci_device_id id;
+};
+
 int pci_add_dynid(struct pci_driver *drv,
 		  unsigned int vendor, unsigned int device,
 		  unsigned int subvendor, unsigned int subdevice,
-- 
2.39.0

[PATCH 2/3] wifi: ath9k: stop loading incompatible DT cal data

[persmule]

From: Edward Chow <equu@openmail.cc>

As reported in https://github.com/openwrt/openwrt/pull/11345 , ath9k
would load calibration data from a device tree node declared
incompatible.

Now, ath9k will first check whether the device tree node is compatible
with it, using the functionality introduced with the first patch of
this series, ("PCI: of: Match pci devices or drivers against OF DT
nodes") and only proceed loading calibration data from compatible node.

Signed-off-by: Edward Chow <equu@openmail.cc>
---
 drivers/net/wireless/ath/ath9k/ath9k.h |  1 +
 drivers/net/wireless/ath/ath9k/init.c  | 26 ++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/pci.c   |  2 +-
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 2cc23605c9fc..4f6f0383a5f8 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -35,6 +35,7 @@ struct ath_node;
 struct ath_vif;
 
 extern struct ieee80211_ops ath9k_ops;
+extern struct pci_driver ath_pci_driver;
 extern int ath9k_modparam_nohwcrypt;
 extern int ath9k_led_blink;
 extern bool is_ath9k_unloaded;
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 4f00400c7ffb..f88a48e8456b 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -22,6 +22,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_net.h>
+#include <linux/pci.h>
 #include <linux/nvmem-consumer.h>
 #include <linux/relay.h>
 #include <linux/dmi.h>
@@ -577,6 +578,31 @@ static int ath9k_nvmem_request_eeprom(struct ath_softc *sc)
 	size_t len;
 	int err;
 
+	/* devm_nvmem_cell_get() will get a cell first from the OF
+	 * DT node representing the given device with nvmem-cell-name
+	 * "calibration", and from the global lookup table as a fallback,
+	 * and an ath9k device could be either a pci one or a platform one.
+	 *
+	 * If the OF DT node is not compatible with the real device, the
+	 * calibration data got from the node should not be applied.
+	 *
+	 * dev_is_pci(sc->dev) && ( no OF node || caldata not from node
+	 * || not compatible ) -> do not use caldata .
+	 *
+	 * !dev_is_pci(sc->dev) -> always use caldata .
+	 */
+	if (dev_is_pci(sc->dev) &&
+	    (!sc->dev->of_node ||
+	     !of_property_match_string(sc->dev->of_node,
+				       "nvmem-cell-names",
+				       "calibration") ||
+	     !of_pci_node_match_driver(sc->dev->of_node,
+				       &ath_pci_driver)))
+		/* follow the "just return 0;" convention as
+		 * noted below.
+		 */
+		return 0;
+
 	cell = devm_nvmem_cell_get(sc->dev, "calibration");
 	if (IS_ERR(cell)) {
 		err = PTR_ERR(cell);
diff --git a/drivers/net/wireless/ath/ath9k/pci.c b/drivers/net/wireless/ath/ath9k/pci.c
index a074e23013c5..fcb19761e60d 100644
--- a/drivers/net/wireless/ath/ath9k/pci.c
+++ b/drivers/net/wireless/ath/ath9k/pci.c
@@ -1074,7 +1074,7 @@ static SIMPLE_DEV_PM_OPS(ath9k_pm_ops, ath_pci_suspend, ath_pci_resume);
 
 MODULE_DEVICE_TABLE(pci, ath_pci_id_table);
 
-static struct pci_driver ath_pci_driver = {
+struct pci_driver ath_pci_driver = {
 	.name       = "ath9k",
 	.id_table   = ath_pci_id_table,
 	.probe      = ath_pci_probe,
-- 
2.39.0

[PATCH 3/3] wifi: ath10k: only load compatible DT cal data

[persmule]

From: Edward Chow <equu@openmail.cc>

ath10k might also be sensitive to the issue reported on
https://github.com/openwrt/openwrt/pull/11345 , loading calibration
data from a device tree node declared incompatible.

ath10k will first check whether the device tree node is compatible
with it, using the functionality introduced with the first patch of
this series, ("PCI: of: Match pci devices or drivers against OF DT
nodes") and only proceed loading calibration data from compatible node.

Signed-off-by: Edward Chow <equu@openmail.cc>
---
 drivers/net/wireless/ath/ath10k/core.c | 28 ++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/pci.c  |  2 +-
 drivers/net/wireless/ath/ath10k/pci.h  |  2 ++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 5eb131ab916f..e4d7ec7f3b59 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -26,6 +26,7 @@
 #include "testmode.h"
 #include "wmi-ops.h"
 #include "coredump.h"
+#include "pci.h"
 
 unsigned int ath10k_debug_mask;
 EXPORT_SYMBOL(ath10k_debug_mask);
@@ -1958,6 +1959,33 @@ static int ath10k_download_cal_nvmem(struct ath10k *ar, const char *cell_name)
 	size_t len;
 	int ret;
 
+	/* devm_nvmem_cell_get() will get a cell first from the OF
+	 * DT node representing the given device with nvmem-cell-name
+	 * "calibration", and from the global lookup table as a fallback,
+	 * and an ath9k device could be either a pci one or a platform one.
+	 *
+	 * If the OF DT node is not compatible with the real device, the
+	 * calibration data got from the node should not be applied.
+	 *
+	 * dev_is_pci(ar->dev) && ( no OF node || caldata not from node
+	 * || not compatible ) -> do not use caldata .
+	 *
+	 * !dev_is_pci(ar->dev) -> always use caldata .
+	 *
+	 * The judgement for compatibility differs with ath9k for many
+	 * DT using "qcom,ath10k" as compatibility string.
+	 */
+	if (dev_is_pci(ar->dev) &&
+	    (!ar->dev->of_node ||
+	     (of_property_match_string(ar->dev->of_node,
+				       "nvmem-cell-names",
+				       cell_name) < 0) ||
+	     !of_device_is_compatible(ar->dev->of_node,
+				      "qcom,ath10k") ||
+	     !of_pci_node_match_device(ar->dev->of_node,
+				       &ath10k_pci_driver)))
+		return ERR_PTR(-ENOENT);
+
 	cell = devm_nvmem_cell_get(ar->dev, cell_name);
 	if (IS_ERR(cell)) {
 		ret = PTR_ERR(cell);
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 728d607289c3..5d9f6046f8cf 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -3780,7 +3780,7 @@ static SIMPLE_DEV_PM_OPS(ath10k_pci_pm_ops,
 			 ath10k_pci_pm_suspend,
 			 ath10k_pci_pm_resume);
 
-static struct pci_driver ath10k_pci_driver = {
+struct pci_driver ath10k_pci_driver = {
 	.name = "ath10k_pci",
 	.id_table = ath10k_pci_id_table,
 	.probe = ath10k_pci_probe,
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 480cd97ab739..de676797b736 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -209,6 +209,8 @@ static inline struct ath10k_pci *ath10k_pci_priv(struct ath10k *ar)
 #define DIAG_ACCESS_CE_TIMEOUT_US 10000 /* 10 ms */
 #define DIAG_ACCESS_CE_WAIT_US	50
 
+extern struct pci_driver ath10k_pci_driver;
+
 void ath10k_pci_write32(struct ath10k *ar, u32 offset, u32 value);
 void ath10k_pci_soc_write32(struct ath10k *ar, u32 addr, u32 val);
 void ath10k_pci_reg_write32(struct ath10k *ar, u32 addr, u32 val);
-- 
2.39.0