From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gavin Shan Subject: Re: [PATCH v8 03/45] powerpc/pci: Cleanup on struct pci_controller_ops Date: Wed, 20 Apr 2016 09:59:20 +1000 Message-ID: <20160419235919.GA11304@gwshan> References: <1455680668-23298-1-git-send-email-gwshan@linux.vnet.ibm.com> <1455680668-23298-4-git-send-email-gwshan@linux.vnet.ibm.com> <570DDE99.7070103@ozlabs.ru> Reply-To: Gavin Shan Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <570DDE99.7070103@ozlabs.ru> Sender: linux-pci-owner@vger.kernel.org To: Alexey Kardashevskiy Cc: Gavin Shan , linuxppc-dev@lists.ozlabs.org, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, benh@kernel.crashing.org, mpe@ellerman.id.au, dja@axtens.net, bhelgaas@google.com, robherring2@gmail.com, grant.likely@linaro.org List-Id: devicetree@vger.kernel.org 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 >>Reviewed-by: Daniel Axtens > >With tiny nit below, > >Reviewed-by: Alexey Kardashevskiy > > > >>--- >> 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 >