* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Tomi Valkeinen @ 2012-06-25 9:30 UTC (permalink / raw)
To: Jassi Brar; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <CAJe_Zhf-H0dLtFsFAcryDy3av60RjmWmP+d3GSaxq8dNZKiXWw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 937 bytes --]
On Mon, 2012-06-25 at 14:19 +0530, Jassi Brar wrote:
> On 25 June 2012 11:50, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Sat, 2012-06-23 at 13:36 +0530, jaswinder.singh@linaro.org wrote:
> ....
> >> Currenlty HDMI fails to come up in the suspend-resume path.
> >> This patch helps that real-world scenario.
> >
> > What is the problem there? It'd be good to explain the problem in the
> > patch description. Does the pm_runtime_get return -EACCES?
> >
> Yes, it returns -EACCESS because RPM on devices is disabled during the
> period from suspend-start to resume-finished.
So... You didn't answer my first comment, how can the code work? The
driver needs to enable the HW and the call to pm_runtime_get() is
skipped. Won't this lead to crash as the DSS registers are accessed
without the HW in enabled state? And what happens if the
pm_runtime_get() call is skipped, but pm_runtime_put() is not?
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] grvga: Fix error handling issues
From: Emil Goode @ 2012-06-25 9:58 UTC (permalink / raw)
To: Julia Lawall
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors,
w.sang
In-Reply-To: <alpine.DEB.2.02.1206250718360.1874@hadrien>
It would be nice to get rid of the indention levels in the error
handling code and the devm is to prefer from the older api.
I'm traveling but will probably send another version of this patch this
evening.
Thanks,
Emil
On Mon, 2012-06-25 at 07:20 +0200, Julia Lawall wrote:
> On Sun, 24 Jun 2012, Emil Goode wrote:
>
> > Yes I considered converting to devm_ and keeping the of_iounmap in.
> > But I just feel like I'm mixing two api's. If converting to devm_ really
> > is the right thing to do, tell me and I will send another patch :)
>
> devm is safer and simpler, because you can just get rid of error handling
> code rather than ensuring over time that it continues to be done in all of
> the place where it is needed. So I would think that it is always better
> to use it. But others may think differently. Another option would be to
> only use it for request_mem_region, which you can do consistently in this
> case, and leave both of_ioremap and ioremap as they are.
>
> julia
>
>
> >
> > Thanks,
> >
> > Emil
> >
> > On Sun, 2012-06-24 at 07:15 +0200, Julia Lawall wrote:
> > > I see. You could still use a devm_ function for request_mem_region.
> > > Also I noticed that the remove function uses iounmap directly, not
> > > io_iounmap. The latter is just a wrapper for the former, but it could be
> > > good to use the right name.
> > >
> > > julia
> > >
> > > On Sun, 24 Jun 2012, Emil Goode wrote:
> > >
> > > > The of_ioremap function is used in this code as well and I don't know of
> > > > a devm_ equivalent for it. For consistency I think it is better to leave
> > > > it as it is in this case. So I stick with v1 of this patch.
> > > >
> > > > Thanks,
> > > >
> > > > Emil
> > > >
> > > > On Fri, 2012-06-22 at 18:21 +0200, Julia Lawall wrote:
> > > >> On Fri, 22 Jun 2012, Emil Goode wrote:
> > > >>
> > > >>> Good idea, I will take another look at it tomorrow.
> > > >>
> > > >> There is a devm_ function that combines request_mem_region and ioremap
> > > >> that could be useful.
> > > >>
> > > >> julia
> > > >>
> > > >>>
> > > >>> Thanks,
> > > >>>
> > > >>> Emil
> > > >>>
> > > >>> On Fri, 2012-06-22 at 18:12 +0200, Julia Lawall wrote:
> > > >>>> Maybe you could used the devm_ functions for request_mem_region and
> > > >>>> ioremap so that the error handling can just be dropped?
> > > >>>>
> > > >>>> julia
> > > >>>>
> > > >>>> On Fri, 22 Jun 2012, Emil Goode wrote:
> > > >>>>
> > > >>>>> This patch fixes two problems with the error handling in the
> > > >>>>> grvga_probe function.
> > > >>>>>
> > > >>>>> - If the call to grvga_parse_custom on line 370 fails we use
> > > >>>>> the wrong label so that release_mem_region will be called
> > > >>>>> without a call to request_mem_region being made.
> > > >>>>>
> > > >>>>> - If the call to ioremap on line 436 fails we should not try
> > > >>>>> to call iounmap. I added an if statement to check whether or
> > > >>>>> not a call to iounmap should be made.
> > > >>>>>
> > > >>>>> - I also changed the names of the labels to make the code
> > > >>>>> easier to read.
> > > >>>>>
> > > >>>>> Signed-off-by: Emil Goode <emilgoode@gmail.com>
> > > >>>>> ---
> > > >>>>> drivers/video/grvga.c | 34 ++++++++++++++++++----------------
> > > >>>>> 1 file changed, 18 insertions(+), 16 deletions(-)
> > > >>>>>
> > > >>>>> diff --git a/drivers/video/grvga.c b/drivers/video/grvga.c
> > > >>>>> index da066c2..d9d688a 100644
> > > >>>>> --- a/drivers/video/grvga.c
> > > >>>>> +++ b/drivers/video/grvga.c
> > > >>>>> @@ -354,7 +354,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > >>>>> */
> > > >>>>> if (fb_get_options("grvga", &options)) {
> > > >>>>> retval = -ENODEV;
> > > >>>>> - goto err;
> > > >>>>> + goto free_fb;
> > > >>>>> }
> > > >>>>>
> > > >>>>> if (!options || !*options)
> > > >>>>> @@ -370,7 +370,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > >>>>> if (grvga_parse_custom(this_opt, &info->var) < 0) {
> > > >>>>> dev_err(&dev->dev, "Failed to parse custom mode (%s).\n", this_opt);
> > > >>>>> retval = -EINVAL;
> > > >>>>> - goto err1;
> > > >>>>> + goto free_fb;
> > > >>>>> }
> > > >>>>> } else if (!strncmp(this_opt, "addr", 4))
> > > >>>>> grvga_fix_addr = simple_strtoul(this_opt + 5, NULL, 16);
> > > >>>>> @@ -390,7 +390,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > >>>>> if (!request_mem_region(dev->resource[0].start, resource_size(&dev->resource[0]), "grlib-svgactrl regs")) {
> > > >>>>> dev_err(&dev->dev, "registers already mapped\n");
> > > >>>>> retval = -EBUSY;
> > > >>>>> - goto err;
> > > >>>>> + goto free_fb;
> > > >>>>> }
> > > >>>>>
> > > >>>>> par->regs = of_ioremap(&dev->resource[0], 0,
> > > >>>>> @@ -400,14 +400,14 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > >>>>> if (!par->regs) {
> > > >>>>> dev_err(&dev->dev, "failed to map registers\n");
> > > >>>>> retval = -ENOMEM;
> > > >>>>> - goto err1;
> > > >>>>> + goto release_regs;
> > > >>>>> }
> > > >>>>>
> > > >>>>> retval = fb_alloc_cmap(&info->cmap, 256, 0);
> > > >>>>> if (retval < 0) {
> > > >>>>> dev_err(&dev->dev, "failed to allocate mem with fb_alloc_cmap\n");
> > > >>>>> retval = -ENOMEM;
> > > >>>>> - goto err2;
> > > >>>>> + goto unmap_regs;
> > > >>>>> }
> > > >>>>>
> > > >>>>> if (mode_opt) {
> > > >>>>> @@ -415,7 +415,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > >>>>> grvga_modedb, sizeof(grvga_modedb), &grvga_modedb[0], 8);
> > > >>>>> if (!retval || retval = 4) {
> > > >>>>> retval = -EINVAL;
> > > >>>>> - goto err3;
> > > >>>>> + goto dealloc_cmap;
> > > >>>>> }
> > > >>>>> }
> > > >>>>>
> > > >>>>> @@ -430,7 +430,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > >>>>> if (!request_mem_region(physical_start, grvga_mem_size, dev->name)) {
> > > >>>>> dev_err(&dev->dev, "failed to request memory region\n");
> > > >>>>> retval = -ENOMEM;
> > > >>>>> - goto err3;
> > > >>>>> + goto dealloc_cmap;
> > > >>>>> }
> > > >>>>>
> > > >>>>> virtual_start = (unsigned long) ioremap(physical_start, grvga_mem_size);
> > > >>>>> @@ -438,7 +438,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > >>>>> if (!virtual_start) {
> > > >>>>> dev_err(&dev->dev, "error mapping framebuffer memory\n");
> > > >>>>> retval = -ENOMEM;
> > > >>>>> - goto err4;
> > > >>>>> + goto free_mem;
> > > >>>>> }
> > > >>>>> } else { /* Allocate frambuffer memory */
> > > >>>>>
> > > >>>>> @@ -451,7 +451,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > >>>>> "unable to allocate framebuffer memory (%lu bytes)\n",
> > > >>>>> grvga_mem_size);
> > > >>>>> retval = -ENOMEM;
> > > >>>>> - goto err3;
> > > >>>>> + goto dealloc_cmap;
> > > >>>>> }
> > > >>>>>
> > > >>>>> physical_start = dma_map_single(&dev->dev, (void *)virtual_start, grvga_mem_size, DMA_TO_DEVICE);
> > > >>>>> @@ -484,7 +484,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > >>>>> retval = register_framebuffer(info);
> > > >>>>> if (retval < 0) {
> > > >>>>> dev_err(&dev->dev, "failed to register framebuffer\n");
> > > >>>>> - goto err4;
> > > >>>>> + goto free_mem;
> > > >>>>> }
> > > >>>>>
> > > >>>>> __raw_writel(physical_start, &par->regs->fb_pos);
> > > >>>>> @@ -493,21 +493,23 @@ static int __devinit grvga_probe(struct platform_device *dev)
> > > >>>>>
> > > >>>>> return 0;
> > > >>>>>
> > > >>>>> -err4:
> > > >>>>> +free_mem:
> > > >>>>> dev_set_drvdata(&dev->dev, NULL);
> > > >>>>> if (grvga_fix_addr) {
> > > >>>>> release_mem_region(physical_start, grvga_mem_size);
> > > >>>>> - iounmap((void *)virtual_start);
> > > >>>>> + if (virtual_start)
> > > >>>>> + iounmap((void *)virtual_start);
> > > >>>>> } else
> > > >>>>> kfree((void *)virtual_start);
> > > >>>>> -err3:
> > > >>>>> +
> > > >>>>> +dealloc_cmap:
> > > >>>>> fb_dealloc_cmap(&info->cmap);
> > > >>>>> -err2:
> > > >>>>> +unmap_regs:
> > > >>>>> of_iounmap(&dev->resource[0], par->regs,
> > > >>>>> resource_size(&dev->resource[0]));
> > > >>>>> -err1:
> > > >>>>> +release_regs:
> > > >>>>> release_mem_region(dev->resource[0].start, resource_size(&dev->resource[0]));
> > > >>>>> -err:
> > > >>>>> +free_fb:
> > > >>>>> framebuffer_release(info);
> > > >>>>>
> > > >>>>> return retval;
> > > >>>>> --
> > > >>>>> 1.7.10
> > > >>>>>
> > > >>>>> --
> > > >>>>> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> > > >>>>> the body of a message to majordomo@vger.kernel.org
> > > >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > >>>>>
> > > >>>
> > > >>>
> > > >>> --
> > > >>> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> > > >>> the body of a message to majordomo@vger.kernel.org
> > > >>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > >>>
> > > >
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > >
> >
> >
> >
^ permalink raw reply
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
From: Russell King - ARM Linux @ 2012-06-25 11:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1340604478.12683.25.camel@lappyti>
On Mon, Jun 25, 2012 at 09:07:58AM +0300, Tomi Valkeinen wrote:
> On Fri, 2012-06-22 at 19:18 +0530, Rajendra Nayak wrote:
> > In preparation of OMAP moving to Common Clk Framework(CCF) add clk_prepare()
> > and clk_unprepare() for the omapdss clocks.
>
> You used clk_prepare and clk_unprepare instead of clk_prepare_enable and
> clk_disable_unprepare. I didn't check the dss driver yet, but my hunch
> is that the clocks are normally not enabled/disabled from atomic
> context.
>
> What does the prepare/unprepare actually do? Is there any benefit in
> delaying preparing, i.e. is there a difference between prepare right
> after clk_get, or prepare right before clk_enable? (And similarly for
> unprepare)
I think you're looking at this the wrong way.
Think of clk_prepare() as the _sleepable_ part of clk_enable(). So, think
of clk_prepare() as ideally being placed just before clk_enable() with the
exception that if that's not possible (because you're in a non-atomic
context) it should be placed in a position as close to clk_enable() as
possible which is atomic.
Don't think of it as an extra step to be done after clk_get() but as an
extra step needing to be done before clk_enable().
^ permalink raw reply
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
From: Rajendra Nayak @ 2012-06-25 11:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1340611133.3395.3.camel@deskari>
On Monday 25 June 2012 01:28 PM, Tomi Valkeinen wrote:
> On Mon, 2012-06-25 at 12:29 +0530, Rajendra Nayak wrote:
>> On Monday 25 June 2012 11:37 AM, Tomi Valkeinen wrote:
>>> Hi,
>>>
>>> On Fri, 2012-06-22 at 19:18 +0530, Rajendra Nayak wrote:
>>>> In preparation of OMAP moving to Common Clk Framework(CCF) add clk_prepare()
>>>> and clk_unprepare() for the omapdss clocks.
>>>
>>> You used clk_prepare and clk_unprepare instead of clk_prepare_enable and
>>> clk_disable_unprepare. I didn't check the dss driver yet, but my hunch
>>> is that the clocks are normally not enabled/disabled from atomic
>>> context.
>>>
>>> What does the prepare/unprepare actually do? Is there any benefit in
>>> delaying preparing, i.e. is there a difference between prepare right
>>> after clk_get, or prepare right before clk_enable? (And similarly for
>>> unprepare)
>>
>> clk_prepare/unprepare are useful for clocks which need the 'enable'
>> logic to be implemented as a slow part (which can sleep) and a fast part
>> (which does not sleep). For all the dss clocks in question we don't need
>> a slow part and hence they do not have a .clk_prepare/unprepare
>> platform hook.
>>
>> The framework however still does prepare usecounting (it has a prepare
>> count and an enable count, and prepare count is expected to be non-zero
>> while the clock is being enabled) and uses a mutex around to guard it.
>> So while the dss driver would do multiple clk_enable/disable while its
>> active, it seems fair to just prepare/unprepare the clocks once just
>> after clk_get() and before clk_put() in this particular case.
>
> But the driver should not presume anything special about the clocks. In
> this case the dss driver would presume that the clocks it uses do not
> have prepare and unprepare hooks.
>
> If the generally proper way to use prepare/unprepare is in combination
> of enable/disable, then I think we should try to do that.
makes sense. Lets see if any of the clk_enable/disable happen in atomic
context, if not it would be just a matter of replacing all with a
clk_prepare_enable/disable_unprepare. Else we might have to find a safe
place sometime before clk_enable to prepare the clk and after
clk_disable to unprepare it.
>
> I'll check if any of the dss clocks are enabled or disabled in atomic
> context.
>
> Tomi
>
^ permalink raw reply
* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Grazvydas Ignotas @ 2012-06-25 12:05 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: jaswinder.singh, mythripk, linux-omap, linux-fbdev, andy.green,
n-dechesne
In-Reply-To: <1340605221.12683.30.camel@lappyti>
On Mon, Jun 25, 2012 at 9:20 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Sat, 2012-06-23 at 13:36 +0530, jaswinder.singh@linaro.org wrote:
>>
>> Currenlty HDMI fails to come up in the suspend-resume path.
>> This patch helps that real-world scenario.
>
> What is the problem there? It'd be good to explain the problem in the
> patch description. Does the pm_runtime_get return -EACCES?
On slightly different but related issue, currently OMAPDSS always
spits lots of backtraces when it's compiled without CONFIG_PM_RUNTIME,
because pm_runtime_put* always return -ENOSYS without
CONFIG_PM_RUNTIME. So something like this patch proposes is needed, or
maybe WARN_ON should check for -ENOSYS, I don't know..
--
Gražvydas
^ permalink raw reply
* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Tomi Valkeinen @ 2012-06-25 12:30 UTC (permalink / raw)
To: Grazvydas Ignotas
Cc: jaswinder.singh, mythripk, linux-omap, linux-fbdev, andy.green,
n-dechesne
In-Reply-To: <CANOLnOOodXtqV7Hs-SsEUfB3Uo1PSGb8bkaeN7M48jVpQrb-cA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]
On Mon, 2012-06-25 at 15:05 +0300, Grazvydas Ignotas wrote:
> On Mon, Jun 25, 2012 at 9:20 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Sat, 2012-06-23 at 13:36 +0530, jaswinder.singh@linaro.org wrote:
> >>
> >> Currenlty HDMI fails to come up in the suspend-resume path.
> >> This patch helps that real-world scenario.
> >
> > What is the problem there? It'd be good to explain the problem in the
> > patch description. Does the pm_runtime_get return -EACCES?
>
> On slightly different but related issue, currently OMAPDSS always
> spits lots of backtraces when it's compiled without CONFIG_PM_RUNTIME,
> because pm_runtime_put* always return -ENOSYS without
> CONFIG_PM_RUNTIME. So something like this patch proposes is needed, or
> maybe WARN_ON should check for -ENOSYS, I don't know..
Hmm. I guess I'm missing some understanding about runtime PM. omapdss
uses runtime PM to enable the underlying DSS hardware. If there's no
runtime PM, how does the driver work? Or is it the job of
hwmod/omap_device to keep all the hardware always enabled if runtime PM
is not compiled in?
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Jassi Brar @ 2012-06-25 12:39 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <1340616643.3395.19.camel@deskari>
On 25 June 2012 15:00, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Mon, 2012-06-25 at 14:19 +0530, Jassi Brar wrote:
>> On 25 June 2012 11:50, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> > On Sat, 2012-06-23 at 13:36 +0530, jaswinder.singh@linaro.org wrote:
>> ....
>> >> Currenlty HDMI fails to come up in the suspend-resume path.
>> >> This patch helps that real-world scenario.
>> >
>> > What is the problem there? It'd be good to explain the problem in the
>> > patch description. Does the pm_runtime_get return -EACCES?
>> >
>> Yes, it returns -EACCESS because RPM on devices is disabled during the
>> period from suspend-start to resume-finished.
>
> So... You didn't answer my first comment, how can the code work?
>
Sorry, don't know why I thought I didn't miss anything.
> The driver needs to enable the HW and the call to pm_runtime_get() is
> skipped. Won't this lead to crash as the DSS registers are accessed
> without the HW in enabled state?
>
Hmm... how does the extant code in hdmi driver ensures DSS is up and running ?
While it does sound important even to my limited knowledge of OMAPDSS,
I see rpm of HDMI, VENC and RFBI only dependent on DISPC, not DSS.
And for DISPC these drivers already hold a reference in their display
enable/resume and keep it until disable/suspend. So we don't lose
DISPC anytime it is really required.
> And what happens if the pm_runtime_get() call is skipped, but pm_runtime_put() is not?
>
Not sure in what newly introduced scenario by this patch, because
get/put both check for pm_enabled before proceeding. Am I overlooking
something?
thnx
^ permalink raw reply
* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Tomi Valkeinen @ 2012-06-25 12:41 UTC (permalink / raw)
To: Jassi Brar; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <CAJe_ZhftuNggxH+269-ZJbWJo5nSzSjo1dDCTQGMbsjPWoUpyg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2444 bytes --]
On Mon, 2012-06-25 at 17:57 +0530, Jassi Brar wrote:
> On 25 June 2012 15:00, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > The driver needs to enable the HW and the call to pm_runtime_get() is
> > skipped. Won't this lead to crash as the DSS registers are accessed
> > without the HW in enabled state?
> >
> Hmm... how does the extant code in hdmi driver ensures DSS is up and running ?
> While it does sound important even to my limited knowledge of OMAPDSS,
> I see rpm of HDMI, VENC and RFBI only dependent on DISPC, not DSS.
DSS device is parent to all the DSS subdevices. So when a subdevice is
enabled, DSS device is enabled first.
But anyway, I wasn't referring to the DSS part of OMAPDSS, but to
omapdss generally. If we do this:
/* this is skipped, if runtime PM is disabled */
dispc_runtime_get();
/* this accesses a register, but the HW is disabled? */
dispc_read_reg(FOO);
So again, I don't understand how the underlying HW gets enabled. Or does
hwmod/omap_device code make sure that it's enabled while the board is
being resumed? If so, what would happen if we continue the above
scenario as follows:
/* this is skipped, if runtime PM is disabled */
dispc_runtime_get();
/* this accesses a register, the HW is kept enabled by hwmod */
dispc_read_reg(FOO);
/* at some time later the resume procedure ends, and hwmod doesn't keep
the HW enabled any more */
/* this accesses a register, the HW is disabled */
dispc_read_reg(FOO);
> And for DISPC these drivers already hold a reference in their display
> enable/resume and keep it until disable/suspend. So we don't lose
> DISPC anytime it is really required.
If all the displays are disabled, nobody keeps a reference to dispc.
> > And what happens if the pm_runtime_get() call is skipped, but pm_runtime_put() is not?
> >
> Not sure in what newly introduced scenario by this patch, because
> get/put both check for pm_enabled before proceeding. Am I overlooking
> something?
Currently (for example) dispc_runtime_get/put call
pm_runtime_get/put_sync. When somebody uses dispc_runtime_get, the same
somebody knows it needs to call dispc_runtime_put later.
Now, what happens if dispc_runtime_get is called when runtime PM is
disabled (i.e. pm_runtime_get_sync is skipped), but runtime PM is
enabled later when that somebody calls dispc_runtime_put (i.e.
pm_runtime_put_sync is _not_ skipped)?
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Jassi Brar @ 2012-06-25 12:45 UTC (permalink / raw)
To: Grazvydas Ignotas
Cc: Tomi Valkeinen, mythripk, linux-omap, linux-fbdev, andy.green,
n-dechesne
In-Reply-To: <CANOLnOOodXtqV7Hs-SsEUfB3Uo1PSGb8bkaeN7M48jVpQrb-cA@mail.gmail.com>
On 25 June 2012 17:35, Grazvydas Ignotas <notasas@gmail.com> wrote:
> On Mon, Jun 25, 2012 at 9:20 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> On Sat, 2012-06-23 at 13:36 +0530, jaswinder.singh@linaro.org wrote:
>>>
>>> Currenlty HDMI fails to come up in the suspend-resume path.
>>> This patch helps that real-world scenario.
>>
>> What is the problem there? It'd be good to explain the problem in the
>> patch description. Does the pm_runtime_get return -EACCES?
>
> On slightly different but related issue, currently OMAPDSS always
> spits lots of backtraces when it's compiled without CONFIG_PM_RUNTIME,
> because pm_runtime_put* always return -ENOSYS without
> CONFIG_PM_RUNTIME. So something like this patch proposes is needed, or
> maybe WARN_ON should check for -ENOSYS, I don't know..
>
I didn't check, but this patch should already fix that I think ?
IMHO, for omapdss, we need not differentiate between -ENOSYS and
-EACCESS because anyway the ultimate functions dispc_runtime_resume()
and dispc_runtime_suspend() can't report failure (they always return
0).
-j
^ permalink raw reply
* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Tomi Valkeinen @ 2012-06-25 12:50 UTC (permalink / raw)
To: Rajendra Nayak
Cc: Grazvydas Ignotas, jaswinder.singh, mythripk, linux-omap,
linux-fbdev, andy.green, n-dechesne
In-Reply-To: <4FE85CCA.80903@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2379 bytes --]
On Mon, 2012-06-25 at 18:12 +0530, Rajendra Nayak wrote:
> On Monday 25 June 2012 06:00 PM, Tomi Valkeinen wrote:
> > On Mon, 2012-06-25 at 15:05 +0300, Grazvydas Ignotas wrote:
> >> On Mon, Jun 25, 2012 at 9:20 AM, Tomi Valkeinen<tomi.valkeinen@ti.com> wrote:
> >>> On Sat, 2012-06-23 at 13:36 +0530, jaswinder.singh@linaro.org wrote:
> >>>>
> >>>> Currenlty HDMI fails to come up in the suspend-resume path.
> >>>> This patch helps that real-world scenario.
> >>>
> >>> What is the problem there? It'd be good to explain the problem in the
> >>> patch description. Does the pm_runtime_get return -EACCES?
> >>
> >> On slightly different but related issue, currently OMAPDSS always
> >> spits lots of backtraces when it's compiled without CONFIG_PM_RUNTIME,
> >> because pm_runtime_put* always return -ENOSYS without
> >> CONFIG_PM_RUNTIME. So something like this patch proposes is needed, or
> >> maybe WARN_ON should check for -ENOSYS, I don't know..
> >
> > Hmm. I guess I'm missing some understanding about runtime PM. omapdss
> > uses runtime PM to enable the underlying DSS hardware. If there's no
> > runtime PM, how does the driver work? Or is it the job of
> > hwmod/omap_device to keep all the hardware always enabled if runtime PM
> > is not compiled in?
>
> Yes, the below trick keeps all hwmods always enabled post the initial
> setup if runtime PM is disabled.
>
> from arch/arm/mach-omap2/io.c
>
> static void __init omap_hwmod_init_postsetup(void)
> {
> u8 postsetup_state;
>
> /* Set the default postsetup state for all hwmods */
> #ifdef CONFIG_PM_RUNTIME
> postsetup_state = _HWMOD_STATE_IDLE;
> #else
> postsetup_state = _HWMOD_STATE_ENABLED;
> #endif
> omap_hwmod_for_each(_set_hwmod_postsetup_state, &postsetup_state);
>
> omap_pm_if_early_init();
> }
Ah, ok, thanks.
Do you know how the drivers should handle CONFIG_PM_RUNTIME=n?
Are they supposed to handle the error values returned by runtime PM
functions somehow, or should they use #ifdef CONFIG_PM_RUNTIME?
Both options sound a bit difficult to me... With the first one it's
difficult to see if there was an actual error and we should somehow
react to it, or is everything fine and we just shouldn't care about
runtime PM. The second one requires ifdefs in many places.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Rajendra Nayak @ 2012-06-25 12:54 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Grazvydas Ignotas, jaswinder.singh, mythripk, linux-omap,
linux-fbdev, andy.green, n-dechesne
In-Reply-To: <1340627459.3395.54.camel@deskari>
On Monday 25 June 2012 06:00 PM, Tomi Valkeinen wrote:
> On Mon, 2012-06-25 at 15:05 +0300, Grazvydas Ignotas wrote:
>> On Mon, Jun 25, 2012 at 9:20 AM, Tomi Valkeinen<tomi.valkeinen@ti.com> wrote:
>>> On Sat, 2012-06-23 at 13:36 +0530, jaswinder.singh@linaro.org wrote:
>>>>
>>>> Currenlty HDMI fails to come up in the suspend-resume path.
>>>> This patch helps that real-world scenario.
>>>
>>> What is the problem there? It'd be good to explain the problem in the
>>> patch description. Does the pm_runtime_get return -EACCES?
>>
>> On slightly different but related issue, currently OMAPDSS always
>> spits lots of backtraces when it's compiled without CONFIG_PM_RUNTIME,
>> because pm_runtime_put* always return -ENOSYS without
>> CONFIG_PM_RUNTIME. So something like this patch proposes is needed, or
>> maybe WARN_ON should check for -ENOSYS, I don't know..
>
> Hmm. I guess I'm missing some understanding about runtime PM. omapdss
> uses runtime PM to enable the underlying DSS hardware. If there's no
> runtime PM, how does the driver work? Or is it the job of
> hwmod/omap_device to keep all the hardware always enabled if runtime PM
> is not compiled in?
Yes, the below trick keeps all hwmods always enabled post the initial
setup if runtime PM is disabled.
from arch/arm/mach-omap2/io.c
static void __init omap_hwmod_init_postsetup(void)
{
u8 postsetup_state;
/* Set the default postsetup state for all hwmods */
#ifdef CONFIG_PM_RUNTIME
postsetup_state = _HWMOD_STATE_IDLE;
#else
postsetup_state = _HWMOD_STATE_ENABLED;
#endif
omap_hwmod_for_each(_set_hwmod_postsetup_state, &postsetup_state);
omap_pm_if_early_init();
}
>
> Tomi
>
^ permalink raw reply
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
From: Tomi Valkeinen @ 2012-06-25 13:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4FE85005.4090303@ti.com>
[-- Attachment #1: Type: text/plain, Size: 3644 bytes --]
On Mon, 2012-06-25 at 17:18 +0530, Rajendra Nayak wrote:
> On Monday 25 June 2012 01:28 PM, Tomi Valkeinen wrote:
> > On Mon, 2012-06-25 at 12:29 +0530, Rajendra Nayak wrote:
> >> On Monday 25 June 2012 11:37 AM, Tomi Valkeinen wrote:
> >>> Hi,
> >>>
> >>> On Fri, 2012-06-22 at 19:18 +0530, Rajendra Nayak wrote:
> >>>> In preparation of OMAP moving to Common Clk Framework(CCF) add clk_prepare()
> >>>> and clk_unprepare() for the omapdss clocks.
> >>>
> >>> You used clk_prepare and clk_unprepare instead of clk_prepare_enable and
> >>> clk_disable_unprepare. I didn't check the dss driver yet, but my hunch
> >>> is that the clocks are normally not enabled/disabled from atomic
> >>> context.
> >>>
> >>> What does the prepare/unprepare actually do? Is there any benefit in
> >>> delaying preparing, i.e. is there a difference between prepare right
> >>> after clk_get, or prepare right before clk_enable? (And similarly for
> >>> unprepare)
> >>
> >> clk_prepare/unprepare are useful for clocks which need the 'enable'
> >> logic to be implemented as a slow part (which can sleep) and a fast part
> >> (which does not sleep). For all the dss clocks in question we don't need
> >> a slow part and hence they do not have a .clk_prepare/unprepare
> >> platform hook.
> >>
> >> The framework however still does prepare usecounting (it has a prepare
> >> count and an enable count, and prepare count is expected to be non-zero
> >> while the clock is being enabled) and uses a mutex around to guard it.
> >> So while the dss driver would do multiple clk_enable/disable while its
> >> active, it seems fair to just prepare/unprepare the clocks once just
> >> after clk_get() and before clk_put() in this particular case.
> >
> > But the driver should not presume anything special about the clocks. In
> > this case the dss driver would presume that the clocks it uses do not
> > have prepare and unprepare hooks.
> >
> > If the generally proper way to use prepare/unprepare is in combination
> > of enable/disable, then I think we should try to do that.
>
> makes sense. Lets see if any of the clk_enable/disable happen in atomic
> context, if not it would be just a matter of replacing all with a
> clk_prepare_enable/disable_unprepare. Else we might have to find a safe
> place sometime before clk_enable to prepare the clk and after
> clk_disable to unprepare it.
>
> >
> > I'll check if any of the dss clocks are enabled or disabled in atomic
> > context.
venc and hdmi use clk_enable/disable in runtime PM callbacks (suspend &
resume). If I understand correctly, the callbacks are not called in
atomic context if pm_runtime_irq_safe() has not been used. And it is not
used in omapdss.
dsi uses clk_enable/disable in a different manner, but not in atomic
context.
So as far as I see, clocks are never handled in atomic context. Is
everything related to the base clk stuff already in mainline? Can I take
the clk_prepare/unprepare patch into my omapdss tree?
A question about clk_prepare/unprepare, not directly related: let's say
I have a driver for some HW block. The driver doesn't use clk functions,
but uses runtime PM. The driver also sets pm_runtime_irq_safe().
Now, the driver can call pm_runtime_get_sync() in an atomic context, and
this would lead to the underlying framework (hwmod, omap_device, I don't
know who =) enabling the func clock for that HW. But this would happen
in atomic context, so the underlying framework can't use clk_prepare.
How does the underlying framework handle that case? (sorry if that's a
stupid question =).
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Jassi Brar @ 2012-06-25 13:43 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <1340628094.3395.63.camel@deskari>
On 25 June 2012 18:11, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Mon, 2012-06-25 at 17:57 +0530, Jassi Brar wrote:
>> On 25 June 2012 15:00, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
>> > The driver needs to enable the HW and the call to pm_runtime_get() is
>> > skipped. Won't this lead to crash as the DSS registers are accessed
>> > without the HW in enabled state?
>> >
>> Hmm... how does the extant code in hdmi driver ensures DSS is up and running ?
>> While it does sound important even to my limited knowledge of OMAPDSS,
>> I see rpm of HDMI, VENC and RFBI only dependent on DISPC, not DSS.
>
> DSS device is parent to all the DSS subdevices. So when a subdevice is
> enabled, DSS device is enabled first.
>
> But anyway, I wasn't referring to the DSS part of OMAPDSS, but to
> omapdss generally. If we do this:
>
> /* this is skipped, if runtime PM is disabled */
> dispc_runtime_get();
>
I hope you do realize that there is difference between "PM is disabled
on a device"
and "the device is in some low-power state". pm_runtime_enabled()
checks for the former.
So under this light...
> /* this accesses a register, but the HW is disabled? */
> dispc_read_reg(FOO);
>
.... the H/W is already always enabled if CONFIG_PM_RUNTIME is not defined.
If CONFIG_PM_RUNTIME is indeed defined, pm_runtime_enabled() will
always return true after pm_runtime_enable() unless someone disables
it explicitly - omapdss or the RPM stack(during suspend/resume).
OMAPDSS never does so in the lifetime of a driver. So the only period
in which pm_runtime_enabled() returns false, is when the platform is
suspending, suspended or resuming.
>> > And what happens if the pm_runtime_get() call is skipped, but pm_runtime_put() is not?
>> >
>> Not sure in what newly introduced scenario by this patch, because
>> get/put both check for pm_enabled before proceeding. Am I overlooking
>> something?
>
> Currently (for example) dispc_runtime_get/put call
> pm_runtime_get/put_sync. When somebody uses dispc_runtime_get, the same
> somebody knows it needs to call dispc_runtime_put later.
>
> Now, what happens if dispc_runtime_get is called when runtime PM is
> disabled (i.e. pm_runtime_get_sync is skipped), but runtime PM is
> enabled later when that somebody calls dispc_runtime_put (i.e.
> pm_runtime_put_sync is _not_ skipped)?
>
As I said, for omapdss, PM is disabled (not device deactivated) only
during rpm suspend/resume.
And it should be no different than any lock protected section
preempted by suspend-resume before reaching its end.
^ permalink raw reply
* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Tomi Valkeinen @ 2012-06-25 13:49 UTC (permalink / raw)
To: Jassi Brar; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <CAJe_ZhetdEwrgDEfV9rLHU1_fmb3O+954hCKcoENBgQ6Bi6BXQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3936 bytes --]
On Mon, 2012-06-25 at 19:01 +0530, Jassi Brar wrote:
> On 25 June 2012 18:11, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Mon, 2012-06-25 at 17:57 +0530, Jassi Brar wrote:
> >> On 25 June 2012 15:00, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >
> >> > The driver needs to enable the HW and the call to pm_runtime_get() is
> >> > skipped. Won't this lead to crash as the DSS registers are accessed
> >> > without the HW in enabled state?
> >> >
> >> Hmm... how does the extant code in hdmi driver ensures DSS is up and running ?
> >> While it does sound important even to my limited knowledge of OMAPDSS,
> >> I see rpm of HDMI, VENC and RFBI only dependent on DISPC, not DSS.
> >
> > DSS device is parent to all the DSS subdevices. So when a subdevice is
> > enabled, DSS device is enabled first.
> >
> > But anyway, I wasn't referring to the DSS part of OMAPDSS, but to
> > omapdss generally. If we do this:
> >
> > /* this is skipped, if runtime PM is disabled */
> > dispc_runtime_get();
> >
> I hope you do realize that there is difference between "PM is disabled
> on a device"
> and "the device is in some low-power state". pm_runtime_enabled()
> checks for the former.
> So under this light...
>
> > /* this accesses a register, but the HW is disabled? */
> > dispc_read_reg(FOO);
> >
> .... the H/W is already always enabled if CONFIG_PM_RUNTIME is not defined.
>
> If CONFIG_PM_RUNTIME is indeed defined, pm_runtime_enabled() will
> always return true after pm_runtime_enable() unless someone disables
> it explicitly - omapdss or the RPM stack(during suspend/resume).
> OMAPDSS never does so in the lifetime of a driver. So the only period
> in which pm_runtime_enabled() returns false, is when the platform is
> suspending, suspended or resuming.
Right. So what happens in my example above?
Normally if the driver does dispc_runtime_get() and dispc_read_reg(),
the first call will enable the HW so the reg read works.
But if the pm_runtime is disabled, say, during system suspend, with your
patch dispc_runtime_get() will just return 0 without doing anything, and
the dispc_read_reg() will crash because the HW is disabled (because
nobody enabled it).
Perhaps I'm missing something, but I don't quite understand how this
works.
> >> > And what happens if the pm_runtime_get() call is skipped, but pm_runtime_put() is not?
> >> >
> >> Not sure in what newly introduced scenario by this patch, because
> >> get/put both check for pm_enabled before proceeding. Am I overlooking
> >> something?
> >
> > Currently (for example) dispc_runtime_get/put call
> > pm_runtime_get/put_sync. When somebody uses dispc_runtime_get, the same
> > somebody knows it needs to call dispc_runtime_put later.
> >
> > Now, what happens if dispc_runtime_get is called when runtime PM is
> > disabled (i.e. pm_runtime_get_sync is skipped), but runtime PM is
> > enabled later when that somebody calls dispc_runtime_put (i.e.
> > pm_runtime_put_sync is _not_ skipped)?
> >
> As I said, for omapdss, PM is disabled (not device deactivated) only
> during rpm suspend/resume.
> And it should be no different than any lock protected section
> preempted by suspend-resume before reaching its end.
I'm not sure if I understand... If the driver does dispc_runtime_get()
while the PM is disabled, say, during system resume, dispc_runtime_get()
will do nothing and return 0. The driver thinks it succeeded, and will
call dispc_runtime_put() later.
Calling the dispc_runtime_put() could happen very soon, while runtime PM
is still disabled, in which case everything works fine. But there's no
rule to say dispc_runtime_put() has to be called very soon after
dispc_runtime_get(). The driver might as well call put later, when
runtime PM is enabled.
This would end up with a pm_runtime_put call without a matching
pm_runtime_get call.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Jassi Brar @ 2012-06-25 17:18 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <1340632161.3395.100.camel@deskari>
On 25 June 2012 19:19, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Mon, 2012-06-25 at 19:01 +0530, Jassi Brar wrote:
>> > /* this accesses a register, but the HW is disabled? */
>> > dispc_read_reg(FOO);
>> >
>> .... the H/W is already always enabled if CONFIG_PM_RUNTIME is not defined.
>>
>> If CONFIG_PM_RUNTIME is indeed defined, pm_runtime_enabled() will
>> always return true after pm_runtime_enable() unless someone disables
>> it explicitly - omapdss or the RPM stack(during suspend/resume).
>> OMAPDSS never does so in the lifetime of a driver. So the only period
>> in which pm_runtime_enabled() returns false, is when the platform is
>> suspending, suspended or resuming.
>
> Right. So what happens in my example above?
>
> Normally if the driver does dispc_runtime_get() and dispc_read_reg(),
> the first call will enable the HW so the reg read works.
>
> But if the pm_runtime is disabled, say, during system suspend, with your
> patch dispc_runtime_get() will just return 0 without doing anything, and
> the dispc_read_reg() will crash because the HW is disabled (because
> nobody enabled it).
>
Hmm, I am not sure if new calls would/should be made to dispc.c after
the system has suspended and before resumed. That is, anything other
than from runtime_resume/suspend callbacks of DSS, DISPC, HDMI, VENC
and RFBI, which rightly don't touch any dss reg but only
enable/disable a clock.
As we know, a subsystem should make sure any active work is cleared
out before suspending and set some flag so that nothing runs until it
has resumed. I don't say we can't crash the system with this patch,
but then we would be violating rules of suspend-resume.
>>
>> As I said, for omapdss, PM is disabled (not device deactivated) only
>> during rpm suspend/resume.
>> And it should be no different than any lock protected section
>> preempted by suspend-resume before reaching its end.
>
> I'm not sure if I understand... If the driver does dispc_runtime_get()
> while the PM is disabled, say, during system resume, dispc_runtime_get()
> will do nothing and return 0. The driver thinks it succeeded, and will
> call dispc_runtime_put() later.
>
> Calling the dispc_runtime_put() could happen very soon, while runtime PM
> is still disabled, in which case everything works fine. But there's no
> rule to say dispc_runtime_put() has to be called very soon after
> dispc_runtime_get(). The driver might as well call put later, when
> runtime PM is enabled.
>
> This would end up with a pm_runtime_put call without a matching
> pm_runtime_get call.
>
Again, we need to see if there is really some situation where
something new is attempted before the subsystem has resumed. If there
is indeed, maybe omapdss need to flag it's suspended and prevent such
thing until it has resumed.
Btw, even without this patch, when dispc_runtime_get() does return
error under rpm disabled, we disturb the dev.power.usage_count balance
by not calling dispc_runtime_put()
This patch doesn't make everything perfect, but only improve upon the
current situation.
thnx
^ permalink raw reply
* [PATCH v2] grvga: Fix error handling issues
From: Emil Goode @ 2012-06-25 22:37 UTC (permalink / raw)
To: FlorianSchandinat; +Cc: linux-fbdev, linux-kernel, kernel-janitors, Emil Goode
This patch fixes two problems with the error handling in the
grvga_probe function and simplifies it making the code
easier to read.
- If the call to grvga_parse_custom on line 370 fails we use
the wrong label so that release_mem_region will be called
without a call to request_mem_region being made.
- If the call to ioremap on line 436 fails we should not try
to call iounmap in the error handling code.
This patch introduces the following changes:
- Converts request_mem_region into its devm_ equivalent
which simplifies the otherwise messy clean up code.
- Changes the labels for correct error handling and their
names to make the code easier to read.
Signed-off-by: Emil Goode <emilgoode@gmail.com>
---
v2: Simplifies the error handling by converting to
devm_request_mem_region
drivers/video/grvga.c | 47 ++++++++++++++++++++++-------------------------
1 file changed, 22 insertions(+), 25 deletions(-)
diff --git a/drivers/video/grvga.c b/drivers/video/grvga.c
index da066c2..5245f9a 100644
--- a/drivers/video/grvga.c
+++ b/drivers/video/grvga.c
@@ -354,7 +354,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
*/
if (fb_get_options("grvga", &options)) {
retval = -ENODEV;
- goto err;
+ goto free_fb;
}
if (!options || !*options)
@@ -370,7 +370,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
if (grvga_parse_custom(this_opt, &info->var) < 0) {
dev_err(&dev->dev, "Failed to parse custom mode (%s).\n", this_opt);
retval = -EINVAL;
- goto err1;
+ goto free_fb;
}
} else if (!strncmp(this_opt, "addr", 4))
grvga_fix_addr = simple_strtoul(this_opt + 5, NULL, 16);
@@ -387,10 +387,11 @@ static int __devinit grvga_probe(struct platform_device *dev)
info->flags = FBINFO_DEFAULT | FBINFO_PARTIAL_PAN_OK | FBINFO_HWACCEL_YPAN;
info->fix.smem_len = grvga_mem_size;
- if (!request_mem_region(dev->resource[0].start, resource_size(&dev->resource[0]), "grlib-svgactrl regs")) {
+ if (!devm_request_mem_region(&dev->dev, dev->resource[0].start,
+ resource_size(&dev->resource[0]), "grlib-svgactrl regs")) {
dev_err(&dev->dev, "registers already mapped\n");
retval = -EBUSY;
- goto err;
+ goto free_fb;
}
par->regs = of_ioremap(&dev->resource[0], 0,
@@ -400,14 +401,14 @@ static int __devinit grvga_probe(struct platform_device *dev)
if (!par->regs) {
dev_err(&dev->dev, "failed to map registers\n");
retval = -ENOMEM;
- goto err1;
+ goto free_fb;
}
retval = fb_alloc_cmap(&info->cmap, 256, 0);
if (retval < 0) {
dev_err(&dev->dev, "failed to allocate mem with fb_alloc_cmap\n");
retval = -ENOMEM;
- goto err2;
+ goto unmap_regs;
}
if (mode_opt) {
@@ -415,7 +416,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
grvga_modedb, sizeof(grvga_modedb), &grvga_modedb[0], 8);
if (!retval || retval = 4) {
retval = -EINVAL;
- goto err3;
+ goto dealloc_cmap;
}
}
@@ -427,10 +428,11 @@ static int __devinit grvga_probe(struct platform_device *dev)
physical_start = grvga_fix_addr;
- if (!request_mem_region(physical_start, grvga_mem_size, dev->name)) {
+ if (!devm_request_mem_region(&dev->dev, physical_start,
+ grvga_mem_size, dev->name)) {
dev_err(&dev->dev, "failed to request memory region\n");
retval = -ENOMEM;
- goto err3;
+ goto dealloc_cmap;
}
virtual_start = (unsigned long) ioremap(physical_start, grvga_mem_size);
@@ -438,7 +440,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
if (!virtual_start) {
dev_err(&dev->dev, "error mapping framebuffer memory\n");
retval = -ENOMEM;
- goto err4;
+ goto dealloc_cmap;
}
} else { /* Allocate frambuffer memory */
@@ -451,7 +453,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
"unable to allocate framebuffer memory (%lu bytes)\n",
grvga_mem_size);
retval = -ENOMEM;
- goto err3;
+ goto dealloc_cmap;
}
physical_start = dma_map_single(&dev->dev, (void *)virtual_start, grvga_mem_size, DMA_TO_DEVICE);
@@ -484,7 +486,7 @@ static int __devinit grvga_probe(struct platform_device *dev)
retval = register_framebuffer(info);
if (retval < 0) {
dev_err(&dev->dev, "failed to register framebuffer\n");
- goto err4;
+ goto free_mem;
}
__raw_writel(physical_start, &par->regs->fb_pos);
@@ -493,21 +495,18 @@ static int __devinit grvga_probe(struct platform_device *dev)
return 0;
-err4:
+free_mem:
dev_set_drvdata(&dev->dev, NULL);
- if (grvga_fix_addr) {
- release_mem_region(physical_start, grvga_mem_size);
+ if (grvga_fix_addr)
iounmap((void *)virtual_start);
- } else
+ else
kfree((void *)virtual_start);
-err3:
+dealloc_cmap:
fb_dealloc_cmap(&info->cmap);
-err2:
+unmap_regs:
of_iounmap(&dev->resource[0], par->regs,
resource_size(&dev->resource[0]));
-err1:
- release_mem_region(dev->resource[0].start, resource_size(&dev->resource[0]));
-err:
+free_fb:
framebuffer_release(info);
return retval;
@@ -524,12 +523,10 @@ static int __devexit grvga_remove(struct platform_device *device)
of_iounmap(&device->resource[0], par->regs,
resource_size(&device->resource[0]));
- release_mem_region(device->resource[0].start, resource_size(&device->resource[0]));
- if (!par->fb_alloced) {
- release_mem_region(info->fix.smem_start, info->fix.smem_len);
+ if (!par->fb_alloced)
iounmap(info->screen_base);
- } else
+ else
kfree((void *)info->screen_base);
framebuffer_release(info);
--
1.7.10
^ permalink raw reply related
* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Rajendra Nayak @ 2012-06-26 4:55 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Grazvydas Ignotas, jaswinder.singh, mythripk, linux-omap,
linux-fbdev, andy.green, n-dechesne
In-Reply-To: <1340628648.3395.69.camel@deskari>
On Monday 25 June 2012 06:20 PM, Tomi Valkeinen wrote:
> On Mon, 2012-06-25 at 18:12 +0530, Rajendra Nayak wrote:
>> On Monday 25 June 2012 06:00 PM, Tomi Valkeinen wrote:
>>> On Mon, 2012-06-25 at 15:05 +0300, Grazvydas Ignotas wrote:
>>>> On Mon, Jun 25, 2012 at 9:20 AM, Tomi Valkeinen<tomi.valkeinen@ti.com> wrote:
>>>>> On Sat, 2012-06-23 at 13:36 +0530, jaswinder.singh@linaro.org wrote:
>>>>>>
>>>>>> Currenlty HDMI fails to come up in the suspend-resume path.
>>>>>> This patch helps that real-world scenario.
>>>>>
>>>>> What is the problem there? It'd be good to explain the problem in the
>>>>> patch description. Does the pm_runtime_get return -EACCES?
>>>>
>>>> On slightly different but related issue, currently OMAPDSS always
>>>> spits lots of backtraces when it's compiled without CONFIG_PM_RUNTIME,
>>>> because pm_runtime_put* always return -ENOSYS without
>>>> CONFIG_PM_RUNTIME. So something like this patch proposes is needed, or
>>>> maybe WARN_ON should check for -ENOSYS, I don't know..
>>>
>>> Hmm. I guess I'm missing some understanding about runtime PM. omapdss
>>> uses runtime PM to enable the underlying DSS hardware. If there's no
>>> runtime PM, how does the driver work? Or is it the job of
>>> hwmod/omap_device to keep all the hardware always enabled if runtime PM
>>> is not compiled in?
>>
>> Yes, the below trick keeps all hwmods always enabled post the initial
>> setup if runtime PM is disabled.
>>
>> from arch/arm/mach-omap2/io.c
>>
>> static void __init omap_hwmod_init_postsetup(void)
>> {
>> u8 postsetup_state;
>>
>> /* Set the default postsetup state for all hwmods */
>> #ifdef CONFIG_PM_RUNTIME
>> postsetup_state = _HWMOD_STATE_IDLE;
>> #else
>> postsetup_state = _HWMOD_STATE_ENABLED;
>> #endif
>> omap_hwmod_for_each(_set_hwmod_postsetup_state,&postsetup_state);
>>
>> omap_pm_if_early_init();
>> }
>
> Ah, ok, thanks.
>
> Do you know how the drivers should handle CONFIG_PM_RUNTIME=n?
> Are they supposed to handle the error values returned by runtime PM
> functions somehow, or should they use #ifdef CONFIG_PM_RUNTIME?
hmm, I always though with CONFIG_RUNTIME_PM=n, the functions would
be stubbed to return success and not failure. And the _pm_runtime_resume
function indeed seems to return 1, which is not failure but just saying
that your device is already active/enabled.
The _pm_runtime_suspend and _pm_runtime_idle do return a -ENOSYS, which
is something only returned when CONFIG_RUNTIME_PM=n, so if you really
want to handle failing pm_runtime_put_sync cases, maybe you still can.
But then, I don't know if there is anything you can do to recover from
a failing pm_runtime_put_sync, except for warning the user maybe.
>
> Both options sound a bit difficult to me... With the first one it's
> difficult to see if there was an actual error and we should somehow
> react to it, or is everything fine and we just shouldn't care about
> runtime PM. The second one requires ifdefs in many places.
>
> Tomi
>
^ permalink raw reply
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
From: Rajendra Nayak @ 2012-06-26 5:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1340630090.3395.85.camel@deskari>
On Monday 25 June 2012 06:44 PM, Tomi Valkeinen wrote:
> venc and hdmi use clk_enable/disable in runtime PM callbacks (suspend&
> resume). If I understand correctly, the callbacks are not called in
> atomic context if pm_runtime_irq_safe() has not been used. And it is not
> used in omapdss.
>
> dsi uses clk_enable/disable in a different manner, but not in atomic
> context.
>
> So as far as I see, clocks are never handled in atomic context. Is
> everything related to the base clk stuff already in mainline? Can I take
> the clk_prepare/unprepare patch into my omapdss tree?
Well the Common Clk framework is already in mainline, but we still don;t
have CONFIG_COMMON_CLK enabled for our builds yet. So until we do so,
clk_prepare/unprepare will just be stubs which do nothing.
I will repost the patch getting rid of the clk_prepare/unprepare and
adding clk_prepare_enable/disable_unprepare instead.
>
>
> A question about clk_prepare/unprepare, not directly related: let's say
> I have a driver for some HW block. The driver doesn't use clk functions,
> but uses runtime PM. The driver also sets pm_runtime_irq_safe().
>
> Now, the driver can call pm_runtime_get_sync() in an atomic context, and
> this would lead to the underlying framework (hwmod, omap_device, I don't
> know who =) enabling the func clock for that HW. But this would happen
> in atomic context, so the underlying framework can't use clk_prepare.
>
> How does the underlying framework handle that case? (sorry if that's a
> stupid question =).
No, its not a stupid question at all. I have been thinking about this
too to figure out whats the best way to handle this. For now, the
patches I posted, do an early clk_prepare (like I did for dss) for all
hwmod clocks as I have no way to know which ones will have their
_enable, _idle etc called in atomic context. Maybe I should see if there
is a way I can do so only for those devices which end up calling a
pm_runtime_irq_safe() and not do it early for all.
>
^ permalink raw reply
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
From: Tomi Valkeinen @ 2012-06-26 6:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4FE941EA.7050108@ti.com>
[-- Attachment #1: Type: text/plain, Size: 788 bytes --]
On Tue, 2012-06-26 at 10:30 +0530, Rajendra Nayak wrote:
> > So as far as I see, clocks are never handled in atomic context. Is
> > everything related to the base clk stuff already in mainline? Can I take
> > the clk_prepare/unprepare patch into my omapdss tree?
>
> Well the Common Clk framework is already in mainline, but we still don;t
> have CONFIG_COMMON_CLK enabled for our builds yet. So until we do so,
> clk_prepare/unprepare will just be stubs which do nothing.
But if I understood correctly, clk_prepare and clk_unprepare are anyway
no-ops with dss clocks, even when CONFIG_COMMON_CLK is enabled?
My point was only to understand if I can safely take the patch into
omapdss tree, instead of it going through l-o, to avoid any possible
conflicts.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Tomi Valkeinen @ 2012-06-26 7:19 UTC (permalink / raw)
To: Jassi Brar; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <CAJe_ZheHoCikVzy6zJoKHuL8_oQ0ZnPASUUWimvbPRr+AZ4zWg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2510 bytes --]
On Mon, 2012-06-25 at 22:36 +0530, Jassi Brar wrote:
> On 25 June 2012 19:19, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Mon, 2012-06-25 at 19:01 +0530, Jassi Brar wrote:
>
> >> > /* this accesses a register, but the HW is disabled? */
> >> > dispc_read_reg(FOO);
> >> >
> >> .... the H/W is already always enabled if CONFIG_PM_RUNTIME is not defined.
> >>
> >> If CONFIG_PM_RUNTIME is indeed defined, pm_runtime_enabled() will
> >> always return true after pm_runtime_enable() unless someone disables
> >> it explicitly - omapdss or the RPM stack(during suspend/resume).
> >> OMAPDSS never does so in the lifetime of a driver. So the only period
> >> in which pm_runtime_enabled() returns false, is when the platform is
> >> suspending, suspended or resuming.
> >
> > Right. So what happens in my example above?
> >
> > Normally if the driver does dispc_runtime_get() and dispc_read_reg(),
> > the first call will enable the HW so the reg read works.
> >
> > But if the pm_runtime is disabled, say, during system suspend, with your
> > patch dispc_runtime_get() will just return 0 without doing anything, and
> > the dispc_read_reg() will crash because the HW is disabled (because
> > nobody enabled it).
> >
> Hmm, I am not sure if new calls would/should be made to dispc.c after
> the system has suspended and before resumed. That is, anything other
> than from runtime_resume/suspend callbacks of DSS, DISPC, HDMI, VENC
> and RFBI, which rightly don't touch any dss reg but only
> enable/disable a clock.
They do touch the registers. For example, dispc's callbacks save and
restore the registers. The HW should be fully functional during the
callbacks. The point of the callbacks is to suspend/resume the HW in
question, which of course requires accessing the HW.
> As we know, a subsystem should make sure any active work is cleared
> out before suspending and set some flag so that nothing runs until it
> has resumed. I don't say we can't crash the system with this patch,
> but then we would be violating rules of suspend-resume.
Let's go back a bit. I feel like I'm missing some pieces of information,
as I still don't quite grasp the problem.
In the patch you said this fixes an issue with HDMI. Can you tell more
about the problem? What code path is being run? Any error messages?
I tried system suspend with omap4-sdp and panda, with 3.5-rc2, but
neither board seems to wake up from the suspend. Does it work for you?
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
From: Rajendra Nayak @ 2012-06-26 7:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1340693758.2093.15.camel@lappyti>
On Tuesday 26 June 2012 12:25 PM, Tomi Valkeinen wrote:
> On Tue, 2012-06-26 at 10:30 +0530, Rajendra Nayak wrote:
>
>>> So as far as I see, clocks are never handled in atomic context. Is
>>> everything related to the base clk stuff already in mainline? Can I take
>>> the clk_prepare/unprepare patch into my omapdss tree?
>>
>> Well the Common Clk framework is already in mainline, but we still don;t
>> have CONFIG_COMMON_CLK enabled for our builds yet. So until we do so,
>> clk_prepare/unprepare will just be stubs which do nothing.
>
> But if I understood correctly, clk_prepare and clk_unprepare are anyway
> no-ops with dss clocks, even when CONFIG_COMMON_CLK is enabled?
With CONFIG_COMMON_CLK enabled, they will do prepare use-counting with
a mutex lock/unlock around it.
>
> My point was only to understand if I can safely take the patch into
> omapdss tree, instead of it going through l-o, to avoid any possible
> conflicts.
yes, I don't see any issues with it going through dss tree.
>
> Tomi
>
^ permalink raw reply
* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Andy Green @ 2012-06-26 8:40 UTC (permalink / raw)
To: Jassi Brar; +Cc: Tomi Valkeinen, mythripk, linux-omap, linux-fbdev, n-dechesne
In-Reply-To: <CAJe_Zhev3V85K1jhj+W+d+0kv132MZ=-7rChhCLgtWO7WN4u1w@mail.gmail.com>
On 06/26/12 16:32, the mail apparently from Jassi Brar included:
> On 26 June 2012 12:49, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> On Mon, 2012-06-25 at 22:36 +0530, Jassi Brar wrote:
>
>>>>
>>>> Normally if the driver does dispc_runtime_get() and dispc_read_reg(),
>>>> the first call will enable the HW so the reg read works.
>>>>
>>>> But if the pm_runtime is disabled, say, during system suspend, with your
>>>> patch dispc_runtime_get() will just return 0 without doing anything, and
>>>> the dispc_read_reg() will crash because the HW is disabled (because
>>>> nobody enabled it).
>>>>
>>> Hmm, I am not sure if new calls would/should be made to dispc.c after
>>> the system has suspended and before resumed. That is, anything other
>>> than from runtime_resume/suspend callbacks of DSS, DISPC, HDMI, VENC
>>> and RFBI, which rightly don't touch any dss reg but only
>>> enable/disable a clock.
>>
>> They do touch the registers. For example, dispc's callbacks save and
>> restore the registers. The HW should be fully functional during the
>> callbacks. The point of the callbacks is to suspend/resume the HW in
>> question, which of course requires accessing the HW.
>>
> DISPC being held by HDMI, VENC and RFBI would be the last to suspend
> and first to resume. And it won't have its registers touched between
> dispc_runtime_suspend() and dispc_runtime_resume(), which seems ok to
> me (?)
> HDMI, VENC and RFBI directly fooling around with DISPC regs would have
> been a problem, which isn't the case.
>
>>> As we know, a subsystem should make sure any active work is cleared
>>> out before suspending and set some flag so that nothing runs until it
>>> has resumed. I don't say we can't crash the system with this patch,
>>> but then we would be violating rules of suspend-resume.
>>
>> Let's go back a bit. I feel like I'm missing some pieces of information,
>> as I still don't quite grasp the problem.
>>
>> In the patch you said this fixes an issue with HDMI. Can you tell more
>> about the problem? What code path is being run? Any error messages?
>>
>> I tried system suspend with omap4-sdp and panda, with 3.5-rc2, but
>> neither board seems to wake up from the suspend. Does it work for you?
>>
> Something non-omapdss in vanilla breaks suspend/resume.
> Without this patch I see the upstream's display broken after the
> suspend attempt.
> $ echo mem > /sys/power/state
>
> I work on TILT tree, which has suspend/resume working after some more
> local patches.
> http://git.linaro.org/gitweb?p=people/andygreen/kernel-tilt.git;a=shortlog;h=refs/heads/tilt-3.4
>
> I don't have SDP so not sure, but it should simply be testable with
> Panda4460 and the omap4plus_defconfig there. Please feel free to ask
> if you have any issue checking that out.
We don't have access to Blaze and don't test that tree against it, but
it's worth trying on PandaBoard ES which we do have and test against
(omap4plus_defconfig).
Here, mem suspend is working with HDMI raster coming back on resume, but
we don't always get a desktop redraw (suspending again can "correct"
that). Jassi's patches are present in this tree.
A slightly side-issue, I have a TV here that only issues hpd 700ms after
the Panda provides 5V at the HDMI link. It has always been touch-and-go
if dss will recognize it or not, compared to a monitor which issues hpd
high within some us of the link being powered.
The patches from Jassi about permanently enabling the external HDMI PHY
chip section that performs level-conversion for hpd, and the existing
work to use irq management of hpd, seems to have really solved detecting
that TV for the first time.
-Andy
--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106 -
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog
^ permalink raw reply
* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Jassi Brar @ 2012-06-26 8:44 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <1340695166.2093.22.camel@lappyti>
On 26 June 2012 12:49, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Mon, 2012-06-25 at 22:36 +0530, Jassi Brar wrote:
>> >
>> > Normally if the driver does dispc_runtime_get() and dispc_read_reg(),
>> > the first call will enable the HW so the reg read works.
>> >
>> > But if the pm_runtime is disabled, say, during system suspend, with your
>> > patch dispc_runtime_get() will just return 0 without doing anything, and
>> > the dispc_read_reg() will crash because the HW is disabled (because
>> > nobody enabled it).
>> >
>> Hmm, I am not sure if new calls would/should be made to dispc.c after
>> the system has suspended and before resumed. That is, anything other
>> than from runtime_resume/suspend callbacks of DSS, DISPC, HDMI, VENC
>> and RFBI, which rightly don't touch any dss reg but only
>> enable/disable a clock.
>
> They do touch the registers. For example, dispc's callbacks save and
> restore the registers. The HW should be fully functional during the
> callbacks. The point of the callbacks is to suspend/resume the HW in
> question, which of course requires accessing the HW.
>
DISPC being held by HDMI, VENC and RFBI would be the last to suspend
and first to resume. And it won't have its registers touched between
dispc_runtime_suspend() and dispc_runtime_resume(), which seems ok to
me (?)
HDMI, VENC and RFBI directly fooling around with DISPC regs would have
been a problem, which isn't the case.
>> As we know, a subsystem should make sure any active work is cleared
>> out before suspending and set some flag so that nothing runs until it
>> has resumed. I don't say we can't crash the system with this patch,
>> but then we would be violating rules of suspend-resume.
>
> Let's go back a bit. I feel like I'm missing some pieces of information,
> as I still don't quite grasp the problem.
>
> In the patch you said this fixes an issue with HDMI. Can you tell more
> about the problem? What code path is being run? Any error messages?
>
> I tried system suspend with omap4-sdp and panda, with 3.5-rc2, but
> neither board seems to wake up from the suspend. Does it work for you?
>
Something non-omapdss in vanilla breaks suspend/resume.
Without this patch I see the upstream's display broken after the
suspend attempt.
$ echo mem > /sys/power/state
I work on TILT tree, which has suspend/resume working after some more
local patches.
http://git.linaro.org/gitweb?p=people/andygreen/kernel-tilt.git;a=shortlog;h=refs/heads/tilt-3.4
I don't have SDP so not sure, but it should simply be testable with
Panda4460 and the omap4plus_defconfig there. Please feel free to ask
if you have any issue checking that out.
Thanks.
^ permalink raw reply
* [PATCH 0/2] simple device tree support for mxsfb
From: Shawn Guo @ 2012-06-26 9:02 UTC (permalink / raw)
To: linux-arm-kernel
It adds a simple device tree support for mxsfb driver.
Shawn Guo (2):
video: mxsfb: move mxsfb.h into include/linux
video: mxsfb: add simple device tree probe
Documentation/devicetree/bindings/fb/mxsfb.txt | 21 +++++++
arch/arm/mach-mxs/devices-mx23.h | 2 +-
arch/arm/mach-mxs/devices-mx28.h | 2 +-
arch/arm/mach-mxs/devices/platform-mxsfb.c | 2 +-
drivers/video/mxsfb.c | 62 +++++++++++++++-----
.../include/mach => include/linux}/mxsfb.h | 6 +-
6 files changed, 75 insertions(+), 20 deletions(-)
create mode 100644 Documentation/devicetree/bindings/fb/mxsfb.txt
rename {arch/arm/mach-mxs/include/mach => include/linux}/mxsfb.h (95%)
--
1.7.5.4
^ permalink raw reply
* [PATCH 1/2] video: mxsfb: move mxsfb.h into include/linux
From: Shawn Guo @ 2012-06-26 9:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1340701379-8619-1-git-send-email-shawn.guo@linaro.org>
Move mxsfb.h into include/linux, so that mxsfb driver does not have to
include <mach/*> header.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
arch/arm/mach-mxs/devices-mx23.h | 2 +-
arch/arm/mach-mxs/devices-mx28.h | 2 +-
arch/arm/mach-mxs/devices/platform-mxsfb.c | 2 +-
drivers/video/mxsfb.c | 2 +-
.../include/mach => include/linux}/mxsfb.h | 6 +++---
5 files changed, 7 insertions(+), 7 deletions(-)
rename {arch/arm/mach-mxs/include/mach => include/linux}/mxsfb.h (95%)
diff --git a/arch/arm/mach-mxs/devices-mx23.h b/arch/arm/mach-mxs/devices-mx23.h
index 9acdd63..9ee5ced 100644
--- a/arch/arm/mach-mxs/devices-mx23.h
+++ b/arch/arm/mach-mxs/devices-mx23.h
@@ -10,7 +10,7 @@
*/
#include <mach/mx23.h>
#include <mach/devices-common.h>
-#include <mach/mxsfb.h>
+#include <linux/mxsfb.h>
#include <linux/amba/bus.h>
static inline int mx23_add_duart(void)
diff --git a/arch/arm/mach-mxs/devices-mx28.h b/arch/arm/mach-mxs/devices-mx28.h
index 84b2960..fcab431 100644
--- a/arch/arm/mach-mxs/devices-mx28.h
+++ b/arch/arm/mach-mxs/devices-mx28.h
@@ -10,7 +10,7 @@
*/
#include <mach/mx28.h>
#include <mach/devices-common.h>
-#include <mach/mxsfb.h>
+#include <linux/mxsfb.h>
#include <linux/amba/bus.h>
static inline int mx28_add_duart(void)
diff --git a/arch/arm/mach-mxs/devices/platform-mxsfb.c b/arch/arm/mach-mxs/devices/platform-mxsfb.c
index 5a75b71..76b53f73 100644
--- a/arch/arm/mach-mxs/devices/platform-mxsfb.c
+++ b/arch/arm/mach-mxs/devices/platform-mxsfb.c
@@ -10,7 +10,7 @@
#include <mach/mx23.h>
#include <mach/mx28.h>
#include <mach/devices-common.h>
-#include <mach/mxsfb.h>
+#include <linux/mxsfb.h>
#ifdef CONFIG_SOC_IMX23
struct platform_device *__init mx23_add_mxsfb(
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
index abbe691..08dad8d 100644
--- a/drivers/video/mxsfb.c
+++ b/drivers/video/mxsfb.c
@@ -46,7 +46,7 @@
#include <linux/dma-mapping.h>
#include <linux/io.h>
#include <linux/pinctrl/consumer.h>
-#include <mach/mxsfb.h>
+#include <linux/mxsfb.h>
#define REG_SET 4
#define REG_CLR 8
diff --git a/arch/arm/mach-mxs/include/mach/mxsfb.h b/include/linux/mxsfb.h
similarity index 95%
rename from arch/arm/mach-mxs/include/mach/mxsfb.h
rename to include/linux/mxsfb.h
index e4d7979..f14943d 100644
--- a/arch/arm/mach-mxs/include/mach/mxsfb.h
+++ b/include/linux/mxsfb.h
@@ -14,8 +14,8 @@
* MA 02110-1301, USA.
*/
-#ifndef __MACH_FB_H
-#define __MACH_FB_H
+#ifndef __LINUX_MXSFB_H
+#define __LINUX_MXSFB_H
#include <linux/fb.h>
@@ -46,4 +46,4 @@ struct mxsfb_platform_data {
*/
};
-#endif /* __MACH_FB_H */
+#endif /* __LINUX_MXSFB_H */
--
1.7.5.4
^ permalink raw reply related
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