atmel_lcdfb: fix usage of wrong registers in suspend/resume

Started by Hubert Feurstein on Jun 6, 2011 · 6 messages from 4 authors · Last reply Jan 30, 2012

Subsystems (2)

Maintainers (2)

STALE5401d

open the first message on its own page

  1. [PATCH v2] atmel_lcdfb: fix usage of CONTRAST_CTR in suspend/resume
    Related patches
  2. [PATCH v2] atmel_lcdfb: fix usage of CONTRAST_CTR in suspend/resume
    Related patches
  3. [PATCH v2] atmel_lcdfb: fix usage of CONTRAST_CTR in suspend/resume
    Related patches
Message details and related content
I assume the intention was to set the contrast value to 0 and not
the contrast control register (in atmel_lcdfb_suspend). And in
atmel_lcdfb_resume the contrast value should be restored.

Signed-off-by: Hubert Feurstein <redacted>
Cc: Nicolas Ferre <redacted>
Cc: Jean-Christophe PLAGNIOL-VILLARD <redacted>
---
 drivers/video/atmel_lcdfb.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 4484c72..2ed7ec1 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -1086,7 +1086,7 @@ static int atmel_lcdfb_suspend(struct platform_device *pdev, pm_message_t mesg)
 	lcdc_writel(sinfo, ATMEL_LCDC_IDR, ~0UL);
 
 	sinfo->saved_lcdcon = lcdc_readl(sinfo, ATMEL_LCDC_CONTRAST_VAL);
-	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, 0);
+	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_VAL, 0);
 	if (sinfo->atmel_lcdfb_power_control)
 		sinfo->atmel_lcdfb_power_control(0);
 
@@ -1105,7 +1105,7 @@ static int atmel_lcdfb_resume(struct platform_device *pdev)
 	atmel_lcdfb_start(sinfo);
 	if (sinfo->atmel_lcdfb_power_control)
 		sinfo->atmel_lcdfb_power_control(1);
-	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, sinfo->saved_lcdcon);
+	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_VAL, sinfo->saved_lcdcon);
 
 	/* Enable FIFO & DMA errors */
 	lcdc_writel(sinfo, ATMEL_LCDC_IER, ATMEL_LCDC_UFLWI
-- 
1.7.1
Message details and related content
Or it must be this way:
---
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 4484c72..c2ceae4 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -1085,7 +1085,7 @@ static int atmel_lcdfb_suspend(struct
platform_device *pdev, pm_message_t mesg)
 	 */
 	lcdc_writel(sinfo, ATMEL_LCDC_IDR, ~0UL);

-	sinfo->saved_lcdcon = lcdc_readl(sinfo, ATMEL_LCDC_CONTRAST_VAL);
+	sinfo->saved_lcdcon = lcdc_readl(sinfo, ATMEL_LCDC_CONTRAST_CTR);
 	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, 0);
 	if (sinfo->atmel_lcdfb_power_control)
 		sinfo->atmel_lcdfb_power_control(0);
--
So which solution was originally intended?

Best regards
Hubert

2011/6/6 Hubert Feurstein [off-list ref]:
quoted hunk
I assume the intention was to set the contrast value to 0 and not
the contrast control register (in atmel_lcdfb_suspend). And in
atmel_lcdfb_resume the contrast value should be restored.

Signed-off-by: Hubert Feurstein <redacted>
Cc: Nicolas Ferre <redacted>
Cc: Jean-Christophe PLAGNIOL-VILLARD <redacted>
---
 drivers/video/atmel_lcdfb.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 4484c72..2ed7ec1 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -1086,7 +1086,7 @@ static int atmel_lcdfb_suspend(struct platform_device *pdev, pm_message_t mesg)
       lcdc_writel(sinfo, ATMEL_LCDC_IDR, ~0UL);

       sinfo->saved_lcdcon = lcdc_readl(sinfo, ATMEL_LCDC_CONTRAST_VAL);
-       lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, 0);
+       lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_VAL, 0);
       if (sinfo->atmel_lcdfb_power_control)
               sinfo->atmel_lcdfb_power_control(0);
@@ -1105,7 +1105,7 @@ static int atmel_lcdfb_resume(struct platform_device *pdev)
       atmel_lcdfb_start(sinfo);
       if (sinfo->atmel_lcdfb_power_control)
               sinfo->atmel_lcdfb_power_control(1);
-       lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, sinfo->saved_lcdcon);
+       lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_VAL, sinfo->saved_lcdcon);

       /* Enable FIFO & DMA errors */
       lcdc_writel(sinfo, ATMEL_LCDC_IER, ATMEL_LCDC_UFLWI
--
1.7.1
Message details and related content
On 06/06/2011 03:24 PM, Hubert Feurstein :
quoted hunk
Or it must be this way:
---
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 4484c72..c2ceae4 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -1085,7 +1085,7 @@ static int atmel_lcdfb_suspend(struct
platform_device *pdev, pm_message_t mesg)
 	 */
 	lcdc_writel(sinfo, ATMEL_LCDC_IDR, ~0UL);

-	sinfo->saved_lcdcon = lcdc_readl(sinfo, ATMEL_LCDC_CONTRAST_VAL);
+	sinfo->saved_lcdcon = lcdc_readl(sinfo, ATMEL_LCDC_CONTRAST_CTR);
 	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, 0);
 	if (sinfo->atmel_lcdfb_power_control)
 		sinfo->atmel_lcdfb_power_control(0);
--
So which solution was originally intended?
Yes exactly: this solution is the preferred one.

I rebuild the patch with your authorship and send it again.

Thanks Hubert, best regards,

Best regards
Hubert

2011/6/6 Hubert Feurstein [off-list ref]:
quoted
I assume the intention was to set the contrast value to 0 and not
the contrast control register (in atmel_lcdfb_suspend). And in
atmel_lcdfb_resume the contrast value should be restored.

Signed-off-by: Hubert Feurstein <redacted>
Cc: Nicolas Ferre <redacted>
Cc: Jean-Christophe PLAGNIOL-VILLARD <redacted>
---
 drivers/video/atmel_lcdfb.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 4484c72..2ed7ec1 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -1086,7 +1086,7 @@ static int atmel_lcdfb_suspend(struct platform_device *pdev, pm_message_t mesg)
       lcdc_writel(sinfo, ATMEL_LCDC_IDR, ~0UL);

       sinfo->saved_lcdcon = lcdc_readl(sinfo, ATMEL_LCDC_CONTRAST_VAL);
-       lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, 0);
+       lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_VAL, 0);
       if (sinfo->atmel_lcdfb_power_control)
               sinfo->atmel_lcdfb_power_control(0);
@@ -1105,7 +1105,7 @@ static int atmel_lcdfb_resume(struct platform_device *pdev)
       atmel_lcdfb_start(sinfo);
       if (sinfo->atmel_lcdfb_power_control)
               sinfo->atmel_lcdfb_power_control(1);
-       lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, sinfo->saved_lcdcon);
+       lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_VAL, sinfo->saved_lcdcon);

       /* Enable FIFO & DMA errors */
       lcdc_writel(sinfo, ATMEL_LCDC_IER, ATMEL_LCDC_UFLWI
--
1.7.1

3 more quoted lines


-- 
Nicolas Ferre

[PATCH v2] atmel_lcdfb: fix usage of CONTRAST_CTR in suspend/resume

Message details and related content
From: Hubert Feurstein <redacted>

An error was existing in the saving of CONTRAST_CTR register
across suspend/resume.

Signed-off-by: Hubert Feurstein <redacted>
Signed-off-by: Nicolas Ferre <redacted>
Cc: stable <redacted>
---
 drivers/video/atmel_lcdfb.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index d7e3712..d99505b 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -1102,7 +1102,7 @@ static int atmel_lcdfb_suspend(struct platform_device *pdev, pm_message_t mesg)
 	 */
 	lcdc_writel(sinfo, ATMEL_LCDC_IDR, ~0UL);
 
-	sinfo->saved_lcdcon = lcdc_readl(sinfo, ATMEL_LCDC_CONTRAST_VAL);
+	sinfo->saved_lcdcon = lcdc_readl(sinfo, ATMEL_LCDC_CONTRAST_CTR);
 	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, 0);
 	if (sinfo->atmel_lcdfb_power_control)
 		sinfo->atmel_lcdfb_power_control(0);
-- 
1.7.5.4

[PATCH v2] atmel_lcdfb: fix usage of CONTRAST_CTR in suspend/resume

Message details and related content
On 17:23 Mon 09 Jan     , Nicolas Ferre wrote:
From: Hubert Feurstein <redacted>

An error was existing in the saving of CONTRAST_CTR register
across suspend/resume.

Signed-off-by: Hubert Feurstein <redacted>
Signed-off-by: Nicolas Ferre <redacted>
Cc: stable <redacted>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <redacted>

Best Regards,
J.

[PATCH v2] atmel_lcdfb: fix usage of CONTRAST_CTR in suspend/resume

Message details and related content
On 01/09/2012 04:23 PM, Nicolas Ferre wrote:
From: Hubert Feurstein <redacted>

An error was existing in the saving of CONTRAST_CTR register
across suspend/resume.

Signed-off-by: Hubert Feurstein <redacted>
Signed-off-by: Nicolas Ferre <redacted>
Cc: stable <redacted>
Applied.


Thanks,

Florian Tobias Schandinat
quoted hunk
---
 drivers/video/atmel_lcdfb.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index d7e3712..d99505b 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -1102,7 +1102,7 @@ static int atmel_lcdfb_suspend(struct platform_device *pdev, pm_message_t mesg)
 	 */
 	lcdc_writel(sinfo, ATMEL_LCDC_IDR, ~0UL);
 
-	sinfo->saved_lcdcon = lcdc_readl(sinfo, ATMEL_LCDC_CONTRAST_VAL);
+	sinfo->saved_lcdcon = lcdc_readl(sinfo, ATMEL_LCDC_CONTRAST_CTR);
 	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, 0);
 	if (sinfo->atmel_lcdfb_power_control)
 		sinfo->atmel_lcdfb_power_control(0);
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help