[PATCH v5 05/14] ARM: OMAP2+: gpmc: resource creation helpers

[jon-hunter@ti.com (Jon Hunter)]

On 06/12/2012 03:30 AM, Mohammed, Afzal wrote:
> Hi Jon,
> 
> On Tue, Jun 12, 2012 at 02:27:09, Hunter, Jon wrote:
> 
>>> +static __devinit int gpmc_setup_cs_mem(struct gpmc_cs_data *cs,
>>> +						struct resource *res)
> 
>>> +	return 1;
>>> +}
>>
>> Nit-pick, CodingStyle chapter 16 states that 0 should be used for
>> success when returning from a function.
> 
> I doubt whether this falls to the category you are referring.
> This function returns number of resources setup, please see
> similar for irq too, there it can be 2 also, success or failure
> is not sufficient for the other, and with this we have both
> functions returning values similar way.

Well looking at the function it seems that you either return an error
code or 1. So if you are never going to return anything other than 1 on
success it may as well be 0.

>>> +static inline unsigned gpmc_bit_to_irq(unsigned bitmask)
>>> +{
>>> +	return bitmask;
>>> +}
>>
>> I have to ask what is the value in this helper function? ;-)
> 
> It is to stress the fact that flag user is passing need not be
> same as irq bit (here both have same value), this kind of an
> arrangement would be required later if waitpin interrupt also
> has to be handled, do you still prefer removing it now ?

I don't have strong feelings, but if you are not planning to add support
for the wait pin irq anytime soon I would leave it out.

Jon