Re: [PATCH v5 12/42] powerpc/pci: Cleanup on pci_controller_ops

2015-06-10 Thread Gavin Shan 
On Wed, Jun 10, 2015 at 02:43:57PM +1000, Alexey Kardashevskiy wrote:
On 06/04/2015 04:41 PM, Gavin Shan wrote:
Each PHB maintains 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.

The patch removes all explicit names of the arguments to the
callbacks in struct pci_controller_ops to keep the code look
consistent.

imho it is a bad idea. Self-documeted code gets less self-documented - how do
I know what unsigned long parameters are for without grepping?


Ok. I'll change the function definations to always have explicit
argument names.


Cc: Daniel Axtens d...@axtens.net
Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com
---
v5:
   * Newly introduced
---
  arch/powerpc/include/asm/pci-bridge.h | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/pci-bridge.h 
b/arch/powerpc/include/asm/pci-bridge.h
index 744884b..1252cd5 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -18,8 +18,8 @@ struct device_node;
   * PCI controller operations
   */
  struct pci_controller_ops {
- void(*dma_dev_setup)(struct pci_dev *dev);
- void(*dma_bus_setup)(struct pci_bus *bus);
+ void(*dma_dev_setup)(struct pci_dev *);
+ void(*dma_bus_setup)(struct pci_bus *);

  int (*probe_mode)(struct pci_bus *);

@@ -28,8 +28,8 @@ struct pci_controller_ops {
  bool(*enable_device_hook)(struct pci_dev *);

  /* Called during PCI resource reassignment */
- resource_size_t (*window_alignment)(struct pci_bus *, unsigned long 
type);
- void(*reset_secondary_bus)(struct pci_dev *dev);
+ resource_size_t (*window_alignment)(struct pci_bus *, unsigned long);
+ void(*reset_secondary_bus)(struct pci_dev *);
  };

  /*


Thanks,
Gavin

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v5 12/42] powerpc/pci: Cleanup on pci_controller_ops

2015-06-09 Thread Alexey Kardashevskiy 

On 06/04/2015 04:41 PM, Gavin Shan wrote:

Each PHB maintains 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.

The patch removes all explicit names of the arguments to the
callbacks in struct pci_controller_ops to keep the code look
consistent.


imho it is a bad idea. Self-documeted code gets less self-documented - how 
do I know what unsigned long parameters are for without grepping?





Cc: Daniel Axtens d...@axtens.net
Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com
---
v5:
   * Newly introduced
---
  arch/powerpc/include/asm/pci-bridge.h | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/pci-bridge.h 
b/arch/powerpc/include/asm/pci-bridge.h
index 744884b..1252cd5 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -18,8 +18,8 @@ struct device_node;
   * PCI controller operations
   */
  struct pci_controller_ops {
-   void(*dma_dev_setup)(struct pci_dev *dev);
-   void(*dma_bus_setup)(struct pci_bus *bus);
+   void(*dma_dev_setup)(struct pci_dev *);
+   void(*dma_bus_setup)(struct pci_bus *);

int (*probe_mode)(struct pci_bus *);

@@ -28,8 +28,8 @@ struct pci_controller_ops {
bool(*enable_device_hook)(struct pci_dev *);

/* Called during PCI resource reassignment */
-   resource_size_t (*window_alignment)(struct pci_bus *, unsigned long 
type);
-   void(*reset_secondary_bus)(struct pci_dev *dev);
+   resource_size_t (*window_alignment)(struct pci_bus *, unsigned long);
+   void(*reset_secondary_bus)(struct pci_dev *);
  };

  /*




--
Alexey
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v5 12/42] powerpc/pci: Cleanup on pci_controller_ops

2015-06-04 Thread Gavin Shan 
Each PHB maintains 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.

The patch removes all explicit names of the arguments to the
callbacks in struct pci_controller_ops to keep the code look
consistent.

Cc: Daniel Axtens d...@axtens.net
Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com
---
v5:
  * Newly introduced
---
 arch/powerpc/include/asm/pci-bridge.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/pci-bridge.h 
b/arch/powerpc/include/asm/pci-bridge.h
index 744884b..1252cd5 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -18,8 +18,8 @@ struct device_node;
  * PCI controller operations
  */
 struct pci_controller_ops {
-   void(*dma_dev_setup)(struct pci_dev *dev);
-   void(*dma_bus_setup)(struct pci_bus *bus);
+   void(*dma_dev_setup)(struct pci_dev *);
+   void(*dma_bus_setup)(struct pci_bus *);
 
int (*probe_mode)(struct pci_bus *);
 
@@ -28,8 +28,8 @@ struct pci_controller_ops {
bool(*enable_device_hook)(struct pci_dev *);
 
/* Called during PCI resource reassignment */
-   resource_size_t (*window_alignment)(struct pci_bus *, unsigned long 
type);
-   void(*reset_secondary_bus)(struct pci_dev *dev);
+   resource_size_t (*window_alignment)(struct pci_bus *, unsigned long);
+   void(*reset_secondary_bus)(struct pci_dev *);
 };
 
 /*
-- 
2.1.0

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev