* Re: [RFC PATCH 1/3] amba-clcd: Add Device Tree support to amba-clcd driver
From: Ryan Harkin @ 2012-09-21 10:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20120920102453.GG32603@e102652-lin.cambridge.arm.com>
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.
>
^ permalink raw reply
* Re: [alsa-devel] [PATCH v2 00/34] i.MX multi-platform support
From: Olof Johansson @ 2012-09-21 8:26 UTC (permalink / raw)
To: Shawn Guo
Cc: alsa-devel, linux-usb, Artem Bityutskiy, linux-fbdev,
Wim Van Sebroeck, linux-mtd, linux-i2c, rtc-linux,
Florian Tobias Schandinat, Paulius Zaleckas, Chris Ball,
linux-media, linux-watchdog, Arnd Bergmann, Sascha Hauer,
Rob Herring, linux-arm-kernel, Vinod Koul, Greg Kroah-Hartman,
Mark Brown, linux-mmc, Wolfram Sang, Javier Martin, Andrew Morton
In-Reply-To: <20120921080123.GM2450@S2101-09.ap.freescale.net>
On Fri, Sep 21, 2012 at 1:01 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Thu, Sep 20, 2012 at 03:56:56PM +0000, Arnd Bergmann wrote:
>> Ok, fair enough. I think we can put it in arm-soc/for-next as a staging
>> branch anyway to give it some exposure to linux-next, and then we can
>> decide whether a rebase is necessary before sending it to Linus.
>>
> I just saw the announcement from Olof - no more major merge for 3.7
> will be accepted from now on. Can this be an exception or should I
> plan this for 3.8?
I'll take a look at merging it tomorrow after I've dealt with smp_ops;
if it looks reasonably conflict-free I'll pull it in. We need the
sound dependency sorted out (or agreed upon) first though.
-Olof
^ permalink raw reply
* Re: [alsa-devel] [PATCH v2 00/34] i.MX multi-platform support
From: Shawn Guo @ 2012-09-21 8:01 UTC (permalink / raw)
To: Arnd Bergmann, Olof Johansson
Cc: alsa-devel, Mark Brown, Artem Bityutskiy, linux-fbdev,
Wim Van Sebroeck, linux-mtd, linux-i2c, Florian Tobias Schandinat,
Paulius Zaleckas, Chris Ball, linux-media, linux-watchdog,
rtc-linux, Sascha Hauer, Rob Herring, linux-arm-kernel,
Vinod Koul, Greg Kroah-Hartman, linux-usb, linux-mmc,
Wolfram Sang, Javier Martin, Andrew Morton, Guennadi
In-Reply-To: <201209201556.57171.arnd@arndb.de>
On Thu, Sep 20, 2012 at 03:56:56PM +0000, Arnd Bergmann wrote:
> Ok, fair enough. I think we can put it in arm-soc/for-next as a staging
> branch anyway to give it some exposure to linux-next, and then we can
> decide whether a rebase is necessary before sending it to Linus.
>
I just saw the announcement from Olof - no more major merge for 3.7
will be accepted from now on. Can this be an exception or should I
plan this for 3.8?
Shawn
^ permalink raw reply
* Re: [PATCHv3 1/4] video: mmp display subsystem
From: Zhou Zhu @ 2012-09-21 7:50 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1347007704-27645-1-git-send-email-zzhu3@marvell.com>
Hi, Florian,
Would you please help to apply these patches?
Looks they have already been submitted for a long time, and no more
review comments recently.
Thank you for your help!
> On Fri, Sep 7, 2012 at 4:48 PM, Zhou Zhu <zzhu3@marvell.com> wrote:
>> Added mmp display subsystem to support Marvell MMP display controllers.
>>
>> This subsystem contains 4 parts:
>> --fb folder
>> --core.c
>> --hw folder
>> --panel folder
>>
>> 1. fb folder contains implementation of fb.
>> fb get path and ovly from common interface and operates on these structures.
>>
>> 2. core.c provides common interface for a hardware abstraction.
>> Major parts of this interface are:
>> a) Path: path is a output device connected to a panel or HDMI TV.
>> Main operations of the path is set/get timing/output color.
>> fb operates output device through path structure.
>> b) Ovly: Ovly is a buffer shown on the path.
>> Ovly describes frame buffer and its source/destination size, offset, input
>> color, buffer address, z-order, and so on.
>> Each fb device maps to one ovly.
>>
>> 3. hw folder contains implementation of hardware operations defined by core.c.
>> It registers paths for fb use.
>>
>> 4. panel folder contains implementation of panels.
>> It's connected to path. Panel drivers would also regiester panels and linked
>> to path when probe.
>>
>> Signed-off-by: Zhou Zhu <zzhu3@marvell.com>
>> Signed-off-by: Lisa Du <cldu@marvell.com>
>
> Thanks,
> -Zhou
--
Thanks,
-Zhou
^ permalink raw reply
* RE: [PATCH v2 2/2] video: exynos_mipi_dsi: Remove unnecessary NULL check
From: Inki Dae @ 2012-09-21 6:01 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1348204007-1347-2-git-send-email-sachin.kamat@linaro.org>
Acked-by: Inki Dae <inki.dae@samsung.com>
> -----Original Message-----
> From: Sachin Kamat [mailto:sachin.kamat@linaro.org]
> Sent: Friday, September 21, 2012 2:07 PM
> To: linux-fbdev@vger.kernel.org
> Cc: inki.dae@samsung.com; FlorianSchandinat@gmx.de;
> sachin.kamat@linaro.org; patches@linaro.org
> Subject: [PATCH v2 2/2] video: exynos_mipi_dsi: Remove unnecessary NULL
> check
>
> 'dsim' is allocated and checked for NULL in the probe function.
> Hence this check is redundant. This cleanup also fixes a potential NULL
> pointer dereference error when dsim which is NULL references its member
> in the error print message.
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
> drivers/video/exynos/exynos_mipi_dsi_common.c | 5 -----
> 1 files changed, 0 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_mipi_dsi_common.c
> b/drivers/video/exynos/exynos_mipi_dsi_common.c
> index 7cc4113..3cd29a4 100644
> --- a/drivers/video/exynos/exynos_mipi_dsi_common.c
> +++ b/drivers/video/exynos/exynos_mipi_dsi_common.c
> @@ -79,11 +79,6 @@ irqreturn_t exynos_mipi_dsi_interrupt_handler(int irq,
> void *dev_id)
> struct mipi_dsim_device *dsim = dev_id;
> unsigned int intsrc, intmsk;
>
> - if (dsim = NULL) {
> - dev_err(dsim->dev, "%s: wrong parameter\n", __func__);
> - return IRQ_NONE;
> - }
> -
> intsrc = exynos_mipi_dsi_read_interrupt(dsim);
> intmsk = exynos_mipi_dsi_read_interrupt_mask(dsim);
> intmsk = ~intmsk & intsrc;
> --
> 1.7.4.1
^ permalink raw reply
* RE: [PATCH v2 1/2] video: exynos_mipi_dsi: Remove unused code
From: Inki Dae @ 2012-09-21 6:01 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1348204007-1347-1-git-send-email-sachin.kamat@linaro.org>
Acked-by: Inki Dae <inki.dae@samsung.com>
> -----Original Message-----
> From: Sachin Kamat [mailto:sachin.kamat@linaro.org]
> Sent: Friday, September 21, 2012 2:07 PM
> To: linux-fbdev@vger.kernel.org
> Cc: inki.dae@samsung.com; FlorianSchandinat@gmx.de;
> sachin.kamat@linaro.org; patches@linaro.org
> Subject: [PATCH v2 1/2] video: exynos_mipi_dsi: Remove unused code
>
> This code is never executed and hence removed.
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> Acked-by: Inki Dae <inki.dae@samsung.com>
>
> ---
> drivers/video/exynos/exynos_mipi_dsi_common.c | 3 ---
> 1 files changed, 0 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_mipi_dsi_common.c
> b/drivers/video/exynos/exynos_mipi_dsi_common.c
> index 47b533a..7cc4113 100644
> --- a/drivers/video/exynos/exynos_mipi_dsi_common.c
> +++ b/drivers/video/exynos/exynos_mipi_dsi_common.c
> @@ -288,9 +288,6 @@ int exynos_mipi_dsi_wr_data(struct mipi_dsim_device
> *dsim, unsigned int data_id,
> mutex_unlock(&dsim->lock);
> return -EINVAL;
> }
> -
> - mutex_unlock(&dsim->lock);
> - return 0;
> }
>
> static unsigned int exynos_mipi_dsi_long_data_rd(struct mipi_dsim_device
> *dsim,
> --
> 1.7.4.1
^ permalink raw reply
* [PATCH v2 2/2] video: exynos_mipi_dsi: Remove unnecessary NULL check
From: Sachin Kamat @ 2012-09-21 5:18 UTC (permalink / raw)
To: linux-fbdev
'dsim' is allocated and checked for NULL in the probe function.
Hence this check is redundant. This cleanup also fixes a potential NULL
pointer dereference error when dsim which is NULL references its member
in the error print message.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
drivers/video/exynos/exynos_mipi_dsi_common.c | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/drivers/video/exynos/exynos_mipi_dsi_common.c b/drivers/video/exynos/exynos_mipi_dsi_common.c
index 7cc4113..3cd29a4 100644
--- a/drivers/video/exynos/exynos_mipi_dsi_common.c
+++ b/drivers/video/exynos/exynos_mipi_dsi_common.c
@@ -79,11 +79,6 @@ irqreturn_t exynos_mipi_dsi_interrupt_handler(int irq, void *dev_id)
struct mipi_dsim_device *dsim = dev_id;
unsigned int intsrc, intmsk;
- if (dsim = NULL) {
- dev_err(dsim->dev, "%s: wrong parameter\n", __func__);
- return IRQ_NONE;
- }
-
intsrc = exynos_mipi_dsi_read_interrupt(dsim);
intmsk = exynos_mipi_dsi_read_interrupt_mask(dsim);
intmsk = ~intmsk & intsrc;
--
1.7.4.1
^ permalink raw reply related
* [PATCH v2 1/2] video: exynos_mipi_dsi: Remove unused code
From: Sachin Kamat @ 2012-09-21 5:18 UTC (permalink / raw)
To: linux-fbdev
This code is never executed and hence removed.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Acked-by: Inki Dae <inki.dae@samsung.com>
---
drivers/video/exynos/exynos_mipi_dsi_common.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/drivers/video/exynos/exynos_mipi_dsi_common.c b/drivers/video/exynos/exynos_mipi_dsi_common.c
index 47b533a..7cc4113 100644
--- a/drivers/video/exynos/exynos_mipi_dsi_common.c
+++ b/drivers/video/exynos/exynos_mipi_dsi_common.c
@@ -288,9 +288,6 @@ int exynos_mipi_dsi_wr_data(struct mipi_dsim_device *dsim, unsigned int data_id,
mutex_unlock(&dsim->lock);
return -EINVAL;
}
-
- mutex_unlock(&dsim->lock);
- return 0;
}
static unsigned int exynos_mipi_dsi_long_data_rd(struct mipi_dsim_device *dsim,
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCHv4 7/9] arm: vt8500: doc: Add device tree bindings for arch-vt8500 devices
From: Rob Herring @ 2012-09-20 23:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1345707346-9035-8-git-send-email-linux@prisktech.co.nz>
On 08/23/2012 02:35 AM, Tony Prisk wrote:
> Bindings for gpio, interrupt controller, power management controller,
> timer, realtime clock, serial uart, ehci and uhci controllers and
> framebuffer controllers used on the arch-vt8500 platform.
>
> Framebuffer binding also specifies a 'display' node which is required
> for determining the lcd panel data.
Overall, these bindings look okay, but there is some effort to
standardize a binding for displays:
http://comments.gmane.org/gmane.linux.drivers.devicetree/21386
I would like to see that used rather than something custom.
Rob
^ permalink raw reply
* Re: [PATCH 2/5] drivers/video/gbefb.c: use devm_ functions
From: Florian Tobias Schandinat @ 2012-09-20 21:58 UTC (permalink / raw)
To: Geert Uytterhoeven, Damien Cassou
Cc: kernel-janitors, linux-fbdev, linux-kernel
In-Reply-To: <CAMuHMdWsiXmt=2uo3C8ZC_apnHRV2aHi3yZrYV_jgwB22=3Mfw@mail.gmail.com>
Hi Geert,
On 09/13/2012 07:06 PM, Geert Uytterhoeven wrote:
> On Thu, Aug 23, 2012 at 10:37 PM, Florian Tobias Schandinat
> <FlorianSchandinat@gmx.de> wrote:
>> On 07/31/2012 04:39 PM, Damien Cassou wrote:
>>> From: Damien Cassou <damien.cassou@lifl.fr>
>>>
>>> The various devm_ functions allocate memory that is released when a driver
>>> detaches. This patch uses these functions for data that is allocated in
>>> the probe function of a platform device and is only freed in the remove
>>> function.
>>>
>>> Signed-off-by: Damien Cassou <damien.cassou@lifl.fr>
>>
>> Applied.
>
>>> diff --git a/drivers/video/gbefb.c b/drivers/video/gbefb.c
>>> index 7e7b7a9..9b79d38 100644
>>> --- a/drivers/video/gbefb.c
>>> +++ b/drivers/video/gbefb.c
>>> @@ -1156,7 +1156,8 @@ static int __devinit gbefb_probe(struct platform_device *p_dev)
>>> goto out_release_framebuffer;
>>> }
>>>
>>> - gbe = (struct sgi_gbe *) ioremap(GBE_BASE, sizeof(struct sgi_gbe));
>>> + gbe = (struct sgi_gbe *) devm_ioremap(&p_dev->dev, GBE_BASE,
>>> + sizeof(struct sgi_gbe));
>
> drivers/video/gbefb.c:1159:16: error: implicit declaration of function
> 'devm_ioremap' [-Werror=implicit-function-declaration]
> drivers/video/gbefb.c:1179:3: error: implicit declaration of function
> 'devm_ioremap_nocache' [-Werror=implicit-function-declaration]
Thanks for pointing this out. I guess the solution is to replace the
include asm/io.h by linux/io.h as Axel Lin did for some other driver in
this patch series.
@Damien:
Can you please do a patch as I suggested and preferable also recheck the
other drivers you touched?
And it probably would have been a good idea to mention that you didn't
compile-test it. I do some compile tests before pushing my branch but
that is far from complete due to arch and platform specific drivers.
Best regards,
Florian Tobias Schandinat
>
> http://kisskb.ellerman.id.au/kisskb/buildresult/7187731/
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
^ permalink raw reply
* Re: [PATCH v4] of: Add videomode helper
From: Laurent Pinchart @ 2012-09-20 21:29 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Steffen Trumtrar, devicetree-discuss, linux-fbdev, dri-devel,
kernel, Sascha Hauer, linux-media
In-Reply-To: <1348046362.2565.16.camel@deskari>
Hi,
(CC'ing the linux-media mailing list, as video modes are of interest there as
well)
On Wednesday 19 September 2012 12:19:22 Tomi Valkeinen wrote:
> On Wed, 2012-09-19 at 10:20 +0200, Steffen Trumtrar wrote:
> > This patch adds a helper function for parsing videomodes from the
> > devicetree. The videomode can be either converted to a struct
> > drm_display_mode or a struct fb_videomode.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > ---
> >
> > Hi!
> >
> > changes since v3:
> > - print error messages
> > - free alloced memory
> > - general cleanup
> >
> > Regards
> > Steffen
> >
> > .../devicetree/bindings/video/displaymode | 74 +++++
> > drivers/of/Kconfig | 5 +
> > drivers/of/Makefile | 1 +
> > drivers/of/of_videomode.c | 283 +++++++++++++++
> > include/linux/of_videomode.h | 56 ++++
> > 5 files changed, 419 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/video/displaymode
> > create mode 100644 drivers/of/of_videomode.c
> > create mode 100644 include/linux/of_videomode.h
> >
> > diff --git a/Documentation/devicetree/bindings/video/displaymode
> > b/Documentation/devicetree/bindings/video/displaymode new file mode
> > 100644
> > index 0000000..990ca52
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/displaymode
> > @@ -0,0 +1,74 @@
> > +videomode bindings
> > +=========
> > +
> > +Required properties:
> > + - hactive, vactive: Display resolution
> > + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing
> > parameters
> > + in pixels
> > + vfront-porch, vback-porch, vsync-len: Vertical display timing
> > parameters in
> > + lines
> > + - clock: displayclock in Hz
> > +
> > +Optional properties:
> > + - width-mm, height-mm: Display dimensions in mm
> > + - hsync-active-high (bool): Hsync pulse is active high
> > + - vsync-active-high (bool): Vsync pulse is active high
> > + - interlaced (bool): This is an interlaced mode
> > + - doublescan (bool): This is a doublescan mode
> > +
> > +There are different ways of describing a display mode. The devicetree
> > representation
> > +corresponds to the one commonly found in datasheets for displays.
> > +The description of the display and its mode is split in two parts: first
> > the display
> > +properties like size in mm and (optionally) multiple subnodes with the
> > supported modes.
> > +
> > +Example:
> > +
> > + display@0 {
> > + width-mm = <800>;
> > + height-mm = <480>;
> > + modes {
> > + mode0: mode@0 {
> > + /* 1920x1080p24 */
> > + clock = <52000000>;
> > + hactive = <1920>;
> > + vactive = <1080>;
> > + hfront-porch = <25>;
> > + hback-porch = <25>;
> > + hsync-len = <25>;
> > + vback-porch = <2>;
> > + vfront-porch = <2>;
> > + vsync-len = <2>;
> > + hsync-active-high;
> > + };
> > + };
> > + };
> > +
> > +Every property also supports the use of ranges, so the commonly used
> > datasheet +description with <min typ max>-tuples can be used.
> > +
> > +Example:
> > +
> > + mode1: mode@1 {
> > + /* 1920x1080p24 */
> > + clock = <148500000>;
> > + hactive = <1920>;
> > + vactive = <1080>;
> > + hsync-len = <0 44 60>;
> > + hfront-porch = <80 88 95>;
> > + hback-porch = <100 148 160>;
> > + vfront-porch = <0 4 6>;
> > + vback-porch = <0 36 50>;
> > + vsync-len = <0 5 6>;
> > + };
> > +
> > +The videomode can be linked to a connector via phandles. The connector
> > has to
> > +support the display- and default-mode-property to link to and select a
> > mode.
> > +
> > +Example:
> > +
> > + hdmi@00120000 {
> > + status = "okay";
> > + display = <&benq>;
> > + default-mode = <&mode1>;
> > + };
> > +
> > diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> > index dfba3e6..a3acaa3 100644
> > --- a/drivers/of/Kconfig
> > +++ b/drivers/of/Kconfig
> > @@ -83,4 +83,9 @@ config OF_MTD
> >
> > depends on MTD
> > def_bool y
> >
> > +config OF_VIDEOMODE
> > + def_bool y
> > + help
> > + helper to parse videomodes from the devicetree
> > +
> >
> > endmenu # OF
> >
> > diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> > index e027f44..80e6db3 100644
> > --- a/drivers/of/Makefile
> > +++ b/drivers/of/Makefile
> > @@ -11,3 +11,4 @@ obj-$(CONFIG_OF_MDIO) += of_mdio.o
> >
> > obj-$(CONFIG_OF_PCI) += of_pci.o
> > obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
> > obj-$(CONFIG_OF_MTD) += of_mtd.o
> >
> > +obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
> > diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
> > new file mode 100644
> > index 0000000..52bfc74
> > --- /dev/null
> > +++ b/drivers/of/of_videomode.c
> > @@ -0,0 +1,283 @@
> > +/*
> > + * OF helpers for parsing display modes
> > + *
> > + * Copyright (c) 2012 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
> > + * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>,
> > Pengutronix
> > + *
> > + * This file is released under the GPLv2
> > + */
> > +#include <linux/of.h>
> > +#include <linux/fb.h>
> > +#include <linux/export.h>
> > +#include <linux/slab.h>
> > +#include <drm/drmP.h>
> > +#include <drm/drm_crtc.h>
> > +#include <linux/of_videomode.h>
> > +
> > +static u32 of_video_get_value(struct mode_property *prop)
> > +{
> > + return (prop->min >= prop->typ) ? prop->min : prop->typ;
> > +}
>
> Why is this needed? If the prop->min is higher than prop->typ, isn't
> that an error in the DT data, and should be rejected when parsing?
>
> > +
> > +/* read property into new mode_property */
> > +static int of_video_parse_property(struct device_node *np, char *name,
> > + struct mode_property *result)
> > +{
> > + struct property *prop;
> > + int length;
> > + int cells;
> > + int ret;
> > +
> > + prop = of_find_property(np, name, &length);
> > + if (!prop) {
> > + pr_err("%s: could not find property %s\n", __func__,
> > + name);
> > + return -EINVAL;
> > + }
> > +
> > + cells = length / sizeof(u32);
> > +
> > + memset(result, 0, sizeof(*result));
> > +
> > + ret = of_property_read_u32_array(np, name, &result->min, cells);
> > +
> > + return ret;
>
> Ah, I guess this is the reason for the of_video_get_value... Wouldn't it
> be cleaner to be more explicit here? If there's one cell, it's the
> typical value, otherwise if there are 3 cells, they are min/typ/max,
> else it's an error.
I agree. We should flag errors and fail instead of trying to fix them
silently. That's the only way to make sure that device tree authors will get
it right (or at least less wrong). We might need to accept wrong DT data later
to support buggy devices found in the wild, but we should not start that way.
> And I think the above also trashes memory if there happens to be 4+ cells.
>
> > +}
> > +
> > +static int of_video_free(struct display *disp)
> > +{
> > + int i;
> > +
> > + for (i=0; i<disp->num_modes; i++)
Spaces around = and < please.
> > + kfree(disp->modes[i]);
> > + kfree(disp->modes);
> > +
> > + return 0;
> > +}
> > +
> > +int videomode_to_display_mode(struct display *disp, struct
> > drm_display_mode *dmode, int index)
Shouldn't this use drm_mode_modeinfo instead of drm_display_mode ?
> > +{
> > + struct videomode *vm;
> > +
> > + memset(dmode, 0, sizeof(*dmode));
> > +
> > + if (index > disp->num_modes) {
> > + pr_err("%s: wrong index: %d from %d\n", __func__, index,
> > disp->num_modes); + return -EINVAL;
> > + }
> > +
> > + vm = disp->modes[index];
> > +
> > + dmode->hdisplay = of_video_get_value(&vm->hactive);
> > + dmode->hsync_start = of_video_get_value(&vm->hactive) +
> > of_video_get_value(&vm->hfront_porch);
You could replace of_video_get_value(&vm->hactive) with dmode->hdisplay here,
and similarly below (hsync_end = hsync_start + of_video_get_value(&vm-
>hsync_len), ...). Beside shortening lines, it would save calls to
of_video_get_value().
> > + dmode->hsync_end = of_video_get_value(&vm->hactive) +
> > of_video_get_value(&vm->hfront_porch)
> > + + of_video_get_value(&vm->hsync_len);
> > + dmode->htotal = of_video_get_value(&vm->hactive) +
> > of_video_get_value(&vm->hfront_porch)
> > + + of_video_get_value(&vm->hsync_len) +
> > of_video_get_value(&vm->hback_porch);
> > + dmode->vdisplay = of_video_get_value(&vm->vactive);
> > + dmode->vsync_start = of_video_get_value(&vm->vactive) +
> > of_video_get_value(&vm->vfront_porch); + dmode->vsync_end > > of_video_get_value(&vm->vactive) + of_video_get_value(&vm->vfront_porch)
> > + + of_video_get_value(&vm->vsync_len);
> > + dmode->vtotal = of_video_get_value(&vm->vactive) +
> > of_video_get_value(&vm->vfront_porch) +
> > + of_video_get_value(&vm->vsync_len) +
> > of_video_get_value(&vm->vback_porch); +
> > + dmode->width_mm = disp->width_mm;
> > + dmode->height_mm = disp->height_mm;
> > +
> > + dmode->clock = of_video_get_value(&vm->clock) / 1000;
> > +
> > + if (vm->hah)
> > + dmode->flags |= DRM_MODE_FLAG_PHSYNC;
> > + else
> > + dmode->flags |= DRM_MODE_FLAG_NHSYNC;
> > + if (vm->vah)
> > + dmode->flags |= DRM_MODE_FLAG_PVSYNC;
> > + else
> > + dmode->flags |= DRM_MODE_FLAG_NVSYNC;
> > + if (vm->interlaced)
> > + dmode->flags |= DRM_MODE_FLAG_INTERLACE;
> > + if (vm->doublescan)
> > + dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
> > +
> > + drm_mode_set_name(dmode);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(videomode_to_display_mode);
> > +
> > +int videomode_to_fb_mode(struct display *disp, struct fb_videomode
> > *fbmode, int index)
disp is only used to retrieve the mode, you could pass a struct videomode
instead of disp and index.
Thinking about it, I would make videomode_to_display_mode take a struct
videomode as well. The only reason it needs struct display is to get the
display physical dimensions. Those are only used in very specific cases in the
DRM subsystem, I don't think they should be copied to every struct
drm_mode_modeinfo.
> > +{
> > + struct videomode *vm;
> > +
> > + memset(fbmode, 0, sizeof(*fbmode));
> > +
> > + if (index > disp->num_modes) {
> > + pr_err("%s: wrong index: %d from %d\n", __func__, index,
> > disp->num_modes);
> > + return -EINVAL;
> > + }
> > +
> > + vm = disp->modes[index];
> > +
> > + fbmode->xres = of_video_get_value(&vm->hactive);
> > + fbmode->left_margin = of_video_get_value(&vm->hback_porch);
> > + fbmode->right_margin = of_video_get_value(&vm->hfront_porch);
> > + fbmode->hsync_len = of_video_get_value(&vm->hsync_len);
> > +
> > + fbmode->yres = of_video_get_value(&vm->vactive);
> > + fbmode->upper_margin = of_video_get_value(&vm->vback_porch);
> > + fbmode->lower_margin = of_video_get_value(&vm->vfront_porch);
> > + fbmode->vsync_len = of_video_get_value(&vm->vsync_len);
> > +
> > + fbmode->pixclock = KHZ2PICOS(of_video_get_value(&vm->clock) / 1000);
> > +
> > + if (vm->hah)
> > + fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> > + if (vm->vah)
> > + fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> > + if (vm->interlaced)
> > + fbmode->vmode |= FB_VMODE_INTERLACED;
> > + if (vm->doublescan)
> > + fbmode->vmode |= FB_VMODE_DOUBLE;
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(videomode_to_fb_mode);
> > +
> > +int of_get_video_modes(struct device_node *np, struct display *disp)
> > +{
> > + struct device_node *display_np;
> > + struct device_node *mode_np;
> > + struct device_node *modes_np;
> > + char *default_mode;
> > +
> > + int ret = 0;
> > +
> > + memset(disp, 0, sizeof(*disp));
> > + disp->modes = kmalloc(sizeof(struct videomode*), GFP_KERNEL);
> > +
> > + if (!np) {
> > + pr_err("%s: no node provided\n", __func__);
> > + return -EINVAL;
> > + }
> > +
> > + display_np = of_parse_phandle(np, "display", 0);
> > +
> > + if (!display_np) {
> > + pr_err("%s: could not find display node\n", __func__);
> > + return -EINVAL;
> > + }
> > +
> > + of_property_read_u32(display_np, "width-mm", &disp->width_mm);
> > + of_property_read_u32(display_np, "height-mm", &disp->height_mm);
> > +
> > + mode_np = of_parse_phandle(np, "default-mode", 0);
> > +
> > + if (!mode_np) {
> > + pr_info("%s: no default-mode specified.\n", __func__);
> > + mode_np = of_find_node_by_name(np, "mode");
> > + }
> > +
> > + if (!mode_np) {
> > + pr_err("%s: could not find any mode specification\n", __func__);
> > + return -EINVAL;
> > + }
> > +
> > + default_mode = (char *)mode_np->full_name;
> > +
> > + modes_np = of_find_node_by_name(np, "modes");
> > + for_each_child_of_node(modes_np, mode_np) {
> > + struct videomode *vm;
> > +
> > + vm = kmalloc(sizeof(struct videomode*), GFP_KERNEL);
> > + disp->modes[disp->num_modes] = kmalloc(sizeof(struct videomode*),
> > GFP_KERNEL);
> > +
> > + ret |= of_video_parse_property(mode_np, "hback-porch",
> > &vm->hback_porch);
> > + ret |= of_video_parse_property(mode_np, "hfront-porch",
> > &vm->hfront_porch);
> > + ret |= of_video_parse_property(mode_np, "hactive", &vm->hactive);
> > + ret |= of_video_parse_property(mode_np, "hsync-len",
> > &vm->hsync_len);
> > + ret |= of_video_parse_property(mode_np, "vback-porch",
> > &vm->vback_porch);
> > + ret |= of_video_parse_property(mode_np, "vfront-porch",
> > &vm->vfront_porch);
> > + ret |= of_video_parse_property(mode_np, "vactive", &vm->vactive);
> > + ret |= of_video_parse_property(mode_np, "vsync-len",
> > &vm->vsync_len);
> > + ret |= of_video_parse_property(mode_np, "clock", &vm->clock);
> > +
> > + if (ret)
> > + return -EINVAL;
> > +
> > + vm->hah = of_property_read_bool(mode_np, "hsync-active-high");
> > + vm->vah = of_property_read_bool(mode_np, "vsync-active-high");
> > + vm->interlaced = of_property_read_bool(mode_np, "interlaced");
> > + vm->doublescan = of_property_read_bool(mode_np, "doublescan");
> > +
> > + if (strcmp(default_mode,mode_np->full_name) = 0)
> > + disp->default_mode = disp->num_modes;
> > +
> > + disp->modes[disp->num_modes] = vm;
> > + disp->num_modes++;
> > + }
> > + of_node_put(display_np);
> > +
> > + pr_info("%s: found %d modelines. Using #%d as default\n", __func__,
> > + disp->num_modes, disp->default_mode + 1);
> > +
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(of_get_video_modes);
> > +
> > +int of_video_mode_exists(struct device_node *np)
> > +{
> > + struct device_node *display_np;
> > + struct device_node *mode_np;
> > +
> > + if (!np)
> > + return -EINVAL;
> > +
> > + display_np = of_parse_phandle(np, "display", 0);
> > +
> > + if (!display_np)
> > + return -EINVAL;
> > +
> > + mode_np = of_parse_phandle(np, "default-mode", 0);
> > +
> > + if (mode_np)
> > + return 0;
> > +
> > + return -EINVAL;
> > +}
> > +EXPORT_SYMBOL_GPL(of_video_mode_exists);
> > +
> > +int of_get_display_mode(struct device_node *np, struct drm_display_mode
> > *dmode, int index)
> > +{
> > + struct display disp;
> > +
> > + of_get_video_modes(np, &disp);
> > +
> > + if (index = OF_MODE_SELECTION)
> > + index = disp.default_mode;
> > + if (dmode)
> > + videomode_to_display_mode(&disp, dmode, index);
> > +
> > + of_video_free(&disp);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(of_get_display_mode);
> > +
> > +int of_get_fb_videomode(struct device_node *np, struct fb_videomode
> > *fbmode, int index)
> > +{
> > + struct display disp;
> > +
> > + of_get_video_modes(np, &disp);
> > +
> > + if (index = OF_MODE_SELECTION)
> > + index = disp.default_mode;
> > + if (fbmode)
> > + videomode_to_fb_mode(&disp, fbmode, index);
> > +
> > + of_video_free(&disp);
> > +
> > + return 0;
>
> This and of_get_display_mode() do not handle errors from
> of_get_video_modes() nor from videomode_to_xxx_mode(). And I don't see a
> reason for the if (fbmode) check (and the same for dmode), as there's no
> reason to call these functions except to get the video modes.
>
> > +}
> > +EXPORT_SYMBOL_GPL(of_get_fb_videomode);
What are those two functions used for ? Aren't drivers expected to parse the
modes into a struct display and then operate on that structure, instead of
getting individual modes from the DT node ?
> > diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h
> > new file mode 100644
> > index 0000000..5571ce3
> > --- /dev/null
> > +++ b/include/linux/of_videomode.h
> > @@ -0,0 +1,56 @@
> > +/*
> > + * Copyright 2012 Sascha Hauer <s.hauer@pengutronix.de>
> > + * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > + *
> > + * OF helpers for videomodes.
> > + *
> > + * This file is released under the GPLv2
> > + */
> > +
> > +#ifndef __LINUX_OF_VIDEOMODE_H
> > +#define __LINUX_OF_VIDEOMODE_H
> > +
> > +#define OF_MODE_SELECTION -1
> > +
> > +struct mode_property {
> > + u32 min;
> > + u32 typ;
> > + u32 max;
> > +};
> > +
> > +struct display {
> > + u32 width_mm;
> > + u32 height_mm;
> > + struct videomode **modes;
> > + int default_mode;
> > + int num_modes;
default_mode and num_modes are non-negative, what about using an unsigned int
type for them ?
> > +};
> > +
> > +/* describe videomode in terms of hardware parameters */
> > +struct videomode {
> > + struct mode_property hback_porch;
> > + struct mode_property hfront_porch;
> > + struct mode_property hactive;
> > + struct mode_property hsync_len;
> > +
> > + struct mode_property vback_porch;
> > + struct mode_property vfront_porch;
> > + struct mode_property vactive;
> > + struct mode_property vsync_len;
> > +
> > + struct mode_property clock;
> > +
> > + bool hah;
> > + bool vah;
> > + bool interlaced;
> > + bool doublescan;
> > +};
>
> I think the names display and videomode are a bit too generic here, and
> could clash with names from kernel drivers. Also, the videomode is not
> really a videomode, but a "template" (or something) for videomode. A
> real videomode doesn't have min & max values, only the actual value.
>
> Perhaps these should be prefixed with "of_"? Then again, they are not
> really of specific either...
If feel this is an important topic.
A generic video mode structure is definitely needed, as well as helper
functions to convert between the new structure and existing video mode
structures (struct fb_videomode, struct drm_mode_modeinfo and struct
v4l2_bt_timings). The structure and the helper functions should be generic, as
the goal is to gradually replace subsystem-specific video mode structures
where possible (userspace APIs will still need to keep the old structures). We
thus need to drop the dependency on OF for everything but the DT parsing code.
For that reason an of_ prefix wouldn't be a good idea. As the goal is to
create a truly generic video mode structure, a generic name is a good idea (I
might prefer struct video_mode instead of struct videomode, but that's just
bikeshedding).
We might need two video mode structures, one to represent a video mode, and
one to represent a range of video modes. I don't have enough experience with
video modes to have a really strong opinion on this, having a single structure
to represent both could be useful as well. This particular topic needs to be
discussed.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH] pwm: Call pwm_enable() before pwm_config()
From: Mark Brown @ 2012-09-20 19:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20120823190402.GA8127@avionic-0098.mockup.avionic-design.de>
On Thu, Aug 23, 2012 at 09:04:02PM +0200, Thierry Reding wrote:
> Using runtime PM for this sounds indeed like the most generic approach.
> I'm not very familiar with the API, but I thought it required explicit
> architecture or bus support (or the driver itself can provide hooks via
> pm_ops), so it might be difficult to implement for platforms that don't
> have working runtime PM support. I'll have to look into this some more.
It doesn't require explicit arch support, though many architectures
choose to integrate their PM frameworks with it either directly or via
power domains. If the architecture doesn't do anything with it then it
should just work at the driver level.
^ permalink raw reply
* Re: [PATCH] pwm: Call pwm_enable() before pwm_config()
From: Mark Brown @ 2012-09-20 19:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50366464.4070801@metafoo.de>
On Thu, Aug 23, 2012 at 07:12:04PM +0200, Lars-Peter Clausen wrote:
> On 08/23/2012 06:57 PM, Beno??t Th??baudeau wrote:
> > exists with several other PWM drivers.
> Since this seems to be a common pattern in a number of PWM drivers it might
> make sense to simply add support for enabling/disabling a clk to the pwm core.
> Or maybe just use the runtime pm API for this. This probably makes even more
> sense and grab a reference to the pm context when the enable() is called,
> release it when disable() is called and also grab it before calling the
> device's config callback and release it afterward.
It's a problem with IPs in general, a good proportion of modern SoCs
need a clock supplying in order to interact with the registers on most
of their IPs. runtime PM is the normal way forwards, Tegra recently
started using regmap's cache code to handle this transparently in the
driver.
^ permalink raw reply
* Re: [PATCH 07/29] ARM: OMAP1: Move lcd_mipid.h from plat to mach
From: Tony Lindgren @ 2012-09-20 18:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20120919210552.28074.21443.stgit@muffinssi>
* Tony Lindgren <tony@atomide.com> [120919 14:07]:
> This is only used by omap1.
This too is handled by Arnd's series already, dropping this one.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH 04/29] ARM: OMAP1: Move board-ams-delta.h from plat to mach
From: Tony Lindgren @ 2012-09-20 17:12 UTC (permalink / raw)
To: Mark Brown
Cc: linux-arm-kernel, linux-fbdev, Janusz Krzysztofik,
Artem Bityutskiy, Dmitry Torokhov, alsa-devel, Peter Ujfalusi,
Tomi Valkeinen, linux-mtd, linux-input, linux-omap, Liam Girdwood
In-Reply-To: <20120920011630.GB17666@opensource.wolfsonmicro.com>
* Mark Brown <broonie@opensource.wolfsonmicro.com> [120919 19:05]:
> On Wed, Sep 19, 2012 at 02:05:43PM -0700, Tony Lindgren wrote:
> > This is only used by omap1.
> >
> > And to fix things properly, this should not be included
> > from the drivers at all.
>
> Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
>
> or I can apply it if that's easier.
Thanks I'll do a series for Arnd on top of the multiplatform
changes to avoid more merge conflicts.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH 04/29] ARM: OMAP1: Move board-ams-delta.h from plat to mach
From: Tony Lindgren @ 2012-09-20 17:10 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: linux-arm-kernel, linux-fbdev, Mark Brown, Artem Bityutskiy,
Dmitry Torokhov, alsa-devel, Peter Ujfalusi, Tomi Valkeinen,
linux-mtd, linux-input, linux-omap, Liam Girdwood
In-Reply-To: <6194989.y5R5mbkCAc@acer>
* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [120920 06:48]:
> On Wed, 19 Sep 2012 14:05:43 Tony Lindgren wrote:
> > This is only used by omap1.
> >
> > And to fix things properly, this should not be included
> > from the drivers at all.
>
> Akced-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
>
> I'll take care of updating the drivers when I have some spare time.
Thanks those can be fixed by passing them in the platform data.
Regards,
Tony
^ permalink raw reply
* [PATCH 2/2] ARM: dts: mxs: add oled support for the cfa-10036
From: Maxime Ripard @ 2012-09-20 16:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1348158800-23039-1-git-send-email-maxime.ripard@free-electrons.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Brian Lilly <brian@crystalfontz.com>
---
arch/arm/boot/dts/imx28-cfa10036.dts | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/arch/arm/boot/dts/imx28-cfa10036.dts b/arch/arm/boot/dts/imx28-cfa10036.dts
index c03a577..e5ee46e 100644
--- a/arch/arm/boot/dts/imx28-cfa10036.dts
+++ b/arch/arm/boot/dts/imx28-cfa10036.dts
@@ -33,11 +33,31 @@
};
apbx@80040000 {
+ pwm: pwm@80064000 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm4_pins_a>;
+ status = "okay";
+ };
+
duart: serial@80074000 {
pinctrl-names = "default";
pinctrl-0 = <&duart_pins_b>;
status = "okay";
};
+
+ i2c0: i2c@80058000 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c0_pins_b>;
+ status = "okay";
+
+ ssd1307: oled@3c {
+ compatible = "solomon,ssd1307fb-i2c";
+ reg = <0x3c>;
+ pwms = <&pwm 4 3000>;
+ reset-gpios = <&gpio2 7 1>;
+ reset-active-low;
+ };
+ };
};
};
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/2] video: Add support for the Solomon SSD1307 OLED Controller
From: Maxime Ripard @ 2012-09-20 16:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1348158800-23039-1-git-send-email-maxime.ripard@free-electrons.com>
This patch adds support for the Solomon SSD1307 OLED
controller found on the Crystalfontz CFA10036 board.
This controller can drive a display with a resolution up
to 128x39 and can operate over I2C or SPI.
The current driver has only been tested on the CFA-10036,
that is using this controller over I2C to driver a 96x16
OLED screen.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Brian Lilly <brian@crystalfontz.com>
---
.../devicetree/bindings/video/ssd1307fb.txt | 24 ++
drivers/video/Kconfig | 13 +
drivers/video/Makefile | 1 +
drivers/video/ssd1307fb.c | 407 ++++++++++++++++++++
4 files changed, 445 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/ssd1307fb.txt
create mode 100644 drivers/video/ssd1307fb.c
diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt b/Documentation/devicetree/bindings/video/ssd1307fb.txt
new file mode 100644
index 0000000..3d0060c
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/ssd1307fb.txt
@@ -0,0 +1,24 @@
+* Solomon SSD1307 Framebuffer Driver
+
+Required properties:
+ - compatible: Should be "solomon,ssd1307fb-<bus>". The only supported bus for
+ now is i2c.
+ - reg: Should contain address of the controller on the I2C bus. Most likely
+ 0x3c or 0x3d
+ - pwm: Should contain the pwm to use according to the OF device tree PWM
+ specification [0]
+ - reset-gpios: Should contain the GPIO used to reset the OLED display
+
+Optional properties:
+ - reset-active-low: Is the reset gpio is active on physical low?
+
+[0]: Documentation/devicetree/bindings/pwm/pwm.txt
+
+Examples:
+ssd1307: oled@3c {
+ compatible = "solomon,ssd1307fb-i2c";
+ reg = <0x3c>;
+ pwms = <&pwm 4 3000>;
+ reset-gpios = <&gpio2 7>;
+ reset-active-low;
+};
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 0217f74..2776829 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2469,4 +2469,17 @@ config FB_SH_MOBILE_MERAM
Up to 4 memory channels can be configured, allowing 4 RGB or
2 YCbCr framebuffers to be configured.
+config FB_SSD1307
+ tristate "Solomon SSD1307 framebuffer support"
+ depends on FB && I2C
+ select FB_SYS_FOPS
+ select FB_SYS_FILLRECT
+ select FB_SYS_COPYAREA
+ select FB_SYS_IMAGEBLIT
+ select FB_DEFERRED_IO
+ select PWM
+ help
+ This driver implements support for the Solomon SSD1307
+ OLED controller over I2C.
+
endmenu
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index ee8dafb..6bbb72c 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -164,6 +164,7 @@ obj-$(CONFIG_FB_BFIN_7393) += bfin_adv7393fb.o
obj-$(CONFIG_FB_MX3) += mx3fb.o
obj-$(CONFIG_FB_DA8XX) += da8xx-fb.o
obj-$(CONFIG_FB_MXS) += mxsfb.o
+obj-$(CONFIG_FB_SSD1307) += ssd1307fb.o
# the test framebuffer is last
obj-$(CONFIG_FB_VIRTUAL) += vfb.o
diff --git a/drivers/video/ssd1307fb.c b/drivers/video/ssd1307fb.c
new file mode 100644
index 0000000..49e42ed
--- /dev/null
+++ b/drivers/video/ssd1307fb.c
@@ -0,0 +1,407 @@
+/*
+ * Driver for the Solomon SSD1307 OLED controler
+ *
+ * Copyright 2012 Free Electrons
+ *
+ * Licensed under the GPLv2 or later.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/i2c.h>
+#include <linux/fb.h>
+#include <linux/uaccess.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/pwm.h>
+#include <linux/delay.h>
+
+#define SSD1307FB_WIDTH 96
+#define SSD1307FB_HEIGHT 16
+
+#define SSD1307FB_DATA 0x40
+#define SSD1307FB_COMMAND 0x80
+
+#define SSD1307FB_CONTRAST 0x81
+#define SSD1307FB_SEG_REMAP_ON 0xa1
+#define SSD1307FB_DISPLAY_OFF 0xae
+#define SSD1307FB_DISPLAY_ON 0xaf
+#define SSD1307FB_START_PAGE_ADDRESS 0xb0
+
+struct ssd1307fb_par {
+ struct i2c_client *client;
+ struct fb_info *info;
+ struct pwm_device *pwm;
+ u32 pwm_period;
+ int reset;
+};
+
+static struct fb_fix_screeninfo ssd1307fb_fix __devinitdata = {
+ .id = "Solomon SSD1307",
+ .type = FB_TYPE_PACKED_PIXELS,
+ .visual = FB_VISUAL_MONO10,
+ .xpanstep = 0,
+ .ypanstep = 0,
+ .ywrapstep = 0,
+ .line_length = SSD1307FB_WIDTH / 8,
+ .accel = FB_ACCEL_NONE,
+};
+
+static struct fb_var_screeninfo ssd1307fb_var __devinitdata = {
+ .xres = SSD1307FB_WIDTH,
+ .yres = SSD1307FB_HEIGHT,
+ .xres_virtual = SSD1307FB_WIDTH,
+ .yres_virtual = SSD1307FB_HEIGHT,
+ .bits_per_pixel = 1,
+};
+
+static int ssd1307fb_write_array(struct i2c_client *client, u8 type, u8* cmd, u32 len)
+{
+ u8 *buf;
+ int ret = 0;
+
+ buf = kzalloc(len + 1, GFP_KERNEL);
+ if (!buf) {
+ dev_err(&client->dev, "Couldn't allocate sending buffer.\n");
+ return -ENOMEM;
+ }
+
+ buf[0] = type;
+ memcpy(buf + 1, cmd, len);
+
+ ret = i2c_master_send(client, buf, len + 1);
+ if (ret != len + 1) {
+ dev_err(&client->dev, "Couldn't send I2C command.\n");
+ goto error;
+ }
+
+error:
+ kfree(buf);
+ return ret;
+}
+
+static inline int ssd1307fb_write_cmd_array(struct i2c_client *client, u8* cmd, u32 len)
+{
+ return ssd1307fb_write_array(client, SSD1307FB_COMMAND, cmd, len);
+}
+
+static inline int ssd1307fb_write_cmd(struct i2c_client *client, u8 cmd)
+{
+ return ssd1307fb_write_cmd_array(client, &cmd, 1);
+}
+
+static inline int ssd1307fb_write_data_array(struct i2c_client *client, u8* cmd, u32 len)
+{
+ return ssd1307fb_write_array(client, SSD1307FB_DATA, cmd, len);
+}
+
+static inline int ssd1307fb_write_data(struct i2c_client *client, u8 data)
+{
+ return ssd1307fb_write_data_array(client, &data, 1);
+}
+
+static void ssd1307fb_update_display(struct ssd1307fb_par *par)
+{
+ u8 *vmem = par->info->screen_base;
+ int i, j, k;
+
+ /*
+ * The screen is divided in pages, each having a height of 8
+ * pixels, and the width of the screen. When sending a byte of
+ * data to the controller, it gives the 8 bits for the current
+ * column. I.e, the first byte are the 8 bits of the first
+ * column, then the 8 bits for the second column, etc.
+ *
+ *
+ * Representation of the screen, assuming it is 5 bits
+ * wide. Each letter-number combination is a bit that controls
+ * one pixel.
+ *
+ * A0 A1 A2 A3 A4
+ * B0 B1 B2 B3 B4
+ * C0 C1 C2 C3 C4
+ * D0 D1 D2 D3 D4
+ * E0 E1 E2 E3 E4
+ * F0 F1 F2 F3 F4
+ * G0 G1 G2 G3 G4
+ * H0 H1 H2 H3 H4
+ *
+ * If you want to update this screen, you need to send 5 bytes:
+ * (1) A0 B0 C0 D0 E0 F0 G0 H0
+ * (2) A1 B1 C1 D1 E1 F1 G1 H1
+ * (3) A2 B2 C2 D2 E2 F2 G2 H2
+ * (4) A3 B3 C3 D3 E3 F3 G3 H3
+ * (5) A4 B4 C4 D4 E4 F4 G4 H4
+ */
+
+ for (i = 0; i < (SSD1307FB_HEIGHT / 8); i++) {
+ ssd1307fb_write_cmd(par->client, SSD1307FB_START_PAGE_ADDRESS + (i + 1));
+ ssd1307fb_write_cmd(par->client, 0x00);
+ ssd1307fb_write_cmd(par->client, 0x10);
+
+ for (j = 0; j < SSD1307FB_WIDTH; j++) {
+ u8 buf = 0;
+ for (k = 0; k < 8; k++) {
+ u32 page_length = SSD1307FB_WIDTH * i;
+ u32 index = page_length + (SSD1307FB_WIDTH * k + j) / 8;
+ u8 byte = *(vmem + index);
+ u8 bit = byte & (1 << (7 - (j % 8)));
+ bit = bit >> (7 - (j % 8));
+ buf |= bit << k;
+ }
+ ssd1307fb_write_data(par->client, buf);
+ }
+ }
+}
+
+
+static ssize_t ssd1307fb_write(struct fb_info *info, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct ssd1307fb_par *par = info->par;
+ unsigned long total_size;
+ unsigned long p = *ppos;
+ u8 __iomem *dst;
+ int err = 0;
+
+ total_size = info->screen_size;
+
+ if (total_size = 0)
+ total_size = info->fix.smem_len;
+
+ if (p > total_size)
+ return -EFBIG;
+
+ if (count > total_size) {
+ err = -EFBIG;
+ count = total_size;
+ }
+
+ if (count + p > total_size) {
+ if (!err)
+ err = -ENOSPC;
+
+ count = total_size - p;
+ }
+
+ dst = (void __force *) (info->screen_base + p);
+
+ if (copy_from_user(dst, buf, count))
+ err = -EFAULT;
+
+ if (!err)
+ *ppos += count;
+
+ ssd1307fb_update_display(par);
+
+ return (err) ? err : count;
+}
+
+static void ssd1307fb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
+{
+ struct ssd1307fb_par *par = info->par;
+ sys_fillrect(info, rect);
+ ssd1307fb_update_display(par);
+}
+
+static void ssd1307fb_copyarea(struct fb_info *info, const struct fb_copyarea *area)
+{
+ struct ssd1307fb_par *par = info->par;
+ sys_copyarea(info, area);
+ ssd1307fb_update_display(par);
+}
+
+static void ssd1307fb_imageblit(struct fb_info *info, const struct fb_image *image)
+{
+ struct ssd1307fb_par *par = info->par;
+ sys_imageblit(info, image);
+ ssd1307fb_update_display(par);
+}
+
+static struct fb_ops ssd1307fb_ops = {
+ .owner = THIS_MODULE,
+ .fb_read = fb_sys_read,
+ .fb_write = ssd1307fb_write,
+ .fb_fillrect = ssd1307fb_fillrect,
+ .fb_copyarea = ssd1307fb_copyarea,
+ .fb_imageblit = ssd1307fb_imageblit,
+};
+
+static void ssd1307fb_deferred_io(struct fb_info *info,
+ struct list_head *pagelist)
+{
+ ssd1307fb_update_display(info->par);
+}
+
+static struct fb_deferred_io ssd1307fb_defio = {
+ .delay = HZ,
+ .deferred_io = ssd1307fb_deferred_io,
+};
+
+static int __devinit ssd1307fb_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+ struct fb_info *info;
+ u32 vmem_size = SSD1307FB_WIDTH * SSD1307FB_HEIGHT / 8;
+ struct ssd1307fb_par *par;
+ u8 *vmem;
+ int ret;
+
+ if (!client->dev.of_node) {
+ dev_err(&client->dev, "No device tree data found!\n");
+ return -EINVAL;
+ }
+
+ info = framebuffer_alloc(sizeof(struct ssd1307fb_par), &client->dev);
+ if (!info) {
+ dev_err(&client->dev, "Couldn't allocate framebuffer.\n");
+ return -ENOMEM;
+ }
+
+ vmem = devm_kzalloc(&client->dev, vmem_size, GFP_KERNEL);
+ if (!vmem) {
+ dev_err(&client->dev, "Couldn't allocate graphical memory.\n");
+ ret = -ENOMEM;
+ goto fb_alloc_error;
+ }
+
+ info->fbops = &ssd1307fb_ops;
+ info->fix = ssd1307fb_fix;
+ info->fbdefio = &ssd1307fb_defio;
+
+ info->var = ssd1307fb_var;
+ info->var.red.length = 1;
+ info->var.red.offset = 0;
+ info->var.green.length = 1;
+ info->var.green.offset = 0;
+ info->var.blue.length = 1;
+ info->var.blue.offset = 0;
+
+ info->screen_base = (u8 __force __iomem *)vmem;
+ info->fix.smem_start = (unsigned long)vmem;
+ info->fix.smem_len = vmem_size;
+
+ fb_deferred_io_init(info);
+
+ par = info->par;
+ par->info = info;
+ par->client = client;
+
+ par->reset = of_get_named_gpio(client->dev.of_node,
+ "reset-gpios", 0);
+ if (gpio_is_valid(par->reset)) {
+ int flags = GPIOF_OUT_INIT_HIGH;
+ if (of_get_property(client->dev.of_node,
+ "reset-active-low", NULL))
+ flags = GPIOF_OUT_INIT_LOW;
+ ret = devm_gpio_request_one(&client->dev, par->reset,
+ flags, "oled-reset");
+ if (ret) {
+ dev_err(&client->dev,
+ "failed to request gpio %d: %d\n",
+ par->reset, ret);
+ goto reset_oled_error;
+ }
+ }
+
+ par->pwm = pwm_get(&client->dev, NULL);
+ if (IS_ERR(par->pwm)) {
+ dev_err(&client->dev, "Could not get PWM from device tree!\n");
+ ret = PTR_ERR(par->pwm);
+ goto pwm_error;
+ }
+
+ par->pwm_period = pwm_get_period(par->pwm);
+
+ dev_dbg(&client->dev, "Using PWM%d with a %dns period.\n", par->pwm->pwm, par->pwm_period);
+
+ ret = register_framebuffer(info);
+ if (ret) {
+ dev_err(&client->dev, "Couldn't register the framebuffer\n");
+ goto fbreg_error;
+ }
+
+ i2c_set_clientdata(client, info);
+
+ /* Reset the screen */
+ gpio_set_value(par->reset, 1);
+ udelay(4);
+ gpio_set_value(par->reset, 0);
+ udelay(4);
+
+ /* Enable the PWM */
+ pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
+ pwm_enable(par->pwm);
+
+ /* Map column 127 of the OLED to segment 0 */
+ ret = ssd1307fb_write_cmd(client, SSD1307FB_SEG_REMAP_ON);
+ if (ret < 0) {
+ dev_err(&client->dev, "Couldn't remap the screen.\n");
+ goto remap_error;
+ }
+
+ /* Turn on the display */
+ ret = ssd1307fb_write_cmd(client, SSD1307FB_DISPLAY_ON);
+ if (ret < 0) {
+ dev_err(&client->dev, "Couldn't turn the display on.\n");
+ goto remap_error;
+ }
+
+ dev_info(&client->dev, "fb%d: %s framebuffer device registered, using %d bytes of video memory\n", info->node, info->fix.id, vmem_size);
+
+ return 0;
+
+remap_error:
+ unregister_framebuffer(info);
+ pwm_disable(par->pwm);
+fbreg_error:
+ pwm_put(par->pwm);
+pwm_error:
+reset_oled_error:
+ fb_deferred_io_cleanup(info);
+fb_alloc_error:
+ framebuffer_release(info);
+ return ret;
+}
+
+static int __devexit ssd1307fb_remove(struct i2c_client *client)
+{
+ struct fb_info *info = i2c_get_clientdata(client);
+ struct ssd1307fb_par *par = info->par;
+
+ unregister_framebuffer(info);
+ pwm_disable(par->pwm);
+ pwm_put(par->pwm);
+ fb_deferred_io_cleanup(info);
+ framebuffer_release(info);
+
+ return 0;
+}
+
+static const struct i2c_device_id ssd1307fb_i2c_id[] = {
+ { "ssd1307fb", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, ssd1307fb_i2c_id);
+
+static const struct of_device_id ssd1307fb_of_match[] = {
+ { .compatible = "solomon,ssd1307fb-i2c" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ssd1307fb_of_match);
+
+static struct i2c_driver ssd1307fb_driver = {
+ .probe = ssd1307fb_probe,
+ .remove = __devexit_p(ssd1307fb_remove),
+ .id_table = ssd1307fb_i2c_id,
+ .driver = {
+ .name = "ssd1307fb",
+ .of_match_table = of_match_ptr(ssd1307fb_of_match),
+ .owner = THIS_MODULE,
+ },
+};
+
+module_i2c_driver(ssd1307fb_driver);
+
+MODULE_DESCRIPTION("FB driver for the Solomon SSD1307 OLED controler");
+MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
+MODULE_LICENSE("GPL");
--
1.7.9.5
^ permalink raw reply related
* [PATCHv5 0/2] Add support for the OLED in the CFA10036
From: Maxime Ripard @ 2012-09-20 16:33 UTC (permalink / raw)
To: linux-arm-kernel
Hi everyone,
This patchset adds support for the solomon SSD1307 OLED controller present
in the CFA-10036 board.
It first adds the framebuffer driver for this controller, and then the
needed bits to enable it in the cfa10036 dts.
Considering that the driver has been around since almost two months now, and
I have not received any comments on the framebuffer part, I'd very much like it
to be included in 3.7 if possible.
Thanks,
Maxime
Changes from v4:
* Fix kernel crash when writing more data than the internal buffer space by
adding boundary check for the fb_write function
Changes from v3:
* Changed the dt property name to remove the leading oled- and changed the
reset-gpio property to reset-gpios
Changes from v2:
* Add the pin name as comment to the pwm4 pinctrl declaration
* Change slightly the documentation of the dt bindings for the gpios to remove
the mxs-specific cell.
Changes from v1:
* Did some factorisation of the ssd1307fb_write* functions.
* Filled the smem_start field of the fb_fix_screeninfo structure
* Fixed the broken pwm declaration
* Change the names of oled-reset-gpios and reset-active-low properties in the
device tree to oled-reset-gpio and oled-reset-active-low for consistency
Maxime Ripard (2):
video: Add support for the Solomon SSD1307 OLED Controller
ARM: dts: mxs: add oled support for the cfa-10036
.../devicetree/bindings/video/ssd1307fb.txt | 24 ++
arch/arm/boot/dts/imx28-cfa10036.dts | 20 +
drivers/video/Kconfig | 13 +
drivers/video/Makefile | 1 +
drivers/video/ssd1307fb.c | 407 ++++++++++++++++++++
5 files changed, 465 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/ssd1307fb.txt
create mode 100644 drivers/video/ssd1307fb.c
--
1.7.9.5
^ permalink raw reply
* Re: [PATCH v2 00/34] i.MX multi-platform support
From: Arnd Bergmann @ 2012-09-20 15:56 UTC (permalink / raw)
To: Shawn Guo
Cc: alsa-devel, Mark Brown, Artem Bityutskiy, linux-fbdev,
Wim Van Sebroeck, linux-mtd, linux-i2c, Florian Tobias Schandinat,
Paulius Zaleckas, Chris Ball, linux-media, linux-watchdog,
rtc-linux, Sascha Hauer, Rob Herring, linux-arm-kernel,
Vinod Koul, Greg Kroah-Hartman, linux-usb, linux-mmc,
Wolfram Sang, Javier Martin, Andrew Morton, Guennadi
In-Reply-To: <20120920145342.GI2450@S2101-09.ap.freescale.net>
On Thursday 20 September 2012, Shawn Guo wrote:
>
> On Thu, Sep 20, 2012 at 07:39:34AM +0000, Arnd Bergmann wrote:
> > The first five branches are scheduled to go through the arm-soc tree, so
> > I'm fine with that. For the sound/for-3.7 branch, I'd like to know when
> > to expect that hitting mainline. If it always gets in very early during the
> > merge window, it's probably ok to put the imx/multi-platform patches into
> > the same branch as the other ones in arm-soc and wait for the sound stuff
> > to hit mainline first, otherwise I'd suggest we start a second
> > next/multiplatform-2 branch for imx and send the first part early on
> > but then wait with the second batch before sound gets in.
> >
> It seems that we will have to go with next/multiplatform-2. I just
> tried to merge the series with linux-next together, and got some
> non-trivial conflicts with media and mtd tree. I might have to rebase
> my series on top of these trees to sort out those conflicts. That said,
> I will have several dependencies outside arm-soc tree, and have to pend
> my series until all those trees get merged into mainline.
Ok, fair enough. I think we can put it in arm-soc/for-next as a staging
branch anyway to give it some exposure to linux-next, and then we can
decide whether a rebase is necessary before sending it to Linus.
Arnd
^ permalink raw reply
* Re: [PATCH v2 00/34] i.MX multi-platform support
From: Shawn Guo @ 2012-09-20 14:53 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sascha Hauer,
Javier Martin, Rob Herring, Mark Brown,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Florian Tobias Schandinat,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Chris Ball,
linux-mmc-u79uwXL29TY76Z2rM5mHXA, Guennadi Liakhovetski,
linux-media-u79uwXL29TY76Z2rM5mHXA, Andrew Morton,
rtc-linux-/JYPxA39Uh5TLH3MbocFFw, Artem Bityutskiy,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Wolfram Sang,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, Wim Van Sebroeck,
linux-watchdog-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Vinod Koul, Paulius
In-Reply-To: <201209200739.34899.arnd-r2nGTMty4D4@public.gmane.org>
On Thu, Sep 20, 2012 at 07:39:34AM +0000, Arnd Bergmann wrote:
> The first five branches are scheduled to go through the arm-soc tree, so
> I'm fine with that. For the sound/for-3.7 branch, I'd like to know when
> to expect that hitting mainline. If it always gets in very early during the
> merge window, it's probably ok to put the imx/multi-platform patches into
> the same branch as the other ones in arm-soc and wait for the sound stuff
> to hit mainline first, otherwise I'd suggest we start a second
> next/multiplatform-2 branch for imx and send the first part early on
> but then wait with the second batch before sound gets in.
>
It seems that we will have to go with next/multiplatform-2. I just
tried to merge the series with linux-next together, and got some
non-trivial conflicts with media and mtd tree. I might have to rebase
my series on top of these trees to sort out those conflicts. That said,
I will have several dependencies outside arm-soc tree, and have to pend
my series until all those trees get merged into mainline.
Shawn
^ permalink raw reply
* Re: [PATCH 04/29] ARM: OMAP1: Move board-ams-delta.h from plat to mach
From: Janusz Krzysztofik @ 2012-09-20 13:41 UTC (permalink / raw)
To: Tony Lindgren
Cc: linux-arm-kernel, linux-fbdev, Mark Brown, Artem Bityutskiy,
Dmitry Torokhov, alsa-devel, Peter Ujfalusi, Tomi Valkeinen,
linux-mtd, linux-input, linux-omap, Liam Girdwood
In-Reply-To: <20120919210543.28074.10214.stgit@muffinssi>
On Wed, 19 Sep 2012 14:05:43 Tony Lindgren wrote:
> This is only used by omap1.
>
> And to fix things properly, this should not be included
> from the drivers at all.
Akced-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
I'll take care of updating the drivers when I have some spare time.
Thanks,
Janusz
^ permalink raw reply
* Re: [PATCH v2 00/34] i.MX multi-platform support
From: Shawn Guo @ 2012-09-20 13:36 UTC (permalink / raw)
To: Mark Brown
Cc: Arnd Bergmann, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Sascha Hauer, Javier Martin, Rob Herring,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Florian Tobias Schandinat,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Chris Ball,
linux-mmc-u79uwXL29TY76Z2rM5mHXA, Guennadi Liakhovetski,
linux-media-u79uwXL29TY76Z2rM5mHXA, Andrew Morton,
rtc-linux-/JYPxA39Uh5TLH3MbocFFw, Artem Bityutskiy,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Wolfram Sang,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, Wim Van Sebroeck,
linux-watchdog-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
Paulius Zaleckas <paulius.zalecka>
In-Reply-To: <20120920124709.GN17666-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
On Thu, Sep 20, 2012 at 08:47:10AM -0400, Mark Brown wrote:
> On Thu, Sep 20, 2012 at 07:52:15PM +0800, Shawn Guo wrote:
> > On Thu, Sep 20, 2012 at 07:41:50AM -0400, Mark Brown wrote:
>
> > > It's usually pretty early but Takashi will be on holiday this time so
> > > I'm not sure if things might be different (he was going to send the pull
> > > request from holiday). I also didn't guarantee that it'll be stable
> > > yet, can someone please tell me what the depenency is here?
>
> > We need the patch to have all imx drivers mach/* inclusion free,
> > so that we can enable multi-platform support for imx, which is the
> > whole point of the series.
>
> That doesn't answer the question. What is the dependency - what is it
> about this patch that something else depends on? Your cover letters
> just say you'd like to do this but don't mention dependencies at all and
> when I asked the question last night you said the same thing. I've not
> seen the rest of the series...
>
Ah, right. You did get copied on the whole series. So the whole point
of patch "ASoC: mx27vis: retrieve gpio numbers from platform_data" is
to get rid of inclusion mach/iomux-mx27.h. This has to be done before
we enable multi-platform support for imx, since mach/* has to be removed
completely for multi-platform build.
> > If your for-3.7 is not stable anyway, I guess the easiest the way
>
> It probably *is* stable but I'm not enthused about people pulling
> unsigned tags. I might rebase, though - I'm going to finalise the tree
> in the next few days.
>
> > to do it might be you drop the patch "ASoC: mx27vis: retrieve gpio
> > numbers from platform_data" from your tree and I have it be part of
> > the series to go via arm-soc tree as a whole. (This is the original
> > plan that I mentioned in v1 cover letter)
>
> You just mentioned it as a preference (you said it's something you'd
> like to do), please if you're doing this sort of cross tree thing be
> explicit about what the inter-tree relationships are. If things need to
> go in via the same tree say so explicitly (and ideally say way this is).
>
Ok, my bad.
> The main reason I applied it straight away was that Javier mentioned
> that it was a bug fix and it's near the merge window and these random
> ARM cleanup serieses never seem to go in quickly.
The series is planned for 3.7 merge window. Is it still possible for
you to drop the patch from your tree to ease the process? Or I will
hold my pull-request to arm-soc until you tell me you have your tree
finalized.
Shawn
^ permalink raw reply
* Re: [PATCH v2 00/34] i.MX multi-platform support
From: Mark Brown @ 2012-09-20 12:47 UTC (permalink / raw)
To: Shawn Guo
Cc: Arnd Bergmann, linux-arm-kernel, Sascha Hauer, Javier Martin,
Rob Herring, alsa-devel, Florian Tobias Schandinat, linux-fbdev,
Chris Ball, linux-mmc, Guennadi Liakhovetski, linux-media,
Andrew Morton, rtc-linux, Artem Bityutskiy, linux-mtd,
Wolfram Sang, linux-i2c, Wim Van Sebroeck, linux-watchdog,
Greg Kroah-Hartman, linux-usb, Vinod Koul,
Paulius Zaleckas <paulius.zalecka>
In-Reply-To: <20120920115213.GF2450@S2101-09.ap.freescale.net>
On Thu, Sep 20, 2012 at 07:52:15PM +0800, Shawn Guo wrote:
> On Thu, Sep 20, 2012 at 07:41:50AM -0400, Mark Brown wrote:
> > It's usually pretty early but Takashi will be on holiday this time so
> > I'm not sure if things might be different (he was going to send the pull
> > request from holiday). I also didn't guarantee that it'll be stable
> > yet, can someone please tell me what the depenency is here?
> We need the patch to have all imx drivers mach/* inclusion free,
> so that we can enable multi-platform support for imx, which is the
> whole point of the series.
That doesn't answer the question. What is the dependency - what is it
about this patch that something else depends on? Your cover letters
just say you'd like to do this but don't mention dependencies at all and
when I asked the question last night you said the same thing. I've not
seen the rest of the series...
> If your for-3.7 is not stable anyway, I guess the easiest the way
It probably *is* stable but I'm not enthused about people pulling
unsigned tags. I might rebase, though - I'm going to finalise the tree
in the next few days.
> to do it might be you drop the patch "ASoC: mx27vis: retrieve gpio
> numbers from platform_data" from your tree and I have it be part of
> the series to go via arm-soc tree as a whole. (This is the original
> plan that I mentioned in v1 cover letter)
You just mentioned it as a preference (you said it's something you'd
like to do), please if you're doing this sort of cross tree thing be
explicit about what the inter-tree relationships are. If things need to
go in via the same tree say so explicitly (and ideally say way this is).
The main reason I applied it straight away was that Javier mentioned
that it was a bug fix and it's near the merge window and these random
ARM cleanup serieses never seem to go in quickly.
^ permalink raw reply
* Re: [PATCH v2 00/34] i.MX multi-platform support
From: Shawn Guo @ 2012-09-20 11:52 UTC (permalink / raw)
To: Mark Brown
Cc: Arnd Bergmann, linux-arm-kernel, Sascha Hauer, Javier Martin,
Rob Herring, alsa-devel, Florian Tobias Schandinat, linux-fbdev,
Chris Ball, linux-mmc, Guennadi Liakhovetski, linux-media,
Andrew Morton, rtc-linux, Artem Bityutskiy, linux-mtd,
Wolfram Sang, linux-i2c, Wim Van Sebroeck, linux-watchdog,
Greg Kroah-Hartman, linux-usb, Vinod Koul,
Paulius Zaleckas <paulius.zalecka>
In-Reply-To: <20120920114148.GH17666@opensource.wolfsonmicro.com>
On Thu, Sep 20, 2012 at 07:41:50AM -0400, Mark Brown wrote:
> It's usually pretty early but Takashi will be on holiday this time so
> I'm not sure if things might be different (he was going to send the pull
> request from holiday). I also didn't guarantee that it'll be stable
> yet, can someone please tell me what the depenency is here?
We need the patch to have all imx drivers mach/* inclusion free,
so that we can enable multi-platform support for imx, which is the
whole point of the series.
If your for-3.7 is not stable anyway, I guess the easiest the way
to do it might be you drop the patch "ASoC: mx27vis: retrieve gpio
numbers from platform_data" from your tree and I have it be part of
the series to go via arm-soc tree as a whole. (This is the original
plan that I mentioned in v1 cover letter)
Shawn
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox