Re: [PATCH 1/8] OMAP: hwmod: Fix the addr spaces count API.

["Cousson, Benoit" <b-cousson@ti.com>]

On 9/8/2011 11:32 AM, Shilimkar, Santosh wrote:
> On Thursday 08 September 2011 02:45 PM, Sricharan R wrote:
>> Hi Benoit,
>>
>>> -----Original Message-----
>>> From: Sricharan R [mailto:r.sricharan@ti.com]
>>> Sent: Thursday, September 08, 2011 2:35 PM
>>> To: Sricharan R
>>> Subject: Re: [PATCH 1/8] OMAP: hwmod: Fix the addr spaces count API.
>>>
>
> [...]
>
>>>>
>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c
>>> b/arch/arm/mach-omap2/omap_hwmod.c
>>>> index 84cc0bd..32a0f48a 100644
>>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>>> @@ -791,9 +791,11 @@ static int _count_ocp_if_addr_spaces(struct
>>> omap_hwmod_ocp_if *os)
>>>>     	if (!os || !os->addr)
>>>>     		return 0;
>>>>
>>>> -	do {
>>>> -		mem =&os->addr[i++];
>>>> -	} while (mem->pa_start != mem->pa_end);
>>>> +	mem =&os->addr[i];
>>>> +
> This extran line isn't needed Sricharan.
>
>>>> +	while (mem->pa_start != mem->pa_end) {
>>>> +		mem =&os->addr[++i];
>>>> +	};
>>>>
>>>>     	return i;
>>>
>>> Cannot you just do "return i - 1"?
>>
>> Right. That was the first idea.
>> But after some discussion concluded that
>> right way is to, not to take in to account, the
>> null structure itself, rather than incrementing and
>> decrementing.
>>
> I was the one who objected to the i-1 or i--
> change. The count logic itself should
> handle all the scenario's.

But this is the case even with the i-1.
That's a details, but it looks to me nicer to do a return i-1 to 
highlight the extra empty entry that is added, instead of doing twice 
"mem =&os->addr[i];"
The loop is counting the real number of entries, but you return only the 
relevant number by removing the terminator.

BTW, don't we have the same issue with _count_sdma_reqs and _count_mpu_irqs?

Regards,
Benoit