| Message ID | 20240327180234.1529164-1-helgaas@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 30d2462a8bf190e702082a1d743503c75f43d8fd |
| Headers | show |
| Series | PCI: Update pci_find_capability() stub return values | expand |
On Wed, Mar 27, 2024 at 01:02:34PM -0500, Bjorn Helgaas wrote: > From: Bjorn Helgaas <bhelgaas@google.com> > > f646c2a0a668 ("PCI: Return u8 from pci_find_capability() and similar") and > ee8b1c478a9f ("PCI: Return u16 from pci_find_ext_capability() and similar") > updated the return type of the extern declarations, but neglected to update > the type of the stubs used CONFIG_PCI is not enabled. > > Update them to match the extern declarations. > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Provisionally applied to pci/enumeration for v6.10, let me know if you see any issues. > --- > include/linux/pci.h | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index b19992a5dfaf..6a09bd9636d5 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -2011,10 +2011,9 @@ static inline int pci_register_driver(struct pci_driver *drv) > static inline void pci_unregister_driver(struct pci_driver *drv) { } > static inline u8 pci_find_capability(struct pci_dev *dev, int cap) > { return 0; } > -static inline int pci_find_next_capability(struct pci_dev *dev, u8 post, > - int cap) > +static inline u8 pci_find_next_capability(struct pci_dev *dev, u8 post, int cap) > { return 0; } > -static inline int pci_find_ext_capability(struct pci_dev *dev, int cap) > +static inline u16 pci_find_ext_capability(struct pci_dev *dev, int cap) > { return 0; } > > static inline u64 pci_get_dsn(struct pci_dev *dev) > -- > 2.34.1 >
On 3/27/24 11:02 AM, Bjorn Helgaas wrote: > From: Bjorn Helgaas <bhelgaas@google.com> > > f646c2a0a668 ("PCI: Return u8 from pci_find_capability() and similar") and > ee8b1c478a9f ("PCI: Return u16 from pci_find_ext_capability() and similar") > updated the return type of the extern declarations, but neglected to update > the type of the stubs used CONFIG_PCI is not enabled. > > Update them to match the extern declarations. > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > --- This change looks fine to me. Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> But the callers of these functions still seems to use int declaration to store the output. Any reason for not changing them? Like the usages in drivers/pci/pci.c? > include/linux/pci.h | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index b19992a5dfaf..6a09bd9636d5 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -2011,10 +2011,9 @@ static inline int pci_register_driver(struct pci_driver *drv) > static inline void pci_unregister_driver(struct pci_driver *drv) { } > static inline u8 pci_find_capability(struct pci_dev *dev, int cap) > { return 0; } > -static inline int pci_find_next_capability(struct pci_dev *dev, u8 post, > - int cap) > +static inline u8 pci_find_next_capability(struct pci_dev *dev, u8 post, int cap) > { return 0; } > -static inline int pci_find_ext_capability(struct pci_dev *dev, int cap) > +static inline u16 pci_find_ext_capability(struct pci_dev *dev, int cap) > { return 0; } > > static inline u64 pci_get_dsn(struct pci_dev *dev)
On Wed, Mar 27, 2024 at 02:49:34PM -0700, Kuppuswamy Sathyanarayanan wrote: > On 3/27/24 11:02 AM, Bjorn Helgaas wrote: > > From: Bjorn Helgaas <bhelgaas@google.com> > > > > f646c2a0a668 ("PCI: Return u8 from pci_find_capability() and similar") and > > ee8b1c478a9f ("PCI: Return u16 from pci_find_ext_capability() and similar") > > updated the return type of the extern declarations, but neglected to update > > the type of the stubs used CONFIG_PCI is not enabled. > > > > Update them to match the extern declarations. > > > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > > --- > > This change looks fine to me. > > Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> Thanks for reviewing it! > But the callers of these functions still seems to use int > declaration to store the output. Any reason for not changing them? > Like the usages in drivers/pci/pci.c? This patch is just to make the extern declarations match the stubs. No particular reason not to change users other than I didn't want to change the users before the declarations (to avoid warnings about assigning an int to a u8 or u16), and there's not a lot of value in changing local variables, where an int is just on the stack and works fine. Changing stored values in a struct would have more benefit. I took a quick look and found these possibilities: struct controller.cap_offset (drivers/pci/hotplug/shpchp.h SHPC cap) struct pci_sriov.pos (drivers/pci/pci.h SR-IOV ext cap) struct altera_pcie_data.cap_offset (drivers/pci/controller/pcie-altera.c PCIe cap) struct tg3.msi_cap (broadcom/tg3.h MSI cap) struct tg3.pcix_cap (broadcom/tg3.h PCI-X cap) struct bnx2.pm_cap (broadcom/bnx2.h PM cap) struct bnx2.pcix_cap (broadcom/bnx2.h PCI-X cap) struct bnx2x_sriov.cap (broadcom/bnx2x/bnx2x_sriov.h SR-IOV ext cap) struct amd8111e_priv.pm_cap (amd/amd8111e.c, removed [1]) struct pci_params.pm_cap (qlogic/qed/qed.h, removed [2]) struct qed_hw_sriov_info.cap (qlogic/qed/qed_sriov.h SR-IOV ext cap) struct eeh_dev.pcix_cap (powerpc/include/asm/eeh.h PCI-X cap) struct eeh_dev.pcie_cap (powerpc/include/asm/eeh.h PCIe cap) struct eeh_dev.aer_cap (powerpc/include/asm/eeh.h AER ext cap) struct eeh_dev.af_cap (powerpc/include/asm/eeh.h AF cap) struct icm.vnd_cap (drivers/thunderbolt/icm.c VNDR ext cap) [1] https://lore.kernel.org/all/20240325220633.1453180-1-helgaas@kernel.org/ [2] https://lore.kernel.org/all/20240325224931.1462051-1-helgaas@kernel.org/ Bjorn
Hi, On Thu, Mar 28, 2024 at 9:18 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Wed, Mar 27, 2024 at 02:49:34PM -0700, Kuppuswamy Sathyanarayanan wrote: > > On 3/27/24 11:02 AM, Bjorn Helgaas wrote: > > > From: Bjorn Helgaas <bhelgaas@google.com> > > > > > > f646c2a0a668 ("PCI: Return u8 from pci_find_capability() and similar") and > > > ee8b1c478a9f ("PCI: Return u16 from pci_find_ext_capability() and similar") > > > updated the return type of the extern declarations, but neglected to update > > > the type of the stubs used CONFIG_PCI is not enabled. > > > > > > Update them to match the extern declarations. > > > > > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > > > --- > > > > This change looks fine to me. > > > > Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > > Thanks for reviewing it! > > > But the callers of these functions still seems to use int > > declaration to store the output. Any reason for not changing them? > > Like the usages in drivers/pci/pci.c? > > This patch is just to make the extern declarations match the stubs. > > No particular reason not to change users other than I didn't want to > change the users before the declarations (to avoid warnings about > assigning an int to a u8 or u16), and there's not a lot of value in > changing local variables, where an int is just on the stack and works > fine. > > Changing stored values in a struct would have more benefit. I took a > quick look and found these possibilities: > Agree. Thanks for clarifying it. > struct controller.cap_offset (drivers/pci/hotplug/shpchp.h SHPC cap) > struct pci_sriov.pos (drivers/pci/pci.h SR-IOV ext cap) > struct altera_pcie_data.cap_offset (drivers/pci/controller/pcie-altera.c PCIe cap) > > struct tg3.msi_cap (broadcom/tg3.h MSI cap) > struct tg3.pcix_cap (broadcom/tg3.h PCI-X cap) > struct bnx2.pm_cap (broadcom/bnx2.h PM cap) > struct bnx2.pcix_cap (broadcom/bnx2.h PCI-X cap) > struct bnx2x_sriov.cap (broadcom/bnx2x/bnx2x_sriov.h SR-IOV ext cap) > struct amd8111e_priv.pm_cap (amd/amd8111e.c, removed [1]) > struct pci_params.pm_cap (qlogic/qed/qed.h, removed [2]) > struct qed_hw_sriov_info.cap (qlogic/qed/qed_sriov.h SR-IOV ext cap) > struct eeh_dev.pcix_cap (powerpc/include/asm/eeh.h PCI-X cap) > struct eeh_dev.pcie_cap (powerpc/include/asm/eeh.h PCIe cap) > struct eeh_dev.aer_cap (powerpc/include/asm/eeh.h AER ext cap) > struct eeh_dev.af_cap (powerpc/include/asm/eeh.h AF cap) > struct icm.vnd_cap (drivers/thunderbolt/icm.c VNDR ext cap) > > [1] https://lore.kernel.org/all/20240325220633.1453180-1-helgaas@kernel.org/ > [2] https://lore.kernel.org/all/20240325224931.1462051-1-helgaas@kernel.org/ > > Bjorn
diff --git a/include/linux/pci.h b/include/linux/pci.h index b19992a5dfaf..6a09bd9636d5 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -2011,10 +2011,9 @@ static inline int pci_register_driver(struct pci_driver *drv) static inline void pci_unregister_driver(struct pci_driver *drv) { } static inline u8 pci_find_capability(struct pci_dev *dev, int cap) { return 0; } -static inline int pci_find_next_capability(struct pci_dev *dev, u8 post, - int cap) +static inline u8 pci_find_next_capability(struct pci_dev *dev, u8 post, int cap) { return 0; } -static inline int pci_find_ext_capability(struct pci_dev *dev, int cap) +static inline u16 pci_find_ext_capability(struct pci_dev *dev, int cap) { return 0; } static inline u64 pci_get_dsn(struct pci_dev *dev)