atmel_lcdfb: introduce driver data

From b1cb4bbebbe8f2ef7049cdc8604f516bb0108403 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <[email protected]>
Date: Sat, 6 Jan 2018 14:33:53 +0100
Subject: [PATCH] atmel_lcdfb: introduce driver data

Introduce driver data like known from the kernel. This allows us to get rid of the hack where the intensity bit support was included in the lcd wiring mode. (No longer any support for IBGR, IRBG)

It has the nice side-effect that all places where we test for hacks can now use flags and not a set of cpu's. So we keep all the configuration in one place.

The configuration is included for non-DT users.

Signed-off-by Sam Ravnborg <[email protected]>

---

When using this for a proprietary board I realized that using IBGR & friends was not a good idea.
So drop it and introduce the same concept as used in the kernel. This has the side effect that some parts in the
driver got a little cleaner.

Tested on my at91sam9263ek board.

As we have many boards that are not yet DT enabled I made sure to keep this in sync too - but not tested.

Sam

 drivers/video/atmel_lcdfb.c      | 48 ++++++++++++++++++++++++++++++----------
 drivers/video/atmel_lcdfb.h      |  8 +++++++
 drivers/video/atmel_lcdfb_core.c | 32 ++++++++++++++++-----------
 3 files changed, 63 insertions(+), 25 deletions(-)

drivers/video/atmel_lcdfb.c +31 -10

index 7c05e857b..d343c5c05 100644
@@ -23,7 +23,6 @@
 #include <init.h>
 #include <mach/hardware.h>
 #include <mach/io.h>
-#include <mach/cpu.h>
 #include <errno.h>
 #include <linux/clk.h>
 

@@ -34,12 +33,12 @@
 #define ATMEL_LCDC_DMA_BURST_LEN	8	/* words */
 #define ATMEL_LCDC_FIFO_SIZE		512	/* words */
 
-static unsigned long compute_hozval(unsigned long xres, unsigned long lcdcon2)
+static unsigned long compute_hozval(struct atmel_lcdfb_info *sinfo,
+				    unsigned long xres, unsigned long lcdcon2)
 {
 	unsigned long value;
 
-	if (!(cpu_is_at91sam9261() || cpu_is_at91sam9g10()
-		|| cpu_is_at32ap7000()))
+	if (!sinfo->have_hozval)
 		return xres;
 
 	value = xres;

@@ -133,7 +132,7 @@ static void atmel_lcdfb_setup_core(struct fb_info *info)
 	lcdc_writel(sinfo, ATMEL_LCDC_DMAFRMCFG, value);
 
 	/* Set pixel clock */
-	if (cpu_is_at91sam9g45() && !cpu_is_at91sam9g45es())
+	if (sinfo->have_alt_pixclock)
 		pix_factor = 1;
 
 	clk_value_khz = clk_get_rate(sinfo->lcdc_clk) / 1000;

@@ -191,7 +190,7 @@ static void atmel_lcdfb_setup_core(struct fb_info *info)
 	lcdc_writel(sinfo, ATMEL_LCDC_TIM2, value);
 
 	/* Horizontal value (aka line size) */
-	hozval_linesz = compute_hozval(mode->xres,
+	hozval_linesz = compute_hozval(sinfo, mode->xres,
 					lcdc_readl(sinfo, ATMEL_LCDC_LCDCON2));
 
 	/* Display size */

@@ -243,14 +242,39 @@ static int atmel_lcdc_probe(struct device_d *dev)
 	return atmel_lcdc_register(dev, &atmel_lcdfb_data);
 }
 
+static struct atmel_lcdfb_config at91sam9261_config = {
+	.have_hozval		= true,
+	.have_intensity_bit	= true,
+};
+
+static struct atmel_lcdfb_config at91sam9263_config = {
+	.have_intensity_bit	= true,
+};
+
+static struct atmel_lcdfb_config at91sam9g10_config = {
+	.have_hozval		= true,
+};
+
+static struct atmel_lcdfb_config at91sam9g45_config = {
+	.have_alt_pixclock	= true,
+};
+
+static struct atmel_lcdfb_config at91sam9rl_config = {
+	.have_intensity_bit	= true,
+};
+
+static struct atmel_lcdfb_config at32ap_config = {
+	.have_hozval		= true,
+};
+
 static __maybe_unused struct of_device_id atmel_lcdfb_compatible[] = {
-	{ .compatible = "atmel,at91sam9261-lcdc", },
-	{ .compatible = "atmel,at91sam9263-lcdc", },
-	{ .compatible = "atmel,at91sam9g10-lcdc", },
-	{ .compatible = "atmel,at91sam9g45-lcdc", },
+	{ .compatible = "atmel,at91sam9261-lcdc",	.data = &at91sam9261_config, },

Quoted 7 lines

drivers/video/atmel_lcdfb.c +2 -4

+
 static __maybe_unused struct of_device_id atmel_lcdfb_compatible[] = {
-	{ .compatible = "atmel,at91sam9261-lcdc", },
-	{ .compatible = "atmel,at91sam9263-lcdc", },
-	{ .compatible = "atmel,at91sam9g10-lcdc", },
-	{ .compatible = "atmel,at91sam9g45-lcdc", },
+	{ .compatible = "atmel,at91sam9261-lcdc",	.data = &at91sam9261_config, },

drivers/video/atmel_lcdfb.c static int atmel_lcdc_probe(struct device_d *dev) · line 279

Quoted 10 lines

drivers/video/atmel_lcdfb.c +5 -2

+	{ .compatible = "atmel,at91sam9263-lcdc",	.data = &at91sam9263_config, },
+	{ .compatible = "atmel,at91sam9g10-lcdc",	.data = &at91sam9g10_config, },
+	{ .compatible = "atmel,at91sam9g45-lcdc",	.data = &at91sam9g45_config, },
 	{ .compatible = "atmel,at91sam9g45es-lcdc", },
-	{ .compatible = "atmel,at91sam9rl-lcdc", },
-	{ .compatible = "atmel,at32ap-lcdc", },
+	{ .compatible = "atmel,at91sam9rl-lcdc",	.data = &at91sam9rl_config, },
+	{ .compatible = "atmel,at32ap-lcdc",		.data = &at32ap_config, },
 	{ /* sentinel */ }
 };

Note that we can match on different platform_device_ids aswell, so you could also attach driver_data to platform devices aswell. For an example see drivers/serial/serial_imx.c.

I leave it up to you if you change this patch accordingly, I can also apply it as is.

Hi Sasha.

Quoted 22 lines

Quoted 7 lines

drivers/video/atmel_lcdfb.c +2 -4

+
 static __maybe_unused struct of_device_id atmel_lcdfb_compatible[] = {
-	{ .compatible = "atmel,at91sam9261-lcdc", },
-	{ .compatible = "atmel,at91sam9263-lcdc", },
-	{ .compatible = "atmel,at91sam9g10-lcdc", },
-	{ .compatible = "atmel,at91sam9g45-lcdc", },
+	{ .compatible = "atmel,at91sam9261-lcdc",	.data = &at91sam9261_config, },

drivers/video/atmel_lcdfb.c static int atmel_lcdc_probe(struct device_d *dev) · line 279

Quoted 10 lines

drivers/video/atmel_lcdfb.c +5 -2

+	{ .compatible = "atmel,at91sam9263-lcdc",	.data = &at91sam9263_config, },
+	{ .compatible = "atmel,at91sam9g10-lcdc",	.data = &at91sam9g10_config, },
+	{ .compatible = "atmel,at91sam9g45-lcdc",	.data = &at91sam9g45_config, },
 	{ .compatible = "atmel,at91sam9g45es-lcdc", },
-	{ .compatible = "atmel,at91sam9rl-lcdc", },
-	{ .compatible = "atmel,at32ap-lcdc", },
+	{ .compatible = "atmel,at91sam9rl-lcdc",	.data = &at91sam9rl_config, },
+	{ .compatible = "atmel,at32ap-lcdc",		.data = &at32ap_config, },
 	{ /* sentinel */ }
 };

Note that we can match on different platform_device_ids aswell, so you could also attach driver_data to platform devices aswell. For an example see drivers/serial/serial_imx.c.

Quoted 2 lines

I leave it up to you if you change this patch accordingly, I can also apply it as is.

I took a look at it today.
With these Atmel CPUs we have different behaviour of each IP block depending on what CPU it is included in which is why I have added all these _config blocks (I could have re-used a few).

So it is not that we see that for example at91sam9rl-lcdc
is used in many CPU's.
Adding the platform_id in this case resulted in extra code with no added benefit - or at least no benefit I could see.
I had to maintain both a of_device_id table and a platform_device_id table, so it also looked like a duplicate.

So unless I miss something then the original patch is the best solution.