* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Alexandre Courbot @ 2012-11-22 2:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121121151209.GA4048@avionic-0098.adnet.avionic-design.de>
On Thu, Nov 22, 2012 at 12:12 AM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> On Thu, Nov 22, 2012 at 12:02:47AM +0900, Alexandre Courbot wrote:
>> Mmmm so maybe I am misinterpreting things, but it looks like we
>> have just buried the power sequences here, haven't we?
>
> I don't think so. In fact I was just starting to think that maybe for
> Tegra we could have a generic panel driver which used power sequences
> to abstract away the pin differences for powering up the panel. Since
> most likely that will be where the differences are there is a lot of
> potential to factor things out into sequences.
>
> Perhaps what we may want to postpone for now is the DT representation
> since that's what Tomi and Grant seem to be mostly opposed to.
The thing I don't understand here is why would anyone want power
sequences without the DT representation. Guys, that's the whole point! :)
If we are to implement things into drivers, then callback functions
are going to serve us just as well - even better, for they are more
flexible. All we need to do is define a dedicated ops structure and
have the driver plug the right callback functions depending on the
"compatible" property of the DT device node. We don't need a framework
for that.
And if we are not going to use power seqs that's probably what we
should do in order to get panels to work on Tegra boards for now. Then
see how things turn out with the panel framework and whether there is
a use for power seqs in it. But by the meantime, I don't see any
motivation to merge power seqs sans DT support.
Alex.
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Mark Brown @ 2012-11-22 2:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAVeFuK_V6nb874bygioi35mFNxaWuveKOedLT=YYsP4JfVqgA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 807 bytes --]
On Thu, Nov 22, 2012 at 11:01:34AM +0900, Alexandre Courbot wrote:
> The thing I don't understand here is why would anyone want power
> sequences without the DT representation. Guys, that's the whole point! :)
> If we are to implement things into drivers, then callback functions
> are going to serve us just as well - even better, for they are more
> flexible. All we need to do is define a dedicated ops structure and
> have the driver plug the right callback functions depending on the
> "compatible" property of the DT device node. We don't need a framework
> for that.
It allows drivers (both board drivers and actual drivers) to write these
things in a semi-scripted form instead of having to open code everything
each time, it'd save a bunch of tedious stuff with resource requesting
for example.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Alexandre Courbot @ 2012-11-22 3:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121122020649.GH4371@opensource.wolfsonmicro.com>
On Thu, Nov 22, 2012 at 11:06 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Nov 22, 2012 at 11:01:34AM +0900, Alexandre Courbot wrote:
>
>> The thing I don't understand here is why would anyone want power
>> sequences without the DT representation. Guys, that's the whole point! :)
>
>> If we are to implement things into drivers, then callback functions
>> are going to serve us just as well - even better, for they are more
>> flexible. All we need to do is define a dedicated ops structure and
>> have the driver plug the right callback functions depending on the
>> "compatible" property of the DT device node. We don't need a framework
>> for that.
>
> It allows drivers (both board drivers and actual drivers) to write these
> things in a semi-scripted form instead of having to open code everything
> each time, it'd save a bunch of tedious stuff with resource requesting
> for example.
Mmm, I overlooked that point - that's fair enough. Guess I should
remove all DT support and stress that point in the documentation. Then
maybe we'll have a deal.
Alex.
^ permalink raw reply
* Re: [PATCH v12 3/6] fbmon: add videomode helpers
From: Sascha Hauer @ 2012-11-22 6:20 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, David Airlie,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
Florian Tobias Schandinat,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Steffen Trumtrar,
Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <96696218.4l3uYOulV3@avalon>
Hi Laurent,
On Wed, Nov 21, 2012 at 11:02:44PM +0100, Laurent Pinchart wrote:
> Hi Steffen,
>
> > +
> > + htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
> > + vm->hsync_len;
> > + vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
> > + vm->vsync_len;
> > + fbmode->refresh = (vm->pixelclock * 1000) / (htotal * vtotal);
>
> This will fail if vm->pixelclock >= ((1 << 32) / 1000). The easiest solution
> is probably to use 64-bit computation.
You have displays with a pixelclock > 4GHz?
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
* [PATCH 0/4] video: vt8500: cleanup and use of devm_ apis
From: Tushar Behera @ 2012-11-22 6:54 UTC (permalink / raw)
To: linux-kernel, linux-fbdev; +Cc: FlorianSchandinat, linux, patches
Patch 1 fixes a possible memory leak in the driver.
Rest 3 patches add/update usage of devm_ apis.
These patches are only build-tested.
Tushar Behera (4):
video: vt8500: Fix memory leak in probe function
video: vt8500: Fix invalid free of devm_ allocated data
video: vt8500: Convert to use devm_request_mem_region()
video: vt8500: Convert to use devm_ioremap()
drivers/video/vt8500lcdfb.c | 47 +++++++++++++++---------------------------
1 files changed, 17 insertions(+), 30 deletions(-)
--
1.7.4.1
^ permalink raw reply
* [PATCH 1/4] video: vt8500: Fix memory leak in probe function
From: Tushar Behera @ 2012-11-22 6:54 UTC (permalink / raw)
To: linux-kernel, linux-fbdev; +Cc: FlorianSchandinat, linux, patches
In-Reply-To: <1353566531-31251-1-git-send-email-tushar.behera@linaro.org>
In case of error in probe function, there is a possibility of
either iounmap not getting called or memory allocated for fb_mem_virt
not getting freed.
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
drivers/video/vt8500lcdfb.c | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/video/vt8500lcdfb.c b/drivers/video/vt8500lcdfb.c
index 9af8da7..5777adc 100644
--- a/drivers/video/vt8500lcdfb.c
+++ b/drivers/video/vt8500lcdfb.c
@@ -350,7 +350,7 @@ static int __devinit vt8500lcd_probe(struct platform_device *pdev)
if (!np) {
pr_err("%s: No display description in Device Tree\n", __func__);
ret = -EINVAL;
- goto failed_free_res;
+ goto failed_free_io;
}
/*
@@ -369,7 +369,7 @@ static int __devinit vt8500lcd_probe(struct platform_device *pdev)
ret |= of_property_read_u32(np, "bpp", &bpp);
if (ret) {
pr_err("%s: Unable to read display properties\n", __func__);
- goto failed_free_res;
+ goto failed_free_io;
}
of_mode.vmode = FB_VMODE_NONINTERLACED;
@@ -379,7 +379,8 @@ static int __devinit vt8500lcd_probe(struct platform_device *pdev)
GFP_KERNEL);
if (!fb_mem_virt) {
pr_err("%s: Failed to allocate framebuffer\n", __func__);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto failed_free_io;
};
fbi->fb.fix.smem_start = fb_mem_phys;
@@ -394,7 +395,7 @@ static int __devinit vt8500lcd_probe(struct platform_device *pdev)
if (fbi->palette_cpu = NULL) {
dev_err(&pdev->dev, "Failed to allocate palette buffer\n");
ret = -ENOMEM;
- goto failed_free_io;
+ goto failed_free_fbmem;
}
irq = platform_get_irq(pdev, 0);
@@ -458,6 +459,9 @@ failed_free_irq:
failed_free_palette:
dma_free_coherent(&pdev->dev, fbi->palette_size,
fbi->palette_cpu, fbi->palette_phys);
+failed_free_fbmem:
+ dma_free_coherent(&pdev->dev, fbi->fb.fix.smem_len,
+ fbi->fb.screen_base, fbi->fb.fix.smem_start);
failed_free_io:
iounmap(fbi->regbase);
failed_free_res:
@@ -485,6 +489,10 @@ static int __devexit vt8500lcd_remove(struct platform_device *pdev)
irq = platform_get_irq(pdev, 0);
free_irq(irq, fbi);
+ if (fbi->fb.screen_base)
+ dma_free_coherent(&pdev->dev, fbi->fb.fix.smem_len,
+ fbi->fb.screen_base, fbi->fb.fix.smem_start);
+
dma_free_coherent(&pdev->dev, fbi->palette_size,
fbi->palette_cpu, fbi->palette_phys);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 2/4] video: vt8500: Fix invalid free of devm_ allocated data
From: Tushar Behera @ 2012-11-22 6:54 UTC (permalink / raw)
To: linux-kernel, linux-fbdev; +Cc: FlorianSchandinat, linux, patches
In-Reply-To: <1353566531-31251-1-git-send-email-tushar.behera@linaro.org>
While at it, also fix the related return statements.
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
drivers/video/vt8500lcdfb.c | 15 +++------------
1 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/video/vt8500lcdfb.c b/drivers/video/vt8500lcdfb.c
index 5777adc..2438368 100644
--- a/drivers/video/vt8500lcdfb.c
+++ b/drivers/video/vt8500lcdfb.c
@@ -294,8 +294,7 @@ static int __devinit vt8500lcd_probe(struct platform_device *pdev)
+ sizeof(u32) * 16, GFP_KERNEL);
if (!fbi) {
dev_err(&pdev->dev, "Failed to initialize framebuffer device\n");
- ret = -ENOMEM;
- goto failed;
+ return -ENOMEM;
}
strcpy(fbi->fb.fix.id, "VT8500 LCD");
@@ -328,15 +327,13 @@ static int __devinit vt8500lcd_probe(struct platform_device *pdev)
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (res = NULL) {
dev_err(&pdev->dev, "no I/O memory resource defined\n");
- ret = -ENODEV;
- goto failed_fbi;
+ return -ENODEV;
}
res = request_mem_region(res->start, resource_size(res), "vt8500lcd");
if (res = NULL) {
dev_err(&pdev->dev, "failed to request I/O memory\n");
- ret = -EBUSY;
- goto failed_fbi;
+ return -EBUSY;
}
fbi->regbase = ioremap(res->start, resource_size(res));
@@ -466,10 +463,6 @@ failed_free_io:
iounmap(fbi->regbase);
failed_free_res:
release_mem_region(res->start, resource_size(res));
-failed_fbi:
- platform_set_drvdata(pdev, NULL);
- kfree(fbi);
-failed:
return ret;
}
@@ -501,8 +494,6 @@ static int __devexit vt8500lcd_remove(struct platform_device *pdev)
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
release_mem_region(res->start, resource_size(res));
- kfree(fbi);
-
return 0;
}
--
1.7.4.1
^ permalink raw reply related
* [PATCH 3/4] video: vt8500: Convert to use devm_request_mem_region()
From: Tushar Behera @ 2012-11-22 6:54 UTC (permalink / raw)
To: linux-kernel, linux-fbdev; +Cc: FlorianSchandinat, linux, patches
In-Reply-To: <1353566531-31251-1-git-send-email-tushar.behera@linaro.org>
While at it, fix the return statements.
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
drivers/video/vt8500lcdfb.c | 12 +++---------
1 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/video/vt8500lcdfb.c b/drivers/video/vt8500lcdfb.c
index 2438368..c42f15d 100644
--- a/drivers/video/vt8500lcdfb.c
+++ b/drivers/video/vt8500lcdfb.c
@@ -330,7 +330,8 @@ static int __devinit vt8500lcd_probe(struct platform_device *pdev)
return -ENODEV;
}
- res = request_mem_region(res->start, resource_size(res), "vt8500lcd");
+ res = devm_request_mem_region(&pdev->dev, res->start,
+ resource_size(res), "vt8500lcd");
if (res = NULL) {
dev_err(&pdev->dev, "failed to request I/O memory\n");
return -EBUSY;
@@ -339,8 +340,7 @@ static int __devinit vt8500lcd_probe(struct platform_device *pdev)
fbi->regbase = ioremap(res->start, resource_size(res));
if (fbi->regbase = NULL) {
dev_err(&pdev->dev, "failed to map I/O memory\n");
- ret = -EBUSY;
- goto failed_free_res;
+ return -EBUSY;
}
np = of_parse_phandle(pdev->dev.of_node, "default-mode", 0);
@@ -461,15 +461,12 @@ failed_free_fbmem:
fbi->fb.screen_base, fbi->fb.fix.smem_start);
failed_free_io:
iounmap(fbi->regbase);
-failed_free_res:
- release_mem_region(res->start, resource_size(res));
return ret;
}
static int __devexit vt8500lcd_remove(struct platform_device *pdev)
{
struct vt8500lcd_info *fbi = platform_get_drvdata(pdev);
- struct resource *res;
int irq;
unregister_framebuffer(&fbi->fb);
@@ -491,9 +488,6 @@ static int __devexit vt8500lcd_remove(struct platform_device *pdev)
iounmap(fbi->regbase);
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- release_mem_region(res->start, resource_size(res));
-
return 0;
}
--
1.7.4.1
^ permalink raw reply related
* [PATCH 4/4] video: vt8500: Convert to use devm_ioremap()
From: Tushar Behera @ 2012-11-22 6:54 UTC (permalink / raw)
To: linux-kernel, linux-fbdev; +Cc: FlorianSchandinat, linux, patches
In-Reply-To: <1353566531-31251-1-git-send-email-tushar.behera@linaro.org>
While at it, fix the return statements.
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
drivers/video/vt8500lcdfb.c | 14 ++++----------
1 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/video/vt8500lcdfb.c b/drivers/video/vt8500lcdfb.c
index c42f15d..96a955e 100644
--- a/drivers/video/vt8500lcdfb.c
+++ b/drivers/video/vt8500lcdfb.c
@@ -337,7 +337,7 @@ static int __devinit vt8500lcd_probe(struct platform_device *pdev)
return -EBUSY;
}
- fbi->regbase = ioremap(res->start, resource_size(res));
+ fbi->regbase = devm_ioremap(&pdev->dev, res->start, resource_size(res));
if (fbi->regbase = NULL) {
dev_err(&pdev->dev, "failed to map I/O memory\n");
return -EBUSY;
@@ -346,8 +346,7 @@ static int __devinit vt8500lcd_probe(struct platform_device *pdev)
np = of_parse_phandle(pdev->dev.of_node, "default-mode", 0);
if (!np) {
pr_err("%s: No display description in Device Tree\n", __func__);
- ret = -EINVAL;
- goto failed_free_io;
+ return -EINVAL;
}
/*
@@ -366,7 +365,7 @@ static int __devinit vt8500lcd_probe(struct platform_device *pdev)
ret |= of_property_read_u32(np, "bpp", &bpp);
if (ret) {
pr_err("%s: Unable to read display properties\n", __func__);
- goto failed_free_io;
+ return ret;
}
of_mode.vmode = FB_VMODE_NONINTERLACED;
@@ -376,8 +375,7 @@ static int __devinit vt8500lcd_probe(struct platform_device *pdev)
GFP_KERNEL);
if (!fb_mem_virt) {
pr_err("%s: Failed to allocate framebuffer\n", __func__);
- ret = -ENOMEM;
- goto failed_free_io;
+ return -ENOMEM;
};
fbi->fb.fix.smem_start = fb_mem_phys;
@@ -459,8 +457,6 @@ failed_free_palette:
failed_free_fbmem:
dma_free_coherent(&pdev->dev, fbi->fb.fix.smem_len,
fbi->fb.screen_base, fbi->fb.fix.smem_start);
-failed_free_io:
- iounmap(fbi->regbase);
return ret;
}
@@ -486,8 +482,6 @@ static int __devexit vt8500lcd_remove(struct platform_device *pdev)
dma_free_coherent(&pdev->dev, fbi->palette_size,
fbi->palette_cpu, fbi->palette_phys);
- iounmap(fbi->regbase);
-
return 0;
}
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH v12 3/6] fbmon: add videomode helpers
From: Laurent Pinchart @ 2012-11-22 8:50 UTC (permalink / raw)
To: Sascha Hauer
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, David Airlie,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
Florian Tobias Schandinat,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Steffen Trumtrar,
Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20121122062000.GW10369-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Hi Sascha,
On Thursday 22 November 2012 07:20:00 Sascha Hauer wrote:
> On Wed, Nov 21, 2012 at 11:02:44PM +0100, Laurent Pinchart wrote:
> > Hi Steffen,
> >
> > > +
> > > + htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
> > > + vm->hsync_len;
> > > + vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
> > > + vm->vsync_len;
> > > + fbmode->refresh = (vm->pixelclock * 1000) / (htotal * vtotal);
> >
> > This will fail if vm->pixelclock >= ((1 << 32) / 1000). The easiest
> > solution is probably to use 64-bit computation.
>
> You have displays with a pixelclock > 4GHz?
vm->pixelclock is expressed in Hz. vm->pixelclock * 1000 will thus overflow if
the clock frequency is >= ~4.3 MHz. I have displays with a clock frequency
higher than that :-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v12 3/6] fbmon: add videomode helpers
From: Sascha Hauer @ 2012-11-22 8:53 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, David Airlie,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
Florian Tobias Schandinat,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Steffen Trumtrar,
Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1554720.pFHYnMF1G4@avalon>
On Thu, Nov 22, 2012 at 09:50:10AM +0100, Laurent Pinchart wrote:
> Hi Sascha,
>
> On Thursday 22 November 2012 07:20:00 Sascha Hauer wrote:
> > On Wed, Nov 21, 2012 at 11:02:44PM +0100, Laurent Pinchart wrote:
> > > Hi Steffen,
> > >
> > > > +
> > > > + htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
> > > > + vm->hsync_len;
> > > > + vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
> > > > + vm->vsync_len;
> > > > + fbmode->refresh = (vm->pixelclock * 1000) / (htotal * vtotal);
> > >
> > > This will fail if vm->pixelclock >= ((1 << 32) / 1000). The easiest
> > > solution is probably to use 64-bit computation.
> >
> > You have displays with a pixelclock > 4GHz?
>
> vm->pixelclock is expressed in Hz. vm->pixelclock * 1000 will thus overflow if
> the clock frequency is >= ~4.3 MHz. I have displays with a clock frequency
> higher than that :-)
If vm->pixelclock is in Hz, then the * 1000 above is wrong.
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
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Linus Walleij @ 2012-11-22 8:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121121013133.GE4673@opensource.wolfsonmicro.com>
On Wed, Nov 21, 2012 at 2:31 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> As I said in my earlier reviews I think this is a useful thing to have
> as a utility library for drivers independantly of the DT bindings, it
> would allow drivers to become more data driven. Perhaps we can rework
> the series so that the DT bindings are added as a final patch? All the
> rest of the code seems OK.
I have the same (limited by experience) opinion. Working sort of
near som audio drivers I have understood that power sequencing
is a big issue not only for things like this backlight example, but
even more so in the area of audio to avoid clicks and pops.
Is it correct to assume that this library will be useful also for ALSA
SoC type of devices?
Then whether to couple that to DT or ACPI or SFI or whatever
is a different discussion altogether.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v12 3/6] fbmon: add videomode helpers
From: Laurent Pinchart @ 2012-11-22 9:07 UTC (permalink / raw)
To: Sascha Hauer
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, David Airlie,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
Florian Tobias Schandinat,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Steffen Trumtrar,
Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20121122085342.GB10369-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On Thursday 22 November 2012 09:53:42 Sascha Hauer wrote:
> On Thu, Nov 22, 2012 at 09:50:10AM +0100, Laurent Pinchart wrote:
> > On Thursday 22 November 2012 07:20:00 Sascha Hauer wrote:
> > > On Wed, Nov 21, 2012 at 11:02:44PM +0100, Laurent Pinchart wrote:
> > > > Hi Steffen,
> > > >
> > > > > +
> > > > > + htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
> > > > > + vm->hsync_len;
> > > > > + vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
> > > > > + vm->vsync_len;
> > > > > + fbmode->refresh = (vm->pixelclock * 1000) / (htotal * vtotal);
> > > >
> > > > This will fail if vm->pixelclock >= ((1 << 32) / 1000). The easiest
> > > > solution is probably to use 64-bit computation.
> > >
> > > You have displays with a pixelclock > 4GHz?
> >
> > vm->pixelclock is expressed in Hz. vm->pixelclock * 1000 will thus
> > overflow if the clock frequency is >= ~4.3 MHz. I have displays with a
> > clock frequency higher than that :-)
>
> If vm->pixelclock is in Hz, then the * 1000 above is wrong.
My bad, I though refresh was expressed in mHz. So yes, the above computation
is wrong.
BTW it seems that the refreshrate field in struct videomode isn't used. It
should then be removed.
I've just realized that the struct videomode fields are not documented.
kerneldoc in include/linux/videomode.h would be a good addition.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v12 2/6] video: add of helper for videomode
From: Steffen Trumtrar @ 2012-11-22 9:10 UTC (permalink / raw)
To: Rob Herring
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org, David Airlie,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
Florian Tobias Schandinat,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
Manjunathappa, Prakash, Valkeinen, Tomi, Laurent Pinchart,
Philipp Zabel, Guennady Liakhovetski,
linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <50ACED4A.5040806-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi!
On Wed, Nov 21, 2012 at 09:03:38AM -0600, Rob Herring wrote:
> On 11/21/2012 05:52 AM, Thierry Reding wrote:
> > On Wed, Nov 21, 2012 at 12:48:43PM +0100, Steffen Trumtrar wrote:
> >> Hi!
> >>
> >> On Wed, Nov 21, 2012 at 10:12:43AM +0000, Manjunathappa, Prakash wrote:
> >>> Hi Steffen,
> >>>
> >>> On Tue, Nov 20, 2012 at 21:24:52, Steffen Trumtrar wrote:
> >>>> +/**
> >>>> + * of_get_display_timings - parse all display_timing entries from a device_node
> >>>> + * @np: device_node with the subnodes
> >>>> + **/
> >>>> +struct display_timings *of_get_display_timings(const struct device_node *np)
> >>>> +{
> >>>> + struct device_node *timings_np;
> >>>> + struct device_node *entry;
> >>>> + struct device_node *native_mode;
> >>>> + struct display_timings *disp;
> >>>> +
> >>>> + if (!np) {
> >>>> + pr_err("%s: no devicenode given\n", __func__);
> >>>> + return NULL;
> >>>> + }
> >>>> +
> >>>> + timings_np = of_find_node_by_name(np, "display-timings");
> >>>
> >>> I get below build warnings on this line
> >>> drivers/video/of_display_timing.c: In function 'of_get_display_timings':
> >>> drivers/video/of_display_timing.c:109:2: warning: passing argument 1 of 'of_find_node_by_name' discards qualifiers from pointer target type
> >>> include/linux/of.h:167:28: note: expected 'struct device_node *' but argument is of type 'const struct device_node *'
> >>>
> >>>> + * of_display_timings_exists - check if a display-timings node is provided
> >>>> + * @np: device_node with the timing
> >>>> + **/
> >>>> +int of_display_timings_exists(const struct device_node *np)
> >>>> +{
> >>>> + struct device_node *timings_np;
> >>>> +
> >>>> + if (!np)
> >>>> + return -EINVAL;
> >>>> +
> >>>> + timings_np = of_parse_phandle(np, "display-timings", 0);
> >>>
> >>> Also here:
> >>> drivers/video/of_display_timing.c: In function 'of_display_timings_exists':
> >>> drivers/video/of_display_timing.c:209:2: warning: passing argument 1 of 'of_parse_phandle' discards qualifiers from pointer target type
> >>> include/linux/of.h:258:28: note: expected 'struct device_node *' but argument is of type 'const struct device_node *'
> >>>
> >>
> >> The warnings are because the of-functions do not use const pointers where they
> >> should. I had two options: don't use const pointers even if they should be and
> >> have no warnings or use const pointers and have a correct API. (Third option:
> >> send patches for of-functions). I chose the second option.
> >
> > Maybe a better approach would be a combination of 1 and 3: don't use
> > const pointers for struct device_node for now and bring the issue up
> > with the OF maintainers, possibly with patches attached that fix the
> > problematic functions.
>
> Why does this need to be const? Since some DT functions increment
> refcount the node, I'm not sure that making struct device_node const in
> general is right thing to do. I do think it should be okay for
> of_parse_phandle.
>
Okay, that seems right. I went a little to far with const'ing.
I will send a patch for of_parse_phandle as this function does
not seem to change the pointer it is given, but returns a new one
on which the refcount gets incremented.
Regards,
Steffen
--
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
* [PATCH v2] video console: add a driver for lcd2s character display
From: Lars Poeschel @ 2012-11-22 9:33 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Lars Poeschel, FlorianSchandinat, mathieu.poirier, linux-fbdev,
linux-kernel
In-Reply-To: <201211201611.28646.arnd@arndb.de>
From: Lars Poeschel <poeschel@lemonage.de>
This driver allows to use a lcd2s 20x4 character display as
a linux console output device.
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
drivers/video/console/Kconfig | 10 ++
drivers/video/console/Makefile | 1 +
drivers/video/console/lcd2scon.c | 360 ++++++++++++++++++++++++++++++++++++++
3 files changed, 371 insertions(+)
create mode 100644 drivers/video/console/lcd2scon.c
diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index e2c96d0..44fc3bf 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -129,6 +129,16 @@ config STI_CONSOLE
machines. Say Y here to build support for it into your kernel.
The alternative is to use your primary serial port as a console.
+config LCD2S_CONSOLE
+ tristate "lcd2s 20x4 character display over I2C console"
+ depends on I2C
+ default n
+ help
+ This is a driver that lets you use the lcd2s 20x4 character display
+ from modtronix engineering as a console output device. The display
+ is a simple single color character display. You have to connect it
+ to an I2C bus.
+
config FONTS
bool "Select compiled-in fonts"
depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE
diff --git a/drivers/video/console/Makefile b/drivers/video/console/Makefile
index a862e91..74d5993 100644
--- a/drivers/video/console/Makefile
+++ b/drivers/video/console/Makefile
@@ -23,6 +23,7 @@ font-objs += $(font-objs-y)
obj-$(CONFIG_DUMMY_CONSOLE) += dummycon.o
obj-$(CONFIG_SGI_NEWPORT_CONSOLE) += newport_con.o font.o
obj-$(CONFIG_STI_CONSOLE) += sticon.o sticore.o font.o
+obj-$(CONFIG_LCD2S_CONSOLE) += lcd2scon.o
obj-$(CONFIG_VGA_CONSOLE) += vgacon.o
obj-$(CONFIG_MDA_CONSOLE) += mdacon.o
obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += fbcon.o bitblit.o font.o softcursor.o
diff --git a/drivers/video/console/lcd2scon.c b/drivers/video/console/lcd2scon.c
new file mode 100644
index 0000000..c139811
--- /dev/null
+++ b/drivers/video/console/lcd2scon.c
@@ -0,0 +1,360 @@
+/*
+ * console driver for LCD2S 4x20 character displays connected through i2c.
+ *
+ * This is a driver allowing you to use a LCD2S 4x20 from modtronix
+ * engineering as console output device.
+ *
+ * (C) 2012 by Lemonage Software GmbH
+ * Author: Lars Poeschel <poeschel@lemonage.de>
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/kd.h>
+#include <linux/tty.h>
+#include <linux/console_struct.h>
+#include <linux/console.h>
+#include <linux/vt_kern.h>
+#include <linux/i2c.h>
+
+#define LCD2S_CMD_CUR_BLINK_OFF 0x10
+#define LCD2S_CMD_CUR_UL_OFF 0x11
+#define LCD2S_CMD_DISPLAY_OFF 0x12
+#define LCD2S_CMD_CUR_BLINK_ON 0x18
+#define LCD2S_CMD_CUR_UL_ON 0x19
+#define LCD2S_CMD_DISPLAY_ON 0x1a
+#define LCD2S_CMD_BACKLIGHT_OFF 0x20
+#define LCD2S_CMD_BACKLIGHT_ON 0x28
+#define LCD2S_CMD_WRITE 0x80
+#define LCD2S_CMD_SHIFT_UP 0x87
+#define LCD2S_CMD_SHIFT_DOWN 0x88
+#define LCD2S_CMD_CUR_ADDR 0x89
+#define LCD2S_CMD_CUR_POS 0x8a
+#define LCD2S_CMD_CUR_RESET 0x8b
+#define LCD2S_CMD_CLEAR 0x8c
+
+#define LCD2S_FIRST 8
+#define LCD2S_LAST 9
+
+#define LCD2S_ROWS 4
+#define LCD2S_COLS 20
+
+struct lcd2s_data {
+ struct i2c_client *i2c;
+ int row;
+ int col;
+ unsigned int cur_blink:1;
+ unsigned int cur_ul:1;
+};
+
+static struct lcd2s_data lcd2s;
+
+static void lcd2s_set_cursor(int row, int col)
+{
+ u8 buf[] = { LCD2S_CMD_CUR_POS, row + 1, col + 1};
+
+ if (row = lcd2s.row && col = lcd2s.col)
+ return;
+
+ i2c_master_send(lcd2s.i2c, buf, sizeof(buf));
+ lcd2s.row = row;
+ lcd2s.col = col;
+}
+
+static void lcd2s_reset_cursor(void)
+{
+ i2c_smbus_write_byte(lcd2s.i2c, LCD2S_CMD_CUR_RESET);
+ lcd2s.row = 0;
+ lcd2s.col = 0;
+}
+
+static void lcd2s_increase_cursor(struct vc_data *con, int inc)
+{
+ lcd2s.col += inc;
+ if ((lcd2s.col / con->vc_cols) >= 1) {
+ lcd2s.row += (lcd2s.col / con->vc_cols);
+ lcd2s.col %= con->vc_cols;
+ }
+}
+
+static void lcd2s_cursor_ul_on(void)
+{
+ if (!lcd2s.cur_ul) {
+ i2c_smbus_write_byte(lcd2s.i2c, LCD2S_CMD_CUR_UL_ON);
+ lcd2s.cur_ul = 1;
+ }
+}
+
+static void lcd2s_cursor_ul_off(void)
+{
+ if (lcd2s.cur_ul) {
+ i2c_smbus_write_byte(lcd2s.i2c, LCD2S_CMD_CUR_UL_OFF);
+ lcd2s.cur_ul = 0;
+ }
+}
+
+static void lcd2s_cursor_blink_on(void)
+{
+ if (!lcd2s.cur_blink) {
+ i2c_smbus_write_byte(lcd2s.i2c, LCD2S_CMD_CUR_BLINK_ON);
+ lcd2s.cur_blink = 1;
+ }
+}
+
+static void lcd2s_cursor_blink_off(void)
+{
+ if (lcd2s.cur_blink) {
+ i2c_smbus_write_byte(lcd2s.i2c, LCD2S_CMD_CUR_BLINK_OFF);
+ lcd2s.cur_blink = 0;
+ }
+}
+
+static const char *lcd2s_startup(void)
+{
+ return "lcd2s console";
+}
+
+
+/*
+ * init is set if console is currently allocated during init
+ */
+static void lcd2s_init(struct vc_data *con, int init)
+{
+ lcd2s.cur_blink = 0;
+ lcd2s.cur_ul = 0;
+
+ /* turn display on */
+ i2c_smbus_write_byte(lcd2s.i2c, LCD2S_CMD_DISPLAY_ON);
+ i2c_smbus_write_byte(lcd2s.i2c, LCD2S_CMD_BACKLIGHT_ON);
+ i2c_smbus_write_byte(lcd2s.i2c, LCD2S_CMD_CLEAR);
+ lcd2s_cursor_ul_on();
+ lcd2s_reset_cursor();
+
+ con->vc_can_do_color = 0;
+ con->vc_hi_font_mask = 0;
+
+ if (init) {
+ con->vc_rows = LCD2S_ROWS;
+ con->vc_cols = LCD2S_COLS;
+ } else
+ vc_resize(con, LCD2S_COLS, LCD2S_ROWS);
+}
+
+static void lcd2s_deinit(struct vc_data *con)
+{
+ i2c_smbus_write_byte(lcd2s.i2c, LCD2S_CMD_DISPLAY_OFF);
+ i2c_smbus_write_byte(lcd2s.i2c, LCD2S_CMD_BACKLIGHT_OFF);
+ lcd2s_cursor_blink_off();
+ lcd2s_cursor_ul_off();
+}
+
+static void lcd2s_clear(struct vc_data *con, int s_row, int s_col,
+ int height, int width)
+{
+ u8 buf[width];
+ int i;
+
+ if (width <= 0 || height <= 0)
+ return;
+
+ /* if the whole display is to clear, we have a single command */
+ if (s_col = 0 && s_row = 0 &&
+ height >= con->vc_rows - 1 && width >= con->vc_cols - 1) {
+ i2c_smbus_write_byte(lcd2s.i2c, LCD2S_CMD_CLEAR);
+ return;
+ }
+
+ for (i = 0; i <= width; i++)
+ buf[i] = ' ';
+
+ for (i = s_col; i <= height; i++) {
+ lcd2s_set_cursor(s_row, i);
+ i2c_master_send(lcd2s.i2c, buf, width);
+ }
+}
+
+static void lcd2s_putc(struct vc_data *con, int data, int row, int col)
+{
+ u8 buf[] = {LCD2S_CMD_WRITE, data};
+
+ lcd2s_set_cursor(row, col);
+
+ i2c_master_send(lcd2s.i2c, buf, sizeof(buf));
+ lcd2s_increase_cursor(con, 1);
+}
+
+static void lcd2s_putcs(struct vc_data *con, const unsigned short *buf,
+ int len, int row, int col)
+{
+ u8 *i2c_buf;
+ int i;
+
+ i2c_buf = kzalloc(len + 1, GFP_KERNEL);
+ if (!i2c_buf)
+ return;
+
+ lcd2s_set_cursor(row, col);
+
+ i2c_buf[0] = LCD2S_CMD_WRITE;
+ for (i = 0; i < len ; i++)
+ i2c_buf[i + 1] = buf[i];
+
+ i2c_master_send(lcd2s.i2c, i2c_buf, len + 1);
+ kfree(i2c_buf);
+ lcd2s_increase_cursor(con, len);
+}
+
+static void lcd2s_cursor(struct vc_data *con, int mode)
+{
+ switch (mode) {
+ case CM_ERASE:
+ lcd2s_cursor_blink_off();
+ lcd2s_cursor_ul_off();
+ break;
+ case CM_MOVE:
+ case CM_DRAW:
+ lcd2s_set_cursor(con->vc_y, con->vc_x);
+
+ switch (con->vc_cursor_type & CUR_HWMASK) {
+ case CUR_UNDERLINE:
+ lcd2s_cursor_ul_on();
+ lcd2s_cursor_blink_off();
+ break;
+ case CUR_NONE:
+ lcd2s_cursor_blink_off();
+ lcd2s_cursor_ul_off();
+ break;
+ default:
+ lcd2s_cursor_blink_on();
+ lcd2s_cursor_ul_off();
+ break;
+ }
+ break;
+ }
+}
+
+static int lcd2s_scroll(struct vc_data *con, int top, int bot,
+ int dir, int lines)
+{
+ /* we can only scroll the whole display */
+ if (top = 0 && bot = con->vc_rows) {
+ while (lines--) {
+ switch (dir) {
+ case SM_UP:
+ i2c_smbus_write_byte(lcd2s.i2c,
+ LCD2S_CMD_SHIFT_UP);
+ break;
+ case SM_DOWN:
+ i2c_smbus_write_byte(lcd2s.i2c,
+ LCD2S_CMD_SHIFT_DOWN);
+ break;
+ }
+ }
+ return 0;
+ }
+ return 1;
+}
+
+static void lcd2s_bmove(struct vc_data *conp, int s_row, int s_col,
+ int d_row, int d_col, int height, int width)
+{
+}
+
+static int lcd2s_switch(struct vc_data *con)
+{
+ return 1;
+}
+
+static int lcd2s_blank(struct vc_data *con, int blank, int mode_switch)
+{
+ switch (blank) {
+ case 0: /* unblank */
+ i2c_smbus_write_byte(lcd2s.i2c, LCD2S_CMD_DISPLAY_ON);
+ i2c_smbus_write_byte(lcd2s.i2c, LCD2S_CMD_BACKLIGHT_ON);
+ break;
+ case 1: /* normal blanking */
+ i2c_smbus_write_byte(lcd2s.i2c, LCD2S_CMD_DISPLAY_OFF);
+ i2c_smbus_write_byte(lcd2s.i2c, LCD2S_CMD_BACKLIGHT_OFF);
+ break;
+ }
+ return 0;
+}
+
+static int lcd2s_set_palette(struct vc_data *con, unsigned char *table)
+{
+ return -EINVAL;
+}
+
+static int lcd2s_scrolldelta(struct vc_data *con, int lines)
+{
+ return 0;
+}
+
+static struct consw lcd2s_con = {
+ .owner = THIS_MODULE,
+ .con_startup = lcd2s_startup,
+ .con_init = lcd2s_init,
+ .con_deinit = lcd2s_deinit,
+ .con_clear = lcd2s_clear,
+ .con_putc = lcd2s_putc,
+ .con_putcs = lcd2s_putcs,
+ .con_cursor = lcd2s_cursor,
+ .con_scroll = lcd2s_scroll,
+ .con_bmove = lcd2s_bmove,
+ .con_switch = lcd2s_switch,
+ .con_blank = lcd2s_blank,
+ .con_set_palette = lcd2s_set_palette,
+ .con_scrolldelta = lcd2s_scrolldelta,
+};
+
+static int __devinit lcd2s_i2c_probe(struct i2c_client *i2c,
+ const struct i2c_device_id *id)
+{
+ if (!i2c_check_functionality(i2c->adapter,
+ I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
+ I2C_FUNC_SMBUS_WRITE_BLOCK_DATA))
+ return -EIO;
+
+ lcd2s.i2c = i2c;
+
+ take_over_console(&lcd2s_con, LCD2S_FIRST, LCD2S_LAST, 1);
+
+ return 0;
+}
+
+static __devexit int lcd2s_i2c_remove(struct i2c_client *i2c)
+{
+ /* unregister from console subsystem */
+ unregister_con_driver(&lcd2s_con);
+ return 0;
+}
+
+static const struct i2c_device_id lcd2s_i2c_id[] = {
+ { "lcd2s", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, lcd2s_i2c_id);
+
+static struct i2c_driver lcd2s_i2c_driver = {
+ .driver = {
+ .name = "lcd2s",
+ .owner = THIS_MODULE,
+ },
+ .probe = lcd2s_i2c_probe,
+ .remove = __devexit_p(lcd2s_i2c_remove),
+ .id_table = lcd2s_i2c_id,
+};
+
+module_i2c_driver(lcd2s_i2c_driver)
+
+MODULE_DESCRIPTION("LCD2S character display console driver");
+MODULE_AUTHOR("Lars Poeschel");
+MODULE_LICENSE("GPL");
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Alexandre Courbot @ 2012-11-22 9:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdaNmBpfONDpt7zFqLaqfiGm+ELpO-v5gZmM0rEi_AzijQ@mail.gmail.com>
On Thu, Nov 22, 2012 at 5:57 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> I have the same (limited by experience) opinion. Working sort of
> near som audio drivers I have understood that power sequencing
> is a big issue not only for things like this backlight example, but
> even more so in the area of audio to avoid clicks and pops.
>
> Is it correct to assume that this library will be useful also for ALSA
> SoC type of devices?
There should be nothing against that - provided ALSA's needs remain
similar to the current set of actions (regulators, gpios, ...) it
should be perfectly usable. Can you think of any usage that involves
more than dumb regulator/gpio manipulation?
If power seqs are to be used this way (as opposed to through the DT),
it will become urgent to implement that gpio handlers library we were
talking about in another thread, as we don't want to have GPIO numbers
hard-coded into the sequences.
Alex.
^ permalink raw reply
* Re: [PATCH v12 3/6] fbmon: add videomode helpers
From: Steffen Trumtrar @ 2012-11-22 11:23 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, David Airlie, Sascha Hauer,
Florian Tobias Schandinat,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Guennady Liakhovetski,
Tomi Valkeinen, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <2254661.PBsclu7Klj@avalon>
On Thu, Nov 22, 2012 at 10:07:07AM +0100, Laurent Pinchart wrote:
> On Thursday 22 November 2012 09:53:42 Sascha Hauer wrote:
> > On Thu, Nov 22, 2012 at 09:50:10AM +0100, Laurent Pinchart wrote:
> > > On Thursday 22 November 2012 07:20:00 Sascha Hauer wrote:
> > > > On Wed, Nov 21, 2012 at 11:02:44PM +0100, Laurent Pinchart wrote:
> > > > > Hi Steffen,
> > > > >
> > > > > > +
> > > > > > + htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
> > > > > > + vm->hsync_len;
> > > > > > + vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
> > > > > > + vm->vsync_len;
> > > > > > + fbmode->refresh = (vm->pixelclock * 1000) / (htotal * vtotal);
> > > > >
> > > > > This will fail if vm->pixelclock >= ((1 << 32) / 1000). The easiest
> > > > > solution is probably to use 64-bit computation.
> > > >
> > > > You have displays with a pixelclock > 4GHz?
> > >
> > > vm->pixelclock is expressed in Hz. vm->pixelclock * 1000 will thus
> > > overflow if the clock frequency is >= ~4.3 MHz. I have displays with a
> > > clock frequency higher than that :-)
> >
> > If vm->pixelclock is in Hz, then the * 1000 above is wrong.
>
> My bad, I though refresh was expressed in mHz. So yes, the above computation
> is wrong.
>
Okay. I will fix that with the next version...
> BTW it seems that the refreshrate field in struct videomode isn't used. It
> should then be removed.
>
...and remove this field.
> I've just realized that the struct videomode fields are not documented.
> kerneldoc in include/linux/videomode.h would be a good addition.
>
Regards,
Steffen
--
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
* Re: [PATCH 2/2] OMAP: DSS: panel-n8x0: register the DSS driver after SPI probe
From: Tomi Valkeinen @ 2012-11-22 12:18 UTC (permalink / raw)
To: Aaro Koskinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1353527332-5216-2-git-send-email-aaro.koskinen@iki.fi>
[-- Attachment #1: Type: text/plain, Size: 352 bytes --]
Hi,
On 2012-11-21 21:48, Aaro Koskinen wrote:
> Register the DSS driver after SPI probe. This simplifies the
> initialization. This is similar to what is being done e.g.
> in panel-acx565akm.
Is this a fix? The description sounds like it's just a cleanup. The
first patch in the series needs to be sent for the next -rc, right?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply
* [BUGFIX PATCH] video: mxsfb: fix crash when unblanking the display
From: Lothar Waßmann @ 2012-11-22 12:49 UTC (permalink / raw)
To: linux-arm-kernel
The VDCTRL4 register does not provide the MXS SET/CLR/TOGGLE feature.
The write in mxsfb_disable_controller() sets the data_cnt for the LCD
DMA to 0 which obviously means the max. count for the LCD DMA and
leads to overwriting arbitrary memory when the display is unblanked.
Note: I already sent this patch in December 2011 but noone picked it up obviously.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/video/mxsfb.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
index 49619b4..f2a49ef 100644
--- a/drivers/video/mxsfb.c
+++ b/drivers/video/mxsfb.c
@@ -369,7 +369,8 @@ static void mxsfb_disable_controller(struct fb_info *fb_info)
loop--;
}
- writel(VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4 + REG_CLR);
+ reg = readl(host->base + LCDC_VDCTRL4);
+ writel(reg & ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);
clk_disable_unprepare(host->clk);
--
1.7.2.5
^ permalink raw reply related
* [PATCH] video: mxsfb: fix typo: 'FAILING' instead of 'FALLING'
From: Lothar Waßmann @ 2012-11-22 12:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1353588555-13168-1-git-send-email-LW@KARO-electronics.de>
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
arch/arm/mach-mxs/mach-mxs.c | 6 +++---
drivers/video/mxsfb.c | 6 +++---
include/linux/mxsfb.h | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
index d61b915..6c17cb7 100644
--- a/arch/arm/mach-mxs/mach-mxs.c
+++ b/arch/arm/mach-mxs/mach-mxs.c
@@ -42,7 +42,7 @@ static struct fb_videomode mx23evk_video_modes[] = {
.hsync_len = 1,
.vsync_len = 1,
.sync = FB_SYNC_DATA_ENABLE_HIGH_ACT |
- FB_SYNC_DOTCLK_FAILING_ACT,
+ FB_SYNC_DOTCLK_FALLING_ACT,
},
};
@@ -60,7 +60,7 @@ static struct fb_videomode mx28evk_video_modes[] = {
.hsync_len = 10,
.vsync_len = 10,
.sync = FB_SYNC_DATA_ENABLE_HIGH_ACT |
- FB_SYNC_DOTCLK_FAILING_ACT,
+ FB_SYNC_DOTCLK_FALLING_ACT,
},
};
@@ -96,7 +96,7 @@ static struct fb_videomode apx4devkit_video_modes[] = {
.vsync_len = 3,
.sync = FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT |
FB_SYNC_DATA_ENABLE_HIGH_ACT |
- FB_SYNC_DOTCLK_FAILING_ACT,
+ FB_SYNC_DOTCLK_FALLING_ACT,
},
};
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
index f2a49ef..eb45d32 100644
--- a/drivers/video/mxsfb.c
+++ b/drivers/video/mxsfb.c
@@ -107,7 +107,7 @@
#define VDCTRL0_ENABLE_PRESENT (1 << 28)
#define VDCTRL0_VSYNC_ACT_HIGH (1 << 27)
#define VDCTRL0_HSYNC_ACT_HIGH (1 << 26)
-#define VDCTRL0_DOTCLK_ACT_FAILING (1 << 25)
+#define VDCTRL0_DOTCLK_ACT_FALLING (1 << 25)
#define VDCTRL0_ENABLE_ACT_HIGH (1 << 24)
#define VDCTRL0_VSYNC_PERIOD_UNIT (1 << 21)
#define VDCTRL0_VSYNC_PULSE_WIDTH_UNIT (1 << 20)
@@ -458,8 +458,8 @@ static int mxsfb_set_par(struct fb_info *fb_info)
vdctrl0 |= VDCTRL0_VSYNC_ACT_HIGH;
if (fb_info->var.sync & FB_SYNC_DATA_ENABLE_HIGH_ACT)
vdctrl0 |= VDCTRL0_ENABLE_ACT_HIGH;
- if (fb_info->var.sync & FB_SYNC_DOTCLK_FAILING_ACT)
- vdctrl0 |= VDCTRL0_DOTCLK_ACT_FAILING;
+ if (fb_info->var.sync & FB_SYNC_DOTCLK_FALLING_ACT)
+ vdctrl0 |= VDCTRL0_DOTCLK_ACT_FALLING;
writel(vdctrl0, host->base + LCDC_VDCTRL0);
diff --git a/include/linux/mxsfb.h b/include/linux/mxsfb.h
index f14943d..a9eca68 100644
--- a/include/linux/mxsfb.h
+++ b/include/linux/mxsfb.h
@@ -25,7 +25,7 @@
#define STMLCDIF_24BIT 3 /** pixel data bus to the display is of 24 bit width */
#define FB_SYNC_DATA_ENABLE_HIGH_ACT (1 << 6)
-#define FB_SYNC_DOTCLK_FAILING_ACT (1 << 7) /* failing/negtive edge sampling */
+#define FB_SYNC_DOTCLK_FALLING_ACT (1 << 7) /* falling/negative edge sampling */
struct mxsfb_platform_data {
struct fb_videomode *mode_list;
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Grant Likely @ 2012-11-22 13:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4275468.QAmZy3xUK6@percival>
On Wed, Nov 21, 2012 at 10:00 AM, Alex Courbot <acourbot@nvidia.com> wrote:
> On Wednesday 21 November 2012 16:48:45 Tomi Valkeinen wrote:
>> If the power-off sequence disables a regulator that was supposed to be
>> enabled by the power-on sequence (but wasn't enabled because of an
>> error), the regulator_disable is still called when the driver runs the
>> power-off sequence, isn't it? Regulator enables and disables are ref
>> counted, and the enables should match the disables.
>
> And there collapses my theory.
>
>> > Failures might be better handled if sequences have some "recovery policy"
>> > about what to do when they fail, as mentioned in the link above. As you
>> > pointed out, the driver might not always know enough about the resources
>> > involved to do the right thing.
>>
>> Yes, I think such recovery policy would be needed.
>
> Indeed, from your last paragraph this makes even more sense now.
>
> Oh, and I noticed I forgot to reply to this:
>
>> This I didn't understand. Doesn't "<&pwm 2 xyz>" point to a single
>> device, no matter where and how many times it's used?
>
> That's true - however when dereferencing the phandle, the underlying framework
> will try to acquire the PWM, which will result in failure if the same resource
> is referenced several times.
>
> One could compare the phandles to avoid this, but in your example you must
> know that for PWMs the "xyz" part is not relevant for comparison.
>
> This makes referencing of resources by name much easier to implement and more
> elegant with respect to frameworks leveraging.
I would rather (at least for how the DT bindings settle out) see the
design separate the resource references from the sequence. ie. Declare
which resources are used by a device's sequences all in one place and
have the commands index into that.
g.
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Grant Likely @ 2012-11-22 13:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <13540495.epaCf4JVn9@percival>
On Wed, 21 Nov 2012 13:23:06 +0900, Alex Courbot <acourbot@nvidia.com> wrote:
> Hi Grant,
>
> On Wednesday 21 November 2012 05:54:29 Grant Likely wrote:
> > > With the advent of the device tree and of ARM kernels that are not
> > > board-tied, we cannot rely on these board-specific hooks anymore but
> >
> > This isn't strictly true. It is still perfectly fine to have board
> > specific code when necessary. However, there is strong encouragement to
> > enable that code in device drivers as much as possible and new board
> > files need to have very strong justification.
>
> But doesn't introducing board-specific code into the kernel just defeats the
> purpose of the DT? If we extend this logic, we are heading straight back to
> board-definition files. To a lesser extent than before I agree, but the problem
> is fundamentally the same.
There is *always* board specific code. There is always something fiddly
enough, complex enough, or just plain ugly enough that it is best
handled in C code. The trick is to make the board-specific stuff the
exception, not the rule.
When you can factor out common behavour (like you are doing here), then
it is a really good candidate for a common binding, but still please
always ask yourself the question is this going to make things better or
worse in the long run.
> > I'm thinking about the scripts as trivial-to-parse ascii strings that
> > have a very simple set of commands. The commands use resources already
> > defined in the node. ie. 'g0' refers to the first entry in the gpios
> > property. 'r0' for the regulator, 'p0' for the pwms, 'd' means delay. By
> > no means take this as the format to use, it is just an example off the
> > top of my head, but it is already way easier to work with than putting
> > each command into a node.
>
> I'd argue that this opens the door wide open towards having a complete
> interpreter in the device tree. At least DT nodes restrict what we can do
> conveniently.
I disagree... see below.
> > The trick is still to define a syntax that doesn't fall apart when it
> > needs to be extended. I would also like to get opinions on whether or
> > not conditionals or loops should be supported (ie. loop waiting for a
> > status to change). If they should then we need to be a lot more careful
> > about the design (and due to my aforementioned nervousness, somebody may
> > need to get me therapy).
>
> That's IMHO the main argument in favor of using DT nodes here (the syntax
> extension, not your therapy).
hahaha :-)
> They can be extended simply by adding properties
> to them, and I was relying on this to make power seqs extensible while keeping
> things consistent (and backward-compatible). For instance, when we want to
> support voltage setting in regulators we can just add a "voltage" property for
> that.
>
> So to sum up the pros of the current node-base representation:
> - give a "data like" representation of powering sequences (what they should
> be, actually)
> - puts sanity barriers for not turning power seqs into a real code interpreter
> - easily extensible
>
> There are probably a few cons, the numbering of steps being one, but at least
> we don't need to design a new DSL and care too much about extensibility which
> is, in the nodes case, built-in and free.
No matter how it is encoded, this *IS* a new DSL. Using nodes and
properties doesn't change that. Extensibility is no more built-in and
free with nodes that it is with another encoding. If there aren't clear
guidelines on how to extend it then we'll get weird stuff. For example,
even with the nodes case someone might do this:
step3 {
type = "loop";
count = 10;
step1 {
type = "gpio";
gpio = <&gpio 1>;
value = 1;
};
step2 {
type = "delay";
value = 10000;
};
step3 {
type = "gpio";
gpio = <&gpio 1>;
value = 0;
};
step4 {
type = "delay";
value = 10000;
};
};
And even while cringing as I type the above, I also have to consider
that looping may just be a valid use case for sequences.
And even here, a very simple sequence fragment required 22 lines of
code.
Next, I'm concerned about where these will show up. Say for instance
there needs to be a power sequence added to an spi bus node. Already spi
bus child nodes have a defined meaning; they are spi slaves. How then
should the sequence be attached to the spi bus?
> If that makes you feel better, maybe we can try and fix what is wrong with the
> current bindings instead of introducing a new language that will be, by
> nature, more complex to handle and difficult to extend without breaking things?
Okay, here are 3 concrete objections with the proposed binding:
- The syntax concerns of it being too verbose and it effectively uses
line numbers for ordering (Do you remember fighting with BASIC?).
- There are many devices that won't be able to use the binding because
they already have a meaning for child nodes
- I think resources should be declared separate from the sequence based
on the assumption that multiple steps will be operating on the same
resource.
I do think that each sequence should be contained within a single
property, but I'm open to other suggestions.
g.
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Grant Likely @ 2012-11-22 13:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121121151209.GA4048@avionic-0098.adnet.avionic-design.de>
On Wed, Nov 21, 2012 at 3:12 PM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> On Thu, Nov 22, 2012 at 12:02:47AM +0900, Alexandre Courbot wrote:
>> Mmmm so maybe I am misinterpreting things, but it looks like we
>> have just buried the power sequences here, haven't we?
>
> I don't think so. In fact I was just starting to think that maybe for
> Tegra we could have a generic panel driver which used power sequences
> to abstract away the pin differences for powering up the panel. Since
> most likely that will be where the differences are there is a lot of
> potential to factor things out into sequences.
>
> Perhaps what we may want to postpone for now is the DT representation
> since that's what Tomi and Grant seem to be mostly opposed to.
Don't give up. I'm am being cautions and trying to think of all the
implications because that is what I have to do as a maintainer. I'm
not opposed to the feature but I need a good level of confidence that
it has been fully thought out.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 0/5] OMAPFB: use dma_alloc instead of omap's vram
From: Tomi Valkeinen @ 2012-11-22 13:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPFyZi52e+BPLFQoSn_scdru=MP2NeBUNUkHnym8W3t90JW2jA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 625 bytes --]
Hi,
On 2012-11-21 16:22, Jello huang wrote:
> HI Tomi,
> we need one rank of cma to allocate the memory for driver in kernel
> space .And the default CMA is for allocating memory frome usespace.So
> if we allocate the memory from the
> default CMA zone ,there maybe introduce fragmention to the default CMA
> zone.The kernel space memory donot touch the memory from userspace
Can you elaborate a bit? I didn't understand your point. Are you saying
each kernel driver that uses dma_alloc should have their own CMA zone?
That doesn't make sense...
How do you allocate CMA memory from userspace?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply
* Temporary fbdev maintainership for 3.8 merge window
From: Tomi Valkeinen @ 2012-11-22 14:21 UTC (permalink / raw)
To: linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 909 bytes --]
Hi Florian, all,
Florian told me he's very busy for the time being, and this probably
last at least until sometime early December. The merge window is coming
near, and instead of everybody sending their fbdev related patches
directly to Linus, I offer my services as a temporary fbdev maintainer
for the next merge window.
Florian, say if this is not what you intended (the details were left a
bit open in our private emails).
I don't have the capacity to really review much of the patches, so I'd
probably only pull things that are quite clear or specific to one
particular driver.
If you have just a few patches, I can apply them directly. If you have
more, please send a proper pull request to me.
If you still have some fbdev related fixes for 3.7, send them very soon,
as the next -rc may be the last. I'm planning to send a pull request
with omapdss fixes tomorrow.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ 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