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

On 01/05/2008 03:29 AM, Arnd Bergmann wrote:

On Saturday 05 January 2008, Geoff Levand wrote:

quoted

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

Yes, this code was just cut and pasted from a device that
had more members.

quoted

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

I'll fix it in the next post.

-Geoff