| Message ID | 1455680668-23298-4-git-send-email-gwshan@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On 17/02/16 14:43, Gavin Shan wrote: > Each PHB has one instance of "struct pci_controller_ops", which > includes various callbacks called by PCI subsystem. In the definition > of this struct, some callbacks have explicit names for its arguments, > but the left don't have. > > This adds all explicit names of the arguments to the callbacks in > "struct pci_controller_ops" so that the code looks consistent. > > Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> > Reviewed-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
On 02/17/2016 02:43 PM, Gavin Shan wrote: > Each PHB has one instance of "struct pci_controller_ops", which > includes various callbacks called by PCI subsystem. In the definition > of this struct, some callbacks have explicit names for its arguments, > but the left don't have. > > This adds all explicit names of the arguments to the callbacks in > "struct pci_controller_ops" so that the code looks consistent. > > Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> > Reviewed-by: Daniel Axtens <dja@axtens.net> With tiny nit below, Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > arch/powerpc/include/asm/pci-bridge.h | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h > index b688d04..4dd6ef4 100644 > --- a/arch/powerpc/include/asm/pci-bridge.h > +++ b/arch/powerpc/include/asm/pci-bridge.h > @@ -21,18 +21,19 @@ struct pci_controller_ops { > void (*dma_dev_setup)(struct pci_dev *dev); > void (*dma_bus_setup)(struct pci_bus *bus); > > - int (*probe_mode)(struct pci_bus *); > + int (*probe_mode)(struct pci_bus *bus); > > /* Called when pci_enable_device() is called. Returns true to > * allow assignment/enabling of the device. */ > - bool (*enable_device_hook)(struct pci_dev *); > + bool (*enable_device_hook)(struct pci_dev *dev); "pdev" is slightly better as it is of the "pci_dev" type (4130 occurrences of "pci_dev *pdev" and just 2833 of "pci_dev *dev" in the current kernel), "dev" is for "struct device". > > - void (*disable_device)(struct pci_dev *); > + void (*disable_device)(struct pci_dev *dev); > > - void (*release_device)(struct pci_dev *); > + void (*release_device)(struct pci_dev *dev); > > /* Called during PCI resource reassignment */ > - resource_size_t (*window_alignment)(struct pci_bus *, unsigned long type); > + resource_size_t (*window_alignment)(struct pci_bus *bus, > + unsigned long type); > void (*setup_bridge)(struct pci_bus *bus, > unsigned long type); > void (*reset_secondary_bus)(struct pci_dev *dev); > @@ -46,7 +47,7 @@ struct pci_controller_ops { > int (*dma_set_mask)(struct pci_dev *dev, u64 dma_mask); > u64 (*dma_get_required_mask)(struct pci_dev *dev); > > - void (*shutdown)(struct pci_controller *); > + void (*shutdown)(struct pci_controller *hose); > }; > > /* >
On Wed, Apr 13, 2016 at 03:52:25PM +1000, Alexey Kardashevskiy wrote: >On 02/17/2016 02:43 PM, Gavin Shan wrote: >>Each PHB has one instance of "struct pci_controller_ops", which >>includes various callbacks called by PCI subsystem. In the definition >>of this struct, some callbacks have explicit names for its arguments, >>but the left don't have. >> >>This adds all explicit names of the arguments to the callbacks in >>"struct pci_controller_ops" so that the code looks consistent. >> >>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >>Reviewed-by: Daniel Axtens <dja@axtens.net> > >With tiny nit below, > >Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > > >>--- >> arch/powerpc/include/asm/pci-bridge.h | 13 +++++++------ >> 1 file changed, 7 insertions(+), 6 deletions(-) >> >>diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h >>index b688d04..4dd6ef4 100644 >>--- a/arch/powerpc/include/asm/pci-bridge.h >>+++ b/arch/powerpc/include/asm/pci-bridge.h >>@@ -21,18 +21,19 @@ struct pci_controller_ops { >> void (*dma_dev_setup)(struct pci_dev *dev); >> void (*dma_bus_setup)(struct pci_bus *bus); >> >>- int (*probe_mode)(struct pci_bus *); >>+ int (*probe_mode)(struct pci_bus *bus); >> >> /* Called when pci_enable_device() is called. Returns true to >> * allow assignment/enabling of the device. */ >>- bool (*enable_device_hook)(struct pci_dev *); >>+ bool (*enable_device_hook)(struct pci_dev *dev); > > >"pdev" is slightly better as it is of the "pci_dev" type (4130 occurrences of >"pci_dev *pdev" and just 2833 of "pci_dev *dev" in the current kernel), "dev" >is for "struct device". > Thanks for your review. I don't know if "dev" is for "struct device" only. Usually, "dev" and "pdev" are interchangeably used for "struct pci_dev". Especially the code written in old days uses "dev" for "struct pci_dev" heavily. Yes, I agree "pdev" is better than "dev" in this case and I'm going to fix this up in next revision. >> >>- void (*disable_device)(struct pci_dev *); >>+ void (*disable_device)(struct pci_dev *dev); >> >>- void (*release_device)(struct pci_dev *); >>+ void (*release_device)(struct pci_dev *dev); >> >> /* Called during PCI resource reassignment */ >>- resource_size_t (*window_alignment)(struct pci_bus *, unsigned long type); >>+ resource_size_t (*window_alignment)(struct pci_bus *bus, >>+ unsigned long type); >> void (*setup_bridge)(struct pci_bus *bus, >> unsigned long type); >> void (*reset_secondary_bus)(struct pci_dev *dev); >>@@ -46,7 +47,7 @@ struct pci_controller_ops { >> int (*dma_set_mask)(struct pci_dev *dev, u64 dma_mask); >> u64 (*dma_get_required_mask)(struct pci_dev *dev); >> >>- void (*shutdown)(struct pci_controller *); >>+ void (*shutdown)(struct pci_controller *hose); >> }; >> >> /* >> > > >-- >Alexey > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h index b688d04..4dd6ef4 100644 --- a/arch/powerpc/include/asm/pci-bridge.h +++ b/arch/powerpc/include/asm/pci-bridge.h @@ -21,18 +21,19 @@ struct pci_controller_ops { void (*dma_dev_setup)(struct pci_dev *dev); void (*dma_bus_setup)(struct pci_bus *bus); - int (*probe_mode)(struct pci_bus *); + int (*probe_mode)(struct pci_bus *bus); /* Called when pci_enable_device() is called. Returns true to * allow assignment/enabling of the device. */ - bool (*enable_device_hook)(struct pci_dev *); + bool (*enable_device_hook)(struct pci_dev *dev); - void (*disable_device)(struct pci_dev *); + void (*disable_device)(struct pci_dev *dev); - void (*release_device)(struct pci_dev *); + void (*release_device)(struct pci_dev *dev); /* Called during PCI resource reassignment */ - resource_size_t (*window_alignment)(struct pci_bus *, unsigned long type); + resource_size_t (*window_alignment)(struct pci_bus *bus, + unsigned long type); void (*setup_bridge)(struct pci_bus *bus, unsigned long type); void (*reset_secondary_bus)(struct pci_dev *dev); @@ -46,7 +47,7 @@ struct pci_controller_ops { int (*dma_set_mask)(struct pci_dev *dev, u64 dma_mask); u64 (*dma_get_required_mask)(struct pci_dev *dev); - void (*shutdown)(struct pci_controller *); + void (*shutdown)(struct pci_controller *hose); }; /*