* [PATCH 1/5] drivers/video/bf537-lq035.c: use devm_ functions
From: Damien Cassou @ 2012-07-31 13:54 UTC (permalink / raw)
To: Florian Tobias Schandinat; +Cc: kernel-janitors, linux-fbdev, linux-kernel
In-Reply-To: <1343742860-16213-1-git-send-email-damien.cassou@lifl.fr>
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>
---
drivers/video/bf537-lq035.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/video/bf537-lq035.c b/drivers/video/bf537-lq035.c
index befbc80..7347aa1 100644
--- a/drivers/video/bf537-lq035.c
+++ b/drivers/video/bf537-lq035.c
@@ -760,18 +760,20 @@ static int __devinit bfin_lq035_probe(struct platform_device *pdev)
bfin_lq035_fb.flags = FBINFO_DEFAULT;
- bfin_lq035_fb.pseudo_palette = kzalloc(sizeof(u32) * 16, GFP_KERNEL);
+ bfin_lq035_fb.pseudo_palette = devm_kzalloc(&pdev->dev,
+ sizeof(u32) * 16,
+ GFP_KERNEL);
if (bfin_lq035_fb.pseudo_palette = NULL) {
pr_err("failed to allocate pseudo_palette\n");
ret = -ENOMEM;
- goto out_palette;
+ goto out_table;
}
if (fb_alloc_cmap(&bfin_lq035_fb.cmap, NBR_PALETTE, 0) < 0) {
pr_err("failed to allocate colormap (%d entries)\n",
NBR_PALETTE);
ret = -EFAULT;
- goto out_cmap;
+ goto out_table;
}
if (register_framebuffer(&bfin_lq035_fb) < 0) {
@@ -804,9 +806,6 @@ out_lcd:
unregister_framebuffer(&bfin_lq035_fb);
out_reg:
fb_dealloc_cmap(&bfin_lq035_fb.cmap);
-out_cmap:
- kfree(bfin_lq035_fb.pseudo_palette);
-out_palette:
out_table:
dma_free_coherent(NULL, TOTAL_VIDEO_MEM_SIZE, fb_buffer, 0);
fb_buffer = NULL;
@@ -834,7 +833,6 @@ static int __devexit bfin_lq035_remove(struct platform_device *pdev)
free_dma(CH_PPI);
- kfree(bfin_lq035_fb.pseudo_palette);
fb_dealloc_cmap(&bfin_lq035_fb.cmap);
^ permalink raw reply related
* Re: [PATCH 2/5] drivers/video/bfin-t350mcqb-fb.c: use devm_ functions
From: Mike Frysinger @ 2012-07-31 13:57 UTC (permalink / raw)
To: Damien Cassou
Cc: Florian Tobias Schandinat, kernel-janitors, linux-fbdev,
linux-kernel
In-Reply-To: <1343742860-16213-5-git-send-email-damien.cassou@lifl.fr>
On Tue, Jul 31, 2012 at 9:54 AM, Damien Cassou <damien.cassou@lifl.fr> wrote:
> 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.
Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike
^ permalink raw reply
* Re: [RFC][PATCH v3 1/3] runtime interpreted power sequences
From: Mark Brown @ 2012-07-31 14:11 UTC (permalink / raw)
To: Alex Courbot
Cc: Thierry Reding, Stephen Warren, Simon Glass, Grant Likely,
Rob Herring, Greg Kroah-Hartman, Arnd Bergmann,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fbdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org
In-Reply-To: <5017AA87.2040503@nvidia.com>
On Tue, Jul 31, 2012 at 06:51:03PM +0900, Alex Courbot wrote:
> 2) On cleanup, it cleans the resources that needs to be freed (i.e.
> those that are not devm-handled).
> 2) can certainly be removed either by enforcing use of devm, or by
> doing reference counting. 1) seems more difficult to avoid - we need
I'd just add devm_ for everything that's not got it.
^ permalink raw reply
* Re: [RFC][PATCH v3 1/3] runtime interpreted power sequences
From: Mark Brown @ 2012-07-31 14:13 UTC (permalink / raw)
To: Thierry Reding
Cc: Alex Courbot, Stephen Warren, Stephen Warren, Simon Glass,
Grant Likely, Rob Herring, Greg Kroah-Hartman, Arnd Bergmann,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <20120731105640.GD16155-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org>
On Tue, Jul 31, 2012 at 12:56:40PM +0200, Thierry Reding wrote:
> On Tue, Jul 31, 2012 at 07:32:20PM +0900, Alex Courbot wrote:
> > The thing is that I am not sure what happens to the platform data
> > once probe() is done. Isn't it customary to mark it with __devinit
> > and have it freed after probing is successful?
> No, platform data should stay around forever. Otherwise, consider what
> would happen if your driver is built as a module and you unload and load
> it again.
__devinit can be discarded if you disable enough kernel features,
HOTPLUG is the main one IIRC, modules might also need to go - drivers
really ought to take a copy of platform data they plan to use at
runtime, though practically speaking you have to try to trigger any
problems.
^ permalink raw reply
* Re: [RFC][PATCH v3 1/3] runtime interpreted power sequences
From: Thierry Reding @ 2012-07-31 14:22 UTC (permalink / raw)
To: Mark Brown
Cc: Alex Courbot, Stephen Warren, Stephen Warren, Simon Glass,
Grant Likely, Rob Herring, Greg Kroah-Hartman, Arnd Bergmann,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fbdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org
In-Reply-To: <20120731141328.GS4468@opensource.wolfsonmicro.com>
[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]
On Tue, Jul 31, 2012 at 03:13:29PM +0100, Mark Brown wrote:
> On Tue, Jul 31, 2012 at 12:56:40PM +0200, Thierry Reding wrote:
> > On Tue, Jul 31, 2012 at 07:32:20PM +0900, Alex Courbot wrote:
>
> > > The thing is that I am not sure what happens to the platform data
> > > once probe() is done. Isn't it customary to mark it with __devinit
> > > and have it freed after probing is successful?
>
> > No, platform data should stay around forever. Otherwise, consider what
> > would happen if your driver is built as a module and you unload and load
> > it again.
>
> __devinit can be discarded if you disable enough kernel features,
> HOTPLUG is the main one IIRC, modules might also need to go - drivers
> really ought to take a copy of platform data they plan to use at
> runtime, though practically speaking you have to try to trigger any
> problems.
HOTPLUG is marked EXPERT and explicitly states that it should only be
disabled if you're not using modules or dynamic device discovery. I
think if you've ignored all of that you're no longer entitled to
complain.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC][PATCH v3 1/3] runtime interpreted power sequences
From: Mark Brown @ 2012-07-31 14:23 UTC (permalink / raw)
To: Alex Courbot
Cc: Simon Glass, Stephen Warren, Thierry Reding, Grant Likely,
Rob Herring, Greg Kroah-Hartman, Arnd Bergmann,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fbdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org
In-Reply-To: <50179933.9090501@nvidia.com>
On Tue, Jul 31, 2012 at 05:37:07PM +0900, Alex Courbot wrote:
> On 07/30/2012 08:00 PM, Simon Glass wrote:
> >For the delay, I think milliseconds is reasonable. I suppose there is
> >no reasonable need for microseconds?
> I don't see any need for microseconds myself - anybody sees use for
> finer-grained delays?
Bouncing reset lines, often the time required for reset is down in the
microseconds.
^ permalink raw reply
* Re: [RFC][PATCH v3 1/3] runtime interpreted power sequences
From: Mark Brown @ 2012-07-31 14:26 UTC (permalink / raw)
To: Thierry Reding
Cc: Alex Courbot, Stephen Warren, Stephen Warren, Simon Glass,
Grant Likely, Rob Herring, Greg Kroah-Hartman, Arnd Bergmann,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fbdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org
In-Reply-To: <20120731142216.GA19591@avionic-0098.adnet.avionic-design.de>
On Tue, Jul 31, 2012 at 04:22:17PM +0200, Thierry Reding wrote:
> On Tue, Jul 31, 2012 at 03:13:29PM +0100, Mark Brown wrote:
> > __devinit can be discarded if you disable enough kernel features,
> > HOTPLUG is the main one IIRC, modules might also need to go - drivers
> > really ought to take a copy of platform data they plan to use at
> > runtime, though practically speaking you have to try to trigger any
> > problems.
> HOTPLUG is marked EXPERT and explicitly states that it should only be
> disabled if you're not using modules or dynamic device discovery. I
> think if you've ignored all of that you're no longer entitled to
> complain.
This is framework code - it doesn't have much option. Disabling HOTPLUG
is totally reasonable on space constrained systems, there's no reason
for the code to break things for people.
^ permalink raw reply
* Re: [RFC][PATCH v3 1/3] runtime interpreted power sequences
From: Thierry Reding @ 2012-07-31 14:32 UTC (permalink / raw)
To: Mark Brown
Cc: Alex Courbot, Stephen Warren, Stephen Warren, Simon Glass,
Grant Likely, Rob Herring, Greg Kroah-Hartman, Arnd Bergmann,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <20120731142607.GV4468-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1161 bytes --]
On Tue, Jul 31, 2012 at 03:26:07PM +0100, Mark Brown wrote:
> On Tue, Jul 31, 2012 at 04:22:17PM +0200, Thierry Reding wrote:
> > On Tue, Jul 31, 2012 at 03:13:29PM +0100, Mark Brown wrote:
>
> > > __devinit can be discarded if you disable enough kernel features,
> > > HOTPLUG is the main one IIRC, modules might also need to go - drivers
> > > really ought to take a copy of platform data they plan to use at
> > > runtime, though practically speaking you have to try to trigger any
> > > problems.
>
> > HOTPLUG is marked EXPERT and explicitly states that it should only be
> > disabled if you're not using modules or dynamic device discovery. I
> > think if you've ignored all of that you're no longer entitled to
> > complain.
>
> This is framework code - it doesn't have much option. Disabling HOTPLUG
> is totally reasonable on space constrained systems, there's no reason
> for the code to break things for people.
Still if you use this code and disable HOTPLUG, then you shouldn't be
using modules either. I mean there is no way you can write a driver that
can gracefully handle its platform data being discarded.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC][PATCH v3 1/3] runtime interpreted power sequences
From: Mark Brown @ 2012-07-31 15:39 UTC (permalink / raw)
To: Thierry Reding
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Stephen Warren, Greg Kroah-Hartman,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
Alex Courbot, linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <20120731143235.GA21126-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org>
On Tue, Jul 31, 2012 at 04:32:35PM +0200, Thierry Reding wrote:
> On Tue, Jul 31, 2012 at 03:26:07PM +0100, Mark Brown wrote:
> > This is framework code - it doesn't have much option. Disabling HOTPLUG
> > is totally reasonable on space constrained systems, there's no reason
> > for the code to break things for people.
> Still if you use this code and disable HOTPLUG, then you shouldn't be
> using modules either. I mean there is no way you can write a driver that
Of course.
> can gracefully handle its platform data being discarded.
Sure there is - take a copy of the platform data in probe().
^ permalink raw reply
* Re: [PATCH 4/5] drivers/video/da8xx-fb.c: use devm_ functions
From: Sachin Kamat @ 2012-07-31 15:59 UTC (permalink / raw)
To: Damien Cassou
Cc: Florian Tobias Schandinat, kernel-janitors, linux-fbdev,
linux-kernel
In-Reply-To: <1343742860-16213-3-git-send-email-damien.cassou@lifl.fr>
On 31 July 2012 19:24, Damien Cassou <damien.cassou@lifl.fr> 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>
>
> ---
> drivers/video/da8xx-fb.c | 34 ++++++++--------------------------
> 1 file changed, 8 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> index 7ae9d53..0429431 100644
> --- a/drivers/video/da8xx-fb.c
> +++ b/drivers/video/da8xx-fb.c
> @@ -947,12 +947,9 @@ static int __devexit fb_remove(struct platform_device *dev)
> par->p_palette_base);
> dma_free_coherent(NULL, par->vram_size, par->vram_virt,
> par->vram_phys);
> - free_irq(par->irq, par);
> clk_disable(par->lcdc_clk);
> clk_put(par->lcdc_clk);
> framebuffer_release(info);
> - iounmap((void __iomem *)da8xx_fb_reg_base);
> - release_mem_region(lcdc_regs->start, resource_size(lcdc_regs));
>
> }
> return 0;
> @@ -1149,7 +1146,7 @@ static int __devinit fb_probe(struct platform_device *device)
> struct fb_info *da8xx_fb_info;
> struct clk *fb_clk = NULL;
> struct da8xx_fb_par *par;
> - resource_size_t len;
> +
> int ret, i;
> unsigned long ulcm;
>
> @@ -1159,29 +1156,20 @@ static int __devinit fb_probe(struct platform_device *device)
> }
>
> lcdc_regs = platform_get_resource(device, IORESOURCE_MEM, 0);
> - if (!lcdc_regs) {
> - dev_err(&device->dev,
> - "Can not get memory resource for LCD controller\n");
> - return -ENOENT;
> - }
> -
> - len = resource_size(lcdc_regs);
>
> - lcdc_regs = request_mem_region(lcdc_regs->start, len, lcdc_regs->name);
> - if (!lcdc_regs)
> - return -EBUSY;
> -
> - da8xx_fb_reg_base = (resource_size_t)ioremap(lcdc_regs->start, len);
> + da8xx_fb_reg_base > + (resource_size_t) devm_request_and_ioremap(&device->dev,
> + lcdc_regs);
> if (!da8xx_fb_reg_base) {
> ret = -EBUSY;
> - goto err_request_mem;
> + return ret;
You can directly do
return -EBUSY;
and remove the braces.
> }
>
> fb_clk = clk_get(&device->dev, NULL);
> if (IS_ERR(fb_clk)) {
> dev_err(&device->dev, "Can not get device clock\n");
> ret = -ENODEV;
> - goto err_ioremap;
> + return ret;
ditto
> }
> ret = clk_enable(fb_clk);
> if (ret)
> @@ -1359,8 +1347,8 @@ static int __devinit fb_probe(struct platform_device *device)
> else
> lcdc_irq_handler = lcdc_irq_handler_rev02;
>
> - ret = request_irq(par->irq, lcdc_irq_handler, 0,
> - DRIVER_NAME, par);
> + ret = devm_request_irq(&device->dev, par->irq, lcdc_irq_handler, 0,
> + DRIVER_NAME, par);
> if (ret)
> goto irq_freq;
> return 0;
> @@ -1391,12 +1379,6 @@ err_clk_disable:
> err_clk_put:
> clk_put(fb_clk);
>
> -err_ioremap:
> - iounmap((void __iomem *)da8xx_fb_reg_base);
> -
> -err_request_mem:
> - release_mem_region(lcdc_regs->start, len);
> -
> return ret;
> }
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
With warm regards,
Sachin
^ permalink raw reply
* Re: [PATCH 4/5] drivers/video/da8xx-fb.c: use devm_ functions
From: Damien Cassou @ 2012-07-31 16:01 UTC (permalink / raw)
To: Sachin Kamat
Cc: Florian Tobias Schandinat, kernel-janitors, linux-fbdev,
linux-kernel
In-Reply-To: <CAK9yfHxy4DGCwZjDCdc3A-a3oKoMGQruDMhfq0uR+2+7nfFHoA@mail.gmail.com>
The whole patch is incorrect because of:
On Tue, Jul 31, 2012 at 5:47 PM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>> - ret = request_irq(par->irq, lcdc_irq_handler, 0,
>> - DRIVER_NAME, par);
>> + ret = devm_request_irq(&device->dev, par->irq, lcdc_irq_handler, 0,
>> + DRIVER_NAME, par);
The following message explains why this is problematic:
http://marc.info/?l=kernel-janitors&m\x134374464625591&w=2
Please ignore this whole patch altogether.
Sorry about that.
--
Damien Cassou
http://damiencassou.seasidehosting.st
"Lambdas are relegated to relative obscurity until Java makes them
popular by not having them." James Iry
^ permalink raw reply
* Re: [PATCH 2/5] drivers/video/bfin-t350mcqb-fb.c: use devm_ functions
From: Damien Cassou @ 2012-07-31 16:18 UTC (permalink / raw)
To: Mike Frysinger
Cc: Florian Tobias Schandinat, kernel-janitors, linux-fbdev,
linux-kernel
In-Reply-To: <CAMjpGUdo0SavFWR_tk50_6nZMsJmxYO5BxH9Fp=UTw5sW-0+Rw@mail.gmail.com>
On Tue, Jul 31, 2012 at 3:57 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tue, Jul 31, 2012 at 9:54 AM, Damien Cassou <damien.cassou@lifl.fr> wrote:
>> 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.
>
> Acked-by: Mike Frysinger <vapier@gentoo.org>
This whole patch is invalid, because of this part:
- info->fb_buffer = dma_alloc_coherent(NULL, fbinfo->fix.smem_len +
- ACTIVE_VIDEO_MEM_OFFSET,
- &info->dma_handle, GFP_KERNEL);
+ info->fb_buffer = dmam_alloc_coherent(&pdev->dev, NULL,
+ fbinfo->fix.smem_len +
+ ACTIVE_VIDEO_MEM_OFFSET,
+ &info->dma_handle, GFP_KERNEL);
dmam_alloc_coherent() is called with 5 arguments but only accepts 4.
Please ignore this whole patch altogether.
I am sorry about that.
--
Damien Cassou
http://damiencassou.seasidehosting.st
"Lambdas are relegated to relative obscurity until Java makes them
popular by not having them." James Iry
^ permalink raw reply
* Re: [RFC][PATCH v3 1/3] runtime interpreted power sequences
From: Greg Kroah-Hartman @ 2012-07-31 16:19 UTC (permalink / raw)
To: Mark Brown
Cc: Thierry Reding, Alex Courbot, Stephen Warren, Stephen Warren,
Simon Glass, Grant Likely, Rob Herring, Arnd Bergmann,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fbdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org
In-Reply-To: <20120731153941.GF4468@opensource.wolfsonmicro.com>
On Tue, Jul 31, 2012 at 04:39:41PM +0100, Mark Brown wrote:
> On Tue, Jul 31, 2012 at 04:32:35PM +0200, Thierry Reding wrote:
> > On Tue, Jul 31, 2012 at 03:26:07PM +0100, Mark Brown wrote:
>
> > > This is framework code - it doesn't have much option. Disabling HOTPLUG
> > > is totally reasonable on space constrained systems, there's no reason
> > > for the code to break things for people.
>
> > Still if you use this code and disable HOTPLUG, then you shouldn't be
> > using modules either. I mean there is no way you can write a driver that
>
> Of course.
>
> > can gracefully handle its platform data being discarded.
>
> Sure there is - take a copy of the platform data in probe().
Ick ick ick, devdata needs to die as it's pretty much pointless these
days.
It's on my TODO list actually...
greg k-h
^ permalink raw reply
* Re: [RFC][PATCH v3 1/3] runtime interpreted power sequences
From: Mark Brown @ 2012-07-31 16:22 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Thierry Reding, Alex Courbot, Stephen Warren, Stephen Warren,
Simon Glass, Grant Likely, Rob Herring, Arnd Bergmann,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <20120731161954.GB4941-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
On Tue, Jul 31, 2012 at 09:19:54AM -0700, Greg Kroah-Hartman wrote:
> On Tue, Jul 31, 2012 at 04:39:41PM +0100, Mark Brown wrote:
> > On Tue, Jul 31, 2012 at 04:32:35PM +0200, Thierry Reding wrote:
> > > can gracefully handle its platform data being discarded.
> > Sure there is - take a copy of the platform data in probe().
> Ick ick ick, devdata needs to die as it's pretty much pointless these
> days.
Hrm? I'm not sure I understand the direct relevance here - we're
talking about platform data.
^ permalink raw reply
* Re: [RFC][PATCH v3 1/3] runtime interpreted power sequences
From: Stephen Warren @ 2012-07-31 16:34 UTC (permalink / raw)
To: Alex Courbot
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Greg Kroah-Hartman, Mark Brown, Arnd Bergmann,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fbdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org
In-Reply-To: <5017B434.2010706@nvidia.com>
On 07/31/2012 04:32 AM, Alex Courbot wrote:
> On 07/31/2012 07:45 AM, Stephen Warren wrote:
...
>> If the nodes have a unit address (i.e. end in "@n"), which they will
>> have to if all named "step" and there's more than one of them, then they
>> will need a matching reg property. Equally, the parent node will need
>> #address-cells and #size-cells too. So, the last couple lines would be:
>>
>> power-on-sequence {
>> #address-cells = <1>;
>> #size-cells = <0>;
>> step@0 {
>> reg = <0>;
>
> That's precisely what I would like to avoid - I don't need the steps to
> be numbered and I certainly have no use for a reg property. Isn't there
> a way to make it simpler?
You may be able to get away without using the reg values in the code.
However, to have a semantically correct device tree, you really do need
all of those properties.
That said, I think you might need to use the reg values in code. I
believe there's no guarantee of the order in which nodes enumerate in
device tree, so you need to look at the reg property in order to find
the order in which to execute the nodes/steps.
... although perhaps the desire to avoid a dependency on DT ordering
applies more to the order in which devices instantiated from DT get
probed, rather than the order of low-level node enumeration, so I may be
wrong here. I've definitely seen Grant caution people not to rely on
device probe order, but perhaps he didn't make the same comment about
node order.
^ permalink raw reply
* [PATCH 0/5] use devm_ functions
From: Damien Cassou @ 2012-07-31 16:39 UTC (permalink / raw)
To: Florian Tobias Schandinat; +Cc: kernel-janitors, linux-fbdev, linux-kernel
In-Reply-To: <1343742860-16213-1-git-send-email-damien.cassou@lifl.fr>
These patches introduce devm_ functions in some video drivers.
^ permalink raw reply
* [PATCH 3/5] drivers/video/mbx/mbxfb.c: use devm_ functions
From: Damien Cassou @ 2012-07-31 16:39 UTC (permalink / raw)
To: Florian Tobias Schandinat; +Cc: kernel-janitors, linux-fbdev, linux-kernel
In-Reply-To: <1343752762-16861-1-git-send-email-damien.cassou@lifl.fr>
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>
---
drivers/video/mbx/mbxfb.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/video/mbx/mbxfb.c b/drivers/video/mbx/mbxfb.c
index 85e4f44..9229acf 100644
--- a/drivers/video/mbx/mbxfb.c
+++ b/drivers/video/mbx/mbxfb.c
@@ -939,8 +939,9 @@ static int __devinit mbxfb_probe(struct platform_device *dev)
}
mfbi->reg_phys_addr = mfbi->reg_res->start;
- mfbi->reg_virt_addr = ioremap_nocache(mfbi->reg_phys_addr,
- res_size(mfbi->reg_req));
+ mfbi->reg_virt_addr = devm_ioremap_nocache(&dev->dev,
+ mfbi->reg_phys_addr,
+ res_size(mfbi->reg_req));
if (!mfbi->reg_virt_addr) {
dev_err(&dev->dev, "failed to ioremap Marathon registers\n");
ret = -EINVAL;
@@ -948,12 +949,12 @@ static int __devinit mbxfb_probe(struct platform_device *dev)
}
virt_base_2700 = mfbi->reg_virt_addr;
- mfbi->fb_virt_addr = ioremap_nocache(mfbi->fb_phys_addr,
- res_size(mfbi->fb_req));
+ mfbi->fb_virt_addr = devm_ioremap_nocache(&dev->dev, mfbi->fb_phys_addr,
+ res_size(mfbi->fb_req));
if (!mfbi->fb_virt_addr) {
dev_err(&dev->dev, "failed to ioremap frame buffer\n");
ret = -EINVAL;
- goto err4;
+ goto err3;
}
fbi->screen_base = (char __iomem *)(mfbi->fb_virt_addr + 0x60000);
@@ -971,7 +972,7 @@ static int __devinit mbxfb_probe(struct platform_device *dev)
if (ret < 0) {
dev_err(&dev->dev, "fb_alloc_cmap failed\n");
ret = -EINVAL;
- goto err5;
+ goto err3;
}
platform_set_drvdata(dev, fbi);
@@ -996,10 +997,6 @@ static int __devinit mbxfb_probe(struct platform_device *dev)
err6:
fb_dealloc_cmap(&fbi->cmap);
-err5:
- iounmap(mfbi->fb_virt_addr);
-err4:
- iounmap(mfbi->reg_virt_addr);
err3:
release_mem_region(mfbi->reg_res->start, res_size(mfbi->reg_res));
err2:
@@ -1026,10 +1023,7 @@ static int __devexit mbxfb_remove(struct platform_device *dev)
if (mfbi->platform_remove)
mfbi->platform_remove(fbi);
- if (mfbi->fb_virt_addr)
- iounmap(mfbi->fb_virt_addr);
- if (mfbi->reg_virt_addr)
- iounmap(mfbi->reg_virt_addr);
+
if (mfbi->reg_req)
release_mem_region(mfbi->reg_req->start,
res_size(mfbi->reg_req));
^ permalink raw reply related
* [PATCH 2/5] drivers/video/gbefb.c: use devm_ functions
From: Damien Cassou @ 2012-07-31 16:39 UTC (permalink / raw)
To: Florian Tobias Schandinat; +Cc: kernel-janitors, linux-fbdev, linux-kernel
In-Reply-To: <1343752762-16861-1-git-send-email-damien.cassou@lifl.fr>
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>
---
drivers/video/gbefb.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
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));
if (!gbe) {
printk(KERN_ERR "gbefb: couldn't map mmio region\n");
ret = -ENXIO;
@@ -1170,12 +1171,13 @@ static int __devinit gbefb_probe(struct platform_device *p_dev)
if (!gbe_tiles.cpu) {
printk(KERN_ERR "gbefb: couldn't allocate tiles table\n");
ret = -ENOMEM;
- goto out_unmap;
+ goto out_release_mem_region;
}
if (gbe_mem_phys) {
/* memory was allocated at boot time */
- gbe_mem = ioremap_nocache(gbe_mem_phys, gbe_mem_size);
+ gbe_mem = devm_ioremap_nocache(&p_dev->dev, gbe_mem_phys,
+ gbe_mem_size);
if (!gbe_mem) {
printk(KERN_ERR "gbefb: couldn't map framebuffer\n");
ret = -ENOMEM;
@@ -1241,13 +1243,9 @@ static int __devinit gbefb_probe(struct platform_device *p_dev)
out_gbe_unmap:
if (gbe_dma_addr)
dma_free_coherent(NULL, gbe_mem_size, gbe_mem, gbe_mem_phys);
- else
- iounmap(gbe_mem);
out_tiles_free:
dma_free_coherent(NULL, GBE_TLB_SIZE * sizeof(uint16_t),
(void *)gbe_tiles.cpu, gbe_tiles.dma);
-out_unmap:
- iounmap(gbe);
out_release_mem_region:
release_mem_region(GBE_BASE, sizeof(struct sgi_gbe));
out_release_framebuffer:
@@ -1264,12 +1262,9 @@ static int __devexit gbefb_remove(struct platform_device* p_dev)
gbe_turn_off();
if (gbe_dma_addr)
dma_free_coherent(NULL, gbe_mem_size, gbe_mem, gbe_mem_phys);
- else
- iounmap(gbe_mem);
dma_free_coherent(NULL, GBE_TLB_SIZE * sizeof(uint16_t),
(void *)gbe_tiles.cpu, gbe_tiles.dma);
release_mem_region(GBE_BASE, sizeof(struct sgi_gbe));
- iounmap(gbe);
gbefb_remove_sysfs(&p_dev->dev);
framebuffer_release(info);
^ permalink raw reply related
* [PATCH 1/5] drivers/video/fsl-diu-fb.c: use devm_ functions
From: Damien Cassou @ 2012-07-31 16:39 UTC (permalink / raw)
To: Florian Tobias Schandinat; +Cc: kernel-janitors, linux-fbdev, linux-kernel
In-Reply-To: <1343752762-16861-1-git-send-email-damien.cassou@lifl.fr>
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>
---
drivers/video/fsl-diu-fb.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 458c006..19194c5 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1501,8 +1501,8 @@ static int __devinit fsl_diu_probe(struct platform_device *pdev)
unsigned int i;
int ret;
- data = dma_alloc_coherent(&pdev->dev, sizeof(struct fsl_diu_data),
- &dma_addr, GFP_DMA | __GFP_ZERO);
+ data = dmam_alloc_coherent(&pdev->dev, sizeof(struct fsl_diu_data),
+ &dma_addr, GFP_DMA | __GFP_ZERO);
if (!data)
return -ENOMEM;
data->dma_addr = dma_addr;
@@ -1628,9 +1628,6 @@ error:
iounmap(data->diu_reg);
- dma_free_coherent(&pdev->dev, sizeof(struct fsl_diu_data), data,
- data->dma_addr);
-
return ret;
}
@@ -1648,9 +1645,6 @@ static int fsl_diu_remove(struct platform_device *pdev)
iounmap(data->diu_reg);
- dma_free_coherent(&pdev->dev, sizeof(struct fsl_diu_data), data,
- data->dma_addr);
-
return 0;
}
^ permalink raw reply related
* [PATCH 4/5] drivers/video/exynos/exynos_mipi_dsi.c: use devm_ functions
From: Damien Cassou @ 2012-07-31 16:39 UTC (permalink / raw)
To: Inki Dae
Cc: kernel-janitors, Donghwa Lee, Kyungmin Park,
Florian Tobias Schandinat, linux-fbdev, linux-kernel
In-Reply-To: <1343752762-16861-1-git-send-email-damien.cassou@lifl.fr>
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>
---
drivers/video/exynos/exynos_mipi_dsi.c | 41 ++++++++++++---------------------
1 file changed, 15 insertions(+), 26 deletions(-)
diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
index 4bc2b8a..c277a07 100644
--- a/drivers/video/exynos/exynos_mipi_dsi.c
+++ b/drivers/video/exynos/exynos_mipi_dsi.c
@@ -336,7 +336,8 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
struct mipi_dsim_ddi *dsim_ddi;
int ret = -EINVAL;
- dsim = kzalloc(sizeof(struct mipi_dsim_device), GFP_KERNEL);
+ dsim = devm_kzalloc(&pdev->dev, sizeof(struct mipi_dsim_device),
+ GFP_KERNEL);
if (!dsim) {
dev_err(&pdev->dev, "failed to allocate dsim object.\n");
return -ENOMEM;
@@ -350,13 +351,13 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
dsim_pd = (struct mipi_dsim_platform_data *)dsim->pd;
if (dsim_pd = NULL) {
dev_err(&pdev->dev, "failed to get platform data for dsim.\n");
- goto err_clock_get;
+ goto err_platform_get_irq;
}
/* get mipi_dsim_config. */
dsim_config = dsim_pd->dsim_config;
if (dsim_config = NULL) {
dev_err(&pdev->dev, "failed to get dsim config data.\n");
- goto err_clock_get;
+ goto err_platform_get_irq;
}
dsim->dsim_config = dsim_config;
@@ -367,13 +368,13 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
ret = regulator_bulk_get(&pdev->dev, ARRAY_SIZE(supplies), supplies);
if (ret) {
dev_err(&pdev->dev, "Failed to get regulators: %d\n", ret);
- goto err_clock_get;
+ goto err_platform_get_irq;
}
- dsim->clock = clk_get(&pdev->dev, "dsim0");
+ dsim->clock = devm_clk_get(&pdev->dev, "dsim0");
if (IS_ERR(dsim->clock)) {
dev_err(&pdev->dev, "failed to get dsim clock source\n");
- goto err_clock_get;
+ goto err_platform_get_irq;
}
clk_enable(dsim->clock);
@@ -384,15 +385,17 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
goto err_platform_get;
}
- dsim->res = request_mem_region(res->start, resource_size(res),
- dev_name(&pdev->dev));
+ dsim->res = devm_request_mem_region(&pdev->dev, res->start,
+ resource_size(res),
+ dev_name(&pdev->dev));
if (!dsim->res) {
dev_err(&pdev->dev, "failed to request io memory region\n");
ret = -ENOMEM;
- goto err_mem_region;
+ goto err_platform_get;
}
- dsim->reg_base = ioremap(res->start, resource_size(res));
+ dsim->reg_base = devm_ioremap(&pdev->dev, res->start,
+ resource_size(res));
if (!dsim->reg_base) {
dev_err(&pdev->dev, "failed to remap io region\n");
ret = -ENOMEM;
@@ -405,7 +408,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
dsim_ddi = exynos_mipi_dsi_bind_lcd_ddi(dsim, dsim_pd->lcd_panel_name);
if (!dsim_ddi) {
dev_err(&pdev->dev, "mipi_dsim_ddi object not found.\n");
- goto err_bind;
+ goto err_ioremap;
}
dsim->irq = platform_get_irq(pdev, 0);
@@ -424,7 +427,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
if (ret != 0) {
dev_err(&pdev->dev, "failed to request dsim irq\n");
ret = -EINVAL;
- goto err_bind;
+ goto err_ioremap;
}
/* enable interrupts */
@@ -466,20 +469,11 @@ done:
return 0;
-err_bind:
- iounmap(dsim->reg_base);
-
err_ioremap:
release_mem_region(dsim->res->start, resource_size(dsim->res));
-err_mem_region:
- release_resource(dsim->res);
-
err_platform_get:
clk_disable(dsim->clock);
- clk_put(dsim->clock);
-err_clock_get:
- kfree(dsim);
err_platform_get_irq:
return ret;
@@ -491,12 +485,8 @@ static int __devexit exynos_mipi_dsi_remove(struct platform_device *pdev)
struct mipi_dsim_ddi *dsim_ddi, *next;
struct mipi_dsim_lcd_driver *dsim_lcd_drv;
- iounmap(dsim->reg_base);
-
clk_disable(dsim->clock);
- clk_put(dsim->clock);
- release_resource(dsim->res);
release_mem_region(dsim->res->start, resource_size(dsim->res));
list_for_each_entry_safe(dsim_ddi, next, &dsim_ddi_list, list) {
@@ -514,7 +504,6 @@ static int __devexit exynos_mipi_dsi_remove(struct platform_device *pdev)
}
regulator_bulk_free(ARRAY_SIZE(supplies), supplies);
- kfree(dsim);
return 0;
}
^ permalink raw reply related
* [PATCH 5/5] drivers/video/exynos/exynos_dp_core.c: use devm_ functions
From: Damien Cassou @ 2012-07-31 16:39 UTC (permalink / raw)
To: Jingoo Han
Cc: kernel-janitors, Florian Tobias Schandinat, linux-fbdev,
linux-kernel
In-Reply-To: <1343752762-16861-1-git-send-email-damien.cassou@lifl.fr>
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>
---
drivers/video/exynos/exynos_dp_core.c | 27 +++++++--------------------
1 file changed, 7 insertions(+), 20 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index c6c016a..00fe4f0 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -872,7 +872,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
dp->dev = &pdev->dev;
- dp->clock = clk_get(&pdev->dev, "dp");
+ dp->clock = devm_clk_get(&pdev->dev, "dp");
if (IS_ERR(dp->clock)) {
dev_err(&pdev->dev, "failed to get clock\n");
return PTR_ERR(dp->clock);
@@ -881,31 +881,24 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
clk_enable(dp->clock);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(&pdev->dev, "failed to get registers\n");
- ret = -EINVAL;
- goto err_clock;
- }
dp->reg_base = devm_request_and_ioremap(&pdev->dev, res);
if (!dp->reg_base) {
dev_err(&pdev->dev, "failed to ioremap\n");
- ret = -ENOMEM;
- goto err_clock;
+ return -ENOMEM;
}
dp->irq = platform_get_irq(pdev, 0);
if (!dp->irq) {
dev_err(&pdev->dev, "failed to get irq\n");
- ret = -ENODEV;
- goto err_clock;
+ return -ENODEV;
}
ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
"exynos-dp", dp);
if (ret) {
dev_err(&pdev->dev, "failed to request irq\n");
- goto err_clock;
+ return ret;
}
dp->video_info = pdata->video_info;
@@ -917,7 +910,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
ret = exynos_dp_detect_hpd(dp);
if (ret) {
dev_err(&pdev->dev, "unable to detect hpd\n");
- goto err_clock;
+ return ret;
}
exynos_dp_handle_edid(dp);
@@ -926,7 +919,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
dp->video_info->link_rate);
if (ret) {
dev_err(&pdev->dev, "unable to do link train\n");
- goto err_clock;
+ return ret;
}
exynos_dp_enable_scramble(dp, 1);
@@ -940,17 +933,12 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
ret = exynos_dp_config_video(dp, dp->video_info);
if (ret) {
dev_err(&pdev->dev, "unable to config video\n");
- goto err_clock;
+ return ret;
}
platform_set_drvdata(pdev, dp);
return 0;
-
-err_clock:
- clk_put(dp->clock);
-
- return ret;
}
static int __devexit exynos_dp_remove(struct platform_device *pdev)
@@ -962,7 +950,6 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
pdata->phy_exit();
clk_disable(dp->clock);
- clk_put(dp->clock);
return 0;
}
^ permalink raw reply related
* Re: [RFC][PATCH v3 1/3] runtime interpreted power sequences
From: Greg Kroah-Hartman @ 2012-07-31 16:42 UTC (permalink / raw)
To: Mark Brown
Cc: Thierry Reding, Alex Courbot, Stephen Warren, Stephen Warren,
Simon Glass, Grant Likely, Rob Herring, Arnd Bergmann,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <20120731162230.GE11892-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
On Tue, Jul 31, 2012 at 05:22:30PM +0100, Mark Brown wrote:
> On Tue, Jul 31, 2012 at 09:19:54AM -0700, Greg Kroah-Hartman wrote:
> > On Tue, Jul 31, 2012 at 04:39:41PM +0100, Mark Brown wrote:
> > > On Tue, Jul 31, 2012 at 04:32:35PM +0200, Thierry Reding wrote:
>
> > > > can gracefully handle its platform data being discarded.
>
> > > Sure there is - take a copy of the platform data in probe().
>
> > Ick ick ick, devdata needs to die as it's pretty much pointless these
> > days.
>
> Hrm? I'm not sure I understand the direct relevance here - we're
> talking about platform data.
The platform data was marked __devdata, and you said it could be thrown
away after init so you should keep it around somehow.
Anyway, not really the main point here at all, sorry for the noise...
greg k-h
^ permalink raw reply
* Re: [RFC][PATCH v3 1/3] runtime interpreted power sequences
From: Mark Brown @ 2012-07-31 16:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Thierry Reding, Alex Courbot, Stephen Warren, Stephen Warren,
Simon Glass, Grant Likely, Rob Herring, Arnd Bergmann,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fbdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org
In-Reply-To: <20120731164245.GA5762@kroah.com>
On Tue, Jul 31, 2012 at 09:42:45AM -0700, Greg Kroah-Hartman wrote:
> On Tue, Jul 31, 2012 at 05:22:30PM +0100, Mark Brown wrote:
> > Hrm? I'm not sure I understand the direct relevance here - we're
> > talking about platform data.
> The platform data was marked __devdata, and you said it could be thrown
> away after init so you should keep it around somehow.
Ah, sorry - I was thinking __devinitdata.
^ permalink raw reply
* Re: [PATCH] OMAPDSS: Do not require a VDDS_DSI regulator on am35xx
From: Raphaël Assénat @ 2012-07-31 20:46 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <1343724323.2633.22.camel@deskari>
On 31/07/12 04:45 AM, Tomi Valkeinen wrote:
> On Thu, 2012-07-19 at 16:04 -0400, Raphael Assenat wrote:
>> On our AM3505 based board, dpi.c complains that there is no VDSS_DSI regulator
>> and the framebuffer cannot be enabled. However, this check does not seem to
>> apply to AM3505/17 chips.
>>
>> I am not the first facing this issue, see this thread from Nov. 2011:
>> http://marc.info/?l=linux-omap&m\x132147745930213&w=2
>>
>> The string 'vdds_dsi' does appear once in the technical reference manual[1]
>> but there is no corresponding power pin on the package[2]. I failed to
>> locate any signal that could be an equivalent. I am trying to obtain some
>> clarifications on TI's forum[3]...
>>
>> In any case, I am currently running with the patch below. In order to keep
>> cpu_is_xx uses to a minimum, I check for am35xx once at init time and allow
>> dpi.vdds_dsi_reg to be NULL from then on, getting rid of all the other
>> cpu_is_omap34xx uses in the process.
>>
>> Your comments would be appreciated. Please also consider for merging.
>
> VDDS_DSI is used to power up some of the DSS pins on OMAP3. I don't know
> why the HW was designed like that... If you have a correct image without
> the power, then obviously it's not needed.
Yes, I confirm the image is displayed properly.
> We don't currently deal with AM3xxx SoCs in any way in the driver. It's
> difficult enough trying to handle just OMAP DSS versions, and now we
> need to add AM3xxx to the mix. Sigh =).
>
> However, I don't want to apply this patch, as we're trying to remove the
> cpu_is checks (soc_is goes in the same category).
>
> I guess we need to add entries for the AM3xxx SoCs in the
> dss_features.c.
>
> Any idea what other differences AM3xxx has compared to OMAP3?
This is the only one I am aware of.
Best regards,
Raphaël Assénat
^ permalink raw reply
* Re: [PATCH 5/5] drivers/video/exynos/exynos_dp_core.c: use devm_ functions
From: Jingoo Han @ 2012-07-31 23:21 UTC (permalink / raw)
To: 'Damien Cassou'
Cc: kernel-janitors, 'Florian Tobias Schandinat', linux-fbdev,
linux-kernel, 'Jingoo Han'
In-Reply-To: <1343752762-16861-6-git-send-email-damien.cassou@lifl.fr>
On Wednesday, August 01, 2012 1:39 AM 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>
>
> ---
> drivers/video/exynos/exynos_dp_core.c | 27 +++++++--------------------
> 1 file changed, 7 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index c6c016a..00fe4f0 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -872,7 +872,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>
> dp->dev = &pdev->dev;
>
> - dp->clock = clk_get(&pdev->dev, "dp");
> + dp->clock = devm_clk_get(&pdev->dev, "dp");
> if (IS_ERR(dp->clock)) {
> dev_err(&pdev->dev, "failed to get clock\n");
> return PTR_ERR(dp->clock);
> @@ -881,31 +881,24 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> clk_enable(dp->clock);
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!res) {
> - dev_err(&pdev->dev, "failed to get registers\n");
> - ret = -EINVAL;
> - goto err_clock;
> - }
Why do you remove this return check?
If there is no reason, please, do it as follows:
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
dev_err(&pdev->dev, "failed to get registers\n");
- ret = -EINVAL;
- goto err_clock;
+ return -EINVAL;
}
Best regards,
Jingoo Han
>
> dp->reg_base = devm_request_and_ioremap(&pdev->dev, res);
> if (!dp->reg_base) {
> dev_err(&pdev->dev, "failed to ioremap\n");
> - ret = -ENOMEM;
> - goto err_clock;
> + return -ENOMEM;
> }
>
> dp->irq = platform_get_irq(pdev, 0);
> if (!dp->irq) {
> dev_err(&pdev->dev, "failed to get irq\n");
> - ret = -ENODEV;
> - goto err_clock;
> + return -ENODEV;
> }
>
> ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
> "exynos-dp", dp);
> if (ret) {
> dev_err(&pdev->dev, "failed to request irq\n");
> - goto err_clock;
> + return ret;
> }
>
> dp->video_info = pdata->video_info;
> @@ -917,7 +910,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> ret = exynos_dp_detect_hpd(dp);
> if (ret) {
> dev_err(&pdev->dev, "unable to detect hpd\n");
> - goto err_clock;
> + return ret;
> }
>
> exynos_dp_handle_edid(dp);
> @@ -926,7 +919,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> dp->video_info->link_rate);
> if (ret) {
> dev_err(&pdev->dev, "unable to do link train\n");
> - goto err_clock;
> + return ret;
> }
>
> exynos_dp_enable_scramble(dp, 1);
> @@ -940,17 +933,12 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> ret = exynos_dp_config_video(dp, dp->video_info);
> if (ret) {
> dev_err(&pdev->dev, "unable to config video\n");
> - goto err_clock;
> + return ret;
> }
>
> platform_set_drvdata(pdev, dp);
>
> return 0;
> -
> -err_clock:
> - clk_put(dp->clock);
> -
> - return ret;
> }
>
> static int __devexit exynos_dp_remove(struct platform_device *pdev)
> @@ -962,7 +950,6 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
> pdata->phy_exit();
>
> clk_disable(dp->clock);
> - clk_put(dp->clock);
>
> return 0;
> }
^ 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