* [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() @ 2021-01-20 10:52 Michael Walle 2021-01-20 14:23 ` Rob Herring ` (2 more replies) 0 siblings, 3 replies; 30+ messages in thread From: Michael Walle @ 2021-01-20 10:52 UTC (permalink / raw) To: linuxppc-dev, linux-pci, linux-arm-kernel, linux-kernel Cc: Minghuan Lian, Mingkai Hu, Roy Zang, Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, Greg Kroah-Hartman, Saravana Kannan, Michael Walle fw_devlink will defer the probe until all suppliers are ready. We can't use builtin_platform_driver_probe() because it doesn't retry after probe deferral. Convert it to builtin_platform_driver(). Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") Signed-off-by: Michael Walle <michael@walle.cc> --- drivers/pci/controller/dwc/pci-layerscape.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/pci/controller/dwc/pci-layerscape.c b/drivers/pci/controller/dwc/pci-layerscape.c index 44ad34cdc3bc..5b9c625df7b8 100644 --- a/drivers/pci/controller/dwc/pci-layerscape.c +++ b/drivers/pci/controller/dwc/pci-layerscape.c @@ -232,7 +232,7 @@ static const struct of_device_id ls_pcie_of_match[] = { { }, }; -static int __init ls_pcie_probe(struct platform_device *pdev) +static int ls_pcie_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct dw_pcie *pci; @@ -271,10 +271,11 @@ static int __init ls_pcie_probe(struct platform_device *pdev) } static struct platform_driver ls_pcie_driver = { + .probe = ls_pcie_probe, .driver = { .name = "layerscape-pcie", .of_match_table = ls_pcie_of_match, .suppress_bind_attrs = true, }, }; -builtin_platform_driver_probe(ls_pcie_driver, ls_pcie_probe); +builtin_platform_driver(ls_pcie_driver); -- 2.20.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-20 10:52 [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() Michael Walle @ 2021-01-20 14:23 ` Rob Herring 2021-01-20 14:34 ` Michael Walle ` (2 more replies) 2021-01-26 10:02 ` Lorenzo Pieralisi 2021-01-26 10:55 ` Lorenzo Pieralisi 2 siblings, 3 replies; 30+ messages in thread From: Rob Herring @ 2021-01-20 14:23 UTC (permalink / raw) To: Michael Walle Cc: linuxppc-dev, PCI, linux-arm-kernel, linux-kernel, Minghuan Lian, Mingkai Hu, Roy Zang, Lorenzo Pieralisi, Bjorn Helgaas, Greg Kroah-Hartman, Saravana Kannan On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> wrote: > > fw_devlink will defer the probe until all suppliers are ready. We can't > use builtin_platform_driver_probe() because it doesn't retry after probe > deferral. Convert it to builtin_platform_driver(). If builtin_platform_driver_probe() doesn't work with fw_devlink, then shouldn't it be fixed or removed? Then we're not fixing drivers later when folks start caring about deferred probe and devlink. I'd really prefer if we convert this to a module instead. (And all the other PCI host drivers.) > Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") This happened!? > Signed-off-by: Michael Walle <michael@walle.cc> > --- > drivers/pci/controller/dwc/pci-layerscape.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pci-layerscape.c b/drivers/pci/controller/dwc/pci-layerscape.c > index 44ad34cdc3bc..5b9c625df7b8 100644 > --- a/drivers/pci/controller/dwc/pci-layerscape.c > +++ b/drivers/pci/controller/dwc/pci-layerscape.c > @@ -232,7 +232,7 @@ static const struct of_device_id ls_pcie_of_match[] = { > { }, > }; > > -static int __init ls_pcie_probe(struct platform_device *pdev) > +static int ls_pcie_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > struct dw_pcie *pci; > @@ -271,10 +271,11 @@ static int __init ls_pcie_probe(struct platform_device *pdev) > } > > static struct platform_driver ls_pcie_driver = { > + .probe = ls_pcie_probe, > .driver = { > .name = "layerscape-pcie", > .of_match_table = ls_pcie_of_match, > .suppress_bind_attrs = true, > }, > }; > -builtin_platform_driver_probe(ls_pcie_driver, ls_pcie_probe); > +builtin_platform_driver(ls_pcie_driver); > -- > 2.20.1 > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-20 14:23 ` Rob Herring @ 2021-01-20 14:34 ` Michael Walle 2021-01-20 15:50 ` Greg Kroah-Hartman 2021-01-20 19:02 ` Saravana Kannan 2 siblings, 0 replies; 30+ messages in thread From: Michael Walle @ 2021-01-20 14:34 UTC (permalink / raw) To: Rob Herring Cc: linuxppc-dev, PCI, linux-arm-kernel, linux-kernel, Minghuan Lian, Mingkai Hu, Roy Zang, Lorenzo Pieralisi, Bjorn Helgaas, Greg Kroah-Hartman, Saravana Kannan Am 2021-01-20 15:23, schrieb Rob Herring: > On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> wrote: >> >> fw_devlink will defer the probe until all suppliers are ready. We >> can't >> use builtin_platform_driver_probe() because it doesn't retry after >> probe >> deferral. Convert it to builtin_platform_driver(). > > If builtin_platform_driver_probe() doesn't work with fw_devlink, then > shouldn't it be fixed or removed? Then we're not fixing drivers later > when folks start caring about deferred probe and devlink. > > I'd really prefer if we convert this to a module instead. (And all the > other PCI host drivers.) I briefly looked into adding a remove() op. But I don't know what will have to be undo of the ls_pcie_host_init(). That would be up to the maintainers of this driver. _But_ I'd really like to see PCI working on my board again ;) At least until the end of this development cycle. -michael >> Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") > > This happened!? > >> Signed-off-by: Michael Walle <michael@walle.cc> >> --- >> drivers/pci/controller/dwc/pci-layerscape.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/pci/controller/dwc/pci-layerscape.c >> b/drivers/pci/controller/dwc/pci-layerscape.c >> index 44ad34cdc3bc..5b9c625df7b8 100644 >> --- a/drivers/pci/controller/dwc/pci-layerscape.c >> +++ b/drivers/pci/controller/dwc/pci-layerscape.c >> @@ -232,7 +232,7 @@ static const struct of_device_id >> ls_pcie_of_match[] = { >> { }, >> }; >> >> -static int __init ls_pcie_probe(struct platform_device *pdev) >> +static int ls_pcie_probe(struct platform_device *pdev) >> { >> struct device *dev = &pdev->dev; >> struct dw_pcie *pci; >> @@ -271,10 +271,11 @@ static int __init ls_pcie_probe(struct >> platform_device *pdev) >> } >> >> static struct platform_driver ls_pcie_driver = { >> + .probe = ls_pcie_probe, >> .driver = { >> .name = "layerscape-pcie", >> .of_match_table = ls_pcie_of_match, >> .suppress_bind_attrs = true, >> }, >> }; >> -builtin_platform_driver_probe(ls_pcie_driver, ls_pcie_probe); >> +builtin_platform_driver(ls_pcie_driver); >> -- >> 2.20.1 >> ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-20 14:23 ` Rob Herring 2021-01-20 14:34 ` Michael Walle @ 2021-01-20 15:50 ` Greg Kroah-Hartman 2021-01-20 19:02 ` Saravana Kannan 2 siblings, 0 replies; 30+ messages in thread From: Greg Kroah-Hartman @ 2021-01-20 15:50 UTC (permalink / raw) To: Rob Herring Cc: Michael Walle, linuxppc-dev, PCI, linux-arm-kernel, linux-kernel, Minghuan Lian, Mingkai Hu, Roy Zang, Lorenzo Pieralisi, Bjorn Helgaas, Saravana Kannan On Wed, Jan 20, 2021 at 08:23:59AM -0600, Rob Herring wrote: > On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> wrote: > > > > fw_devlink will defer the probe until all suppliers are ready. We can't > > use builtin_platform_driver_probe() because it doesn't retry after probe > > deferral. Convert it to builtin_platform_driver(). > > If builtin_platform_driver_probe() doesn't work with fw_devlink, then > shouldn't it be fixed or removed? Then we're not fixing drivers later > when folks start caring about deferred probe and devlink. > > I'd really prefer if we convert this to a module instead. (And all the > other PCI host drivers.) > > > Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") > > This happened!? It's in linux-next in my tree, but is looking like it might be reverted soon. But finding these issues is good. thanks, greg k-h ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-20 14:23 ` Rob Herring 2021-01-20 14:34 ` Michael Walle 2021-01-20 15:50 ` Greg Kroah-Hartman @ 2021-01-20 19:02 ` Saravana Kannan 2021-01-20 19:25 ` Michael Walle 2021-01-20 19:28 ` Michael Walle 2 siblings, 2 replies; 30+ messages in thread From: Saravana Kannan @ 2021-01-20 19:02 UTC (permalink / raw) To: Rob Herring Cc: Michael Walle, linuxppc-dev, PCI, linux-arm-kernel, linux-kernel, Minghuan Lian, Mingkai Hu, Roy Zang, Lorenzo Pieralisi, Bjorn Helgaas, Greg Kroah-Hartman On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: > > On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> wrote: > > > > fw_devlink will defer the probe until all suppliers are ready. We can't > > use builtin_platform_driver_probe() because it doesn't retry after probe > > deferral. Convert it to builtin_platform_driver(). > > If builtin_platform_driver_probe() doesn't work with fw_devlink, then > shouldn't it be fixed or removed? I was actually thinking about this too. The problem with fixing builtin_platform_driver_probe() to behave like builtin_platform_driver() is that these probe functions could be marked with __init. But there are also only 20 instances of builtin_platform_driver_probe() in the kernel: $ git grep ^builtin_platform_driver_probe | wc -l 20 So it might be easier to just fix them to not use builtin_platform_driver_probe(). Michael, Any chance you'd be willing to help me by converting all these to builtin_platform_driver() and delete builtin_platform_driver_probe()? -Saravana > Then we're not fixing drivers later > when folks start caring about deferred probe and devlink. > > I'd really prefer if we convert this to a module instead. (And all the > other PCI host drivers.) > > > Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") > > This happened!? > > > Signed-off-by: Michael Walle <michael@walle.cc> > > --- > > drivers/pci/controller/dwc/pci-layerscape.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pci-layerscape.c b/drivers/pci/controller/dwc/pci-layerscape.c > > index 44ad34cdc3bc..5b9c625df7b8 100644 > > --- a/drivers/pci/controller/dwc/pci-layerscape.c > > +++ b/drivers/pci/controller/dwc/pci-layerscape.c > > @@ -232,7 +232,7 @@ static const struct of_device_id ls_pcie_of_match[] = { > > { }, > > }; > > > > -static int __init ls_pcie_probe(struct platform_device *pdev) > > +static int ls_pcie_probe(struct platform_device *pdev) > > { > > struct device *dev = &pdev->dev; > > struct dw_pcie *pci; > > @@ -271,10 +271,11 @@ static int __init ls_pcie_probe(struct platform_device *pdev) > > } > > > > static struct platform_driver ls_pcie_driver = { > > + .probe = ls_pcie_probe, > > .driver = { > > .name = "layerscape-pcie", > > .of_match_table = ls_pcie_of_match, > > .suppress_bind_attrs = true, > > }, > > }; > > -builtin_platform_driver_probe(ls_pcie_driver, ls_pcie_probe); > > +builtin_platform_driver(ls_pcie_driver); > > -- > > 2.20.1 > > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-20 19:02 ` Saravana Kannan @ 2021-01-20 19:25 ` Michael Walle 2021-01-20 19:28 ` Michael Walle 1 sibling, 0 replies; 30+ messages in thread From: Michael Walle @ 2021-01-20 19:25 UTC (permalink / raw) Am 2021-01-20 20:02, schrieb Saravana Kannan: > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> >> wrote: >> > >> > fw_devlink will defer the probe until all suppliers are ready. We can't >> > use builtin_platform_driver_probe() because it doesn't retry after probe >> > deferral. Convert it to builtin_platform_driver(). >> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then >> shouldn't it be fixed or removed? > > I was actually thinking about this too. The problem with fixing > builtin_platform_driver_probe() to behave like > builtin_platform_driver() is that these probe functions could be > marked with __init. But there are also only 20 instances of > builtin_platform_driver_probe() in the kernel: > $ git grep ^builtin_platform_driver_probe | wc -l > 20 > > So it might be easier to just fix them to not use > builtin_platform_driver_probe(). > > Michael, > > Any chance you'd be willing to help me by converting all these to > builtin_platform_driver() and delete builtin_platform_driver_probe()? If it just moving the probe function to the _driver struct and remove the __init annotations. I could look into that. -michael ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-20 19:02 ` Saravana Kannan 2021-01-20 19:25 ` Michael Walle @ 2021-01-20 19:28 ` Michael Walle 2021-01-20 19:47 ` Saravana Kannan 2021-01-25 16:50 ` Lorenzo Pieralisi 1 sibling, 2 replies; 30+ messages in thread From: Michael Walle @ 2021-01-20 19:28 UTC (permalink / raw) To: Saravana Kannan Cc: Rob Herring, linuxppc-dev, PCI, linux-arm-kernel, linux-kernel, Minghuan Lian, Mingkai Hu, Roy Zang, Lorenzo Pieralisi, Bjorn Helgaas, Greg Kroah-Hartman [RESEND, fat-fingered the buttons of my mail client and converted all CCs to BCCs :(] Am 2021-01-20 20:02, schrieb Saravana Kannan: > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> >> wrote: >> > >> > fw_devlink will defer the probe until all suppliers are ready. We can't >> > use builtin_platform_driver_probe() because it doesn't retry after probe >> > deferral. Convert it to builtin_platform_driver(). >> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then >> shouldn't it be fixed or removed? > > I was actually thinking about this too. The problem with fixing > builtin_platform_driver_probe() to behave like > builtin_platform_driver() is that these probe functions could be > marked with __init. But there are also only 20 instances of > builtin_platform_driver_probe() in the kernel: > $ git grep ^builtin_platform_driver_probe | wc -l > 20 > > So it might be easier to just fix them to not use > builtin_platform_driver_probe(). > > Michael, > > Any chance you'd be willing to help me by converting all these to > builtin_platform_driver() and delete builtin_platform_driver_probe()? If it just moving the probe function to the _driver struct and remove the __init annotations. I could look into that. -michael ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-20 19:28 ` Michael Walle @ 2021-01-20 19:47 ` Saravana Kannan 2021-01-20 19:47 ` Saravana Kannan 2021-01-20 23:53 ` Michael Walle 2021-01-25 16:50 ` Lorenzo Pieralisi 1 sibling, 2 replies; 30+ messages in thread From: Saravana Kannan @ 2021-01-20 19:47 UTC (permalink / raw) To: Michael Walle Cc: Rob Herring, linuxppc-dev, PCI, linux-arm-kernel, LKML, Minghuan Lian, Mingkai Hu, Roy Zang, Lorenzo Pieralisi, Bjorn Helgaas, Greg Kroah-Hartman On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc> wrote: > > [RESEND, fat-fingered the buttons of my mail client and converted > all CCs to BCCs :(] > > Am 2021-01-20 20:02, schrieb Saravana Kannan: > > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: > >> > >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> > >> wrote: > >> > > >> > fw_devlink will defer the probe until all suppliers are ready. We can't > >> > use builtin_platform_driver_probe() because it doesn't retry after probe > >> > deferral. Convert it to builtin_platform_driver(). > >> > >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then > >> shouldn't it be fixed or removed? > > > > I was actually thinking about this too. The problem with fixing > > builtin_platform_driver_probe() to behave like > > builtin_platform_driver() is that these probe functions could be > > marked with __init. But there are also only 20 instances of > > builtin_platform_driver_probe() in the kernel: > > $ git grep ^builtin_platform_driver_probe | wc -l > > 20 > > > > So it might be easier to just fix them to not use > > builtin_platform_driver_probe(). > > > > Michael, > > > > Any chance you'd be willing to help me by converting all these to > > builtin_platform_driver() and delete builtin_platform_driver_probe()? > > If it just moving the probe function to the _driver struct and > remove the __init annotations. I could look into that. Yup. That's pretty much it AFAICT. builtin_platform_driver_probe() also makes sure the driver doesn't ask for async probe, etc. But I doubt anyone is actually setting async flags and still using builtin_platform_driver_probe(). -Saravana ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-20 19:47 ` Saravana Kannan @ 2021-01-20 19:47 ` Saravana Kannan 2021-01-20 23:53 ` Michael Walle 1 sibling, 0 replies; 30+ messages in thread From: Saravana Kannan @ 2021-01-20 19:47 UTC (permalink / raw) To: Michael Walle Cc: Rob Herring, linuxppc-dev, PCI, linux-arm-kernel, LKML, Minghuan Lian, Mingkai Hu, Roy Zang, Lorenzo Pieralisi, Bjorn Helgaas, Greg Kroah-Hartman On Wed, Jan 20, 2021 at 11:47 AM Saravana Kannan <saravanak@google.com> wrote: > > On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc> wrote: > > > > [RESEND, fat-fingered the buttons of my mail client and converted > > all CCs to BCCs :(] > > > > Am 2021-01-20 20:02, schrieb Saravana Kannan: > > > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: > > >> > > >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> > > >> wrote: > > >> > > > >> > fw_devlink will defer the probe until all suppliers are ready. We can't > > >> > use builtin_platform_driver_probe() because it doesn't retry after probe > > >> > deferral. Convert it to builtin_platform_driver(). > > >> > > >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then > > >> shouldn't it be fixed or removed? > > > > > > I was actually thinking about this too. The problem with fixing > > > builtin_platform_driver_probe() to behave like > > > builtin_platform_driver() is that these probe functions could be > > > marked with __init. But there are also only 20 instances of > > > builtin_platform_driver_probe() in the kernel: > > > $ git grep ^builtin_platform_driver_probe | wc -l > > > 20 > > > > > > So it might be easier to just fix them to not use > > > builtin_platform_driver_probe(). > > > > > > Michael, > > > > > > Any chance you'd be willing to help me by converting all these to > > > builtin_platform_driver() and delete builtin_platform_driver_probe()? > > > > If it just moving the probe function to the _driver struct and > > remove the __init annotations. I could look into that. > > Yup. That's pretty much it AFAICT. > > builtin_platform_driver_probe() also makes sure the driver doesn't ask > for async probe, etc. But I doubt anyone is actually setting async > flags and still using builtin_platform_driver_probe(). > And thanks for agreeing to help! -Saravana ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-20 19:47 ` Saravana Kannan 2021-01-20 19:47 ` Saravana Kannan @ 2021-01-20 23:53 ` Michael Walle 2021-01-20 23:58 ` Saravana Kannan 1 sibling, 1 reply; 30+ messages in thread From: Michael Walle @ 2021-01-20 23:53 UTC (permalink / raw) To: Saravana Kannan Cc: Rob Herring, linuxppc-dev, PCI, linux-arm-kernel, LKML, Minghuan Lian, Mingkai Hu, Roy Zang, Lorenzo Pieralisi, Bjorn Helgaas, Greg Kroah-Hartman Am 2021-01-20 20:47, schrieb Saravana Kannan: > On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc> > wrote: >> >> [RESEND, fat-fingered the buttons of my mail client and converted >> all CCs to BCCs :(] >> >> Am 2021-01-20 20:02, schrieb Saravana Kannan: >> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: >> >> >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> >> >> wrote: >> >> > >> >> > fw_devlink will defer the probe until all suppliers are ready. We can't >> >> > use builtin_platform_driver_probe() because it doesn't retry after probe >> >> > deferral. Convert it to builtin_platform_driver(). >> >> >> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then >> >> shouldn't it be fixed or removed? >> > >> > I was actually thinking about this too. The problem with fixing >> > builtin_platform_driver_probe() to behave like >> > builtin_platform_driver() is that these probe functions could be >> > marked with __init. But there are also only 20 instances of >> > builtin_platform_driver_probe() in the kernel: >> > $ git grep ^builtin_platform_driver_probe | wc -l >> > 20 >> > >> > So it might be easier to just fix them to not use >> > builtin_platform_driver_probe(). >> > >> > Michael, >> > >> > Any chance you'd be willing to help me by converting all these to >> > builtin_platform_driver() and delete builtin_platform_driver_probe()? >> >> If it just moving the probe function to the _driver struct and >> remove the __init annotations. I could look into that. > > Yup. That's pretty much it AFAICT. > > builtin_platform_driver_probe() also makes sure the driver doesn't ask > for async probe, etc. But I doubt anyone is actually setting async > flags and still using builtin_platform_driver_probe(). Hasn't module_platform_driver_probe() the same problem? And there are ~80 drivers which uses that. -michael ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-20 23:53 ` Michael Walle @ 2021-01-20 23:58 ` Saravana Kannan 2021-01-21 11:01 ` Geert Uytterhoeven 0 siblings, 1 reply; 30+ messages in thread From: Saravana Kannan @ 2021-01-20 23:58 UTC (permalink / raw) To: Michael Walle Cc: Rob Herring, linuxppc-dev, PCI, linux-arm-kernel, LKML, Minghuan Lian, Mingkai Hu, Roy Zang, Lorenzo Pieralisi, Bjorn Helgaas, Greg Kroah-Hartman On Wed, Jan 20, 2021 at 3:53 PM Michael Walle <michael@walle.cc> wrote: > > Am 2021-01-20 20:47, schrieb Saravana Kannan: > > On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc> > > wrote: > >> > >> [RESEND, fat-fingered the buttons of my mail client and converted > >> all CCs to BCCs :(] > >> > >> Am 2021-01-20 20:02, schrieb Saravana Kannan: > >> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: > >> >> > >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> > >> >> wrote: > >> >> > > >> >> > fw_devlink will defer the probe until all suppliers are ready. We can't > >> >> > use builtin_platform_driver_probe() because it doesn't retry after probe > >> >> > deferral. Convert it to builtin_platform_driver(). > >> >> > >> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then > >> >> shouldn't it be fixed or removed? > >> > > >> > I was actually thinking about this too. The problem with fixing > >> > builtin_platform_driver_probe() to behave like > >> > builtin_platform_driver() is that these probe functions could be > >> > marked with __init. But there are also only 20 instances of > >> > builtin_platform_driver_probe() in the kernel: > >> > $ git grep ^builtin_platform_driver_probe | wc -l > >> > 20 > >> > > >> > So it might be easier to just fix them to not use > >> > builtin_platform_driver_probe(). > >> > > >> > Michael, > >> > > >> > Any chance you'd be willing to help me by converting all these to > >> > builtin_platform_driver() and delete builtin_platform_driver_probe()? > >> > >> If it just moving the probe function to the _driver struct and > >> remove the __init annotations. I could look into that. > > > > Yup. That's pretty much it AFAICT. > > > > builtin_platform_driver_probe() also makes sure the driver doesn't ask > > for async probe, etc. But I doubt anyone is actually setting async > > flags and still using builtin_platform_driver_probe(). > > Hasn't module_platform_driver_probe() the same problem? And there > are ~80 drivers which uses that. Yeah. The biggest problem with all of these is the __init markers. Maybe some familiar with coccinelle can help? -Saravana ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-20 23:58 ` Saravana Kannan @ 2021-01-21 11:01 ` Geert Uytterhoeven 2021-01-25 19:49 ` Michael Walle 0 siblings, 1 reply; 30+ messages in thread From: Geert Uytterhoeven @ 2021-01-21 11:01 UTC (permalink / raw) To: Saravana Kannan Cc: Michael Walle, Lorenzo Pieralisi, Roy Zang, PCI, LKML, Minghuan Lian, Mingkai Hu, Greg Kroah-Hartman, Bjorn Helgaas, linuxppc-dev, linux-arm-kernel Hi Saravana, On Thu, Jan 21, 2021 at 1:05 AM Saravana Kannan <saravanak@google.com> wrote: > On Wed, Jan 20, 2021 at 3:53 PM Michael Walle <michael@walle.cc> wrote: > > Am 2021-01-20 20:47, schrieb Saravana Kannan: > > > On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc> > > > wrote: > > >> > > >> [RESEND, fat-fingered the buttons of my mail client and converted > > >> all CCs to BCCs :(] > > >> > > >> Am 2021-01-20 20:02, schrieb Saravana Kannan: > > >> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: > > >> >> > > >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> > > >> >> wrote: > > >> >> > > > >> >> > fw_devlink will defer the probe until all suppliers are ready. We can't > > >> >> > use builtin_platform_driver_probe() because it doesn't retry after probe > > >> >> > deferral. Convert it to builtin_platform_driver(). > > >> >> > > >> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then > > >> >> shouldn't it be fixed or removed? > > >> > > > >> > I was actually thinking about this too. The problem with fixing > > >> > builtin_platform_driver_probe() to behave like > > >> > builtin_platform_driver() is that these probe functions could be > > >> > marked with __init. But there are also only 20 instances of > > >> > builtin_platform_driver_probe() in the kernel: > > >> > $ git grep ^builtin_platform_driver_probe | wc -l > > >> > 20 > > >> > > > >> > So it might be easier to just fix them to not use > > >> > builtin_platform_driver_probe(). > > >> > > > >> > Michael, > > >> > > > >> > Any chance you'd be willing to help me by converting all these to > > >> > builtin_platform_driver() and delete builtin_platform_driver_probe()? > > >> > > >> If it just moving the probe function to the _driver struct and > > >> remove the __init annotations. I could look into that. > > > > > > Yup. That's pretty much it AFAICT. > > > > > > builtin_platform_driver_probe() also makes sure the driver doesn't ask > > > for async probe, etc. But I doubt anyone is actually setting async > > > flags and still using builtin_platform_driver_probe(). > > > > Hasn't module_platform_driver_probe() the same problem? And there > > are ~80 drivers which uses that. > > Yeah. The biggest problem with all of these is the __init markers. > Maybe some familiar with coccinelle can help? And dropping them will increase memory usage. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-21 11:01 ` Geert Uytterhoeven @ 2021-01-25 19:49 ` Michael Walle 2021-01-25 22:41 ` Saravana Kannan 2021-01-28 10:00 ` Tony Lindgren 0 siblings, 2 replies; 30+ messages in thread From: Michael Walle @ 2021-01-25 19:49 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Saravana Kannan, Lorenzo Pieralisi, Roy Zang, PCI, LKML, Minghuan Lian, Mingkai Hu, Greg Kroah-Hartman, Bjorn Helgaas, linuxppc-dev, linux-arm-kernel Am 2021-01-21 12:01, schrieb Geert Uytterhoeven: > Hi Saravana, > > On Thu, Jan 21, 2021 at 1:05 AM Saravana Kannan <saravanak@google.com> > wrote: >> On Wed, Jan 20, 2021 at 3:53 PM Michael Walle <michael@walle.cc> >> wrote: >> > Am 2021-01-20 20:47, schrieb Saravana Kannan: >> > > On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc> >> > > wrote: >> > >> >> > >> [RESEND, fat-fingered the buttons of my mail client and converted >> > >> all CCs to BCCs :(] >> > >> >> > >> Am 2021-01-20 20:02, schrieb Saravana Kannan: >> > >> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: >> > >> >> >> > >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> >> > >> >> wrote: >> > >> >> > >> > >> >> > fw_devlink will defer the probe until all suppliers are ready. We can't >> > >> >> > use builtin_platform_driver_probe() because it doesn't retry after probe >> > >> >> > deferral. Convert it to builtin_platform_driver(). >> > >> >> >> > >> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then >> > >> >> shouldn't it be fixed or removed? >> > >> > >> > >> > I was actually thinking about this too. The problem with fixing >> > >> > builtin_platform_driver_probe() to behave like >> > >> > builtin_platform_driver() is that these probe functions could be >> > >> > marked with __init. But there are also only 20 instances of >> > >> > builtin_platform_driver_probe() in the kernel: >> > >> > $ git grep ^builtin_platform_driver_probe | wc -l >> > >> > 20 >> > >> > >> > >> > So it might be easier to just fix them to not use >> > >> > builtin_platform_driver_probe(). >> > >> > >> > >> > Michael, >> > >> > >> > >> > Any chance you'd be willing to help me by converting all these to >> > >> > builtin_platform_driver() and delete builtin_platform_driver_probe()? >> > >> >> > >> If it just moving the probe function to the _driver struct and >> > >> remove the __init annotations. I could look into that. >> > > >> > > Yup. That's pretty much it AFAICT. >> > > >> > > builtin_platform_driver_probe() also makes sure the driver doesn't ask >> > > for async probe, etc. But I doubt anyone is actually setting async >> > > flags and still using builtin_platform_driver_probe(). >> > >> > Hasn't module_platform_driver_probe() the same problem? And there >> > are ~80 drivers which uses that. >> >> Yeah. The biggest problem with all of these is the __init markers. >> Maybe some familiar with coccinelle can help? > > And dropping them will increase memory usage. Although I do have the changes for the builtin_platform_driver_probe() ready, I don't think it makes much sense to send these unless we agree on the increased memory footprint. While there are just a few builtin_platform_driver_probe() and memory increase _might_ be negligible, there are many more module_platform_driver_probe(). -michael ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-25 19:49 ` Michael Walle @ 2021-01-25 22:41 ` Saravana Kannan 2021-01-26 8:50 ` Geert Uytterhoeven 2021-01-28 10:00 ` Tony Lindgren 1 sibling, 1 reply; 30+ messages in thread From: Saravana Kannan @ 2021-01-25 22:41 UTC (permalink / raw) To: Michael Walle Cc: Geert Uytterhoeven, Lorenzo Pieralisi, Roy Zang, PCI, LKML, Minghuan Lian, Mingkai Hu, Greg Kroah-Hartman, Bjorn Helgaas, linuxppc-dev, linux-arm-kernel On Mon, Jan 25, 2021 at 11:49 AM Michael Walle <michael@walle.cc> wrote: > > Am 2021-01-21 12:01, schrieb Geert Uytterhoeven: > > Hi Saravana, > > > > On Thu, Jan 21, 2021 at 1:05 AM Saravana Kannan <saravanak@google.com> > > wrote: > >> On Wed, Jan 20, 2021 at 3:53 PM Michael Walle <michael@walle.cc> > >> wrote: > >> > Am 2021-01-20 20:47, schrieb Saravana Kannan: > >> > > On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc> > >> > > wrote: > >> > >> > >> > >> [RESEND, fat-fingered the buttons of my mail client and converted > >> > >> all CCs to BCCs :(] > >> > >> > >> > >> Am 2021-01-20 20:02, schrieb Saravana Kannan: > >> > >> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: > >> > >> >> > >> > >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> > >> > >> >> wrote: > >> > >> >> > > >> > >> >> > fw_devlink will defer the probe until all suppliers are ready. We can't > >> > >> >> > use builtin_platform_driver_probe() because it doesn't retry after probe > >> > >> >> > deferral. Convert it to builtin_platform_driver(). > >> > >> >> > >> > >> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then > >> > >> >> shouldn't it be fixed or removed? > >> > >> > > >> > >> > I was actually thinking about this too. The problem with fixing > >> > >> > builtin_platform_driver_probe() to behave like > >> > >> > builtin_platform_driver() is that these probe functions could be > >> > >> > marked with __init. But there are also only 20 instances of > >> > >> > builtin_platform_driver_probe() in the kernel: > >> > >> > $ git grep ^builtin_platform_driver_probe | wc -l > >> > >> > 20 > >> > >> > > >> > >> > So it might be easier to just fix them to not use > >> > >> > builtin_platform_driver_probe(). > >> > >> > > >> > >> > Michael, > >> > >> > > >> > >> > Any chance you'd be willing to help me by converting all these to > >> > >> > builtin_platform_driver() and delete builtin_platform_driver_probe()? > >> > >> > >> > >> If it just moving the probe function to the _driver struct and > >> > >> remove the __init annotations. I could look into that. > >> > > > >> > > Yup. That's pretty much it AFAICT. > >> > > > >> > > builtin_platform_driver_probe() also makes sure the driver doesn't ask > >> > > for async probe, etc. But I doubt anyone is actually setting async > >> > > flags and still using builtin_platform_driver_probe(). > >> > > >> > Hasn't module_platform_driver_probe() the same problem? And there > >> > are ~80 drivers which uses that. > >> > >> Yeah. The biggest problem with all of these is the __init markers. > >> Maybe some familiar with coccinelle can help? > > > > And dropping them will increase memory usage. > > Although I do have the changes for the builtin_platform_driver_probe() > ready, I don't think it makes much sense to send these unless we agree > on the increased memory footprint. While there are just a few > builtin_platform_driver_probe() and memory increase _might_ be > negligible, there are many more module_platform_driver_probe(). While it's good to drop code that'll not be used past kernel init, the module_platform_driver_probe() is going even more extreme. It doesn't even allow deferred probe (well before kernel init is done). I don't think that behavior is right and that's why we should delete it. Also, I doubt if any of these probe functions even take up 4KB of memory. -Saravana ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-25 22:41 ` Saravana Kannan @ 2021-01-26 8:50 ` Geert Uytterhoeven 2021-01-27 0:44 ` Saravana Kannan 0 siblings, 1 reply; 30+ messages in thread From: Geert Uytterhoeven @ 2021-01-26 8:50 UTC (permalink / raw) To: Saravana Kannan Cc: Michael Walle, Lorenzo Pieralisi, Roy Zang, PCI, LKML, Minghuan Lian, Mingkai Hu, Greg Kroah-Hartman, Bjorn Helgaas, linuxppc-dev, linux-arm-kernel Hi Saravana, On Mon, Jan 25, 2021 at 11:42 PM Saravana Kannan <saravanak@google.com> wrote: > On Mon, Jan 25, 2021 at 11:49 AM Michael Walle <michael@walle.cc> wrote: > > Am 2021-01-21 12:01, schrieb Geert Uytterhoeven: > > > On Thu, Jan 21, 2021 at 1:05 AM Saravana Kannan <saravanak@google.com> > > > wrote: > > >> On Wed, Jan 20, 2021 at 3:53 PM Michael Walle <michael@walle.cc> > > >> wrote: > > >> > Am 2021-01-20 20:47, schrieb Saravana Kannan: > > >> > > On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc> > > >> > > wrote: > > >> > >> > > >> > >> [RESEND, fat-fingered the buttons of my mail client and converted > > >> > >> all CCs to BCCs :(] > > >> > >> > > >> > >> Am 2021-01-20 20:02, schrieb Saravana Kannan: > > >> > >> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: > > >> > >> >> > > >> > >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> > > >> > >> >> wrote: > > >> > >> >> > > > >> > >> >> > fw_devlink will defer the probe until all suppliers are ready. We can't > > >> > >> >> > use builtin_platform_driver_probe() because it doesn't retry after probe > > >> > >> >> > deferral. Convert it to builtin_platform_driver(). > > >> > >> >> > > >> > >> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then > > >> > >> >> shouldn't it be fixed or removed? > > >> > >> > > > >> > >> > I was actually thinking about this too. The problem with fixing > > >> > >> > builtin_platform_driver_probe() to behave like > > >> > >> > builtin_platform_driver() is that these probe functions could be > > >> > >> > marked with __init. But there are also only 20 instances of > > >> > >> > builtin_platform_driver_probe() in the kernel: > > >> > >> > $ git grep ^builtin_platform_driver_probe | wc -l > > >> > >> > 20 > > >> > >> > > > >> > >> > So it might be easier to just fix them to not use > > >> > >> > builtin_platform_driver_probe(). > > >> > >> > > > >> > >> > Michael, > > >> > >> > > > >> > >> > Any chance you'd be willing to help me by converting all these to > > >> > >> > builtin_platform_driver() and delete builtin_platform_driver_probe()? > > >> > >> > > >> > >> If it just moving the probe function to the _driver struct and > > >> > >> remove the __init annotations. I could look into that. > > >> > > > > >> > > Yup. That's pretty much it AFAICT. > > >> > > > > >> > > builtin_platform_driver_probe() also makes sure the driver doesn't ask > > >> > > for async probe, etc. But I doubt anyone is actually setting async > > >> > > flags and still using builtin_platform_driver_probe(). > > >> > > > >> > Hasn't module_platform_driver_probe() the same problem? And there > > >> > are ~80 drivers which uses that. > > >> > > >> Yeah. The biggest problem with all of these is the __init markers. > > >> Maybe some familiar with coccinelle can help? > > > > > > And dropping them will increase memory usage. > > > > Although I do have the changes for the builtin_platform_driver_probe() > > ready, I don't think it makes much sense to send these unless we agree > > on the increased memory footprint. While there are just a few > > builtin_platform_driver_probe() and memory increase _might_ be > > negligible, there are many more module_platform_driver_probe(). > > While it's good to drop code that'll not be used past kernel init, the > module_platform_driver_probe() is going even more extreme. It doesn't > even allow deferred probe (well before kernel init is done). I don't > think that behavior is right and that's why we should delete it. Also, This construct is typically used for builtin hardware for which the dependencies are registered very early, and thus known to probe at first try (if present). > I doubt if any of these probe functions even take up 4KB of memory. How many 4 KiB pages do you have in a system with 10 MiB of SRAM? How many can you afford to waste? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-26 8:50 ` Geert Uytterhoeven @ 2021-01-27 0:44 ` Saravana Kannan 2021-01-27 7:43 ` Geert Uytterhoeven 0 siblings, 1 reply; 30+ messages in thread From: Saravana Kannan @ 2021-01-27 0:44 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Michael Walle, Lorenzo Pieralisi, Roy Zang, PCI, LKML, Minghuan Lian, Mingkai Hu, Greg Kroah-Hartman, Bjorn Helgaas, linuxppc-dev, linux-arm-kernel On Tue, Jan 26, 2021 at 12:50 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Saravana, > > On Mon, Jan 25, 2021 at 11:42 PM Saravana Kannan <saravanak@google.com> wrote: > > On Mon, Jan 25, 2021 at 11:49 AM Michael Walle <michael@walle.cc> wrote: > > > Am 2021-01-21 12:01, schrieb Geert Uytterhoeven: > > > > On Thu, Jan 21, 2021 at 1:05 AM Saravana Kannan <saravanak@google.com> > > > > wrote: > > > >> On Wed, Jan 20, 2021 at 3:53 PM Michael Walle <michael@walle.cc> > > > >> wrote: > > > >> > Am 2021-01-20 20:47, schrieb Saravana Kannan: > > > >> > > On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc> > > > >> > > wrote: > > > >> > >> > > > >> > >> [RESEND, fat-fingered the buttons of my mail client and converted > > > >> > >> all CCs to BCCs :(] > > > >> > >> > > > >> > >> Am 2021-01-20 20:02, schrieb Saravana Kannan: > > > >> > >> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: > > > >> > >> >> > > > >> > >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> > > > >> > >> >> wrote: > > > >> > >> >> > > > > >> > >> >> > fw_devlink will defer the probe until all suppliers are ready. We can't > > > >> > >> >> > use builtin_platform_driver_probe() because it doesn't retry after probe > > > >> > >> >> > deferral. Convert it to builtin_platform_driver(). > > > >> > >> >> > > > >> > >> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then > > > >> > >> >> shouldn't it be fixed or removed? > > > >> > >> > > > > >> > >> > I was actually thinking about this too. The problem with fixing > > > >> > >> > builtin_platform_driver_probe() to behave like > > > >> > >> > builtin_platform_driver() is that these probe functions could be > > > >> > >> > marked with __init. But there are also only 20 instances of > > > >> > >> > builtin_platform_driver_probe() in the kernel: > > > >> > >> > $ git grep ^builtin_platform_driver_probe | wc -l > > > >> > >> > 20 > > > >> > >> > > > > >> > >> > So it might be easier to just fix them to not use > > > >> > >> > builtin_platform_driver_probe(). > > > >> > >> > > > > >> > >> > Michael, > > > >> > >> > > > > >> > >> > Any chance you'd be willing to help me by converting all these to > > > >> > >> > builtin_platform_driver() and delete builtin_platform_driver_probe()? > > > >> > >> > > > >> > >> If it just moving the probe function to the _driver struct and > > > >> > >> remove the __init annotations. I could look into that. > > > >> > > > > > >> > > Yup. That's pretty much it AFAICT. > > > >> > > > > > >> > > builtin_platform_driver_probe() also makes sure the driver doesn't ask > > > >> > > for async probe, etc. But I doubt anyone is actually setting async > > > >> > > flags and still using builtin_platform_driver_probe(). > > > >> > > > > >> > Hasn't module_platform_driver_probe() the same problem? And there > > > >> > are ~80 drivers which uses that. > > > >> > > > >> Yeah. The biggest problem with all of these is the __init markers. > > > >> Maybe some familiar with coccinelle can help? > > > > > > > > And dropping them will increase memory usage. > > > > > > Although I do have the changes for the builtin_platform_driver_probe() > > > ready, I don't think it makes much sense to send these unless we agree > > > on the increased memory footprint. While there are just a few > > > builtin_platform_driver_probe() and memory increase _might_ be > > > negligible, there are many more module_platform_driver_probe(). > > > > While it's good to drop code that'll not be used past kernel init, the > > module_platform_driver_probe() is going even more extreme. It doesn't > > even allow deferred probe (well before kernel init is done). I don't > > think that behavior is right and that's why we should delete it. Also, > > This construct is typically used for builtin hardware for which the > dependencies are registered very early, and thus known to probe at > first try (if present). > > > I doubt if any of these probe functions even take up 4KB of memory. > > How many 4 KiB pages do you have in a system with 10 MiB of SRAM? > How many can you afford to waste? There are only a few instances of this macro in the kernel. How many of those actually fit the description above? We can probably just check the DT? -Saravana ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-27 0:44 ` Saravana Kannan @ 2021-01-27 7:43 ` Geert Uytterhoeven 2021-01-27 16:41 ` Saravana Kannan 0 siblings, 1 reply; 30+ messages in thread From: Geert Uytterhoeven @ 2021-01-27 7:43 UTC (permalink / raw) To: Saravana Kannan Cc: Michael Walle, Lorenzo Pieralisi, Roy Zang, PCI, LKML, Minghuan Lian, Mingkai Hu, Greg Kroah-Hartman, Bjorn Helgaas, linuxppc-dev, linux-arm-kernel Hi Saravana, On Wed, Jan 27, 2021 at 1:44 AM Saravana Kannan <saravanak@google.com> wrote: > On Tue, Jan 26, 2021 at 12:50 AM Geert Uytterhoeven > <geert@linux-m68k.org> wrote: > > On Mon, Jan 25, 2021 at 11:42 PM Saravana Kannan <saravanak@google.com> wrote: > > > On Mon, Jan 25, 2021 at 11:49 AM Michael Walle <michael@walle.cc> wrote: > > > > Am 2021-01-21 12:01, schrieb Geert Uytterhoeven: > > > > > On Thu, Jan 21, 2021 at 1:05 AM Saravana Kannan <saravanak@google.com> > > > > > wrote: > > > > >> On Wed, Jan 20, 2021 at 3:53 PM Michael Walle <michael@walle.cc> > > > > >> wrote: > > > > >> > Am 2021-01-20 20:47, schrieb Saravana Kannan: > > > > >> > > On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc> > > > > >> > > wrote: > > > > >> > >> > > > > >> > >> [RESEND, fat-fingered the buttons of my mail client and converted > > > > >> > >> all CCs to BCCs :(] > > > > >> > >> > > > > >> > >> Am 2021-01-20 20:02, schrieb Saravana Kannan: > > > > >> > >> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: > > > > >> > >> >> > > > > >> > >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> > > > > >> > >> >> wrote: > > > > >> > >> >> > > > > > >> > >> >> > fw_devlink will defer the probe until all suppliers are ready. We can't > > > > >> > >> >> > use builtin_platform_driver_probe() because it doesn't retry after probe > > > > >> > >> >> > deferral. Convert it to builtin_platform_driver(). > > > > >> > >> >> > > > > >> > >> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then > > > > >> > >> >> shouldn't it be fixed or removed? > > > > >> > >> > > > > > >> > >> > I was actually thinking about this too. The problem with fixing > > > > >> > >> > builtin_platform_driver_probe() to behave like > > > > >> > >> > builtin_platform_driver() is that these probe functions could be > > > > >> > >> > marked with __init. But there are also only 20 instances of > > > > >> > >> > builtin_platform_driver_probe() in the kernel: > > > > >> > >> > $ git grep ^builtin_platform_driver_probe | wc -l > > > > >> > >> > 20 > > > > >> > >> > > > > > >> > >> > So it might be easier to just fix them to not use > > > > >> > >> > builtin_platform_driver_probe(). > > > > >> > >> > > > > > >> > >> > Michael, > > > > >> > >> > > > > > >> > >> > Any chance you'd be willing to help me by converting all these to > > > > >> > >> > builtin_platform_driver() and delete builtin_platform_driver_probe()? > > > > >> > >> > > > > >> > >> If it just moving the probe function to the _driver struct and > > > > >> > >> remove the __init annotations. I could look into that. > > > > >> > > > > > > >> > > Yup. That's pretty much it AFAICT. > > > > >> > > > > > > >> > > builtin_platform_driver_probe() also makes sure the driver doesn't ask > > > > >> > > for async probe, etc. But I doubt anyone is actually setting async > > > > >> > > flags and still using builtin_platform_driver_probe(). > > > > >> > > > > > >> > Hasn't module_platform_driver_probe() the same problem? And there > > > > >> > are ~80 drivers which uses that. > > > > >> > > > > >> Yeah. The biggest problem with all of these is the __init markers. > > > > >> Maybe some familiar with coccinelle can help? > > > > > > > > > > And dropping them will increase memory usage. > > > > > > > > Although I do have the changes for the builtin_platform_driver_probe() > > > > ready, I don't think it makes much sense to send these unless we agree > > > > on the increased memory footprint. While there are just a few > > > > builtin_platform_driver_probe() and memory increase _might_ be > > > > negligible, there are many more module_platform_driver_probe(). > > > > > > While it's good to drop code that'll not be used past kernel init, the > > > module_platform_driver_probe() is going even more extreme. It doesn't > > > even allow deferred probe (well before kernel init is done). I don't > > > think that behavior is right and that's why we should delete it. Also, > > > > This construct is typically used for builtin hardware for which the > > dependencies are registered very early, and thus known to probe at > > first try (if present). > > > > > I doubt if any of these probe functions even take up 4KB of memory. > > > > How many 4 KiB pages do you have in a system with 10 MiB of SRAM? > > How many can you afford to waste? > > There are only a few instances of this macro in the kernel. How many $ git grep -lw builtin_platform_driver_probe | wc -l 21 $ git grep -lw module_platform_driver_probe | wc -l 86 + the ones that haven't been converted to the above yet: $ git grep -lw platform_driver_probe | wc -l 58 > of those actually fit the description above? We can probably just > check the DT? What do you mean by checking the DT? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-27 7:43 ` Geert Uytterhoeven @ 2021-01-27 16:41 ` Saravana Kannan 2021-01-27 16:56 ` Geert Uytterhoeven 0 siblings, 1 reply; 30+ messages in thread From: Saravana Kannan @ 2021-01-27 16:41 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Michael Walle, Lorenzo Pieralisi, Roy Zang, PCI, LKML, Minghuan Lian, Mingkai Hu, Greg Kroah-Hartman, Bjorn Helgaas, linuxppc-dev, linux-arm-kernel On Tue, Jan 26, 2021 at 11:43 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Saravana, > > On Wed, Jan 27, 2021 at 1:44 AM Saravana Kannan <saravanak@google.com> wrote: > > On Tue, Jan 26, 2021 at 12:50 AM Geert Uytterhoeven > > <geert@linux-m68k.org> wrote: > > > On Mon, Jan 25, 2021 at 11:42 PM Saravana Kannan <saravanak@google.com> wrote: > > > > On Mon, Jan 25, 2021 at 11:49 AM Michael Walle <michael@walle.cc> wrote: > > > > > Am 2021-01-21 12:01, schrieb Geert Uytterhoeven: > > > > > > On Thu, Jan 21, 2021 at 1:05 AM Saravana Kannan <saravanak@google.com> > > > > > > wrote: > > > > > >> On Wed, Jan 20, 2021 at 3:53 PM Michael Walle <michael@walle.cc> > > > > > >> wrote: > > > > > >> > Am 2021-01-20 20:47, schrieb Saravana Kannan: > > > > > >> > > On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc> > > > > > >> > > wrote: > > > > > >> > >> > > > > > >> > >> [RESEND, fat-fingered the buttons of my mail client and converted > > > > > >> > >> all CCs to BCCs :(] > > > > > >> > >> > > > > > >> > >> Am 2021-01-20 20:02, schrieb Saravana Kannan: > > > > > >> > >> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: > > > > > >> > >> >> > > > > > >> > >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> > > > > > >> > >> >> wrote: > > > > > >> > >> >> > > > > > > >> > >> >> > fw_devlink will defer the probe until all suppliers are ready. We can't > > > > > >> > >> >> > use builtin_platform_driver_probe() because it doesn't retry after probe > > > > > >> > >> >> > deferral. Convert it to builtin_platform_driver(). > > > > > >> > >> >> > > > > > >> > >> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then > > > > > >> > >> >> shouldn't it be fixed or removed? > > > > > >> > >> > > > > > > >> > >> > I was actually thinking about this too. The problem with fixing > > > > > >> > >> > builtin_platform_driver_probe() to behave like > > > > > >> > >> > builtin_platform_driver() is that these probe functions could be > > > > > >> > >> > marked with __init. But there are also only 20 instances of > > > > > >> > >> > builtin_platform_driver_probe() in the kernel: > > > > > >> > >> > $ git grep ^builtin_platform_driver_probe | wc -l > > > > > >> > >> > 20 > > > > > >> > >> > > > > > > >> > >> > So it might be easier to just fix them to not use > > > > > >> > >> > builtin_platform_driver_probe(). > > > > > >> > >> > > > > > > >> > >> > Michael, > > > > > >> > >> > > > > > > >> > >> > Any chance you'd be willing to help me by converting all these to > > > > > >> > >> > builtin_platform_driver() and delete builtin_platform_driver_probe()? > > > > > >> > >> > > > > > >> > >> If it just moving the probe function to the _driver struct and > > > > > >> > >> remove the __init annotations. I could look into that. > > > > > >> > > > > > > > >> > > Yup. That's pretty much it AFAICT. > > > > > >> > > > > > > > >> > > builtin_platform_driver_probe() also makes sure the driver doesn't ask > > > > > >> > > for async probe, etc. But I doubt anyone is actually setting async > > > > > >> > > flags and still using builtin_platform_driver_probe(). > > > > > >> > > > > > > >> > Hasn't module_platform_driver_probe() the same problem? And there > > > > > >> > are ~80 drivers which uses that. > > > > > >> > > > > > >> Yeah. The biggest problem with all of these is the __init markers. > > > > > >> Maybe some familiar with coccinelle can help? > > > > > > > > > > > > And dropping them will increase memory usage. > > > > > > > > > > Although I do have the changes for the builtin_platform_driver_probe() > > > > > ready, I don't think it makes much sense to send these unless we agree > > > > > on the increased memory footprint. While there are just a few > > > > > builtin_platform_driver_probe() and memory increase _might_ be > > > > > negligible, there are many more module_platform_driver_probe(). > > > > > > > > While it's good to drop code that'll not be used past kernel init, the > > > > module_platform_driver_probe() is going even more extreme. It doesn't > > > > even allow deferred probe (well before kernel init is done). I don't > > > > think that behavior is right and that's why we should delete it. Also, > > > > > > This construct is typically used for builtin hardware for which the > > > dependencies are registered very early, and thus known to probe at > > > first try (if present). > > > > > > > I doubt if any of these probe functions even take up 4KB of memory. > > > > > > How many 4 KiB pages do you have in a system with 10 MiB of SRAM? > > > How many can you afford to waste? > > > > There are only a few instances of this macro in the kernel. How many > > $ git grep -lw builtin_platform_driver_probe | wc -l > 21 > $ git grep -lw module_platform_driver_probe | wc -l > 86 > > + the ones that haven't been converted to the above yet: > > $ git grep -lw platform_driver_probe | wc -l > 58 > Yeah, this adds up in terms of the number of places we'd need to fix. But thinking more about it, a couple of points: 1. Not all builtin_platform_driver_probe() are problems for fw_devlink. So we can just fix them as we go if we need to. 2. The problem with builtin_platform_driver_probe() isn't really with the use of __init. It's the fact that it doesn't allow deferred probes. builtin_platform_driver_probe()/platform_driver_probe() could still be fixed up to allow deferred probe until we get to the point where we free the __init section (so at least till late_initcall). > > of those actually fit the description above? We can probably just > > check the DT? > > What do you mean by checking the DT? I was talking about checking the DT to see if the board has very little memory, but that's not always obvious from DT nor does it scale with the number of instances we have. So, ignore this comment. Anyway, time to get back to actually writing the code to deal with this and other corner cases. -Saravana ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-27 16:41 ` Saravana Kannan @ 2021-01-27 16:56 ` Geert Uytterhoeven 2021-01-27 17:10 ` Saravana Kannan 0 siblings, 1 reply; 30+ messages in thread From: Geert Uytterhoeven @ 2021-01-27 16:56 UTC (permalink / raw) To: Saravana Kannan Cc: Michael Walle, Lorenzo Pieralisi, Roy Zang, PCI, LKML, Minghuan Lian, Mingkai Hu, Greg Kroah-Hartman, Bjorn Helgaas, linuxppc-dev, linux-arm-kernel Hi Saravana, On Wed, Jan 27, 2021 at 5:42 PM Saravana Kannan <saravanak@google.com> wrote: > On Tue, Jan 26, 2021 at 11:43 PM Geert Uytterhoeven > <geert@linux-m68k.org> wrote: > > On Wed, Jan 27, 2021 at 1:44 AM Saravana Kannan <saravanak@google.com> wrote: > > > On Tue, Jan 26, 2021 at 12:50 AM Geert Uytterhoeven > > > <geert@linux-m68k.org> wrote: > > > > On Mon, Jan 25, 2021 at 11:42 PM Saravana Kannan <saravanak@google.com> wrote: > > > > > On Mon, Jan 25, 2021 at 11:49 AM Michael Walle <michael@walle.cc> wrote: > > > > > > Am 2021-01-21 12:01, schrieb Geert Uytterhoeven: > > > > > > > On Thu, Jan 21, 2021 at 1:05 AM Saravana Kannan <saravanak@google.com> > > > > > > > wrote: > > > > > > >> On Wed, Jan 20, 2021 at 3:53 PM Michael Walle <michael@walle.cc> > > > > > > >> wrote: > > > > > > >> > Am 2021-01-20 20:47, schrieb Saravana Kannan: > > > > > > >> > > On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc> > > > > > > >> > > wrote: > > > > > > >> > >> > > > > > > >> > >> [RESEND, fat-fingered the buttons of my mail client and converted > > > > > > >> > >> all CCs to BCCs :(] > > > > > > >> > >> > > > > > > >> > >> Am 2021-01-20 20:02, schrieb Saravana Kannan: > > > > > > >> > >> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: > > > > > > >> > >> >> > > > > > > >> > >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> > > > > > > >> > >> >> wrote: > > > > > > >> > >> >> > > > > > > > >> > >> >> > fw_devlink will defer the probe until all suppliers are ready. We can't > > > > > > >> > >> >> > use builtin_platform_driver_probe() because it doesn't retry after probe > > > > > > >> > >> >> > deferral. Convert it to builtin_platform_driver(). > > > > > > >> > >> >> > > > > > > >> > >> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then > > > > > > >> > >> >> shouldn't it be fixed or removed? > > > > > > >> > >> > > > > > > > >> > >> > I was actually thinking about this too. The problem with fixing > > > > > > >> > >> > builtin_platform_driver_probe() to behave like > > > > > > >> > >> > builtin_platform_driver() is that these probe functions could be > > > > > > >> > >> > marked with __init. But there are also only 20 instances of > > > > > > >> > >> > builtin_platform_driver_probe() in the kernel: > > > > > > >> > >> > $ git grep ^builtin_platform_driver_probe | wc -l > > > > > > >> > >> > 20 > > > > > > >> > >> > > > > > > > >> > >> > So it might be easier to just fix them to not use > > > > > > >> > >> > builtin_platform_driver_probe(). > > > > > > >> > >> > > > > > > > >> > >> > Michael, > > > > > > >> > >> > > > > > > > >> > >> > Any chance you'd be willing to help me by converting all these to > > > > > > >> > >> > builtin_platform_driver() and delete builtin_platform_driver_probe()? > > > > > > >> > >> > > > > > > >> > >> If it just moving the probe function to the _driver struct and > > > > > > >> > >> remove the __init annotations. I could look into that. > > > > > > >> > > > > > > > > >> > > Yup. That's pretty much it AFAICT. > > > > > > >> > > > > > > > > >> > > builtin_platform_driver_probe() also makes sure the driver doesn't ask > > > > > > >> > > for async probe, etc. But I doubt anyone is actually setting async > > > > > > >> > > flags and still using builtin_platform_driver_probe(). > > > > > > >> > > > > > > > >> > Hasn't module_platform_driver_probe() the same problem? And there > > > > > > >> > are ~80 drivers which uses that. > > > > > > >> > > > > > > >> Yeah. The biggest problem with all of these is the __init markers. > > > > > > >> Maybe some familiar with coccinelle can help? > > > > > > > > > > > > > > And dropping them will increase memory usage. > > > > > > > > > > > > Although I do have the changes for the builtin_platform_driver_probe() > > > > > > ready, I don't think it makes much sense to send these unless we agree > > > > > > on the increased memory footprint. While there are just a few > > > > > > builtin_platform_driver_probe() and memory increase _might_ be > > > > > > negligible, there are many more module_platform_driver_probe(). > > > > > > > > > > While it's good to drop code that'll not be used past kernel init, the > > > > > module_platform_driver_probe() is going even more extreme. It doesn't > > > > > even allow deferred probe (well before kernel init is done). I don't > > > > > think that behavior is right and that's why we should delete it. Also, > > > > > > > > This construct is typically used for builtin hardware for which the > > > > dependencies are registered very early, and thus known to probe at > > > > first try (if present). > > > > > > > > > I doubt if any of these probe functions even take up 4KB of memory. > > > > > > > > How many 4 KiB pages do you have in a system with 10 MiB of SRAM? > > > > How many can you afford to waste? > > > > > > There are only a few instances of this macro in the kernel. How many > > > > $ git grep -lw builtin_platform_driver_probe | wc -l > > 21 > > $ git grep -lw module_platform_driver_probe | wc -l > > 86 > > > > + the ones that haven't been converted to the above yet: > > > > $ git grep -lw platform_driver_probe | wc -l > > 58 > > > > Yeah, this adds up in terms of the number of places we'd need to fix. > But thinking more about it, a couple of points: > 1. Not all builtin_platform_driver_probe() are problems for > fw_devlink. So we can just fix them as we go if we need to. > > 2. The problem with builtin_platform_driver_probe() isn't really with > the use of __init. It's the fact that it doesn't allow deferred > probes. builtin_platform_driver_probe()/platform_driver_probe() could > still be fixed up to allow deferred probe until we get to the point > where we free the __init section (so at least till late_initcall). That's intentional: it is used for cases that will (must) never be deferred. That's why it's safe to use __init. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-27 16:56 ` Geert Uytterhoeven @ 2021-01-27 17:10 ` Saravana Kannan 2021-01-28 9:25 ` Geert Uytterhoeven 0 siblings, 1 reply; 30+ messages in thread From: Saravana Kannan @ 2021-01-27 17:10 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Michael Walle, Lorenzo Pieralisi, Roy Zang, PCI, LKML, Minghuan Lian, Mingkai Hu, Greg Kroah-Hartman, Bjorn Helgaas, linuxppc-dev, linux-arm-kernel On Wed, Jan 27, 2021 at 8:56 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Saravana, > > On Wed, Jan 27, 2021 at 5:42 PM Saravana Kannan <saravanak@google.com> wrote: > > On Tue, Jan 26, 2021 at 11:43 PM Geert Uytterhoeven > > <geert@linux-m68k.org> wrote: > > > On Wed, Jan 27, 2021 at 1:44 AM Saravana Kannan <saravanak@google.com> wrote: > > > > On Tue, Jan 26, 2021 at 12:50 AM Geert Uytterhoeven > > > > <geert@linux-m68k.org> wrote: > > > > > On Mon, Jan 25, 2021 at 11:42 PM Saravana Kannan <saravanak@google.com> wrote: > > > > > > On Mon, Jan 25, 2021 at 11:49 AM Michael Walle <michael@walle.cc> wrote: > > > > > > > Am 2021-01-21 12:01, schrieb Geert Uytterhoeven: > > > > > > > > On Thu, Jan 21, 2021 at 1:05 AM Saravana Kannan <saravanak@google.com> > > > > > > > > wrote: > > > > > > > >> On Wed, Jan 20, 2021 at 3:53 PM Michael Walle <michael@walle.cc> > > > > > > > >> wrote: > > > > > > > >> > Am 2021-01-20 20:47, schrieb Saravana Kannan: > > > > > > > >> > > On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc> > > > > > > > >> > > wrote: > > > > > > > >> > >> > > > > > > > >> > >> [RESEND, fat-fingered the buttons of my mail client and converted > > > > > > > >> > >> all CCs to BCCs :(] > > > > > > > >> > >> > > > > > > > >> > >> Am 2021-01-20 20:02, schrieb Saravana Kannan: > > > > > > > >> > >> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: > > > > > > > >> > >> >> > > > > > > > >> > >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> > > > > > > > >> > >> >> wrote: > > > > > > > >> > >> >> > > > > > > > > >> > >> >> > fw_devlink will defer the probe until all suppliers are ready. We can't > > > > > > > >> > >> >> > use builtin_platform_driver_probe() because it doesn't retry after probe > > > > > > > >> > >> >> > deferral. Convert it to builtin_platform_driver(). > > > > > > > >> > >> >> > > > > > > > >> > >> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then > > > > > > > >> > >> >> shouldn't it be fixed or removed? > > > > > > > >> > >> > > > > > > > > >> > >> > I was actually thinking about this too. The problem with fixing > > > > > > > >> > >> > builtin_platform_driver_probe() to behave like > > > > > > > >> > >> > builtin_platform_driver() is that these probe functions could be > > > > > > > >> > >> > marked with __init. But there are also only 20 instances of > > > > > > > >> > >> > builtin_platform_driver_probe() in the kernel: > > > > > > > >> > >> > $ git grep ^builtin_platform_driver_probe | wc -l > > > > > > > >> > >> > 20 > > > > > > > >> > >> > > > > > > > > >> > >> > So it might be easier to just fix them to not use > > > > > > > >> > >> > builtin_platform_driver_probe(). > > > > > > > >> > >> > > > > > > > > >> > >> > Michael, > > > > > > > >> > >> > > > > > > > > >> > >> > Any chance you'd be willing to help me by converting all these to > > > > > > > >> > >> > builtin_platform_driver() and delete builtin_platform_driver_probe()? > > > > > > > >> > >> > > > > > > > >> > >> If it just moving the probe function to the _driver struct and > > > > > > > >> > >> remove the __init annotations. I could look into that. > > > > > > > >> > > > > > > > > > >> > > Yup. That's pretty much it AFAICT. > > > > > > > >> > > > > > > > > > >> > > builtin_platform_driver_probe() also makes sure the driver doesn't ask > > > > > > > >> > > for async probe, etc. But I doubt anyone is actually setting async > > > > > > > >> > > flags and still using builtin_platform_driver_probe(). > > > > > > > >> > > > > > > > > >> > Hasn't module_platform_driver_probe() the same problem? And there > > > > > > > >> > are ~80 drivers which uses that. > > > > > > > >> > > > > > > > >> Yeah. The biggest problem with all of these is the __init markers. > > > > > > > >> Maybe some familiar with coccinelle can help? > > > > > > > > > > > > > > > > And dropping them will increase memory usage. > > > > > > > > > > > > > > Although I do have the changes for the builtin_platform_driver_probe() > > > > > > > ready, I don't think it makes much sense to send these unless we agree > > > > > > > on the increased memory footprint. While there are just a few > > > > > > > builtin_platform_driver_probe() and memory increase _might_ be > > > > > > > negligible, there are many more module_platform_driver_probe(). > > > > > > > > > > > > While it's good to drop code that'll not be used past kernel init, the > > > > > > module_platform_driver_probe() is going even more extreme. It doesn't > > > > > > even allow deferred probe (well before kernel init is done). I don't > > > > > > think that behavior is right and that's why we should delete it. Also, > > > > > > > > > > This construct is typically used for builtin hardware for which the > > > > > dependencies are registered very early, and thus known to probe at > > > > > first try (if present). > > > > > > > > > > > I doubt if any of these probe functions even take up 4KB of memory. > > > > > > > > > > How many 4 KiB pages do you have in a system with 10 MiB of SRAM? > > > > > How many can you afford to waste? > > > > > > > > There are only a few instances of this macro in the kernel. How many > > > > > > $ git grep -lw builtin_platform_driver_probe | wc -l > > > 21 > > > $ git grep -lw module_platform_driver_probe | wc -l > > > 86 > > > > > > + the ones that haven't been converted to the above yet: > > > > > > $ git grep -lw platform_driver_probe | wc -l > > > 58 > > > > > > > Yeah, this adds up in terms of the number of places we'd need to fix. > > But thinking more about it, a couple of points: > > 1. Not all builtin_platform_driver_probe() are problems for > > fw_devlink. So we can just fix them as we go if we need to. > > > > 2. The problem with builtin_platform_driver_probe() isn't really with > > the use of __init. It's the fact that it doesn't allow deferred > > probes. builtin_platform_driver_probe()/platform_driver_probe() could > > still be fixed up to allow deferred probe until we get to the point > > where we free the __init section (so at least till late_initcall). > > That's intentional: it is used for cases that will (must) never be deferred. > That's why it's safe to use __init. So was the usage of builtin_platform_driver_probe() wrong in the driver Michael fixed? Because, deferring and probing again clearly works? Also, "must never be deferred" seems like a weird condition to enforce. I think the real "rule" is that if it defers, the platform is not expected to work. But disallowing a probe reattempt seems weird. What is it going to hurt if it's attempted again? At worst it fails one more time? Also, I'd argue that all/most of the "can't defer, but I'm still a proper struct device" cases are all just patchwork to deal with the fact we were playing initcall chicken when there was no fw_devlink. I'm hoping we can move people away from that mindset. And the first step towards that would be to allow *platform_probe() to allow deferred probes until late_initcall(). -Saravana ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-27 17:10 ` Saravana Kannan @ 2021-01-28 9:25 ` Geert Uytterhoeven 2021-01-28 10:35 ` Tony Lindgren 0 siblings, 1 reply; 30+ messages in thread From: Geert Uytterhoeven @ 2021-01-28 9:25 UTC (permalink / raw) To: Saravana Kannan Cc: Michael Walle, Lorenzo Pieralisi, Roy Zang, PCI, LKML, Minghuan Lian, Mingkai Hu, Greg Kroah-Hartman, Bjorn Helgaas, linuxppc-dev, linux-arm-kernel Hi Saravana, On Wed, Jan 27, 2021 at 6:11 PM Saravana Kannan <saravanak@google.com> wrote: > On Wed, Jan 27, 2021 at 8:56 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Wed, Jan 27, 2021 at 5:42 PM Saravana Kannan <saravanak@google.com> wrote: > > > On Tue, Jan 26, 2021 at 11:43 PM Geert Uytterhoeven > > > <geert@linux-m68k.org> wrote: > > > > On Wed, Jan 27, 2021 at 1:44 AM Saravana Kannan <saravanak@google.com> wrote: > > > > > On Tue, Jan 26, 2021 at 12:50 AM Geert Uytterhoeven > > > > > <geert@linux-m68k.org> wrote: > > > > > > On Mon, Jan 25, 2021 at 11:42 PM Saravana Kannan <saravanak@google.com> wrote: > > > > > > > On Mon, Jan 25, 2021 at 11:49 AM Michael Walle <michael@walle.cc> wrote: > > > > > > > > Am 2021-01-21 12:01, schrieb Geert Uytterhoeven: > > > > > > > > > On Thu, Jan 21, 2021 at 1:05 AM Saravana Kannan <saravanak@google.com> > > > > > > > > > wrote: > > > > > > > > >> On Wed, Jan 20, 2021 at 3:53 PM Michael Walle <michael@walle.cc> > > > > > > > > >> wrote: > > > > > > > > >> > Am 2021-01-20 20:47, schrieb Saravana Kannan: > > > > > > > > >> > > On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc> > > > > > > > > >> > > wrote: > > > > > > > > >> > >> > > > > > > > > >> > >> [RESEND, fat-fingered the buttons of my mail client and converted > > > > > > > > >> > >> all CCs to BCCs :(] > > > > > > > > >> > >> > > > > > > > > >> > >> Am 2021-01-20 20:02, schrieb Saravana Kannan: > > > > > > > > >> > >> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: > > > > > > > > >> > >> >> > > > > > > > > >> > >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> > > > > > > > > >> > >> >> wrote: > > > > > > > > >> > >> >> > > > > > > > > > >> > >> >> > fw_devlink will defer the probe until all suppliers are ready. We can't > > > > > > > > >> > >> >> > use builtin_platform_driver_probe() because it doesn't retry after probe > > > > > > > > >> > >> >> > deferral. Convert it to builtin_platform_driver(). > > > > > > > > >> > >> >> > > > > > > > > >> > >> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then > > > > > > > > >> > >> >> shouldn't it be fixed or removed? > > > > > > > > >> > >> > > > > > > > > > >> > >> > I was actually thinking about this too. The problem with fixing > > > > > > > > >> > >> > builtin_platform_driver_probe() to behave like > > > > > > > > >> > >> > builtin_platform_driver() is that these probe functions could be > > > > > > > > >> > >> > marked with __init. But there are also only 20 instances of > > > > > > > > >> > >> > builtin_platform_driver_probe() in the kernel: > > > > > > > > >> > >> > $ git grep ^builtin_platform_driver_probe | wc -l > > > > > > > > >> > >> > 20 > > > > > > > > >> > >> > > > > > > > > > >> > >> > So it might be easier to just fix them to not use > > > > > > > > >> > >> > builtin_platform_driver_probe(). > > > > > > > > >> > >> > > > > > > > > > >> > >> > Michael, > > > > > > > > >> > >> > > > > > > > > > >> > >> > Any chance you'd be willing to help me by converting all these to > > > > > > > > >> > >> > builtin_platform_driver() and delete builtin_platform_driver_probe()? > > > > > > > > >> > >> > > > > > > > > >> > >> If it just moving the probe function to the _driver struct and > > > > > > > > >> > >> remove the __init annotations. I could look into that. > > > > > > > > >> > > > > > > > > > > >> > > Yup. That's pretty much it AFAICT. > > > > > > > > >> > > > > > > > > > > >> > > builtin_platform_driver_probe() also makes sure the driver doesn't ask > > > > > > > > >> > > for async probe, etc. But I doubt anyone is actually setting async > > > > > > > > >> > > flags and still using builtin_platform_driver_probe(). > > > > > > > > >> > > > > > > > > > >> > Hasn't module_platform_driver_probe() the same problem? And there > > > > > > > > >> > are ~80 drivers which uses that. > > > > > > > > >> > > > > > > > > >> Yeah. The biggest problem with all of these is the __init markers. > > > > > > > > >> Maybe some familiar with coccinelle can help? > > > > > > > > > > > > > > > > > > And dropping them will increase memory usage. > > > > > > > > > > > > > > > > Although I do have the changes for the builtin_platform_driver_probe() > > > > > > > > ready, I don't think it makes much sense to send these unless we agree > > > > > > > > on the increased memory footprint. While there are just a few > > > > > > > > builtin_platform_driver_probe() and memory increase _might_ be > > > > > > > > negligible, there are many more module_platform_driver_probe(). > > > > > > > > > > > > > > While it's good to drop code that'll not be used past kernel init, the > > > > > > > module_platform_driver_probe() is going even more extreme. It doesn't > > > > > > > even allow deferred probe (well before kernel init is done). I don't > > > > > > > think that behavior is right and that's why we should delete it. Also, > > > > > > > > > > > > This construct is typically used for builtin hardware for which the > > > > > > dependencies are registered very early, and thus known to probe at > > > > > > first try (if present). > > > > > > > > > > > > > I doubt if any of these probe functions even take up 4KB of memory. > > > > > > > > > > > > How many 4 KiB pages do you have in a system with 10 MiB of SRAM? > > > > > > How many can you afford to waste? > > > > > > > > > > There are only a few instances of this macro in the kernel. How many > > > > > > > > $ git grep -lw builtin_platform_driver_probe | wc -l > > > > 21 > > > > $ git grep -lw module_platform_driver_probe | wc -l > > > > 86 > > > > > > > > + the ones that haven't been converted to the above yet: > > > > > > > > $ git grep -lw platform_driver_probe | wc -l > > > > 58 > > > > > > > > > > Yeah, this adds up in terms of the number of places we'd need to fix. > > > But thinking more about it, a couple of points: > > > 1. Not all builtin_platform_driver_probe() are problems for > > > fw_devlink. So we can just fix them as we go if we need to. > > > > > > 2. The problem with builtin_platform_driver_probe() isn't really with > > > the use of __init. It's the fact that it doesn't allow deferred > > > probes. builtin_platform_driver_probe()/platform_driver_probe() could > > > still be fixed up to allow deferred probe until we get to the point > > > where we free the __init section (so at least till late_initcall). > > > > That's intentional: it is used for cases that will (must) never be deferred. > > That's why it's safe to use __init. > > So was the usage of builtin_platform_driver_probe() wrong in the > driver Michael fixed? Because, deferring and probing again clearly > works? It wasn't. The regression is that the driver no longer probes at first try, because its dependencies are now probed later. The question is: why are the dependencies now probed later? How to fix that? > Also, "must never be deferred" seems like a weird condition to > enforce. I think the real "rule" is that if it defers, the platform is > not expected to work. But disallowing a probe reattempt seems weird. > What is it going to hurt if it's attempted again? At worst it fails > one more time? "must never be deferred" is not the real condition, but "must not be probed after __init is freed" is (one of them, there may be other, cfr. the last paragraph below). The simplest way to guarantee that is to probe the driver immediately, and only once. > Also, I'd argue that all/most of the "can't defer, but I'm still a > proper struct device" cases are all just patchwork to deal with the > fact we were playing initcall chicken when there was no fw_devlink. > I'm hoping we can move people away from that mindset. And the first I agree, partially. Still, how come the dependencies are no longer probed before their consumers when fw_devlinks are enabled? I thought fw_devlinks is supposed to avoid exactly that? > step towards that would be to allow *platform_probe() to allow > deferred probes until late_initcall(). At which increase of complexity, to keep track of which drivers can and cannot be reprobed anymore after late_initcall? Still, many of these drivers use platform_driver_probe() early for a reason: because they need to initialize the hardware early. Probing them later may introduce subtle bugs. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-28 9:25 ` Geert Uytterhoeven @ 2021-01-28 10:35 ` Tony Lindgren 0 siblings, 0 replies; 30+ messages in thread From: Tony Lindgren @ 2021-01-28 10:35 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Saravana Kannan, Roy Zang, Lorenzo Pieralisi, PCI, LKML, Minghuan Lian, Michael Walle, linux-arm-kernel, Greg Kroah-Hartman, Bjorn Helgaas, linuxppc-dev, Mingkai Hu * Geert Uytterhoeven <geert@linux-m68k.org> [210128 09:32]: > It wasn't. The regression is that the driver no longer probes at first > try, because its dependencies are now probed later. The question is: > why are the dependencies now probed later? How to fix that? I'm afraid that may be unfixable.. It depends on things like the bus driver probe that might get also deferred. As suggested, I agree it's best to get rid of builtin_platform_driver_probe where possible at the cost of dropping the __init as needed. To me it seems we can't even add a warning to __platform_driver_probe() if there's drv->driver.of_match_table for example. That warning would show up on all the devices with driver in question built in even if the device has no such hardware. Regards, Tony ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-25 19:49 ` Michael Walle 2021-01-25 22:41 ` Saravana Kannan @ 2021-01-28 10:00 ` Tony Lindgren 1 sibling, 0 replies; 30+ messages in thread From: Tony Lindgren @ 2021-01-28 10:00 UTC (permalink / raw) To: Michael Walle Cc: Geert Uytterhoeven, Roy Zang, Lorenzo Pieralisi, Saravana Kannan, PCI, LKML, Minghuan Lian, linux-arm-kernel, Greg Kroah-Hartman, Bjorn Helgaas, linuxppc-dev, Mingkai Hu, Kishon Vijay Abraham I Hi, * Michael Walle <michael@walle.cc> [210125 19:52]: > Although I do have the changes for the builtin_platform_driver_probe() > ready, I don't think it makes much sense to send these unless we agree > on the increased memory footprint. While there are just a few > builtin_platform_driver_probe() and memory increase _might_ be > negligible, there are many more module_platform_driver_probe(). I just noticed this thread today and have pretty much come to the same conclusions. No need to post a patch for pci-dra7xx.c, I already posted a patch for pci-dra7xx.c yesterday as part of genpd related changes. For me probing started breaking as the power-domains property got added. FYI, it's the following patch: [PATCH 01/15] PCI: pci-dra7xx: Prepare for deferred probe with module_platform_driver Regards, Tony ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-20 19:28 ` Michael Walle 2021-01-20 19:47 ` Saravana Kannan @ 2021-01-25 16:50 ` Lorenzo Pieralisi 2021-01-25 18:58 ` Saravana Kannan 1 sibling, 1 reply; 30+ messages in thread From: Lorenzo Pieralisi @ 2021-01-25 16:50 UTC (permalink / raw) To: Michael Walle Cc: Saravana Kannan, Rob Herring, linuxppc-dev, PCI, linux-arm-kernel, linux-kernel, Minghuan Lian, Mingkai Hu, Roy Zang, Bjorn Helgaas, Greg Kroah-Hartman On Wed, Jan 20, 2021 at 08:28:36PM +0100, Michael Walle wrote: > [RESEND, fat-fingered the buttons of my mail client and converted > all CCs to BCCs :(] > > Am 2021-01-20 20:02, schrieb Saravana Kannan: > > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: > > > > > > On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> > > > wrote: > > > > > > > > fw_devlink will defer the probe until all suppliers are ready. We can't > > > > use builtin_platform_driver_probe() because it doesn't retry after probe > > > > deferral. Convert it to builtin_platform_driver(). > > > > > > If builtin_platform_driver_probe() doesn't work with fw_devlink, then > > > shouldn't it be fixed or removed? > > > > I was actually thinking about this too. The problem with fixing > > builtin_platform_driver_probe() to behave like > > builtin_platform_driver() is that these probe functions could be > > marked with __init. But there are also only 20 instances of > > builtin_platform_driver_probe() in the kernel: > > $ git grep ^builtin_platform_driver_probe | wc -l > > 20 > > > > So it might be easier to just fix them to not use > > builtin_platform_driver_probe(). > > > > Michael, > > > > Any chance you'd be willing to help me by converting all these to > > builtin_platform_driver() and delete builtin_platform_driver_probe()? > > If it just moving the probe function to the _driver struct and > remove the __init annotations. I could look into that. Can I drop this patch then ? Thanks, Lorenzo ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-25 16:50 ` Lorenzo Pieralisi @ 2021-01-25 18:58 ` Saravana Kannan 2021-01-25 19:44 ` Michael Walle 0 siblings, 1 reply; 30+ messages in thread From: Saravana Kannan @ 2021-01-25 18:58 UTC (permalink / raw) To: Lorenzo Pieralisi Cc: Michael Walle, Rob Herring, linuxppc-dev, PCI, linux-arm-kernel, LKML, Minghuan Lian, Mingkai Hu, Roy Zang, Bjorn Helgaas, Greg Kroah-Hartman On Mon, Jan 25, 2021 at 8:50 AM Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > > On Wed, Jan 20, 2021 at 08:28:36PM +0100, Michael Walle wrote: > > [RESEND, fat-fingered the buttons of my mail client and converted > > all CCs to BCCs :(] > > > > Am 2021-01-20 20:02, schrieb Saravana Kannan: > > > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: > > > > > > > > On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> > > > > wrote: > > > > > > > > > > fw_devlink will defer the probe until all suppliers are ready. We can't > > > > > use builtin_platform_driver_probe() because it doesn't retry after probe > > > > > deferral. Convert it to builtin_platform_driver(). > > > > > > > > If builtin_platform_driver_probe() doesn't work with fw_devlink, then > > > > shouldn't it be fixed or removed? > > > > > > I was actually thinking about this too. The problem with fixing > > > builtin_platform_driver_probe() to behave like > > > builtin_platform_driver() is that these probe functions could be > > > marked with __init. But there are also only 20 instances of > > > builtin_platform_driver_probe() in the kernel: > > > $ git grep ^builtin_platform_driver_probe | wc -l > > > 20 > > > > > > So it might be easier to just fix them to not use > > > builtin_platform_driver_probe(). > > > > > > Michael, > > > > > > Any chance you'd be willing to help me by converting all these to > > > builtin_platform_driver() and delete builtin_platform_driver_probe()? > > > > If it just moving the probe function to the _driver struct and > > remove the __init annotations. I could look into that. > > Can I drop this patch then ? No, please pick it up. Michael and I were talking about doing similar changes for other drivers. -Saravana ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-25 18:58 ` Saravana Kannan @ 2021-01-25 19:44 ` Michael Walle 0 siblings, 0 replies; 30+ messages in thread From: Michael Walle @ 2021-01-25 19:44 UTC (permalink / raw) To: Saravana Kannan Cc: Lorenzo Pieralisi, Rob Herring, linuxppc-dev, PCI, linux-arm-kernel, LKML, Minghuan Lian, Mingkai Hu, Roy Zang, Bjorn Helgaas, Greg Kroah-Hartman Am 2021-01-25 19:58, schrieb Saravana Kannan: > On Mon, Jan 25, 2021 at 8:50 AM Lorenzo Pieralisi > <lorenzo.pieralisi@arm.com> wrote: >> >> On Wed, Jan 20, 2021 at 08:28:36PM +0100, Michael Walle wrote: >> > [RESEND, fat-fingered the buttons of my mail client and converted >> > all CCs to BCCs :(] >> > >> > Am 2021-01-20 20:02, schrieb Saravana Kannan: >> > > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote: >> > > > >> > > > On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc> >> > > > wrote: >> > > > > >> > > > > fw_devlink will defer the probe until all suppliers are ready. We can't >> > > > > use builtin_platform_driver_probe() because it doesn't retry after probe >> > > > > deferral. Convert it to builtin_platform_driver(). >> > > > >> > > > If builtin_platform_driver_probe() doesn't work with fw_devlink, then >> > > > shouldn't it be fixed or removed? >> > > >> > > I was actually thinking about this too. The problem with fixing >> > > builtin_platform_driver_probe() to behave like >> > > builtin_platform_driver() is that these probe functions could be >> > > marked with __init. But there are also only 20 instances of >> > > builtin_platform_driver_probe() in the kernel: >> > > $ git grep ^builtin_platform_driver_probe | wc -l >> > > 20 >> > > >> > > So it might be easier to just fix them to not use >> > > builtin_platform_driver_probe(). >> > > >> > > Michael, >> > > >> > > Any chance you'd be willing to help me by converting all these to >> > > builtin_platform_driver() and delete builtin_platform_driver_probe()? >> > >> > If it just moving the probe function to the _driver struct and >> > remove the __init annotations. I could look into that. >> >> Can I drop this patch then ? > > No, please pick it up. Michael and I were talking about doing similar > changes for other drivers. Yes please, I was just about to answer, but Saravana beat me. -michael ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-20 10:52 [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() Michael Walle 2021-01-20 14:23 ` Rob Herring @ 2021-01-26 10:02 ` Lorenzo Pieralisi 2021-01-26 10:39 ` Michael Walle 2021-01-26 10:55 ` Lorenzo Pieralisi 2 siblings, 1 reply; 30+ messages in thread From: Lorenzo Pieralisi @ 2021-01-26 10:02 UTC (permalink / raw) To: Michael Walle Cc: linuxppc-dev, linux-pci, linux-arm-kernel, linux-kernel, Minghuan Lian, Mingkai Hu, Roy Zang, Rob Herring, Bjorn Helgaas, Greg Kroah-Hartman, Saravana Kannan On Wed, Jan 20, 2021 at 11:52:46AM +0100, Michael Walle wrote: > fw_devlink will defer the probe until all suppliers are ready. We can't > use builtin_platform_driver_probe() because it doesn't retry after probe > deferral. Convert it to builtin_platform_driver(). > > Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") I will have to drop this Fixes: tag if you don't mind, it is not in the mainline. Lorenzo > Signed-off-by: Michael Walle <michael@walle.cc> > --- > drivers/pci/controller/dwc/pci-layerscape.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pci-layerscape.c b/drivers/pci/controller/dwc/pci-layerscape.c > index 44ad34cdc3bc..5b9c625df7b8 100644 > --- a/drivers/pci/controller/dwc/pci-layerscape.c > +++ b/drivers/pci/controller/dwc/pci-layerscape.c > @@ -232,7 +232,7 @@ static const struct of_device_id ls_pcie_of_match[] = { > { }, > }; > > -static int __init ls_pcie_probe(struct platform_device *pdev) > +static int ls_pcie_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > struct dw_pcie *pci; > @@ -271,10 +271,11 @@ static int __init ls_pcie_probe(struct platform_device *pdev) > } > > static struct platform_driver ls_pcie_driver = { > + .probe = ls_pcie_probe, > .driver = { > .name = "layerscape-pcie", > .of_match_table = ls_pcie_of_match, > .suppress_bind_attrs = true, > }, > }; > -builtin_platform_driver_probe(ls_pcie_driver, ls_pcie_probe); > +builtin_platform_driver(ls_pcie_driver); > -- > 2.20.1 > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-26 10:02 ` Lorenzo Pieralisi @ 2021-01-26 10:39 ` Michael Walle 2021-01-26 10:56 ` Geert Uytterhoeven 0 siblings, 1 reply; 30+ messages in thread From: Michael Walle @ 2021-01-26 10:39 UTC (permalink / raw) To: Lorenzo Pieralisi Cc: linuxppc-dev, linux-pci, linux-arm-kernel, linux-kernel, Minghuan Lian, Mingkai Hu, Roy Zang, Rob Herring, Bjorn Helgaas, Greg Kroah-Hartman, Saravana Kannan Am 2021-01-26 11:02, schrieb Lorenzo Pieralisi: > On Wed, Jan 20, 2021 at 11:52:46AM +0100, Michael Walle wrote: >> fw_devlink will defer the probe until all suppliers are ready. We >> can't >> use builtin_platform_driver_probe() because it doesn't retry after >> probe >> deferral. Convert it to builtin_platform_driver(). >> >> Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") > > I will have to drop this Fixes: tag if you don't mind, it is not > in the mainline. That commit is in Greg's for-next tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/commit/?h=driver-core-next&id=e590474768f1cc04852190b61dec692411b22e2a I was under the impression there are other commits with this particular fixes tag, too. Either it was removed from for-next queues or I was confused. But I'm fine with removing the tag, assuming this will end up together with the "driver core: Set fw_devlink=on by default" commit in 5.11. -michael ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-26 10:39 ` Michael Walle @ 2021-01-26 10:56 ` Geert Uytterhoeven 0 siblings, 0 replies; 30+ messages in thread From: Geert Uytterhoeven @ 2021-01-26 10:56 UTC (permalink / raw) To: Michael Walle Cc: Lorenzo Pieralisi, Roy Zang, Saravana Kannan, linux-pci, Linux Kernel Mailing List, Minghuan Lian, Linux ARM, Greg Kroah-Hartman, Bjorn Helgaas, linuxppc-dev, Mingkai Hu Hi Michael, On Tue, Jan 26, 2021 at 11:46 AM Michael Walle <michael@walle.cc> wrote: > Am 2021-01-26 11:02, schrieb Lorenzo Pieralisi: > > On Wed, Jan 20, 2021 at 11:52:46AM +0100, Michael Walle wrote: > >> fw_devlink will defer the probe until all suppliers are ready. We > >> can't > >> use builtin_platform_driver_probe() because it doesn't retry after > >> probe > >> deferral. Convert it to builtin_platform_driver(). > >> > >> Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") > > > > I will have to drop this Fixes: tag if you don't mind, it is not > > in the mainline. > > That commit is in Greg's for-next tree: > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/commit/?h=driver-core-next&id=e590474768f1cc04852190b61dec692411b22e2a > > I was under the impression there are other commits with this > particular fixes tag, too. Either it was removed from > for-next queues or I was confused. > > But I'm fine with removing the tag, assuming this will end > up together with the "driver core: Set fw_devlink=on by default" > commit in 5.11. Definitely not v5.11. And I sincerely doubt it will be applied for v5.12. It's already way too late to implement all changes to existing drivers needed, and get them accepted for v5.12. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() 2021-01-20 10:52 [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() Michael Walle 2021-01-20 14:23 ` Rob Herring 2021-01-26 10:02 ` Lorenzo Pieralisi @ 2021-01-26 10:55 ` Lorenzo Pieralisi 2 siblings, 0 replies; 30+ messages in thread From: Lorenzo Pieralisi @ 2021-01-26 10:55 UTC (permalink / raw) To: linux-kernel, Michael Walle, linux-pci, linux-arm-kernel, linuxppc-dev Cc: Lorenzo Pieralisi, Mingkai Hu, Greg Kroah-Hartman, Saravana Kannan, Bjorn Helgaas, Minghuan Lian, Rob Herring, Roy Zang On Wed, 20 Jan 2021 11:52:46 +0100, Michael Walle wrote: > fw_devlink will defer the probe until all suppliers are ready. We can't > use builtin_platform_driver_probe() because it doesn't retry after probe > deferral. Convert it to builtin_platform_driver(). Applied to pci/dwc, thanks! [1/1] PCI: dwc: layerscape: Convert to builtin_platform_driver() https://git.kernel.org/lpieralisi/pci/c/538157be1e Thanks, Lorenzo ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2021-01-28 10:36 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-20 10:52 [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver() Michael Walle 2021-01-20 14:23 ` Rob Herring 2021-01-20 14:34 ` Michael Walle 2021-01-20 15:50 ` Greg Kroah-Hartman 2021-01-20 19:02 ` Saravana Kannan 2021-01-20 19:25 ` Michael Walle 2021-01-20 19:28 ` Michael Walle 2021-01-20 19:47 ` Saravana Kannan 2021-01-20 19:47 ` Saravana Kannan 2021-01-20 23:53 ` Michael Walle 2021-01-20 23:58 ` Saravana Kannan 2021-01-21 11:01 ` Geert Uytterhoeven 2021-01-25 19:49 ` Michael Walle 2021-01-25 22:41 ` Saravana Kannan 2021-01-26 8:50 ` Geert Uytterhoeven 2021-01-27 0:44 ` Saravana Kannan 2021-01-27 7:43 ` Geert Uytterhoeven 2021-01-27 16:41 ` Saravana Kannan 2021-01-27 16:56 ` Geert Uytterhoeven 2021-01-27 17:10 ` Saravana Kannan 2021-01-28 9:25 ` Geert Uytterhoeven 2021-01-28 10:35 ` Tony Lindgren 2021-01-28 10:00 ` Tony Lindgren 2021-01-25 16:50 ` Lorenzo Pieralisi 2021-01-25 18:58 ` Saravana Kannan 2021-01-25 19:44 ` Michael Walle 2021-01-26 10:02 ` Lorenzo Pieralisi 2021-01-26 10:39 ` Michael Walle 2021-01-26 10:56 ` Geert Uytterhoeven 2021-01-26 10:55 ` Lorenzo Pieralisi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).