* [RFC PATCH 0/3] amba-clcd: add device tree support @ 2012-09-19 16:04 Ryan Harkin 2012-09-19 16:04 ` [RFC PATCH 1/3] amba-clcd: Add Device Tree support to amba-clcd driver Ryan Harkin ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Ryan Harkin @ 2012-09-19 16:04 UTC (permalink / raw) To: ryan.harkin, arnd.bergmann, shiraz.hashim, stigge, pawel.moll, tixy, liviu.dudau, spear-devel, viresh.linux, linux-arm-kernel, linux-fbdev, devicetree-discuss Hi all, The first patch adds device tree support to the amba-clcd video driver. The next two modify the Versatile Express platform to use device tree for CLCD. I've tested this code on Versatile Express A5, A9, A15-TC1, A15-A7-TC2 Core Tiles and on RTSM. I'd appreciate it if users of this driver and vexpress could please review these patches and provide feedback. I'd like to get them into shape so that I can upstream in a format suitable for all users of the driver. Regards, Ryan. Ryan Harkin (3): amba-clcd: Add Device Tree support to amba-clcd driver ARM: vexpress: Add device tree support for CLCD driver ARM: vexpress: configure CLCD driver device tree support for A9 CoreTile arch/arm/boot/dts/clcd-panels.dtsi | 52 +++++++ arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 8 +- arch/arm/boot/dts/vexpress-v2m.dtsi | 8 +- arch/arm/boot/dts/vexpress-v2p-ca9.dts | 6 + arch/arm/mach-vexpress/v2m.c | 58 +++++++ drivers/video/amba-clcd.c | 253 +++++++++++++++++++++++++++++++ 6 files changed, 373 insertions(+), 12 deletions(-) create mode 100644 arch/arm/boot/dts/clcd-panels.dtsi -- 1.7.9.5 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC PATCH 1/3] amba-clcd: Add Device Tree support to amba-clcd driver 2012-09-19 16:04 [RFC PATCH 0/3] amba-clcd: add device tree support Ryan Harkin @ 2012-09-19 16:04 ` Ryan Harkin 2012-09-20 10:24 ` Liviu Dudau ` (2 more replies) 2012-09-19 16:04 ` [RFC PATCH 2/3] ARM: vexpress: Add device tree support for CLCD driver Ryan Harkin 2012-09-19 16:04 ` [RFC PATCH 3/3] ARM: vexpress: configure CLCD driver device tree support for A9 CoreTile Ryan Harkin 2 siblings, 3 replies; 14+ messages in thread From: Ryan Harkin @ 2012-09-19 16:04 UTC (permalink / raw) To: ryan.harkin, arnd.bergmann, shiraz.hashim, stigge, pawel.moll, tixy, liviu.dudau, spear-devel, viresh.linux, linux-arm-kernel, linux-fbdev, devicetree-discuss Add support to parse the display configuration from device tree. If the board does not provide platform specific functions in the struct clcd_board contained with the amba device info, then defaults are provided by the driver. The device tree configuration can either ask for a DMA setup or provide a framebuffer address to be remapped into the driver. Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org> --- drivers/video/amba-clcd.c | 253 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 253 insertions(+) diff --git a/drivers/video/amba-clcd.c b/drivers/video/amba-clcd.c index 0a2cce7..01dbad1 100644 --- a/drivers/video/amba-clcd.c +++ b/drivers/video/amba-clcd.c @@ -16,7 +16,10 @@ #include <linux/string.h> #include <linux/slab.h> #include <linux/delay.h> +#include <linux/dma-mapping.h> +#include <linux/memblock.h> #include <linux/mm.h> +#include <linux/of.h> #include <linux/fb.h> #include <linux/init.h> #include <linux/ioport.h> @@ -391,6 +394,19 @@ static int clcdfb_blank(int blank_mode, struct fb_info *info) } return 0; } +int clcdfb_mmap_dma(struct clcd_fb *fb, struct vm_area_struct *vma) +{ + return dma_mmap_writecombine(&fb->dev->dev, vma, + fb->fb.screen_base, + fb->fb.fix.smem_start, + fb->fb.fix.smem_len); +} + +void clcdfb_remove_dma(struct clcd_fb *fb) +{ + dma_free_writecombine(&fb->dev->dev, fb->fb.fix.smem_len, + fb->fb.screen_base, fb->fb.fix.smem_start); +} static int clcdfb_mmap(struct fb_info *info, struct vm_area_struct *vma) @@ -542,12 +558,249 @@ static int clcdfb_register(struct clcd_fb *fb) return ret; } +struct string_lookup { + const char *string; + const u32 val; +}; + +static struct string_lookup vmode_lookups[] = { + { "FB_VMODE_NONINTERLACED", FB_VMODE_NONINTERLACED}, + { "FB_VMODE_INTERLACED", FB_VMODE_INTERLACED}, + { "FB_VMODE_DOUBLE", FB_VMODE_DOUBLE}, + { "FB_VMODE_ODD_FLD_FIRST", FB_VMODE_ODD_FLD_FIRST}, + { NULL, 0 }, +}; + +static struct string_lookup tim2_lookups[] = { + { "TIM2_CLKSEL", TIM2_CLKSEL}, + { "TIM2_IVS", TIM2_IVS}, + { "TIM2_IHS", TIM2_IHS}, + { "TIM2_IPC", TIM2_IPC}, + { "TIM2_IOE", TIM2_IOE}, + { "TIM2_BCD", TIM2_BCD}, + { NULL, 0}, +}; +static struct string_lookup cntl_lookups[] = { + {"CNTL_LCDEN", CNTL_LCDEN}, + {"CNTL_LCDBPP1", CNTL_LCDBPP1}, + {"CNTL_LCDBPP2", CNTL_LCDBPP2}, + {"CNTL_LCDBPP4", CNTL_LCDBPP4}, + {"CNTL_LCDBPP8", CNTL_LCDBPP8}, + {"CNTL_LCDBPP16", CNTL_LCDBPP16}, + {"CNTL_LCDBPP16_565", CNTL_LCDBPP16_565}, + {"CNTL_LCDBPP16_444", CNTL_LCDBPP16_444}, + {"CNTL_LCDBPP24", CNTL_LCDBPP24}, + {"CNTL_LCDBW", CNTL_LCDBW}, + {"CNTL_LCDTFT", CNTL_LCDTFT}, + {"CNTL_LCDMONO8", CNTL_LCDMONO8}, + {"CNTL_LCDDUAL", CNTL_LCDDUAL}, + {"CNTL_BGR", CNTL_BGR}, + {"CNTL_BEBO", CNTL_BEBO}, + {"CNTL_BEPO", CNTL_BEPO}, + {"CNTL_LCDPWR", CNTL_LCDPWR}, + {"CNTL_LCDVCOMP(1)", CNTL_LCDVCOMP(1)}, + {"CNTL_LCDVCOMP(2)", CNTL_LCDVCOMP(2)}, + {"CNTL_LCDVCOMP(3)", CNTL_LCDVCOMP(3)}, + {"CNTL_LCDVCOMP(4)", CNTL_LCDVCOMP(4)}, + {"CNTL_LCDVCOMP(5)", CNTL_LCDVCOMP(5)}, + {"CNTL_LCDVCOMP(6)", CNTL_LCDVCOMP(6)}, + {"CNTL_LCDVCOMP(7)", CNTL_LCDVCOMP(7)}, + {"CNTL_LDMAFIFOTIME", CNTL_LDMAFIFOTIME}, + {"CNTL_WATERMARK", CNTL_WATERMARK}, + { NULL, 0}, +}; +static struct string_lookup caps_lookups[] = { + {"CLCD_CAP_RGB444", CLCD_CAP_RGB444}, + {"CLCD_CAP_RGB5551", CLCD_CAP_RGB5551}, + {"CLCD_CAP_RGB565", CLCD_CAP_RGB565}, + {"CLCD_CAP_RGB888", CLCD_CAP_RGB888}, + {"CLCD_CAP_BGR444", CLCD_CAP_BGR444}, + {"CLCD_CAP_BGR5551", CLCD_CAP_BGR5551}, + {"CLCD_CAP_BGR565", CLCD_CAP_BGR565}, + {"CLCD_CAP_BGR888", CLCD_CAP_BGR888}, + {"CLCD_CAP_444", CLCD_CAP_444}, + {"CLCD_CAP_5551", CLCD_CAP_5551}, + {"CLCD_CAP_565", CLCD_CAP_565}, + {"CLCD_CAP_888", CLCD_CAP_888}, + {"CLCD_CAP_RGB", CLCD_CAP_RGB}, + {"CLCD_CAP_BGR", CLCD_CAP_BGR}, + {"CLCD_CAP_ALL", CLCD_CAP_ALL}, + { NULL, 0}, +}; + +u32 parse_setting(struct string_lookup *lookup, const char *name) +{ + int i = 0; + while (lookup[i].string != NULL) { + if (strcmp(lookup[i].string, name) == 0) + return lookup[i].val; + ++i; + } + return -EINVAL; +} + +u32 get_string_lookup(struct device_node *node, const char *name, + struct string_lookup *lookup) +{ + const char *string; + int count, i, ret = 0; + + count = of_property_count_strings(node, name); + if (count >= 0) + for (i = 0; i < count; i++) + if (of_property_read_string_index(node, name, i, + &string) == 0) + ret |= parse_setting(lookup, string); + return ret; +} + +int get_val(struct device_node *node, const char *string) +{ + u32 ret = 0; + + if (of_property_read_u32(node, string, &ret)) + ret = -1; + return ret; +} + +struct clcd_panel *getPanel(struct device_node *node) +{ + static struct clcd_panel panel; + + panel.mode.refresh = get_val(node, "refresh"); + panel.mode.xres = get_val(node, "xres"); + panel.mode.yres = get_val(node, "yres"); + panel.mode.pixclock = get_val(node, "pixclock"); + panel.mode.left_margin = get_val(node, "left_margin"); + panel.mode.right_margin = get_val(node, "right_margin"); + panel.mode.upper_margin = get_val(node, "upper_margin"); + panel.mode.lower_margin = get_val(node, "lower_margin"); + panel.mode.hsync_len = get_val(node, "hsync_len"); + panel.mode.vsync_len = get_val(node, "vsync_len"); + panel.mode.sync = get_val(node, "sync"); + panel.bpp = get_val(node, "bpp"); + panel.width = (signed short) get_val(node, "width"); + panel.height = (signed short) get_val(node, "height"); + + panel.mode.vmode = get_string_lookup(node, "vmode", vmode_lookups); + panel.tim2 = get_string_lookup(node, "tim2", tim2_lookups); + panel.cntl = get_string_lookup(node, "cntl", cntl_lookups); + panel.caps = get_string_lookup(node, "caps", caps_lookups); + + return &panel; +} + +struct clcd_panel *clcdfb_get_panel(const char *name) +{ + struct device_node *node = NULL; + const char *mode; + struct clcd_panel *panel = NULL; + + do { + node = of_find_compatible_node(node, NULL, "panel"); + if (node) + if (of_property_read_string(node, "mode", &mode) == 0) + if (strcmp(mode, name) == 0) { + panel = getPanel(node); + panel->mode.name = name; + } + } while (node != NULL); + + return panel; +} + +#ifdef CONFIG_OF +static int clcdfb_dt_init(struct clcd_fb *fb) +{ + int err = 0; + struct device_node *node; + const char *mode; + dma_addr_t dma; + u32 use_dma; + const __be32 *prop; + int len, na, ns; + phys_addr_t fb_base, fb_size; + + node = fb->dev->dev.of_node; + if (!node) + return -ENODEV; + + na = of_n_addr_cells(node); + ns = of_n_size_cells(node); + + if (WARN_ON(of_property_read_string(node, "mode", &mode))) + return -ENODEV; + + fb->panel = clcdfb_get_panel(mode); + if (!fb->panel) + return -EINVAL; + fb->fb.fix.smem_len = fb->panel->mode.xres * fb->panel->mode.yres * 2; + + if (of_property_read_u32(node, "use_dma", &use_dma)) + use_dma = 0; + if (use_dma) { + fb->fb.screen_base = dma_alloc_writecombine(&fb->dev->dev, + fb->fb.fix.smem_len, &dma, GFP_KERNEL); + if (!fb->fb.screen_base) { + pr_err("CLCD: unable to map framebuffer\n"); + err = -ENOMEM; + } else + fb->fb.fix.smem_start = dma; + } else { + prop = of_get_property(node, "framebuffer", &len); + if (WARN_ON(!prop || len < (na + ns) * sizeof(*prop))) + return -EINVAL; + fb_base = of_read_number(prop, na); + fb_size = of_read_number(prop + na, ns); + + if (memblock_remove(fb_base, fb_size) != 0) + return -EINVAL; + + fb->fb.fix.smem_start = fb_base; + fb->fb.screen_base = ioremap_wc(fb->fb.fix.smem_start, fb_size); + } + return err; +} +#endif /* CONFIG_OF */ + static int clcdfb_probe(struct amba_device *dev, const struct amba_id *id) { struct clcd_board *board = dev->dev.platform_data; struct clcd_fb *fb; int ret; +#ifdef CONFIG_OF + if (dev->dev.of_node) { + const __be32 *prop; + int len, na, ns; + phys_addr_t reg_base; + + na = of_n_addr_cells(dev->dev.of_node); + ns = of_n_size_cells(dev->dev.of_node); + + prop = of_get_property(dev->dev.of_node, "reg", &len); + if (WARN_ON(!prop || len < (na + ns) * sizeof(*prop))) + return -EINVAL; + reg_base = of_read_number(prop, na); + + if (dev->res.start != reg_base) + return -EINVAL; + + if (!board) { + board = kzalloc(sizeof(struct clcd_board), GFP_KERNEL); + if (!board) + return -EINVAL; + board->name = "Device Tree CLCD PL111"; + board->caps = CLCD_CAP_5551 | CLCD_CAP_565; + board->check = clcdfb_check; + board->decode = clcdfb_decode; + board->setup = clcdfb_dt_init; + board->mmap = clcdfb_mmap_dma; + board->remove = clcdfb_remove_dma; + } + } +#endif /* CONFIG_OF */ + if (!board) return -EINVAL; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 1/3] amba-clcd: Add Device Tree support to amba-clcd driver 2012-09-19 16:04 ` [RFC PATCH 1/3] amba-clcd: Add Device Tree support to amba-clcd driver Ryan Harkin @ 2012-09-20 10:24 ` Liviu Dudau [not found] ` <20120920102453.GG32603-CibnQJhq84/ZROr8t4l/smS4ubULX0JqMm0uRHvK7Nw@public.gmane.org> [not found] ` <1348070666-9153-2-git-send-email-ryan.harkin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2012-10-10 10:13 ` Jon Medhurst (Tixy) 2 siblings, 1 reply; 14+ messages in thread From: Liviu Dudau @ 2012-09-20 10:24 UTC (permalink / raw) To: Ryan Harkin Cc: stigge, tixy, linux-fbdev, Pawel Moll, devicetree-discuss, spear-devel, shiraz.hashim, viresh.linux, arnd.bergmann, linux-arm-kernel On Wed, Sep 19, 2012 at 05:04:24PM +0100, Ryan Harkin wrote: > Add support to parse the display configuration from device tree. > > If the board does not provide platform specific functions in the struct > clcd_board contained with the amba device info, then defaults are provided > by the driver. > > The device tree configuration can either ask for a DMA setup or provide a > framebuffer address to be remapped into the driver. > > Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org> > --- > drivers/video/amba-clcd.c | 253 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 253 insertions(+) > > diff --git a/drivers/video/amba-clcd.c b/drivers/video/amba-clcd.c > index 0a2cce7..01dbad1 100644 > --- a/drivers/video/amba-clcd.c > +++ b/drivers/video/amba-clcd.c > @@ -16,7 +16,10 @@ > #include <linux/string.h> > #include <linux/slab.h> > #include <linux/delay.h> > +#include <linux/dma-mapping.h> > +#include <linux/memblock.h> > #include <linux/mm.h> > +#include <linux/of.h> > #include <linux/fb.h> > #include <linux/init.h> > #include <linux/ioport.h> > @@ -391,6 +394,19 @@ static int clcdfb_blank(int blank_mode, struct fb_info *info) > } > return 0; > } > +int clcdfb_mmap_dma(struct clcd_fb *fb, struct vm_area_struct *vma) > +{ > + return dma_mmap_writecombine(&fb->dev->dev, vma, > + fb->fb.screen_base, > + fb->fb.fix.smem_start, > + fb->fb.fix.smem_len); > +} > + > +void clcdfb_remove_dma(struct clcd_fb *fb) > +{ > + dma_free_writecombine(&fb->dev->dev, fb->fb.fix.smem_len, > + fb->fb.screen_base, fb->fb.fix.smem_start); > +} > > static int clcdfb_mmap(struct fb_info *info, > struct vm_area_struct *vma) > @@ -542,12 +558,249 @@ static int clcdfb_register(struct clcd_fb *fb) > return ret; > } > > +struct string_lookup { > + const char *string; > + const u32 val; > +}; > + > +static struct string_lookup vmode_lookups[] = { > + { "FB_VMODE_NONINTERLACED", FB_VMODE_NONINTERLACED}, > + { "FB_VMODE_INTERLACED", FB_VMODE_INTERLACED}, > + { "FB_VMODE_DOUBLE", FB_VMODE_DOUBLE}, > + { "FB_VMODE_ODD_FLD_FIRST", FB_VMODE_ODD_FLD_FIRST}, > + { NULL, 0 }, > +}; > + > +static struct string_lookup tim2_lookups[] = { > + { "TIM2_CLKSEL", TIM2_CLKSEL}, > + { "TIM2_IVS", TIM2_IVS}, > + { "TIM2_IHS", TIM2_IHS}, > + { "TIM2_IPC", TIM2_IPC}, > + { "TIM2_IOE", TIM2_IOE}, > + { "TIM2_BCD", TIM2_BCD}, > + { NULL, 0}, > +}; > +static struct string_lookup cntl_lookups[] = { > + {"CNTL_LCDEN", CNTL_LCDEN}, > + {"CNTL_LCDBPP1", CNTL_LCDBPP1}, > + {"CNTL_LCDBPP2", CNTL_LCDBPP2}, > + {"CNTL_LCDBPP4", CNTL_LCDBPP4}, > + {"CNTL_LCDBPP8", CNTL_LCDBPP8}, > + {"CNTL_LCDBPP16", CNTL_LCDBPP16}, > + {"CNTL_LCDBPP16_565", CNTL_LCDBPP16_565}, > + {"CNTL_LCDBPP16_444", CNTL_LCDBPP16_444}, > + {"CNTL_LCDBPP24", CNTL_LCDBPP24}, > + {"CNTL_LCDBW", CNTL_LCDBW}, > + {"CNTL_LCDTFT", CNTL_LCDTFT}, > + {"CNTL_LCDMONO8", CNTL_LCDMONO8}, > + {"CNTL_LCDDUAL", CNTL_LCDDUAL}, > + {"CNTL_BGR", CNTL_BGR}, > + {"CNTL_BEBO", CNTL_BEBO}, > + {"CNTL_BEPO", CNTL_BEPO}, > + {"CNTL_LCDPWR", CNTL_LCDPWR}, > + {"CNTL_LCDVCOMP(1)", CNTL_LCDVCOMP(1)}, > + {"CNTL_LCDVCOMP(2)", CNTL_LCDVCOMP(2)}, > + {"CNTL_LCDVCOMP(3)", CNTL_LCDVCOMP(3)}, > + {"CNTL_LCDVCOMP(4)", CNTL_LCDVCOMP(4)}, > + {"CNTL_LCDVCOMP(5)", CNTL_LCDVCOMP(5)}, > + {"CNTL_LCDVCOMP(6)", CNTL_LCDVCOMP(6)}, > + {"CNTL_LCDVCOMP(7)", CNTL_LCDVCOMP(7)}, > + {"CNTL_LDMAFIFOTIME", CNTL_LDMAFIFOTIME}, > + {"CNTL_WATERMARK", CNTL_WATERMARK}, > + { NULL, 0}, > +}; > +static struct string_lookup caps_lookups[] = { > + {"CLCD_CAP_RGB444", CLCD_CAP_RGB444}, > + {"CLCD_CAP_RGB5551", CLCD_CAP_RGB5551}, > + {"CLCD_CAP_RGB565", CLCD_CAP_RGB565}, > + {"CLCD_CAP_RGB888", CLCD_CAP_RGB888}, > + {"CLCD_CAP_BGR444", CLCD_CAP_BGR444}, > + {"CLCD_CAP_BGR5551", CLCD_CAP_BGR5551}, > + {"CLCD_CAP_BGR565", CLCD_CAP_BGR565}, > + {"CLCD_CAP_BGR888", CLCD_CAP_BGR888}, > + {"CLCD_CAP_444", CLCD_CAP_444}, > + {"CLCD_CAP_5551", CLCD_CAP_5551}, > + {"CLCD_CAP_565", CLCD_CAP_565}, > + {"CLCD_CAP_888", CLCD_CAP_888}, > + {"CLCD_CAP_RGB", CLCD_CAP_RGB}, > + {"CLCD_CAP_BGR", CLCD_CAP_BGR}, > + {"CLCD_CAP_ALL", CLCD_CAP_ALL}, > + { NULL, 0}, > +}; > + > +u32 parse_setting(struct string_lookup *lookup, const char *name) > +{ > + int i = 0; > + while (lookup[i].string != NULL) { > + if (strcmp(lookup[i].string, name) == 0) > + return lookup[i].val; > + ++i; > + } > + return -EINVAL; > +} > + > +u32 get_string_lookup(struct device_node *node, const char *name, > + struct string_lookup *lookup) > +{ I have this feeling that swapping the names of the two functions above would reflect better their actual functionality. > + const char *string; > + int count, i, ret = 0; > + > + count = of_property_count_strings(node, name); > + if (count >= 0) > + for (i = 0; i < count; i++) > + if (of_property_read_string_index(node, name, i, > + &string) == 0) > + ret |= parse_setting(lookup, string); > + return ret; > +} > + > +int get_val(struct device_node *node, const char *string) > +{ > + u32 ret = 0; > + > + if (of_property_read_u32(node, string, &ret)) > + ret = -1; > + return ret; > +} > + > +struct clcd_panel *getPanel(struct device_node *node) > +{ > + static struct clcd_panel panel; > + > + panel.mode.refresh = get_val(node, "refresh"); > + panel.mode.xres = get_val(node, "xres"); > + panel.mode.yres = get_val(node, "yres"); > + panel.mode.pixclock = get_val(node, "pixclock"); > + panel.mode.left_margin = get_val(node, "left_margin"); > + panel.mode.right_margin = get_val(node, "right_margin"); > + panel.mode.upper_margin = get_val(node, "upper_margin"); > + panel.mode.lower_margin = get_val(node, "lower_margin"); > + panel.mode.hsync_len = get_val(node, "hsync_len"); > + panel.mode.vsync_len = get_val(node, "vsync_len"); > + panel.mode.sync = get_val(node, "sync"); > + panel.bpp = get_val(node, "bpp"); > + panel.width = (signed short) get_val(node, "width"); > + panel.height = (signed short) get_val(node, "height"); > + > + panel.mode.vmode = get_string_lookup(node, "vmode", vmode_lookups); > + panel.tim2 = get_string_lookup(node, "tim2", tim2_lookups); > + panel.cntl = get_string_lookup(node, "cntl", cntl_lookups); > + panel.caps = get_string_lookup(node, "caps", caps_lookups); > + > + return &panel; > +} > + > +struct clcd_panel *clcdfb_get_panel(const char *name) > +{ > + struct device_node *node = NULL; > + const char *mode; > + struct clcd_panel *panel = NULL; > + > + do { > + node = of_find_compatible_node(node, NULL, "panel"); > + if (node) > + if (of_property_read_string(node, "mode", &mode) == 0) > + if (strcmp(mode, name) == 0) { > + panel = getPanel(node); > + panel->mode.name = name; > + } > + } while (node != NULL); > + > + return panel; > +} > + > +#ifdef CONFIG_OF shouldn't this #ifdef be earlier? you are calling of_property_read_string() and while there are empty definitions if CONFIG_OF is not defined, the code will do nothing in that case. Is that intended? The clcdfb_get_panel() function only gets called if CONFIG_OF *is* defined. > +static int clcdfb_dt_init(struct clcd_fb *fb) > +{ > + int err = 0; > + struct device_node *node; > + const char *mode; > + dma_addr_t dma; > + u32 use_dma; > + const __be32 *prop; > + int len, na, ns; > + phys_addr_t fb_base, fb_size; > + > + node = fb->dev->dev.of_node; > + if (!node) > + return -ENODEV; > + > + na = of_n_addr_cells(node); > + ns = of_n_size_cells(node); > + > + if (WARN_ON(of_property_read_string(node, "mode", &mode))) > + return -ENODEV; > + > + fb->panel = clcdfb_get_panel(mode); > + if (!fb->panel) > + return -EINVAL; > + fb->fb.fix.smem_len = fb->panel->mode.xres * fb->panel->mode.yres * 2; > + > + if (of_property_read_u32(node, "use_dma", &use_dma)) I haven't seen any documentation for this property. What's the intended use? > + use_dma = 0; > + if (use_dma) { > + fb->fb.screen_base = dma_alloc_writecombine(&fb->dev->dev, > + fb->fb.fix.smem_len, &dma, GFP_KERNEL); > + if (!fb->fb.screen_base) { > + pr_err("CLCD: unable to map framebuffer\n"); > + err = -ENOMEM; > + } else > + fb->fb.fix.smem_start = dma; > + } else { > + prop = of_get_property(node, "framebuffer", &len); > + if (WARN_ON(!prop || len < (na + ns) * sizeof(*prop))) > + return -EINVAL; > + fb_base = of_read_number(prop, na); > + fb_size = of_read_number(prop + na, ns); > + > + if (memblock_remove(fb_base, fb_size) != 0) > + return -EINVAL; > + > + fb->fb.fix.smem_start = fb_base; > + fb->fb.screen_base = ioremap_wc(fb->fb.fix.smem_start, fb_size); > + } > + return err; > +} > +#endif /* CONFIG_OF */ > + > static int clcdfb_probe(struct amba_device *dev, const struct amba_id *id) > { > struct clcd_board *board = dev->dev.platform_data; > struct clcd_fb *fb; > int ret; > > +#ifdef CONFIG_OF > + if (dev->dev.of_node) { > + const __be32 *prop; > + int len, na, ns; > + phys_addr_t reg_base; > + > + na = of_n_addr_cells(dev->dev.of_node); > + ns = of_n_size_cells(dev->dev.of_node); > + > + prop = of_get_property(dev->dev.of_node, "reg", &len); > + if (WARN_ON(!prop || len < (na + ns) * sizeof(*prop))) > + return -EINVAL; > + reg_base = of_read_number(prop, na); > + > + if (dev->res.start != reg_base) > + return -EINVAL; > + > + if (!board) { > + board = kzalloc(sizeof(struct clcd_board), GFP_KERNEL); > + if (!board) > + return -EINVAL; > + board->name = "Device Tree CLCD PL111"; > + board->caps = CLCD_CAP_5551 | CLCD_CAP_565; > + board->check = clcdfb_check; > + board->decode = clcdfb_decode; > + board->setup = clcdfb_dt_init; > + board->mmap = clcdfb_mmap_dma; > + board->remove = clcdfb_remove_dma; > + } > + } > +#endif /* CONFIG_OF */ > + > if (!board) > return -EINVAL; > > -- > 1.7.9.5 > > -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯ -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <20120920102453.GG32603-CibnQJhq84/ZROr8t4l/smS4ubULX0JqMm0uRHvK7Nw@public.gmane.org>]
* Re: [RFC PATCH 1/3] amba-clcd: Add Device Tree support to amba-clcd driver [not found] ` <20120920102453.GG32603-CibnQJhq84/ZROr8t4l/smS4ubULX0JqMm0uRHvK7Nw@public.gmane.org> @ 2012-09-21 10:35 ` Ryan Harkin 2012-09-21 10:44 ` Pawel Moll 0 siblings, 1 reply; 14+ messages in thread From: Ryan Harkin @ 2012-09-21 10:35 UTC (permalink / raw) To: Ryan Harkin, arnd.bergmann-QSEj5FYQhm4dnm+yROfE0A, shiraz.hashim-qxv4g6HH51o, stigge-uj/7R2tJ6VmzQB+pC5nmwQ, Pawel Moll, tixy-QSEj5FYQhm4dnm+yROfE0A, spear-devel-nkJGhpqTU55BDgjK7y7TUQ, viresh.linux-Re5JQEeQqe8AvxtiuMwx3w, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-fbdev-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ Hi Liviu, Thanks for your feedback. All good stuff... On 20 September 2012 11:24, Liviu Dudau <Liviu.Dudau@arm.com> wrote: > On Wed, Sep 19, 2012 at 05:04:24PM +0100, Ryan Harkin wrote: >> Add support to parse the display configuration from device tree. >> >> If the board does not provide platform specific functions in the struct >> clcd_board contained with the amba device info, then defaults are provided >> by the driver. >> >> The device tree configuration can either ask for a DMA setup or provide a >> framebuffer address to be remapped into the driver. >> >> Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org> >> --- >> drivers/video/amba-clcd.c | 253 +++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 253 insertions(+) >> >> diff --git a/drivers/video/amba-clcd.c b/drivers/video/amba-clcd.c >> index 0a2cce7..01dbad1 100644 >> --- a/drivers/video/amba-clcd.c >> +++ b/drivers/video/amba-clcd.c >> @@ -16,7 +16,10 @@ >> #include <linux/string.h> >> #include <linux/slab.h> >> #include <linux/delay.h> >> +#include <linux/dma-mapping.h> >> +#include <linux/memblock.h> >> #include <linux/mm.h> >> +#include <linux/of.h> >> #include <linux/fb.h> >> #include <linux/init.h> >> #include <linux/ioport.h> >> @@ -391,6 +394,19 @@ static int clcdfb_blank(int blank_mode, struct fb_info *info) >> } >> return 0; >> } >> +int clcdfb_mmap_dma(struct clcd_fb *fb, struct vm_area_struct *vma) >> +{ >> + return dma_mmap_writecombine(&fb->dev->dev, vma, >> + fb->fb.screen_base, >> + fb->fb.fix.smem_start, >> + fb->fb.fix.smem_len); >> +} >> + >> +void clcdfb_remove_dma(struct clcd_fb *fb) >> +{ >> + dma_free_writecombine(&fb->dev->dev, fb->fb.fix.smem_len, >> + fb->fb.screen_base, fb->fb.fix.smem_start); >> +} >> >> static int clcdfb_mmap(struct fb_info *info, >> struct vm_area_struct *vma) >> @@ -542,12 +558,249 @@ static int clcdfb_register(struct clcd_fb *fb) >> return ret; >> } >> >> +struct string_lookup { >> + const char *string; >> + const u32 val; >> +}; >> + >> +static struct string_lookup vmode_lookups[] = { >> + { "FB_VMODE_NONINTERLACED", FB_VMODE_NONINTERLACED}, >> + { "FB_VMODE_INTERLACED", FB_VMODE_INTERLACED}, >> + { "FB_VMODE_DOUBLE", FB_VMODE_DOUBLE}, >> + { "FB_VMODE_ODD_FLD_FIRST", FB_VMODE_ODD_FLD_FIRST}, >> + { NULL, 0 }, >> +}; >> + >> +static struct string_lookup tim2_lookups[] = { >> + { "TIM2_CLKSEL", TIM2_CLKSEL}, >> + { "TIM2_IVS", TIM2_IVS}, >> + { "TIM2_IHS", TIM2_IHS}, >> + { "TIM2_IPC", TIM2_IPC}, >> + { "TIM2_IOE", TIM2_IOE}, >> + { "TIM2_BCD", TIM2_BCD}, >> + { NULL, 0}, >> +}; >> +static struct string_lookup cntl_lookups[] = { >> + {"CNTL_LCDEN", CNTL_LCDEN}, >> + {"CNTL_LCDBPP1", CNTL_LCDBPP1}, >> + {"CNTL_LCDBPP2", CNTL_LCDBPP2}, >> + {"CNTL_LCDBPP4", CNTL_LCDBPP4}, >> + {"CNTL_LCDBPP8", CNTL_LCDBPP8}, >> + {"CNTL_LCDBPP16", CNTL_LCDBPP16}, >> + {"CNTL_LCDBPP16_565", CNTL_LCDBPP16_565}, >> + {"CNTL_LCDBPP16_444", CNTL_LCDBPP16_444}, >> + {"CNTL_LCDBPP24", CNTL_LCDBPP24}, >> + {"CNTL_LCDBW", CNTL_LCDBW}, >> + {"CNTL_LCDTFT", CNTL_LCDTFT}, >> + {"CNTL_LCDMONO8", CNTL_LCDMONO8}, >> + {"CNTL_LCDDUAL", CNTL_LCDDUAL}, >> + {"CNTL_BGR", CNTL_BGR}, >> + {"CNTL_BEBO", CNTL_BEBO}, >> + {"CNTL_BEPO", CNTL_BEPO}, >> + {"CNTL_LCDPWR", CNTL_LCDPWR}, >> + {"CNTL_LCDVCOMP(1)", CNTL_LCDVCOMP(1)}, >> + {"CNTL_LCDVCOMP(2)", CNTL_LCDVCOMP(2)}, >> + {"CNTL_LCDVCOMP(3)", CNTL_LCDVCOMP(3)}, >> + {"CNTL_LCDVCOMP(4)", CNTL_LCDVCOMP(4)}, >> + {"CNTL_LCDVCOMP(5)", CNTL_LCDVCOMP(5)}, >> + {"CNTL_LCDVCOMP(6)", CNTL_LCDVCOMP(6)}, >> + {"CNTL_LCDVCOMP(7)", CNTL_LCDVCOMP(7)}, >> + {"CNTL_LDMAFIFOTIME", CNTL_LDMAFIFOTIME}, >> + {"CNTL_WATERMARK", CNTL_WATERMARK}, >> + { NULL, 0}, >> +}; >> +static struct string_lookup caps_lookups[] = { >> + {"CLCD_CAP_RGB444", CLCD_CAP_RGB444}, >> + {"CLCD_CAP_RGB5551", CLCD_CAP_RGB5551}, >> + {"CLCD_CAP_RGB565", CLCD_CAP_RGB565}, >> + {"CLCD_CAP_RGB888", CLCD_CAP_RGB888}, >> + {"CLCD_CAP_BGR444", CLCD_CAP_BGR444}, >> + {"CLCD_CAP_BGR5551", CLCD_CAP_BGR5551}, >> + {"CLCD_CAP_BGR565", CLCD_CAP_BGR565}, >> + {"CLCD_CAP_BGR888", CLCD_CAP_BGR888}, >> + {"CLCD_CAP_444", CLCD_CAP_444}, >> + {"CLCD_CAP_5551", CLCD_CAP_5551}, >> + {"CLCD_CAP_565", CLCD_CAP_565}, >> + {"CLCD_CAP_888", CLCD_CAP_888}, >> + {"CLCD_CAP_RGB", CLCD_CAP_RGB}, >> + {"CLCD_CAP_BGR", CLCD_CAP_BGR}, >> + {"CLCD_CAP_ALL", CLCD_CAP_ALL}, >> + { NULL, 0}, >> +}; >> + >> +u32 parse_setting(struct string_lookup *lookup, const char *name) >> +{ >> + int i = 0; >> + while (lookup[i].string != NULL) { >> + if (strcmp(lookup[i].string, name) == 0) >> + return lookup[i].val; >> + ++i; >> + } >> + return -EINVAL; >> +} >> + >> +u32 get_string_lookup(struct device_node *node, const char *name, >> + struct string_lookup *lookup) >> +{ > > I have this feeling that swapping the names of the two functions above > would reflect better their actual functionality. I see what you mean. I'm happy to swap them if there are no objections. > >> + const char *string; >> + int count, i, ret = 0; >> + >> + count = of_property_count_strings(node, name); >> + if (count >= 0) >> + for (i = 0; i < count; i++) >> + if (of_property_read_string_index(node, name, i, >> + &string) == 0) >> + ret |= parse_setting(lookup, string); >> + return ret; >> +} >> + >> +int get_val(struct device_node *node, const char *string) >> +{ >> + u32 ret = 0; >> + >> + if (of_property_read_u32(node, string, &ret)) >> + ret = -1; >> + return ret; >> +} >> + >> +struct clcd_panel *getPanel(struct device_node *node) >> +{ >> + static struct clcd_panel panel; >> + >> + panel.mode.refresh = get_val(node, "refresh"); >> + panel.mode.xres = get_val(node, "xres"); >> + panel.mode.yres = get_val(node, "yres"); >> + panel.mode.pixclock = get_val(node, "pixclock"); >> + panel.mode.left_margin = get_val(node, "left_margin"); >> + panel.mode.right_margin = get_val(node, "right_margin"); >> + panel.mode.upper_margin = get_val(node, "upper_margin"); >> + panel.mode.lower_margin = get_val(node, "lower_margin"); >> + panel.mode.hsync_len = get_val(node, "hsync_len"); >> + panel.mode.vsync_len = get_val(node, "vsync_len"); >> + panel.mode.sync = get_val(node, "sync"); >> + panel.bpp = get_val(node, "bpp"); >> + panel.width = (signed short) get_val(node, "width"); >> + panel.height = (signed short) get_val(node, "height"); >> + >> + panel.mode.vmode = get_string_lookup(node, "vmode", vmode_lookups); >> + panel.tim2 = get_string_lookup(node, "tim2", tim2_lookups); >> + panel.cntl = get_string_lookup(node, "cntl", cntl_lookups); >> + panel.caps = get_string_lookup(node, "caps", caps_lookups); >> + >> + return &panel; >> +} >> + >> +struct clcd_panel *clcdfb_get_panel(const char *name) >> +{ >> + struct device_node *node = NULL; >> + const char *mode; >> + struct clcd_panel *panel = NULL; >> + >> + do { >> + node = of_find_compatible_node(node, NULL, "panel"); >> + if (node) >> + if (of_property_read_string(node, "mode", &mode) == 0) >> + if (strcmp(mode, name) == 0) { >> + panel = getPanel(node); >> + panel->mode.name = name; >> + } >> + } while (node != NULL); >> + >> + return panel; >> +} >> + >> +#ifdef CONFIG_OF > > shouldn't this #ifdef be earlier? you are calling of_property_read_string() > and while there are empty definitions if CONFIG_OF is not defined, the code > will do nothing in that case. Is that intended? The clcdfb_get_panel() > function only gets called if CONFIG_OF *is* defined. Agree. > > >> +static int clcdfb_dt_init(struct clcd_fb *fb) >> +{ >> + int err = 0; >> + struct device_node *node; >> + const char *mode; >> + dma_addr_t dma; >> + u32 use_dma; >> + const __be32 *prop; >> + int len, na, ns; >> + phys_addr_t fb_base, fb_size; >> + >> + node = fb->dev->dev.of_node; >> + if (!node) >> + return -ENODEV; >> + >> + na = of_n_addr_cells(node); >> + ns = of_n_size_cells(node); >> + >> + if (WARN_ON(of_property_read_string(node, "mode", &mode))) >> + return -ENODEV; >> + >> + fb->panel = clcdfb_get_panel(mode); >> + if (!fb->panel) >> + return -EINVAL; >> + fb->fb.fix.smem_len = fb->panel->mode.xres * fb->panel->mode.yres * 2; >> + >> + if (of_property_read_u32(node, "use_dma", &use_dma)) > > I haven't seen any documentation for this property. What's the intended use? Good point. Sorry for my ignorance, but is there a place I should put such documentation? When the A9 CoreTile uses the on-tile CLCD controller, it can use DMA to handle the framebuffer. DMA is not available to the motherboard CLCD controller. I was trying to keep the properties simple, but they are more complex than just the two settings: use_dma and framebuffer. If use_dma is specified, the framebuffer property is not used; in this case, the framebuffer is allocated by the DMA framework and the framebuffer property is ignored. If use_dma is not present or is set to <0>, the code uses the framebuffer property to specify the address. > >> + use_dma = 0; >> + if (use_dma) { >> + fb->fb.screen_base = dma_alloc_writecombine(&fb->dev->dev, >> + fb->fb.fix.smem_len, &dma, GFP_KERNEL); >> + if (!fb->fb.screen_base) { >> + pr_err("CLCD: unable to map framebuffer\n"); >> + err = -ENOMEM; >> + } else >> + fb->fb.fix.smem_start = dma; >> + } else { >> + prop = of_get_property(node, "framebuffer", &len); >> + if (WARN_ON(!prop || len < (na + ns) * sizeof(*prop))) >> + return -EINVAL; >> + fb_base = of_read_number(prop, na); >> + fb_size = of_read_number(prop + na, ns); >> + >> + if (memblock_remove(fb_base, fb_size) != 0) >> + return -EINVAL; >> + >> + fb->fb.fix.smem_start = fb_base; >> + fb->fb.screen_base = ioremap_wc(fb->fb.fix.smem_start, fb_size); >> + } >> + return err; >> +} >> +#endif /* CONFIG_OF */ >> + >> static int clcdfb_probe(struct amba_device *dev, const struct amba_id *id) >> { >> struct clcd_board *board = dev->dev.platform_data; >> struct clcd_fb *fb; >> int ret; >> >> +#ifdef CONFIG_OF >> + if (dev->dev.of_node) { >> + const __be32 *prop; >> + int len, na, ns; >> + phys_addr_t reg_base; >> + >> + na = of_n_addr_cells(dev->dev.of_node); >> + ns = of_n_size_cells(dev->dev.of_node); >> + >> + prop = of_get_property(dev->dev.of_node, "reg", &len); >> + if (WARN_ON(!prop || len < (na + ns) * sizeof(*prop))) >> + return -EINVAL; >> + reg_base = of_read_number(prop, na); >> + >> + if (dev->res.start != reg_base) >> + return -EINVAL; >> + >> + if (!board) { >> + board = kzalloc(sizeof(struct clcd_board), GFP_KERNEL); >> + if (!board) >> + return -EINVAL; >> + board->name = "Device Tree CLCD PL111"; >> + board->caps = CLCD_CAP_5551 | CLCD_CAP_565; >> + board->check = clcdfb_check; >> + board->decode = clcdfb_decode; >> + board->setup = clcdfb_dt_init; >> + board->mmap = clcdfb_mmap_dma; >> + board->remove = clcdfb_remove_dma; >> + } >> + } >> +#endif /* CONFIG_OF */ >> + >> if (!board) >> return -EINVAL; >> >> -- >> 1.7.9.5 >> >> > > -- > ==================== > | I would like to | > | fix the world, | > | but they're not | > | giving me the | > \ source code! / > --------------- > ¯\_(ツ)_/¯ > > -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. > _______________________________________________ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 1/3] amba-clcd: Add Device Tree support to amba-clcd driver 2012-09-21 10:35 ` Ryan Harkin @ 2012-09-21 10:44 ` Pawel Moll 0 siblings, 0 replies; 14+ messages in thread From: Pawel Moll @ 2012-09-21 10:44 UTC (permalink / raw) To: Ryan Harkin Cc: stigge, tixy, linux-fbdev, devicetree-discuss, spear-devel, shiraz.hashim, viresh.linux, arnd.bergmann, linux-arm-kernel On Fri, 2012-09-21 at 11:35 +0100, Ryan Harkin wrote: > Good point. Sorry for my ignorance, but is there a place I should put > such documentation? Documentation/devicetree/bindings/fb/amba-clcd.txt :-) > When the A9 CoreTile uses the on-tile CLCD controller, it can use DMA > to handle the framebuffer. DMA is not available to the motherboard > CLCD controller. Uh, you probably mean that the motherboard CLCD must use the specialised video memory, while the one in the test chip would normally use a buffer allocated via the DMA API... > I was trying to keep the properties simple, but they are more complex > than just the two settings: use_dma and framebuffer. If use_dma is > specified, the framebuffer property is not used; in this case, the > framebuffer is allocated by the DMA framework and the framebuffer > property is ignored. If use_dma is not present or is set to <0>, the > code uses the framebuffer property to specify the address. I'm not sure if the "framebuffer" property is really needed, at least in the form you have it there now. I think I know you where you get it from - HDLCD driver, right? It was sort-of-needed there, because we had to reserve memory for the framebuffer, because you couldn't allocate big enough (8MB if I remember correctly) area using the DMA alloc function. But now, with the CMA in place it should be possible, so I believe it's not even necessary in that case. Simply speaking - if the driver is not told what address to use, it should get the memory on its own. Now, as to the motherboard CLCD... That's where you actually could use this property in the way you have there, to enforce the address of the video RAM as the framebuffer. But maybe it would be better to have a phandle to the video RAM node? Cheers! Paweł _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <1348070666-9153-2-git-send-email-ryan.harkin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]
* Re: [RFC PATCH 1/3] amba-clcd: Add Device Tree support to amba-clcd driver [not found] ` <1348070666-9153-2-git-send-email-ryan.harkin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> @ 2012-09-21 11:02 ` Russell King - ARM Linux 2012-09-21 11:43 ` Sascha Hauer 1 sibling, 0 replies; 14+ messages in thread From: Russell King - ARM Linux @ 2012-09-21 11:02 UTC (permalink / raw) To: Ryan Harkin Cc: stigge-uj/7R2tJ6VmzQB+pC5nmwQ, tixy-QSEj5FYQhm4dnm+yROfE0A, linux-fbdev-u79uwXL29TY76Z2rM5mHXA, pawel.moll-5wv7dgnIgG8, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, spear-devel-nkJGhpqTU55BDgjK7y7TUQ, liviu.dudau-5wv7dgnIgG8, shiraz.hashim-qxv4g6HH51o, viresh.linux-Re5JQEeQqe8AvxtiuMwx3w, arnd.bergmann-QSEj5FYQhm4dnm+yROfE0A, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r This patch has a number of serious problems with it. On Wed, Sep 19, 2012 at 05:04:24PM +0100, Ryan Harkin wrote: > @@ -391,6 +394,19 @@ static int clcdfb_blank(int blank_mode, struct fb_info *info) > } > return 0; > } This needs a blank line. > +int clcdfb_mmap_dma(struct clcd_fb *fb, struct vm_area_struct *vma) > +{ > + return dma_mmap_writecombine(&fb->dev->dev, vma, > + fb->fb.screen_base, > + fb->fb.fix.smem_start, > + fb->fb.fix.smem_len); > +} > + > +void clcdfb_remove_dma(struct clcd_fb *fb) > +{ > + dma_free_writecombine(&fb->dev->dev, fb->fb.fix.smem_len, > + fb->fb.screen_base, fb->fb.fix.smem_start); > +} > > static int clcdfb_mmap(struct fb_info *info, > struct vm_area_struct *vma) > @@ -542,12 +558,249 @@ static int clcdfb_register(struct clcd_fb *fb) > return ret; > } > > +struct string_lookup { > + const char *string; > + const u32 val; > +}; > + > +static struct string_lookup vmode_lookups[] = { > + { "FB_VMODE_NONINTERLACED", FB_VMODE_NONINTERLACED}, > + { "FB_VMODE_INTERLACED", FB_VMODE_INTERLACED}, > + { "FB_VMODE_DOUBLE", FB_VMODE_DOUBLE}, > + { "FB_VMODE_ODD_FLD_FIRST", FB_VMODE_ODD_FLD_FIRST}, > + { NULL, 0 }, > +}; > + > +static struct string_lookup tim2_lookups[] = { > + { "TIM2_CLKSEL", TIM2_CLKSEL}, > + { "TIM2_IVS", TIM2_IVS}, > + { "TIM2_IHS", TIM2_IHS}, Inversion of the sync control signals are part of the framebuffer API, and should not be specified as part of this register definition. Instead, they should be specified using FB_SYNC_HOR_HIGH_ACT and FB_SYNC_VERT_HIGH_ACT. Setting them in ->tim2 is bad news because it just confuses these settings. > + { "TIM2_IOE", TIM2_IOE}, > + { "TIM2_BCD", TIM2_BCD}, > + { NULL, 0}, > +}; Another blank line required. > +static struct string_lookup cntl_lookups[] = { > + {"CNTL_LCDEN", CNTL_LCDEN}, Err, no. > + {"CNTL_LCDBPP1", CNTL_LCDBPP1}, > + {"CNTL_LCDBPP2", CNTL_LCDBPP2}, > + {"CNTL_LCDBPP4", CNTL_LCDBPP4}, > + {"CNTL_LCDBPP8", CNTL_LCDBPP8}, > + {"CNTL_LCDBPP16", CNTL_LCDBPP16}, > + {"CNTL_LCDBPP16_565", CNTL_LCDBPP16_565}, > + {"CNTL_LCDBPP16_444", CNTL_LCDBPP16_444}, > + {"CNTL_LCDBPP24", CNTL_LCDBPP24}, The colour depth is derived from the video mode setting, which will be overridden anyway - and the allowable depths are derived from the panel capabilities and the board capabilities. > + {"CNTL_LCDBW", CNTL_LCDBW}, > + {"CNTL_LCDTFT", CNTL_LCDTFT}, > + {"CNTL_LCDMONO8", CNTL_LCDMONO8}, > + {"CNTL_LCDDUAL", CNTL_LCDDUAL}, These four are properties of the panel, so they're fine. > + {"CNTL_BGR", CNTL_BGR}, BGR is also overridden by the driver. > + {"CNTL_BEBO", CNTL_BEBO}, > + {"CNTL_BEPO", CNTL_BEPO}, >From what I remember, these are framebuffer layout control bits. They aren't panel properties, and so they shouldn't be specified in DT. > + {"CNTL_LCDPWR", CNTL_LCDPWR}, Err, no. Specifying power control here is bad news - have you read the data sheet for the CLCD controller (which specifies a certain sequence for LCDEN and LCDPWR)? Have you read the driver code and realized that the driver controls these bits, and therefore specifying them in DT is absurd? > + {"CNTL_LCDVCOMP(1)", CNTL_LCDVCOMP(1)}, > + {"CNTL_LCDVCOMP(2)", CNTL_LCDVCOMP(2)}, > + {"CNTL_LCDVCOMP(3)", CNTL_LCDVCOMP(3)}, > + {"CNTL_LCDVCOMP(4)", CNTL_LCDVCOMP(4)}, > + {"CNTL_LCDVCOMP(5)", CNTL_LCDVCOMP(5)}, > + {"CNTL_LCDVCOMP(6)", CNTL_LCDVCOMP(6)}, > + {"CNTL_LCDVCOMP(7)", CNTL_LCDVCOMP(7)}, > + {"CNTL_LDMAFIFOTIME", CNTL_LDMAFIFOTIME}, > + {"CNTL_WATERMARK", CNTL_WATERMARK}, > + { NULL, 0}, > +}; Another blank line required. > +static struct string_lookup caps_lookups[] = { > + {"CLCD_CAP_RGB444", CLCD_CAP_RGB444}, > + {"CLCD_CAP_RGB5551", CLCD_CAP_RGB5551}, > + {"CLCD_CAP_RGB565", CLCD_CAP_RGB565}, > + {"CLCD_CAP_RGB888", CLCD_CAP_RGB888}, > + {"CLCD_CAP_BGR444", CLCD_CAP_BGR444}, > + {"CLCD_CAP_BGR5551", CLCD_CAP_BGR5551}, > + {"CLCD_CAP_BGR565", CLCD_CAP_BGR565}, > + {"CLCD_CAP_BGR888", CLCD_CAP_BGR888}, > + {"CLCD_CAP_444", CLCD_CAP_444}, > + {"CLCD_CAP_5551", CLCD_CAP_5551}, > + {"CLCD_CAP_565", CLCD_CAP_565}, > + {"CLCD_CAP_888", CLCD_CAP_888}, > + {"CLCD_CAP_RGB", CLCD_CAP_RGB}, > + {"CLCD_CAP_BGR", CLCD_CAP_BGR}, > + {"CLCD_CAP_ALL", CLCD_CAP_ALL}, > + { NULL, 0}, > +}; > + > +u32 parse_setting(struct string_lookup *lookup, const char *name) > +{ > + int i = 0; > + while (lookup[i].string != NULL) { > + if (strcmp(lookup[i].string, name) == 0) > + return lookup[i].val; > + ++i; > + } > + return -EINVAL; > +} Why is this non-static? > + > +u32 get_string_lookup(struct device_node *node, const char *name, > + struct string_lookup *lookup) > +{ > + const char *string; > + int count, i, ret = 0; > + > + count = of_property_count_strings(node, name); > + if (count >= 0) > + for (i = 0; i < count; i++) > + if (of_property_read_string_index(node, name, i, > + &string) == 0) > + ret |= parse_setting(lookup, string); > + return ret; > +} And this? > + > +int get_val(struct device_node *node, const char *string) > +{ > + u32 ret = 0; > + > + if (of_property_read_u32(node, string, &ret)) > + ret = -1; > + return ret; > +} And this? > + > +struct clcd_panel *getPanel(struct device_node *node) Have you read coding style? We don't use capitals in function names. > +{ > + static struct clcd_panel panel; > + > + panel.mode.refresh = get_val(node, "refresh"); > + panel.mode.xres = get_val(node, "xres"); > + panel.mode.yres = get_val(node, "yres"); > + panel.mode.pixclock = get_val(node, "pixclock"); It's debatable whether we want to specify the pixel clock in DT or not - it can be calculated from the other parameters here 1e12/(refresh * htotal * vtotal) ps. > + panel.mode.left_margin = get_val(node, "left_margin"); > + panel.mode.right_margin = get_val(node, "right_margin"); > + panel.mode.upper_margin = get_val(node, "upper_margin"); > + panel.mode.lower_margin = get_val(node, "lower_margin"); > + panel.mode.hsync_len = get_val(node, "hsync_len"); > + panel.mode.vsync_len = get_val(node, "vsync_len"); > + panel.mode.sync = get_val(node, "sync"); An integer sync property? You're exposing kernel internal definitions into DT here which is unacceptable. > + panel.bpp = get_val(node, "bpp"); > + panel.width = (signed short) get_val(node, "width"); > + panel.height = (signed short) get_val(node, "height"); > + > + panel.mode.vmode = get_string_lookup(node, "vmode", vmode_lookups); > + panel.tim2 = get_string_lookup(node, "tim2", tim2_lookups); > + panel.cntl = get_string_lookup(node, "cntl", cntl_lookups); > + panel.caps = get_string_lookup(node, "caps", caps_lookups); > + > + return &panel; > +} > + > +struct clcd_panel *clcdfb_get_panel(const char *name) Why is this exported? > +{ > + struct device_node *node = NULL; > + const char *mode; > + struct clcd_panel *panel = NULL; > + > + do { > + node = of_find_compatible_node(node, NULL, "panel"); > + if (node) > + if (of_property_read_string(node, "mode", &mode) == 0) > + if (strcmp(mode, name) == 0) { > + panel = getPanel(node); > + panel->mode.name = name; > + } > + } while (node != NULL); > + > + return panel; > +} > + > +#ifdef CONFIG_OF > +static int clcdfb_dt_init(struct clcd_fb *fb) > +{ > + int err = 0; > + struct device_node *node; > + const char *mode; > + dma_addr_t dma; > + u32 use_dma; > + const __be32 *prop; > + int len, na, ns; > + phys_addr_t fb_base, fb_size; > + > + node = fb->dev->dev.of_node; > + if (!node) > + return -ENODEV; > + > + na = of_n_addr_cells(node); > + ns = of_n_size_cells(node); > + > + if (WARN_ON(of_property_read_string(node, "mode", &mode))) > + return -ENODEV; > + > + fb->panel = clcdfb_get_panel(mode); > + if (!fb->panel) > + return -EINVAL; > + fb->fb.fix.smem_len = fb->panel->mode.xres * fb->panel->mode.yres * 2; > + > + if (of_property_read_u32(node, "use_dma", &use_dma)) > + use_dma = 0; > + if (use_dma) { > + fb->fb.screen_base = dma_alloc_writecombine(&fb->dev->dev, > + fb->fb.fix.smem_len, &dma, GFP_KERNEL); > + if (!fb->fb.screen_base) { > + pr_err("CLCD: unable to map framebuffer\n"); > + err = -ENOMEM; > + } else > + fb->fb.fix.smem_start = dma; > + } else { > + prop = of_get_property(node, "framebuffer", &len); > + if (WARN_ON(!prop || len < (na + ns) * sizeof(*prop))) > + return -EINVAL; > + fb_base = of_read_number(prop, na); > + fb_size = of_read_number(prop + na, ns); > + > + if (memblock_remove(fb_base, fb_size) != 0) > + return -EINVAL; No. You can not do this here, calling this function after the reserve callback into the platform code has completed will corrupt the kernel. > + > + fb->fb.fix.smem_start = fb_base; > + fb->fb.screen_base = ioremap_wc(fb->fb.fix.smem_start, fb_size); > + } > + return err; > +} > +#endif /* CONFIG_OF */ > + > static int clcdfb_probe(struct amba_device *dev, const struct amba_id *id) > { > struct clcd_board *board = dev->dev.platform_data; > struct clcd_fb *fb; > int ret; > > +#ifdef CONFIG_OF > + if (dev->dev.of_node) { > + const __be32 *prop; > + int len, na, ns; > + phys_addr_t reg_base; > + > + na = of_n_addr_cells(dev->dev.of_node); > + ns = of_n_size_cells(dev->dev.of_node); > + > + prop = of_get_property(dev->dev.of_node, "reg", &len); > + if (WARN_ON(!prop || len < (na + ns) * sizeof(*prop))) > + return -EINVAL; > + reg_base = of_read_number(prop, na); > + > + if (dev->res.start != reg_base) > + return -EINVAL; > + > + if (!board) { > + board = kzalloc(sizeof(struct clcd_board), GFP_KERNEL); > + if (!board) > + return -EINVAL; > + board->name = "Device Tree CLCD PL111"; > + board->caps = CLCD_CAP_5551 | CLCD_CAP_565; This looks like it's been hard-coded for one particular board, and one particular board only. > + board->check = clcdfb_check; > + board->decode = clcdfb_decode; > + board->setup = clcdfb_dt_init; > + board->mmap = clcdfb_mmap_dma; > + board->remove = clcdfb_remove_dma; > + } > + } > +#endif /* CONFIG_OF */ > + > if (!board) > return -EINVAL; > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 1/3] amba-clcd: Add Device Tree support to amba-clcd driver [not found] ` <1348070666-9153-2-git-send-email-ryan.harkin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2012-09-21 11:02 ` Russell King - ARM Linux @ 2012-09-21 11:43 ` Sascha Hauer [not found] ` <20120921114345.GE24458-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> 1 sibling, 1 reply; 14+ messages in thread From: Sascha Hauer @ 2012-09-21 11:43 UTC (permalink / raw) To: Ryan Harkin Cc: stigge-uj/7R2tJ6VmzQB+pC5nmwQ, tixy-QSEj5FYQhm4dnm+yROfE0A, linux-fbdev-u79uwXL29TY76Z2rM5mHXA, pawel.moll-5wv7dgnIgG8, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, spear-devel-nkJGhpqTU55BDgjK7y7TUQ, liviu.dudau-5wv7dgnIgG8, shiraz.hashim-qxv4g6HH51o, viresh.linux-Re5JQEeQqe8AvxtiuMwx3w, arnd.bergmann-QSEj5FYQhm4dnm+yROfE0A, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r On Wed, Sep 19, 2012 at 05:04:24PM +0100, Ryan Harkin wrote: > Add support to parse the display configuration from device tree. > > If the board does not provide platform specific functions in the struct > clcd_board contained with the amba device info, then defaults are provided > by the driver. > > The device tree configuration can either ask for a DMA setup or provide a > framebuffer address to be remapped into the driver. > > Signed-off-by: Ryan Harkin <ryan.harkin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > --- [...] > + > +struct clcd_panel *getPanel(struct device_node *node) > +{ > + static struct clcd_panel panel; > + > + panel.mode.refresh = get_val(node, "refresh"); > + panel.mode.xres = get_val(node, "xres"); > + panel.mode.yres = get_val(node, "yres"); > + panel.mode.pixclock = get_val(node, "pixclock"); > + panel.mode.left_margin = get_val(node, "left_margin"); > + panel.mode.right_margin = get_val(node, "right_margin"); > + panel.mode.upper_margin = get_val(node, "upper_margin"); > + panel.mode.lower_margin = get_val(node, "lower_margin"); > + panel.mode.hsync_len = get_val(node, "hsync_len"); > + panel.mode.vsync_len = get_val(node, "vsync_len"); > + panel.mode.sync = get_val(node, "sync"); We are currently discussing a common panel description for the devicetree. You are invited to join in here: http://comments.gmane.org/gmane.linux.drivers.devicetree/21386 We shouldn't add any more device specific panel descriptions. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <20120921114345.GE24458-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]
* Re: [RFC PATCH 1/3] amba-clcd: Add Device Tree support to amba-clcd driver [not found] ` <20120921114345.GE24458-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> @ 2012-09-21 12:19 ` Russell King - ARM Linux [not found] ` <20120921121903.GE15609-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org> 0 siblings, 1 reply; 14+ messages in thread From: Russell King - ARM Linux @ 2012-09-21 12:19 UTC (permalink / raw) To: Sascha Hauer Cc: stigge-uj/7R2tJ6VmzQB+pC5nmwQ, tixy-QSEj5FYQhm4dnm+yROfE0A, linux-fbdev-u79uwXL29TY76Z2rM5mHXA, pawel.moll-5wv7dgnIgG8, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, spear-devel-nkJGhpqTU55BDgjK7y7TUQ, liviu.dudau-5wv7dgnIgG8, shiraz.hashim-qxv4g6HH51o, viresh.linux-Re5JQEeQqe8AvxtiuMwx3w, arnd.bergmann-QSEj5FYQhm4dnm+yROfE0A, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r On Fri, Sep 21, 2012 at 01:43:45PM +0200, Sascha Hauer wrote: > We are currently discussing a common panel description for the > devicetree. You are invited to join in here: > > http://comments.gmane.org/gmane.linux.drivers.devicetree/21386 > > We shouldn't add any more device specific panel descriptions. Just make sure that you don't end up encoding in integers in the device tree Linux kernel specific bitfields such as the 'sync' property above. Also, look at how the AMBA CLCD driver handles the panels with its capability bitmask for the colour formats - this allows the driver to determine what colour formats are supported, and allows platform code to work out how to program any MUXes for any particular panel and requested colour depth setting. It also allows figuring out whether BGR or RGB format should be used. This is necessary because of the beweildering array of panels, muxes and other funky hardware which ARM Ltd like to put into their boards, and the fact that the LCDs tend to be entirely separate entities to the boards themselves. ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <20120921121903.GE15609-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>]
* Re: [RFC PATCH 1/3] amba-clcd: Add Device Tree support to amba-clcd driver [not found] ` <20120921121903.GE15609-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org> @ 2012-09-24 7:07 ` Ryan Harkin 0 siblings, 0 replies; 14+ messages in thread From: Ryan Harkin @ 2012-09-24 7:07 UTC (permalink / raw) To: Russell King - ARM Linux Cc: stigge-uj/7R2tJ6VmzQB+pC5nmwQ, tixy-QSEj5FYQhm4dnm+yROfE0A, linux-fbdev-u79uwXL29TY76Z2rM5mHXA, pawel.moll-5wv7dgnIgG8, Sascha Hauer, spear-devel-nkJGhpqTU55BDgjK7y7TUQ, liviu.dudau-5wv7dgnIgG8, shiraz.hashim-qxv4g6HH51o, viresh.linux-Re5JQEeQqe8AvxtiuMwx3w, arnd.bergmann-QSEj5FYQhm4dnm+yROfE0A, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Thanks for all the comments. Pawel, good point about the framebuffer. Fixing this will also address one of Russell's concerns. I don't know how to use a phandle, so I have some reading to do. Russell, all good points and exactly the type of feedback I needed. I'll re-work your suggestions into my code and re-post a V2 of the patches. Sascha, I see your point. I'll work on the feedback above first before getting to looking at how to handle the thread you linked to. Thanks all, Regards, Ryan. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 1/3] amba-clcd: Add Device Tree support to amba-clcd driver 2012-09-19 16:04 ` [RFC PATCH 1/3] amba-clcd: Add Device Tree support to amba-clcd driver Ryan Harkin 2012-09-20 10:24 ` Liviu Dudau [not found] ` <1348070666-9153-2-git-send-email-ryan.harkin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> @ 2012-10-10 10:13 ` Jon Medhurst (Tixy) 2 siblings, 0 replies; 14+ messages in thread From: Jon Medhurst (Tixy) @ 2012-10-10 10:13 UTC (permalink / raw) To: Ryan Harkin Cc: devicetree-discuss, linux-fbdev, liviu.dudau, linux-arm-kernel, pawel.moll On Wed, 2012-09-19 at 17:04 +0100, Ryan Harkin wrote: > Add support to parse the display configuration from device tree. > > If the board does not provide platform specific functions in the struct > clcd_board contained with the amba device info, then defaults are provided > by the driver. > > The device tree configuration can either ask for a DMA setup or provide a > framebuffer address to be remapped into the driver. > > Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org> > --- <big snip> > static int clcdfb_probe(struct amba_device *dev, const struct amba_id *id) > { > struct clcd_board *board = dev->dev.platform_data; > struct clcd_fb *fb; > int ret; > > +#ifdef CONFIG_OF > + if (dev->dev.of_node) { > + const __be32 *prop; > + int len, na, ns; > + phys_addr_t reg_base; > + > + na = of_n_addr_cells(dev->dev.of_node); > + ns = of_n_size_cells(dev->dev.of_node); > + > + prop = of_get_property(dev->dev.of_node, "reg", &len); > + if (WARN_ON(!prop || len < (na + ns) * sizeof(*prop))) > + return -EINVAL; > + reg_base = of_read_number(prop, na); > + > + if (dev->res.start != reg_base) > + return -EINVAL; When the motherboard CLCD is used, the CLCD node is under the iofga node and has a reg value equal to the offset from the start of the iofgpa, however dev->res.start holds the calculated address of the start of the iofga region plus the offset of the clcd, this means that the above check fails. The question is, what is the purpose of this check? Can't we rely on the value of dev->res.start being consistent with dev->dev.of_node and so just drop all the above code from "const __be32 *prop" onwards? -- Tixy ^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC PATCH 2/3] ARM: vexpress: Add device tree support for CLCD driver 2012-09-19 16:04 [RFC PATCH 0/3] amba-clcd: add device tree support Ryan Harkin 2012-09-19 16:04 ` [RFC PATCH 1/3] amba-clcd: Add Device Tree support to amba-clcd driver Ryan Harkin @ 2012-09-19 16:04 ` Ryan Harkin 2012-09-19 16:11 ` Pawel Moll 2012-09-19 16:04 ` [RFC PATCH 3/3] ARM: vexpress: configure CLCD driver device tree support for A9 CoreTile Ryan Harkin 2 siblings, 1 reply; 14+ messages in thread From: Ryan Harkin @ 2012-09-19 16:04 UTC (permalink / raw) To: ryan.harkin, arnd.bergmann, shiraz.hashim, stigge, pawel.moll, tixy, liviu.dudau, spear-devel, viresh.linux, linux-arm-kernel, linux-fbdev, devicetree-discuss Add support for device tree in the amba-clcd PL111 video driver. Special case support is added for the A9 CoreTile which uses the "legacy" address map and has a PL111 device on-board. The default case is to configure the device on the motherboard. Oscillator support is added for the A9 CoreTile's CLCD driver. Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org> --- arch/arm/mach-vexpress/v2m.c | 58 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c index 37608f2..799e00e 100644 --- a/arch/arm/mach-vexpress/v2m.c +++ b/arch/arm/mach-vexpress/v2m.c @@ -3,6 +3,7 @@ */ #include <linux/device.h> #include <linux/amba/bus.h> +#include <linux/amba/clcd.h> #include <linux/amba/mmci.h> #include <linux/io.h> #include <linux/init.h> @@ -37,6 +38,7 @@ #include <mach/ct-ca9x4.h> #include <mach/motherboard.h> +#include <plat/clcd.h> #include <plat/sched_clock.h> #include "core.h" @@ -541,6 +543,54 @@ MACHINE_END #if defined(CONFIG_ARCH_VEXPRESS_DT) +static struct v2m_osc v2m_dt_clcd_osc = { + .rate_min = 10000000, + .rate_max = 165000000, + .rate_default = 23750000, +}; + +static int v2m_dt_clcd_init(void) +{ + struct device_node *node; + u32 osc; + u32 clcd_site; + u32 dvimode; + const __be32 *prop; + int len, na, ns; + phys_addr_t reg_base; + + node = of_find_compatible_node(NULL, NULL, "arm,pl111"); + if (!node) + return -ENODEV; + + na = of_n_addr_cells(node); + ns = of_n_size_cells(node); + + prop = of_get_property(node, "reg", &len); + if (WARN_ON(!prop || len < (na + ns) * sizeof(*prop))) + return -EINVAL; + reg_base = of_read_number(prop, na); + + switch (reg_base) { + case CT_CA9X4_CLCDC: + clcd_site = v2m_get_master_site(); + dvimode = 2; + break; + default: + clcd_site = SYS_CFG_SITE_MB; + dvimode = 0; + break; + } + + if (of_property_read_u32(node, "arm,vexpress-osc", &osc) != 0) + return -EINVAL; + v2m_dt_clcd_osc.site = clcd_site; + v2m_dt_clcd_osc.osc = osc; + v2m_cfg_write(SYS_CFG_MUXFPGA | clcd_site, clcd_site); + v2m_cfg_write(SYS_CFG_DVIMODE | clcd_site, dvimode); + return 0; +} + static struct map_desc v2m_rs1_io_desc __initdata = { .virtual = V2M_PERIPH, .pfn = __phys_to_pfn(0x1c000000), @@ -598,6 +648,8 @@ void __init v2m_dt_init_early(void) pr_warning("vexpress: DT HBI (%x) is not matching " "hardware (%x)!\n", dt_hbi, hbi); } + + v2m_dt_clcd_init(); } static struct of_device_id vexpress_irq_match[] __initdata = { @@ -631,6 +683,12 @@ static void __init v2m_dt_timer_init(void) if (arch_timer_sched_clock_init() != 0) versatile_sched_clock_init(v2m_sysreg_base + V2M_SYS_24MHZ, 24000000); + + if (v2m_dt_clcd_osc.site) { + /* core tile clcd controller for A9 */ + clk = v2m_osc_register("10020000.clcd", &v2m_dt_clcd_osc); + clk_register_clkdev(clk, NULL, "10020000.clcd"); + } } static struct sys_timer v2m_dt_timer = { -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 2/3] ARM: vexpress: Add device tree support for CLCD driver 2012-09-19 16:04 ` [RFC PATCH 2/3] ARM: vexpress: Add device tree support for CLCD driver Ryan Harkin @ 2012-09-19 16:11 ` Pawel Moll 0 siblings, 0 replies; 14+ messages in thread From: Pawel Moll @ 2012-09-19 16:11 UTC (permalink / raw) To: Ryan Harkin Cc: stigge, tixy, linux-fbdev, devicetree-discuss, spear-devel, Liviu Dudau, shiraz.hashim, viresh.linux, arnd.bergmann, linux-arm-kernel On Wed, 2012-09-19 at 17:04 +0100, Ryan Harkin wrote: > Add support for device tree in the amba-clcd PL111 video driver. > > Special case support is added for the A9 CoreTile which uses the "legacy" > address map and has a PL111 device on-board. The default case is to configure > the device on the motherboard. > > Oscillator support is added for the A9 CoreTile's CLCD driver. > > Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org> > --- > arch/arm/mach-vexpress/v2m.c | 58 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 58 insertions(+) > > diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c > index 37608f2..799e00e 100644 > --- a/arch/arm/mach-vexpress/v2m.c > +++ b/arch/arm/mach-vexpress/v2m.c > @@ -3,6 +3,7 @@ > */ > #include <linux/device.h> > #include <linux/amba/bus.h> > +#include <linux/amba/clcd.h> > #include <linux/amba/mmci.h> > #include <linux/io.h> > #include <linux/init.h> > @@ -37,6 +38,7 @@ > #include <mach/ct-ca9x4.h> > #include <mach/motherboard.h> > > +#include <plat/clcd.h> > #include <plat/sched_clock.h> > > #include "core.h" > @@ -541,6 +543,54 @@ MACHINE_END > > #if defined(CONFIG_ARCH_VEXPRESS_DT) > > +static struct v2m_osc v2m_dt_clcd_osc = { > + .rate_min = 10000000, > + .rate_max = 165000000, > + .rate_default = 23750000, > +}; > + > +static int v2m_dt_clcd_init(void) > +{ > + struct device_node *node; > + u32 osc; > + u32 clcd_site; > + u32 dvimode; > + const __be32 *prop; > + int len, na, ns; > + phys_addr_t reg_base; > + > + node = of_find_compatible_node(NULL, NULL, "arm,pl111"); > + if (!node) > + return -ENODEV; > + > + na = of_n_addr_cells(node); > + ns = of_n_size_cells(node); > + > + prop = of_get_property(node, "reg", &len); > + if (WARN_ON(!prop || len < (na + ns) * sizeof(*prop))) > + return -EINVAL; > + reg_base = of_read_number(prop, na); > + > + switch (reg_base) { > + case CT_CA9X4_CLCDC: > + clcd_site = v2m_get_master_site(); > + dvimode = 2; > + break; > + default: > + clcd_site = SYS_CFG_SITE_MB; > + dvimode = 0; > + break; > + } > + > + if (of_property_read_u32(node, "arm,vexpress-osc", &osc) != 0) > + return -EINVAL; > + v2m_dt_clcd_osc.site = clcd_site; > + v2m_dt_clcd_osc.osc = osc; > + v2m_cfg_write(SYS_CFG_MUXFPGA | clcd_site, clcd_site); > + v2m_cfg_write(SYS_CFG_DVIMODE | clcd_site, dvimode); > + return 0; > +} > + > static struct map_desc v2m_rs1_io_desc __initdata = { > .virtual = V2M_PERIPH, > .pfn = __phys_to_pfn(0x1c000000), > @@ -598,6 +648,8 @@ void __init v2m_dt_init_early(void) > pr_warning("vexpress: DT HBI (%x) is not matching " > "hardware (%x)!\n", dt_hbi, hbi); > } > + > + v2m_dt_clcd_init(); > } > > static struct of_device_id vexpress_irq_match[] __initdata = { > @@ -631,6 +683,12 @@ static void __init v2m_dt_timer_init(void) > > if (arch_timer_sched_clock_init() != 0) > versatile_sched_clock_init(v2m_sysreg_base + V2M_SYS_24MHZ, 24000000); > + > + if (v2m_dt_clcd_osc.site) { > + /* core tile clcd controller for A9 */ > + clk = v2m_osc_register("10020000.clcd", &v2m_dt_clcd_osc); > + clk_register_clkdev(clk, NULL, "10020000.clcd"); > + } > } > > static struct sys_timer v2m_dt_timer = { When (if ;-) the changes I proposed recently make their way into mainline, all this stuff will not be necessary - both clocking and display control are sorted there. Pawel ^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC PATCH 3/3] ARM: vexpress: configure CLCD driver device tree support for A9 CoreTile 2012-09-19 16:04 [RFC PATCH 0/3] amba-clcd: add device tree support Ryan Harkin 2012-09-19 16:04 ` [RFC PATCH 1/3] amba-clcd: Add Device Tree support to amba-clcd driver Ryan Harkin 2012-09-19 16:04 ` [RFC PATCH 2/3] ARM: vexpress: Add device tree support for CLCD driver Ryan Harkin @ 2012-09-19 16:04 ` Ryan Harkin 2012-09-20 10:29 ` Liviu Dudau 2 siblings, 1 reply; 14+ messages in thread From: Ryan Harkin @ 2012-09-19 16:04 UTC (permalink / raw) To: ryan.harkin, arnd.bergmann, shiraz.hashim, stigge, pawel.moll, tixy, liviu.dudau, spear-devel, viresh.linux, linux-arm-kernel, linux-fbdev, devicetree-discuss Configuration for the amba-clcd PL111 driver is added to the A9 CoreTile's DTS file. Configuration of the motherboard CLCD driver is removed from the DTSI files to prevent duplicate CLCD drivers being registered. A generic set of CLCD panel descriptions has been split into its own DTSI file. Currently, only XVGA and VGA monitors are described. Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org> --- arch/arm/boot/dts/clcd-panels.dtsi | 52 +++++++++++++++++++++++++++++++ arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 8 ++--- arch/arm/boot/dts/vexpress-v2m.dtsi | 8 ++--- arch/arm/boot/dts/vexpress-v2p-ca9.dts | 6 ++++ 4 files changed, 62 insertions(+), 12 deletions(-) diff --git a/arch/arm/boot/dts/clcd-panels.dtsi b/arch/arm/boot/dts/clcd-panels.dtsi new file mode 100644 index 0000000..0b0ff6e --- /dev/null +++ b/arch/arm/boot/dts/clcd-panels.dtsi @@ -0,0 +1,52 @@ +/* + * ARM Ltd. Versatile Express + * + */ + +/ { + panels { + panel@0 { + compatible = "panel"; + mode = "VGA"; + refresh = <60>; + xres = <640>; + yres = <480>; + pixclock = <39721>; + left_margin = <40>; + right_margin = <24>; + upper_margin = <32>; + lower_margin = <11>; + hsync_len = <96>; + vsync_len = <2>; + sync = <0>; + vmode = "FB_VMODE_NONINTERLACED"; + + tim2 = "TIM2_BCD", "TIM2_IPC"; + cntl = "CNTL_LCDTFT", "CNTL_BGR", "CNTL_LCDVCOMP(1)"; + caps = "CLCD_CAP_5551", "CLCD_CAP_565", "CLCD_CAP_888"; + bpp = <16>; + }; + + panel@1 { + compatible = "panel"; + mode = "XVGA"; + refresh = <60>; + xres = <1024>; + yres = <768>; + pixclock = <15748>; + left_margin = <152>; + right_margin = <48>; + upper_margin = <23>; + lower_margin = <3>; + hsync_len = <104>; + vsync_len = <4>; + sync = <0>; + vmode = "FB_VMODE_NONINTERLACED"; + + tim2 = "TIM2_BCD", "TIM2_IPC"; + cntl = "CNTL_LCDTFT", "CNTL_BGR", "CNTL_LCDVCOMP(1)"; + caps = "CLCD_CAP_5551", "CLCD_CAP_565", "CLCD_CAP_888"; + bpp = <16>; + }; + }; +}; diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi index d8a827b..301d3f6 100644 --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi @@ -17,6 +17,8 @@ * CHANGES TO vexpress-v2m.dtsi! */ +/include/ "clcd-panels.dtsi" + / { aliases { arm,v2m_timer = &v2m_timer01; @@ -193,12 +195,6 @@ 0x1a0100 0xf00>; reg-shift = <2>; }; - - clcd@1f0000 { - compatible = "arm,pl111", "arm,primecell"; - reg = <0x1f0000 0x1000>; - interrupts = <14>; - }; }; v2m_fixed_3v3: fixedregulator@0 { diff --git a/arch/arm/boot/dts/vexpress-v2m.dtsi b/arch/arm/boot/dts/vexpress-v2m.dtsi index dba53fd..43cd86f 100644 --- a/arch/arm/boot/dts/vexpress-v2m.dtsi +++ b/arch/arm/boot/dts/vexpress-v2m.dtsi @@ -17,6 +17,8 @@ * CHANGES TO vexpress-v2m-rs1.dtsi! */ +/include/ "clcd-panels.dtsi" + / { aliases { arm,v2m_timer = &v2m_timer01; @@ -192,12 +194,6 @@ 0x1a100 0xf00>; reg-shift = <2>; }; - - clcd@1f000 { - compatible = "arm,pl111", "arm,primecell"; - reg = <0x1f000 0x1000>; - interrupts = <14>; - }; }; v2m_fixed_3v3: fixedregulator@0 { diff --git a/arch/arm/boot/dts/vexpress-v2p-ca9.dts b/arch/arm/boot/dts/vexpress-v2p-ca9.dts index 3f0c736..2ebb132 100644 --- a/arch/arm/boot/dts/vexpress-v2p-ca9.dts +++ b/arch/arm/boot/dts/vexpress-v2p-ca9.dts @@ -9,6 +9,8 @@ /dts-v1/; +/memreserve/ 0x9f000000 0x01000000; + / { model = "V2P-CA9"; arm,hbi = <0x191>; @@ -70,6 +72,10 @@ compatible = "arm,pl111", "arm,primecell"; reg = <0x10020000 0x1000>; interrupts = <0 44 4>; + mode = "XVGA"; + arm,vexpress-osc = <1>; + use_dma = <1>; + framebuffer = <0x9f000000 0x01000000>; }; memory-controller@100e0000 { -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 3/3] ARM: vexpress: configure CLCD driver device tree support for A9 CoreTile 2012-09-19 16:04 ` [RFC PATCH 3/3] ARM: vexpress: configure CLCD driver device tree support for A9 CoreTile Ryan Harkin @ 2012-09-20 10:29 ` Liviu Dudau 0 siblings, 0 replies; 14+ messages in thread From: Liviu Dudau @ 2012-09-20 10:29 UTC (permalink / raw) To: Ryan Harkin Cc: stigge, tixy, linux-fbdev, Pawel Moll, devicetree-discuss, spear-devel, shiraz.hashim, viresh.linux, arnd.bergmann, linux-arm-kernel On Wed, Sep 19, 2012 at 05:04:26PM +0100, Ryan Harkin wrote: > Configuration for the amba-clcd PL111 driver is added to the A9 CoreTile's DTS > file. > > Configuration of the motherboard CLCD driver is removed from the DTSI files to > prevent duplicate CLCD drivers being registered. > > A generic set of CLCD panel descriptions has been split into its own DTSI file. > Currently, only XVGA and VGA monitors are described. Hi Ryan, For this and [2/3] patch: Reviewed-by: Liviu Dudau <Liviu.Dudau@arm.com> > > Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org> > --- > arch/arm/boot/dts/clcd-panels.dtsi | 52 +++++++++++++++++++++++++++++++ > arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 8 ++--- > arch/arm/boot/dts/vexpress-v2m.dtsi | 8 ++--- > arch/arm/boot/dts/vexpress-v2p-ca9.dts | 6 ++++ > 4 files changed, 62 insertions(+), 12 deletions(-) > > diff --git a/arch/arm/boot/dts/clcd-panels.dtsi b/arch/arm/boot/dts/clcd-panels.dtsi > new file mode 100644 > index 0000000..0b0ff6e > --- /dev/null > +++ b/arch/arm/boot/dts/clcd-panels.dtsi > @@ -0,0 +1,52 @@ > +/* > + * ARM Ltd. Versatile Express > + * > + */ > + > +/ { > + panels { > + panel@0 { > + compatible = "panel"; > + mode = "VGA"; > + refresh = <60>; > + xres = <640>; > + yres = <480>; > + pixclock = <39721>; > + left_margin = <40>; > + right_margin = <24>; > + upper_margin = <32>; > + lower_margin = <11>; > + hsync_len = <96>; > + vsync_len = <2>; > + sync = <0>; > + vmode = "FB_VMODE_NONINTERLACED"; > + > + tim2 = "TIM2_BCD", "TIM2_IPC"; > + cntl = "CNTL_LCDTFT", "CNTL_BGR", "CNTL_LCDVCOMP(1)"; > + caps = "CLCD_CAP_5551", "CLCD_CAP_565", "CLCD_CAP_888"; > + bpp = <16>; > + }; > + > + panel@1 { > + compatible = "panel"; > + mode = "XVGA"; > + refresh = <60>; > + xres = <1024>; > + yres = <768>; > + pixclock = <15748>; > + left_margin = <152>; > + right_margin = <48>; > + upper_margin = <23>; > + lower_margin = <3>; > + hsync_len = <104>; > + vsync_len = <4>; > + sync = <0>; > + vmode = "FB_VMODE_NONINTERLACED"; > + > + tim2 = "TIM2_BCD", "TIM2_IPC"; > + cntl = "CNTL_LCDTFT", "CNTL_BGR", "CNTL_LCDVCOMP(1)"; > + caps = "CLCD_CAP_5551", "CLCD_CAP_565", "CLCD_CAP_888"; > + bpp = <16>; > + }; > + }; > +}; > diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi > index d8a827b..301d3f6 100644 > --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi > +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi > @@ -17,6 +17,8 @@ > * CHANGES TO vexpress-v2m.dtsi! > */ > > +/include/ "clcd-panels.dtsi" > + > / { > aliases { > arm,v2m_timer = &v2m_timer01; > @@ -193,12 +195,6 @@ > 0x1a0100 0xf00>; > reg-shift = <2>; > }; > - > - clcd@1f0000 { > - compatible = "arm,pl111", "arm,primecell"; > - reg = <0x1f0000 0x1000>; > - interrupts = <14>; > - }; > }; > > v2m_fixed_3v3: fixedregulator@0 { > diff --git a/arch/arm/boot/dts/vexpress-v2m.dtsi b/arch/arm/boot/dts/vexpress-v2m.dtsi > index dba53fd..43cd86f 100644 > --- a/arch/arm/boot/dts/vexpress-v2m.dtsi > +++ b/arch/arm/boot/dts/vexpress-v2m.dtsi > @@ -17,6 +17,8 @@ > * CHANGES TO vexpress-v2m-rs1.dtsi! > */ > > +/include/ "clcd-panels.dtsi" > + > / { > aliases { > arm,v2m_timer = &v2m_timer01; > @@ -192,12 +194,6 @@ > 0x1a100 0xf00>; > reg-shift = <2>; > }; > - > - clcd@1f000 { > - compatible = "arm,pl111", "arm,primecell"; > - reg = <0x1f000 0x1000>; > - interrupts = <14>; > - }; > }; > > v2m_fixed_3v3: fixedregulator@0 { > diff --git a/arch/arm/boot/dts/vexpress-v2p-ca9.dts b/arch/arm/boot/dts/vexpress-v2p-ca9.dts > index 3f0c736..2ebb132 100644 > --- a/arch/arm/boot/dts/vexpress-v2p-ca9.dts > +++ b/arch/arm/boot/dts/vexpress-v2p-ca9.dts > @@ -9,6 +9,8 @@ > > /dts-v1/; > > +/memreserve/ 0x9f000000 0x01000000; > + > / { > model = "V2P-CA9"; > arm,hbi = <0x191>; > @@ -70,6 +72,10 @@ > compatible = "arm,pl111", "arm,primecell"; > reg = <0x10020000 0x1000>; > interrupts = <0 44 4>; > + mode = "XVGA"; > + arm,vexpress-osc = <1>; > + use_dma = <1>; > + framebuffer = <0x9f000000 0x01000000>; > }; > > memory-controller@100e0000 { > -- > 1.7.9.5 > > -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯ -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-10-10 10:13 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-09-19 16:04 [RFC PATCH 0/3] amba-clcd: add device tree support Ryan Harkin 2012-09-19 16:04 ` [RFC PATCH 1/3] amba-clcd: Add Device Tree support to amba-clcd driver Ryan Harkin 2012-09-20 10:24 ` Liviu Dudau [not found] ` <20120920102453.GG32603-CibnQJhq84/ZROr8t4l/smS4ubULX0JqMm0uRHvK7Nw@public.gmane.org> 2012-09-21 10:35 ` Ryan Harkin 2012-09-21 10:44 ` Pawel Moll [not found] ` <1348070666-9153-2-git-send-email-ryan.harkin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2012-09-21 11:02 ` Russell King - ARM Linux 2012-09-21 11:43 ` Sascha Hauer [not found] ` <20120921114345.GE24458-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> 2012-09-21 12:19 ` Russell King - ARM Linux [not found] ` <20120921121903.GE15609-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org> 2012-09-24 7:07 ` Ryan Harkin 2012-10-10 10:13 ` Jon Medhurst (Tixy) 2012-09-19 16:04 ` [RFC PATCH 2/3] ARM: vexpress: Add device tree support for CLCD driver Ryan Harkin 2012-09-19 16:11 ` Pawel Moll 2012-09-19 16:04 ` [RFC PATCH 3/3] ARM: vexpress: configure CLCD driver device tree support for A9 CoreTile Ryan Harkin 2012-09-20 10:29 ` Liviu Dudau
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).