* [PATCH 3/6] video: fbdev: controlfb.c: use container_of to resolve fb_info_control from fb_info
From: Fabian Frederick @ 2014-09-17 19:00 UTC (permalink / raw)
To: linux-kernel
Cc: daniel.vetter, Fabian Frederick, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev
In-Reply-To: <1410980419-5326-1-git-send-email-fabf@skynet.be>
Use container_of instead of casting first structure member.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
PPC32 cross-compiled but untested.
drivers/video/fbdev/controlfb.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/controlfb.c b/drivers/video/fbdev/controlfb.c
index fdadef9..080fdd2 100644
--- a/drivers/video/fbdev/controlfb.c
+++ b/drivers/video/fbdev/controlfb.c
@@ -218,7 +218,8 @@ static int controlfb_check_var (struct fb_var_screeninfo *var, struct fb_info *i
*/
static int controlfb_set_par (struct fb_info *info)
{
- struct fb_info_control *p = (struct fb_info_control *) info;
+ struct fb_info_control *p + container_of(info, struct fb_info_control, info);
struct fb_par_control par;
int err;
@@ -258,7 +259,8 @@ static int controlfb_pan_display(struct fb_var_screeninfo *var,
struct fb_info *info)
{
unsigned int xoffset, hstep;
- struct fb_info_control *p = (struct fb_info_control *)info;
+ struct fb_info_control *p + container_of(info, struct fb_info_control, info);
struct fb_par_control *par = &p->par;
/*
@@ -309,7 +311,8 @@ static int controlfb_mmap(struct fb_info *info,
static int controlfb_blank(int blank_mode, struct fb_info *info)
{
- struct fb_info_control *p = (struct fb_info_control *) info;
+ struct fb_info_control *p + container_of(info, struct fb_info_control, info);
unsigned ctrl;
ctrl = ld_le32(CNTRL_REG(p,ctrl));
@@ -342,7 +345,8 @@ static int controlfb_blank(int blank_mode, struct fb_info *info)
static int controlfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
u_int transp, struct fb_info *info)
{
- struct fb_info_control *p = (struct fb_info_control *) info;
+ struct fb_info_control *p + container_of(info, struct fb_info_control, info);
__u8 r, g, b;
if (regno > 255)
@@ -833,7 +837,8 @@ static int control_var_to_par(struct fb_var_screeninfo *var,
unsigned hperiod, hssync, hsblank, hesync, heblank, piped, heq, hlfln,
hserr, vperiod, vssync, vesync, veblank, vsblank, vswin, vewin;
unsigned long pixclock;
- struct fb_info_control *p = (struct fb_info_control *) fb_info;
+ struct fb_info_control *p + container_of(fb_info, struct fb_info_control, info);
struct control_regvals *r = &par->regvals;
switch (var->bits_per_pixel) {
--
2.1.0
^ permalink raw reply related
* [PATCH 4/6] video: fbdev: cyber2000fb.c: use container_of to resolve cfb_info from fb_info
From: Fabian Frederick @ 2014-09-17 19:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1410980419-5326-1-git-send-email-fabf@skynet.be>
Use container_of instead of casting first structure member.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
Compiled but untested.
drivers/video/fbdev/cyber2000fb.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/video/fbdev/cyber2000fb.c b/drivers/video/fbdev/cyber2000fb.c
index b0a950f..99acf53 100644
--- a/drivers/video/fbdev/cyber2000fb.c
+++ b/drivers/video/fbdev/cyber2000fb.c
@@ -159,7 +159,7 @@ cyber2000_seqw(unsigned int reg, unsigned int val, struct cfb_info *cfb)
static void
cyber2000fb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
{
- struct cfb_info *cfb = (struct cfb_info *)info;
+ struct cfb_info *cfb = container_of(info, struct cfb_info, fb);
unsigned long dst, col;
if (!(cfb->fb.var.accel_flags & FB_ACCELF_TEXT)) {
@@ -191,7 +191,7 @@ cyber2000fb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
static void
cyber2000fb_copyarea(struct fb_info *info, const struct fb_copyarea *region)
{
- struct cfb_info *cfb = (struct cfb_info *)info;
+ struct cfb_info *cfb = container_of(info, struct cfb_info, fb);
unsigned int cmd = CO_CMD_L_PATTERN_FGCOL;
unsigned long src, dst;
@@ -241,7 +241,7 @@ cyber2000fb_imageblit(struct fb_info *info, const struct fb_image *image)
static int cyber2000fb_sync(struct fb_info *info)
{
- struct cfb_info *cfb = (struct cfb_info *)info;
+ struct cfb_info *cfb = container_of(info, struct cfb_info, fb);
int count = 100000;
if (!(cfb->fb.var.accel_flags & FB_ACCELF_TEXT))
@@ -276,7 +276,7 @@ static int
cyber2000fb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
u_int transp, struct fb_info *info)
{
- struct cfb_info *cfb = (struct cfb_info *)info;
+ struct cfb_info *cfb = container_of(info, struct cfb_info, fb);
struct fb_var_screeninfo *var = &cfb->fb.var;
u32 pseudo_val;
int ret = 1;
@@ -758,7 +758,7 @@ cyber2000fb_decode_clock(struct par_info *hw, struct cfb_info *cfb,
static int
cyber2000fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
{
- struct cfb_info *cfb = (struct cfb_info *)info;
+ struct cfb_info *cfb = container_of(info, struct cfb_info, fb);
struct par_info hw;
unsigned int mem;
int err;
@@ -861,7 +861,7 @@ cyber2000fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
static int cyber2000fb_set_par(struct fb_info *info)
{
- struct cfb_info *cfb = (struct cfb_info *)info;
+ struct cfb_info *cfb = container_of(info, struct cfb_info, fb);
struct fb_var_screeninfo *var = &cfb->fb.var;
struct par_info hw;
unsigned int mem;
@@ -971,7 +971,7 @@ static int cyber2000fb_set_par(struct fb_info *info)
static int
cyber2000fb_pan_display(struct fb_var_screeninfo *var, struct fb_info *info)
{
- struct cfb_info *cfb = (struct cfb_info *)info;
+ struct cfb_info *cfb = container_of(info, struct cfb_info, fb);
if (cyber2000fb_update_start(cfb, var))
return -EINVAL;
@@ -1007,7 +1007,7 @@ cyber2000fb_pan_display(struct fb_var_screeninfo *var, struct fb_info *info)
*/
static int cyber2000fb_blank(int blank, struct fb_info *info)
{
- struct cfb_info *cfb = (struct cfb_info *)info;
+ struct cfb_info *cfb = container_of(info, struct cfb_info, fb);
unsigned int sync = 0;
int i;
--
2.1.0
^ permalink raw reply related
* [PATCH 5/6] video: fbdev: pxafb.c: use container_of to resolve pxafb_info/layer from fb_info
From: Fabian Frederick @ 2014-09-17 19:00 UTC (permalink / raw)
To: linux-kernel
Cc: daniel.vetter, Fabian Frederick, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev
In-Reply-To: <1410980419-5326-1-git-send-email-fabf@skynet.be>
Use container_of instead of casting first structure member.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
ARM-PXA cross-compiled but untested.
drivers/video/fbdev/pxafb.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
index 1ecd9ce..a5acca8 100644
--- a/drivers/video/fbdev/pxafb.c
+++ b/drivers/video/fbdev/pxafb.c
@@ -138,7 +138,7 @@ static int
pxafb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
u_int trans, struct fb_info *info)
{
- struct pxafb_info *fbi = (struct pxafb_info *)info;
+ struct pxafb_info *fbi = container_of(info, struct pxafb_info, fb);
u_int val;
if (regno >= fbi->palette_size)
@@ -183,7 +183,7 @@ static int
pxafb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
u_int trans, struct fb_info *info)
{
- struct pxafb_info *fbi = (struct pxafb_info *)info;
+ struct pxafb_info *fbi = container_of(info, struct pxafb_info, fb);
unsigned int val;
int ret = 1;
@@ -456,7 +456,7 @@ static int pxafb_adjust_timing(struct pxafb_info *fbi,
*/
static int pxafb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
{
- struct pxafb_info *fbi = (struct pxafb_info *)info;
+ struct pxafb_info *fbi = container_of(info, struct pxafb_info, fb);
struct pxafb_mach_info *inf = dev_get_platdata(fbi->dev);
int err;
@@ -494,7 +494,7 @@ static int pxafb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
*/
static int pxafb_set_par(struct fb_info *info)
{
- struct pxafb_info *fbi = (struct pxafb_info *)info;
+ struct pxafb_info *fbi = container_of(info, struct pxafb_info, fb);
struct fb_var_screeninfo *var = &info->var;
if (var->bits_per_pixel >= 16)
@@ -533,7 +533,7 @@ static int pxafb_set_par(struct fb_info *info)
static int pxafb_pan_display(struct fb_var_screeninfo *var,
struct fb_info *info)
{
- struct pxafb_info *fbi = (struct pxafb_info *)info;
+ struct pxafb_info *fbi = container_of(info, struct pxafb_info, fb);
struct fb_var_screeninfo newvar;
int dma = DMA_MAX + DMA_BASE;
@@ -566,7 +566,7 @@ static int pxafb_pan_display(struct fb_var_screeninfo *var,
*/
static int pxafb_blank(int blank, struct fb_info *info)
{
- struct pxafb_info *fbi = (struct pxafb_info *)info;
+ struct pxafb_info *fbi = container_of(info, struct pxafb_info, fb);
int i;
switch (blank) {
@@ -725,7 +725,7 @@ static struct pxafb_layer_ops ofb_ops[] = {
static int overlayfb_open(struct fb_info *info, int user)
{
- struct pxafb_layer *ofb = (struct pxafb_layer *)info;
+ struct pxafb_layer *ofb = container_of(info, struct pxafb_layer, fb);
/* no support for framebuffer console on overlay */
if (user = 0)
@@ -743,7 +743,7 @@ static int overlayfb_open(struct fb_info *info, int user)
static int overlayfb_release(struct fb_info *info, int user)
{
- struct pxafb_layer *ofb = (struct pxafb_layer*) info;
+ struct pxafb_layer *ofb = container_of(info, struct pxafb_layer, fb);
if (ofb->usage = 1) {
ofb->ops->disable(ofb);
@@ -760,7 +760,7 @@ static int overlayfb_release(struct fb_info *info, int user)
static int overlayfb_check_var(struct fb_var_screeninfo *var,
struct fb_info *info)
{
- struct pxafb_layer *ofb = (struct pxafb_layer *)info;
+ struct pxafb_layer *ofb = container_of(info, struct pxafb_layer, fb);
struct fb_var_screeninfo *base_var = &ofb->fbi->fb.var;
int xpos, ypos, pfor, bpp;
@@ -836,7 +836,7 @@ static int overlayfb_check_video_memory(struct pxafb_layer *ofb)
static int overlayfb_set_par(struct fb_info *info)
{
- struct pxafb_layer *ofb = (struct pxafb_layer *)info;
+ struct pxafb_layer *ofb = container_of(info, struct pxafb_layer, fb);
struct fb_var_screeninfo *var = &info->var;
int xpos, ypos, pfor, bpp, ret;
--
2.1.0
^ permalink raw reply related
* [PATCH 6/6] video: fbdev: valkyriefb.c: use container_of to resolve fb_info_valkyrie from fb_info
From: Fabian Frederick @ 2014-09-17 19:00 UTC (permalink / raw)
To: linux-kernel
Cc: daniel.vetter, Fabian Frederick, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev
In-Reply-To: <1410980419-5326-1-git-send-email-fabf@skynet.be>
Use container_of instead of casting first structure member.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
PPC32 cross-compiled but untested.
drivers/video/fbdev/valkyriefb.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/video/fbdev/valkyriefb.c b/drivers/video/fbdev/valkyriefb.c
index 97cb9bd..9c98945 100644
--- a/drivers/video/fbdev/valkyriefb.c
+++ b/drivers/video/fbdev/valkyriefb.c
@@ -136,7 +136,8 @@ static struct fb_ops valkyriefb_ops = {
/* Sets the video mode according to info->var */
static int valkyriefb_set_par(struct fb_info *info)
{
- struct fb_info_valkyrie *p = (struct fb_info_valkyrie *) info;
+ struct fb_info_valkyrie *p + container_of(info, struct fb_info_valkyrie, info);
volatile struct valkyrie_regs __iomem *valkyrie_regs = p->valkyrie_regs;
struct fb_par_valkyrie *par = info->par;
struct valkyrie_regvals *init;
@@ -194,7 +195,8 @@ valkyriefb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
*/
static int valkyriefb_blank(int blank_mode, struct fb_info *info)
{
- struct fb_info_valkyrie *p = (struct fb_info_valkyrie *) info;
+ struct fb_info_valkyrie *p + container_of(info, struct fb_info_valkyrie, info);
struct fb_par_valkyrie *par = info->par;
struct valkyrie_regvals *init = par->init;
@@ -226,7 +228,8 @@ static int valkyriefb_blank(int blank_mode, struct fb_info *info)
static int valkyriefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
u_int transp, struct fb_info *info)
{
- struct fb_info_valkyrie *p = (struct fb_info_valkyrie *) info;
+ struct fb_info_valkyrie *p + container_of(info, struct fb_info_valkyrie, info);
volatile struct cmap_regs __iomem *cmap_regs = p->cmap_regs;
struct fb_par_valkyrie *par = info->par;
@@ -465,7 +468,8 @@ static int valkyrie_var_to_par(struct fb_var_screeninfo *var,
{
int vmode, cmode;
struct valkyrie_regvals *init;
- struct fb_info_valkyrie *p = (struct fb_info_valkyrie *) fb_info;
+ struct fb_info_valkyrie *p + container_of(fb_info, struct fb_info_valkyrie, info);
if (mac_var_to_vmode(var, &vmode, &cmode) != 0) {
printk(KERN_ERR "valkyriefb: can't do %dx%dx%d.\n",
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] fbdev: fsl-sii902x: HDMI support for Freescale SoCs
From: Tomi Valkeinen @ 2014-09-19 13:45 UTC (permalink / raw)
To: Li.Xiubo@freescale.com
Cc: robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com,
ijc+devicetree@hellion.org.uk, galak@codeaurora.org,
plagnioj@jcrosoft.com, grant.likely@linaro.org, arnd@arndb.de,
peter.griffin@linaro.org, jg1.han@samsung.com,
daniel.vetter@ffwll.ch, laurent.pinchart@ideasonboard.com,
robdclark@gmail.com, linux-fbdev@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <d94bf53d85884b4da0fb4da3041e5742@BY2PR0301MB0613.namprd03.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 2906 bytes --]
On 15/09/14 05:17, Li.Xiubo@freescale.com wrote:
> Hi Tomi,
>
> Thanks very much for your comments.
>
>
>> Subject: Re: [PATCH] fbdev: fsl-sii902x: HDMI support for Freescale SoCs
>>
>> Hi,
>>
>> On 05/09/14 07:48, Xiubo Li wrote:
>>> Some Freescale SoCs, there has an DVI/HDMI controller and a PHY,
>>> attached to one of their display controller unit's LCDC interfaces.
>>> This patch adds a preliminary static support for such controllers.
>>>
>>> This will support for many modes and a dynamic switching between
>>> them.
>>>
>>> Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
>>> ---
>>> .../devicetree/bindings/video/fsl-sii902x.txt | 17 +
>>> drivers/video/fbdev/Kconfig | 7 +
>>> drivers/video/fbdev/Makefile | 1 +
>>> drivers/video/fbdev/fsl-sii902x.c | 526
>> +++++++++++++++++++++
>>> 4 files changed, 551 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/video/fsl-sii902x.txt
>>> create mode 100644 drivers/video/fbdev/fsl-sii902x.c
>>
>> I don't know how you picked the names of the people you sent this patch
>> to, but looks to me that most of them are probably not interested in
>> this patch.
>>
>
> I just using the get_maintainer.pl script.
Yes, and that's good, but you have to use your judgment also.
get_maintainer.pl gives you names of people who have at some point
touched the files or directories you are changing. Usually only the
first names returned by get_maintainer.pl are relevant.
>> Anyway, a few quick comments on the patch:
>>
>> - You should probably use regmap instead of direct i2c calls.
>> Interestingly, you define regmap variable, but you never use it.
>
> Yes, this it's my another version in my local machine using regmap which
> need much more test.
>
>> - Use defines for register offsets, instead of magic numbers.
>
> This will be fixed together with regmap using.
Well, it's better to send the patch only when you have tested and
cleaned up your driver.
>> - You should not use static variables. They prevent having multiple
>> instances of the device.
>>
>
> Okay.
>
>> So the SiI902x chip is on the SoC, not on the board? And it's a plain
>> standard SiI902x in other aspects?
>>
>
> No, it's on board.
>
> And it will be used for i.MX and LS1+ platforms.
Ok. In that case, I would suggest you to move to the DRM framework. The
fbdev framework is not suited to adding external encoders. The end
result with fbdev will be a SoC/board specific hack driver.
That said, we already have such drivers for fbdev, so I'm not 100%
against adding a new one. But I'm very very seriously recommending
moving to DRM.
And if this driver is added to fbdev, I think the device tree bindings
should use the video ports/endpoints to describe the connections.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* HOW ARE YOU?
From: Benjamin Siaka @ 2014-09-22 1:15 UTC (permalink / raw)
To: linux-fbdev
Hello my Dear,
I will greatly appreciate my correspondence meets you in good health condition.
My name is Mr. Benjamin Siaka. I am seeking for your co-operation for investment partnership in your Country. I shall provide the FUND for the investment. When you acknowledged the receipt of this correspondence, thereafter I will give you the Full Details of my investment proposal.
I await your response in earliest.
My regards,
Mr. Benjamin Siaka.
^ permalink raw reply
* [PATCH/RFC] fbdev: sh_mobile_hdmi: Re-init regs before irq re-enable on resume
From: Geert Uytterhoeven @ 2014-09-23 12:21 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Rafael J. Wysocki
Cc: Ulf Hansson, Guennadi Liakhovetski, linux-fbdev, linux-pm,
linux-sh, Geert Uytterhoeven
When the PM domain containing the HDMI hardware block is powered down,
the HDMI register values (incl. interrupt polarity settings) are lost.
During resume, after powering up the PM domain, interrupts are
re-enabled, and an interrupt storm happens due to incorrect interrupt
polarity settings:
irq 163: nobody cared (try booting with the "irqpoll" option)
...
Disabling IRQ #163
To fix this, re-initialize the interrupt polarity settings, and the
htop1 register block (if present), during resume.
As the .suspend_noirq() and .resume_noirq() callbacks are not called
when using the generic PM domain, the normal .resume() callback is used,
and the device interrupt needs to be disabled/enabled manually.
This fixes resume from s2ram with power down of the A4MP PM domain on
r8a7740/Armadillo.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Is there a specific reason why the .suspend_noirq() and .resume_noirq()
callbacks are not called when using genpd, unlike .suspend(),
.suspend_late(), .resume_early(), and .resume()?
---
drivers/video/fbdev/sh_mobile_hdmi.c | 44 ++++++++++++++++++++++++++++++++++--
1 file changed, 42 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/sh_mobile_hdmi.c b/drivers/video/fbdev/sh_mobile_hdmi.c
index 9a33ee0413fb584d..7c72a3f02056445d 100644
--- a/drivers/video/fbdev/sh_mobile_hdmi.c
+++ b/drivers/video/fbdev/sh_mobile_hdmi.c
@@ -281,6 +281,7 @@ struct sh_hdmi {
u8 edid_block_addr;
u8 edid_segment_nr;
u8 edid_blocks;
+ int irq;
struct clk *hdmi_clk;
struct device *dev;
struct delayed_work edid_work;
@@ -1299,6 +1300,7 @@ static int __init sh_hdmi_probe(struct platform_device *pdev)
hdmi->dev = &pdev->dev;
hdmi->entity.owner = THIS_MODULE;
hdmi->entity.ops = &sh_hdmi_ops;
+ hdmi->irq = irq;
hdmi->hdmi_clk = clk_get(&pdev->dev, "ick");
if (IS_ERR(hdmi->hdmi_clk)) {
@@ -1415,12 +1417,11 @@ static int __exit sh_hdmi_remove(struct platform_device *pdev)
{
struct sh_hdmi *hdmi = entity_to_sh_hdmi(platform_get_drvdata(pdev));
struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- int irq = platform_get_irq(pdev, 0);
snd_soc_unregister_codec(&pdev->dev);
/* No new work will be scheduled, wait for running ISR */
- free_irq(irq, hdmi);
+ free_irq(hdmi->irq, hdmi);
/* Wait for already scheduled work */
cancel_delayed_work_sync(&hdmi->edid_work);
pm_runtime_put(&pdev->dev);
@@ -1435,10 +1436,49 @@ static int __exit sh_hdmi_remove(struct platform_device *pdev)
return 0;
}
+static int sh_hdmi_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct sh_hdmi *hdmi = entity_to_sh_hdmi(platform_get_drvdata(pdev));
+
+ disable_irq(hdmi->irq);
+ /* Wait for already scheduled work */
+ cancel_delayed_work_sync(&hdmi->edid_work);
+ return 0;
+}
+
+static int sh_hdmi_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct sh_mobile_hdmi_info *pdata = dev_get_platdata(dev);
+ struct sh_hdmi *hdmi = entity_to_sh_hdmi(platform_get_drvdata(pdev));
+
+ /* Re-init interrupt polarity */
+ if (pdata->flags & HDMI_OUTPUT_PUSH_PULL)
+ hdmi_bit_set(hdmi, 0x02, 0x02, HDMI_SYSTEM_CTRL);
+
+ if (pdata->flags & HDMI_OUTPUT_POLARITY_HI)
+ hdmi_bit_set(hdmi, 0x01, 0x01, HDMI_SYSTEM_CTRL);
+
+ /* Re-init htop1 */
+ if (hdmi->htop1)
+ sh_hdmi_htop1_init(hdmi);
+
+ /* Now it's safe to enable interrupts again */
+ enable_irq(hdmi->irq);
+ return 0;
+}
+
+static const struct dev_pm_ops sh_hdmi_pm_ops = {
+ .suspend = sh_hdmi_suspend,
+ .resume = sh_hdmi_resume,
+};
+
static struct platform_driver sh_hdmi_driver = {
.remove = __exit_p(sh_hdmi_remove),
.driver = {
.name = "sh-mobile-hdmi",
+ .pm = &sh_hdmi_pm_ops,
},
};
--
1.9.1
^ permalink raw reply related
* Re: [PATCH/RFC] fbdev: sh_mobile_hdmi: Re-init regs before irq re-enable on resume
From: Ulf Hansson @ 2014-09-23 17:26 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Rafael J. Wysocki, Guennadi Liakhovetski, linux-fbdev,
linux-pm@vger.kernel.org, Linux-sh list
In-Reply-To: <1411474918-2955-1-git-send-email-geert+renesas@glider.be>
On 23 September 2014 14:21, Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> When the PM domain containing the HDMI hardware block is powered down,
> the HDMI register values (incl. interrupt polarity settings) are lost.
> During resume, after powering up the PM domain, interrupts are
> re-enabled, and an interrupt storm happens due to incorrect interrupt
> polarity settings:
>
> irq 163: nobody cared (try booting with the "irqpoll" option)
> ...
> Disabling IRQ #163
>
> To fix this, re-initialize the interrupt polarity settings, and the
> htop1 register block (if present), during resume.
>
> As the .suspend_noirq() and .resume_noirq() callbacks are not called
> when using the generic PM domain, the normal .resume() callback is used,
> and the device interrupt needs to be disabled/enabled manually.
>
> This fixes resume from s2ram with power down of the A4MP PM domain on
> r8a7740/Armadillo.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Is there a specific reason why the .suspend_noirq() and .resume_noirq()
> callbacks are not called when using genpd, unlike .suspend(),
> .suspend_late(), .resume_early(), and .resume()?
Hi Geert,
Interesting issue you are trying to fix here.
In principle I consider the *noirq() callbacks in genpd as workarounds
to handle the corner cases we had when using runtime PM and system PM
together. This is at least how the OMAP PM domain uses them.
Today, there are a better option which is to use
pm_runtime_force_suspend|resume() and to declare the runtime PM
callbacks within CONFIG_PM instead of CONFIG_PM_RUNTIME. I am actually
working on a patchset that tries this approach, do you think that
could solve your issue?
That said, I also think it's a bug in genpd to not invoke
pm_generic_suspend|resume_noirq() from its corresponding callbacks. I
think we should add them, but let's see if its trivial to do that. :-)
Kind regards
Uffe
> ---
> drivers/video/fbdev/sh_mobile_hdmi.c | 44 ++++++++++++++++++++++++++++++++++--
> 1 file changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/sh_mobile_hdmi.c b/drivers/video/fbdev/sh_mobile_hdmi.c
> index 9a33ee0413fb584d..7c72a3f02056445d 100644
> --- a/drivers/video/fbdev/sh_mobile_hdmi.c
> +++ b/drivers/video/fbdev/sh_mobile_hdmi.c
> @@ -281,6 +281,7 @@ struct sh_hdmi {
> u8 edid_block_addr;
> u8 edid_segment_nr;
> u8 edid_blocks;
> + int irq;
> struct clk *hdmi_clk;
> struct device *dev;
> struct delayed_work edid_work;
> @@ -1299,6 +1300,7 @@ static int __init sh_hdmi_probe(struct platform_device *pdev)
> hdmi->dev = &pdev->dev;
> hdmi->entity.owner = THIS_MODULE;
> hdmi->entity.ops = &sh_hdmi_ops;
> + hdmi->irq = irq;
>
> hdmi->hdmi_clk = clk_get(&pdev->dev, "ick");
> if (IS_ERR(hdmi->hdmi_clk)) {
> @@ -1415,12 +1417,11 @@ static int __exit sh_hdmi_remove(struct platform_device *pdev)
> {
> struct sh_hdmi *hdmi = entity_to_sh_hdmi(platform_get_drvdata(pdev));
> struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - int irq = platform_get_irq(pdev, 0);
>
> snd_soc_unregister_codec(&pdev->dev);
>
> /* No new work will be scheduled, wait for running ISR */
> - free_irq(irq, hdmi);
> + free_irq(hdmi->irq, hdmi);
> /* Wait for already scheduled work */
> cancel_delayed_work_sync(&hdmi->edid_work);
> pm_runtime_put(&pdev->dev);
> @@ -1435,10 +1436,49 @@ static int __exit sh_hdmi_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static int sh_hdmi_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct sh_hdmi *hdmi = entity_to_sh_hdmi(platform_get_drvdata(pdev));
> +
> + disable_irq(hdmi->irq);
> + /* Wait for already scheduled work */
> + cancel_delayed_work_sync(&hdmi->edid_work);
> + return 0;
> +}
> +
> +static int sh_hdmi_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct sh_mobile_hdmi_info *pdata = dev_get_platdata(dev);
> + struct sh_hdmi *hdmi = entity_to_sh_hdmi(platform_get_drvdata(pdev));
> +
> + /* Re-init interrupt polarity */
> + if (pdata->flags & HDMI_OUTPUT_PUSH_PULL)
> + hdmi_bit_set(hdmi, 0x02, 0x02, HDMI_SYSTEM_CTRL);
> +
> + if (pdata->flags & HDMI_OUTPUT_POLARITY_HI)
> + hdmi_bit_set(hdmi, 0x01, 0x01, HDMI_SYSTEM_CTRL);
> +
> + /* Re-init htop1 */
> + if (hdmi->htop1)
> + sh_hdmi_htop1_init(hdmi);
> +
> + /* Now it's safe to enable interrupts again */
> + enable_irq(hdmi->irq);
> + return 0;
> +}
> +
> +static const struct dev_pm_ops sh_hdmi_pm_ops = {
> + .suspend = sh_hdmi_suspend,
> + .resume = sh_hdmi_resume,
> +};
> +
> static struct platform_driver sh_hdmi_driver = {
> .remove = __exit_p(sh_hdmi_remove),
> .driver = {
> .name = "sh-mobile-hdmi",
> + .pm = &sh_hdmi_pm_ops,
> },
> };
>
> --
> 1.9.1
>
^ permalink raw reply
* RE: [PATCH] fbdev: fsl-sii902x: HDMI support for Freescale SoCs
From: Li.Xiubo @ 2014-09-24 6:45 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
pawel.moll-5wv7dgnIgG8@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org,
grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
arnd-r2nGTMty4D4@public.gmane.org,
peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org,
daniel.vetter-/w4YWyX8dFk@public.gmane.org,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org,
robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <541C3371.8080100-l0cyMroinI0@public.gmane.org>
Hi,
[...]
> >>> 4 files changed, 551 insertions(+)
> >>> create mode 100644 Documentation/devicetree/bindings/video/fsl-
> sii902x.txt
> >>> create mode 100644 drivers/video/fbdev/fsl-sii902x.c
> >>
> >> I don't know how you picked the names of the people you sent this patch
> >> to, but looks to me that most of them are probably not interested in
> >> this patch.
> >>
> >
> > I just using the get_maintainer.pl script.
>
> Yes, and that's good, but you have to use your judgment also.
> get_maintainer.pl gives you names of people who have at some point
> touched the files or directories you are changing. Usually only the
> first names returned by get_maintainer.pl are relevant.
>
Okay :)
> >> Anyway, a few quick comments on the patch:
> >>
> >> - You should probably use regmap instead of direct i2c calls.
> >> Interestingly, you define regmap variable, but you never use it.
> >
> > Yes, this it's my another version in my local machine using regmap which
> > need much more test.
> >
> >> - Use defines for register offsets, instead of magic numbers.
> >
> > This will be fixed together with regmap using.
>
> Well, it's better to send the patch only when you have tested and
> cleaned up your driver.
>
> >> - You should not use static variables. They prevent having multiple
> >> instances of the device.
> >>
> >
> > Okay.
> >
> >> So the SiI902x chip is on the SoC, not on the board? And it's a plain
> >> standard SiI902x in other aspects?
> >>
> >
> > No, it's on board.
> >
> > And it will be used for i.MX and LS1+ platforms.
>
> Ok. In that case, I would suggest you to move to the DRM framework. The
> fbdev framework is not suited to adding external encoders. The end
> result with fbdev will be a SoC/board specific hack driver.
>
> That said, we already have such drivers for fbdev, so I'm not 100%
> against adding a new one. But I'm very very seriously recommending
> moving to DRM.
>
> And if this driver is added to fbdev, I think the device tree bindings
> should use the video ports/endpoints to describe the connections.
>
I will have a try to use the DRM framework.
Thanks,
BRs
Xiubo
> Tomi
>
^ permalink raw reply
* Re: [PATCH/RFC] fbdev: sh_mobile_hdmi: Re-init regs before irq re-enable on resume
From: Geert Uytterhoeven @ 2014-09-24 8:32 UTC (permalink / raw)
To: Ulf Hansson
Cc: Geert Uytterhoeven, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, Rafael J. Wysocki, Guennadi Liakhovetski,
Linux Fbdev development list, linux-pm@vger.kernel.org,
Linux-sh list
In-Reply-To: <CAPDyKFrGiEJqHe2+SSNpUhBKfhGraqfomskUbemFmucszv8ukw@mail.gmail.com>
Hi Ulf,
On Tue, Sep 23, 2014 at 7:26 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 23 September 2014 14:21, Geert Uytterhoeven <geert+renesas@glider.be> wrote:
>> When the PM domain containing the HDMI hardware block is powered down,
>> the HDMI register values (incl. interrupt polarity settings) are lost.
>> During resume, after powering up the PM domain, interrupts are
>> re-enabled, and an interrupt storm happens due to incorrect interrupt
>> polarity settings:
>>
>> irq 163: nobody cared (try booting with the "irqpoll" option)
>> ...
>> Disabling IRQ #163
>>
>> To fix this, re-initialize the interrupt polarity settings, and the
>> htop1 register block (if present), during resume.
>>
>> As the .suspend_noirq() and .resume_noirq() callbacks are not called
>> when using the generic PM domain, the normal .resume() callback is used,
>> and the device interrupt needs to be disabled/enabled manually.
>>
>> This fixes resume from s2ram with power down of the A4MP PM domain on
>> r8a7740/Armadillo.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> Is there a specific reason why the .suspend_noirq() and .resume_noirq()
>> callbacks are not called when using genpd, unlike .suspend(),
>> .suspend_late(), .resume_early(), and .resume()?
>
> Hi Geert,
>
> Interesting issue you are trying to fix here.
>
> In principle I consider the *noirq() callbacks in genpd as workarounds
> to handle the corner cases we had when using runtime PM and system PM
> together. This is at least how the OMAP PM domain uses them.
>
> Today, there are a better option which is to use
> pm_runtime_force_suspend|resume() and to declare the runtime PM
> callbacks within CONFIG_PM instead of CONFIG_PM_RUNTIME. I am actually
> working on a patchset that tries this approach, do you think that
> could solve your issue?
I don't follow 100% what you're telling me, but I don't think this would help:
the HDMI interrupt is used for HDMI hotplug detection, so it should stay
enabled even when the HDMI device is runtime suspended.
Only when the system is suspended, and the PM domain will be powered
down, the interrupt can be disabled.
After powering up the PM domain, but before the interrupt is enabled,
the registers must be restored.
> That said, I also think it's a bug in genpd to not invoke
> pm_generic_suspend|resume_noirq() from its corresponding callbacks. I
> think we should add them, but let's see if its trivial to do that. :-)
pm_genpd_suspend() and pm_genpd_suspend_late() call the
pm_generic_*() version if no genpd->suspend_power_off callback is
set up.
pm_genpd_suspend_noirq() does some really different things (i.e.
powering down the PM domain), so IMHO this isn't trivial to change.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH/RFC] fbdev: sh_mobile_hdmi: Re-init regs before irq re-enable on resume
From: Ulf Hansson @ 2014-09-24 13:07 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Geert Uytterhoeven, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, Rafael J. Wysocki, Guennadi Liakhovetski,
Linux Fbdev development list, linux-pm@vger.kernel.org,
Linux-sh list
In-Reply-To: <CAMuHMdXoD6b-SNKKQpS6gzM_sDm4guepn1O1p_KK_7SyTC5CDQ@mail.gmail.com>
On 24 September 2014 10:32, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Ulf,
>
> On Tue, Sep 23, 2014 at 7:26 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 23 September 2014 14:21, Geert Uytterhoeven <geert+renesas@glider.be> wrote:
>>> When the PM domain containing the HDMI hardware block is powered down,
>>> the HDMI register values (incl. interrupt polarity settings) are lost.
>>> During resume, after powering up the PM domain, interrupts are
>>> re-enabled, and an interrupt storm happens due to incorrect interrupt
>>> polarity settings:
>>>
>>> irq 163: nobody cared (try booting with the "irqpoll" option)
>>> ...
>>> Disabling IRQ #163
>>>
>>> To fix this, re-initialize the interrupt polarity settings, and the
>>> htop1 register block (if present), during resume.
>>>
>>> As the .suspend_noirq() and .resume_noirq() callbacks are not called
>>> when using the generic PM domain, the normal .resume() callback is used,
>>> and the device interrupt needs to be disabled/enabled manually.
>>>
>>> This fixes resume from s2ram with power down of the A4MP PM domain on
>>> r8a7740/Armadillo.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> ---
>>> Is there a specific reason why the .suspend_noirq() and .resume_noirq()
>>> callbacks are not called when using genpd, unlike .suspend(),
>>> .suspend_late(), .resume_early(), and .resume()?
>>
>> Hi Geert,
>>
>> Interesting issue you are trying to fix here.
>>
>> In principle I consider the *noirq() callbacks in genpd as workarounds
>> to handle the corner cases we had when using runtime PM and system PM
>> together. This is at least how the OMAP PM domain uses them.
>>
>> Today, there are a better option which is to use
>> pm_runtime_force_suspend|resume() and to declare the runtime PM
>> callbacks within CONFIG_PM instead of CONFIG_PM_RUNTIME. I am actually
>> working on a patchset that tries this approach, do you think that
>> could solve your issue?
>
> I don't follow 100% what you're telling me, but I don't think this would help:
> the HDMI interrupt is used for HDMI hotplug detection, so it should stay
> enabled even when the HDMI device is runtime suspended.
> Only when the system is suspended, and the PM domain will be powered
> down, the interrupt can be disabled.
>
> After powering up the PM domain, but before the interrupt is enabled,
> the registers must be restored.
I checked the details about the runtime PM support in the driver. To
me, it seems there are some additional pieces missing.
For a short while, let's just ignore the behaviour of the generic PM
domain. Then I would suggest to add the following below to the driver.
Please tell me if you think I am wrong, I don't no much about ALSA and HDMI. :-)
1) Add a runtime PM suspend callback to:
- Save register context.
- Enable wakeup IRQ if "wakeup IRQ capabilities" is set.
2) Add a runtime PM resume callback to:
- Restore register context.
- Disable wakeup IRQ.
3) Add a system PM suspend callback to:
- Disable "wakeup IRQ capabilities".
- Put the device into low power state.
Could likely be done by:
pm_runtime_get_sync();
"disable wakeup irq cap";
pm_runtime_put();
pm_runtime_force_suspend();
4) Add a system PM resume callback to:
- Enable "wakeup IRQ capabilities".
- Restore power to the device.
Could likely be done by:
"enable wakeup irq_cap";
pm_runtime_force_resume();
5) Currently the ->probe() method invokes pm_runtime_get_sync(),
causing the runtime PM resume callbacks to be invoked in this path.
Unless I am missing a point, this will cause the device stay runtime
PM resumed all the time. Is that really what you want?
In this context, and what I am trying to understand, is what changes
are needed to the generic PM domain, such it can be used under these
circumstances.
Kind regards
Uffe
>
>> That said, I also think it's a bug in genpd to not invoke
>> pm_generic_suspend|resume_noirq() from its corresponding callbacks. I
>> think we should add them, but let's see if its trivial to do that. :-)
>
> pm_genpd_suspend() and pm_genpd_suspend_late() call the
> pm_generic_*() version if no genpd->suspend_power_off callback is
> set up.
>
> pm_genpd_suspend_noirq() does some really different things (i.e.
> powering down the PM domain), so IMHO this isn't trivial to change.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
^ permalink raw reply
* Re: [PATCH/RFC] fbdev: sh_mobile_hdmi: Re-init regs before irq re-enable on resume
From: Geert Uytterhoeven @ 2014-09-24 13:42 UTC (permalink / raw)
To: Ulf Hansson
Cc: Geert Uytterhoeven, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, Rafael J. Wysocki, Guennadi Liakhovetski,
Linux Fbdev development list, linux-pm@vger.kernel.org,
Linux-sh list
In-Reply-To: <CAPDyKFoGuKsDVnHfALRRtXu8kxXvUG-a-7308VRo=KQP7wAaRQ@mail.gmail.com>
Hi Ulf,
On Wed, Sep 24, 2014 at 3:07 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 24 September 2014 10:32, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Tue, Sep 23, 2014 at 7:26 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> On 23 September 2014 14:21, Geert Uytterhoeven <geert+renesas@glider.be> wrote:
>>>> When the PM domain containing the HDMI hardware block is powered down,
>>>> the HDMI register values (incl. interrupt polarity settings) are lost.
>>>> During resume, after powering up the PM domain, interrupts are
>>>> re-enabled, and an interrupt storm happens due to incorrect interrupt
>>>> polarity settings:
>>>>
>>>> irq 163: nobody cared (try booting with the "irqpoll" option)
>>>> ...
>>>> Disabling IRQ #163
>>>>
>>>> To fix this, re-initialize the interrupt polarity settings, and the
>>>> htop1 register block (if present), during resume.
>>>>
>>>> As the .suspend_noirq() and .resume_noirq() callbacks are not called
>>>> when using the generic PM domain, the normal .resume() callback is used,
>>>> and the device interrupt needs to be disabled/enabled manually.
>>>>
>>>> This fixes resume from s2ram with power down of the A4MP PM domain on
>>>> r8a7740/Armadillo.
>>>>
>>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>> ---
>>>> Is there a specific reason why the .suspend_noirq() and .resume_noirq()
>>>> callbacks are not called when using genpd, unlike .suspend(),
>>>> .suspend_late(), .resume_early(), and .resume()?
>>>
>>> Hi Geert,
>>>
>>> Interesting issue you are trying to fix here.
>>>
>>> In principle I consider the *noirq() callbacks in genpd as workarounds
>>> to handle the corner cases we had when using runtime PM and system PM
>>> together. This is at least how the OMAP PM domain uses them.
>>>
>>> Today, there are a better option which is to use
>>> pm_runtime_force_suspend|resume() and to declare the runtime PM
>>> callbacks within CONFIG_PM instead of CONFIG_PM_RUNTIME. I am actually
>>> working on a patchset that tries this approach, do you think that
>>> could solve your issue?
>>
>> I don't follow 100% what you're telling me, but I don't think this would help:
>> the HDMI interrupt is used for HDMI hotplug detection, so it should stay
>> enabled even when the HDMI device is runtime suspended.
>> Only when the system is suspended, and the PM domain will be powered
>> down, the interrupt can be disabled.
>>
>> After powering up the PM domain, but before the interrupt is enabled,
>> the registers must be restored.
>
> I checked the details about the runtime PM support in the driver. To
> me, it seems there are some additional pieces missing.
>
> For a short while, let's just ignore the behaviour of the generic PM
> domain. Then I would suggest to add the following below to the driver.
> Please tell me if you think I am wrong, I don't no much about ALSA and HDMI. :-)
>
> 1) Add a runtime PM suspend callback to:
> - Save register context.
> - Enable wakeup IRQ if "wakeup IRQ capabilities" is set.
>
> 2) Add a runtime PM resume callback to:
> - Restore register context.
> - Disable wakeup IRQ.
>
> 3) Add a system PM suspend callback to:
> - Disable "wakeup IRQ capabilities".
> - Put the device into low power state.
> Could likely be done by:
> pm_runtime_get_sync();
> "disable wakeup irq cap";
> pm_runtime_put();
> pm_runtime_force_suspend();
>
> 4) Add a system PM resume callback to:
> - Enable "wakeup IRQ capabilities".
> - Restore power to the device.
> Could likely be done by:
> "enable wakeup irq_cap";
> pm_runtime_force_resume();
Thanks, those changes all look OK to me.
Unfortunately I do not have documentation for the HDMI hardware block,
and I am not in the position to do proper regression testing (the driver
doesn't really work on the hardware I do have), so I don't plan to implement
these changes.
> 5) Currently the ->probe() method invokes pm_runtime_get_sync(),
> causing the runtime PM resume callbacks to be invoked in this path.
> Unless I am missing a point, this will cause the device stay runtime
> PM resumed all the time. Is that really what you want?
That's correct. Right now the device is never runtime-suspended.
Given my above reasoning, I think that's fine for now. I just wanted to fix
the most severe issue (a system lock-up).
> In this context, and what I am trying to understand, is what changes
> are needed to the generic PM domain, such it can be used under these
> circumstances.
It seems no changes are needed to the generic PM domain.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
From: Vinod Koul @ 2014-09-24 16:26 UTC (permalink / raw)
To: linux-kernel
Cc: linux-wireless, Baruch Siach, patches, linux-doc, Seungwon Jeon,
Daniel Vetter, Chris Ball, dri-devel, Marcin Wojtas,
Rafael J. Wysocki, Laurent Pinchart, David Herrmann,
Shinya Kuribayashi, Laurent Pinchart, vinod.koul, Jingoo Han,
Jaehoon Chung, Jani Nikula, Tomi Valkeinen, Alan Stern,
Ben Skeggs, Bjorn Andersson, Wolfram Sang
This patch series adds a simple macro pm_runtime_last_busy_and_autosuspend()
which invokes pm_runtime_mark_last_busy() and pm_runtime_put_autosuspend()
sequentially. Then we do a tree wide update of current patterns which are
present. As evident from log below this pattern is frequent in the
kernel.
This series can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git
topic/pm_runtime_last_busy_and_autosuspend
Fengguang's kbuild has tested it so it shouldn't break things for anyone.
Barring one patch (explictyly mentioned in its changelog) rest are simple
replacements.
If all are okay, this should be merged thru PM tree as it depends on macro
addition.
Subhransu S. Prusty (1):
PM: Add helper pm_runtime_last_busy_and_autosuspend()
Vinod Koul (26):
dmaengine: ste_dma: use pm_runtime_last_busy_and_autosuspend helper
extcon: arizona: use pm_runtime_last_busy_and_autosuspend helper
drm/i915: use pm_runtime_last_busy_and_autosuspend helper
drm/nouveau: use pm_runtime_last_busy_and_autosuspend helper
drm/radeon: use pm_runtime_last_busy_and_autosuspend helper
vga_switcheroo: use pm_runtime_last_busy_and_autosuspend helper
i2c: designware: use pm_runtime_last_busy_and_autosuspend helper
i2c: omap: use pm_runtime_last_busy_and_autosuspend helper
i2c: qup: use pm_runtime_last_busy_and_autosuspend helper
mfd: ab8500-gpadc: use pm_runtime_last_busy_and_autosuspend helper
mfd: arizona: use pm_runtime_last_busy_and_autosuspend helper
mei: use pm_runtime_last_busy_and_autosuspend helper
mmc: use pm_runtime_last_busy_and_autosuspend helper
mmc: mmci: use pm_runtime_last_busy_and_autosuspend helper
mmc: omap_hsmmc: use pm_runtime_last_busy_and_autosuspend helper
mmc: sdhci-pxav3: use pm_runtime_last_busy_and_autosuspend helper
mmc: sdhci: use pm_runtime_last_busy_and_autosuspend helper
NFC: trf7970a: use pm_runtime_last_busy_and_autosuspend helper
pm2301-charger: use pm_runtime_last_busy_and_autosuspend helper
spi: omap2-mcspi: use pm_runtime_last_busy_and_autosuspend helper
spi: orion: use pm_runtime_last_busy_and_autosuspend helper
spi: ti-qspi: use pm_runtime_last_busy_and_autosuspend helper
spi: core: use pm_runtime_last_busy_and_autosuspend helper
tty: serial: omap: use pm_runtime_last_busy_and_autosuspend helper
usb: musb: omap2430: use pm_runtime_last_busy_and_autosuspend helper
video: fbdev: use pm_runtime_last_busy_and_autosuspend helper
Documentation/power/runtime_pm.txt | 4 ++
drivers/dma/ste_dma40.c | 30 ++++---------
drivers/extcon/extcon-arizona.c | 6 +--
drivers/gpu/drm/i915/intel_pm.c | 3 +-
drivers/gpu/drm/nouveau/nouveau_connector.c | 3 +-
drivers/gpu/drm/nouveau/nouveau_drm.c | 9 +---
drivers/gpu/drm/radeon/radeon_connectors.c | 15 ++----
drivers/gpu/drm/radeon/radeon_drv.c | 5 +-
drivers/gpu/drm/radeon/radeon_kms.c | 6 +--
drivers/gpu/vga/vga_switcheroo.c | 7 +--
drivers/i2c/busses/i2c-designware-core.c | 3 +-
drivers/i2c/busses/i2c-omap.c | 6 +--
drivers/i2c/busses/i2c-qup.c | 3 +-
drivers/mfd/ab8500-gpadc.c | 6 +--
drivers/mfd/arizona-irq.c | 3 +-
drivers/misc/mei/client.c | 12 ++----
drivers/mmc/core/core.c | 3 +-
drivers/mmc/host/mmci.c | 12 ++----
drivers/mmc/host/omap_hsmmc.c | 19 ++-------
drivers/mmc/host/sdhci-pxav3.c | 6 +--
drivers/mmc/host/sdhci.c | 3 +-
drivers/nfc/trf7970a.c | 3 +-
drivers/power/pm2301_charger.c | 3 +-
drivers/spi/spi-omap2-mcspi.c | 9 +---
drivers/spi/spi-orion.c | 3 +-
drivers/spi/spi-ti-qspi.c | 5 +-
drivers/spi/spi.c | 6 +--
drivers/tty/serial/omap-serial.c | 60 +++++++++------------------
drivers/usb/musb/omap2430.c | 6 +--
drivers/video/fbdev/auo_k190x.c | 9 +---
include/linux/pm_runtime.h | 6 +++
31 files changed, 97 insertions(+), 177 deletions(-)
Thanks
--
~Vinod
^ permalink raw reply
* [PATCH 27/27] video: fbdev: use pm_runtime_last_busy_and_autosuspend helper
From: Vinod Koul @ 2014-09-24 16:27 UTC (permalink / raw)
To: linux-kernel
Cc: subhransu.s.prusty, vinod.koul, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, Jingoo Han, Laurent Pinchart, Rob Clark,
Daniel Vetter, linux-fbdev
In-Reply-To: <1411575342-31048-1-git-send-email-vinod.koul@intel.com>
Use the new pm_runtime_last_busy_and_autosuspend helper instead of open
coding the same code
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
drivers/video/fbdev/auo_k190x.c | 9 +++------
1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/video/fbdev/auo_k190x.c b/drivers/video/fbdev/auo_k190x.c
index 8d2499d..e12f513 100644
--- a/drivers/video/fbdev/auo_k190x.c
+++ b/drivers/video/fbdev/auo_k190x.c
@@ -354,8 +354,7 @@ static void auok190xfb_dpy_deferred_io(struct fb_info *info,
par->update_partial(par, y1, min((u16) (y1 + h), yres));
out:
- pm_runtime_mark_last_busy(info->device);
- pm_runtime_put_autosuspend(info->device);
+ pm_runtime_last_busy_and_autosuspend(info->device);
}
/*
@@ -591,8 +590,7 @@ static int auok190x_read_temperature(struct auok190xfb_par *par)
mutex_unlock(&(par->io_lock));
- pm_runtime_mark_last_busy(dev);
- pm_runtime_put_autosuspend(dev);
+ pm_runtime_last_busy_and_autosuspend(dev);
/* sanitize and split of half-degrees for now */
temp = ((data[0] & AUOK190X_VERSION_TEMP_MASK) >> 1);
@@ -630,8 +628,7 @@ static void auok190x_identify(struct auok190xfb_par *par)
par->panel_size_int, par->panel_size_float, par->panel_model,
par->epd_type, par->tcon_version, par->lut_version);
- pm_runtime_mark_last_busy(dev);
- pm_runtime_put_autosuspend(dev);
+ pm_runtime_last_busy_and_autosuspend(dev);
}
/*
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
From: Rafael J. Wysocki @ 2014-09-24 20:28 UTC (permalink / raw)
To: Vinod Koul
Cc: linux-kernel, subhransu.s.prusty, Alan Stern, Alexandre Courbot,
Andrew Morton, Andy Gross, Baruch Siach, Ben Skeggs,
Bjorn Andersson, Chew, Chiau Ee, Chris Ball, Dan Carpenter,
Daniel Vetter, Dave Airlie, David Herrmann, dmaengine, dri-devel,
Du, Wenkai, Grant Grundler, Ilia Mirkin, intel-gfx,
Ivan T. Ivanov, Jaehoon Chung, Jani Nikula <jani.nikul>
In-Reply-To: <1411575342-31048-1-git-send-email-vinod.koul@intel.com>
On Wednesday, September 24, 2014 09:44:50 PM Vinod Koul wrote:
> This patch series adds a simple macro pm_runtime_last_busy_and_autosuspend()
> which invokes pm_runtime_mark_last_busy() and pm_runtime_put_autosuspend()
> sequentially. Then we do a tree wide update of current patterns which are
> present. As evident from log below this pattern is frequent in the
> kernel.
>
> This series can be found at
> git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git
> topic/pm_runtime_last_busy_and_autosuspend
>
> Fengguang's kbuild has tested it so it shouldn't break things for anyone.
> Barring one patch (explictyly mentioned in its changelog) rest are simple
> replacements.
>
> If all are okay, this should be merged thru PM tree as it depends on macro
> addition.
>
> Subhransu S. Prusty (1):
> PM: Add helper pm_runtime_last_busy_and_autosuspend()
>
> Vinod Koul (26):
> dmaengine: ste_dma: use pm_runtime_last_busy_and_autosuspend helper
> extcon: arizona: use pm_runtime_last_busy_and_autosuspend helper
> drm/i915: use pm_runtime_last_busy_and_autosuspend helper
> drm/nouveau: use pm_runtime_last_busy_and_autosuspend helper
> drm/radeon: use pm_runtime_last_busy_and_autosuspend helper
> vga_switcheroo: use pm_runtime_last_busy_and_autosuspend helper
> i2c: designware: use pm_runtime_last_busy_and_autosuspend helper
> i2c: omap: use pm_runtime_last_busy_and_autosuspend helper
> i2c: qup: use pm_runtime_last_busy_and_autosuspend helper
> mfd: ab8500-gpadc: use pm_runtime_last_busy_and_autosuspend helper
> mfd: arizona: use pm_runtime_last_busy_and_autosuspend helper
> mei: use pm_runtime_last_busy_and_autosuspend helper
> mmc: use pm_runtime_last_busy_and_autosuspend helper
> mmc: mmci: use pm_runtime_last_busy_and_autosuspend helper
> mmc: omap_hsmmc: use pm_runtime_last_busy_and_autosuspend helper
> mmc: sdhci-pxav3: use pm_runtime_last_busy_and_autosuspend helper
> mmc: sdhci: use pm_runtime_last_busy_and_autosuspend helper
> NFC: trf7970a: use pm_runtime_last_busy_and_autosuspend helper
> pm2301-charger: use pm_runtime_last_busy_and_autosuspend helper
> spi: omap2-mcspi: use pm_runtime_last_busy_and_autosuspend helper
> spi: orion: use pm_runtime_last_busy_and_autosuspend helper
> spi: ti-qspi: use pm_runtime_last_busy_and_autosuspend helper
> spi: core: use pm_runtime_last_busy_and_autosuspend helper
> tty: serial: omap: use pm_runtime_last_busy_and_autosuspend helper
> usb: musb: omap2430: use pm_runtime_last_busy_and_autosuspend helper
> video: fbdev: use pm_runtime_last_busy_and_autosuspend helper
>
> Documentation/power/runtime_pm.txt | 4 ++
> drivers/dma/ste_dma40.c | 30 ++++---------
> drivers/extcon/extcon-arizona.c | 6 +--
> drivers/gpu/drm/i915/intel_pm.c | 3 +-
> drivers/gpu/drm/nouveau/nouveau_connector.c | 3 +-
> drivers/gpu/drm/nouveau/nouveau_drm.c | 9 +---
> drivers/gpu/drm/radeon/radeon_connectors.c | 15 ++----
> drivers/gpu/drm/radeon/radeon_drv.c | 5 +-
> drivers/gpu/drm/radeon/radeon_kms.c | 6 +--
> drivers/gpu/vga/vga_switcheroo.c | 7 +--
> drivers/i2c/busses/i2c-designware-core.c | 3 +-
> drivers/i2c/busses/i2c-omap.c | 6 +--
> drivers/i2c/busses/i2c-qup.c | 3 +-
> drivers/mfd/ab8500-gpadc.c | 6 +--
> drivers/mfd/arizona-irq.c | 3 +-
> drivers/misc/mei/client.c | 12 ++----
> drivers/mmc/core/core.c | 3 +-
> drivers/mmc/host/mmci.c | 12 ++----
> drivers/mmc/host/omap_hsmmc.c | 19 ++-------
> drivers/mmc/host/sdhci-pxav3.c | 6 +--
> drivers/mmc/host/sdhci.c | 3 +-
> drivers/nfc/trf7970a.c | 3 +-
> drivers/power/pm2301_charger.c | 3 +-
> drivers/spi/spi-omap2-mcspi.c | 9 +---
> drivers/spi/spi-orion.c | 3 +-
> drivers/spi/spi-ti-qspi.c | 5 +-
> drivers/spi/spi.c | 6 +--
> drivers/tty/serial/omap-serial.c | 60 +++++++++------------------
> drivers/usb/musb/omap2430.c | 6 +--
> drivers/video/fbdev/auo_k190x.c | 9 +---
> include/linux/pm_runtime.h | 6 +++
> 31 files changed, 97 insertions(+), 177 deletions(-)
OK, I guess this is as good as it gets.
What tree would you like it go through?
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
From: Vinod Koul @ 2014-09-25 7:58 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-wireless, Baruch Siach, patches, linux-doc, Seungwon Jeon,
Daniel Vetter, Chris Ball, dri-devel, Marcin Wojtas,
Rafael J. Wysocki, Laurent Pinchart, David Herrmann,
Shinya Kuribayashi, Laurent Pinchart, Jingoo Han, Jaehoon Chung,
Jani Nikula, Tomi Valkeinen, Alan Stern, Ben Skeggs,
Bjorn Andersson, Wolfram Sang, Dave Airlie
In-Reply-To: <6374313.I12loOK9hA@vostro.rjw.lan>
On Wed, Sep 24, 2014 at 10:28:07PM +0200, Rafael J. Wysocki wrote:
>
> OK, I guess this is as good as it gets.
>
> What tree would you like it go through?
Since rest of the patches are dependent upon 1st patch which should go thru
your tree, we should merge this thru your tree
Thanks
--
~Vinod
^ permalink raw reply
* Re: [PATCH] drm/exynos/fbdev: set smem_len for fbdev
From: Geert Uytterhoeven @ 2014-09-25 9:32 UTC (permalink / raw)
To: Daniel Kurtz
Cc: Linux Fbdev development list, Seung-Woo Kim, Kyungmin Park,
DRI Development, linux-kernel@vger.kernel.org, Günther Noack
In-Reply-To: <1408891857-14258-1-git-send-email-djkurtz@chromium.org>
On Sun, Aug 24, 2014 at 4:50 PM, Daniel Kurtz <djkurtz@chromium.org> wrote:
> Commit [0] stopped setting fix.smem_start and fix.smem_len when creating
> the fbdev.
>
> [0] 2f1eab8d8ab59e799f7d51d62410b398607a7bc3
> drm/exynos/fbdev: don't set fix.smem/mmio_{start,len}
>
> However, smem_len is used by some userland applications to calculate the
> size for mmap. In particular, it is used by xf86-video-fbdev:
>
> http://cgit.freedesktop.org/xorg/xserver/tree/hw/xfree86/fbdevhw/fbdevhw.c?id=xorg-server-1.15.99.903#n571
>
> So, let's restore setting the smem_len to unbreak things for these users.
>
> Note: we are still leaving smem_start set to 0.
Doesn't this cause a system crash when userspace (e.g. fbtest) writes into
the mmap()ed /dev/fb*, as the wrong MMIO region is mapped?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] drm/exynos/fbdev: set smem_len for fbdev
From: Daniel Kurtz @ 2014-09-25 10:07 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linux Fbdev development list, Seung-Woo Kim, Kyungmin Park,
DRI Development, linux-kernel@vger.kernel.org, Günther Noack
In-Reply-To: <CAMuHMdUoh6+gDcZty76HJhcQcJvM5DyBiD4YrZGuAX9e+F32-w@mail.gmail.com>
On Thu, Sep 25, 2014 at 5:32 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Sun, Aug 24, 2014 at 4:50 PM, Daniel Kurtz <djkurtz@chromium.org> wrote:
>> Commit [0] stopped setting fix.smem_start and fix.smem_len when creating
>> the fbdev.
>>
>> [0] 2f1eab8d8ab59e799f7d51d62410b398607a7bc3
>> drm/exynos/fbdev: don't set fix.smem/mmio_{start,len}
>>
>> However, smem_len is used by some userland applications to calculate the
>> size for mmap. In particular, it is used by xf86-video-fbdev:
>>
>> http://cgit.freedesktop.org/xorg/xserver/tree/hw/xfree86/fbdevhw/fbdevhw.c?id=xorg-server-1.15.99.903#n571
>>
>> So, let's restore setting the smem_len to unbreak things for these users.
>>
>> Note: we are still leaving smem_start set to 0.
>
> Doesn't this cause a system crash when userspace (e.g. fbtest) writes into
> the mmap()ed /dev/fb*, as the wrong MMIO region is mapped?
Do you see a crash during testing, or is your question hypothetical?
I don't think there will be one. exynos's fbev defines its own
fb_ops.fb_mmap, which uses dma_mmap_attrs() to mmap the fb's gem
buffer, which was allocated by dma_alloc_attrs(). This bypasses the
code in drivers/video/fbmem.c:fb_mmap() that references
fix.smem_start.
But, perhaps I am missing something?
-Dan
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] drm/exynos/fbdev: set smem_len for fbdev
From: Geert Uytterhoeven @ 2014-09-25 10:14 UTC (permalink / raw)
To: Daniel Kurtz
Cc: Linux Fbdev development list, Seung-Woo Kim, Kyungmin Park,
DRI Development, linux-kernel@vger.kernel.org, Günther Noack
In-Reply-To: <CAGS+omD055Bk+czk5=SDMRu1g-HGZHBobgasKC9PjF80Ci-czA@mail.gmail.com>
On Thu, Sep 25, 2014 at 12:07 PM, Daniel Kurtz <djkurtz@chromium.org> wrote:
> On Thu, Sep 25, 2014 at 5:32 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Sun, Aug 24, 2014 at 4:50 PM, Daniel Kurtz <djkurtz@chromium.org> wrote:
>>> Commit [0] stopped setting fix.smem_start and fix.smem_len when creating
>>> the fbdev.
>>>
>>> [0] 2f1eab8d8ab59e799f7d51d62410b398607a7bc3
>>> drm/exynos/fbdev: don't set fix.smem/mmio_{start,len}
>>>
>>> However, smem_len is used by some userland applications to calculate the
>>> size for mmap. In particular, it is used by xf86-video-fbdev:
>>>
>>> http://cgit.freedesktop.org/xorg/xserver/tree/hw/xfree86/fbdevhw/fbdevhw.c?id=xorg-server-1.15.99.903#n571
>>>
>>> So, let's restore setting the smem_len to unbreak things for these users.
>>>
>>> Note: we are still leaving smem_start set to 0.
>>
>> Doesn't this cause a system crash when userspace (e.g. fbtest) writes into
>> the mmap()ed /dev/fb*, as the wrong MMIO region is mapped?
>
> Do you see a crash during testing, or is your question hypothetical?
It was hypothetical.
> I don't think there will be one. exynos's fbev defines its own
> fb_ops.fb_mmap, which uses dma_mmap_attrs() to mmap the fb's gem
> buffer, which was allocated by dma_alloc_attrs(). This bypasses the
> code in drivers/video/fbmem.c:fb_mmap() that references
> fix.smem_start.
>
> But, perhaps I am missing something?
Thanks, having your own mapping function explains the lack of crashes.
Still, some weird software may map /dev/mem instead of /dev/fb0, and
use fb_fix.smem_start. But that's also unsafe in the context of LPAE.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
From: Rafael J. Wysocki @ 2014-09-25 19:54 UTC (permalink / raw)
To: Wolfram Sang, Vinod Koul
Cc: linux-wireless, Baruch Siach, patches, linux-doc, Seungwon Jeon,
Daniel Vetter, Chris Ball, dri-devel, Marcin Wojtas,
Rafael J. Wysocki, Laurent Pinchart, David Herrmann,
Shinya Kuribayashi, Laurent Pinchart, Jingoo Han, Jaehoon Chung,
Jani Nikula, Tomi Valkeinen, Alan Stern, Ben Skeggs,
Bjorn Andersson, Dave Airlie, Ulf Hansson <ulf.hanss>
In-Reply-To: <20140925142758.GB1316@katana>
On Thursday, September 25, 2014 04:27:58 PM Wolfram Sang wrote:
>
> --Bn2rw/3z4jIqBvZU
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> Content-Transfer-Encoding: quoted-printable
>
> On Thu, Sep 25, 2014 at 09:22:01AM -0500, Felipe Balbi wrote:
> > On Thu, Sep 25, 2014 at 01:27:18PM +0530, Vinod Koul wrote:
> > > On Wed, Sep 24, 2014 at 03:32:19PM -0500, Felipe Balbi wrote:
> > > > > > > OK, I guess this is as good as it gets.
> > > > > > >
> > > > > > > What tree would you like it go through?
> > > > > >
> > > > > > Do we really need this new helper ? I mean, the very moment when > we
> > > > > > decide to implement ->runtime_idle() we will need to get rid of t> his
> > > > > > change. I wonder if it's really valid...
> > > > >
> > > > > I'm not sure I'm following? This seems to simply implement what dr> ivers
> > > > > have been doing already as one function. Why would it be invalid t> o reduce
> > > > > code duplication?
> > > >
> > > > For two reasons:
> > > >
> > > > 1) the helper has no inteligence whatsoever. It just calls the same
> > > > functions.
> > > >
> > > > 2) the duplication will vanish whenever someone implements
> > > > ->runtime_idle() and have that call pm_runtime_autosuspend() (like PCI
> > > > and USB buses are doing today). This will just be yet another line th> at
> > > > needs to change.
> > > >
> > > > Frankly though, no strong feelings, I just think it's a commit that
> > > > doesn't bring that any benefits other than looking like one line was
> > > > removed.
> > > and yes that is what it tries to do nothing more nothing less. If in fu> ture
> > > there are no users (today we have quite a few), then we can remove the > dead
> > > macro, no harm. But that is not the situation today.
> >
> > as I said, a commit that's bound to be useless. It's not like you're
> > saving 10 lines of code, it's only one. Replacing two simple lines with
> > a function which takes <joke> almost as many characters to type </joke>.
> >
> > IMO, this is pretty useless and I'd rather not see them in the drivers I
> > maintain, sorry.
>
> It is not a NACK from me; yet from a high-level perspective I agree with
> Felipe.
OK
I'd rather not merge something that driver people don't want to use.
Vinod?
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* [PATCH 0/2] LLVMLinux: Patches to enable the kernel to be compiled with clang/LLVM
From: Behan Webster @ 2014-09-27 1:10 UTC (permalink / raw)
To: plagnioj, tomi.valkeinen
Cc: behanw, archit, daniel.vetter, geert, laurent.pinchart,
linux-fbdev, linux-kernel, linux-omap, robdclark
Replace the use of nested functions where a normal function will suffice.
Nested functions are not liked by upstream kernel developers in general. Their
use breaks the use of clang as a compiler, and doesn't make the code any
better.
This code now works for both gcc and clang.
The LLVMLinux project aims to fully build the Linux kernel using both gcc and
clang (the C front end for the LLVM compiler infrastructure project).
Behan Webster (2):
arm, fbdev, omap2, LLVMLinux: Remove nested function from omap2 dss
arm, fbdev, omap2, LLVMLinux: Remove nested function from omapfb
drivers/video/fbdev/omap2/dss/dispc-compat.c | 9 +++++----
drivers/video/fbdev/omap2/dss/manager-sysfs.c | 16 +++++++++-------
drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 14 +++++++-------
3 files changed, 21 insertions(+), 18 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH 1/2] arm, fbdev, omap2, LLVMLinux: Remove nested function from omap2 dss
From: Behan Webster @ 2014-09-27 1:10 UTC (permalink / raw)
To: plagnioj, tomi.valkeinen
Cc: behanw, archit, daniel.vetter, geert, laurent.pinchart,
linux-fbdev, linux-kernel, linux-omap, robdclark, Arnd Bergmann
In-Reply-To: <1411780253-4492-1-git-send-email-behanw@converseincode.com>
Replace the use of nested functions where a normal function will suffice.
Nested functions are not liked by upstream kernel developers in general. Their
use breaks the use of clang as a compiler, and doesn't make the code any
better.
This code now works for both gcc and clang.
Signed-off-by: Behan Webster <behanw@converseincode.com>
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Cc: Arnd Bergmann <arnd@arndb.de>
---
drivers/video/fbdev/omap2/dss/dispc-compat.c | 9 +++++----
drivers/video/fbdev/omap2/dss/manager-sysfs.c | 16 +++++++++-------
2 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/video/fbdev/omap2/dss/dispc-compat.c b/drivers/video/fbdev/omap2/dss/dispc-compat.c
index 83779c2..633c461 100644
--- a/drivers/video/fbdev/omap2/dss/dispc-compat.c
+++ b/drivers/video/fbdev/omap2/dss/dispc-compat.c
@@ -634,13 +634,14 @@ void dispc_mgr_disable_sync(enum omap_channel channel)
WARN_ON(1);
}
+static inline void dispc_irq_wait_handler(void *data, u32 mask)
+{
+ complete((struct completion *)data);
+}
+
int omap_dispc_wait_for_irq_interruptible_timeout(u32 irqmask,
unsigned long timeout)
{
- void dispc_irq_wait_handler(void *data, u32 mask)
- {
- complete((struct completion *)data);
- }
int r;
DECLARE_COMPLETION_ONSTACK(completion);
diff --git a/drivers/video/fbdev/omap2/dss/manager-sysfs.c b/drivers/video/fbdev/omap2/dss/manager-sysfs.c
index 37b59fe..a7414fb 100644
--- a/drivers/video/fbdev/omap2/dss/manager-sysfs.c
+++ b/drivers/video/fbdev/omap2/dss/manager-sysfs.c
@@ -44,6 +44,13 @@ static ssize_t manager_display_show(struct omap_overlay_manager *mgr, char *buf)
dssdev->name : "<none>");
}
+static int manager_display_match(struct omap_dss_device *dssdev, void *data)
+{
+ const char *str = data;
+
+ return sysfs_streq(dssdev->name, str);
+}
+
static ssize_t manager_display_store(struct omap_overlay_manager *mgr,
const char *buf, size_t size)
{
@@ -52,17 +59,12 @@ static ssize_t manager_display_store(struct omap_overlay_manager *mgr,
struct omap_dss_device *dssdev = NULL;
struct omap_dss_device *old_dssdev;
- int match(struct omap_dss_device *dssdev, void *data)
- {
- const char *str = data;
- return sysfs_streq(dssdev->name, str);
- }
-
if (buf[size-1] = '\n')
--len;
if (len > 0)
- dssdev = omap_dss_find_device((void *)buf, match);
+ dssdev = omap_dss_find_device((void *)buf,
+ manager_display_match);
if (len > 0 && dssdev = NULL)
return -EINVAL;
--
1.9.1
^ permalink raw reply related
* [PATCH 2/2] arm, fbdev, omap2, LLVMLinux: Remove nested function from omapfb
From: Behan Webster @ 2014-09-27 1:10 UTC (permalink / raw)
To: plagnioj, tomi.valkeinen
Cc: behanw, archit, daniel.vetter, geert, laurent.pinchart,
linux-fbdev, linux-kernel, linux-omap, robdclark, Arnd Bergmann
In-Reply-To: <1411780253-4492-1-git-send-email-behanw@converseincode.com>
Replace the use of nested functions where a normal function will suffice.
Nested functions are not liked by upstream kernel developers in general. Their
use breaks the use of clang as a compiler, and doesn't make the code any
better.
This code now works for both gcc and clang.
Signed-off-by: Behan Webster <behanw@converseincode.com>
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Cc: Arnd Bergmann <arnd@arndb.de>
---
drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
index ec2d132..1587243 100644
--- a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
+++ b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
@@ -273,16 +273,16 @@ static struct omapfb_colormode omapfb_colormodes[] = {
},
};
+static bool cmp_component(struct fb_bitfield *f1, struct fb_bitfield *f2)
+{
+ return f1->length = f2->length &&
+ f1->offset = f2->offset &&
+ f1->msb_right = f2->msb_right;
+}
+
static bool cmp_var_to_colormode(struct fb_var_screeninfo *var,
struct omapfb_colormode *color)
{
- bool cmp_component(struct fb_bitfield *f1, struct fb_bitfield *f2)
- {
- return f1->length = f2->length &&
- f1->offset = f2->offset &&
- f1->msb_right = f2->msb_right;
- }
-
if (var->bits_per_pixel = 0 ||
var->red.length = 0 ||
var->blue.length = 0 ||
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 1/2] arm, fbdev, omap2, LLVMLinux: Remove nested function from omap2 dss
From: Felipe Balbi @ 2014-09-27 16:46 UTC (permalink / raw)
To: Behan Webster
Cc: plagnioj, tomi.valkeinen, daniel.vetter, geert, laurent.pinchart,
linux-fbdev, linux-kernel, linux-omap, robdclark, Arnd Bergmann
In-Reply-To: <1411780253-4492-2-git-send-email-behanw@converseincode.com>
[-- Attachment #1: Type: text/plain, Size: 664 bytes --]
On Fri, Sep 26, 2014 at 06:10:52PM -0700, Behan Webster wrote:
> Replace the use of nested functions where a normal function will suffice.
>
> Nested functions are not liked by upstream kernel developers in general. Their
> use breaks the use of clang as a compiler, and doesn't make the code any
> better.
>
> This code now works for both gcc and clang.
>
> Signed-off-by: Behan Webster <behanw@converseincode.com>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
another one that make sense :-) And probably also deserves
checkpatch/coccinelle/sparse.
Reviewed-by: Felipe Balbi <balbi@ti.com>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] arm, fbdev, omap2, LLVMLinux: Remove nested function from omapfb
From: Felipe Balbi @ 2014-09-27 16:46 UTC (permalink / raw)
To: Behan Webster
Cc: plagnioj, tomi.valkeinen, daniel.vetter, geert, laurent.pinchart,
linux-fbdev, linux-kernel, linux-omap, robdclark, Arnd Bergmann
In-Reply-To: <1411780253-4492-3-git-send-email-behanw@converseincode.com>
[-- Attachment #1: Type: text/plain, Size: 571 bytes --]
On Fri, Sep 26, 2014 at 06:10:53PM -0700, Behan Webster wrote:
> Replace the use of nested functions where a normal function will suffice.
>
> Nested functions are not liked by upstream kernel developers in general. Their
> use breaks the use of clang as a compiler, and doesn't make the code any
> better.
>
> This code now works for both gcc and clang.
>
> Signed-off-by: Behan Webster <behanw@converseincode.com>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Felipe Balbi <balbi@ti.com>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 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