Messages in this thread | | | Date | Tue, 30 Nov 2021 08:28:32 +0100 | From | Krzysztof Wilczyński <> | Subject | Re: [PATCH v2] PCI: qcom: Use __be16 for catching cpu_to_be16() return instead of u16 |
| |
Hi Manivannan,
Thank you for sending the patch over! Much appreciated!
A small nitpick, thus feel free to ignore it, of course: if I may, I would suggest the following subject:
PCI: qcom: Use __be16 type to store return value from cpu_to_be16()
Or something along the lines.
> cpu_to_be16() returns __be16 value but the driver uses u16 and that's > incorrect. Fix it by using __be16 as the datatype of bdf_be variable.
It would be "data type" in the above.
Not really a requirement to do so, but you could include the actual warning, as sometimes this is useful for reference later, as per:
drivers/pci/controller/dwc/pcie-qcom.c:1346:30: warning: incorrect type in initializer (different base types) drivers/pci/controller/dwc/pcie-qcom.c:1346:30: expected unsigned short [usertype] bdf_be drivers/pci/controller/dwc/pcie-qcom.c:1346:30: got restricted __be16 [usertype]
> @@ -1343,7 +1343,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;
Thank you!
Reviewed-by: Krzysztof Wilczyński <kw@linux.com>
Also, since I have your attention, it seems we have a number of unused macros in the qcom driver, as per:
drivers/pci/controller/dwc/pcie-qcom.c:#define PCIE20_PARF_BDF_TRANSLATE_CFG 0x24C drivers/pci/controller/dwc/pcie-qcom.c:#define PCIE20_PARF_SID_OFFSET 0x234 drivers/pci/controller/dwc/pcie-qcom.c:#define PCIE20_PARF_SLV_ADDR_SPACE_SIZE 0x16C
And also in the qcom-ep driver, as per:
drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_INT_ALL_BRIDGE_FLUSH_N BIT(12) drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_INT_ALL_MHI_A7 BIT(7) drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_INT_ALL_MHI_Q6 BIT(6) drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_SLV_ADDR_SPACE_SIZE 0x358 drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_INT_ALL_PLS_PME BIT(17) drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_SLV_ADDR_MSB_CTRL 0x2c0 drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_INT_ALL_DEBUG BIT(4) drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_INT_ALL_L1SUB_TIMEOUT BIT(9) drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_DBI_BASE_ADDR_HI 0x354 drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_ATU_BASE_ADDR 0x634 drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_SLV_ADDR_SPACE_SIZE_HI 0x35c drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_INT_ALL_LTR BIT(5) drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_DBI_BASE_ADDR 0x350 drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_SRIS_MODE 0x644 drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_INT_ALL_MMIO_WRITE BIT(10) drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_INT_ALL_PLS_ERR BIT(15) drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_INT_ALL_AER_LEGACY BIT(14) drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_INT_ALL_CFG_WRITE BIT(11) drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_ATU_BASE_ADDR_HI 0x638 drivers/pci/controller/dwc/pcie-qcom-ep.c:#define PARF_INT_ALL_PME_LEGACY BIT(16)
Are these needed, or would it be fine to drop these?
Krzysztof
| |