Re: [PATCH 4/6] crypto/nx-842-{powerpc,pseries}: only load on the appropriate machine type
From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2015-07-03 01:30:33
Also in:
linuxppc-dev
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.
quoted hunk ↗ jump to hunk
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?
quoted hunk ↗ jump to hunk
@@ -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 hunk ↗ jump to hunk
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