[PATCH v8 03/45] powerpc/pci: Cleanup on struct pci_controller_ops

[Gavin Shan]

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 at linux.vnet.ibm.com>
>>Reviewed-by: Daniel Axtens <dja at axtens.net>
>
>With tiny nit below,
>
>Reviewed-by: Alexey Kardashevskiy <aik at 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
>