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 <nacc@xxxxxxxxxxxxxxxxxx>
Cc: Dan Streetman <ddstreet@xxxxxxxxxx>
Cc: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>
Cc: linux-crypto@xxxxxxxxxxxxxxx
Cc: linuxppc-dev@xxxxxxxxxxxxxxxx
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
