From: Rob Herring <robh@kernel.org> To: equu@openmail.cc Cc: lpieralisi@kernel.org, toke@toke.dk, kvalo@kernel.org, linux-pci@vger.kernel.org, linux-wireless@vger.kernel.org, ath10k@lists.infradead.org, kernel test robot <lkp@intel.com> Subject: Re: [PATCH v6 3/3] wifi: ath10k: only load compatible DT cal data Date: Thu, 9 Feb 2023 10:09:08 -0600 [thread overview] Message-ID: <CAL_JsqKTRqaROZh416TBMmfEpYLbfa3ejwhe8+ryDecPthQ6Ew@mail.gmail.com> (raw) In-Reply-To: <20230209045026.1806587-4-equu@openmail.cc> 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
WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org> To: equu@openmail.cc Cc: lpieralisi@kernel.org, toke@toke.dk, kvalo@kernel.org, linux-pci@vger.kernel.org, linux-wireless@vger.kernel.org, ath10k@lists.infradead.org, kernel test robot <lkp@intel.com> Subject: Re: [PATCH v6 3/3] wifi: ath10k: only load compatible DT cal data Date: Thu, 9 Feb 2023 10:09:08 -0600 [thread overview] Message-ID: <CAL_JsqKTRqaROZh416TBMmfEpYLbfa3ejwhe8+ryDecPthQ6Ew@mail.gmail.com> (raw) In-Reply-To: <20230209045026.1806587-4-equu@openmail.cc> 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 _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
next prev parent reply other threads:[~2023-02-09 16:09 UTC|newest] Thread overview: 84+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-01-07 15:14 [PATCH] pci: Add functions to match pci dev or driver against OF DT node Mad Horse 2023-01-10 15:24 ` Bjorn Helgaas 2023-01-17 9:27 ` [PATCH 1/3] PCI: of: Match pci devices or drivers against OF DT nodes Edward Chow 2023-01-17 9:27 ` Edward Chow 2023-01-17 9:27 ` [PATCH 2/3] wifi: ath9k: stop loading incompatible DT cal data Edward Chow 2023-01-17 9:27 ` Edward Chow 2023-01-17 19:46 ` Bjorn Helgaas 2023-01-17 19:46 ` Bjorn Helgaas 2023-02-01 3:02 ` Mad Horse 2023-02-01 3:02 ` Mad Horse 2023-02-01 21:43 ` Bjorn Helgaas 2023-02-01 21:43 ` Bjorn Helgaas 2023-02-02 4:18 ` [PATCH v2 0/3] PCI: of: Load extra data only from compatible DT nodes equu 2023-02-02 4:18 ` equu 2023-02-02 4:18 ` [PATCH v2 1/3] PCI: of: Match pci devices or drivers against OF " equu 2023-02-02 4:18 ` equu 2023-02-02 4:18 ` [PATCH v2 2/3] wifi: ath9k: stop loading incompatible DT cal data equu 2023-02-02 4:18 ` equu 2023-02-02 4:18 ` [PATCH v2 3/3] wifi: ath10k: only load compatible " equu 2023-02-02 4:18 ` equu 2023-02-02 7:26 ` kernel test robot 2023-02-02 7:26 ` kernel test robot 2023-02-02 7:55 ` [PATCH v3 0/3] PCI: of: Load extra data only from compatible DT nodes equu 2023-02-02 7:55 ` equu 2023-02-02 7:55 ` [PATCH v3 1/3] PCI: of: Match pci devices or drivers against OF " equu 2023-02-02 7:55 ` equu 2023-02-03 8:23 ` kernel test robot 2023-02-03 8:23 ` kernel test robot 2023-02-02 7:55 ` [PATCH v3 2/3] wifi: ath9k: stop loading incompatible DT cal data equu 2023-02-02 7:55 ` equu 2023-02-03 9:56 ` kernel test robot 2023-02-03 9:56 ` kernel test robot 2023-02-02 7:55 ` [PATCH v3 3/3] wifi: ath10k: only load compatible " equu 2023-02-02 7:55 ` equu 2023-02-03 11:38 ` kernel test robot 2023-02-03 11:38 ` kernel test robot 2023-02-03 8:37 ` [PATCH v4 0/3] PCI: of: Load extra data only from compatible DT nodes equu 2023-02-03 8:37 ` equu 2023-02-03 8:37 ` [PATCH v4 1/3] PCI: of: Match pci devices or drivers against OF " equu 2023-02-03 8:37 ` equu 2023-02-03 8:37 ` [PATCH v4 2/3] wifi: ath9k: stop loading incompatible DT cal data equu 2023-02-03 8:37 ` equu 2023-02-03 8:37 ` [PATCH v4 3/3] wifi: ath10k: only load compatible " equu 2023-02-03 8:37 ` equu 2023-02-03 10:48 ` [PATCH v5 0/3] PCI: of: Load extra data only from compatible DT nodes equu 2023-02-03 10:48 ` equu 2023-02-03 10:48 ` [PATCH v5 1/3] PCI: of: Match pci devices or drivers against OF " equu 2023-02-03 10:48 ` equu 2023-02-03 10:48 ` [PATCH v5 2/3] wifi: ath9k: stop loading incompatible DT cal data equu 2023-02-03 10:48 ` equu 2023-02-03 10:48 ` [PATCH v5 3/3] wifi: ath10k: only load compatible " equu 2023-02-03 10:48 ` equu 2023-02-03 15:57 ` Rob Herring 2023-02-03 15:57 ` Rob Herring 2023-02-03 17:15 ` equu 2023-02-03 17:15 ` equu 2023-02-03 18:45 ` Rob Herring 2023-02-03 18:45 ` Rob Herring 2023-02-04 4:26 ` equu 2023-02-04 4:26 ` equu 2023-02-09 4:50 ` [PATCH v6 0/3] PCI: of: Load extra data only from compatible DT nodes equu 2023-02-09 4:50 ` equu 2023-02-09 4:50 ` [PATCH v6 1/3] PCI: of: Match pci devices or drivers against OF " equu 2023-02-09 4:50 ` equu 2023-02-09 4:50 ` [PATCH v6 2/3] wifi: ath9k: stop loading incompatible DT cal data equu 2023-02-09 4:50 ` equu 2023-02-09 4:50 ` [PATCH v6 3/3] wifi: ath10k: only load compatible " equu 2023-02-09 4:50 ` equu 2023-02-09 16:09 ` Rob Herring [this message] 2023-02-09 16:09 ` Rob Herring 2023-01-17 9:28 ` [PATCH " Edward Chow 2023-01-17 9:28 ` Edward Chow 2023-01-17 10:01 ` [PATCH 1/3] PCI: of: Match pci devices or drivers against OF DT nodes Mad Horse 2023-01-17 10:01 ` Mad Horse 2023-01-17 10:02 ` [PATCH 2/3] wifi: ath9k: stop loading incompatible DT cal data Mad Horse 2023-01-17 10:02 ` Mad Horse 2023-01-17 10:02 ` [PATCH 3/3] wifi: ath10k: only load compatible " Mad Horse 2023-01-17 10:02 ` Mad Horse 2023-01-17 10:29 ` [PATCH 1/3] PCI: of: Match pci devices or drivers against OF DT nodes Mad Horse 2023-01-17 10:29 ` Mad Horse 2023-01-21 10:00 ` [PATCH 2/3] wifi: ath9k: stop loading incompatible DT cal data persmule 2023-01-21 10:00 ` persmule 2023-01-21 10:06 ` [PATCH 3/3] wifi: ath10k: only load compatible " persmule 2023-01-21 10:06 ` persmule
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAL_JsqKTRqaROZh416TBMmfEpYLbfa3ejwhe8+ryDecPthQ6Ew@mail.gmail.com \ --to=robh@kernel.org \ --cc=ath10k@lists.infradead.org \ --cc=equu@openmail.cc \ --cc=kvalo@kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=linux-wireless@vger.kernel.org \ --cc=lkp@intel.com \ --cc=lpieralisi@kernel.org \ --cc=toke@toke.dk \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.