diff mbox

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

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

Commit Message

Gavin Shan Feb. 17, 2016, 3:43 a.m. UTC
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>
---
 arch/powerpc/include/asm/pci-bridge.h | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Andrew Donnellan Feb. 17, 2016, 4:18 a.m. UTC | #1
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>
Alexey Kardashevskiy April 13, 2016, 5:52 a.m. UTC | #2
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);
>   };
>
>   /*
>
Gavin Shan April 19, 2016, 11:59 p.m. UTC | #3
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 mbox

Patch

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);
 };
 
 /*