Currently, on a LPAR with the nx-842 device disabled, the following
messages are emitted:
nx_compress: no nx842 driver found. [1]
Registering IBM Power 842 compression driver
nx_compress_pseries ibm,compression-v1: nx842_OF_upd_status: status 'disabled' is not 'okay'
nx_compress_pseries ibm,compression-v1: nx842_OF_upd: max_sync_size new:4096 old:0 [2]
nx_compress_pseries ibm,compression-v1: nx842_OF_upd: max_sync_sg new:510 old:0
nx_compress_pseries ibm,compression-v1: nx842_OF_upd: max_sg_len new:4080 old:0
nx_compress_powernv: loading [3]
nx_compress_powernv: no coprocessors found
alg: No test for 842 (842-nx) [4]
[1] is the result of an ordering issue when the CONFIG_ options are set =y. [2]
is the result of nx842_OF_upd_status() not returning the correct error code.
[3] is the result of attempting to load the PowerNV platform driver on a
non-PowerNV platform. [4] is the result of there simply not being any test for
842 in the crypto test manager.
After the changes in the series, the same system as above emits:
Registering IBM Power 842 compression driver
nx_compress_pseries ibm,compression-v1: nx842_OF_upd: device disabled
which seems to me, at least, to be far clearer.
Dan, I think there is still a issue in the code. If CONFIG_DEV_NX_COMPRESS=y
and CONFIG_DEV_NX_COMPRESS_PSERIES=m/CONFIG_DEV_NX_COMPRESS_POWERNV=m, it seems
like the request_module() is not working properly and we simply get a "
nx_compress: no nx842 driver found." at boot (even if I ensure the platform
drivers are in the initrd). If I make CONFIG_DEV_NX_COMPRESS=m, though, the
module(s) load successfully. Does it make sense/is it possible to have these
three symbols always be the same (either all =y or all =m)?
[1/6] crypto/nx-842-pseries: nx842_OF_upd_status should return ENODEV if device is not 'okay'
drivers/crypto/nx/nx-842-pseries.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[2/6] nx-842-pseries: rename nx842_{init,exit} to nx842_pseries_{init,exit}
drivers/crypto/nx/nx-842-pseries.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[3/6] nx-842-pseries: do not emit extra output if status is disabled
drivers/crypto/nx/nx-842-pseries.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
[4/6] crypto/nx-842-{powerpc,pseries}: only load on the appropriate machine type
drivers/crypto/nx/nx-842-powernv.c | 6 ++++++
drivers/crypto/nx/nx-842-pseries.c | 6 ++++++
drivers/crypto/nx/nx-842.h | 1 +
3 files changed, 13 insertions(+)
[5/6] crypto/testmgr: add null test for 842 algorithm
crypto/testmgr.c | 3 +++
1 file changed, 3 insertions(+)
[6/6] nx-842-platform: if NX842 platform drivers are not modules, don't try to load them
drivers/crypto/nx/nx-842-platform.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
The current documention mentions explicitly that EINVAL should be
returned if the device is not available, but nx842_OF_upd_status()
always returns 0. However, nx842_probe() specifically checks for
non-ENODEV returns from nx842_of_upd() (which in turn calls
nx842_OF_upd_status()) and emits an extra error in that case. It seems
like the proper return code of a disabled device is ENODEV.
Signed-off-by: Nishanth Aravamudan <[email protected]>
---
drivers/crypto/nx/nx-842-pseries.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/nx/nx-842-pseries.c b/drivers/crypto/nx/nx-842-pseries.c
index 3040a6091bf2..819c23c546e3 100644
--- a/drivers/crypto/nx/nx-842-pseries.c
+++ b/drivers/crypto/nx/nx-842-pseries.c
@@ -556,7 +556,7 @@ static int nx842_OF_set_defaults(struct nx842_devdata *devdata)
*
* Returns:
* 0 - Device is available
- * -EINVAL - Device is not available
+ * -ENODEV - Device is not available
*/
static int nx842_OF_upd_status(struct nx842_devdata *devdata,
struct property *prop) {
@@ -569,6 +569,7 @@ static int nx842_OF_upd_status(struct nx842_devdata *devdata,
dev_info(devdata->dev, "%s: status '%s' is not 'okay'\n",
__func__, status);
devdata->status = UNAVAILABLE;
+ ret = -ENODEV;
}
return ret;
--
2.1.4
While there is no technical reason that both nx-842.c and
nx-842-pseries.c can have the same name for the init/exit functions, it
is a bit confusing with initcall_debug. Rename the pseries specific
functions appropriately
Signed-off-by: Nishanth Aravamudan <[email protected]>
---
drivers/crypto/nx/nx-842-pseries.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/nx/nx-842-pseries.c b/drivers/crypto/nx/nx-842-pseries.c
index 819c23c546e3..e17f4d2e96e0 100644
--- a/drivers/crypto/nx/nx-842-pseries.c
+++ b/drivers/crypto/nx/nx-842-pseries.c
@@ -1080,7 +1080,7 @@ static struct vio_driver nx842_vio_driver = {
.id_table = nx842_vio_driver_ids,
};
-static int __init nx842_init(void)
+static int __init nx842_pseries_init(void)
{
struct nx842_devdata *new_devdata;
int ret;
@@ -1116,9 +1116,9 @@ static int __init nx842_init(void)
return 0;
}
-module_init(nx842_init);
+module_init(nx842_pseries_init);
-static void __exit nx842_exit(void)
+static void __exit nx842_pseries_exit(void)
{
struct nx842_devdata *old_devdata;
unsigned long flags;
@@ -1137,5 +1137,5 @@ static void __exit nx842_exit(void)
vio_unregister_driver(&nx842_vio_driver);
}
-module_exit(nx842_exit);
+module_exit(nx842_pseries_exit);
--
2.1.4
If the device-tree indicates the nx-842 device's status is 'disabled',
we emit two messages:
nx_compress_pseries ibm,compression-v1: nx842_OF_upd_status: status 'disabled' is not 'okay'.
nx_compress_pseries ibm,compression-v1: nx842_OF_upd: device disabled
Given that 'disabled' is a valid state, and we are going to emit that
the device is disabled, only print out a non-'okay' status if it is not
'disabled'.
Signed-off-by: Nishanth Aravamudan <[email protected]>
---
drivers/crypto/nx/nx-842-pseries.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/nx/nx-842-pseries.c b/drivers/crypto/nx/nx-842-pseries.c
index e17f4d2e96e0..b84b0ceeb46e 100644
--- a/drivers/crypto/nx/nx-842-pseries.c
+++ b/drivers/crypto/nx/nx-842-pseries.c
@@ -566,8 +566,14 @@ static int nx842_OF_upd_status(struct nx842_devdata *devdata,
if (!strncmp(status, "okay", (size_t)prop->length)) {
devdata->status = AVAILABLE;
} else {
- dev_info(devdata->dev, "%s: status '%s' is not 'okay'\n",
+ /*
+ * Caller will log that the device is disabled, so only
+ * output if there is an unexpected status.
+ */
+ if (strncmp(status, "disabled", (size_t)prop->length)) {
+ dev_info(devdata->dev, "%s: status '%s' is not 'okay'\n",
__func__, status);
+ }
devdata->status = UNAVAILABLE;
ret = -ENODEV;
}
--
2.1.4
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.
Signed-off-by: Nishanth Aravamudan <[email protected]>
---
drivers/crypto/nx/nx-842-powernv.c | 6 ++++++
drivers/crypto/nx/nx-842-pseries.c | 6 ++++++
drivers/crypto/nx/nx-842.h | 1 +
3 files changed, 13 insertions(+)
diff --git a/drivers/crypto/nx/nx-842-powernv.c b/drivers/crypto/nx/nx-842-powernv.c
index 33b3b0abf4ae..6b5e5143c95b 100644
--- a/drivers/crypto/nx/nx-842-powernv.c
+++ b/drivers/crypto/nx/nx-842-powernv.c
@@ -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");
for_each_compatible_node(dn, NULL, "ibm,power-nx")
@@ -625,6 +628,9 @@ static void __exit nx842_powernv_exit(void)
{
struct nx842_coproc *coproc, *n;
+ if (!machine_is(powernv))
+ return;
+
nx842_platform_driver_unset(&nx842_powernv_driver);
list_for_each_entry_safe(coproc, n, &nx842_coprocs, list) {
diff --git a/drivers/crypto/nx/nx-842-pseries.c b/drivers/crypto/nx/nx-842-pseries.c
index b84b0ceeb46e..75a7bfdc160e 100644
--- a/drivers/crypto/nx/nx-842-pseries.c
+++ b/drivers/crypto/nx/nx-842-pseries.c
@@ -1091,6 +1091,9 @@ static int __init nx842_pseries_init(void)
struct nx842_devdata *new_devdata;
int ret;
+ if (!machine_is(pseries))
+ return -ENODEV;
+
pr_info("Registering IBM Power 842 compression driver\n");
if (!of_find_compatible_node(NULL, NULL, "ibm,compression"))
@@ -1129,6 +1132,9 @@ static void __exit nx842_pseries_exit(void)
struct nx842_devdata *old_devdata;
unsigned long flags;
+ if (!machine_is(pseries))
+ return;
+
pr_info("Exiting IBM Power 842 compression driver\n");
nx842_platform_driver_unset(&nx842_pseries_driver);
spin_lock_irqsave(&devdata_mutex, flags);
diff --git a/drivers/crypto/nx/nx-842.h b/drivers/crypto/nx/nx-842.h
index ac0ea79d0f8b..ea89c661e476 100644
--- a/drivers/crypto/nx/nx-842.h
+++ b/drivers/crypto/nx/nx-842.h
@@ -10,6 +10,7 @@
#include <linux/io.h>
#include <linux/mm.h>
#include <linux/ratelimit.h>
+#include <asm/machdep.h>
/* Restrictions on Data Descriptor List (DDL) and Entry (DDE) buffers
*
--
2.1.4
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(+)
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index d0a42bd3aae9..ff0f76e0d0b4 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1982,6 +1982,9 @@ static int alg_test_null(const struct alg_test_desc *desc,
/* Please keep this list sorted by algorithm name. */
static const struct alg_test_desc alg_test_descs[] = {
{
+ .alg = "842",
+ .test = alg_test_null,
+ }, {
.alg = "__cbc-cast5-avx",
.test = alg_test_null,
}, {
--
2.1.4
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]
---
drivers/crypto/nx/nx-842-platform.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
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");
--
2.1.4
On Thu, 2015-07-02 at 15:40 -0700, Nishanth Aravamudan wrote:
> 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.
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.
> diff --git a/drivers/crypto/nx/nx-842-powernv.c b/drivers/crypto/nx/nx-842-powernv.c
> index 33b3b0abf4ae..6b5e5143c95b 100644
> --- a/drivers/crypto/nx/nx-842-powernv.c
> +++ b/drivers/crypto/nx/nx-842-powernv.c
> @@ -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.
> 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?
> @@ -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?
> diff --git a/drivers/crypto/nx/nx-842-pseries.c b/drivers/crypto/nx/nx-842-pseries.c
> index b84b0ceeb46e..75a7bfdc160e 100644
> --- a/drivers/crypto/nx/nx-842-pseries.c
> +++ b/drivers/crypto/nx/nx-842-pseries.c
> @@ -1091,6 +1091,9 @@ static int __init nx842_pseries_init(void)
> struct nx842_devdata *new_devdata;
> int ret;
>
> + if (!machine_is(pseries))
> + return -ENODEV;
> +
> pr_info("Registering IBM Power 842 compression driver\n");
Again this is too chatty, just remove it.
> if (!of_find_compatible_node(NULL, NULL, "ibm,compression"))
return -ENODEV;
That should do the trick shouldn't it?
cheers
Am Donnerstag, 2. Juli 2015, 15:41:19 schrieb Nishanth Aravamudan:
Hi Nishanth,
>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(+)
>
>diff --git a/crypto/testmgr.c b/crypto/testmgr.c
>index d0a42bd3aae9..ff0f76e0d0b4 100644
>--- a/crypto/testmgr.c
>+++ b/crypto/testmgr.c
>@@ -1982,6 +1982,9 @@ static int alg_test_null(const struct alg_test_desc
>*desc, /* Please keep this list sorted by algorithm name. */
> static const struct alg_test_desc alg_test_descs[] = {
> {
>+ .alg = "842",
>+ .test = alg_test_null,
>+ }, {
> .alg = "__cbc-cast5-avx",
> .test = alg_test_null,
As this is a compression algo, it is safe to add fips_allowed = 1 here as
otherwise the algo is not available in fips=1
> }, {
Ciao
Stephan
On Thu, Jul 02, 2015 at 03:41:19PM -0700, Nishanth Aravamudan wrote:
> 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.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev
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 <[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.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Thu, Jul 02, 2015 at 03:38:00PM -0700, Nishanth Aravamudan wrote:
> Currently, on a LPAR with the nx-842 device disabled, the following
> messages are emitted:
Patches 1-3 applied.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On 03.07.2015 [11:30:32 +1000], Michael Ellerman wrote:
> On Thu, 2015-07-02 at 15:40 -0700, Nishanth Aravamudan wrote:
> > 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.
>
> 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.
Ok.
> > diff --git a/drivers/crypto/nx/nx-842-powernv.c b/drivers/crypto/nx/nx-842-powernv.c
> > index 33b3b0abf4ae..6b5e5143c95b 100644
> > --- a/drivers/crypto/nx/nx-842-powernv.c
> > +++ b/drivers/crypto/nx/nx-842-powernv.c
> > @@ -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.
>
> > 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?
Yep, will adjust.
> > @@ -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?
Duh, yep.
> > diff --git a/drivers/crypto/nx/nx-842-pseries.c b/drivers/crypto/nx/nx-842-pseries.c
> > index b84b0ceeb46e..75a7bfdc160e 100644
> > --- a/drivers/crypto/nx/nx-842-pseries.c
> > +++ b/drivers/crypto/nx/nx-842-pseries.c
> > @@ -1091,6 +1091,9 @@ static int __init nx842_pseries_init(void)
> > struct nx842_devdata *new_devdata;
> > int ret;
> >
> > + if (!machine_is(pseries))
> > + return -ENODEV;
> > +
> > pr_info("Registering IBM Power 842 compression driver\n");
>
> Again this is too chatty, just remove it.
Will do.
> > if (!of_find_compatible_node(NULL, NULL, "ibm,compression"))
> return -ENODEV;
>
> That should do the trick shouldn't it?
Yep, I think so.
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
drivers/crypto/nx/nx-842-powernv.c | 10 +---------
drivers/crypto/nx/nx-842-pseries.c | 3 ---
2 files changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/crypto/nx/nx-842-powernv.c b/drivers/crypto/nx/nx-842-powernv.c
index 33b3b0abf4ae..7e474562058d 100644
--- a/drivers/crypto/nx/nx-842-powernv.c
+++ b/drivers/crypto/nx/nx-842-powernv.c
@@ -594,15 +594,11 @@ 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);
- pr_info("loading\n");
-
for_each_compatible_node(dn, NULL, "ibm,power-nx")
nx842_powernv_probe(dn);
- if (!nx842_ct) {
- pr_err("no coprocessors found\n");
+ if (!nx842_ct)
return -ENODEV;
- }
if (!nx842_platform_driver_set(&nx842_powernv_driver)) {
struct nx842_coproc *coproc, *n;
@@ -615,8 +611,6 @@ static __init int nx842_powernv_init(void)
return -EEXIST;
}
- pr_info("loaded\n");
-
return 0;
}
module_init(nx842_powernv_init);
@@ -631,7 +625,5 @@ static void __exit nx842_powernv_exit(void)
list_del(&coproc->list);
kfree(coproc);
}
-
- pr_info("unloaded\n");
}
module_exit(nx842_powernv_exit);
diff --git a/drivers/crypto/nx/nx-842-pseries.c b/drivers/crypto/nx/nx-842-pseries.c
index b84b0ceeb46e..d44524da6589 100644
--- a/drivers/crypto/nx/nx-842-pseries.c
+++ b/drivers/crypto/nx/nx-842-pseries.c
@@ -1091,8 +1091,6 @@ static int __init nx842_pseries_init(void)
struct nx842_devdata *new_devdata;
int ret;
- pr_info("Registering IBM Power 842 compression driver\n");
-
if (!of_find_compatible_node(NULL, NULL, "ibm,compression"))
return -ENODEV;
@@ -1129,7 +1127,6 @@ static void __exit nx842_pseries_exit(void)
struct nx842_devdata *old_devdata;
unsigned long flags;
- pr_info("Exiting IBM Power 842 compression driver\n");
nx842_platform_driver_unset(&nx842_pseries_driver);
spin_lock_irqsave(&devdata_mutex, flags);
old_devdata = rcu_dereference_check(devdata,
--
2.1.4
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 <[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.
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
On Mon, 2015-07-06 at 10:06 -0700, Nishanth Aravamudan wrote:
> On 03.07.2015 [11:30:32 +1000], Michael Ellerman wrote:
> > On Thu, 2015-07-02 at 15:40 -0700, Nishanth Aravamudan wrote:
> > > While we never would successfully load on the wrong machine type, there
> > > is extra output by default regardless of machine type.
> > >
>
> Thanks Michael!
Thanks for cleaning it up.
> 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
Acked-by: Michael Ellerman <[email protected]>
I'm assuming Herbert will take this.
cheers
On Mon, Jul 06, 2015 at 10:06:21AM -0700, Nishanth Aravamudan wrote:
>
> v2:
> Rather than not loading, just reduce the verbosity
Applied.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On 04.07.2015 [15:24:53 +0800], Herbert Xu wrote:
> On Thu, Jul 02, 2015 at 03:41:19PM -0700, Nishanth Aravamudan wrote:
> > 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.
On 13.07.2015 [17:05:36 -0700], Nishanth Aravamudan wrote:
> On 04.07.2015 [15:24:53 +0800], Herbert Xu wrote:
> > On Thu, Jul 02, 2015 at 03:41:19PM -0700, Nishanth Aravamudan wrote:
> > > 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.
Apologies, hit send too fast. I'll work with Dan on this when he gets
back from vacation.
-Nish
On Mon, Jul 13, 2015 at 8:05 PM, Nishanth Aravamudan
<[email protected]> wrote:
> On 13.07.2015 [17:05:36 -0700], Nishanth Aravamudan wrote:
>> On 04.07.2015 [15:24:53 +0800], Herbert Xu wrote:
>> > On Thu, Jul 02, 2015 at 03:41:19PM -0700, Nishanth Aravamudan wrote:
>> > > 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.
>
> Apologies, hit send too fast. I'll work with Dan on this when he gets
> back from vacation.
Back from vacation! :-)
I originally didn't add any test vector for NX 842 because the main
driver was loading before the "platform" (pseries/powernv) drivers,
and the test couldn't run as the platform driver hadn't loaded yet.
That's now fixed so we should be able to add a real test for NX 842
now, I can work on that patch.
>
> -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
On Mon, Jul 6, 2015 at 1:07 PM, Nishanth Aravamudan
<[email protected]> 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 <[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.
>
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html