Message ID | 1315459327-3285-1-git-send-email-santosh.shilimkar@ti.com |
---|---|
State | New |
Headers | show |
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
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > Fix the address overlap with Emulation domain (EMU). > > The previous mapping was entering into EMU mapping > and was not as per comments. Fix the mapping accordingly. > > [girishsg@ti.com: Helped fixing comments.] > Signed-off-by: Girish S G <girishsg@ti.com> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > --- > arch/arm/plat-omap/include/plat/io.h | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/plat-omap/include/plat/io.h b/arch/arm/plat-omap/include/plat/io.h > index d72ec85..a2f7d31 100644 > --- a/arch/arm/plat-omap/include/plat/io.h > +++ b/arch/arm/plat-omap/include/plat/io.h > @@ -228,12 +228,12 @@ > > #define OMAP44XX_EMIF2_PHYS OMAP44XX_EMIF2_BASE > /* 0x4d000000 --> 0xfd200000 */ > -#define OMAP44XX_EMIF2_VIRT (OMAP44XX_EMIF2_PHYS + OMAP4_L3_PER_IO_OFFSET) > +#define OMAP44XX_EMIF2_VIRT (OMAP44XX_EMIF1_VIRT + SZ_1M) IMO, this would be much clearer (and future proof) if you used '+ OMAP44XX_EMIF1_SIZE' instead of SZ_1M. > #define OMAP44XX_EMIF2_SIZE SZ_1M > > #define OMAP44XX_DMM_PHYS OMAP44XX_DMM_BASE > /* 0x4e000000 --> 0xfd300000 */ > -#define OMAP44XX_DMM_VIRT (OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET) > +#define OMAP44XX_DMM_VIRT (OMAP44XX_EMIF2_VIRT + SZ_1M) and '+ OMAP44XX_EMIF2_SIZE' here. > #define OMAP44XX_DMM_SIZE SZ_1M > /* > * ---------------------------------------------------------------------------- Kevin
On Friday 16 September 2011 11:26 PM, Kevin Hilman wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> Fix the address overlap with Emulation domain (EMU). >> >> The previous mapping was entering into EMU mapping >> and was not as per comments. Fix the mapping accordingly. >> >> [girishsg@ti.com: Helped fixing comments.] >> Signed-off-by: Girish S G <girishsg@ti.com> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >> --- >> arch/arm/plat-omap/include/plat/io.h | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/plat-omap/include/plat/io.h b/arch/arm/plat-omap/include/plat/io.h >> index d72ec85..a2f7d31 100644 >> --- a/arch/arm/plat-omap/include/plat/io.h >> +++ b/arch/arm/plat-omap/include/plat/io.h >> @@ -228,12 +228,12 @@ >> >> #define OMAP44XX_EMIF2_PHYS OMAP44XX_EMIF2_BASE >> /* 0x4d000000 --> 0xfd200000 */ >> -#define OMAP44XX_EMIF2_VIRT (OMAP44XX_EMIF2_PHYS + OMAP4_L3_PER_IO_OFFSET) >> +#define OMAP44XX_EMIF2_VIRT (OMAP44XX_EMIF1_VIRT + SZ_1M) > > IMO, this would be much clearer (and future proof) if you used > '+ OMAP44XX_EMIF1_SIZE' instead of SZ_1M. > ok. >> #define OMAP44XX_EMIF2_SIZE SZ_1M >> >> #define OMAP44XX_DMM_PHYS OMAP44XX_DMM_BASE >> /* 0x4e000000 --> 0xfd300000 */ >> -#define OMAP44XX_DMM_VIRT (OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET) >> +#define OMAP44XX_DMM_VIRT (OMAP44XX_EMIF2_VIRT + SZ_1M) > > and '+ OMAP44XX_EMIF2_SIZE' here. > Will add OMAP44XX_EMIF_SIZE since 2 EMIFs instaces are and suppose to be identical.Almost missed this email in other traffic. Regards Santosh
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > On Tuesday 20 September 2011 08:31 PM, Santosh Shilimkar wrote: >> On Friday 16 September 2011 11:26 PM, Kevin Hilman wrote: >>> Santosh Shilimkar <santosh.shilimkar@ti.com> writes: >>> > > [...] > >> >>>> #define OMAP44XX_EMIF2_SIZE SZ_1M >>>> >>>> #define OMAP44XX_DMM_PHYS OMAP44XX_DMM_BASE >>>> /* 0x4e000000 --> 0xfd300000 */ >>>> -#define OMAP44XX_DMM_VIRT (OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET) >>>> +#define OMAP44XX_DMM_VIRT (OMAP44XX_EMIF2_VIRT + SZ_1M) >>> >>> and '+ OMAP44XX_EMIF2_SIZE' here. >>> >> Will add OMAP44XX_EMIF_SIZE since 2 EMIFs instaces are and >> suppose to be identical.Almost missed this email in other traffic. > > OPPs. It was already there. Dumb of me not using it. Below > is the update what I will do. > > diff --git a/arch/arm/plat-omap/include/plat/io.h > b/arch/arm/plat-omap/include/plat/io.h > index d72ec85..db36292 100644 > --- a/arch/arm/plat-omap/include/plat/io.h > +++ b/arch/arm/plat-omap/include/plat/io.h > @@ -228,13 +228,13 @@ > > #define OMAP44XX_EMIF2_PHYS OMAP44XX_EMIF2_BASE > /* 0x4d000000 --> 0xfd200000 */ > -#define OMAP44XX_EMIF2_VIRT (OMAP44XX_EMIF2_PHYS + OMAP4_L3_PER_IO_OFFSET) > #define OMAP44XX_EMIF2_SIZE SZ_1M > +#define OMAP44XX_EMIF2_VIRT (OMAP44XX_EMIF1_VIRT + OMAP44XX_EMIF2_SIZE) This doesn't look right either. This #define is for the *start* of EMIF2 virtual address space, so assuming the start address of EMIF2 is immediatly after EMIF1, it should be EMIF1_VIRT + EMIF1_SIZE (not EMIF2_SIZE.) > #define OMAP44XX_DMM_PHYS OMAP44XX_DMM_BASE > /* 0x4e000000 --> 0xfd300000 */ > -#define OMAP44XX_DMM_VIRT (OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET) > #define OMAP44XX_DMM_SIZE SZ_1M > +#define OMAP44XX_DMM_VIRT (OMAP44XX_EMIF2_VIRT + OMAP44XX_DMM_SIZE) And here, assuming DMM range is immediately after EMIF2, this should be EMIF2_VIRT + EMFI2_SIZE) Kevin
On Wed, Sep 21, 2011 at 11:01 PM, Kevin Hilman <khilman@ti.com> wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> On Tuesday 20 September 2011 08:31 PM, Santosh Shilimkar wrote: >>> On Friday 16 September 2011 11:26 PM, Kevin Hilman wrote: >>>> Santosh Shilimkar <santosh.shilimkar@ti.com> writes: >>>> >> >> [...] >> >>> >>>>> #define OMAP44XX_EMIF2_SIZE SZ_1M >>>>> >>>>> #define OMAP44XX_DMM_PHYS OMAP44XX_DMM_BASE >>>>> /* 0x4e000000 --> 0xfd300000 */ >>>>> -#define OMAP44XX_DMM_VIRT (OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET) >>>>> +#define OMAP44XX_DMM_VIRT (OMAP44XX_EMIF2_VIRT + SZ_1M) >>>> >>>> and '+ OMAP44XX_EMIF2_SIZE' here. >>>> >>> Will add OMAP44XX_EMIF_SIZE since 2 EMIFs instaces are and >>> suppose to be identical.Almost missed this email in other traffic. >> >> OPPs. It was already there. Dumb of me not using it. Below >> is the update what I will do. >> >> diff --git a/arch/arm/plat-omap/include/plat/io.h >> b/arch/arm/plat-omap/include/plat/io.h >> index d72ec85..db36292 100644 >> --- a/arch/arm/plat-omap/include/plat/io.h >> +++ b/arch/arm/plat-omap/include/plat/io.h >> @@ -228,13 +228,13 @@ >> >> #define OMAP44XX_EMIF2_PHYS OMAP44XX_EMIF2_BASE >> /* 0x4d000000 --> 0xfd200000 */ >> -#define OMAP44XX_EMIF2_VIRT (OMAP44XX_EMIF2_PHYS + OMAP4_L3_PER_IO_OFFSET) >> #define OMAP44XX_EMIF2_SIZE SZ_1M >> +#define OMAP44XX_EMIF2_VIRT (OMAP44XX_EMIF1_VIRT + OMAP44XX_EMIF2_SIZE) > > This doesn't look right either. > > This #define is for the *start* of EMIF2 virtual address space, so > assuming the start address of EMIF2 is immediatly after EMIF1, it should > be EMIF1_VIRT + EMIF1_SIZE (not EMIF2_SIZE.) > >> #define OMAP44XX_DMM_PHYS OMAP44XX_DMM_BASE >> /* 0x4e000000 --> 0xfd300000 */ >> -#define OMAP44XX_DMM_VIRT (OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET) >> #define OMAP44XX_DMM_SIZE SZ_1M >> +#define OMAP44XX_DMM_VIRT (OMAP44XX_EMIF2_VIRT + OMAP44XX_DMM_SIZE) > > And here, assuming DMM range is immediately after EMIF2, this should be > EMIF2_VIRT + EMFI2_SIZE) > I agree. Regards Santosh
Paul, On Thursday 08 September 2011 10:51 AM, Santosh Shilimkar wrote: > 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 > I need your ack on below two patches so that I can re-base them and send a pull request to Tony. OMAP4: clock: Add CPU local timer clock node. OMAP: Fix sparse warnings in l3 error handler. Regards Santosh
Hi Santosh On Sat, 24 Sep 2011, Santosh Shilimkar wrote: > On Thursday 08 September 2011 10:51 AM, Santosh Shilimkar wrote: > > 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 > > > I need your ack on below two patches so that I can re-base them and > send a pull request to Tony. > > OMAP4: clock: Add CPU local timer clock node. > OMAP: Fix sparse warnings in l3 error handler. Are you targeting these for 3.1-rc ? - Paul
On Saturday 24 September 2011 12:01 PM, Paul Walmsley wrote: > Hi Santosh > > On Sat, 24 Sep 2011, Santosh Shilimkar wrote: > >> On Thursday 08 September 2011 10:51 AM, Santosh Shilimkar wrote: >>> 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 >>> >> I need your ack on below two patches so that I can re-base them and >> send a pull request to Tony. >> >> OMAP4: clock: Add CPU local timer clock node. >> OMAP: Fix sparse warnings in l3 error handler. > > Are you targeting these for 3.1-rc ? > Nope. It's for 3.2 merge window. As such, the series doesn't have any major regression fixes which should make it before 3.2 Regards Santosh
Hi On Sat, 24 Sep 2011, Santosh Shilimkar wrote: > On Saturday 24 September 2011 12:01 PM, Paul Walmsley wrote: > > Hi Santosh > > > > On Sat, 24 Sep 2011, Santosh Shilimkar wrote: > > > >> On Thursday 08 September 2011 10:51 AM, Santosh Shilimkar wrote: > >>> > >> I need your ack on below two patches so that I can re-base them and > >> send a pull request to Tony. > >> > >> OMAP4: clock: Add CPU local timer clock node. > >> OMAP: Fix sparse warnings in l3 error handler. > > > > Are you targeting these for 3.1-rc ? > > > Nope. It's for 3.2 merge window. > As such, the series doesn't have any major regression > fixes which should make it before 3.2 In that case, patches 1 and 7 should go in through me. The rest should go via Tony. So you can just drop 1 and 7 from your branch, I'll pull those off the list. Some comments though: Looks like you need to repost patch 1 due to comments from Benoît? Also, have the autogeneration scripts been updated for patch 7? ... As far as patch 5 goes, you don't need my ack on that one, since I'm not the maintainer or original author of that code. But feel free to add a Reviewed-by:. I also tested the two L3 files with sparse and they are clean now. Thanks very much for taking the initiative on this; I personally appreciate it quite a bit, and I think having clean code increases our credibility with upstream maintainers. - Paul
On Saturday 24 September 2011 01:06 PM, Paul Walmsley wrote: > Hi > > On Sat, 24 Sep 2011, Santosh Shilimkar wrote: > >> On Saturday 24 September 2011 12:01 PM, Paul Walmsley wrote: >>> Hi Santosh >>> >>> On Sat, 24 Sep 2011, Santosh Shilimkar wrote: >>> >>>> On Thursday 08 September 2011 10:51 AM, Santosh Shilimkar wrote: >>>>> >>>> I need your ack on below two patches so that I can re-base them and >>>> send a pull request to Tony. >>>> >>>> OMAP4: clock: Add CPU local timer clock node. >>>> OMAP: Fix sparse warnings in l3 error handler. >>> >>> Are you targeting these for 3.1-rc ? >>> >> Nope. It's for 3.2 merge window. >> As such, the series doesn't have any major regression >> fixes which should make it before 3.2 > > In that case, patches 1 and 7 should go in through me. The rest should go > via Tony. So you can just drop 1 and 7 from your branch, I'll pull those > off the list. > Will do. > Some comments though: Looks like you need to repost patch 1 due to > comments from Benoît? Also, have the autogeneration scripts been updated > for patch 7? > Ok. Will post updated patch1. For the patch 7, I will align with Benoit on the autogen script. It has some differences with mainline OMAP4 clock data file. > ... > > As far as patch 5 goes, you don't need my ack on that one, since I'm not > the maintainer or original author of that code. But feel free to add a > Reviewed-by:. I also tested the two L3 files with sparse and they are > clean now. Thanks very much for taking the initiative on this; I > personally appreciate it quite a bit, and I think having clean code > increases our credibility with upstream maintainers. > Actually I wanted you to have a look to ensure that we addressed the all sparce warnings. Will add your reviewed by on that patch. Couldn't agree more on the clean code remark. Regards Santosh
2011/9/8 Santosh Shilimkar <santosh.shilimkar@ti.com>: > Local timer clock is sourced from the CPU clock and hence changes > along with CPU clock. These per CPU local timers are used as > clock-events, so they need to be reconfigured on CPU frequency > change as part of CPUfreq governor. This requires patch 6956/2 right? http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=6956/2 Can I add your Tested-by: to the patch as well? Yours, Linus Walleij
On Fri, Sep 30, 2011 at 12:42 AM, Paul Walmsley <paul@pwsan.com> wrote: > Hi Santosh, > > On Thu, 8 Sep 2011, Santosh Shilimkar wrote: > >> Local timer clock is sourced from the CPU clock and hence changes >> along with CPU clock. These per CPU local timers are used as >> clock-events, so they need to be reconfigured on CPU frequency >> change as part of CPUfreq governor. >> >> Newly introduced clockevents_reconfigure() needs to know the >> twd clock-rate. Provide a clock-node to make clk_get_rate() work >> for TWD. >> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >> Cc: Paul Walmsley <paul@pwsan.com> >> Cc: Kevin Hilman <khilman@ti.com> > > This clock looks like the ARM Cortex-A9 MPCore PERIPHCLK, correct? From > ARM Cortex-A9 MPCore Technical Reference Manual (DDI 0407G) Section 5.1 > "Clocks"? > Yes. Peripheral clock/2 > Assuming that is so, I've modified this patch appropriately, and added > support to the autogeneration script to generate this clock node. > Care to take a quick look at it to make sure it's okay? > Thanks a lot Paul. It looks good. Regards Santosh
On Fri, Sep 30, 2011 at 3:00 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > 2011/9/8 Santosh Shilimkar <santosh.shilimkar@ti.com>: > >> Local timer clock is sourced from the CPU clock and hence changes >> along with CPU clock. These per CPU local timers are used as >> clock-events, so they need to be reconfigured on CPU frequency >> change as part of CPUfreq governor. > > This requires patch 6956/2 right? > http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=6956/2 > > Can I add your Tested-by: to the patch as well? > Sure.
On Fri, Dec 9, 2011 at 4:32 AM, Turquette, Mike <mturquette@ti.com> wrote: > On Fri, Sep 30, 2011 at 2:15 AM, Shilimkar, Santosh > <santosh.shilimkar@ti.com> wrote: >> On Fri, Sep 30, 2011 at 3:00 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >>> 2011/9/8 Santosh Shilimkar <santosh.shilimkar@ti.com>: >>> >>>> Local timer clock is sourced from the CPU clock and hence changes >>>> along with CPU clock. These per CPU local timers are used as >>>> clock-events, so they need to be reconfigured on CPU frequency >>>> change as part of CPUfreq governor. >>> >>> This requires patch 6956/2 right? >>> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=6956/2 >>> >>> Can I add your Tested-by: to the patch as well? >>> >> Sure. > > Santosh, > > I've taken in Linus' patch as well as this patch while testing clk > notifiers (part of common clk framework code). > > Linus' patch matches the clk based on dev_id, but the clkdev entry in > your patch populated dev_id with NULL and con_id with "smp_twd" so > things never worked correctly. Below changed fixed it for me: > You are right. When I picked up the patch while changing the OMAP44XX stuff to OMAP4430, looks like I miss-placed it. > diff --git a/arch/arm/mach-omap2/clock44xx_data.c > b/arch/arm/mach-omap2/clock44xx_data.c > index ae8c292..0f8cf68 100644 > --- a/arch/arm/mach-omap2/clock44xx_data.c > +++ b/arch/arm/mach-omap2/clock44xx_data.c > @@ -4114,7 +4114,7 @@ static struct omap_clk omap44xx_clks[] = { > CLK(NULL, "l4_div_ck", > &l4_div_ck_hw.clk, CK_443X), > CLK(NULL, "lp_clk_div_ck", > &lp_clk_div_ck_hw.clk, CK_443X), > CLK(NULL, "l4_wkup_clk_mux_ck", > &l4_wkup_clk_mux_ck_hw.clk, CK_443X), > - CLK(NULL, "smp_twd", > &mpu_periphclk_hw.clk, CK_443X), > + CLK("smp_twd", NULL, &mpu_periphclk_hw.clk, > CK_443X), > CLK(NULL, "ocp_abe_iclk", > &ocp_abe_iclk_hw.clk, CK_443X), > CLK(NULL, "per_abe_24m_fclk", > &per_abe_24m_fclk_hw.clk, CK_443X), > CLK(NULL, "per_abe_nc_fclk", > &per_abe_nc_fclk_hw.clk, CK_443X), > > A better change might be for the code in smp_twd.c to match on con_id > but I'll let you guys sort that out. I guess we can stick to what Linus and others have followed. Will post an updated patch. Thanks for catch. Regards Santosh