| Message ID | 1567783514-24947-1-git-send-email-pvanleeuwen@verimatrix.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | [PATCHv3] crypto: inside-secure - Fix unused variable warning when CONFIG_PCI=n | expand |
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,
> -----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
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 }
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(-)