* 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
* [PATCH 0/8] OMAP3/4: Misc fixes and clean-up
@ 2011-09-08 5:21 Santosh Shilimkar
2011-09-08 5:22 ` [PATCH 1/8] OMAP: hwmod: Fix the addr spaces count API Santosh Shilimkar
0 siblings, 1 reply; 6+ messages in thread
From: Santosh Shilimkar @ 2011-09-08 5:21 UTC (permalink / raw)
To: linux-omap; +Cc: linux-arm-kernel, Santosh Shilimkar
The series contains few fixes and clean-up for OMAP.
Briefly,
- HWMOD fix for the address space count
- Improving the L3 register accesses
- Bug fix in the L3 error handler
- Sparce warning and indentation fixes in L3 error handler
- Print master id in case of L3 custom errors for better debug.
- Adding local time clock node for the CPUfreq and time re-calibration
- Fix in the address overlap for emif and emulation domain.
The series is tested on OMAP4430 SDP and OMAP4430 beagle and SDP.
The following changes since commit c6a389f123b9f68d605bb7e0f9b32ec1e3e14132:
Linux 3.1-rc4 (2011-08-28 21:16:01 -0700)
are available in the git repository at:
git://gitorious.org/omap-sw-develoment/linux-omap-dev.git v3.1-rc4-omap-misc
Santosh Shilimkar (2):
OMAP4: clock: Add CPU local timer clock node.
OMAP4: Fix the emif and dmm virtual mapping
Todd Poynor (2):
OMAP: Improve register access in L3 Error handler.
OMAP: Fix a BUG in l3 error handler.
sricharan (4):
OMAP: hwmod: Fix the addr spaces count API.
OMAP: Fix indentation issues in l3 error handler.
OMAP: Fix sparse warnings in l3 error handler.
OMAP: Print Initiator name for l3 custom error.
arch/arm/mach-omap2/clock44xx_data.c | 9 ++
arch/arm/mach-omap2/omap_hwmod.c | 8 +-
arch/arm/mach-omap2/omap_l3_noc.c | 130 ++++++++++----------
arch/arm/mach-omap2/omap_l3_noc.h | 224 +++++++++++++++++++---------------
arch/arm/mach-omap2/omap_l3_smx.c | 91 +++++++-------
arch/arm/mach-omap2/omap_l3_smx.h | 164 ++++++++++++------------
arch/arm/plat-omap/include/plat/io.h | 4 +-
7 files changed, 336 insertions(+), 294 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread* [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
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