drivers/nx-842: reduce verbosity of logging

While we never would successfully load on the wrong machine type, there is extra output by default regardless of machine type.

For instance, on a PowerVM LPAR, we see the following:

nx_compress_powernv: loading
nx_compress_powernv: no coprocessors found

even though those coprocessors could never be found. Similar pseries messages are printed on powernv.

Quoted 8 lines · Nishanth Aravamudan

While we never would successfully load on the wrong machine type, there is extra output by default regardless of machine type.

For instance, on a PowerVM LPAR, we see the following:

nx_compress_powernv: loading
nx_compress_powernv: no coprocessors found

Quoted 2 lines

even though those coprocessors could never be found. Similar pseries messages are printed on powernv.

I know I've been converting init calls to machine_initcalls() to avoid these sort of issues in platform code. But for a driver it should be trivial for it to only probe when the hardware is found.

By which I mean I think we shouldn't need these.

drivers/crypto/nx/nx-842-powernv.c static __init int nx842_powernv_init(void) · line 600

Quoted 12 lines

drivers/crypto/nx/nx-842-powernv.c +3 -0

index 33b3b0abf4ae..6b5e5143c95b 100644
@@ -594,6 +594,9 @@ static __init int nx842_powernv_init(void)
 	BUILD_BUG_ON(DDE_BUFFER_ALIGN % DDE_BUFFER_SIZE_MULT);
 	BUILD_BUG_ON(DDE_BUFFER_SIZE_MULT % DDE_BUFFER_LAST_MULT);
 
+	if (!machine_is(powernv))
+		return -ENODEV;
+
 	pr_info("loading\n");

This is just too chatty, drop it.

drivers/crypto/nx/nx-842-powernv.c static __init int nx842_powernv_init(void) · line 601

Quoted 1 line

drivers/crypto/nx/nx-842-powernv.c +0 -0

 	for_each_compatible_node(dn, NULL, "ibm,power-nx")

It shouldn't be printing anything unless it finds some devices in this loop.

And you should drop the print in here:

	if (!nx842_ct) {
		pr_err("no coprocessors found\n");
		return -ENODEV;
	}

And that should mean no output unless hardware is found I think?

drivers/crypto/nx/nx-842-powernv.c static void __exit nx842_powernv_exit(void) · line 632

Quoted 6 lines

drivers/crypto/nx/nx-842-powernv.c +2 -0

@@ -625,6 +628,9 @@ static void __exit nx842_powernv_exit(void)
 {
 	struct nx842_coproc *coproc, *n;
 
+	if (!machine_is(powernv))
+		return;

You shouldn't need to touch the exit paths if the drivers were never loaded?

Quoted 8 lines

drivers/crypto/nx/nx-842-pseries.c +0 -0

index b84b0ceeb46e..75a7bfdc160e 100644
@@ -1091,6 +1091,9 @@ static int __init nx842_pseries_init(void)
 	struct nx842_devdata *new_devdata;
 	int ret;
 

drivers/crypto/nx/nx-842-pseries.c static int __init nx842_pseries_init(void) · line 1097

Quoted 4 lines

drivers/crypto/nx/nx-842-pseries.c +3 -0

+	if (!machine_is(pseries))
+		return -ENODEV;
+
 	pr_info("Registering IBM Power 842 compression driver\n");

Again this is too chatty, just remove it.

drivers/crypto/nx/nx-842-pseries.c static int __init nx842_pseries_init(void) · line 1098

Quoted 1 line

drivers/crypto/nx/nx-842-pseries.c +0 -0

 	if (!of_find_compatible_node(NULL, NULL, "ibm,compression"))

		return -ENODEV;

That should do the trick shouldn't it?

Quoted 4 lines · Michael Ellerman

Quoted 3 lines · Nishanth Aravamudan

While we never would successfully load on the wrong machine type, there is extra output by default regardless of machine type.

Thanks Michael!

While we never would successfully load on the wrong machine type, there is extra output by default regardless of machine type.

For instance, on a PowerVM LPAR, we see the following:

nx_compress_powernv: loading
nx_compress_powernv: no coprocessors found

even though those coprocessors could never be found.

Signed-off-by Nishanth Aravamudan <[email protected]>

Cc Dan Streetman <[email protected]>

Cc Herbert Xu <[email protected]>

Cc "David S. Miller" <[email protected]>

Cc [email protected]

Cc [email protected]

---
v2:
Rather than not loading, just reduce the verbosity

v2:
Rather than not loading, just reduce the verbosity

Currently, when the nx-842-pseries driver loads, the following message is emitted:

alg: No test for 842 (842-nx)

It seems like the simplest way to fix this message (other than adding a proper test) is to just insert the null test into the list in the testmgr.

Signed-off-by Nishanth Aravamudan <[email protected]>

---
crypto/testmgr.c | 3 +++

1 file changed, 3 insertions(+)

crypto/testmgr.c +0 -0

index d0a42bd3aae9..ff0f76e0d0b4 100644

Currently, when the nx-842-pseries driver loads, the following message is emitted:

alg: No test for 842 (842-nx)

It seems like the simplest way to fix this message (other than adding a proper test) is to just insert the null test into the list in the testmgr.

Signed-off-by Nishanth Aravamudan <[email protected]>

Quoted 10 lines · Nishanth Aravamudan

Currently, when the nx-842-pseries driver loads, the following message is emitted:

alg: No test for 842 (842-nx)

It seems like the simplest way to fix this message (other than adding a proper test) is to just insert the null test into the list in the testmgr.

Signed-off-by Nishanth Aravamudan <[email protected]>

Please add some real test vectors instead.

Quoted 6 lines · Herbert Xu

Quoted 5 lines · Nishanth Aravamudan

Currently, when the nx-842-pseries driver loads, the following message is emitted:

alg: No test for 842 (842-nx)

Quoted 7 lines

Quoted 5 lines

It seems like the simplest way to fix this message (other than adding a proper test) is to just insert the null test into the list in the testmgr.

Signed-off-by Nishanth Aravamudan <[email protected]>

Please add some real test vectors instead.

Quoted 11 lines · Nishanth Aravamudan

Quoted 10 lines · Herbert Xu

Quoted 9 lines · Nishanth Aravamudan

Currently, when the nx-842-pseries driver loads, the following message is emitted:

alg: No test for 842 (842-nx)

It seems like the simplest way to fix this message (other than adding a proper test) is to just insert the null test into the list in the testmgr.

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

Signed-off-by Nishanth Aravamudan <[email protected]>

Please add some real test vectors instead.

Apologies, hit send too fast. I'll work with Dan on this when he gets back from vacation.

-Nish

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

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 <[email protected]>

Cc Dan Streetman <[email protected]>

Cc Herbert Xu <[email protected]>

Cc "David S. Miller" <[email protected]>

Cc [email protected]

Cc [email protected]

Quoted 16 lines · Nishanth Aravamudan

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.

Quoted 6 lines

Signed-off-by Nishanth Aravamudan <[email protected]>

Cc Dan Streetman <[email protected]>

Cc Herbert Xu <[email protected]>

Cc "David S. Miller" <[email protected]>

Cc [email protected]

Cc [email protected]

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.

Quoted 24 lines · Herbert Xu

Quoted 16 lines · Nishanth Aravamudan

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.

Quoted 6 lines

Signed-off-by Nishanth Aravamudan <[email protected]>

Cc Dan Streetman <[email protected]>

Cc Herbert Xu <[email protected]>

Cc "David S. Miller" <[email protected]>

Cc [email protected]

Cc [email protected]

Quoted 3 lines

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.

Quoted 6 lines

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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Currently, on a LPAR with the nx-842 device disabled, the following messages are emitted: