* [PATCH] PCI: qcom: Fix warning generated due to the incorrect data type @ 2021-11-24 14:04 Manivannan Sadhasivam 2021-11-30 3:36 ` Bjorn Helgaas 0 siblings, 1 reply; 6+ messages in thread From: Manivannan Sadhasivam @ 2021-11-24 14:04 UTC (permalink / raw) To: lorenzo.pieralisi, bhelgaas Cc: svarbanov, bjorn.andersson, robh, linux-pci, linux-arm-msm, linux-kernel, Manivannan Sadhasivam, kernel test robot Fix the below sparse warning due to the use of incorrect initializer data type (u16) for bdf_be variable that receives the return value of cpu_to_be16(). The correct type should be __be16. sparse warnings: (new ones prefixed by >>) >> drivers/pci/controller/dwc/pcie-qcom.c:1305:30: sparse: sparse: incorrect type in initializer (different base types) @@ expected unsigned short [usertype] bdf_be @@ got restricted __be16 [usertype] @@ drivers/pci/controller/dwc/pcie-qcom.c:1305:30: sparse: expected unsigned short [usertype] bdf_be drivers/pci/controller/dwc/pcie-qcom.c:1305:30: sparse: got restricted __be16 [usertype] Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- drivers/pci/controller/dwc/pcie-qcom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index 8a7a300163e5..6c3b034e9946 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -1312,7 +1312,7 @@ static int qcom_pcie_config_sid_sm8250(struct qcom_pcie *pcie) /* Look for an available entry to hold the mapping */ for (i = 0; i < nr_map; i++) { - u16 bdf_be = cpu_to_be16(map[i].bdf); + __be16 bdf_be = cpu_to_be16(map[i].bdf); u32 val; u8 hash; -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI: qcom: Fix warning generated due to the incorrect data type 2021-11-24 14:04 [PATCH] PCI: qcom: Fix warning generated due to the incorrect data type Manivannan Sadhasivam @ 2021-11-30 3:36 ` Bjorn Helgaas 2021-11-30 6:21 ` Manivannan Sadhasivam 0 siblings, 1 reply; 6+ messages in thread From: Bjorn Helgaas @ 2021-11-30 3:36 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: lorenzo.pieralisi, bhelgaas, svarbanov, bjorn.andersson, robh, linux-pci, linux-arm-msm, linux-kernel, kernel test robot On Wed, Nov 24, 2021 at 07:34:24PM +0530, Manivannan Sadhasivam wrote: > Fix the below sparse warning due to the use of incorrect initializer > data type (u16) for bdf_be variable that receives the return value of > cpu_to_be16(). The correct type should be __be16. I think the patch looks OK, but the reason to change this is not to "fix the warning". The reason is to fix the *problem*, i.e., cpu_to_be16() returns "__be16", which is incompatible with "u16". The warning is only a helpful hint, and should not be part of the subject line. "cpu_to_be16" or "iommu-map" would be much more useful information in the subject. I'm also curious why pcie-qcom.c is the only driver that does this. "iommu-map" is not specific to qcom, but no other drivers do similar things with it. > sparse warnings: (new ones prefixed by >>) > >> drivers/pci/controller/dwc/pcie-qcom.c:1305:30: sparse: sparse: incorrect type in initializer (different base types) @@ expected unsigned short [usertype] bdf_be @@ got restricted __be16 [usertype] @@ > drivers/pci/controller/dwc/pcie-qcom.c:1305:30: sparse: expected unsigned short [usertype] bdf_be > drivers/pci/controller/dwc/pcie-qcom.c:1305:30: sparse: got restricted __be16 [usertype] > > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > drivers/pci/controller/dwc/pcie-qcom.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > index 8a7a300163e5..6c3b034e9946 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom.c > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > @@ -1312,7 +1312,7 @@ static int qcom_pcie_config_sid_sm8250(struct qcom_pcie *pcie) > > /* Look for an available entry to hold the mapping */ > for (i = 0; i < nr_map; i++) { > - u16 bdf_be = cpu_to_be16(map[i].bdf); > + __be16 bdf_be = cpu_to_be16(map[i].bdf); > u32 val; > u8 hash; > > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI: qcom: Fix warning generated due to the incorrect data type 2021-11-30 3:36 ` Bjorn Helgaas @ 2021-11-30 6:21 ` Manivannan Sadhasivam 2021-11-30 16:03 ` Bjorn Helgaas 0 siblings, 1 reply; 6+ messages in thread From: Manivannan Sadhasivam @ 2021-11-30 6:21 UTC (permalink / raw) To: Bjorn Helgaas Cc: lorenzo.pieralisi, bhelgaas, svarbanov, bjorn.andersson, robh, linux-pci, linux-arm-msm, linux-kernel, kernel test robot Hi Bjorn, On Mon, Nov 29, 2021 at 09:36:14PM -0600, Bjorn Helgaas wrote: > On Wed, Nov 24, 2021 at 07:34:24PM +0530, Manivannan Sadhasivam wrote: > > Fix the below sparse warning due to the use of incorrect initializer > > data type (u16) for bdf_be variable that receives the return value of > > cpu_to_be16(). The correct type should be __be16. > > I think the patch looks OK, but the reason to change this is not to > "fix the warning". The reason is to fix the *problem*, i.e., > cpu_to_be16() returns "__be16", which is incompatible with "u16". > > The warning is only a helpful hint, and should not be part of the > subject line. "cpu_to_be16" or "iommu-map" would be much more > useful information in the subject. > okay > I'm also curious why pcie-qcom.c is the only driver that does this. > "iommu-map" is not specific to qcom, but no other drivers do similar > things with it. > Yes, on the recent qcom platforms starting from sm8250 we need to program the BDF to SID mapping in the controller and that's the reason we are extracting the "iommu-map" property in DT. Thanks, Mani > > sparse warnings: (new ones prefixed by >>) > > >> drivers/pci/controller/dwc/pcie-qcom.c:1305:30: sparse: sparse: incorrect type in initializer (different base types) @@ expected unsigned short [usertype] bdf_be @@ got restricted __be16 [usertype] @@ > > drivers/pci/controller/dwc/pcie-qcom.c:1305:30: sparse: expected unsigned short [usertype] bdf_be > > drivers/pci/controller/dwc/pcie-qcom.c:1305:30: sparse: got restricted __be16 [usertype] > > > > Reported-by: kernel test robot <lkp@intel.com> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > drivers/pci/controller/dwc/pcie-qcom.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > > index 8a7a300163e5..6c3b034e9946 100644 > > --- a/drivers/pci/controller/dwc/pcie-qcom.c > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > > @@ -1312,7 +1312,7 @@ static int qcom_pcie_config_sid_sm8250(struct qcom_pcie *pcie) > > > > /* Look for an available entry to hold the mapping */ > > for (i = 0; i < nr_map; i++) { > > - u16 bdf_be = cpu_to_be16(map[i].bdf); > > + __be16 bdf_be = cpu_to_be16(map[i].bdf); > > u32 val; > > u8 hash; > > > > -- > > 2.25.1 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI: qcom: Fix warning generated due to the incorrect data type 2021-11-30 6:21 ` Manivannan Sadhasivam @ 2021-11-30 16:03 ` Bjorn Helgaas 2021-11-30 16:35 ` Sven Peter 2021-11-30 18:35 ` Marc Zyngier 0 siblings, 2 replies; 6+ messages in thread From: Bjorn Helgaas @ 2021-11-30 16:03 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: lorenzo.pieralisi, bhelgaas, svarbanov, bjorn.andersson, robh, linux-pci, linux-arm-msm, linux-kernel, kernel test robot, Marc Zyngier, Alyssa Rosenzweig, Sven Peter [+cc Marc, Alyssa, Sven for RID-to-SID mapping insight. The patch at https://lore.kernel.org/all/20211130062137.GD205712@thinkpad/ merely fixes a warning. My meta-question is about the qcom BDF-to-SID mapping.] On Tue, Nov 30, 2021 at 11:51:37AM +0530, Manivannan Sadhasivam wrote: > On Mon, Nov 29, 2021 at 09:36:14PM -0600, Bjorn Helgaas wrote: > > ... > > I'm also curious why pcie-qcom.c is the only driver that does this. > > "iommu-map" is not specific to qcom, but no other drivers do similar > > things with it. > > Yes, on the recent qcom platforms starting from sm8250 we need to program > the BDF to SID mapping in the controller and that's the reason we are > extracting the "iommu-map" property in DT. This sounds like something that may not really be specific to sm8250. It looks vaguely similar to apple_pcie_add_device(). Compare the qcom code at [1] with the Apple code at [2]. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/dwc/pcie-qcom.c?id=v5.16-rc1#n1308 [2] https://git.kernel.org/linus/468c8d52c332 > > > sparse warnings: (new ones prefixed by >>) > > > >> drivers/pci/controller/dwc/pcie-qcom.c:1305:30: sparse: sparse: incorrect type in initializer (different base types) @@ expected unsigned short [usertype] bdf_be @@ got restricted __be16 [usertype] @@ > > > drivers/pci/controller/dwc/pcie-qcom.c:1305:30: sparse: expected unsigned short [usertype] bdf_be > > > drivers/pci/controller/dwc/pcie-qcom.c:1305:30: sparse: got restricted __be16 [usertype] > > > > > > Reported-by: kernel test robot <lkp@intel.com> > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > > --- > > > drivers/pci/controller/dwc/pcie-qcom.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > > > index 8a7a300163e5..6c3b034e9946 100644 > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > > > @@ -1312,7 +1312,7 @@ static int qcom_pcie_config_sid_sm8250(struct qcom_pcie *pcie) > > > > > > /* Look for an available entry to hold the mapping */ > > > for (i = 0; i < nr_map; i++) { > > > - u16 bdf_be = cpu_to_be16(map[i].bdf); > > > + __be16 bdf_be = cpu_to_be16(map[i].bdf); > > > u32 val; > > > u8 hash; > > > > > > -- > > > 2.25.1 > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI: qcom: Fix warning generated due to the incorrect data type 2021-11-30 16:03 ` Bjorn Helgaas @ 2021-11-30 16:35 ` Sven Peter 2021-11-30 18:35 ` Marc Zyngier 1 sibling, 0 replies; 6+ messages in thread From: Sven Peter @ 2021-11-30 16:35 UTC (permalink / raw) To: Bjorn Helgaas, Manivannan Sadhasivam Cc: Lorenzo Pieralisi, Bjorn Helgaas, svarbanov, bjorn.andersson, Rob Herring, linux-pci, linux-arm-msm, linux-kernel, kernel test robot, Marc Zyngier, Alyssa Rosenzweig Hi, On Tue, Nov 30, 2021, at 17:03, Bjorn Helgaas wrote: > [+cc Marc, Alyssa, Sven for RID-to-SID mapping insight. The patch at > https://lore.kernel.org/all/20211130062137.GD205712@thinkpad/ merely > fixes a warning. My meta-question is about the qcom BDF-to-SID > mapping.] > > On Tue, Nov 30, 2021 at 11:51:37AM +0530, Manivannan Sadhasivam wrote: >> On Mon, Nov 29, 2021 at 09:36:14PM -0600, Bjorn Helgaas wrote: >> > ... >> > I'm also curious why pcie-qcom.c is the only driver that does this. >> > "iommu-map" is not specific to qcom, but no other drivers do similar >> > things with it. >> >> Yes, on the recent qcom platforms starting from sm8250 we need to program >> the BDF to SID mapping in the controller and that's the reason we are >> extracting the "iommu-map" property in DT. > > This sounds like something that may not really be specific to sm8250. So a single IOMMU can possibly differentiate between N different devices [1]. Each device [1] is identified by some number which is called sid (stream id? security id? who knows.) on Apple hardware (and apparently also on qcom). Now I don't know much about PCI but the way I understand it is that the bus/device/function tuple can be used to uniquely identify a single device on the bus. All iommu-map does is to provide the mapping between those two different spaces [2]. For most iommus this seems to be just a static mapping that's hardwired in silicon and I think that's why almost no PCI driver needs to care about it: The iommu core will just use it to convert the PCI requester ID to a number the iommu driver understands. Apple's frankenchip however allows to configure this mapping from software after a device has been attached and that's why we need that special code inside the PCI driver: We have to make sure that whatever is configured inside iommu-map (and used by the iommu core to match PCI devices to iommu groups) matches to what the HW does. I can only assume that qcom does something similar. It looks like the qcom HW can be fully configured during probe time though while we really have to wait until a device is attached for the Apple chip (mostly because we only have 16 slots there). Hope that helps. Best, Sven [1] Technically the smallest unit are iommu groups which can contain multiple devices but we can just ignore that. The iommu code will do the correct thing if it gets told that two PCI devices have the same identification number. [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/of_iommu.c#n53 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI: qcom: Fix warning generated due to the incorrect data type 2021-11-30 16:03 ` Bjorn Helgaas 2021-11-30 16:35 ` Sven Peter @ 2021-11-30 18:35 ` Marc Zyngier 1 sibling, 0 replies; 6+ messages in thread From: Marc Zyngier @ 2021-11-30 18:35 UTC (permalink / raw) To: Bjorn Helgaas Cc: Manivannan Sadhasivam, lorenzo.pieralisi, bhelgaas, svarbanov, bjorn.andersson, robh, linux-pci, linux-arm-msm, linux-kernel, kernel test robot, Alyssa Rosenzweig, Sven Peter On Tue, 30 Nov 2021 16:03:38 +0000, Bjorn Helgaas <helgaas@kernel.org> wrote: > > [+cc Marc, Alyssa, Sven for RID-to-SID mapping insight. The patch at > https://lore.kernel.org/all/20211130062137.GD205712@thinkpad/ merely > fixes a warning. My meta-question is about the qcom BDF-to-SID > mapping.] > > On Tue, Nov 30, 2021 at 11:51:37AM +0530, Manivannan Sadhasivam wrote: > > On Mon, Nov 29, 2021 at 09:36:14PM -0600, Bjorn Helgaas wrote: > > > ... > > > I'm also curious why pcie-qcom.c is the only driver that does this. > > > "iommu-map" is not specific to qcom, but no other drivers do similar > > > things with it. > > > > Yes, on the recent qcom platforms starting from sm8250 we need to program > > the BDF to SID mapping in the controller and that's the reason we are > > extracting the "iommu-map" property in DT. > > This sounds like something that may not really be specific to sm8250. > > It looks vaguely similar to apple_pcie_add_device(). Compare the qcom > code at [1] with the Apple code at [2]. It looks indeed similar in spirit, though the implementation seems different. The qcom code seems to brute-force the mappings upfront, while the apple driver relies on bus notifiers to map things on demand. The annoying thing with these blocks is that they are neither part of the IOMMU nor the PCIe block. It is just a piece of glue logic in the middle... Thanks, M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-30 18:35 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-24 14:04 [PATCH] PCI: qcom: Fix warning generated due to the incorrect data type Manivannan Sadhasivam 2021-11-30 3:36 ` Bjorn Helgaas 2021-11-30 6:21 ` Manivannan Sadhasivam 2021-11-30 16:03 ` Bjorn Helgaas 2021-11-30 16:35 ` Sven Peter 2021-11-30 18:35 ` Marc Zyngier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).