From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
To: linux-fbdev@vger.kernel.org
Subject: Re: [PATCH 12/57] arm: mach-shmobile: Add LCDC tx_dev field to
Date: Thu, 15 Dec 2011 14:01:27 +0000 [thread overview]
Message-ID: <Pine.LNX.4.64.1112151453060.32653@axis700.grange> (raw)
In-Reply-To: <1323784972-24205-13-git-send-email-laurent.pinchart@ideasonboard.com>
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/
next prev parent reply other threads:[~2011-12-15 14:01 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-13 14:02 [PATCH 12/57] arm: mach-shmobile: Add LCDC tx_dev field to platform data Laurent Pinchart
2011-12-15 14:01 ` Guennadi Liakhovetski [this message]
2011-12-16 10:04 ` Laurent Pinchart
2011-12-19 0:27 ` [PATCH 12/57] arm: mach-shmobile: Add LCDC tx_dev field to Guennadi Liakhovetski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Pine.LNX.4.64.1112151453060.32653@axis700.grange \
--to=g.liakhovetski@gmx.de \
--cc=linux-fbdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox