| Message ID | 20210223090757.57604-1-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: endpoint: Make struct pci_epf_driver::remove return void | expand |
Hello, On Tue, Feb 23, 2021 at 10:07:57AM +0100, Uwe Kleine-König wrote: > The driver core ignores the return value of pci_epf_device_remove() > (because there is only little it can do when a device disappears) and > there are no pci_epf_drivers with a remove callback. > > So make it impossible for future drivers to return an unused error code > by changing the remove prototype to return void. > > The real motivation for this change is the quest to make struct > bus_type::remove return void, too. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> ping! I didn't hear anything on this patch. Is it still one someone's list of patches to review? Best regards Uwe > --- > drivers/pci/endpoint/pci-epf-core.c | 5 ++--- > include/linux/pci-epf.h | 2 +- > 2 files changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c > index 7646c8660d42..a19c375f9ec9 100644 > --- a/drivers/pci/endpoint/pci-epf-core.c > +++ b/drivers/pci/endpoint/pci-epf-core.c > @@ -389,15 +389,14 @@ static int pci_epf_device_probe(struct device *dev) > > static int pci_epf_device_remove(struct device *dev) > { > - int ret = 0; > struct pci_epf *epf = to_pci_epf(dev); > struct pci_epf_driver *driver = to_pci_epf_driver(dev->driver); > > if (driver->remove) > - ret = driver->remove(epf); > + driver->remove(epf); > epf->driver = NULL; > > - return ret; > + return 0; > } > > static struct bus_type pci_epf_bus_type = { > diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h > index 6833e2160ef1..f8a17b6b1d31 100644 > --- a/include/linux/pci-epf.h > +++ b/include/linux/pci-epf.h > @@ -85,7 +85,7 @@ struct pci_epf_ops { > */ > struct pci_epf_driver { > int (*probe)(struct pci_epf *epf); > - int (*remove)(struct pci_epf *epf); > + void (*remove)(struct pci_epf *epf); > > struct device_driver driver; > struct pci_epf_ops *ops; > -- > 2.30.0 > > >
On 2/23/21 10:07 AM, Uwe Kleine-König wrote: > The driver core ignores the return value of pci_epf_device_remove() > (because there is only little it can do when a device disappears) and > there are no pci_epf_drivers with a remove callback. > > So make it impossible for future drivers to return an unused error code > by changing the remove prototype to return void. > > The real motivation for this change is the quest to make struct > bus_type::remove return void, too. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Ping! This patch now waits for more than 2 months on feedback (or application). The 5.13 merge window just closed, this is a great opportunity to apply this patch for next. Thanks for consideration, Uwe > --- > drivers/pci/endpoint/pci-epf-core.c | 5 ++--- > include/linux/pci-epf.h | 2 +- > 2 files changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c > index 7646c8660d42..a19c375f9ec9 100644 > --- a/drivers/pci/endpoint/pci-epf-core.c > +++ b/drivers/pci/endpoint/pci-epf-core.c > @@ -389,15 +389,14 @@ static int pci_epf_device_probe(struct device *dev) > > static int pci_epf_device_remove(struct device *dev) > { > - int ret = 0; > struct pci_epf *epf = to_pci_epf(dev); > struct pci_epf_driver *driver = to_pci_epf_driver(dev->driver); > > if (driver->remove) > - ret = driver->remove(epf); > + driver->remove(epf); > epf->driver = NULL; > > - return ret; > + return 0; > } > > static struct bus_type pci_epf_bus_type = { > diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h > index 6833e2160ef1..f8a17b6b1d31 100644 > --- a/include/linux/pci-epf.h > +++ b/include/linux/pci-epf.h > @@ -85,7 +85,7 @@ struct pci_epf_ops { > */ > struct pci_epf_driver { > int (*probe)(struct pci_epf *epf); > - int (*remove)(struct pci_epf *epf); > + void (*remove)(struct pci_epf *epf); > > struct device_driver driver; > struct pci_epf_ops *ops; >
Hello Bjorn, On Mon, May 10, 2021 at 09:26:37PM +0200, Uwe Kleine-König wrote: > On 2/23/21 10:07 AM, Uwe Kleine-König wrote: > > The driver core ignores the return value of pci_epf_device_remove() > > (because there is only little it can do when a device disappears) and > > there are no pci_epf_drivers with a remove callback. > > > > So make it impossible for future drivers to return an unused error code > > by changing the remove prototype to return void. > > > > The real motivation for this change is the quest to make struct > > bus_type::remove return void, too. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > Ping! This patch now waits for more than 2 months on feedback (or > application). The 5.13 merge window just closed, this is a great opportunity > to apply this patch for next. It seems I don't get feedback from Kishon and Lorenzo. This is one of the last patches I need to actually change bus_type::remove. Would you be willing to take the patch without them reacting? Or do you have a way to trigger them that is more effective than I have? Best regards Uwe > > --- > > drivers/pci/endpoint/pci-epf-core.c | 5 ++--- > > include/linux/pci-epf.h | 2 +- > > 2 files changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c > > index 7646c8660d42..a19c375f9ec9 100644 > > --- a/drivers/pci/endpoint/pci-epf-core.c > > +++ b/drivers/pci/endpoint/pci-epf-core.c > > @@ -389,15 +389,14 @@ static int pci_epf_device_probe(struct device *dev) > > static int pci_epf_device_remove(struct device *dev) > > { > > - int ret = 0; > > struct pci_epf *epf = to_pci_epf(dev); > > struct pci_epf_driver *driver = to_pci_epf_driver(dev->driver); > > if (driver->remove) > > - ret = driver->remove(epf); > > + driver->remove(epf); > > epf->driver = NULL; > > - return ret; > > + return 0; > > } > > static struct bus_type pci_epf_bus_type = { > > diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h > > index 6833e2160ef1..f8a17b6b1d31 100644 > > --- a/include/linux/pci-epf.h > > +++ b/include/linux/pci-epf.h > > @@ -85,7 +85,7 @@ struct pci_epf_ops { > > */ > > struct pci_epf_driver { > > int (*probe)(struct pci_epf *epf); > > - int (*remove)(struct pci_epf *epf); > > + void (*remove)(struct pci_epf *epf); > > struct device_driver driver; > > struct pci_epf_ops *ops; > > > >
On 23/02/21 2:37 pm, Uwe Kleine-König wrote: > The driver core ignores the return value of pci_epf_device_remove() > (because there is only little it can do when a device disappears) and > there are no pci_epf_drivers with a remove callback. > > So make it impossible for future drivers to return an unused error code > by changing the remove prototype to return void. > > The real motivation for this change is the quest to make struct > bus_type::remove return void, too. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Fine with this change! FWIW: Acked-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/pci/endpoint/pci-epf-core.c | 5 ++--- > include/linux/pci-epf.h | 2 +- > 2 files changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c > index 7646c8660d42..a19c375f9ec9 100644 > --- a/drivers/pci/endpoint/pci-epf-core.c > +++ b/drivers/pci/endpoint/pci-epf-core.c > @@ -389,15 +389,14 @@ static int pci_epf_device_probe(struct device *dev) > > static int pci_epf_device_remove(struct device *dev) > { > - int ret = 0; > struct pci_epf *epf = to_pci_epf(dev); > struct pci_epf_driver *driver = to_pci_epf_driver(dev->driver); > > if (driver->remove) > - ret = driver->remove(epf); > + driver->remove(epf); > epf->driver = NULL; > > - return ret; > + return 0; > } > > static struct bus_type pci_epf_bus_type = { > diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h > index 6833e2160ef1..f8a17b6b1d31 100644 > --- a/include/linux/pci-epf.h > +++ b/include/linux/pci-epf.h > @@ -85,7 +85,7 @@ struct pci_epf_ops { > */ > struct pci_epf_driver { > int (*probe)(struct pci_epf *epf); > - int (*remove)(struct pci_epf *epf); > + void (*remove)(struct pci_epf *epf); > > struct device_driver driver; > struct pci_epf_ops *ops; >
On Tue, Jun 22, 2021 at 03:29:27PM +0530, Kishon Vijay Abraham I wrote: > > > On 23/02/21 2:37 pm, Uwe Kleine-König wrote: > > The driver core ignores the return value of pci_epf_device_remove() > > (because there is only little it can do when a device disappears) and > > there are no pci_epf_drivers with a remove callback. > > > > So make it impossible for future drivers to return an unused error code > > by changing the remove prototype to return void. > > > > The real motivation for this change is the quest to make struct > > bus_type::remove return void, too. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > Fine with this change! > > FWIW: > Acked-by: Kishon Vijay Abraham I <kishon@ti.com> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Hello, On Tue, Jun 22, 2021 at 03:29:27PM +0530, Kishon Vijay Abraham I wrote: > On 23/02/21 2:37 pm, Uwe Kleine-König wrote: > > The driver core ignores the return value of pci_epf_device_remove() > > (because there is only little it can do when a device disappears) and > > there are no pci_epf_drivers with a remove callback. > > > > So make it impossible for future drivers to return an unused error code > > by changing the remove prototype to return void. > > > > The real motivation for this change is the quest to make struct > > bus_type::remove return void, too. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > Fine with this change! > > FWIW: > Acked-by: Kishon Vijay Abraham I <kishon@ti.com> Thanks for the ack. How can I expect this patch to go into mainline now? Will Bjorn pick it up now that you acked? Best regards Uwe
On Mon, Jul 05, 2021 at 05:46:50PM +0200, Uwe Kleine-König wrote: > Hello, > > On Tue, Jun 22, 2021 at 03:29:27PM +0530, Kishon Vijay Abraham I wrote: > > On 23/02/21 2:37 pm, Uwe Kleine-König wrote: > > > The driver core ignores the return value of pci_epf_device_remove() > > > (because there is only little it can do when a device disappears) and > > > there are no pci_epf_drivers with a remove callback. > > > > > > So make it impossible for future drivers to return an unused error code > > > by changing the remove prototype to return void. > > > > > > The real motivation for this change is the quest to make struct > > > bus_type::remove return void, too. > > > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > > > Fine with this change! > > > > FWIW: > > Acked-by: Kishon Vijay Abraham I <kishon@ti.com> > > Thanks for the ack. How can I expect this patch to go into mainline now? > Will Bjorn pick it up now that you acked? There is a series[1] that I'd like to get into mainline during the next merge window and that depends on this patch. Ideally it would go in for v5.14-rc1, otherwise I'd like to add it to the series changing bus_type::remove that it goes in together. Would be sad if I had to delay this cleanup for this dependency not going in. Best regards Uwe [1] https://lore.kernel.org/lkml/20210706154803.1631813-1-u.kleine-koenig@pengutronix.de
Hi Lorenzo, On 08/07/21 2:53 pm, Uwe Kleine-König wrote: > On Mon, Jul 05, 2021 at 05:46:50PM +0200, Uwe Kleine-König wrote: >> Hello, >> >> On Tue, Jun 22, 2021 at 03:29:27PM +0530, Kishon Vijay Abraham I wrote: >>> On 23/02/21 2:37 pm, Uwe Kleine-König wrote: >>>> The driver core ignores the return value of pci_epf_device_remove() >>>> (because there is only little it can do when a device disappears) and >>>> there are no pci_epf_drivers with a remove callback. >>>> >>>> So make it impossible for future drivers to return an unused error code >>>> by changing the remove prototype to return void. >>>> >>>> The real motivation for this change is the quest to make struct >>>> bus_type::remove return void, too. >>>> >>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> >>> >>> Fine with this change! >>> >>> FWIW: >>> Acked-by: Kishon Vijay Abraham I <kishon@ti.com> >> >> Thanks for the ack. How can I expect this patch to go into mainline now? >> Will Bjorn pick it up now that you acked? > > There is a series[1] that I'd like to get into mainline during the next > merge window and that depends on this patch. Ideally it would go in > for v5.14-rc1, otherwise I'd like to add it to the series changing > bus_type::remove that it goes in together. Would be sad if I had to > delay this cleanup for this dependency not going in. Can you pick this change in the -rc cycle? Thank You, Kishon > > Best regards > Uwe > > [1] https://lore.kernel.org/lkml/20210706154803.1631813-1-u.kleine-koenig@pengutronix.de > >
On Tue, Feb 23, 2021 at 10:07:57AM +0100, Uwe Kleine-König wrote: > The driver core ignores the return value of pci_epf_device_remove() > (because there is only little it can do when a device disappears) and > there are no pci_epf_drivers with a remove callback. > > So make it impossible for future drivers to return an unused error code > by changing the remove prototype to return void. > > The real motivation for this change is the quest to make struct > bus_type::remove return void, too. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Acked-by: Bjorn Helgaas <bhelgaas@google.com> Can you just include this with the rest of your series that depends on it, like you did for the s390 patches, so they're all together? > --- > drivers/pci/endpoint/pci-epf-core.c | 5 ++--- > include/linux/pci-epf.h | 2 +- > 2 files changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c > index 7646c8660d42..a19c375f9ec9 100644 > --- a/drivers/pci/endpoint/pci-epf-core.c > +++ b/drivers/pci/endpoint/pci-epf-core.c > @@ -389,15 +389,14 @@ static int pci_epf_device_probe(struct device *dev) > > static int pci_epf_device_remove(struct device *dev) > { > - int ret = 0; > struct pci_epf *epf = to_pci_epf(dev); > struct pci_epf_driver *driver = to_pci_epf_driver(dev->driver); > > if (driver->remove) > - ret = driver->remove(epf); > + driver->remove(epf); > epf->driver = NULL; > > - return ret; > + return 0; > } > > static struct bus_type pci_epf_bus_type = { > diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h > index 6833e2160ef1..f8a17b6b1d31 100644 > --- a/include/linux/pci-epf.h > +++ b/include/linux/pci-epf.h > @@ -85,7 +85,7 @@ struct pci_epf_ops { > */ > struct pci_epf_driver { > int (*probe)(struct pci_epf *epf); > - int (*remove)(struct pci_epf *epf); > + void (*remove)(struct pci_epf *epf); > > struct device_driver driver; > struct pci_epf_ops *ops; > -- > 2.30.0 >
Hello Bjorn, On Mon, Jul 12, 2021 at 03:51:49PM -0500, Bjorn Helgaas wrote: > On Tue, Feb 23, 2021 at 10:07:57AM +0100, Uwe Kleine-König wrote: > > The driver core ignores the return value of pci_epf_device_remove() > > (because there is only little it can do when a device disappears) and > > there are no pci_epf_drivers with a remove callback. > > > > So make it impossible for future drivers to return an unused error code > > by changing the remove prototype to return void. > > > > The real motivation for this change is the quest to make struct > > bus_type::remove return void, too. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > Can you just include this with the rest of your series that depends on > it, like you did for the s390 patches, so they're all together? Yeah sure, will resend the complete series later today. I hesitated to include the pci patch as I didn't know your plans for it and didn't want to create a mess by interfering with your workflow. Thanks Uwe
diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c index 7646c8660d42..a19c375f9ec9 100644 --- a/drivers/pci/endpoint/pci-epf-core.c +++ b/drivers/pci/endpoint/pci-epf-core.c @@ -389,15 +389,14 @@ static int pci_epf_device_probe(struct device *dev) static int pci_epf_device_remove(struct device *dev) { - int ret = 0; struct pci_epf *epf = to_pci_epf(dev); struct pci_epf_driver *driver = to_pci_epf_driver(dev->driver); if (driver->remove) - ret = driver->remove(epf); + driver->remove(epf); epf->driver = NULL; - return ret; + return 0; } static struct bus_type pci_epf_bus_type = { diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h index 6833e2160ef1..f8a17b6b1d31 100644 --- a/include/linux/pci-epf.h +++ b/include/linux/pci-epf.h @@ -85,7 +85,7 @@ struct pci_epf_ops { */ struct pci_epf_driver { int (*probe)(struct pci_epf *epf); - int (*remove)(struct pci_epf *epf); + void (*remove)(struct pci_epf *epf); struct device_driver driver; struct pci_epf_ops *ops;
The driver core ignores the return value of pci_epf_device_remove() (because there is only little it can do when a device disappears) and there are no pci_epf_drivers with a remove callback. So make it impossible for future drivers to return an unused error code by changing the remove prototype to return void. The real motivation for this change is the quest to make struct bus_type::remove return void, too. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/pci/endpoint/pci-epf-core.c | 5 ++--- include/linux/pci-epf.h | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-)