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

[Geoff Levand <geoffrey.levand@am.sony.com>]

On 01/05/2008 03:29 AM, Arnd Bergmann wrote:
> 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.

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

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