> -----Original Message-----
> From: linux-crypto-owner@xxxxxxxxxxxxxxx <linux-crypto-owner@xxxxxxxxxxxxxxx> On Behalf
> Of Herbert Xu
> Sent: Friday, September 6, 2019 2:19 PM
> To: Pascal van Leeuwen <pascalvanl@xxxxxxxxx>
> Cc: linux-crypto@xxxxxxxxxxxxxxx; antoine.tenart@xxxxxxxxxxx; davem@xxxxxxxxxxxxx;
> Pascal Van Leeuwen <pvanleeuwen@xxxxxxxxxxxxxx>; Bjorn Helgaas <helgaas@xxxxxxxxxx>
> Subject: Re: [PATCHv2] crypto: inside-secure - Fix unused variable warning when
> CONFIG_PCI=n
>
> On Fri, Sep 06, 2019 at 10:07:23AM +0200, Pascal van Leeuwen wrote:
> >
> > diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-
> secure/safexcel.c
> > index e12a2a3..2331b31 100644
> > --- a/drivers/crypto/inside-secure/safexcel.c
> > +++ b/drivers/crypto/inside-secure/safexcel.c
> > @@ -1505,29 +1505,29 @@ static int __init safexcel_init(void)
> > {
> > int rc;
> >
> > -#if IS_ENABLED(CONFIG_OF)
> > - /* Register platform driver */
> > - platform_driver_register(&crypto_safexcel);
> > +#if IS_ENABLED(CONFIG_PCI)
> > + /* Register PCI driver */
> > + rc = pci_register_driver(&safexcel_pci_driver);
> > #endif
> >
> > -#if IS_ENABLED(CONFIG_PCI)
> > - /* Register PCI driver */
> > - rc = pci_register_driver(&safexcel_pci_driver);
> > +#if IS_ENABLED(CONFIG_OF)
> > + /* Register platform driver */
> > + rc = platform_driver_register(&crypto_safexcel);
> > #endif
> >
> > - return 0;
> > + return rc;
> > }
>
> According to the Kconfig it is theoretically possible for both
> PCI and OF to be off (with COMPILE_TEST enabled). So you should
> add an rc = 0 at the top.
>
Ok
> You also need to check rc after each registration and abort if
> an error is detected. After the second step, aborting would
> also require unwinding the first step.
>
I explicitly DON'T want to abort if the PCI registration fails,
since that may be irrelevant if the OF registration passes AND
the device actually happens to be Device Tree.
So not checking the result value is on purpose here.
> So something like:
>
> int rc = 0;
>
> #if IS_ENABLED(CONFIG_PCI)
> /* Register PCI driver */
> rc = pci_register_driver(&safexcel_pci_driver);
> #endif
> if (rc)
> goto out;
>
> #if IS_ENABLED(CONFIG_OF)
> /* Register platform driver */
> rc = platform_driver_register(&crypto_safexcel);
> #endif
> if (rc)
> goto undo_pci;
>
> undo_pci:
> #if IS_ENABLED(CONFIG_PCI)
> pci_unregister_driver(&safexcel_pci_driver);
> #endif
> out:
> return rc;
>
> As you can see, these ifdefs get out-of-control pretty quickly.
> In fact, we can remove all the CONFIG_PCI ifdefs by adding just
> one more stub function in pci.h for pcim_enable_device.
>
I'm fine with that, too. I did not want these ifdef's in the first
place, I was asked to put them in.
> Thanks,
> --
> Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
www.insidesecure.com
