[patch 2/3] PS3: Add logical performance monitor device support

[Arnd Bergmann]

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 <><