From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753168AbbDHIIk (ORCPT ); Wed, 8 Apr 2015 04:08:40 -0400 Received: from down.free-electrons.com ([37.187.137.238]:39934 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751510AbbDHIIa (ORCPT ); Wed, 8 Apr 2015 04:08:30 -0400 Message-ID: <5524E1F9.6030700@free-electrons.com> Date: Wed, 08 Apr 2015 10:08:25 +0200 From: Gregory CLEMENT User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Yijing Wang CC: Bjorn Helgaas , linux-ia64@vger.kernel.org, linux-pci@vger.kernel.org, Guan Xuetao , Russell King , x86@kernel.org, Geert Uytterhoeven , Benjamin Herrenschmidt , Jason Cooper , Arnd Bergmann , Marc Zyngier , Rusty Russell , linux-m68k@vger.kernel.org, Thomas Gleixner , Yinghai Lu , linux-arm-kernel@lists.infradead.org, dja@axtens.net, Thomas Petazzoni , Liviu Dudau , Tony Luck , linux-kernel@vger.kernel.org, Jiang Liu , linux-alpha@vger.kernel.org, "David S. Miller" Subject: Re: [PATCH v9 23/30] PCI/mvebu: Use pci_common_init_dev() to simplify code References: <1428053164-28277-1-git-send-email-wangyijing@huawei.com> <1428053164-28277-25-git-send-email-wangyijing@huawei.com> In-Reply-To: <1428053164-28277-25-git-send-email-wangyijing@huawei.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Yijing, On 03/04/2015 11:25, Yijing Wang wrote: > Mvebu_pcie_scan_bus() is not necessary, we could use > pci_common_init_dev() instead of pci_common_init(), > and pass the device pointer as the parent. Then > pci_scan_root_bus() will be called to scan the pci busses. > 2 months ago, Thomas Petazzoni was concerned about the removal of mvebu_pcie_scan_bus(). So I dig the archives of the discussion surrounding the pcie-mvebu drive. I found that the main purpose of using this function was to allow to pass "struct device *" pointer. Thanks to the introduction of pci_common_init_dev it was not needed anymore. Actually we should have done this change when this function had been introduced. So for the point of view of the code it's fine. Then I tested your full series on Armada XP, Armada 375 and Armada 38x SoCs, and I didn't saw any regression. So you can add my: Reviewed-by: Gregory CLEMENT Tested-by: Gregory CLEMENT Thanks, Gregory > Signed-off-by: Yijing Wang > CC: Thomas Petazzoni > CC: Jason Cooper > --- > drivers/pci/host/pci-mvebu.c | 18 +----------------- > 1 files changed, 1 insertions(+), 17 deletions(-) > > diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c > index 0cfc494..d5a2b70 100644 > --- a/drivers/pci/host/pci-mvebu.c > +++ b/drivers/pci/host/pci-mvebu.c > @@ -750,21 +750,6 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > return 1; > } > > -static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys) > -{ > - struct mvebu_pcie *pcie = sys_to_pcie(sys); > - struct pci_bus *bus; > - > - bus = pci_create_root_bus(&pcie->pdev->dev, -1, sys->busnr, > - &mvebu_pcie_ops, sys, &sys->resources); > - if (!bus) > - return NULL; > - > - pci_scan_child_bus(bus); > - > - return bus; > -} > - > static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, > const struct resource *res, > resource_size_t start, > @@ -808,12 +793,11 @@ static void mvebu_pcie_enable(struct mvebu_pcie *pcie) > hw.nr_controllers = 1; > hw.private_data = (void **)&pcie; > hw.setup = mvebu_pcie_setup; > - hw.scan = mvebu_pcie_scan_bus; > hw.map_irq = of_irq_parse_and_map_pci; > hw.ops = &mvebu_pcie_ops; > hw.align_resource = mvebu_pcie_align_resource; > > - pci_common_init(&hw); > + pci_common_init_dev(&pcie->pdev->dev, &hw); > } > > /* > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from down.free-electrons.com ([37.187.137.238]:39934 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751510AbbDHIIa (ORCPT ); Wed, 8 Apr 2015 04:08:30 -0400 Message-ID: <5524E1F9.6030700@free-electrons.com> Date: Wed, 08 Apr 2015 10:08:25 +0200 From: Gregory CLEMENT MIME-Version: 1.0 To: Yijing Wang CC: Bjorn Helgaas , linux-ia64@vger.kernel.org, linux-pci@vger.kernel.org, Guan Xuetao , Russell King , x86@kernel.org, Geert Uytterhoeven , Benjamin Herrenschmidt , Jason Cooper , Arnd Bergmann , Marc Zyngier , Rusty Russell , linux-m68k@lists.linux-m68k.org, Thomas Gleixner , Yinghai Lu , linux-arm-kernel@lists.infradead.org, dja@axtens.net, Thomas Petazzoni , Liviu Dudau , Tony Luck , linux-kernel@vger.kernel.org, Jiang Liu , linux-alpha@vger.kernel.org, "David S. Miller" Subject: Re: [PATCH v9 23/30] PCI/mvebu: Use pci_common_init_dev() to simplify code References: <1428053164-28277-1-git-send-email-wangyijing@huawei.com> <1428053164-28277-25-git-send-email-wangyijing@huawei.com> In-Reply-To: <1428053164-28277-25-git-send-email-wangyijing@huawei.com> Content-Type: text/plain; charset=windows-1252 Sender: linux-pci-owner@vger.kernel.org List-ID: Hi Yijing, On 03/04/2015 11:25, Yijing Wang wrote: > Mvebu_pcie_scan_bus() is not necessary, we could use > pci_common_init_dev() instead of pci_common_init(), > and pass the device pointer as the parent. Then > pci_scan_root_bus() will be called to scan the pci busses. > 2 months ago, Thomas Petazzoni was concerned about the removal of mvebu_pcie_scan_bus(). So I dig the archives of the discussion surrounding the pcie-mvebu drive. I found that the main purpose of using this function was to allow to pass "struct device *" pointer. Thanks to the introduction of pci_common_init_dev it was not needed anymore. Actually we should have done this change when this function had been introduced. So for the point of view of the code it's fine. Then I tested your full series on Armada XP, Armada 375 and Armada 38x SoCs, and I didn't saw any regression. So you can add my: Reviewed-by: Gregory CLEMENT Tested-by: Gregory CLEMENT Thanks, Gregory > Signed-off-by: Yijing Wang > CC: Thomas Petazzoni > CC: Jason Cooper > --- > drivers/pci/host/pci-mvebu.c | 18 +----------------- > 1 files changed, 1 insertions(+), 17 deletions(-) > > diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c > index 0cfc494..d5a2b70 100644 > --- a/drivers/pci/host/pci-mvebu.c > +++ b/drivers/pci/host/pci-mvebu.c > @@ -750,21 +750,6 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > return 1; > } > > -static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys) > -{ > - struct mvebu_pcie *pcie = sys_to_pcie(sys); > - struct pci_bus *bus; > - > - bus = pci_create_root_bus(&pcie->pdev->dev, -1, sys->busnr, > - &mvebu_pcie_ops, sys, &sys->resources); > - if (!bus) > - return NULL; > - > - pci_scan_child_bus(bus); > - > - return bus; > -} > - > static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, > const struct resource *res, > resource_size_t start, > @@ -808,12 +793,11 @@ static void mvebu_pcie_enable(struct mvebu_pcie *pcie) > hw.nr_controllers = 1; > hw.private_data = (void **)&pcie; > hw.setup = mvebu_pcie_setup; > - hw.scan = mvebu_pcie_scan_bus; > hw.map_irq = of_irq_parse_and_map_pci; > hw.ops = &mvebu_pcie_ops; > hw.align_resource = mvebu_pcie_align_resource; > > - pci_common_init(&hw); > + pci_common_init_dev(&pcie->pdev->dev, &hw); > } > > /* > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com From mboxrd@z Thu Jan 1 00:00:00 1970 From: gregory.clement@free-electrons.com (Gregory CLEMENT) Date: Wed, 08 Apr 2015 10:08:25 +0200 Subject: [PATCH v9 23/30] PCI/mvebu: Use pci_common_init_dev() to simplify code In-Reply-To: <1428053164-28277-25-git-send-email-wangyijing@huawei.com> References: <1428053164-28277-1-git-send-email-wangyijing@huawei.com> <1428053164-28277-25-git-send-email-wangyijing@huawei.com> Message-ID: <5524E1F9.6030700@free-electrons.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Yijing, On 03/04/2015 11:25, Yijing Wang wrote: > Mvebu_pcie_scan_bus() is not necessary, we could use > pci_common_init_dev() instead of pci_common_init(), > and pass the device pointer as the parent. Then > pci_scan_root_bus() will be called to scan the pci busses. > 2 months ago, Thomas Petazzoni was concerned about the removal of mvebu_pcie_scan_bus(). So I dig the archives of the discussion surrounding the pcie-mvebu drive. I found that the main purpose of using this function was to allow to pass "struct device *" pointer. Thanks to the introduction of pci_common_init_dev it was not needed anymore. Actually we should have done this change when this function had been introduced. So for the point of view of the code it's fine. Then I tested your full series on Armada XP, Armada 375 and Armada 38x SoCs, and I didn't saw any regression. So you can add my: Reviewed-by: Gregory CLEMENT Tested-by: Gregory CLEMENT Thanks, Gregory > Signed-off-by: Yijing Wang > CC: Thomas Petazzoni > CC: Jason Cooper > --- > drivers/pci/host/pci-mvebu.c | 18 +----------------- > 1 files changed, 1 insertions(+), 17 deletions(-) > > diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c > index 0cfc494..d5a2b70 100644 > --- a/drivers/pci/host/pci-mvebu.c > +++ b/drivers/pci/host/pci-mvebu.c > @@ -750,21 +750,6 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > return 1; > } > > -static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys) > -{ > - struct mvebu_pcie *pcie = sys_to_pcie(sys); > - struct pci_bus *bus; > - > - bus = pci_create_root_bus(&pcie->pdev->dev, -1, sys->busnr, > - &mvebu_pcie_ops, sys, &sys->resources); > - if (!bus) > - return NULL; > - > - pci_scan_child_bus(bus); > - > - return bus; > -} > - > static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, > const struct resource *res, > resource_size_t start, > @@ -808,12 +793,11 @@ static void mvebu_pcie_enable(struct mvebu_pcie *pcie) > hw.nr_controllers = 1; > hw.private_data = (void **)&pcie; > hw.setup = mvebu_pcie_setup; > - hw.scan = mvebu_pcie_scan_bus; > hw.map_irq = of_irq_parse_and_map_pci; > hw.ops = &mvebu_pcie_ops; > hw.align_resource = mvebu_pcie_align_resource; > > - pci_common_init(&hw); > + pci_common_init_dev(&pcie->pdev->dev, &hw); > } > > /* > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gregory CLEMENT Date: Wed, 08 Apr 2015 08:08:25 +0000 Subject: Re: [PATCH v9 23/30] PCI/mvebu: Use pci_common_init_dev() to simplify code Message-Id: <5524E1F9.6030700@free-electrons.com> List-Id: References: <1428053164-28277-1-git-send-email-wangyijing@huawei.com> <1428053164-28277-25-git-send-email-wangyijing@huawei.com> In-Reply-To: <1428053164-28277-25-git-send-email-wangyijing@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Yijing Wang Cc: Bjorn Helgaas , linux-ia64@vger.kernel.org, linux-pci@vger.kernel.org, Guan Xuetao , Russell King , x86@kernel.org, Geert Uytterhoeven , Benjamin Herrenschmidt , Jason Cooper , Arnd Bergmann , Marc Zyngier , Rusty Russell , linux-m68k@vger.kernel.org, Thomas Gleixner , Yinghai Lu , linux-arm-kernel@lists.infradead.org, dja@axtens.net, Thomas Petazzoni , Liviu Dudau , Tony Luck , linux-kernel@vger.kernel.org, Jiang Liu , linux-alpha@vger.kernel.org, "David S. Miller" Hi Yijing, On 03/04/2015 11:25, Yijing Wang wrote: > Mvebu_pcie_scan_bus() is not necessary, we could use > pci_common_init_dev() instead of pci_common_init(), > and pass the device pointer as the parent. Then > pci_scan_root_bus() will be called to scan the pci busses. > 2 months ago, Thomas Petazzoni was concerned about the removal of mvebu_pcie_scan_bus(). So I dig the archives of the discussion surrounding the pcie-mvebu drive. I found that the main purpose of using this function was to allow to pass "struct device *" pointer. Thanks to the introduction of pci_common_init_dev it was not needed anymore. Actually we should have done this change when this function had been introduced. So for the point of view of the code it's fine. Then I tested your full series on Armada XP, Armada 375 and Armada 38x SoCs, and I didn't saw any regression. So you can add my: Reviewed-by: Gregory CLEMENT Tested-by: Gregory CLEMENT Thanks, Gregory > Signed-off-by: Yijing Wang > CC: Thomas Petazzoni > CC: Jason Cooper > --- > drivers/pci/host/pci-mvebu.c | 18 +----------------- > 1 files changed, 1 insertions(+), 17 deletions(-) > > diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c > index 0cfc494..d5a2b70 100644 > --- a/drivers/pci/host/pci-mvebu.c > +++ b/drivers/pci/host/pci-mvebu.c > @@ -750,21 +750,6 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > return 1; > } > > -static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys) > -{ > - struct mvebu_pcie *pcie = sys_to_pcie(sys); > - struct pci_bus *bus; > - > - bus = pci_create_root_bus(&pcie->pdev->dev, -1, sys->busnr, > - &mvebu_pcie_ops, sys, &sys->resources); > - if (!bus) > - return NULL; > - > - pci_scan_child_bus(bus); > - > - return bus; > -} > - > static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, > const struct resource *res, > resource_size_t start, > @@ -808,12 +793,11 @@ static void mvebu_pcie_enable(struct mvebu_pcie *pcie) > hw.nr_controllers = 1; > hw.private_data = (void **)&pcie; > hw.setup = mvebu_pcie_setup; > - hw.scan = mvebu_pcie_scan_bus; > hw.map_irq = of_irq_parse_and_map_pci; > hw.ops = &mvebu_pcie_ops; > hw.align_resource = mvebu_pcie_align_resource; > > - pci_common_init(&hw); > + pci_common_init_dev(&pcie->pdev->dev, &hw); > } > > /* > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com