diff mbox

[6/6] nx-842-platform: if NX842 platform drivers are not modules, don't try to load them

Message ID 20150702224225.GG1712@linux.vnet.ibm.com (mailing list archive)
State Changes Requested
Delegated to: Herbert Xu
Headers show

Commit Message

Nishanth Aravamudan July 2, 2015, 10:42 p.m. UTC
Based off the CONFIG_SPU_FS_MODULE code, only attempt to load platform
modules if the nx-842 pseries/powernv drivers are built as modules.

Otherwise, if CONFIG_DEV_NX_COMPRESS=y,
CONFIG_DEV_NX_COMPRESS_PSERIES=y, CONFIG_DEV_NX_POWERNV=y, the following
message is emitted at boot:

nx_compress: no nx842 driver found.

even though the drivers successfully loads.

This is because in the =y case, the module_init() calls get converted to
initcalls and the nx842_init() runs before the platform driver
nx842_pseries_init() or nx842_powernv_init() functions, which are what
normally set the static platform driver.

Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
Cc: Dan Streetman <ddstreet@us.ibm.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-crypto@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
---
 drivers/crypto/nx/nx-842-platform.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Herbert Xu July 6, 2015, 8:13 a.m. UTC | #1
On Thu, Jul 02, 2015 at 03:42:26PM -0700, Nishanth Aravamudan wrote:
> Based off the CONFIG_SPU_FS_MODULE code, only attempt to load platform
> modules if the nx-842 pseries/powernv drivers are built as modules.
> 
> Otherwise, if CONFIG_DEV_NX_COMPRESS=y,
> CONFIG_DEV_NX_COMPRESS_PSERIES=y, CONFIG_DEV_NX_POWERNV=y, the following
> message is emitted at boot:
> 
> nx_compress: no nx842 driver found.
> 
> even though the drivers successfully loads.
> 
> This is because in the =y case, the module_init() calls get converted to
> initcalls and the nx842_init() runs before the platform driver
> nx842_pseries_init() or nx842_powernv_init() functions, which are what
> normally set the static platform driver.
> 
> Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
> Cc: Dan Streetman <ddstreet@us.ibm.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: linux-crypto@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org

Ugh, I think this whole thing is redundant.  The whole point of
the crypto API is to allow the coexistence of multiple underlying
implementations.

Please get rid of nx-842-platform.c completely and move the crypto
registration into the individual platform drivers.  That is, powernv
and pseries should each register their own crypto driver.  They can
of course share a common set of crypto code which can live in its own
module.  There should be no need for mucking with module reference
counts at all.

Thanks,
Nishanth Aravamudan July 6, 2015, 5:07 p.m. UTC | #2
On 06.07.2015 [16:13:07 +0800], Herbert Xu wrote:
> On Thu, Jul 02, 2015 at 03:42:26PM -0700, Nishanth Aravamudan wrote:
> > Based off the CONFIG_SPU_FS_MODULE code, only attempt to load platform
> > modules if the nx-842 pseries/powernv drivers are built as modules.
> > 
> > Otherwise, if CONFIG_DEV_NX_COMPRESS=y,
> > CONFIG_DEV_NX_COMPRESS_PSERIES=y, CONFIG_DEV_NX_POWERNV=y, the following
> > message is emitted at boot:
> > 
> > nx_compress: no nx842 driver found.
> > 
> > even though the drivers successfully loads.
> > 
> > This is because in the =y case, the module_init() calls get converted to
> > initcalls and the nx842_init() runs before the platform driver
> > nx842_pseries_init() or nx842_powernv_init() functions, which are what
> > normally set the static platform driver.
> > 
> > Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
> > Cc: Dan Streetman <ddstreet@us.ibm.com>
> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: linux-crypto@vger.kernel.org
> > Cc: linuxppc-dev@lists.ozlabs.org
> 
> Ugh, I think this whole thing is redundant.  The whole point of
> the crypto API is to allow the coexistence of multiple underlying
> implementations.

Sure, that makes sense -- sorry, I was picking this up while Dan was on
vacation. Will provide a better v2.

> Please get rid of nx-842-platform.c completely and move the crypto
> registration into the individual platform drivers.  That is, powernv
> and pseries should each register their own crypto driver.  They can of
> course share a common set of crypto code which can live in its own
> module.  There should be no need for mucking with module reference
> counts at all.

Will do, thanks!

-Nish

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Streetman July 15, 2015, 2:33 p.m. UTC | #3
On Mon, Jul 6, 2015 at 1:07 PM, Nishanth Aravamudan
<nacc@linux.vnet.ibm.com> wrote:
> On 06.07.2015 [16:13:07 +0800], Herbert Xu wrote:
>> On Thu, Jul 02, 2015 at 03:42:26PM -0700, Nishanth Aravamudan wrote:
>> > Based off the CONFIG_SPU_FS_MODULE code, only attempt to load platform
>> > modules if the nx-842 pseries/powernv drivers are built as modules.
>> >
>> > Otherwise, if CONFIG_DEV_NX_COMPRESS=y,
>> > CONFIG_DEV_NX_COMPRESS_PSERIES=y, CONFIG_DEV_NX_POWERNV=y, the following
>> > message is emitted at boot:
>> >
>> > nx_compress: no nx842 driver found.
>> >
>> > even though the drivers successfully loads.
>> >
>> > This is because in the =y case, the module_init() calls get converted to
>> > initcalls and the nx842_init() runs before the platform driver
>> > nx842_pseries_init() or nx842_powernv_init() functions, which are what
>> > normally set the static platform driver.
>> >
>> > Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
>> > Cc: Dan Streetman <ddstreet@us.ibm.com>
>> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
>> > Cc: "David S. Miller" <davem@davemloft.net>
>> > Cc: linux-crypto@vger.kernel.org
>> > Cc: linuxppc-dev@lists.ozlabs.org
>>
>> Ugh, I think this whole thing is redundant.  The whole point of
>> the crypto API is to allow the coexistence of multiple underlying
>> implementations.
>
> Sure, that makes sense -- sorry, I was picking this up while Dan was on
> vacation. Will provide a better v2.
>
>> Please get rid of nx-842-platform.c completely and move the crypto
>> registration into the individual platform drivers.  That is, powernv
>> and pseries should each register their own crypto driver.  They can of
>> course share a common set of crypto code which can live in its own
>> module.  There should be no need for mucking with module reference
>> counts at all.
>
> Will do, thanks!

Yep, I originally did it this way because I didn't realize crypto
could register different drivers with the same alg name (but different
driver names).  I have some patches already to start doing this but
they weren't ready enough to send before I left for vacation; I'll
finish them up and send them.

>
> -Nish
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/crypto/nx/nx-842-platform.c b/drivers/crypto/nx/nx-842-platform.c
index 664f13dd06ed..5363c72593b4 100644
--- a/drivers/crypto/nx/nx-842-platform.c
+++ b/drivers/crypto/nx/nx-842-platform.c
@@ -53,6 +53,7 @@  void nx842_platform_driver_unset(struct nx842_driver *_driver)
 }
 EXPORT_SYMBOL_GPL(nx842_platform_driver_unset);
 
+#if defined(CONFIG_CRYPTO_DEV_NX_COMPRESS_PSERIES_MODULE) || defined(CONFIG_CRYPTO_DEV_NX_COMPRESS_POWERNV_MODULE)
 bool nx842_platform_driver_get(void)
 {
 	bool ret = false;
@@ -66,7 +67,6 @@  bool nx842_platform_driver_get(void)
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(nx842_platform_driver_get);
 
 void nx842_platform_driver_put(void)
 {
@@ -77,6 +77,17 @@  void nx842_platform_driver_put(void)
 
 	spin_unlock(&driver_lock);
 }
+#else
+bool nx842_platform_driver_get(void)
+{
+	return true;
+}
+
+void nx842_platform_driver_put(void)
+{
+}
+#endif
+EXPORT_SYMBOL_GPL(nx842_platform_driver_get);
 EXPORT_SYMBOL_GPL(nx842_platform_driver_put);
 
 MODULE_LICENSE("GPL");