[PATCH] PCI: endpoint: Make struct pci_epf_driver::remove return void

[PATCH] PCI: endpoint: Make struct pci_epf_driver::remove return void

[Uwe Kleine-König]

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(-)

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

Re: [PATCH] PCI: endpoint: Make struct pci_epf_driver::remove return void

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2217 bytes --]

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
> 
> 
> 

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] PCI: endpoint: Make struct pci_epf_driver::remove return void

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 2121 bytes --]

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;
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] PCI: endpoint: Make struct pci_epf_driver::remove return void

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2702 bytes --]

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;
> > 
> 
> 




-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] PCI: endpoint: Make struct pci_epf_driver::remove return void

[Kishon Vijay Abraham I]

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;
> 

Re: [PATCH] PCI: endpoint: Make struct pci_epf_driver::remove return void

[Greg Kroah-Hartman]

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>

Re: [PATCH] PCI: endpoint: Make struct pci_epf_driver::remove return void

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1059 bytes --]

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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] PCI: endpoint: Make struct pci_epf_driver::remove return void

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1601 bytes --]

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


-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] PCI: endpoint: Make struct pci_epf_driver::remove return void

[Kishon Vijay Abraham I]

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
> 
> 

Re: [PATCH] PCI: endpoint: Make struct pci_epf_driver::remove return void

[Bjorn Helgaas]

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
> 

Re: [PATCH] PCI: endpoint: Make struct pci_epf_driver::remove return void

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1256 bytes --]

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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]