Re: [PATCH v5 10/14] ARM: OMAP2+: gpmc: waitpin helper

[Jon Hunter <jon-hunter@ti.com>]

On 06/12/2012 04:00 AM, Mohammed, Afzal wrote:
> Hi Jon,
> 
> On Tue, Jun 12, 2012 at 04:29:09, Hunter, Jon wrote:
> 
>>> +enum {
>>> +	GPMC_WAITPIN_IDX0,
>>> +	GPMC_WAITPIN_IDX1,
>>> +	GPMC_WAITPIN_IDX2,
>>> +	GPMC_WAITPIN_IDX3,
>>> +	GPMC_NR_WAITPIN
>>> +};
>>
>> Max number of wait pins is 3 for omap4/5. I know that we discussed this
>> in the past, but are you not supporting these devices are the moment? I
>> know that you have not done the hwmod for these, but still I was hoping
>> that you would take these into account.
> 
> I don't see reason for omap4/5 not to work with this

Well, GPMC_NR_WAITPIN will be 4 and this is not accurate for OMAP4/5.

>>
>>> +
>>> +enum {
>>> +	LOW,
>>> +	HIGH
>>> +};
>>
>> To be honest, I don't see the point in either of the above enums when
>> you have the definitions at the bottom. Seems that one set of
>> definitions should be enough.
> 
> Thanks for catching that, it will be removed
> 
>>> +	case GPMC_WAITPIN_0:
>>> +		idx =  GPMC_WAITPIN_IDX0;
>>
>> For example, here you could have ...
>>
>> 		idx = GPMC_WAITPIN_0 - 1;
> 
> Sorry, not sure whether I follow you, can you please clarify


GPMC_WAITPIN_IDX0 = 0
GPMC_WAITPIN_0 = 1

So, GPMC_WAITPIN_IDX0 = GPMC_WAITPIN_0 - 1, assuming that you want idx =
0 and not 1. Or you could change you shift value too. I was just
highlighting that they are not equal but one set of definitions could be
used.

Jon