[PATCHv3] crypto: inside-secure - Fix unused variable warning when CONFIG_PCI=n

[PATCHv3] crypto: inside-secure - Fix unused variable warning when CONFIG_PCI=n

[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

Re: [PATCHv3] crypto: inside-secure - Fix unused variable warning when CONFIG_PCI=n

[Herbert Xu]

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

RE: [PATCHv3] crypto: inside-secure - Fix unused variable warning when CONFIG_PCI=n

[Pascal Van Leeuwen]

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