On Saturday 05 January 2008, Geoff Levand wrote:
> + struct layout {
> + struct ps3_system_bus_device dev;
> + } *p;
What's the point of this data structure? You don't use the
struct anywhere, and it only has one member, so you could
just declare that directly.
> + if (tmp1 != tmp2) {
> + pr_debug("%s:%d: wrong lpar\n",
> + __func__, __LINE__);
> + result = -1;
> + goto fail_rights;
> + }
> +
> + if (!(p->dev.lpm.rights & PS3_LPM_RIGHTS_USE_LPM)) {
> + pr_debug("%s:%d: don't have rights to use lpm\n",
> + __func__, __LINE__);
> + result = -1;
> + goto fail_rights;
> + }
> +
I think __init functions should return error codes like -EPERM or
-EINVAL, not numeric -1.
Apart from that, the patch looks good.
Arnd <><
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
