| Message ID | 1428053164-28277-25-git-send-email-wangyijing@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <gregory.clement@free-electrons.com> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com> Thanks, Gregory > Signed-off-by: Yijing Wang <wangyijing@huawei.com> > CC: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > CC: Jason Cooper <jason@lakedaemon.net> > --- > 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); > } > > /* >
> 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 <gregory.clement@free-electrons.com> > Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com> Great, thanks very much! Thanks! Yijing. > > > >> Signed-off-by: Yijing Wang <wangyijing@huawei.com> >> CC: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> >> CC: Jason Cooper <jason@lakedaemon.net> >> --- >> 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); >> } >> >> /* >> > >
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); } /*
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. Signed-off-by: Yijing Wang <wangyijing@huawei.com> CC: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> CC: Jason Cooper <jason@lakedaemon.net> --- drivers/pci/host/pci-mvebu.c | 18 +----------------- 1 files changed, 1 insertions(+), 17 deletions(-)