Yes sounds definitely reasonable :-). I agree,we should try to do away with the strcmp if possible. I will implement the same and repost this fix.
Regards Thara >>-----Original Message----- >>From: Paul Walmsley [mailto:[email protected]] >>Sent: Friday, October 16, 2009 1:51 PM >>To: Gopinath, Thara >>Cc: [email protected]; [email protected] >>Subject: RE: [PATCH V2] OMAP3: PM: Fix for MPU power domain MEM BANK position >> >>Hi Thara, >> >>On Thu, 15 Oct 2009, Gopinath, Thara wrote: >> >>> Thanks Paul. The bit positions are goofed up only in PWRSTS and >>> PREPWRSTS not in PWSTCTRL register. As per the approach you have >>> suggested below, if we change the mem bank number from 0 to 1 , we will >>> have to change the logic for read/write into PWSTCTRL for mpu pwr >>> domain. >> >>Okay. Here's what I'd propose: rather than testing for the >>powerdomain name in the function, or testing for the MPU PRCM module >>offset, let's add a new powerdomain flag into powerdomain.h, >> >>#define PWRDM_OMAP3_MPU_QUIRK (1 << 1) /* MPU bit pos quirk */ >> >>or something similar. Then let's set that for the mpu_pwrdm in >>powerdomains34xx.h and test for that flag in powerdomain.c. >> >>I prefer this since RMK has previously NAK'ed strcmp()s for this sort of >>thing, for good reason, and testing the PRCM internal module offset has >>many of the same problems. >> >>Sound reasonable? >> >> >>- Paul >> >> >>> >>> Regards >>> Thara >>> >>> >>-----Original Message----- >>> >>From: Paul Walmsley [mailto:[email protected]] >>> >>Sent: Thursday, October 15, 2009 4:51 AM >>> >>To: Gopinath, Thara >>> >>Cc: [email protected]; [email protected] >>> >>Subject: Re: [PATCH V2] OMAP3: PM: Fix for MPU power domain MEM BANK >>> >>position >>> >> >>> >>Hi Thara, >>> >> >>> >>I regret the delay. A comment: >>> >> >>> >>On Fri, 28 Aug 2009, Thara Gopinath wrote: >>> >> >>> >>> MPU power domain bank 0 bits are displayed in position of bank 1 >>> >>> in PWRSTS and PREPWRSTS registers. So read them from correct >>> >>> position >>> >> >>> >>Indeed. What do you think about a slightly different approach: changing >>> >>powerdomains34xx.h to be correct? In other words, instead of >>> >> >>> >> .pwrsts_mem_ret = { >>> >> [0] = PWRSTS_OFF_RET, >>> >> }, >>> >> .pwrsts_mem_on = { >>> >> [0] = PWRSTS_OFF_ON, >>> >> }, >>> >> >>> >>we would use: >>> >> >>> >> .pwrsts_mem_ret = { >>> >> [1] = PWRSTS_OFF_RET, >>> >> }, >>> >> .pwrsts_mem_on = { >>> >> [1] = PWRSTS_OFF_ON, >>> >> }, >>> >> >>> >>We have to deal with the bank count field in struct powerdomain - we could >>> >>just convert it into a bitmap representing available banks. So instead >>> >>of: >>> >> >>> >> .banks = 1, >>> >> >>> >>use maybe: >>> >> >>> >> .banks = PWRDM_BANK_1, /* | PWRDM_BANK_0, etc */ >>> >> >>> >> >>> >>Can you foresee any problems with the above approach? >>> >> >>> >>- Paul >>> >> >>> >>> Patch refresh issue. >>> >>> >>> >>> arch/arm/mach-omap2/powerdomain.c | 19 +++++++++++++++++++ >>> >>> 1 files changed, 19 insertions(+), 0 deletions(-) >>> >>> >>> >>> diff --git a/arch/arm/mach-omap2/powerdomain.c >>> >>> b/arch/arm/mach-omap2/powerdomain.c >>> >>> index 2594cbf..6c5fee9 100644 >>> >>> --- a/arch/arm/mach-omap2/powerdomain.c >>> >>> +++ b/arch/arm/mach-omap2/powerdomain.c >>> >>> @@ -971,6 +971,16 @@ int pwrdm_read_mem_pwrst(struct powerdomain >>> >>> *pwrdm, u8 bank) >>> >>> return -EEXIST; >>> >>> >>> >>> /* >>> >>> + * In 3430, for MPU domain bank 0 status bits >>> >>> + * are displayed in the position of bank1 status bits >>> >>> + * in PWST . So the hack. Think of a cleaner >>> >>> + * way of doing this >>> >>> + */ >>> >>> + if (cpu_is_omap34xx()) >>> >>> + if (!strcmp("mpu_pwrdm", pwrdm->name)) >>> >>> + bank = 1; >>> >>> + >>> >>> + /* >>> >>> * The register bit names below may not correspond to the >>> >>> * actual names of the bits in each powerdomain's register, >>> >>> * but the type of value returned is the same for each >>> >>> @@ -1018,6 +1028,15 @@ int pwrdm_read_prev_mem_pwrst(struct powerdomain >>> >>> *pwrdm, u8 bank) >>> >>> return -EEXIST; >>> >>> >>> >>> /* >>> >>> + * In 3430, for MPU domain bank 0 status bits >>> >>> + * are displayed in the position of bank1 status bits >>> >>> + * in PREPWST . So the hack. Think of a cleaner >>> >>> + * way of doing this >>> >>> + */ >>> >>> + if (cpu_is_omap34xx()) >>> >>> + if (!strcmp("mpu_pwrdm", pwrdm->name)) >>> >>> + bank = 1; >>> >>> + /* >>> >>> * The register bit names below may not correspond to the >>> >>> * actual names of the bits in each powerdomain's register, >>> >>> * but the type of value returned is the same for each >>> >>> -- >>> >>> 1.5.4.7 >>> >>> >>> >>> -- >>> >>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in >>> >>> the body of a message to [email protected] >>> >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >>> >>> >> >>> >> >>> >>- Paul >>> >> >> >>- Paul -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
