video: ARM CLCD: runtime check for Versatile

Started by Linus Walleij on Feb 2, 2016 · 4 messages from 3 authors · Last reply Feb 3, 2016

Subsystems (2)

Maintainers (2)

STALE3934d

open the first message on its own page

Linus Walleij

Message details and related content
Open message details and related content
The current compile-time check for inversed IENB/CNTL does not
work in multiplatform boots: as soon as versatile is included
in the build, the IENB/CNTL is switched and breaks graphics.
Convert this to a runtime switch.

Cc: <redacted>
Cc: Rob Herring <robh@kernel.org>
Cc: Russell King <redacted>
Fixes: a29da136de34 ("ARM: versatile: convert to multi-platform")
Signed-off-by: Linus Walleij <redacted>
---
 drivers/video/fbdev/amba-clcd.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index 9362424c2340..f9ef06d0cd48 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -440,13 +440,14 @@ static int clcdfb_register(struct clcd_fb *fb)
 		fb->off_ienb = CLCD_PL111_IENB;
 		fb->off_cntl = CLCD_PL111_CNTL;
 	} else {
-#ifdef CONFIG_ARCH_VERSATILE
-		fb->off_ienb = CLCD_PL111_IENB;
-		fb->off_cntl = CLCD_PL111_CNTL;
-#else
-		fb->off_ienb = CLCD_PL110_IENB;
-		fb->off_cntl = CLCD_PL110_CNTL;
-#endif
+		if (of_machine_is_compatible("arm,versatile-ab") ||
+		    of_machine_is_compatible("arm,versatile-pb")) {
+			fb->off_ienb = CLCD_PL111_IENB;
+			fb->off_cntl = CLCD_PL111_CNTL;
+		} else {
+			fb->off_ienb = CLCD_PL110_IENB;
+			fb->off_cntl = CLCD_PL110_CNTL;
+		}
 	}
 
 	fb->clk = clk_get(&fb->dev->dev, NULL);
-- 
2.4.3

Arnd Bergmann replied to Linus Walleij

Message details and related content
Open message details and related content
On Tuesday 02 February 2016 21:47:39 Linus Walleij wrote:
quoted hunk
        } else {
-#ifdef CONFIG_ARCH_VERSATILE
-               fb->off_ienb = CLCD_PL111_IENB;
-               fb->off_cntl = CLCD_PL111_CNTL;
-#else
-               fb->off_ienb = CLCD_PL110_IENB;
-               fb->off_cntl = CLCD_PL110_CNTL;
-#endif
+               if (of_machine_is_compatible("arm,versatile-ab") ||
+                   of_machine_is_compatible("arm,versatile-pb")) {
+                       fb->off_ienb = CLCD_PL111_IENB;
+                       fb->off_cntl = CLCD_PL111_CNTL;
+               } else {
+                       fb->off_ienb = CLCD_PL110_IENB;
+                       fb->off_cntl = CLCD_PL110_CNTL;
+               }
        }
Could that be done based on the AMBA device ID instead?

	Arnd

Russell King - ARM Linux replied to Arnd Bergmann

Message details and related content
Open message details and related content
On Tue, Feb 02, 2016 at 10:10:49PM +0100, Arnd Bergmann wrote:
On Tuesday 02 February 2016 21:47:39 Linus Walleij wrote:
quoted
        } else {
-#ifdef CONFIG_ARCH_VERSATILE
-               fb->off_ienb = CLCD_PL111_IENB;
-               fb->off_cntl = CLCD_PL111_CNTL;
-#else
-               fb->off_ienb = CLCD_PL110_IENB;
-               fb->off_cntl = CLCD_PL110_CNTL;
-#endif
+               if (of_machine_is_compatible("arm,versatile-ab") ||
+                   of_machine_is_compatible("arm,versatile-pb")) {
+                       fb->off_ienb = CLCD_PL111_IENB;
+                       fb->off_cntl = CLCD_PL111_CNTL;
+               } else {
+                       fb->off_ienb = CLCD_PL110_IENB;
+                       fb->off_cntl = CLCD_PL110_CNTL;
+               }
        }
Could that be done based on the AMBA device ID instead?
Unfortunately not.  It's a mistake made on one Versatile board
which reverses the registers.  There's nothing to distinguish it
in the primecell itself.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Arnd Bergmann replied to Russell King - ARM Linux

Message details and related content
Open message details and related content
On Wednesday 03 February 2016 00:41:54 Russell King - ARM Linux wrote:
On Tue, Feb 02, 2016 at 10:10:49PM +0100, Arnd Bergmann wrote:
quoted
On Tuesday 02 February 2016 21:47:39 Linus Walleij wrote:
quoted
        } else {
-#ifdef CONFIG_ARCH_VERSATILE
-               fb->off_ienb = CLCD_PL111_IENB;
-               fb->off_cntl = CLCD_PL111_CNTL;
-#else
-               fb->off_ienb = CLCD_PL110_IENB;
-               fb->off_cntl = CLCD_PL110_CNTL;
-#endif
+               if (of_machine_is_compatible("arm,versatile-ab") ||
+                   of_machine_is_compatible("arm,versatile-pb")) {
+                       fb->off_ienb = CLCD_PL111_IENB;
+                       fb->off_cntl = CLCD_PL111_CNTL;
+               } else {
+                       fb->off_ienb = CLCD_PL110_IENB;
+                       fb->off_cntl = CLCD_PL110_CNTL;
+               }
        }
Could that be done based on the AMBA device ID instead?
Unfortunately not.  It's a mistake made on one Versatile board
which reverses the registers.  There's nothing to distinguish it
in the primecell itself.
Ok, I see. It would be nice to have this knowledge locally in
the driver, e.g. through a special compatible string for the
variant on the versatile, or a bool property, but this way seems
good enough as we can be reasonably sure that nobody else made the
same mistake.

	Arnd
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