diff mbox series

PCI: Update pci_find_capability() stub return values

Message ID 20240327180234.1529164-1-helgaas@kernel.org
State New
Headers show
Series PCI: Update pci_find_capability() stub return values | expand

Commit Message

Bjorn Helgaas March 27, 2024, 6:02 p.m. UTC 
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>
---
 include/linux/pci.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Bjorn Helgaas March 27, 2024, 6:12 p.m. UTC | #1 
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
>
Kuppuswamy Sathyanarayanan March 27, 2024, 9:49 p.m. UTC | #2 
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)
Bjorn Helgaas March 28, 2024, 4:18 p.m. UTC | #3 
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
Kuppuswamy Sathyanarayanan March 28, 2024, 10:36 p.m. UTC | #4 
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 mbox series

Patch

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)