[PATCH 12/57] arm: mach-shmobile: Add LCDC tx_dev field to platform data

[PATCH 12/57] arm: mach-shmobile: Add LCDC tx_dev field to platform data

[Laurent Pinchart]

Make sure the transmitter devices get registered before the associated
LCDC devices.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 arch/arm/mach-shmobile/board-ap4evb.c   |  272 ++++++++++++++++---------------
 arch/arm/mach-shmobile/board-mackerel.c |   66 ++++----
 2 files changed, 176 insertions(+), 162 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index dca4860..e0a6b3d 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -445,82 +445,6 @@ static struct platform_device usb1_host_device = {
 	.resource	= usb1_host_resources,
 };
 
-static const struct fb_videomode ap4evb_lcdc_modes[] = {
-	{
-#ifdef CONFIG_AP4EVB_QHD
-		.name		= "R63302(QHD)",
-		.xres		= 544,
-		.yres		= 961,
-		.left_margin	= 72,
-		.right_margin	= 600,
-		.hsync_len	= 16,
-		.upper_margin	= 8,
-		.lower_margin	= 8,
-		.vsync_len	= 2,
-		.sync		= FB_SYNC_VERT_HIGH_ACT | FB_SYNC_HOR_HIGH_ACT,
-#else
-		.name		= "WVGA Panel",
-		.xres		= 800,
-		.yres		= 480,
-		.left_margin	= 220,
-		.right_margin	= 110,
-		.hsync_len	= 70,
-		.upper_margin	= 20,
-		.lower_margin	= 5,
-		.vsync_len	= 5,
-		.sync		= 0,
-#endif
-	},
-};
-static struct sh_mobile_meram_cfg lcd_meram_cfg = {
-	.icb[0] = {
-		.marker_icb     = 28,
-		.cache_icb      = 24,
-		.meram_offset   = 0x0,
-		.meram_size     = 0x40,
-	},
-	.icb[1] = {
-		.marker_icb     = 29,
-		.cache_icb      = 25,
-		.meram_offset   = 0x40,
-		.meram_size     = 0x40,
-	},
-};
-
-static struct sh_mobile_lcdc_info lcdc_info = {
-	.meram_dev = &meram_info,
-	.ch[0] = {
-		.chan = LCDC_CHAN_MAINLCD,
-		.fourcc = V4L2_PIX_FMT_RGB565,
-		.lcd_cfg = ap4evb_lcdc_modes,
-		.num_cfg = ARRAY_SIZE(ap4evb_lcdc_modes),
-		.meram_cfg = &lcd_meram_cfg,
-	}
-};
-
-static struct resource lcdc_resources[] = {
-	[0] = {
-		.name	= "LCDC",
-		.start	= 0xfe940000, /* P4-only space */
-		.end	= 0xfe943fff,
-		.flags	= IORESOURCE_MEM,
-	},
-	[1] = {
-		.start	= intcs_evt2irq(0x580),
-		.flags	= IORESOURCE_IRQ,
-	},
-};
-
-static struct platform_device lcdc_device = {
-	.name		= "sh_mobile_lcdc_fb",
-	.num_resources	= ARRAY_SIZE(lcdc_resources),
-	.resource	= lcdc_resources,
-	.dev	= {
-		.platform_data	= &lcdc_info,
-		.coherent_dma_mask = ~0,
-	},
-};
-
 /*
  * QHD display
  */
@@ -601,6 +525,8 @@ static struct resource mipidsi0_resources[] = {
 	},
 };
 
+static struct sh_mobile_lcdc_info lcdc_info;
+
 static struct sh_mipi_dsi_info mipidsi0_info = {
 	.data_format	= MIPI_RGB888,
 	.lcd_chan	= &lcdc_info.ch[0],
@@ -627,6 +553,86 @@ static struct platform_device *qhd_devices[] __initdata = {
 };
 #endif /* CONFIG_AP4EVB_QHD */
 
+/* LCDC0 */
+static const struct fb_videomode ap4evb_lcdc_modes[] = {
+	{
+#ifdef CONFIG_AP4EVB_QHD
+		.name		= "R63302(QHD)",
+		.xres		= 544,
+		.yres		= 961,
+		.left_margin	= 72,
+		.right_margin	= 600,
+		.hsync_len	= 16,
+		.upper_margin	= 8,
+		.lower_margin	= 8,
+		.vsync_len	= 2,
+		.sync		= FB_SYNC_VERT_HIGH_ACT | FB_SYNC_HOR_HIGH_ACT,
+#else
+		.name		= "WVGA Panel",
+		.xres		= 800,
+		.yres		= 480,
+		.left_margin	= 220,
+		.right_margin	= 110,
+		.hsync_len	= 70,
+		.upper_margin	= 20,
+		.lower_margin	= 5,
+		.vsync_len	= 5,
+		.sync		= 0,
+#endif
+	},
+};
+static struct sh_mobile_meram_cfg lcd_meram_cfg = {
+	.icb[0] = {
+		.marker_icb     = 28,
+		.cache_icb      = 24,
+		.meram_offset   = 0x0,
+		.meram_size     = 0x40,
+	},
+	.icb[1] = {
+		.marker_icb     = 29,
+		.cache_icb      = 25,
+		.meram_offset   = 0x40,
+		.meram_size     = 0x40,
+	},
+};
+
+static struct sh_mobile_lcdc_info lcdc_info = {
+	.meram_dev = &meram_info,
+	.ch[0] = {
+		.chan = LCDC_CHAN_MAINLCD,
+		.fourcc = V4L2_PIX_FMT_RGB565,
+		.lcd_cfg = ap4evb_lcdc_modes,
+		.num_cfg = ARRAY_SIZE(ap4evb_lcdc_modes),
+		.meram_cfg = &lcd_meram_cfg,
+#ifdef CONFIG_AP4EVB_QHD
+		.tx_dev = &mipidsi0_device,
+#endif
+	}
+};
+
+static struct resource lcdc_resources[] = {
+	[0] = {
+		.name	= "LCDC",
+		.start	= 0xfe940000, /* P4-only space */
+		.end	= 0xfe943fff,
+		.flags	= IORESOURCE_MEM,
+	},
+	[1] = {
+		.start	= intcs_evt2irq(0x580),
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device lcdc_device = {
+	.name		= "sh_mobile_lcdc_fb",
+	.num_resources	= ARRAY_SIZE(lcdc_resources),
+	.resource	= lcdc_resources,
+	.dev	= {
+		.platform_data	= &lcdc_info,
+		.coherent_dma_mask = ~0,
+	},
+};
+
 /* FSI */
 #define IRQ_FSI		evt2irq(0x1840)
 static int __fsi_set_rate(struct clk *clk, long rate, int enable)
@@ -793,6 +799,62 @@ static struct platform_device fsi_device = {
 static struct platform_device fsi_ak4643_device = {
 	.name		= "sh_fsi2_a_ak4643",
 };
+
+/* LCDC1 */
+static long ap4evb_clk_optimize(unsigned long target, unsigned long *best_freq,
+				unsigned long *parent_freq);
+
+static struct sh_mobile_lcdc_info sh_mobile_lcdc1_info;
+
+static struct sh_mobile_hdmi_info hdmi_info = {
+	.lcd_chan = &sh_mobile_lcdc1_info.ch[0],
+	.flags = HDMI_SND_SRC_SPDIF,
+	.clk_optimize_parent = ap4evb_clk_optimize,
+};
+
+static struct resource hdmi_resources[] = {
+	[0] = {
+		.name	= "HDMI",
+		.start	= 0xe6be0000,
+		.end	= 0xe6be00ff,
+		.flags	= IORESOURCE_MEM,
+	},
+	[1] = {
+		/* There's also an HDMI interrupt on INTCS @ 0x18e0 */
+		.start	= evt2irq(0x17e0),
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device hdmi_device = {
+	.name		= "sh-mobile-hdmi",
+	.num_resources	= ARRAY_SIZE(hdmi_resources),
+	.resource	= hdmi_resources,
+	.id             = -1,
+	.dev	= {
+		.platform_data	= &hdmi_info,
+	},
+};
+
+static long ap4evb_clk_optimize(unsigned long target, unsigned long *best_freq,
+				unsigned long *parent_freq)
+{
+	struct clk *hdmi_ick = clk_get(&hdmi_device.dev, "ick");
+	long error;
+
+	if (IS_ERR(hdmi_ick)) {
+		int ret = PTR_ERR(hdmi_ick);
+		pr_err("Cannot get HDMI ICK: %d\n", ret);
+		return ret;
+	}
+
+	error = clk_round_parent(hdmi_ick, target, best_freq, parent_freq, 1, 64);
+
+	clk_put(hdmi_ick);
+
+	return error;
+}
+
 static struct sh_mobile_meram_cfg hdmi_meram_cfg = {
 	.icb[0] = {
 		.marker_icb     = 30,
@@ -818,6 +880,7 @@ static struct sh_mobile_lcdc_info sh_mobile_lcdc1_info = {
 		.clock_divider = 1,
 		.flags = LCDC_FLAGS_DWPOL,
 		.meram_cfg = &hdmi_meram_cfg,
+		.tx_dev = &hdmi_device,
 	}
 };
 
@@ -845,63 +908,10 @@ static struct platform_device lcdc1_device = {
 	},
 };
 
-static long ap4evb_clk_optimize(unsigned long target, unsigned long *best_freq,
-				unsigned long *parent_freq);
-
-
-static struct sh_mobile_hdmi_info hdmi_info = {
-	.lcd_chan = &sh_mobile_lcdc1_info.ch[0],
-	.flags = HDMI_SND_SRC_SPDIF,
-	.clk_optimize_parent = ap4evb_clk_optimize,
-};
-
-static struct resource hdmi_resources[] = {
-	[0] = {
-		.name	= "HDMI",
-		.start	= 0xe6be0000,
-		.end	= 0xe6be00ff,
-		.flags	= IORESOURCE_MEM,
-	},
-	[1] = {
-		/* There's also an HDMI interrupt on INTCS @ 0x18e0 */
-		.start	= evt2irq(0x17e0),
-		.flags	= IORESOURCE_IRQ,
-	},
-};
-
-static struct platform_device hdmi_device = {
-	.name		= "sh-mobile-hdmi",
-	.num_resources	= ARRAY_SIZE(hdmi_resources),
-	.resource	= hdmi_resources,
-	.id             = -1,
-	.dev	= {
-		.platform_data	= &hdmi_info,
-	},
-};
-
 static struct platform_device fsi_hdmi_device = {
 	.name		= "sh_fsi2_b_hdmi",
 };
 
-static long ap4evb_clk_optimize(unsigned long target, unsigned long *best_freq,
-				unsigned long *parent_freq)
-{
-	struct clk *hdmi_ick = clk_get(&hdmi_device.dev, "ick");
-	long error;
-
-	if (IS_ERR(hdmi_ick)) {
-		int ret = PTR_ERR(hdmi_ick);
-		pr_err("Cannot get HDMI ICK: %d\n", ret);
-		return ret;
-	}
-
-	error = clk_round_parent(hdmi_ick, target, best_freq, parent_freq, 1, 64);
-
-	clk_put(hdmi_ick);
-
-	return error;
-}
-
 static struct gpio_led ap4evb_leds[] = {
 	{
 		.name			= "led4",
@@ -1036,9 +1046,9 @@ static struct platform_device *ap4evb_devices[] __initdata = {
 	&fsi_ak4643_device,
 	&fsi_hdmi_device,
 	&sh_mmcif_device,
-	&lcdc1_device,
-	&lcdc_device,
 	&hdmi_device,
+	&lcdc_device,
+	&lcdc1_device,
 	&ceu_device,
 	&ap4evb_camera,
 	&meram_device,
diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 4ed0138..ccdd9fc 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -431,6 +431,38 @@ static struct platform_device lcdc_device = {
 	},
 };
 
+/* HDMI */
+static struct sh_mobile_lcdc_info hdmi_lcdc_info;
+
+static struct sh_mobile_hdmi_info hdmi_info = {
+	.lcd_chan	= &hdmi_lcdc_info.ch[0],
+	.flags		= HDMI_SND_SRC_SPDIF,
+};
+
+static struct resource hdmi_resources[] = {
+	[0] = {
+		.name	= "HDMI",
+		.start	= 0xe6be0000,
+		.end	= 0xe6be00ff,
+		.flags	= IORESOURCE_MEM,
+	},
+	[1] = {
+		/* There's also an HDMI interrupt on INTCS @ 0x18e0 */
+		.start	= evt2irq(0x17e0),
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device hdmi_device = {
+	.name		= "sh-mobile-hdmi",
+	.num_resources	= ARRAY_SIZE(hdmi_resources),
+	.resource	= hdmi_resources,
+	.id             = -1,
+	.dev	= {
+		.platform_data	= &hdmi_info,
+	},
+};
+
 static struct sh_mobile_meram_cfg hdmi_meram_cfg = {
 	.icb[0] = {
 		.marker_icb     = 30,
@@ -445,7 +477,7 @@ static struct sh_mobile_meram_cfg hdmi_meram_cfg = {
 		.meram_size     = 0x100,
 	},
 };
-/* HDMI */
+
 static struct sh_mobile_lcdc_info hdmi_lcdc_info = {
 	.meram_dev = &mackerel_meram_info,
 	.clock_source = LCDC_CLK_EXTERNAL,
@@ -456,6 +488,7 @@ static struct sh_mobile_lcdc_info hdmi_lcdc_info = {
 		.clock_divider = 1,
 		.flags = LCDC_FLAGS_DWPOL,
 		.meram_cfg = &hdmi_meram_cfg,
+		.tx_dev = &hdmi_device,
 	}
 };
 
@@ -483,35 +516,6 @@ static struct platform_device hdmi_lcdc_device = {
 	},
 };
 
-static struct sh_mobile_hdmi_info hdmi_info = {
-	.lcd_chan	= &hdmi_lcdc_info.ch[0],
-	.flags		= HDMI_SND_SRC_SPDIF,
-};
-
-static struct resource hdmi_resources[] = {
-	[0] = {
-		.name	= "HDMI",
-		.start	= 0xe6be0000,
-		.end	= 0xe6be00ff,
-		.flags	= IORESOURCE_MEM,
-	},
-	[1] = {
-		/* There's also an HDMI interrupt on INTCS @ 0x18e0 */
-		.start	= evt2irq(0x17e0),
-		.flags	= IORESOURCE_IRQ,
-	},
-};
-
-static struct platform_device hdmi_device = {
-	.name		= "sh-mobile-hdmi",
-	.num_resources	= ARRAY_SIZE(hdmi_resources),
-	.resource	= hdmi_resources,
-	.id             = -1,
-	.dev	= {
-		.platform_data	= &hdmi_info,
-	},
-};
-
 static struct platform_device fsi_hdmi_device = {
 	.name		= "sh_fsi2_b_hdmi",
 };
@@ -1313,8 +1317,8 @@ static struct platform_device *mackerel_devices[] __initdata = {
 	&sh_mmcif_device,
 	&ceu_device,
 	&mackerel_camera,
-	&hdmi_lcdc_device,
 	&hdmi_device,
+	&hdmi_lcdc_device,
 	&meram_device,
 };
 
-- 
1.7.3.4

Re: [PATCH 12/57] arm: mach-shmobile: Add LCDC tx_dev field to

[Guennadi Liakhovetski]

Hi Laurent

On Tue, 13 Dec 2011, Laurent Pinchart wrote:

> Make sure the transmitter devices get registered before the associated
> LCDC devices.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  arch/arm/mach-shmobile/board-ap4evb.c   |  272 ++++++++++++++++---------------
>  arch/arm/mach-shmobile/board-mackerel.c |   66 ++++----
>  2 files changed, 176 insertions(+), 162 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
> index dca4860..e0a6b3d 100644
> --- a/arch/arm/mach-shmobile/board-ap4evb.c
> +++ b/arch/arm/mach-shmobile/board-ap4evb.c
> @@ -445,82 +445,6 @@ static struct platform_device usb1_host_device = {
>  	.resource	= usb1_host_resources,
>  };
>  
> -static const struct fb_videomode ap4evb_lcdc_modes[] = {
> -	{
> -#ifdef CONFIG_AP4EVB_QHD
> -		.name		= "R63302(QHD)",
> -		.xres		= 544,
> -		.yres		= 961,
> -		.left_margin	= 72,
> -		.right_margin	= 600,
> -		.hsync_len	= 16,
> -		.upper_margin	= 8,
> -		.lower_margin	= 8,
> -		.vsync_len	= 2,
> -		.sync		= FB_SYNC_VERT_HIGH_ACT | FB_SYNC_HOR_HIGH_ACT,
> -#else
> -		.name		= "WVGA Panel",
> -		.xres		= 800,
> -		.yres		= 480,
> -		.left_margin	= 220,
> -		.right_margin	= 110,
> -		.hsync_len	= 70,
> -		.upper_margin	= 20,
> -		.lower_margin	= 5,
> -		.vsync_len	= 5,
> -		.sync		= 0,
> -#endif
> -	},
> -};
> -static struct sh_mobile_meram_cfg lcd_meram_cfg = {
> -	.icb[0] = {
> -		.marker_icb     = 28,
> -		.cache_icb      = 24,
> -		.meram_offset   = 0x0,
> -		.meram_size     = 0x40,
> -	},
> -	.icb[1] = {
> -		.marker_icb     = 29,
> -		.cache_icb      = 25,
> -		.meram_offset   = 0x40,
> -		.meram_size     = 0x40,
> -	},
> -};
> -
> -static struct sh_mobile_lcdc_info lcdc_info = {
> -	.meram_dev = &meram_info,
> -	.ch[0] = {
> -		.chan = LCDC_CHAN_MAINLCD,
> -		.fourcc = V4L2_PIX_FMT_RGB565,
> -		.lcd_cfg = ap4evb_lcdc_modes,
> -		.num_cfg = ARRAY_SIZE(ap4evb_lcdc_modes),
> -		.meram_cfg = &lcd_meram_cfg,
> -	}
> -};
> -
> -static struct resource lcdc_resources[] = {
> -	[0] = {
> -		.name	= "LCDC",
> -		.start	= 0xfe940000, /* P4-only space */
> -		.end	= 0xfe943fff,
> -		.flags	= IORESOURCE_MEM,
> -	},
> -	[1] = {
> -		.start	= intcs_evt2irq(0x580),
> -		.flags	= IORESOURCE_IRQ,
> -	},
> -};
> -
> -static struct platform_device lcdc_device = {
> -	.name		= "sh_mobile_lcdc_fb",
> -	.num_resources	= ARRAY_SIZE(lcdc_resources),
> -	.resource	= lcdc_resources,
> -	.dev	= {
> -		.platform_data	= &lcdc_info,
> -		.coherent_dma_mask = ~0,
> -	},
> -};
> -
>  /*
>   * QHD display
>   */
> @@ -601,6 +525,8 @@ static struct resource mipidsi0_resources[] = {
>  	},
>  };
>  
> +static struct sh_mobile_lcdc_info lcdc_info;
> +
>  static struct sh_mipi_dsi_info mipidsi0_info = {
>  	.data_format	= MIPI_RGB888,
>  	.lcd_chan	= &lcdc_info.ch[0],
> @@ -627,6 +553,86 @@ static struct platform_device *qhd_devices[] __initdata = {
>  };
>  #endif /* CONFIG_AP4EVB_QHD */
>  
> +/* LCDC0 */
> +static const struct fb_videomode ap4evb_lcdc_modes[] = {

Hm, I must be missing something. I thought you were moving all the structs 
around to make them available for reference _without_ forward 
declarations. But you _do_ end up using a forward declaration anyway. Here 
and for HDMI below too. So, what's the point? Why not just forward declare 
that one struct, that's needed and leave the rest where it currently is 
in the file?

> +	{
> +#ifdef CONFIG_AP4EVB_QHD
> +		.name		= "R63302(QHD)",
> +		.xres		= 544,
> +		.yres		= 961,
> +		.left_margin	= 72,
> +		.right_margin	= 600,
> +		.hsync_len	= 16,
> +		.upper_margin	= 8,
> +		.lower_margin	= 8,
> +		.vsync_len	= 2,
> +		.sync		= FB_SYNC_VERT_HIGH_ACT | FB_SYNC_HOR_HIGH_ACT,
> +#else
> +		.name		= "WVGA Panel",
> +		.xres		= 800,
> +		.yres		= 480,
> +		.left_margin	= 220,
> +		.right_margin	= 110,
> +		.hsync_len	= 70,
> +		.upper_margin	= 20,
> +		.lower_margin	= 5,
> +		.vsync_len	= 5,
> +		.sync		= 0,
> +#endif
> +	},
> +};
> +static struct sh_mobile_meram_cfg lcd_meram_cfg = {
> +	.icb[0] = {
> +		.marker_icb     = 28,
> +		.cache_icb      = 24,
> +		.meram_offset   = 0x0,
> +		.meram_size     = 0x40,
> +	},
> +	.icb[1] = {
> +		.marker_icb     = 29,
> +		.cache_icb      = 25,
> +		.meram_offset   = 0x40,
> +		.meram_size     = 0x40,
> +	},
> +};
> +
> +static struct sh_mobile_lcdc_info lcdc_info = {
> +	.meram_dev = &meram_info,
> +	.ch[0] = {
> +		.chan = LCDC_CHAN_MAINLCD,
> +		.fourcc = V4L2_PIX_FMT_RGB565,
> +		.lcd_cfg = ap4evb_lcdc_modes,
> +		.num_cfg = ARRAY_SIZE(ap4evb_lcdc_modes),
> +		.meram_cfg = &lcd_meram_cfg,
> +#ifdef CONFIG_AP4EVB_QHD
> +		.tx_dev = &mipidsi0_device,
> +#endif
> +	}
> +};
> +
> +static struct resource lcdc_resources[] = {
> +	[0] = {
> +		.name	= "LCDC",
> +		.start	= 0xfe940000, /* P4-only space */
> +		.end	= 0xfe943fff,
> +		.flags	= IORESOURCE_MEM,
> +	},
> +	[1] = {
> +		.start	= intcs_evt2irq(0x580),
> +		.flags	= IORESOURCE_IRQ,
> +	},
> +};
> +
> +static struct platform_device lcdc_device = {
> +	.name		= "sh_mobile_lcdc_fb",
> +	.num_resources	= ARRAY_SIZE(lcdc_resources),
> +	.resource	= lcdc_resources,
> +	.dev	= {
> +		.platform_data	= &lcdc_info,
> +		.coherent_dma_mask = ~0,
> +	},
> +};
> +
>  /* FSI */
>  #define IRQ_FSI		evt2irq(0x1840)
>  static int __fsi_set_rate(struct clk *clk, long rate, int enable)
> @@ -793,6 +799,62 @@ static struct platform_device fsi_device = {
>  static struct platform_device fsi_ak4643_device = {
>  	.name		= "sh_fsi2_a_ak4643",
>  };
> +
> +/* LCDC1 */
> +static long ap4evb_clk_optimize(unsigned long target, unsigned long *best_freq,
> +				unsigned long *parent_freq);
> +
> +static struct sh_mobile_lcdc_info sh_mobile_lcdc1_info;
> +
> +static struct sh_mobile_hdmi_info hdmi_info = {
> +	.lcd_chan = &sh_mobile_lcdc1_info.ch[0],
> +	.flags = HDMI_SND_SRC_SPDIF,
> +	.clk_optimize_parent = ap4evb_clk_optimize,
> +};
> +
> +static struct resource hdmi_resources[] = {
> +	[0] = {
> +		.name	= "HDMI",
> +		.start	= 0xe6be0000,
> +		.end	= 0xe6be00ff,
> +		.flags	= IORESOURCE_MEM,
> +	},
> +	[1] = {
> +		/* There's also an HDMI interrupt on INTCS @ 0x18e0 */
> +		.start	= evt2irq(0x17e0),
> +		.flags	= IORESOURCE_IRQ,
> +	},
> +};
> +
> +static struct platform_device hdmi_device = {
> +	.name		= "sh-mobile-hdmi",
> +	.num_resources	= ARRAY_SIZE(hdmi_resources),
> +	.resource	= hdmi_resources,
> +	.id             = -1,
> +	.dev	= {
> +		.platform_data	= &hdmi_info,
> +	},
> +};
> +
> +static long ap4evb_clk_optimize(unsigned long target, unsigned long *best_freq,
> +				unsigned long *parent_freq)
> +{
> +	struct clk *hdmi_ick = clk_get(&hdmi_device.dev, "ick");
> +	long error;
> +
> +	if (IS_ERR(hdmi_ick)) {
> +		int ret = PTR_ERR(hdmi_ick);
> +		pr_err("Cannot get HDMI ICK: %d\n", ret);
> +		return ret;
> +	}
> +
> +	error = clk_round_parent(hdmi_ick, target, best_freq, parent_freq, 1, 64);
> +
> +	clk_put(hdmi_ick);
> +
> +	return error;
> +}
> +
>  static struct sh_mobile_meram_cfg hdmi_meram_cfg = {
>  	.icb[0] = {
>  		.marker_icb     = 30,
> @@ -818,6 +880,7 @@ static struct sh_mobile_lcdc_info sh_mobile_lcdc1_info = {
>  		.clock_divider = 1,
>  		.flags = LCDC_FLAGS_DWPOL,
>  		.meram_cfg = &hdmi_meram_cfg,
> +		.tx_dev = &hdmi_device,
>  	}
>  };
>  
> @@ -845,63 +908,10 @@ static struct platform_device lcdc1_device = {
>  	},
>  };
>  
> -static long ap4evb_clk_optimize(unsigned long target, unsigned long *best_freq,
> -				unsigned long *parent_freq);
> -
> -
> -static struct sh_mobile_hdmi_info hdmi_info = {
> -	.lcd_chan = &sh_mobile_lcdc1_info.ch[0],
> -	.flags = HDMI_SND_SRC_SPDIF,
> -	.clk_optimize_parent = ap4evb_clk_optimize,
> -};
> -
> -static struct resource hdmi_resources[] = {
> -	[0] = {
> -		.name	= "HDMI",
> -		.start	= 0xe6be0000,
> -		.end	= 0xe6be00ff,
> -		.flags	= IORESOURCE_MEM,
> -	},
> -	[1] = {
> -		/* There's also an HDMI interrupt on INTCS @ 0x18e0 */
> -		.start	= evt2irq(0x17e0),
> -		.flags	= IORESOURCE_IRQ,
> -	},
> -};
> -
> -static struct platform_device hdmi_device = {
> -	.name		= "sh-mobile-hdmi",
> -	.num_resources	= ARRAY_SIZE(hdmi_resources),
> -	.resource	= hdmi_resources,
> -	.id             = -1,
> -	.dev	= {
> -		.platform_data	= &hdmi_info,
> -	},
> -};
> -
>  static struct platform_device fsi_hdmi_device = {
>  	.name		= "sh_fsi2_b_hdmi",
>  };
>  
> -static long ap4evb_clk_optimize(unsigned long target, unsigned long *best_freq,
> -				unsigned long *parent_freq)
> -{
> -	struct clk *hdmi_ick = clk_get(&hdmi_device.dev, "ick");
> -	long error;
> -
> -	if (IS_ERR(hdmi_ick)) {
> -		int ret = PTR_ERR(hdmi_ick);
> -		pr_err("Cannot get HDMI ICK: %d\n", ret);
> -		return ret;
> -	}
> -
> -	error = clk_round_parent(hdmi_ick, target, best_freq, parent_freq, 1, 64);
> -
> -	clk_put(hdmi_ick);
> -
> -	return error;
> -}
> -
>  static struct gpio_led ap4evb_leds[] = {
>  	{
>  		.name			= "led4",
> @@ -1036,9 +1046,9 @@ static struct platform_device *ap4evb_devices[] __initdata = {
>  	&fsi_ak4643_device,
>  	&fsi_hdmi_device,
>  	&sh_mmcif_device,
> -	&lcdc1_device,
> -	&lcdc_device,
>  	&hdmi_device,
> +	&lcdc_device,
> +	&lcdc1_device,
>  	&ceu_device,
>  	&ap4evb_camera,
>  	&meram_device,
> diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
> index 4ed0138..ccdd9fc 100644
> --- a/arch/arm/mach-shmobile/board-mackerel.c
> +++ b/arch/arm/mach-shmobile/board-mackerel.c
> @@ -431,6 +431,38 @@ static struct platform_device lcdc_device = {
>  	},
>  };
>  
> +/* HDMI */
> +static struct sh_mobile_lcdc_info hdmi_lcdc_info;
> +
> +static struct sh_mobile_hdmi_info hdmi_info = {
> +	.lcd_chan	= &hdmi_lcdc_info.ch[0],
> +	.flags		= HDMI_SND_SRC_SPDIF,
> +};
> +
> +static struct resource hdmi_resources[] = {
> +	[0] = {
> +		.name	= "HDMI",
> +		.start	= 0xe6be0000,
> +		.end	= 0xe6be00ff,
> +		.flags	= IORESOURCE_MEM,
> +	},
> +	[1] = {
> +		/* There's also an HDMI interrupt on INTCS @ 0x18e0 */
> +		.start	= evt2irq(0x17e0),
> +		.flags	= IORESOURCE_IRQ,
> +	},
> +};
> +
> +static struct platform_device hdmi_device = {
> +	.name		= "sh-mobile-hdmi",
> +	.num_resources	= ARRAY_SIZE(hdmi_resources),
> +	.resource	= hdmi_resources,
> +	.id             = -1,
> +	.dev	= {
> +		.platform_data	= &hdmi_info,
> +	},
> +};
> +
>  static struct sh_mobile_meram_cfg hdmi_meram_cfg = {
>  	.icb[0] = {
>  		.marker_icb     = 30,
> @@ -445,7 +477,7 @@ static struct sh_mobile_meram_cfg hdmi_meram_cfg = {
>  		.meram_size     = 0x100,
>  	},
>  };
> -/* HDMI */
> +
>  static struct sh_mobile_lcdc_info hdmi_lcdc_info = {
>  	.meram_dev = &mackerel_meram_info,
>  	.clock_source = LCDC_CLK_EXTERNAL,
> @@ -456,6 +488,7 @@ static struct sh_mobile_lcdc_info hdmi_lcdc_info = {
>  		.clock_divider = 1,
>  		.flags = LCDC_FLAGS_DWPOL,
>  		.meram_cfg = &hdmi_meram_cfg,
> +		.tx_dev = &hdmi_device,
>  	}
>  };
>  
> @@ -483,35 +516,6 @@ static struct platform_device hdmi_lcdc_device = {
>  	},
>  };
>  
> -static struct sh_mobile_hdmi_info hdmi_info = {
> -	.lcd_chan	= &hdmi_lcdc_info.ch[0],
> -	.flags		= HDMI_SND_SRC_SPDIF,
> -};
> -
> -static struct resource hdmi_resources[] = {
> -	[0] = {
> -		.name	= "HDMI",
> -		.start	= 0xe6be0000,
> -		.end	= 0xe6be00ff,
> -		.flags	= IORESOURCE_MEM,
> -	},
> -	[1] = {
> -		/* There's also an HDMI interrupt on INTCS @ 0x18e0 */
> -		.start	= evt2irq(0x17e0),
> -		.flags	= IORESOURCE_IRQ,
> -	},
> -};
> -
> -static struct platform_device hdmi_device = {
> -	.name		= "sh-mobile-hdmi",
> -	.num_resources	= ARRAY_SIZE(hdmi_resources),
> -	.resource	= hdmi_resources,
> -	.id             = -1,
> -	.dev	= {
> -		.platform_data	= &hdmi_info,
> -	},
> -};
> -
>  static struct platform_device fsi_hdmi_device = {
>  	.name		= "sh_fsi2_b_hdmi",
>  };
> @@ -1313,8 +1317,8 @@ static struct platform_device *mackerel_devices[] __initdata = {
>  	&sh_mmcif_device,
>  	&ceu_device,
>  	&mackerel_camera,
> -	&hdmi_lcdc_device,
>  	&hdmi_device,
> +	&hdmi_lcdc_device,
>  	&meram_device,
>  };
>  

Sorry, could you elaborate, why this is needed? If this is really 
important, then I'd prefer to test this before committing. If OTOH this is 
unimportant, and my assumption, that normally it's the platform driver 
registration order, that's important, not device registration order, is 
correct, then you don't have to swap the order?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 12/57] arm: mach-shmobile: Add LCDC tx_dev field to platform data

[Laurent Pinchart]

Hi Guennadi,

On Thursday 15 December 2011 15:01:27 Guennadi Liakhovetski wrote:
> On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > Make sure the transmitter devices get registered before the associated
> > LCDC devices.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

[snip]

> > diff --git a/arch/arm/mach-shmobile/board-ap4evb.c
> > b/arch/arm/mach-shmobile/board-ap4evb.c index dca4860..e0a6b3d 100644
> > --- a/arch/arm/mach-shmobile/board-ap4evb.c
> > +++ b/arch/arm/mach-shmobile/board-ap4evb.c

[snip]

> > @@ -627,6 +553,86 @@ static struct platform_device *qhd_devices[]
> > __initdata = {
> > 
> >  };
> >  #endif /* CONFIG_AP4EVB_QHD */
> > 
> > +/* LCDC0 */
> > +static const struct fb_videomode ap4evb_lcdc_modes[] = {
> 
> Hm, I must be missing something. I thought you were moving all the structs
> around to make them available for reference _without_ forward
> declarations. But you _do_ end up using a forward declaration anyway. Here
> and for HDMI below too. So, what's the point? Why not just forward declare
> that one struct, that's needed and leave the rest where it currently is
> in the file?

As we have circular dependencies forward declaration can't be avoided. I found 
it cleaner to have devices data in registration order in the board file, but I 
can remove this if you prefer.

> > +	{
> > +#ifdef CONFIG_AP4EVB_QHD
> > +		.name		= "R63302(QHD)",
> > +		.xres		= 544,
> > +		.yres		= 961,
> > +		.left_margin	= 72,
> > +		.right_margin	= 600,
> > +		.hsync_len	= 16,
> > +		.upper_margin	= 8,
> > +		.lower_margin	= 8,
> > +		.vsync_len	= 2,
> > +		.sync		= FB_SYNC_VERT_HIGH_ACT | FB_SYNC_HOR_HIGH_ACT,
> > +#else
> > +		.name		= "WVGA Panel",
> > +		.xres		= 800,
> > +		.yres		= 480,
> > +		.left_margin	= 220,
> > +		.right_margin	= 110,
> > +		.hsync_len	= 70,
> > +		.upper_margin	= 20,
> > +		.lower_margin	= 5,
> > +		.vsync_len	= 5,
> > +		.sync		= 0,
> > +#endif
> > +	},
> > +};

[snip]

> > diff --git a/arch/arm/mach-shmobile/board-mackerel.c
> > b/arch/arm/mach-shmobile/board-mackerel.c index 4ed0138..ccdd9fc 100644
> > --- a/arch/arm/mach-shmobile/board-mackerel.c
> > +++ b/arch/arm/mach-shmobile/board-mackerel.c

[snip]

> > @@ -1313,8 +1317,8 @@ static struct platform_device *mackerel_devices[]
> > __initdata = {
> >  	&sh_mmcif_device,
> >  	&ceu_device,
> >  	&mackerel_camera,
> > -	&hdmi_lcdc_device,
> >  	&hdmi_device,
> > +	&hdmi_lcdc_device,
> >  	&meram_device,
> >  };
> 
> Sorry, could you elaborate, why this is needed? If this is really
> important, then I'd prefer to test this before committing. If OTOH this is
> unimportant, and my assumption, that normally it's the platform driver
> registration order, that's important, not device registration order, is
> correct, then you don't have to swap the order?

If both the transmitter and LCDC drivers are built-in, the device probe order 
is the device registration order. As we need the transmitter to be probed 
first, it needs to be registered first.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 12/57] arm: mach-shmobile: Add LCDC tx_dev field to

[Guennadi Liakhovetski]

On Fri, 16 Dec 2011, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Thursday 15 December 2011 15:01:27 Guennadi Liakhovetski wrote:
> > On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > > Make sure the transmitter devices get registered before the associated
> > > LCDC devices.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> [snip]
> 
> > > diff --git a/arch/arm/mach-shmobile/board-ap4evb.c
> > > b/arch/arm/mach-shmobile/board-ap4evb.c index dca4860..e0a6b3d 100644
> > > --- a/arch/arm/mach-shmobile/board-ap4evb.c
> > > +++ b/arch/arm/mach-shmobile/board-ap4evb.c
> 
> [snip]
> 
> > > @@ -627,6 +553,86 @@ static struct platform_device *qhd_devices[]
> > > __initdata = {
> > > 
> > >  };
> > >  #endif /* CONFIG_AP4EVB_QHD */
> > > 
> > > +/* LCDC0 */
> > > +static const struct fb_videomode ap4evb_lcdc_modes[] = {
> > 
> > Hm, I must be missing something. I thought you were moving all the structs
> > around to make them available for reference _without_ forward
> > declarations. But you _do_ end up using a forward declaration anyway. Here
> > and for HDMI below too. So, what's the point? Why not just forward declare
> > that one struct, that's needed and leave the rest where it currently is
> > in the file?
> 
> As we have circular dependencies forward declaration can't be avoided. I found 
> it cleaner to have devices data in registration order in the board file, but I 
> can remove this if you prefer.

As a _general_ guidline I find smaller patches more pleasant than 
bigger;-) but if you really see an important advantage in moving these 
structs around - I can live with that too:-)

> > > +	{
> > > +#ifdef CONFIG_AP4EVB_QHD
> > > +		.name		= "R63302(QHD)",
> > > +		.xres		= 544,
> > > +		.yres		= 961,
> > > +		.left_margin	= 72,
> > > +		.right_margin	= 600,
> > > +		.hsync_len	= 16,
> > > +		.upper_margin	= 8,
> > > +		.lower_margin	= 8,
> > > +		.vsync_len	= 2,
> > > +		.sync		= FB_SYNC_VERT_HIGH_ACT | FB_SYNC_HOR_HIGH_ACT,
> > > +#else
> > > +		.name		= "WVGA Panel",
> > > +		.xres		= 800,
> > > +		.yres		= 480,
> > > +		.left_margin	= 220,
> > > +		.right_margin	= 110,
> > > +		.hsync_len	= 70,
> > > +		.upper_margin	= 20,
> > > +		.lower_margin	= 5,
> > > +		.vsync_len	= 5,
> > > +		.sync		= 0,
> > > +#endif
> > > +	},
> > > +};
> 
> [snip]
> 
> > > diff --git a/arch/arm/mach-shmobile/board-mackerel.c
> > > b/arch/arm/mach-shmobile/board-mackerel.c index 4ed0138..ccdd9fc 100644
> > > --- a/arch/arm/mach-shmobile/board-mackerel.c
> > > +++ b/arch/arm/mach-shmobile/board-mackerel.c
> 
> [snip]
> 
> > > @@ -1313,8 +1317,8 @@ static struct platform_device *mackerel_devices[]
> > > __initdata = {
> > >  	&sh_mmcif_device,
> > >  	&ceu_device,
> > >  	&mackerel_camera,
> > > -	&hdmi_lcdc_device,
> > >  	&hdmi_device,
> > > +	&hdmi_lcdc_device,
> > >  	&meram_device,
> > >  };
> > 
> > Sorry, could you elaborate, why this is needed? If this is really
> > important, then I'd prefer to test this before committing. If OTOH this is
> > unimportant, and my assumption, that normally it's the platform driver
> > registration order, that's important, not device registration order, is
> > correct, then you don't have to swap the order?
> 
> If both the transmitter and LCDC drivers are built-in, the device probe order 
> is the device registration order. As we need the transmitter to be probed 
> first, it needs to be registered first.

Even there I'm not quite sure about - I do remember having to fiddle with 
the Makefiles to change the probe order, which followed the linkage order. 
Could be, that this has changed since I last checked, but I'd really 
double-check this.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/