Re: [PATCHv3 1/2] OMAP3: PM: Smartreflex IP update changes for OMAP3630

[Kevin Hilman]

Thara Gopinath  writes:

> OMAP3430 uses the 65nm version of the smartreflex IP where as
> OMAP3630 and OMAP4430 uses the 45nm updated IP.
>
> This patch adds support for the updated smartreflex IP used
> in OMAP3630 and OMAP4 in the smartreflex driver.
>
> Major changes between the two versions of IP involve:
> 1. Change in offset position for ERRCONFIG and SENERROR registers
> 2. Change in bit positions for VP bound interrupt enable and status
>in ERRCONFIG register.
> 3. Change in bit positions and width of SENNENABLE and SENPENABLE
>bits in SRCONFIG registers.
> 4. Introduction of separate irq registers for MCU bound interrupts.
> 5. Removal of clockactivity bits in ERRCONFIG and introduction of
>   idlemode and wakeupenable bits in ERRCONFIG.
>
> Signed-off-by: Thara Gopinath 

Looks good.  One minor comment for discussion below...

[...]

> @@ -619,6 +717,7 @@ static int __devinit omap_smartreflex_probe(struct 
> platform_device *pdev)
>   sr_info->srid = pdev->id;
>   sr_info->is_autocomp_active = 0;
>   sr_info->clk_length = 0;
> + sr_info->sr_ip_type = odev->hwmods[0]->class->rev;

I'm not crazy about drivers having to know the details of hwmod
structs.  I'd suggest a HWMOD API for getting this revision.

Other ideas?

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

[PATCHv3 1/2] OMAP3: PM: Smartreflex IP update changes for OMAP3630

[Thara Gopinath]

OMAP3430 uses the 65nm version of the smartreflex IP where as
OMAP3630 and OMAP4430 uses the 45nm updated IP.

This patch adds support for the updated smartreflex IP used
in OMAP3630 and OMAP4 in the smartreflex driver.

Major changes between the two versions of IP involve:
1. Change in offset position for ERRCONFIG and SENERROR registers
2. Change in bit positions for VP bound interrupt enable and status
   in ERRCONFIG register.
3. Change in bit positions and width of SENNENABLE and SENPENABLE
   bits in SRCONFIG registers.
4. Introduction of separate irq registers for MCU bound interrupts.
5. Removal of clockactivity bits in ERRCONFIG and introduction of
  idlemode and wakeupenable bits in ERRCONFIG.

Signed-off-by: Thara Gopinath 
---
 arch/arm/mach-omap2/smartreflex.c |  215 +++--
 arch/arm/mach-omap2/smartreflex.h |   44 +++--
 2 files changed, 194 insertions(+), 65 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex.c 
b/arch/arm/mach-omap2/smartreflex.c
index 4dccfd1..b81f9f3 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -43,6 +43,7 @@ struct omap_sr {
int srid;
int is_sr_enable;
int is_autocomp_active;
+   int sr_ip_type;
u32 clk_length;
u32 err_weight;
u32 err_minlimit;
@@ -71,6 +72,7 @@ static inline void sr_modify_reg(struct omap_sr *sr, unsigned 
offset, u32 mask,
 {
struct omap_device *odev = to_omap_device(sr->pdev);
u32 reg_val;
+   u32 errconfig_offs, errconfig_mask;
 
reg_val = omap_hwmod_readl(odev->hwmods[0], offset);
reg_val &= ~mask;
@@ -82,8 +84,15 @@ static inline void sr_modify_reg(struct omap_sr *sr, 
unsigned offset, u32 mask,
 * value. Now if there is an actual reguest to write to these bits
 * they will be set in the nex step.
 */
-   if (offset == ERRCONFIG)
-   reg_val &= ~ERRCONFIG_STATUS_MASK;
+   if (sr->sr_ip_type == SR_TYPE_V1) {
+   errconfig_offs = ERRCONFIG_V1;
+   errconfig_mask = ERRCONFIG_STATUS_V1_MASK;
+   } else if (sr->sr_ip_type == SR_TYPE_V2) {
+   errconfig_offs = ERRCONFIG_V2;
+   errconfig_mask = ERRCONFIG_VPBOUNDINTST_V2;
+   }
+   if (offset == errconfig_offs)
+   reg_val &= ~errconfig_mask;
 
reg_val |= value;
 
@@ -114,13 +123,19 @@ static struct omap_sr *_sr_lookup(int srid)
 static irqreturn_t sr_omap_isr(int irq, void *data)
 {
struct omap_sr *sr_info = (struct omap_sr *)data;
-   u32 status;
-
-   /* Read the status bits */
-   status = sr_read_reg(sr_info, ERRCONFIG);
-
-   /* Clear them by writing back */
-   sr_write_reg(sr_info, ERRCONFIG, status);
+   u32 status = 0;
+
+   if (sr_info->sr_ip_type == SR_TYPE_V1) {
+   /* Read the status bits */
+   status = sr_read_reg(sr_info, ERRCONFIG_V1);
+   /* Clear them by writing back */
+   sr_write_reg(sr_info, ERRCONFIG_V1, status);
+   } else if (sr_info->sr_ip_type == SR_TYPE_V2) {
+   /* Read the status bits */
+   sr_read_reg(sr_info, IRQSTATUS);
+   /* Clear them by writing back */
+   sr_write_reg(sr_info, IRQSTATUS, status);
+   }
 
/* Call the class driver notify function if registered*/
if (sr_class->class_type == SR_CLASS2 && sr_class->notify)
@@ -243,6 +258,77 @@ static int sr_late_init(struct omap_sr *sr_info)
return ret;
 }
 
+static void sr_v1_disable(struct omap_sr *sr)
+{
+   int timeout = 0;
+
+   /* Enable MCUDisableAcknowledge interrupt */
+   sr_modify_reg(sr, ERRCONFIG_V1,
+   ERRCONFIG_MCUDISACKINTEN, ERRCONFIG_MCUDISACKINTEN);
+
+   /* SRCONFIG - disable SR */
+   sr_modify_reg(sr, SRCONFIG, SRCONFIG_SRENABLE, 0x0);
+
+   /* Disable all other SR interrupts and clear the status */
+   sr_modify_reg(sr, ERRCONFIG_V1,
+   (ERRCONFIG_MCUACCUMINTEN | ERRCONFIG_MCUVALIDINTEN |
+   ERRCONFIG_MCUBOUNDINTEN | ERRCONFIG_VPBOUNDINTEN_V1),
+   (ERRCONFIG_MCUACCUMINTST | ERRCONFIG_MCUVALIDINTST |
+   ERRCONFIG_MCUBOUNDINTST |
+   ERRCONFIG_VPBOUNDINTST_V1));
+
+   /*
+* Wait for SR to be disabled.
+* wait until ERRCONFIG.MCUDISACKINTST = 1. Typical latency is 1us.
+*/
+   omap_test_timeout((sr_read_reg(sr, ERRCONFIG_V1) &
+   ERRCONFIG_MCUDISACKINTST), SR_DISABLE_TIMEOUT,
+   timeout);
+
+   if (timeout >= SR_DISABLE_TIMEOUT)
+   pr_warning("SR%d disable timedout\n", sr->srid);
+
+   /* Disable MCUDisableAcknowledge interrupt & clear pending interrupt */
+   s