Re: [RFC][PATCH 2/2] OMAP4: PRCM: Fix usage of prm/cm accessor api's for OMAP4
* Cousson, Benoit [101015 08:58]: > Hi Paul, > > On 10/14/2010 8:44 PM, Paul Walmsley wrote: > >Hello Rajendra, > > > >On Tue, 10 Aug 2010, Rajendra Nayak wrote: > > > >>OMAP's have always had PRCM split into PRM for power and reset > >>management and CM for clock management. > >>In OMAP4 the split (physically) is not very straight forward and > >>there are instances of clock management control registers in PRM > >>and vice versa. > >>However it still makes sense, even on OMAP4 to logically split > >>PRCM into PRM and CM for better understanding and to avoid adding > >>additonal complexity in higher level frameworks which rely on the > >>accessor api;s to do the low level register accesses. > >> > >>Hence this patch makes sure that any clock management code can > >>use the cm_read/write* accessor apis (without knowing the physical split) > >>and power and reset management code can use prm_read/write* > >>accessor api;s. > >> > >>To do this the submodule offsets within PRM/CM1 and CM2 have additonal > >>info embedded in them specifying what base address to use while > >>trying to access registers in the given submodule. > >> > >>The 16 bit signed submodule offset is defined for OMAP4 as > >>|| > >> | | > > > >The concern that I have with embedding multiple parameters into a single > >parameter is that it seems like a hack. Why not add an extra parameter > >for the base identifier, rather than packing it into an existing > >parameter? > > The primary constraint that lead us to that proposal was to minimize > the impact on the existing code and API for previous OMAPs. > That was clearly the goal #1. > The other one was the relative simplicity of the implementation. > > The user of these OMAP4430__MOD macros does not have to care if > this is a real offset or just an id. > > >I am not necessarily opposed to your patch as it exists. But I would like > >to hear your opinions first on separating out the base identifier > >parameter as a separate function parameter, and then adding an extra field > >for it into any data structure that would need it. Could you write > >briefly if you see any significant advantages/disadvantages to that > >approach? > > The disadvantage is the relative complexity for the caller of this > API, that will have to know what partition should be used. > It is fine if the caller is the powerdomain or clockdomain fmwk, but > what about all the other callers we have here and there? > When we looked at that in Bangalore, we realized that a bunch of > code is using these prm/cm APIs. Maybe some cleanup can be done, > like in the save / restore part, but we still have some user of > that. > > For the moment, I do not really see any advantage of such approach. > The partitioning is changing on every OMAPs, so we do have to > abstract that. If it is not done at that level, we will have to > define another API on top of that to do exactly the same job. I think we should be able to deal with the partitions properly by ioremapping them separately as needed. Might as well do it now. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH 2/2] OMAP4: PRCM: Fix usage of prm/cm accessor api's for OMAP4
Hi Paul, On 10/14/2010 8:44 PM, Paul Walmsley wrote: Hello Rajendra, On Tue, 10 Aug 2010, Rajendra Nayak wrote: OMAP's have always had PRCM split into PRM for power and reset management and CM for clock management. In OMAP4 the split (physically) is not very straight forward and there are instances of clock management control registers in PRM and vice versa. However it still makes sense, even on OMAP4 to logically split PRCM into PRM and CM for better understanding and to avoid adding additonal complexity in higher level frameworks which rely on the accessor api;s to do the low level register accesses. Hence this patch makes sure that any clock management code can use the cm_read/write* accessor apis (without knowing the physical split) and power and reset management code can use prm_read/write* accessor api;s. To do this the submodule offsets within PRM/CM1 and CM2 have additonal info embedded in them specifying what base address to use while trying to access registers in the given submodule. The 16 bit signed submodule offset is defined for OMAP4 as | | || The concern that I have with embedding multiple parameters into a single parameter is that it seems like a hack. Why not add an extra parameter for the base identifier, rather than packing it into an existing parameter? The primary constraint that lead us to that proposal was to minimize the impact on the existing code and API for previous OMAPs. That was clearly the goal #1. The other one was the relative simplicity of the implementation. The user of these OMAP4430__MOD macros does not have to care if this is a real offset or just an id. I am not necessarily opposed to your patch as it exists. But I would like to hear your opinions first on separating out the base identifier parameter as a separate function parameter, and then adding an extra field for it into any data structure that would need it. Could you write briefly if you see any significant advantages/disadvantages to that approach? The disadvantage is the relative complexity for the caller of this API, that will have to know what partition should be used. It is fine if the caller is the powerdomain or clockdomain fmwk, but what about all the other callers we have here and there? When we looked at that in Bangalore, we realized that a bunch of code is using these prm/cm APIs. Maybe some cleanup can be done, like in the save / restore part, but we still have some user of that. For the moment, I do not really see any advantage of such approach. The partitioning is changing on every OMAPs, so we do have to abstract that. If it is not done at that level, we will have to define another API on top of that to do exactly the same job. Regards, Benoit For older OMAP's the base identifier is always set to 0. Signed-off-by: Rajendra Nayak Cc: Kevin Hilman Cc: Paul Walmsley Cc: Benoit Cousson --- arch/arm/mach-omap2/cm.h |4 +- arch/arm/mach-omap2/prcm-common.h | 58 --- arch/arm/mach-omap2/prcm.c| 68 ++-- arch/arm/mach-omap2/prm.h |4 +- 4 files changed, 105 insertions(+), 29 deletions(-) diff --git a/arch/arm/mach-omap2/cm.h b/arch/arm/mach-omap2/cm.h index a02ca30..7b7ef09 100644 --- a/arch/arm/mach-omap2/cm.h +++ b/arch/arm/mach-omap2/cm.h @@ -23,9 +23,9 @@ #define OMAP34XX_CM_REGADDR(module, reg) \ OMAP2_L4_IO_ADDRESS(OMAP3430_CM_BASE + (module) + (reg)) #define OMAP44XX_CM1_REGADDR(module, reg) \ - OMAP2_L4_IO_ADDRESS(OMAP4430_CM1_BASE + (module) + (reg)) + OMAP2_L4_IO_ADDRESS(OMAP4430_CM1_BASE + ((module)& (MOD_MASK)) + (reg)) #define OMAP44XX_CM2_REGADDR(module, reg) \ - OMAP2_L4_IO_ADDRESS(OMAP4430_CM2_BASE + (module) + (reg)) + OMAP2_L4_IO_ADDRESS(OMAP4430_CM2_BASE + ((module)& (MOD_MASK)) + (reg)) #include "cm44xx.h" diff --git a/arch/arm/mach-omap2/prcm-common.h b/arch/arm/mach-omap2/prcm-common.h index 995b7ed..b93d33e 100644 --- a/arch/arm/mach-omap2/prcm-common.h +++ b/arch/arm/mach-omap2/prcm-common.h @@ -57,10 +57,26 @@ #define BITFIELD(l_bit, u_bit)\ (BITS(u_bit)& ~((BITS(l_bit))>> 1)) -/* OMAP44XX specific module offsets */ +/* + * OMAP44XX specific module offsets + * The 16 bit submodule offset is defined for OMAP4 as + * | | + * || + */ -/* CM1 instances */ +#define DEFAULT_BASE 0x0 +#define PRM_BASE 0x1 +#define PRCM_MPU_BASE 0x2 +#define CM2_BASE 0x3 +#define BASE_ID_SHIFT 13 +#define MOD_MASK ((1<< BASE_ID_SHIFT)-1) + +#define PRM_BASE_ID(PRM_BASE<< BASE_ID_SHIFT) +#define PRCM_MPU_BASE_ID (PRCM_MPU_BASE<< BASE_ID_SHIFT) +#define CM2_BASE_ID(CM2_BASE<< BASE_ID_SHIFT) + +/* CM1 instances
Re: [RFC][PATCH 2/2] OMAP4: PRCM: Fix usage of prm/cm accessor api's for OMAP4
Hello Rajendra, On Tue, 10 Aug 2010, Rajendra Nayak wrote: > OMAP's have always had PRCM split into PRM for power and reset > management and CM for clock management. > In OMAP4 the split (physically) is not very straight forward and > there are instances of clock management control registers in PRM > and vice versa. > However it still makes sense, even on OMAP4 to logically split > PRCM into PRM and CM for better understanding and to avoid adding > additonal complexity in higher level frameworks which rely on the > accessor api;s to do the low level register accesses. > > Hence this patch makes sure that any clock management code can > use the cm_read/write* accessor apis (without knowing the physical split) > and power and reset management code can use prm_read/write* > accessor api;s. > > To do this the submodule offsets within PRM/CM1 and CM2 have additonal > info embedded in them specifying what base address to use while > trying to access registers in the given submodule. > > The 16 bit signed submodule offset is defined for OMAP4 as > || > | | The concern that I have with embedding multiple parameters into a single parameter is that it seems like a hack. Why not add an extra parameter for the base identifier, rather than packing it into an existing parameter? I am not necessarily opposed to your patch as it exists. But I would like to hear your opinions first on separating out the base identifier parameter as a separate function parameter, and then adding an extra field for it into any data structure that would need it. Could you write briefly if you see any significant advantages/disadvantages to that approach? > For older OMAP's the base identifier is always set to 0. > > Signed-off-by: Rajendra Nayak > Cc: Kevin Hilman > Cc: Paul Walmsley > Cc: Benoit Cousson > --- > arch/arm/mach-omap2/cm.h |4 +- > arch/arm/mach-omap2/prcm-common.h | 58 --- > arch/arm/mach-omap2/prcm.c| 68 ++-- > arch/arm/mach-omap2/prm.h |4 +- > 4 files changed, 105 insertions(+), 29 deletions(-) > > diff --git a/arch/arm/mach-omap2/cm.h b/arch/arm/mach-omap2/cm.h > index a02ca30..7b7ef09 100644 > --- a/arch/arm/mach-omap2/cm.h > +++ b/arch/arm/mach-omap2/cm.h > @@ -23,9 +23,9 @@ > #define OMAP34XX_CM_REGADDR(module, reg) \ > OMAP2_L4_IO_ADDRESS(OMAP3430_CM_BASE + (module) + (reg)) > #define OMAP44XX_CM1_REGADDR(module, reg)\ > - OMAP2_L4_IO_ADDRESS(OMAP4430_CM1_BASE + (module) + > (reg)) > + OMAP2_L4_IO_ADDRESS(OMAP4430_CM1_BASE + ((module) & > (MOD_MASK)) + (reg)) > #define OMAP44XX_CM2_REGADDR(module, reg)\ > - OMAP2_L4_IO_ADDRESS(OMAP4430_CM2_BASE + (module) + > (reg)) > + OMAP2_L4_IO_ADDRESS(OMAP4430_CM2_BASE + ((module) & > (MOD_MASK)) + (reg)) > > #include "cm44xx.h" > > diff --git a/arch/arm/mach-omap2/prcm-common.h > b/arch/arm/mach-omap2/prcm-common.h > index 995b7ed..b93d33e 100644 > --- a/arch/arm/mach-omap2/prcm-common.h > +++ b/arch/arm/mach-omap2/prcm-common.h > @@ -57,10 +57,26 @@ > #define BITFIELD(l_bit, u_bit) \ > (BITS(u_bit) & ~((BITS(l_bit)) >> 1)) > > -/* OMAP44XX specific module offsets */ > +/* > + * OMAP44XX specific module offsets > + * The 16 bit submodule offset is defined for OMAP4 as > + * || > + * | | > + */ > > -/* CM1 instances */ > +#define DEFAULT_BASE 0x0 > +#define PRM_BASE 0x1 > +#define PRCM_MPU_BASE0x2 > +#define CM2_BASE 0x3 > > +#define BASE_ID_SHIFT13 > +#define MOD_MASK ((1 << BASE_ID_SHIFT)-1) > + > +#define PRM_BASE_ID (PRM_BASE << BASE_ID_SHIFT) > +#define PRCM_MPU_BASE_ID (PRCM_MPU_BASE << BASE_ID_SHIFT) > +#define CM2_BASE_ID (CM2_BASE << BASE_ID_SHIFT) > + > +/* CM1 instances */ > #define OMAP4430_CM1_OCP_SOCKET_MOD 0x > #define OMAP4430_CM1_CKGEN_MOD 0x0100 > #define OMAP4430_CM1_MPU_MOD 0x0300 > @@ -71,19 +87,19 @@ > > /* CM2 instances */ > > -#define OMAP4430_CM2_OCP_SOCKET_MOD 0x > -#define OMAP4430_CM2_CKGEN_MOD 0x0100 > -#define OMAP4430_CM2_ALWAYS_ON_MOD 0x0600 > -#define OMAP4430_CM2_CORE_MOD0x0700 > -#define OMAP4430_CM2_IVAHD_MOD 0x0f00 > -#define OMAP4430_CM2_CAM_MOD 0x1000 > -#define OMAP4430_CM2_DSS_MOD 0x1100 > -#define OMAP4430_CM2_GFX_MOD 0x1200 > -#define OMAP4430_CM2_L3INIT_MOD 0x1300 > -#define OMAP4430_CM2_L4PER_MOD 0x1400 > -#define OMAP4430_CM2_CEFUSE_MOD 0x1600 > -#define OMAP4430_CM2_RESTORE_MOD 0x1e00 > -#define OMAP4430_CM2_INSTR_MOD 0x1f00 > +#define OMAP4430_CM2_OCP_SOCKET_MOD 0x | CM2_BASE_ID > +#define OMAP4430_CM2_CKGEN_MOD
RE: [RFC][PATCH 2/2] OMAP4: PRCM: Fix usage of prm/cm accessor api's for OMAP4
> -Original Message- > From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- > ow...@vger.kernel.org] On Behalf Of Nayak, Rajendra > Sent: Tuesday, August 10, 2010 8:33 PM > To: linux-omap@vger.kernel.org > Cc: khil...@deeprootsystems.com; p...@pwsan.com; Cousson, Benoit; Nayak, > Rajendra > Subject: [RFC][PATCH 2/2] OMAP4: PRCM: Fix usage of prm/cm accessor api's for > OMAP4 > > OMAP's have always had PRCM split into PRM for power and reset > management and CM for clock management. > In OMAP4 the split (physically) is not very straight forward and > there are instances of clock management control registers in PRM > and vice versa. > However it still makes sense, even on OMAP4 to logically split > PRCM into PRM and CM for better understanding and to avoid adding > additonal complexity in higher level frameworks which rely on the > accessor api;s to do the low level register accesses. > > Hence this patch makes sure that any clock management code can > use the cm_read/write* accessor apis (without knowing the physical split) > and power and reset management code can use prm_read/write* > accessor api;s. > > To do this the submodule offsets within PRM/CM1 and CM2 have additonal > info embedded in them specifying what base address to use while > trying to access registers in the given submodule. > > The 16 bit signed submodule offset is defined for OMAP4 as > || > | | > > For older OMAP's the base identifier is always set to 0. Hi Paul, Any thoughts on this patch/approach? Regards, Rajendra > > Signed-off-by: Rajendra Nayak > Cc: Kevin Hilman > Cc: Paul Walmsley > Cc: Benoit Cousson > --- > arch/arm/mach-omap2/cm.h |4 +- > arch/arm/mach-omap2/prcm-common.h | 58 -- > - > arch/arm/mach-omap2/prcm.c| 68 > ++-- > arch/arm/mach-omap2/prm.h |4 +- > 4 files changed, 105 insertions(+), 29 deletions(-) > > diff --git a/arch/arm/mach-omap2/cm.h b/arch/arm/mach-omap2/cm.h > index a02ca30..7b7ef09 100644 > --- a/arch/arm/mach-omap2/cm.h > +++ b/arch/arm/mach-omap2/cm.h > @@ -23,9 +23,9 @@ > #define OMAP34XX_CM_REGADDR(module, reg) \ > OMAP2_L4_IO_ADDRESS(OMAP3430_CM_BASE + (module) + > (reg)) > #define OMAP44XX_CM1_REGADDR(module, reg)\ > - OMAP2_L4_IO_ADDRESS(OMAP4430_CM1_BASE + (module) + > (reg)) > + OMAP2_L4_IO_ADDRESS(OMAP4430_CM1_BASE + ((module) & > (MOD_MASK)) + (reg)) > #define OMAP44XX_CM2_REGADDR(module, reg)\ > - OMAP2_L4_IO_ADDRESS(OMAP4430_CM2_BASE + (module) + > (reg)) > + OMAP2_L4_IO_ADDRESS(OMAP4430_CM2_BASE + ((module) & > (MOD_MASK)) + (reg)) > > #include "cm44xx.h" > > diff --git a/arch/arm/mach-omap2/prcm-common.h b/arch/arm/mach-omap2/prcm- > common.h > index 995b7ed..b93d33e 100644 > --- a/arch/arm/mach-omap2/prcm-common.h > +++ b/arch/arm/mach-omap2/prcm-common.h > @@ -57,10 +57,26 @@ > #define BITFIELD(l_bit, u_bit) \ > (BITS(u_bit) & ~((BITS(l_bit)) >> 1)) > > -/* OMAP44XX specific module offsets */ > +/* > + * OMAP44XX specific module offsets > + * The 16 bit submodule offset is defined for OMAP4 as > + * || > + * | | > + */ > > -/* CM1 instances */ > +#define DEFAULT_BASE 0x0 > +#define PRM_BASE 0x1 > +#define PRCM_MPU_BASE0x2 > +#define CM2_BASE 0x3 > > +#define BASE_ID_SHIFT13 > +#define MOD_MASK ((1 << BASE_ID_SHIFT)-1) > + > +#define PRM_BASE_ID (PRM_BASE << BASE_ID_SHIFT) > +#define PRCM_MPU_BASE_ID (PRCM_MPU_BASE << BASE_ID_SHIFT) > +#define CM2_BASE_ID (CM2_BASE << BASE_ID_SHIFT) > + > +/* CM1 instances */ > #define OMAP4430_CM1_OCP_SOCKET_MOD 0x > #define OMAP4430_CM1_CKGEN_MOD 0x0100 > #define OMAP4430_CM1_MPU_MOD 0x0300 > @@ -71,19 +87,19 @@ > > /* CM2 instances */ > > -#define OMAP4430_CM2_OCP_SOCKET_MOD 0x > -#define OMAP4430_CM2_CKGEN_MOD 0x0100 > -#define OMAP4430_CM2_ALWAYS_ON_MOD 0x0600 > -#define OMAP4430_CM2_CORE_MOD0x0700 > -#define OMAP4430_CM2_IVAHD_MOD 0x0f00 > -#define OMAP4430_CM2_CAM_MOD 0x1000 > -#define OMAP4430_CM2_DSS_MOD 0x1100 > -#define OMAP4430_CM2_GFX_MOD 0x1200 > -#define OMAP4430_CM2_L3INIT_MOD 0x1300 > -#define OMAP4430_CM2_L4PER_MOD 0x1400 > -#define OMAP4430_CM2_CEFUS
Re: [RFC][PATCH 2/2] OMAP4: PRCM: Fix usage of prm/cm accessor api's for OMAP4
"Nayak, Rajendra" writes: >> -Original Message- >> From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >> Sent: Wednesday, August 25, 2010 3:10 AM >> To: Nayak, Rajendra >> Cc: linux-omap@vger.kernel.org; p...@pwsan.com; Cousson, Benoit >> Subject: Re: [RFC][PATCH 2/2] OMAP4: PRCM: Fix usage of prm/cm accessor >> api's for >> OMAP4 >> >> Rajendra Nayak writes: >> >> > OMAP's have always had PRCM split into PRM for power and reset >> > management and CM for clock management. >> > In OMAP4 the split (physically) is not very straight forward and >> > there are instances of clock management control registers in PRM >> > and vice versa. >> > However it still makes sense, even on OMAP4 to logically split >> > PRCM into PRM and CM for better understanding and to avoid adding >> > additonal complexity in higher level frameworks which rely on the >> > accessor api;s to do the low level register accesses. >> > >> > Hence this patch makes sure that any clock management code can >> > use the cm_read/write* accessor apis (without knowing the physical split) >> > and power and reset management code can use prm_read/write* >> > accessor api;s. >> > >> > To do this the submodule offsets within PRM/CM1 and CM2 have additonal >> > info embedded in them specifying what base address to use while >> > trying to access registers in the given submodule. >> > >> > The 16 bit signed submodule offset is defined for OMAP4 as >> >|| >> > | | >> > >> > For older OMAP's the base identifier is always set to 0. >> > >> > Signed-off-by: Rajendra Nayak >> > Cc: Kevin Hilman >> > Cc: Paul Walmsley >> > Cc: Benoit Cousson >> >> I'm OK with this approach, but I don't like the extra overhead added for >> every PRCM access on OMAP2/3. >> >> What if you keep the original functions and add new functions for OMAP4, >> and use function pointers init'd at runtime (based on the existence of >> prcm_mpu_base) > I actually have a series to split the powerdomain f/w into platform > specific and platform independent functions. With that I should be > able to get rid of this single function (for prm) for omap2/3 and > omap4 and have separate functions. I can do a similar split for > clockdomain framework and do the same for the cm functions. I will > post the powerdomain split patches soon for review. OK. > Do you think it still makes sense to have this function pointer approach in > the interim? No, it sounds like your split-up approach is better all around. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC][PATCH 2/2] OMAP4: PRCM: Fix usage of prm/cm accessor api's for OMAP4
> -Original Message- > From: Kevin Hilman [mailto:khil...@deeprootsystems.com] > Sent: Wednesday, August 25, 2010 3:10 AM > To: Nayak, Rajendra > Cc: linux-omap@vger.kernel.org; p...@pwsan.com; Cousson, Benoit > Subject: Re: [RFC][PATCH 2/2] OMAP4: PRCM: Fix usage of prm/cm accessor api's > for > OMAP4 > > Rajendra Nayak writes: > > > OMAP's have always had PRCM split into PRM for power and reset > > management and CM for clock management. > > In OMAP4 the split (physically) is not very straight forward and > > there are instances of clock management control registers in PRM > > and vice versa. > > However it still makes sense, even on OMAP4 to logically split > > PRCM into PRM and CM for better understanding and to avoid adding > > additonal complexity in higher level frameworks which rely on the > > accessor api;s to do the low level register accesses. > > > > Hence this patch makes sure that any clock management code can > > use the cm_read/write* accessor apis (without knowing the physical split) > > and power and reset management code can use prm_read/write* > > accessor api;s. > > > > To do this the submodule offsets within PRM/CM1 and CM2 have additonal > > info embedded in them specifying what base address to use while > > trying to access registers in the given submodule. > > > > The 16 bit signed submodule offset is defined for OMAP4 as > > || > > | | > > > > For older OMAP's the base identifier is always set to 0. > > > > Signed-off-by: Rajendra Nayak > > Cc: Kevin Hilman > > Cc: Paul Walmsley > > Cc: Benoit Cousson > > I'm OK with this approach, but I don't like the extra overhead added for > every PRCM access on OMAP2/3. > > What if you keep the original functions and add new functions for OMAP4, > and use function pointers init'd at runtime (based on the existence of > prcm_mpu_base) Hi Kevin, I actually have a series to split the powerdomain f/w into platform specific and platform independent functions. With that I should be able to get rid of this single function (for prm) for omap2/3 and omap4 and have separate functions. I can do a similar split for clockdomain framework and do the same for the cm functions. I will post the powerdomain split patches soon for review. Do you think it still makes sense to have this function pointer approach in the interim? Regards, Rajendra > > Kevin > > > > --- > > arch/arm/mach-omap2/cm.h |4 +- > > arch/arm/mach-omap2/prcm-common.h | 58 > --- > > arch/arm/mach-omap2/prcm.c| 68 > ++-- > > arch/arm/mach-omap2/prm.h |4 +- > > 4 files changed, 105 insertions(+), 29 deletions(-) > > > > diff --git a/arch/arm/mach-omap2/cm.h b/arch/arm/mach-omap2/cm.h > > index a02ca30..7b7ef09 100644 > > --- a/arch/arm/mach-omap2/cm.h > > +++ b/arch/arm/mach-omap2/cm.h > > @@ -23,9 +23,9 @@ > > #define OMAP34XX_CM_REGADDR(module, reg) \ > > OMAP2_L4_IO_ADDRESS(OMAP3430_CM_BASE + (module) + > (reg)) > > #define OMAP44XX_CM1_REGADDR(module, reg) \ > > - OMAP2_L4_IO_ADDRESS(OMAP4430_CM1_BASE + (module) + > (reg)) > > + OMAP2_L4_IO_ADDRESS(OMAP4430_CM1_BASE + ((module) & > (MOD_MASK)) + (reg)) > > #define OMAP44XX_CM2_REGADDR(module, reg) \ > > - OMAP2_L4_IO_ADDRESS(OMAP4430_CM2_BASE + (module) + > (reg)) > > + OMAP2_L4_IO_ADDRESS(OMAP4430_CM2_BASE + ((module) & > (MOD_MASK)) + (reg)) > > > > #include "cm44xx.h" > > > > diff --git a/arch/arm/mach-omap2/prcm-common.h b/arch/arm/mach-omap2/prcm- > common.h > > index 995b7ed..b93d33e 100644 > > --- a/arch/arm/mach-omap2/prcm-common.h > > +++ b/arch/arm/mach-omap2/prcm-common.h > > @@ -57,10 +57,26 @@ > > #define BITFIELD(l_bit, u_bit) \ > > (BITS(u_bit) & ~((BITS(l_bit)) >> 1)) > > > > -/* OMAP44XX specific module offsets */ > > +/* > > + * OMAP44XX specific module offsets > > + * The 16 bit submodule offset is defined for OMAP4 as > > + * || > > + * | | > > + */ > > > > -/* CM1 instances */ > > +#define DEFAULT_BASE 0x0 > > +#define PRM_BASE 0x1 > > +#define PRCM_MPU_BASE 0x2 > > +#define CM2_BASE 0x3 > > > > +#define BASE_ID_SHIFT 13 > > +#define M
Re: [RFC][PATCH 2/2] OMAP4: PRCM: Fix usage of prm/cm accessor api's for OMAP4
Rajendra Nayak writes: > OMAP's have always had PRCM split into PRM for power and reset > management and CM for clock management. > In OMAP4 the split (physically) is not very straight forward and > there are instances of clock management control registers in PRM > and vice versa. > However it still makes sense, even on OMAP4 to logically split > PRCM into PRM and CM for better understanding and to avoid adding > additonal complexity in higher level frameworks which rely on the > accessor api;s to do the low level register accesses. > > Hence this patch makes sure that any clock management code can > use the cm_read/write* accessor apis (without knowing the physical split) > and power and reset management code can use prm_read/write* > accessor api;s. > > To do this the submodule offsets within PRM/CM1 and CM2 have additonal > info embedded in them specifying what base address to use while > trying to access registers in the given submodule. > > The 16 bit signed submodule offset is defined for OMAP4 as > || > | | > > For older OMAP's the base identifier is always set to 0. > > Signed-off-by: Rajendra Nayak > Cc: Kevin Hilman > Cc: Paul Walmsley > Cc: Benoit Cousson I'm OK with this approach, but I don't like the extra overhead added for every PRCM access on OMAP2/3. What if you keep the original functions and add new functions for OMAP4, and use function pointers init'd at runtime (based on the existence of prcm_mpu_base) Kevin > --- > arch/arm/mach-omap2/cm.h |4 +- > arch/arm/mach-omap2/prcm-common.h | 58 --- > arch/arm/mach-omap2/prcm.c| 68 ++-- > arch/arm/mach-omap2/prm.h |4 +- > 4 files changed, 105 insertions(+), 29 deletions(-) > > diff --git a/arch/arm/mach-omap2/cm.h b/arch/arm/mach-omap2/cm.h > index a02ca30..7b7ef09 100644 > --- a/arch/arm/mach-omap2/cm.h > +++ b/arch/arm/mach-omap2/cm.h > @@ -23,9 +23,9 @@ > #define OMAP34XX_CM_REGADDR(module, reg) \ > OMAP2_L4_IO_ADDRESS(OMAP3430_CM_BASE + (module) + (reg)) > #define OMAP44XX_CM1_REGADDR(module, reg)\ > - OMAP2_L4_IO_ADDRESS(OMAP4430_CM1_BASE + (module) + > (reg)) > + OMAP2_L4_IO_ADDRESS(OMAP4430_CM1_BASE + ((module) & > (MOD_MASK)) + (reg)) > #define OMAP44XX_CM2_REGADDR(module, reg)\ > - OMAP2_L4_IO_ADDRESS(OMAP4430_CM2_BASE + (module) + > (reg)) > + OMAP2_L4_IO_ADDRESS(OMAP4430_CM2_BASE + ((module) & > (MOD_MASK)) + (reg)) > > #include "cm44xx.h" > > diff --git a/arch/arm/mach-omap2/prcm-common.h > b/arch/arm/mach-omap2/prcm-common.h > index 995b7ed..b93d33e 100644 > --- a/arch/arm/mach-omap2/prcm-common.h > +++ b/arch/arm/mach-omap2/prcm-common.h > @@ -57,10 +57,26 @@ > #define BITFIELD(l_bit, u_bit) \ > (BITS(u_bit) & ~((BITS(l_bit)) >> 1)) > > -/* OMAP44XX specific module offsets */ > +/* > + * OMAP44XX specific module offsets > + * The 16 bit submodule offset is defined for OMAP4 as > + * || > + * | | > + */ > > -/* CM1 instances */ > +#define DEFAULT_BASE 0x0 > +#define PRM_BASE 0x1 > +#define PRCM_MPU_BASE0x2 > +#define CM2_BASE 0x3 > > +#define BASE_ID_SHIFT13 > +#define MOD_MASK ((1 << BASE_ID_SHIFT)-1) > + > +#define PRM_BASE_ID (PRM_BASE << BASE_ID_SHIFT) > +#define PRCM_MPU_BASE_ID (PRCM_MPU_BASE << BASE_ID_SHIFT) > +#define CM2_BASE_ID (CM2_BASE << BASE_ID_SHIFT) > + > +/* CM1 instances */ > #define OMAP4430_CM1_OCP_SOCKET_MOD 0x > #define OMAP4430_CM1_CKGEN_MOD 0x0100 > #define OMAP4430_CM1_MPU_MOD 0x0300 > @@ -71,19 +87,19 @@ > > /* CM2 instances */ > > -#define OMAP4430_CM2_OCP_SOCKET_MOD 0x > -#define OMAP4430_CM2_CKGEN_MOD 0x0100 > -#define OMAP4430_CM2_ALWAYS_ON_MOD 0x0600 > -#define OMAP4430_CM2_CORE_MOD0x0700 > -#define OMAP4430_CM2_IVAHD_MOD 0x0f00 > -#define OMAP4430_CM2_CAM_MOD 0x1000 > -#define OMAP4430_CM2_DSS_MOD 0x1100 > -#define OMAP4430_CM2_GFX_MOD 0x1200 > -#define OMAP4430_CM2_L3INIT_MOD 0x1300 > -#define OMAP4430_CM2_L4PER_MOD 0x1400 > -#define OMAP4430_CM2_CEFUSE_MOD 0x1600 > -#define OMAP4430_CM2_RESTORE_MOD 0x1e00 > -#define OMAP4430_CM2_INSTR_MOD 0x1f00 > +#define OMAP4430_CM2_OCP_SOCKET_MOD 0x | CM2_BASE_ID > +#define OMAP4430_CM2_CKGEN_MOD 0x0100 | CM2_BASE_ID > +#define OMAP4430_CM2_ALWAYS_ON_MOD 0x0600 | CM2_BASE_ID > +#define OMAP4430_CM2_CORE_MOD0x0700 | CM2_BASE_ID > +#define OMAP4430_CM2_IVAHD_MOD 0x0f00 | CM2_BASE_ID > +#define OMAP4430_CM2_CAM_MOD 0x1000 | CM2_BASE_ID > +#define OMAP4430_CM2_DSS_MOD 0x1100 | CM2_BASE_ID > +#define OMA
[RFC][PATCH 2/2] OMAP4: PRCM: Fix usage of prm/cm accessor api's for OMAP4
OMAP's have always had PRCM split into PRM for power and reset management and CM for clock management. In OMAP4 the split (physically) is not very straight forward and there are instances of clock management control registers in PRM and vice versa. However it still makes sense, even on OMAP4 to logically split PRCM into PRM and CM for better understanding and to avoid adding additonal complexity in higher level frameworks which rely on the accessor api;s to do the low level register accesses. Hence this patch makes sure that any clock management code can use the cm_read/write* accessor apis (without knowing the physical split) and power and reset management code can use prm_read/write* accessor api;s. To do this the submodule offsets within PRM/CM1 and CM2 have additonal info embedded in them specifying what base address to use while trying to access registers in the given submodule. The 16 bit signed submodule offset is defined for OMAP4 as || | | For older OMAP's the base identifier is always set to 0. Signed-off-by: Rajendra Nayak Cc: Kevin Hilman Cc: Paul Walmsley Cc: Benoit Cousson --- arch/arm/mach-omap2/cm.h |4 +- arch/arm/mach-omap2/prcm-common.h | 58 --- arch/arm/mach-omap2/prcm.c| 68 ++-- arch/arm/mach-omap2/prm.h |4 +- 4 files changed, 105 insertions(+), 29 deletions(-) diff --git a/arch/arm/mach-omap2/cm.h b/arch/arm/mach-omap2/cm.h index a02ca30..7b7ef09 100644 --- a/arch/arm/mach-omap2/cm.h +++ b/arch/arm/mach-omap2/cm.h @@ -23,9 +23,9 @@ #define OMAP34XX_CM_REGADDR(module, reg) \ OMAP2_L4_IO_ADDRESS(OMAP3430_CM_BASE + (module) + (reg)) #define OMAP44XX_CM1_REGADDR(module, reg) \ - OMAP2_L4_IO_ADDRESS(OMAP4430_CM1_BASE + (module) + (reg)) + OMAP2_L4_IO_ADDRESS(OMAP4430_CM1_BASE + ((module) & (MOD_MASK)) + (reg)) #define OMAP44XX_CM2_REGADDR(module, reg) \ - OMAP2_L4_IO_ADDRESS(OMAP4430_CM2_BASE + (module) + (reg)) + OMAP2_L4_IO_ADDRESS(OMAP4430_CM2_BASE + ((module) & (MOD_MASK)) + (reg)) #include "cm44xx.h" diff --git a/arch/arm/mach-omap2/prcm-common.h b/arch/arm/mach-omap2/prcm-common.h index 995b7ed..b93d33e 100644 --- a/arch/arm/mach-omap2/prcm-common.h +++ b/arch/arm/mach-omap2/prcm-common.h @@ -57,10 +57,26 @@ #define BITFIELD(l_bit, u_bit) \ (BITS(u_bit) & ~((BITS(l_bit)) >> 1)) -/* OMAP44XX specific module offsets */ +/* + * OMAP44XX specific module offsets + * The 16 bit submodule offset is defined for OMAP4 as + * || + * | | + */ -/* CM1 instances */ +#define DEFAULT_BASE 0x0 +#define PRM_BASE 0x1 +#define PRCM_MPU_BASE 0x2 +#define CM2_BASE 0x3 +#define BASE_ID_SHIFT 13 +#define MOD_MASK ((1 << BASE_ID_SHIFT)-1) + +#define PRM_BASE_ID(PRM_BASE << BASE_ID_SHIFT) +#define PRCM_MPU_BASE_ID (PRCM_MPU_BASE << BASE_ID_SHIFT) +#define CM2_BASE_ID(CM2_BASE << BASE_ID_SHIFT) + +/* CM1 instances */ #define OMAP4430_CM1_OCP_SOCKET_MOD0x #define OMAP4430_CM1_CKGEN_MOD 0x0100 #define OMAP4430_CM1_MPU_MOD 0x0300 @@ -71,19 +87,19 @@ /* CM2 instances */ -#define OMAP4430_CM2_OCP_SOCKET_MOD0x -#define OMAP4430_CM2_CKGEN_MOD 0x0100 -#define OMAP4430_CM2_ALWAYS_ON_MOD 0x0600 -#define OMAP4430_CM2_CORE_MOD 0x0700 -#define OMAP4430_CM2_IVAHD_MOD 0x0f00 -#define OMAP4430_CM2_CAM_MOD 0x1000 -#define OMAP4430_CM2_DSS_MOD 0x1100 -#define OMAP4430_CM2_GFX_MOD 0x1200 -#define OMAP4430_CM2_L3INIT_MOD0x1300 -#define OMAP4430_CM2_L4PER_MOD 0x1400 -#define OMAP4430_CM2_CEFUSE_MOD0x1600 -#define OMAP4430_CM2_RESTORE_MOD 0x1e00 -#define OMAP4430_CM2_INSTR_MOD 0x1f00 +#define OMAP4430_CM2_OCP_SOCKET_MOD0x | CM2_BASE_ID +#define OMAP4430_CM2_CKGEN_MOD 0x0100 | CM2_BASE_ID +#define OMAP4430_CM2_ALWAYS_ON_MOD 0x0600 | CM2_BASE_ID +#define OMAP4430_CM2_CORE_MOD 0x0700 | CM2_BASE_ID +#define OMAP4430_CM2_IVAHD_MOD 0x0f00 | CM2_BASE_ID +#define OMAP4430_CM2_CAM_MOD 0x1000 | CM2_BASE_ID +#define OMAP4430_CM2_DSS_MOD 0x1100 | CM2_BASE_ID +#define OMAP4430_CM2_GFX_MOD 0x1200 | CM2_BASE_ID +#define OMAP4430_CM2_L3INIT_MOD0x1300 | CM2_BASE_ID +#define OMAP4430_CM2_L4PER_MOD 0x1400 | CM2_BASE_ID +#define OMAP4430_CM2_CEFUSE_MOD0x1600 | CM2_BASE_ID +#define OMAP4430_CM2_RESTORE_MOD 0x1e00 | CM2_BASE_ID +#define OMAP4430_CM2_INSTR_MOD 0x1f00 | CM2_BASE_ID /* PRM instances */ @@ -102,9 +118,9 @@ #define OMAP4430_PRM_L4PER_MOD 0x1400 #define OMAP4430_PRM_CEFUSE_MOD0x1600 #define OMAP4430
