diff mbox series

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

Message ID 20230202075524.2911058-3-equu@openmail.cc (mailing list archive)
State Superseded
Delegated to: Toke Høiland-Jørgensen
Headers show
Series [v3,1/3] PCI: of: Match pci devices or drivers against OF DT nodes | expand

Commit Message

Mad Horse Feb. 2, 2023, 7:55 a.m. UTC
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(-)

Comments

kernel test robot Feb. 3, 2023, 9:56 a.m. UTC | #1
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
diff mbox series

Patch

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,