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

From: Gavin Shan <gwshan@linux.vnet.ibm.com>
To: Alexey Kardashevskiy <aik@ozlabs.ru>
Cc: devicetree@vger.kernel.org, linux-pci@vger.kernel.org,
	panto@antoniou-consulting.com,
	Gavin Shan <gwshan@linux.vnet.ibm.com>,
	grant.likely@linaro.org, robherring2@gmail.com,
	bhelgaas@google.com, linuxppc-dev@lists.ozlabs.org,
	Daniel Axtens <dja@axtens.net>
Subject: Re: [PATCH v5 12/42] powerpc/pci: Cleanup on pci_controller_ops
Date: Wed, 10 Jun 2015 16:20:08 +1000	[thread overview]
Message-ID: <20150610062008.GA9875@gwshan> (raw)
In-Reply-To: <5577C08D.1050900@ozlabs.ru>

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 <dja@axtens.net>
>>Signed-off-by: Gavin Shan <gwshan@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