Linux-Crypto Archive on lore.kernel.org
 help / color / Atom feed
* [PATCHv3] crypto: inside-secure - Fix unused variable warning when CONFIG_PCI=n
@ 2019-09-06 15:25 Pascal van Leeuwen
  2019-09-13  9:20 ` Herbert Xu
  0 siblings, 1 reply; 3+ messages in thread
From: Pascal van Leeuwen @ 2019-09-06 15:25 UTC (permalink / raw)
  To: linux-crypto; +Cc: antoine.tenart, herbert, davem, Pascal van Leeuwen

This patch fixes an unused variable warning from the compiler when the
driver is being compiled without PCI support in the kernel.

changes since v1:
- capture the platform_register_driver error code as well
- actually return the (last) error code
- swapped registration to do PCI first as that's just for development
  boards anyway, so in case both are done we want the platform error
  or no error at all if that passes
- also fixes some indentation issue in the affected code

changes since v2:
- handle the situation where both CONFIG_PCI and CONFIG_OF are undefined
  by always returning a -EINVAL error
- only unregister PCI or OF if it was previously successfully registered

Signed-off-by: Pascal van Leeuwen <pvanleeuwen@verimatrix.com>
---
 drivers/crypto/inside-secure/safexcel.c | 35 ++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index e12a2a3..925c90f 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -1501,32 +1501,45 @@ void safexcel_pci_remove(struct pci_dev *pdev)
 };
 #endif
 
-static int __init safexcel_init(void)
-{
-	int rc;
-
+/* Unfortunately, we have to resort to global variables here */
+#if IS_ENABLED(CONFIG_PCI)
+int pcireg_rc = -EINVAL; /* Default safe value */
+#endif
 #if IS_ENABLED(CONFIG_OF)
-		/* Register platform driver */
-		platform_driver_register(&crypto_safexcel);
+int ofreg_rc = -EINVAL; /* Default safe value */
 #endif
 
+static int __init safexcel_init(void)
+{
 #if IS_ENABLED(CONFIG_PCI)
-		/* Register PCI driver */
-		rc = pci_register_driver(&safexcel_pci_driver);
+	/* Register PCI driver */
+	pcireg_rc = pci_register_driver(&safexcel_pci_driver);
 #endif
 
-	return 0;
+#if IS_ENABLED(CONFIG_OF)
+	/* Register platform driver */
+	ofreg_rc = platform_driver_register(&crypto_safexcel);
+	return ofreg_rc;
+#else
+ #if IS_ENABLED(CONFIG_PCI)
+	return pcireg_rc;
+ #else
+	return -EINVAL;
+ #endif
+#endif
 }
 
 static void __exit safexcel_exit(void)
 {
 #if IS_ENABLED(CONFIG_OF)
-		/* Unregister platform driver */
+	/* Unregister platform driver */
+	if (!ofreg_rc)
 		platform_driver_unregister(&crypto_safexcel);
 #endif
 
 #if IS_ENABLED(CONFIG_PCI)
-		/* Unregister PCI driver if successfully registered before */
+	/* Unregister PCI driver if successfully registered before */
+	if (!pcireg_rc)
 		pci_unregister_driver(&safexcel_pci_driver);
 #endif
 }
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCHv3] crypto: inside-secure - Fix unused variable warning when CONFIG_PCI=n
  2019-09-06 15:25 [PATCHv3] crypto: inside-secure - Fix unused variable warning when CONFIG_PCI=n Pascal van Leeuwen
@ 2019-09-13  9:20 ` Herbert Xu
  2019-09-13  9:43   ` Pascal Van Leeuwen
  0 siblings, 1 reply; 3+ messages in thread
From: Herbert Xu @ 2019-09-13  9:20 UTC (permalink / raw)
  To: Pascal van Leeuwen
  Cc: linux-crypto, antoine.tenart, davem, Pascal van Leeuwen

On Fri, Sep 06, 2019 at 05:25:14PM +0200, Pascal van Leeuwen wrote:
> This patch fixes an unused variable warning from the compiler when the
> driver is being compiled without PCI support in the kernel.
> 
> changes since v1:
> - capture the platform_register_driver error code as well
> - actually return the (last) error code
> - swapped registration to do PCI first as that's just for development
>   boards anyway, so in case both are done we want the platform error
>   or no error at all if that passes
> - also fixes some indentation issue in the affected code
> 
> changes since v2:
> - handle the situation where both CONFIG_PCI and CONFIG_OF are undefined
>   by always returning a -EINVAL error
> - only unregister PCI or OF if it was previously successfully registered
> 
> Signed-off-by: Pascal van Leeuwen <pvanleeuwen@verimatrix.com>
> ---
>  drivers/crypto/inside-secure/safexcel.c | 35 ++++++++++++++++++++++-----------
>  1 file changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
> index e12a2a3..925c90f 100644
> --- a/drivers/crypto/inside-secure/safexcel.c
> +++ b/drivers/crypto/inside-secure/safexcel.c
> @@ -1501,32 +1501,45 @@ void safexcel_pci_remove(struct pci_dev *pdev)
>  };
>  #endif
>  
> -static int __init safexcel_init(void)
> -{
> -	int rc;
> -
> +/* Unfortunately, we have to resort to global variables here */
> +#if IS_ENABLED(CONFIG_PCI)
> +int pcireg_rc = -EINVAL; /* Default safe value */
> +#endif
>  #if IS_ENABLED(CONFIG_OF)
> -		/* Register platform driver */
> -		platform_driver_register(&crypto_safexcel);
> +int ofreg_rc = -EINVAL; /* Default safe value */
>  #endif
>  
> +static int __init safexcel_init(void)
> +{
>  #if IS_ENABLED(CONFIG_PCI)
> -		/* Register PCI driver */
> -		rc = pci_register_driver(&safexcel_pci_driver);
> +	/* Register PCI driver */
> +	pcireg_rc = pci_register_driver(&safexcel_pci_driver);
>  #endif
>  
> -	return 0;
> +#if IS_ENABLED(CONFIG_OF)
> +	/* Register platform driver */
> +	ofreg_rc = platform_driver_register(&crypto_safexcel);
> +	return ofreg_rc;

If OF registration fails then you will return an error even if
PCI registration succeeded without undoing the PCI registration.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCHv3] crypto: inside-secure - Fix unused variable warning when CONFIG_PCI=n
  2019-09-13  9:20 ` Herbert Xu
@ 2019-09-13  9:43   ` Pascal Van Leeuwen
  0 siblings, 0 replies; 3+ messages in thread
From: Pascal Van Leeuwen @ 2019-09-13  9:43 UTC (permalink / raw)
  To: Herbert Xu, Pascal van Leeuwen; +Cc: linux-crypto, antoine.tenart, davem

> -----Original Message-----
> From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf
> Of Herbert Xu
> Sent: Friday, September 13, 2019 11:21 AM
> To: Pascal van Leeuwen <pascalvanl@gmail.com>
> Cc: linux-crypto@vger.kernel.org; antoine.tenart@bootlin.com; davem@davemloft.net;
> Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> Subject: Re: [PATCHv3] crypto: inside-secure - Fix unused variable warning when
> CONFIG_PCI=n
> 
> On Fri, Sep 06, 2019 at 05:25:14PM +0200, Pascal van Leeuwen wrote:
> > This patch fixes an unused variable warning from the compiler when the
> > driver is being compiled without PCI support in the kernel.
> >
> > changes since v1:
> > - capture the platform_register_driver error code as well
> > - actually return the (last) error code
> > - swapped registration to do PCI first as that's just for development
> >   boards anyway, so in case both are done we want the platform error
> >   or no error at all if that passes
> > - also fixes some indentation issue in the affected code
> >
> > changes since v2:
> > - handle the situation where both CONFIG_PCI and CONFIG_OF are undefined
> >   by always returning a -EINVAL error
> > - only unregister PCI or OF if it was previously successfully registered
> >
> > Signed-off-by: Pascal van Leeuwen <pvanleeuwen@verimatrix.com>
> > ---
> >  drivers/crypto/inside-secure/safexcel.c | 35 ++++++++++++++++++++++-----------
> >  1 file changed, 24 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-
> secure/safexcel.c
> > index e12a2a3..925c90f 100644
> > --- a/drivers/crypto/inside-secure/safexcel.c
> > +++ b/drivers/crypto/inside-secure/safexcel.c
> > @@ -1501,32 +1501,45 @@ void safexcel_pci_remove(struct pci_dev *pdev)
> >  };
> >  #endif
> >
> > -static int __init safexcel_init(void)
> > -{
> > -	int rc;
> > -
> > +/* Unfortunately, we have to resort to global variables here */
> > +#if IS_ENABLED(CONFIG_PCI)
> > +int pcireg_rc = -EINVAL; /* Default safe value */
> > +#endif
> >  #if IS_ENABLED(CONFIG_OF)
> > -		/* Register platform driver */
> > -		platform_driver_register(&crypto_safexcel);
> > +int ofreg_rc = -EINVAL; /* Default safe value */
> >  #endif
> >
> > +static int __init safexcel_init(void)
> > +{
> >  #if IS_ENABLED(CONFIG_PCI)
> > -		/* Register PCI driver */
> > -		rc = pci_register_driver(&safexcel_pci_driver);
> > +	/* Register PCI driver */
> > +	pcireg_rc = pci_register_driver(&safexcel_pci_driver);
> >  #endif
> >
> > -	return 0;
> > +#if IS_ENABLED(CONFIG_OF)
> > +	/* Register platform driver */
> > +	ofreg_rc = platform_driver_register(&crypto_safexcel);
> > +	return ofreg_rc;
> 
> If OF registration fails then you will return an error even if
> PCI registration succeeded without undoing the PCI registration.
> 
... because the _exit does not get called. Crap, did not realise that :-(
Will fix and send a PATCHv4 shortly ...

> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Thanks,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
www.insidesecure.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-06 15:25 [PATCHv3] crypto: inside-secure - Fix unused variable warning when CONFIG_PCI=n Pascal van Leeuwen
2019-09-13  9:20 ` Herbert Xu
2019-09-13  9:43   ` Pascal Van Leeuwen

Linux-Crypto Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-crypto/0 linux-crypto/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-crypto linux-crypto/ https://lore.kernel.org/linux-crypto \
		linux-crypto@vger.kernel.org linux-crypto@archiver.kernel.org
	public-inbox-index linux-crypto


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-crypto


AGPL code for this site: git clone https://public-inbox.org/ public-inbox