atmel_lcdfb: fix usage of wrong registers in suspend/resume

PATCH In review Hubert Feurstein 4 participants 6 messages Jun 6, 2011 – Jan 30, 2012

Subsystems (2)

Framebuffer layer Arm/Microchip (AT91) SoC support

lore

Share & tools

lore

Hubert Feurstein

author

2011-06-06 08:50 UTC #0 lore raw

PATCH 1/1 atmel_lcdfb: fix usage of wrong registers in suspend/resume

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 <[email protected]>

Cc Nicolas Ferre <[email protected]>

Cc Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

---
 drivers/video/atmel_lcdfb.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/video/atmel_lcdfb.c +2 -2

index 4484c72..2ed7ec1 100644
@@ -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

Signature · 1 line

1.7.1

Hubert Feurstein

author

2011-06-06 13:24 UTC #1 lore raw

Or it must be this way:
---

drivers/video/atmel_lcdfb.c +0 -0

index 4484c72..c2ceae4 100644
@@ -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 <[email protected]>:

Quoted 37 lines

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 <[email protected]>

Cc Nicolas Ferre <[email protected]>

Cc Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

---
drivers/video/atmel_lcdfb.c | 4 ++--

 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/video/atmel_lcdfb.c +2 -2

index 4484c72..2ed7ec1 100644
@@ -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

Nicolas Ferre

2012-01-09 15:45 UTC #2 lore raw

In reply to Hubert Feurstein

On 06/06/2011 03:24 PM, Hubert Feurstein :

Quoted 15 lines

Or it must be this way:
---

drivers/video/atmel_lcdfb.c +1 -1

index 4484c72..c2ceae4 100644
@@ -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)

Quoted 3 lines

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,

Quoted 43 lines

Best regards
Hubert

2011/6/6 Hubert Feurstein <[email protected]>:

Quoted 37 lines

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 <[email protected]>

Cc Nicolas Ferre <[email protected]>

Cc Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

---
 drivers/video/atmel_lcdfb.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/video/atmel_lcdfb.c +2 -3

index 4484c72..2ed7ec1 100644
@@ -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

Signature · 1 line

Nicolas Ferre

Nicolas Ferre

2012-01-09 16:23 UTC #3 lore raw

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

In reply to Hubert Feurstein

From: Hubert Feurstein <[email protected]>

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

Signed-off-by Hubert Feurstein <[email protected]>

Signed-off-by Nicolas Ferre <[email protected]>

Cc stable <[email protected]>

---
 drivers/video/atmel_lcdfb.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/video/atmel_lcdfb.c +1 -1

index d7e3712..d99505b 100644
@@ -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);

Signature · 1 line

1.7.5.4

Jean-Christophe PLAGNIOL-VILLARD

2012-01-09 16:46 UTC #4 lore raw

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

Quoted 8 lines · Nicolas Ferre

From: Hubert Feurstein <[email protected]>
An error was existing in the saving of CONTRAST_CTR register across suspend/resume.
Signed-off-by Hubert Feurstein <[email protected]>
Signed-off-by Nicolas Ferre <[email protected]>
Cc stable <[email protected]>

Acked-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Best Regards,
J.

Florian Tobias Schandinat

Applied

2012-01-30 05:13 UTC #5 lore raw

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

In reply to Nicolas Ferre

Quoted 8 lines · Nicolas Ferre

From: Hubert Feurstein <[email protected]>
An error was existing in the saving of CONTRAST_CTR register across suspend/resume.
Signed-off-by Hubert Feurstein <[email protected]>
Signed-off-by Nicolas Ferre <[email protected]>
Cc stable <[email protected]>

Applied.

Thanks,

Florian Tobias Schandinat

Quoted 17 lines

---
 drivers/video/atmel_lcdfb.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/video/atmel_lcdfb.c +1 -1

index d7e3712..d99505b 100644
@@ -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);