* [PATCH 1/8] OMAP: hwmod: Fix the addr spaces count API.
2011-09-08 5:21 [PATCH 0/8] OMAP3/4: Misc fixes and clean-up Santosh Shilimkar
@ 2011-09-08 5:22 ` Santosh Shilimkar
2011-09-08 7:47 ` Cousson, Benoit
0 siblings, 1 reply; 6+ messages in thread
From: Santosh Shilimkar @ 2011-09-08 5:22 UTC (permalink / raw)
To: linux-omap
Cc: linux-arm-kernel, sricharan, Santosh Shilimkar, Benoit Cousson,
Paul Walmsley, Kevin Hilman
From: sricharan <r.sricharan@ti.com>
The address space count API returns the number of address space
entries for a hwmod including a additional null value present in the
address space structure introduced recently. The devices which
have multiple hwmods and use device_build_ss are broken with this,
as their address resources are populated with a extra null value,
subsequently the probe fails. So fix the API not to add the null value.
Signed-off-by: sricharan <r.sricharan@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@ti.com>
---
arch/arm/mach-omap2/omap_hwmod.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
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];
+
+ while (mem->pa_start != mem->pa_end) {
+ mem = &os->addr[++i];
+ };
return i;
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/8] OMAP: hwmod: Fix the addr spaces count API.
2011-09-08 5:22 ` [PATCH 1/8] OMAP: hwmod: Fix the addr spaces count API Santosh Shilimkar
@ 2011-09-08 7:47 ` Cousson, Benoit
0 siblings, 0 replies; 6+ messages in thread
From: Cousson, Benoit @ 2011-09-08 7:47 UTC (permalink / raw)
To: Shilimkar, Santosh
Cc: linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
R, Sricharan, Paul Walmsley, Hilman, Kevin
Hi Sricharan,
On 9/8/2011 7:22 AM, Shilimkar, Santosh wrote:
> From: sricharan<r.sricharan@ti.com>
>
> The address space count API returns the number of address space
> entries for a hwmod including a additional null value present in the
> address space structure introduced recently.
That's a minor nit, but you might give the commit you are referencing here.
> The devices which
> have multiple hwmods and use device_build_ss are broken with this,
> as their address resources are populated with a extra null value,
> subsequently the probe fails. So fix the API not to add the null value.
It seems that in every cases, we are adding an extra null resource for
nothing. But it is true that will not crash if the driver is just
expecting an unique entry.
What is happening with multiple hwmods is probably the introduction of
that extra null resource in the middle of the real ones, hence shifting
the resource index?
You might give more details here.
> Signed-off-by: sricharan<r.sricharan@ti.com>
> Signed-off-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
> Cc: Benoit Cousson<b-cousson@ti.com>
> Cc: Paul Walmsley<paul@pwsan.com>
> Cc: Kevin Hilman<khilman@ti.com>
> ---
> arch/arm/mach-omap2/omap_hwmod.c | 8 +++++---
> 1 files changed, 5 insertions(+), 3 deletions(-)
>
> 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];
> +
> + while (mem->pa_start != mem->pa_end) {
> + mem =&os->addr[++i];
> + };
>
> return i;
Cannot you just do "return i - 1"?
Regards,
Benoit
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 1/8] OMAP: hwmod: Fix the addr spaces count API.
[not found] <f44b67c9efd46d2e7d078da4d3acd30c@mail.gmail.com>
@ 2011-09-08 9:15 ` Sricharan R
2011-09-08 9:32 ` Santosh
0 siblings, 1 reply; 6+ messages in thread
From: Sricharan R @ 2011-09-08 9:15 UTC (permalink / raw)
To: Benoit Cousson; +Cc: Santosh Shilimkar, linux-omap
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.
>
>Hi Sricharan,
>
>On 9/8/2011 7:22 AM, Shilimkar, Santosh wrote:
>> From: sricharan<r.sricharan@ti.com>
>>
>> The address space count API returns the number of address space
>> entries for a hwmod including a additional null value present in the
>> address space structure introduced recently.
>
>That's a minor nit, but you might give the commit you are referencing
>here.
Ok i will mention the below in detail and resend.
commit 78183f3fdf76f422431a81852468be01b36db325
>
>> The devices which
>> have multiple hwmods and use device_build_ss are broken with this,
>> as their address resources are populated with a extra null value,
>> subsequently the probe fails. So fix the API not to add the null value.
>
>It seems that in every cases, we are adding an extra null resource for
>nothing. But it is true that will not crash if the driver is just
>expecting an unique entry.
>What is happening with multiple hwmods is probably the introduction of
>that extra null resource in the middle of the real ones, hence shifting
>the resource index?
>You might give more details here.
>
That is exactly correct.
So because of the introduction of the null structure in the middle,
the subsequent resource indexes gets shifted by one. So the
original index used by the drivers in those places would return a NULL
resource now, hence the problem.
>
>> Signed-off-by: sricharan<r.sricharan@ti.com>
>> Signed-off-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
>> Cc: Benoit Cousson<b-cousson@ti.com>
>> Cc: Paul Walmsley<paul@pwsan.com>
>> Cc: Kevin Hilman<khilman@ti.com>
>> ---
>> arch/arm/mach-omap2/omap_hwmod.c | 8 +++++---
>> 1 files changed, 5 insertions(+), 3 deletions(-)
>>
>> 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];
>> +
>> + 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.
>
>Regards,
>Benoit
Thanks,
Sricharan
>--
>To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/8] OMAP: hwmod: Fix the addr spaces count API.
2011-09-08 9:15 ` [PATCH 1/8] OMAP: hwmod: Fix the addr spaces count API Sricharan R
@ 2011-09-08 9:32 ` Santosh
2011-09-08 13:03 ` Sricharan R
2011-09-08 15:06 ` Cousson, Benoit
0 siblings, 2 replies; 6+ messages in thread
From: Santosh @ 2011-09-08 9:32 UTC (permalink / raw)
To: Sricharan R; +Cc: Benoit Cousson, linux-omap
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.
Regards
Santosh
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 1/8] OMAP: hwmod: Fix the addr spaces count API.
2011-09-08 9:32 ` Santosh
@ 2011-09-08 13:03 ` Sricharan R
2011-09-08 15:06 ` Cousson, Benoit
1 sibling, 0 replies; 6+ messages in thread
From: Sricharan R @ 2011-09-08 13:03 UTC (permalink / raw)
To: Santosh Shilimkar; +Cc: Benoit Cousson, linux-omap
[...]
>>> -----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.
>
Will correct this, along with benoit's commmets.
Thanks,
Sricharan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/8] OMAP: hwmod: Fix the addr spaces count API.
2011-09-08 9:32 ` Santosh
2011-09-08 13:03 ` Sricharan R
@ 2011-09-08 15:06 ` Cousson, Benoit
1 sibling, 0 replies; 6+ messages in thread
From: Cousson, Benoit @ 2011-09-08 15:06 UTC (permalink / raw)
To: Shilimkar, Santosh
Cc: R, Sricharan, linux-omap@vger.kernel.org, paul Walmsley
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-09-08 15:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <f44b67c9efd46d2e7d078da4d3acd30c@mail.gmail.com>
2011-09-08 9:15 ` [PATCH 1/8] OMAP: hwmod: Fix the addr spaces count API Sricharan R
2011-09-08 9:32 ` Santosh
2011-09-08 13:03 ` Sricharan R
2011-09-08 15:06 ` Cousson, Benoit
2011-09-08 5:21 [PATCH 0/8] OMAP3/4: Misc fixes and clean-up Santosh Shilimkar
2011-09-08 5:22 ` [PATCH 1/8] OMAP: hwmod: Fix the addr spaces count API Santosh Shilimkar
2011-09-08 7:47 ` Cousson, Benoit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox