* [PATCH] OMAPDSS: HDMI: Discard phy_tx_enabled member
From: jaswinder.singh @ 2012-06-23 8:19 UTC (permalink / raw)
To: tomi.valkeinen, mythripk
Cc: linux-omap, linux-fbdev, andy.green, n-dechesne, Jassi Brar
In-Reply-To: <1339797701-11540-1-git-send-email-jaswinder.singh@linaro.org>
From: Jassi Brar <jaswinder.singh@linaro.org>
It is simpler to read the current status from a register as compared
to maintaining a state variable to hold the information.
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
drivers/video/omap2/dss/ti_hdmi.h | 1 -
drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 11 ++++-------
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h
index e734cb4..d174ca1 100644
--- a/drivers/video/omap2/dss/ti_hdmi.h
+++ b/drivers/video/omap2/dss/ti_hdmi.h
@@ -177,7 +177,6 @@ struct hdmi_ip_data {
/* ti_hdmi_4xxx_ip private data. These should be in a separate struct */
int hpd_gpio;
- bool phy_tx_enabled;
};
int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data);
void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data);
diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
index 4dae1b2..3fa3d98 100644
--- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
+++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
@@ -157,6 +157,10 @@ static int hdmi_pll_init(struct hdmi_ip_data *ip_data)
/* PHY_PWR_CMD */
static int hdmi_set_phy_pwr(struct hdmi_ip_data *ip_data, enum hdmi_phy_pwr val)
{
+ /* Return if already the state */
+ if (REG_GET(hdmi_wp_base(ip_data), HDMI_WP_PWR_CTRL, 5, 4) = val)
+ return 0;
+
/* Command for power control of HDMI PHY */
REG_FLD_MOD(hdmi_wp_base(ip_data), HDMI_WP_PWR_CTRL, val, 7, 6);
@@ -241,11 +245,6 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data)
hpd = gpio_get_value(ip_data->hpd_gpio);
- if (hpd = ip_data->phy_tx_enabled) {
- spin_unlock_irqrestore(&phy_tx_lock, flags);
- return 0;
- }
-
if (hpd)
r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON);
else
@@ -257,7 +256,6 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data)
goto err;
}
- ip_data->phy_tx_enabled = hpd;
err:
spin_unlock_irqrestore(&phy_tx_lock, flags);
return r;
@@ -327,7 +325,6 @@ void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data)
free_irq(gpio_to_irq(ip_data->hpd_gpio), ip_data);
hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_OFF);
- ip_data->phy_tx_enabled = false;
}
static int hdmi_core_ddc_init(struct hdmi_ip_data *ip_data)
--
1.7.4.1
^ permalink raw reply related
* [PATCH] OMAPDSS: HDMI: Replace spinlock with mutex in hdmi_check_hpd_state
From: jaswinder.singh @ 2012-06-23 8:19 UTC (permalink / raw)
To: tomi.valkeinen, mythripk
Cc: linux-omap, linux-fbdev, andy.green, n-dechesne, Jassi Brar
From: Jassi Brar <jaswinder.singh@linaro.org>
State change of HDMI PHY could potentially take many millisecs, we can do
better by protecting things in hdmi_set_phy_pwr() with a mutex rather than
a spin_lock_irqsave.
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
drivers/video/omap2/dss/hdmi.c | 1 +
drivers/video/omap2/dss/ti_hdmi.h | 1 +
drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 6 ++----
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 8195c71..0738090 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -758,6 +758,7 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
hdmi.ip_data.core_av_offset = HDMI_CORE_AV;
hdmi.ip_data.pll_offset = HDMI_PLLCTRL;
hdmi.ip_data.phy_offset = HDMI_PHY;
+ mutex_init(&hdmi.ip_data.lock);
hdmi_panel_init();
diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h
index d174ca1..cc292b8 100644
--- a/drivers/video/omap2/dss/ti_hdmi.h
+++ b/drivers/video/omap2/dss/ti_hdmi.h
@@ -177,6 +177,7 @@ struct hdmi_ip_data {
/* ti_hdmi_4xxx_ip private data. These should be in a separate struct */
int hpd_gpio;
+ struct mutex lock;
};
int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data);
void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data);
diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
index 3fa3d98..04acca9 100644
--- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
+++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
@@ -238,10 +238,8 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data)
unsigned long flags;
bool hpd;
int r;
- /* this should be in ti_hdmi_4xxx_ip private data */
- static DEFINE_SPINLOCK(phy_tx_lock);
- spin_lock_irqsave(&phy_tx_lock, flags);
+ mutex_lock(&ip_data->lock);
hpd = gpio_get_value(ip_data->hpd_gpio);
@@ -257,7 +255,7 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data)
}
err:
- spin_unlock_irqrestore(&phy_tx_lock, flags);
+ mutex_unlock(&ip_data->lock);
return r;
}
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH] grvga: Fix error handling issues
From: Emil Goode @ 2012-06-23 22:01 UTC (permalink / raw)
To: Julia Lawall
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <alpine.DEB.2.02.1206221820340.1901@hadrien>
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
> >
^ permalink raw reply
* Re: [PATCH] grvga: Fix error handling issues
From: Dan Carpenter @ 2012-06-23 22:33 UTC (permalink / raw)
To: Emil Goode; +Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <1340381479-6371-1-git-send-email-emilgoode@gmail.com>
On Fri, Jun 22, 2012 at 06:11:19PM +0200, 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.
>
Doesn't iounmap() have a check for NULL? On x86 it does.
if ((void __force *)addr <= high_memory)
return;
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] grvga: Fix error handling issues
From: Julia Lawall @ 2012-06-24 5:15 UTC (permalink / raw)
To: Emil Goode
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors,
w.sang
In-Reply-To: <1340488911.5321.10.camel@localhost>
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] grvga: Fix error handling issues
From: Julia Lawall @ 2012-06-24 5:18 UTC (permalink / raw)
To: Julia Lawall
Cc: Emil Goode, FlorianSchandinat, linux-fbdev, linux-kernel,
kernel-janitors, w.sang
In-Reply-To: <alpine.DEB.2.02.1206240711580.2178@localhost6.localdomain6>
On Sun, 24 Jun 2012, 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.
Oops, drop the last comment; the function uses both.
julia
>
> 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
* boot hang on CONFIG_FB_VGA16
From: Fengguang Wu @ 2012-06-24 5:21 UTC (permalink / raw)
To: Antonino A. Daplas; +Cc: Florian Tobias Schandinat, linux-fbdev, linux-kernel
Hi,
I can reproduce this easily when running CONFIG_FB_VGA16=y kernels
inside KVM. Older kernels like 3.2, 3.3, 3.4 don't have these warning
messages, but just randomly hang during boot.
I'm not sure if this is a relevant problem, but anyway, here is the
bug report.
INFO: rcu_sched self-detected stall on CPU { 1} (t\x15003 jiffies)
Pid: 8, comm: kworker/1:0 Not tainted 3.5.0-rc3-wl+ #11
Call Trace:
<IRQ> [<ffffffff810a1e35>] __rcu_pending+0xc2/0x3c3
[<ffffffff8107a1bc>] ? tick_nohz_handler+0xd5/0xd5
[<ffffffff810a3052>] rcu_check_callbacks+0x9d/0x149
[<ffffffff81061b01>] ? sched_clock_cpu+0xbb/0xcc
[<ffffffff8103db27>] update_process_times+0x3e/0x72
[<ffffffff8107a228>] tick_sched_timer+0x6c/0x95
[<ffffffff81061b21>] ? local_clock+0xf/0x3c
[<ffffffff810537d3>] __run_hrtimer+0xcf/0x1d1
[<ffffffff810541c6>] hrtimer_interrupt+0xc6/0x191
[<ffffffff817bc52a>] smp_apic_timer_interrupt+0x86/0x99
[<ffffffff817bb51c>] apic_timer_interrupt+0x6c/0x80
<EOI> [<ffffffff81030a8c>] ? console_unlock+0x21b/0x2c5
[<ffffffff813304a1>] ? update_attr.isra.1+0x8a/0x8a
[<ffffffff8132b7a7>] fb_flashcursor+0x114/0x123
[<ffffffff81048529>] process_one_work+0x238/0x45c
[<ffffffff8104848c>] ? process_one_work+0x19b/0x45c
[<ffffffff8132b693>] ? get_color+0xfd/0xfd
[<ffffffff8104876e>] process_scheduled_works+0x21/0x2f
[<ffffffff810488f5>] worker_thread+0x179/0x240
Thanks,
Fengguang
^ permalink raw reply
* Re: [PATCH] grvga: Fix error handling issues
From: Emil Goode @ 2012-06-24 9:42 UTC (permalink / raw)
To: Dan Carpenter
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <20120623223319.GG5333@mwanda>
This code is for the sparc architecture and the iounmap doesn't check
for NULL directly as in the x86 version. I think it is safer not to pass
NULL to the sparc version.
Thanks,
Emil
On Sun, 2012-06-24 at 01:33 +0300, Dan Carpenter wrote:
> On Fri, Jun 22, 2012 at 06:11:19PM +0200, 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.
> >
>
> Doesn't iounmap() have a check for NULL? On x86 it does.
>
> if ((void __force *)addr <= high_memory)
> return;
>
> regards,
> dan carpenter
>
>
^ permalink raw reply
* Re: [PATCH] grvga: Fix error handling issues
From: Emil Goode @ 2012-06-24 10:36 UTC (permalink / raw)
To: Julia Lawall
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors,
w.sang
In-Reply-To: <alpine.DEB.2.02.1206240711580.2178@localhost6.localdomain6>
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 :)
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] video: backlight: remove unused header
From: Florian Tobias Schandinat @ 2012-06-24 20:59 UTC (permalink / raw)
To: Paul Bolle; +Cc: linux-fbdev, linux-kernel
In-Reply-To: <1339573651.30984.130.camel@x61.thuisdomein>
On 06/13/2012 07:47 AM, Paul Bolle wrote:
> Commit 9befe40f6e018e508b047eb76d189ede9b4ff03d ("video: backlight:
> support s6e8ax0 panel driver based on MIPI DSI") added s6e8ax0.h, but
> no file includes it. That's probably a good thing, because it declares
> an extern void function that is defined static int in s6e8ax0.c.
> Besides, that function is also wrapped in the module_init() macro, which
> should do everything needed to make that function available to the code
> outside of s6e8ax0.c. This header can safely be removed.
>
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
Applied.
Thanks,
Florian Tobias Schandinat
> ---
> 0) Tested mainly by using various git tools on the (history of the)
> tree.
>
> 1) Shouldn't s6e8ax0_init() and s6e8ax0_exit(), both in s6e8ax0.c, carry
> the usual __init and __exit attributes?
>
> 2) But note that all the module related code in s6e8ax0.c seems moot
> currently: EXYNOS_LCD_S6E8AX0 is a boolean Kconfig symbol, so the code
> can only be used builtin. So, as far as I can tell, either that symbol
> (and the symbols on which it depends) should be made tristate, or the
> module related code can be removed from s6e8ax0.c.
>
> drivers/video/exynos/s6e8ax0.h | 21 ---------------------
> 1 files changed, 0 insertions(+), 21 deletions(-)
> delete mode 100644 drivers/video/exynos/s6e8ax0.h
>
> diff --git a/drivers/video/exynos/s6e8ax0.h b/drivers/video/exynos/s6e8ax0.h
> deleted file mode 100644
> index 1f1b270..0000000
> --- a/drivers/video/exynos/s6e8ax0.h
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -/* linux/drivers/video/backlight/s6e8ax0.h
> - *
> - * MIPI-DSI based s6e8ax0 AMOLED LCD Panel definitions.
> - *
> - * Copyright (c) 2011 Samsung Electronics
> - *
> - * Inki Dae, <inki.dae@samsung.com>
> - * Donghwa Lee <dh09.lee@samsung.com>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> -*/
> -
> -#ifndef _S6E8AX0_H
> -#define _S6E8AX0_H
> -
> -extern void s6e8ax0_init(void);
> -
> -#endif
> -
^ permalink raw reply
* Re: [PATCH v2] video: exynos_dp: fix build warning due to uninitialized value
From: Florian Tobias Schandinat @ 2012-06-24 21:00 UTC (permalink / raw)
To: Jingoo Han; +Cc: linux-fbdev, linux-kernel, 'Olof Johansson'
In-Reply-To: <000301cd4e83$9f5fcb30$de1f6190$%han@samsung.com>
On 06/20/2012 01:25 AM, Jingoo Han wrote:
> This patch fixes build warning due to uninitialized value dereference.
>
> drivers/video/exynos/exynos_dp_core.c: In function 'exynos_dp_set_link_train':
> drivers/video/exynos/exynos_dp_core.c:529:18: warning: 'reg' may be used uninitialized in this function [-Wuninitialized]
> drivers/video/exynos/exynos_dp_core.c:395:6: note: 'reg' was declared here
>
> Signed-off-by: Olof Johansson <olof@lixom.net>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Applied.
Thanks,
Florian Tobias Schandinat
> ---
> drivers/video/exynos/exynos_dp_core.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index a36b2d2..9db7b9f 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -407,6 +407,9 @@ static unsigned int exynos_dp_get_lane_link_training(
> case 3:
> reg = exynos_dp_get_lane3_link_training(dp);
> break;
> + default:
> + WARN_ON(1);
> + return 0;
> }
>
> return reg;
^ permalink raw reply
* Re: [PATCH] video: exynos_dp: remove duplicated declarations from header file
From: Florian Tobias Schandinat @ 2012-06-24 21:00 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <000601cd4f94$415cd750$c41685f0$%han@samsung.com>
On 06/21/2012 09:57 AM, Jingoo Han wrote:
> Some functions are declared twice in header file; thus, these
> declarations are unnecessary.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Applied.
Thanks,
Florian Tobias Schandinat
> ---
> drivers/video/exynos/exynos_dp_core.h | 4 ----
> 1 files changed, 0 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
> index 1e0f998..8526e54 100644
> --- a/drivers/video/exynos/exynos_dp_core.h
> +++ b/drivers/video/exynos/exynos_dp_core.h
> @@ -85,10 +85,6 @@ void exynos_dp_set_link_bandwidth(struct exynos_dp_device *dp, u32 bwtype);
> void exynos_dp_get_link_bandwidth(struct exynos_dp_device *dp, u32 *bwtype);
> void exynos_dp_set_lane_count(struct exynos_dp_device *dp, u32 count);
> void exynos_dp_get_lane_count(struct exynos_dp_device *dp, u32 *count);
> -void exynos_dp_set_link_bandwidth(struct exynos_dp_device *dp, u32 bwtype);
> -void exynos_dp_get_link_bandwidth(struct exynos_dp_device *dp, u32 *bwtype);
> -void exynos_dp_set_lane_count(struct exynos_dp_device *dp, u32 count);
> -void exynos_dp_get_lane_count(struct exynos_dp_device *dp, u32 *count);
> void exynos_dp_enable_enhanced_mode(struct exynos_dp_device *dp, bool enable);
> void exynos_dp_set_training_pattern(struct exynos_dp_device *dp,
> enum pattern_set pattern);
^ permalink raw reply
* Re: [PULL for v3.6] SH Mobile LCDC fixes and planes support
From: Florian Tobias Schandinat @ 2012-06-24 21:01 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <3246665.HJN7RE5zfe@avalon>
On 06/20/2012 08:31 AM, Laurent Pinchart wrote:
> Hi Florian,
>
> On Wednesday 20 June 2012 01:01:31 Florian Tobias Schandinat wrote:
>> On 06/16/2012 02:07 PM, Laurent Pinchart wrote:
>>> Hi Florian,
>>>
>>> Could you please pull the following LCDC patches ?
>>
>> looks good, except...
>>
>>> The following changes since commit
> b67989515defba7412acff01162e5bb1f0f5923a:
>>> video: s3c-fb: fix possible division by zero in s3c_fb_calc_pixclk
>>>
>>> (2012-06-13 17:34:16 +0000)
>>>
>>> are available in the git repository at:
>>> git://linuxtv.org/pinchartl/fbdev.git planes
>>>
>>> Guennadi Liakhovetski (1):
>>> fbdev: sh_mipi_dsi: fix a section mismatch
>>>
>>> Laurent Pinchart (4):
>>> fbdev: sh_mobile_lcdc: Don't confuse line size with pitch
>>
>> ...for this patch it would have been better if you based your work on a
>> more recent tree of Linus that already contains it instead of asking me
>> to pull it again [I now forwarded my fbdev-next to -rc3]. It's not a big
>> issue as git handles such situations but pushing patches around that are
>> essentially nops should be avoided I think. Well, if you decide to fix
>> it I'd be happy, otherwise I'll pull this in a few days nonetheless.
>
> Sorry about that. I've rebased the branch on top of the latest fbdev-next and
> pushed it to git://linuxtv.org/pinchartl/fbdev.git planes, that no-op patches
> isn't present anymore.
Merged.
Thanks,
Florian Tobias Schandinat
^ permalink raw reply
* Re: [PATCH RESEND] video: w100fb: Reduce sleep mode battery discharge
From: Florian Tobias Schandinat @ 2012-06-24 21:24 UTC (permalink / raw)
To: Paul Parsons
Cc: mreimer, philipp.zabel, linux-kernel, linux-fbdev@vger.kernel.org
In-Reply-To: <20120603103355.3845gmx1@mx072.gmx.net>
Hi Paul,
you should als the relevant mailing list, linux-fbdev (done now)
On 06/03/2012 10:33 AM, Paul Parsons wrote:
> In 2006 and 2007 the handhelds.org kernel w100fb driver was patched to
> reduce sleep mode battery discharge. Unfortunately those two patches
> never migrated to the mainline kernel.
>
> Fortunately the function affected - w100_suspend() - has not changed
> since; thus those patches still apply cleanly.
>
> Applying those patches to linux-3.4-rc3 running on an HP iPAQ hx4700
> reduces the sleep mode battery discharge from approximately 26 mA to
> approximately 11 mA.
>
> This patch combines those two patches into a single unified patch.
>
> Signed-off-by: Paul Parsons <lost.distance@yahoo.com>
> Cc: Matt Reimer <mreimer@sdgsystems.com>
> Cc: Philipp Zabel <philipp.zabel@gmail.com>
Actually it would be good if we had the Signed-offs of the original
authors. Oh well, I think if it can't be helped yours should be sufficient.
> ---
>
> Many thanks to Matt Reimer for suggesting this.
>
> The seeming demise of handhelds.org has removed any formal patch logs.
> However the patches can still be viewed online:
> 2006 patch:
> http://paste.lisp.org/display/31700
> 2007 patch:
> http://osdir.com/ml/handhelds.linux.kernel/2007-02/msg00020.html
>
> And the fully patched handhelds.org kernel is still available, e.g.:
> http://www.linuxtogo.org/~goxboxlive/htcuniversal/Kernel/linux-2.6.21-hh20.tar.bz2
>
> drivers/video/w100fb.c | 12 ++++++++++++
> 1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/w100fb.c b/drivers/video/w100fb.c
> index 90a2e30..2f6b2b8 100644
> --- a/drivers/video/w100fb.c
> +++ b/drivers/video/w100fb.c
> @@ -1567,6 +1567,18 @@ static void w100_suspend(u32 mode)
> val = readl(remapped_regs + mmPLL_CNTL);
> val |= 0x00000004; /* bit2=1 */
> writel(val, remapped_regs + mmPLL_CNTL);
> +
> + writel(0x00000000, remapped_regs + mmLCDD_CNTL1);
> + writel(0x00000000, remapped_regs + mmLCDD_CNTL2);
> + writel(0x00000000, remapped_regs + mmGENLCD_CNTL1);
> + writel(0x00000000, remapped_regs + mmGENLCD_CNTL2);
> + writel(0x00000000, remapped_regs + mmGENLCD_CNTL3);
> +
> + val = readl(remapped_regs + mmMEM_EXT_CNTL);
> + val |= 0xF0000000;
> + val &= ~(0x00000001);
> + writel(val, remapped_regs + mmMEM_EXT_CNTL);
> +
If anyone understands what this change does (like for example disabling
the LCD controller) a comment would be useful.
> writel(0x0000001d, remapped_regs + mmPWRMGT_CNTL);
> }
> }
If nobody comments on this I will apply it in a few days.
Best regards,
Florian Tobias Schandinat
^ permalink raw reply
* Re: [PATCH] grvga: Fix error handling issues
From: Julia Lawall @ 2012-06-25 5:20 UTC (permalink / raw)
To: Emil Goode
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors,
w.sang
In-Reply-To: <1340534180.4696.14.camel@localhost>
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: Tomi Valkeinen @ 2012-06-25 6:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1340372890-10091-6-git-send-email-rnayak@ti.com>
[-- Attachment #1: Type: text/plain, Size: 652 bytes --]
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)
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-25 6:20 UTC (permalink / raw)
To: jaswinder.singh; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <1340438771-25587-1-git-send-email-jaswinder.singh@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 841 bytes --]
On Sat, 2012-06-23 at 13:36 +0530, jaswinder.singh@linaro.org wrote:
> From: Jassi Brar <jaswinder.singh@linaro.org>
>
> If the runtime PM of the device is disabled (for example in resume from
> suspend path), it doesn't make sense to attempt pm_runtime_get/put, esp
> when their return values affect the control flow path.
This looks strange. When the driver does pm_runtime_get() it expects the
HW to be functional and registers accessible. If we just skip the
pm_runtime_get(), how can the code work?
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> ---
>
> 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?
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] OMAPDSS: HDMI: Cache EDID
From: Tomi Valkeinen @ 2012-06-25 6:35 UTC (permalink / raw)
To: jaswinder.singh; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <1340438806-25622-1-git-send-email-jaswinder.singh@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 3801 bytes --]
On Sat, 2012-06-23 at 13:36 +0530, jaswinder.singh@linaro.org wrote:
> From: Jassi Brar <jaswinder.singh@linaro.org>
>
> We can easily keep track of latest EDID from the display and hence avoid
> expensive EDID re-reads over I2C.
> This could also help some cheapo displays that provide EDID reliably only
> immediately after asserting HPD and not later.
> Even with good displays, there is something in OMAPDSS that apparantly
> messes up DDC occasionally while EDID is being read, giving the
> "operation stopped when reading edid" error.
>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> ---
> drivers/video/omap2/dss/hdmi.c | 1 +
> drivers/video/omap2/dss/ti_hdmi.h | 2 ++
> drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 23 ++++++++++++++++++++---
> 3 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> index 900e621..0a8c825 100644
> --- a/drivers/video/omap2/dss/hdmi.c
> +++ b/drivers/video/omap2/dss/hdmi.c
> @@ -764,6 +764,7 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
> hdmi.ip_data.core_av_offset = HDMI_CORE_AV;
> hdmi.ip_data.pll_offset = HDMI_PLLCTRL;
> hdmi.ip_data.phy_offset = HDMI_PHY;
> + hdmi.ip_data.edid_len = 0; /* Invalidate EDID Cache */
> mutex_init(&hdmi.ip_data.lock);
Your HDMI patches seem to depend on each other. Please post them as a
proper patch series, instead of each one separately.
> hdmi_panel_init();
> diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h
> index cc292b8..4735860 100644
> --- a/drivers/video/omap2/dss/ti_hdmi.h
> +++ b/drivers/video/omap2/dss/ti_hdmi.h
> @@ -178,6 +178,8 @@ struct hdmi_ip_data {
> /* ti_hdmi_4xxx_ip private data. These should be in a separate struct */
> int hpd_gpio;
> struct mutex lock;
> + u8 edid_cached[256];
> + unsigned edid_len;
> };
> int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data);
> void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data);
> diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
> index 04acca9..2633ade 100644
> --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
> +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
> @@ -243,10 +243,13 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data)
>
> hpd = gpio_get_value(ip_data->hpd_gpio);
>
> - if (hpd)
> + if (hpd) {
> r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON);
> - else
> + } else {
> + /* Invalidate EDID Cache */
> + ip_data->edid_len = 0;
> r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_LDOON);
> + }
>
> if (r) {
> DSSERR("Failed to %s PHY TX power\n",
> @@ -454,6 +457,11 @@ int ti_hdmi_4xxx_read_edid(struct hdmi_ip_data *ip_data,
> {
> int r, l;
>
> + if (ip_data->edid_len) {
> + memcpy(edid, ip_data->edid_cached, ip_data->edid_len);
> + return ip_data->edid_len;
> + }
> +
> if (len < 128)
> return -EINVAL;
>
> @@ -474,12 +482,21 @@ int ti_hdmi_4xxx_read_edid(struct hdmi_ip_data *ip_data,
> l += 128;
> }
>
> + ip_data->edid_len = l;
> + memcpy(ip_data->edid_cached, edid, l);
> +
> return l;
> }
>
> bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
> {
> - return gpio_get_value(ip_data->hpd_gpio);
> + if (gpio_get_value(ip_data->hpd_gpio))
> + return true;
> +
> + /* Invalidate EDID Cache */
> + ip_data->edid_len = 0;
> +
> + return false;
Why is this needed? The HPD interrupt should handle this already. And if
the HPD interrupt doesn't work for some reason, this won't work either,
as the user can plug and unplug his HDMI monitors a thousand times
between two detect calls.
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-25 7:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1340604478.12683.25.camel@lappyti>
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.
>
> Tom
>
^ permalink raw reply
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
From: Tomi Valkeinen @ 2012-06-25 7:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4FE80C43.6090802@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1970 bytes --]
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.
I'll check if any of the dss clocks are enabled or disabled in atomic
context.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] OMAPDSS: HDMI: Cache EDID
From: Jassi Brar @ 2012-06-25 8:08 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <1340606154.12683.34.camel@lappyti>
On 25 June 2012 12:05, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Sat, 2012-06-23 at 13:36 +0530, jaswinder.singh@linaro.org wrote:
>> From: Jassi Brar <jaswinder.singh@linaro.org>
>>
>> We can easily keep track of latest EDID from the display and hence avoid
>> expensive EDID re-reads over I2C.
>> This could also help some cheapo displays that provide EDID reliably only
>> immediately after asserting HPD and not later.
>> Even with good displays, there is something in OMAPDSS that apparantly
>> messes up DDC occasionally while EDID is being read, giving the
>> "operation stopped when reading edid" error.
>>
>> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
>> ---
>> drivers/video/omap2/dss/hdmi.c | 1 +
>> drivers/video/omap2/dss/ti_hdmi.h | 2 ++
>> drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 23 ++++++++++++++++++++---
>> 3 files changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
>> index 900e621..0a8c825 100644
>> --- a/drivers/video/omap2/dss/hdmi.c
>> +++ b/drivers/video/omap2/dss/hdmi.c
>> @@ -764,6 +764,7 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
>> hdmi.ip_data.core_av_offset = HDMI_CORE_AV;
>> hdmi.ip_data.pll_offset = HDMI_PLLCTRL;
>> hdmi.ip_data.phy_offset = HDMI_PHY;
>> + hdmi.ip_data.edid_len = 0; /* Invalidate EDID Cache */
>> mutex_init(&hdmi.ip_data.lock);
>
> Your HDMI patches seem to depend on each other. Please post them as a
> proper patch series, instead of each one separately.
>
They don't depend functionality wise. Any fix can be accepted
regardless of others.
I deliberately avoided a series, because revision of just one could
require resending 3, otherwise
perfectly OK, patches. I just wanted to limit the noise.
I understand, 'git am' might complain but I think that should be trivial to fix.
I am perfectly OK to resend as a patch series, if you want.
>> hdmi_panel_init();
>> diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h
>> index cc292b8..4735860 100644
>> --- a/drivers/video/omap2/dss/ti_hdmi.h
>> +++ b/drivers/video/omap2/dss/ti_hdmi.h
>> @@ -178,6 +178,8 @@ struct hdmi_ip_data {
>> /* ti_hdmi_4xxx_ip private data. These should be in a separate struct */
>> int hpd_gpio;
>> struct mutex lock;
>> + u8 edid_cached[256];
>> + unsigned edid_len;
>> };
>> int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data);
>> void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data);
>> diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
>> index 04acca9..2633ade 100644
>> --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
>> +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
>> @@ -243,10 +243,13 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data)
>>
>> hpd = gpio_get_value(ip_data->hpd_gpio);
>>
>> - if (hpd)
>> + if (hpd) {
>> r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON);
>> - else
>> + } else {
>> + /* Invalidate EDID Cache */
>> + ip_data->edid_len = 0;
>> r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_LDOON);
>> + }
>>
>> if (r) {
>> DSSERR("Failed to %s PHY TX power\n",
>> @@ -454,6 +457,11 @@ int ti_hdmi_4xxx_read_edid(struct hdmi_ip_data *ip_data,
>> {
>> int r, l;
>>
>> + if (ip_data->edid_len) {
>> + memcpy(edid, ip_data->edid_cached, ip_data->edid_len);
>> + return ip_data->edid_len;
>> + }
>> +
>> if (len < 128)
>> return -EINVAL;
>>
>> @@ -474,12 +482,21 @@ int ti_hdmi_4xxx_read_edid(struct hdmi_ip_data *ip_data,
>> l += 128;
>> }
>>
>> + ip_data->edid_len = l;
>> + memcpy(ip_data->edid_cached, edid, l);
>> +
>> return l;
>> }
>>
>> bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
>> {
>> - return gpio_get_value(ip_data->hpd_gpio);
>> + if (gpio_get_value(ip_data->hpd_gpio))
>> + return true;
>> +
>> + /* Invalidate EDID Cache */
>> + ip_data->edid_len = 0;
>> +
>> + return false;
>
> Why is this needed? The HPD interrupt should handle this already. And if
> the HPD interrupt doesn't work for some reason, this won't work either,
> as the user can plug and unplug his HDMI monitors a thousand times
> between two detect calls.
>
I thought it is almost impossible to unplug->plug cycle the HDMI cable
even twice a second, let alone 1000 times against the 10Hz .detect()
poll :) Or you mean some relay controlled HDMI switching
mechanism?
Anyways, that is not the point of this patch. This patch only aims to
avoid un-ncessary EDID reads while doing its best to make sure we
invalidate EDID 'as soon as possible'.
Thanks.
^ permalink raw reply
* Re: [PATCH] OMAPDSS: HDMI: Cache EDID
From: Tomi Valkeinen @ 2012-06-25 8:11 UTC (permalink / raw)
To: Jassi Brar; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <CAJe_Zhf1RsDtKXOpQ97Am-ByBYUtOgjJYyySGYGL9zHNYB_Ejw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4332 bytes --]
On Mon, 2012-06-25 at 13:26 +0530, Jassi Brar wrote:
> On 25 June 2012 12:05, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Sat, 2012-06-23 at 13:36 +0530, jaswinder.singh@linaro.org wrote:
> >> From: Jassi Brar <jaswinder.singh@linaro.org>
> >>
> >> We can easily keep track of latest EDID from the display and hence avoid
> >> expensive EDID re-reads over I2C.
> >> This could also help some cheapo displays that provide EDID reliably only
> >> immediately after asserting HPD and not later.
> >> Even with good displays, there is something in OMAPDSS that apparantly
> >> messes up DDC occasionally while EDID is being read, giving the
> >> "operation stopped when reading edid" error.
> >>
> >> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> >> ---
> >> drivers/video/omap2/dss/hdmi.c | 1 +
> >> drivers/video/omap2/dss/ti_hdmi.h | 2 ++
> >> drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 23 ++++++++++++++++++++---
> >> 3 files changed, 23 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> >> index 900e621..0a8c825 100644
> >> --- a/drivers/video/omap2/dss/hdmi.c
> >> +++ b/drivers/video/omap2/dss/hdmi.c
> >> @@ -764,6 +764,7 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
> >> hdmi.ip_data.core_av_offset = HDMI_CORE_AV;
> >> hdmi.ip_data.pll_offset = HDMI_PLLCTRL;
> >> hdmi.ip_data.phy_offset = HDMI_PHY;
> >> + hdmi.ip_data.edid_len = 0; /* Invalidate EDID Cache */
> >> mutex_init(&hdmi.ip_data.lock);
> >
> > Your HDMI patches seem to depend on each other. Please post them as a
> > proper patch series, instead of each one separately.
> >
> They don't depend functionality wise. Any fix can be accepted
> regardless of others.
> I deliberately avoided a series, because revision of just one could
> require resending 3, otherwise
> perfectly OK, patches. I just wanted to limit the noise.
You don't need to send the whole series, you can just send a revised
patch as a reply to the older version of that patch. (see --in-reply-to
of git send-email).
Of course if you end up changing many of the patches, or one patch lots
of times, it is good to send the whole series at some point later when
the patches have stabilized.
> I understand, 'git am' might complain but I think that should be trivial to fix.
I'd rather not spend time doing trivial fixes, or trying to find latest
versions of individual patches that have dependencies. Having the
patches in a series and replying with new versions to the older versions
makes my life much easier. I'll see all of them in my email client in
one bunch, properly sorted.
> I am perfectly OK to resend as a patch series, if you want.
Yes please.
> >> bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
> >> {
> >> - return gpio_get_value(ip_data->hpd_gpio);
> >> + if (gpio_get_value(ip_data->hpd_gpio))
> >> + return true;
> >> +
> >> + /* Invalidate EDID Cache */
> >> + ip_data->edid_len = 0;
> >> +
> >> + return false;
> >
> > Why is this needed? The HPD interrupt should handle this already. And if
> > the HPD interrupt doesn't work for some reason, this won't work either,
> > as the user can plug and unplug his HDMI monitors a thousand times
> > between two detect calls.
> >
> I thought it is almost impossible to unplug->plug cycle the HDMI cable
> even twice a second, let alone 1000 times against the 10Hz .detect()
> poll :) Or you mean some relay controlled HDMI switching
> mechanism?
omapdss doesn't call the detect function, so it can't presume it's used
in some certain frequency. Also, last time I tested omapdrm, I think
detect was called once in 5 secs or so.
> Anyways, that is not the point of this patch. This patch only aims to
> avoid un-ncessary EDID reads while doing its best to make sure we
> invalidate EDID 'as soon as possible'.
I'm not sure I understand your point. If the HPD interrupt works
properly, EDID is invalidated there, and the code in detect is not
needed. And if the HPD interrupt doesn't work (although I don't see why
it wouldn't), the code in detect doesn't work. In either case it's not
correct.
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 8:53 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <1340605221.12683.30.camel@lappyti>
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.
^ permalink raw reply
* Re: [PATCH] OMAPDSS: HDMI: Cache EDID
From: Jassi Brar @ 2012-06-25 9:16 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <1340611895.3395.14.camel@deskari>
On 25 June 2012 13:41, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
>> I am perfectly OK to resend as a patch series, if you want.
>
> Yes please.
>
OK, will do.
>> >> bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
>> >> {
>> >> - return gpio_get_value(ip_data->hpd_gpio);
>> >> + if (gpio_get_value(ip_data->hpd_gpio))
>> >> + return true;
>> >> +
>> >> + /* Invalidate EDID Cache */
>> >> + ip_data->edid_len = 0;
>> >> +
>> >> + return false;
>> >
>> > Why is this needed? The HPD interrupt should handle this already. And if
>> > the HPD interrupt doesn't work for some reason, this won't work either,
>> > as the user can plug and unplug his HDMI monitors a thousand times
>> > between two detect calls.
>> >
>> I thought it is almost impossible to unplug->plug cycle the HDMI cable
>> even twice a second, let alone 1000 times against the 10Hz .detect()
>> poll :) Or you mean some relay controlled HDMI switching
>> mechanism?
>
> omapdss doesn't call the detect function, so it can't presume it's used
> in some certain frequency. Also, last time I tested omapdrm, I think
> detect was called once in 5 secs or so.
>
It's not omapdss. It's the DRM stack, via the omapdrm, that polls
every 10 secs (not 5).
Sorry I said 10Hz instead of 1/10Hz.
>> Anyways, that is not the point of this patch. This patch only aims to
>> avoid un-ncessary EDID reads while doing its best to make sure we
>> invalidate EDID 'as soon as possible'.
>
> I'm not sure I understand your point. If the HPD interrupt works
> properly, EDID is invalidated there, and the code in detect is not
> needed. And if the HPD interrupt doesn't work (although I don't see why
> it wouldn't), the code in detect doesn't work. In either case it's not
> correct.
>
Well, the idea was to tie edid-cache invalidating with de-asserted
HPD, wherever we read HPD. I will drop it.
thnx
^ permalink raw reply
* 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
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