| Message ID | 1424898082-1522-3-git-send-email-arun.ramamurthy@broadcom.com |
|---|---|
| State | Needs Review / ACK, archived |
| Headers | show |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 1 errors, 0 warnings, 0 lines checked |
| robh/patch-applied | success |
On Wed, 2015-02-25 at 21:01 +0000, Arun Ramamurthy wrote: > Added ioctl and interrupt handler functions to support FBIO_WAITFORVSYNC > Also corrected documentation to make interrupts and interrupt-names > optional as they are not required properties. You may not be aware of this fact, but its the "documentation" what defines what properties are required... > Reviewed-by: Ray Jui <rjui@broadcom.com> > Reviewed-by: Scott Branden <sbranden@broadcom.com> > Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>0 > --- > .../devicetree/bindings/video/arm,pl11x.txt | 11 +-- > drivers/video/fbdev/amba-clcd.c | 82 ++++++++++++++++++++++ > include/linux/amba/clcd.h | 4 ++ > 3 files changed, 89 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/video/arm,pl11x.txt b/Documentation/devicetree/bindings/video/arm,pl11x.txt > index 2262cdb..7d19024 100644 > --- a/Documentation/devicetree/bindings/video/arm,pl11x.txt > +++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt > @@ -10,14 +10,6 @@ Required properties: > > - reg: base address and size of the control registers block > > -- interrupt-names: either the single entry "combined" representing a > - combined interrupt output (CLCDINTR), or the four entries > - "mbe", "vcomp", "lnbu", "fuf" representing the individual > - CLCDMBEINTR, CLCDVCOMPINTR, CLCDLNBUINTR, CLCDFUFINTR interrupts > - > -- interrupts: contains an interrupt specifier for each entry in > - interrupt-names > - > - clock-names: should contain "clcdclk" and "apb_pclk" > > - clocks: contains phandle and clock specifier pairs for the entries So no, you can't do that. Pawel -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15-03-02 08:00 AM, Pawel Moll wrote: > On Wed, 2015-02-25 at 21:01 +0000, Arun Ramamurthy wrote: >> Added ioctl and interrupt handler functions to support FBIO_WAITFORVSYNC >> Also corrected documentation to make interrupts and interrupt-names >> optional as they are not required properties. > > You may not be aware of this fact, but its the "documentation" what > defines what properties are required... > Pawel, I was not aware of that. Since the driver code did not require the interrupts or interrupt-names bindings to load properly, I moved it out of the required properties. I can remove that change. Any other comments on this change? Thanks >> Reviewed-by: Ray Jui <rjui@broadcom.com> >> Reviewed-by: Scott Branden <sbranden@broadcom.com> >> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>0 >> --- >> .../devicetree/bindings/video/arm,pl11x.txt | 11 +-- >> drivers/video/fbdev/amba-clcd.c | 82 ++++++++++++++++++++++ >> include/linux/amba/clcd.h | 4 ++ >> 3 files changed, 89 insertions(+), 8 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/video/arm,pl11x.txt b/Documentation/devicetree/bindings/video/arm,pl11x.txt >> index 2262cdb..7d19024 100644 >> --- a/Documentation/devicetree/bindings/video/arm,pl11x.txt >> +++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt >> @@ -10,14 +10,6 @@ Required properties: >> >> - reg: base address and size of the control registers block >> >> -- interrupt-names: either the single entry "combined" representing a >> - combined interrupt output (CLCDINTR), or the four entries >> - "mbe", "vcomp", "lnbu", "fuf" representing the individual >> - CLCDMBEINTR, CLCDVCOMPINTR, CLCDLNBUINTR, CLCDFUFINTR interrupts >> - >> -- interrupts: contains an interrupt specifier for each entry in >> - interrupt-names >> - >> - clock-names: should contain "clcdclk" and "apb_pclk" >> >> - clocks: contains phandle and clock specifier pairs for the entries > > So no, you can't do that. > > Pawel > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 2, 2015 at 10:00 AM, Pawel Moll <pawel.moll@arm.com> wrote: > On Wed, 2015-02-25 at 21:01 +0000, Arun Ramamurthy wrote: >> Added ioctl and interrupt handler functions to support FBIO_WAITFORVSYNC >> Also corrected documentation to make interrupts and interrupt-names >> optional as they are not required properties. > > You may not be aware of this fact, but its the "documentation" what > defines what properties are required... Except when docs are wrong. Then dts files win. >> Reviewed-by: Ray Jui <rjui@broadcom.com> >> Reviewed-by: Scott Branden <sbranden@broadcom.com> >> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>0 >> --- >> .../devicetree/bindings/video/arm,pl11x.txt | 11 +-- >> drivers/video/fbdev/amba-clcd.c | 82 ++++++++++++++++++++++ >> include/linux/amba/clcd.h | 4 ++ >> 3 files changed, 89 insertions(+), 8 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/video/arm,pl11x.txt b/Documentation/devicetree/bindings/video/arm,pl11x.txt >> index 2262cdb..7d19024 100644 >> --- a/Documentation/devicetree/bindings/video/arm,pl11x.txt >> +++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt >> @@ -10,14 +10,6 @@ Required properties: >> >> - reg: base address and size of the control registers block >> >> -- interrupt-names: either the single entry "combined" representing a >> - combined interrupt output (CLCDINTR), or the four entries >> - "mbe", "vcomp", "lnbu", "fuf" representing the individual >> - CLCDMBEINTR, CLCDVCOMPINTR, CLCDLNBUINTR, CLCDFUFINTR interrupts >> - >> -- interrupts: contains an interrupt specifier for each entry in >> - interrupt-names >> - >> - clock-names: should contain "clcdclk" and "apb_pclk" >> >> - clocks: contains phandle and clock specifier pairs for the entries > > So no, you can't do that. You can't do the other way around (making optional ones required), but I think this is okay if the h/w interrupt lines are not physically connected. However, if it is simply because the driver doesn't use them, then I agree this should not be changed. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 25, 2015 at 3:01 PM, Arun Ramamurthy <arun.ramamurthy@broadcom.com> wrote: > Added ioctl and interrupt handler functions to support FBIO_WAITFORVSYNC > Also corrected documentation to make interrupts and interrupt-names > optional as they are not required properties. > > Reviewed-by: Ray Jui <rjui@broadcom.com> > Reviewed-by: Scott Branden <sbranden@broadcom.com> > Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>0 > --- > .../devicetree/bindings/video/arm,pl11x.txt | 11 +-- Please split bindings to separate patches. > drivers/video/fbdev/amba-clcd.c | 82 ++++++++++++++++++++++ > include/linux/amba/clcd.h | 4 ++ > 3 files changed, 89 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/video/arm,pl11x.txt b/Documentation/devicetree/bindings/video/arm,pl11x.txt > index 2262cdb..7d19024 100644 > --- a/Documentation/devicetree/bindings/video/arm,pl11x.txt > +++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt > @@ -10,14 +10,6 @@ Required properties: > > - reg: base address and size of the control registers block > > -- interrupt-names: either the single entry "combined" representing a > - combined interrupt output (CLCDINTR), or the four entries > - "mbe", "vcomp", "lnbu", "fuf" representing the individual > - CLCDMBEINTR, CLCDVCOMPINTR, CLCDLNBUINTR, CLCDFUFINTR interrupts > - > -- interrupts: contains an interrupt specifier for each entry in > - interrupt-names > - > - clock-names: should contain "clcdclk" and "apb_pclk" > > - clocks: contains phandle and clock specifier pairs for the entries > @@ -54,6 +46,9 @@ Optional properties: > It can be used to configure a virtual y resolution. It > must be a value larger than the actual y resolution. > > +- interrupts: contains an interrupt specifier for the clcd vcomp interrupt > + This is required for the driver to handle FBIO_WAITFORVSYNC ioctls. > + What happened to interrupt-names? Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2015-03-02 at 19:09 +0000, Arun Ramamurthy wrote: > On 15-03-02 08:00 AM, Pawel Moll wrote: > > On Wed, 2015-02-25 at 21:01 +0000, Arun Ramamurthy wrote: > >> Added ioctl and interrupt handler functions to support FBIO_WAITFORVSYNC > >> Also corrected documentation to make interrupts and interrupt-names > >> optional as they are not required properties. > > > > You may not be aware of this fact, but its the "documentation" what > > defines what properties are required... > > > Pawel, I was not aware of that. Since the driver code did not require > the interrupts or interrupt-names bindings to load properly, I moved it > out of the required properties. I can remove that change. Cool. Drivers don't have to use all available properties :-) The interrupt names are required because CLCD can be wired up with a single, combined interrupt or with 4 separate interrupt lines (see "A.4. On-chip signals", in the TRM) and the binding has to provide ways of describing this. Yes, one could say "1 number == combined, 4 numbers == split", but I personally prefer to be explicit than implicit. Just request the interrupt by name and you'll be fine. > Any other comments on this change? Thanks I have no experience with the vsync ioctl, so can't really comment on it. One minor thing I did spot is your use of curly brackets in one of the switch cases: On Wed, 2015-02-25 at 21:01 +0000, Arun Ramamurthy wrote: @@ -466,6 +468,73 @@ static int clcdfb_pan_display(struct fb_var_screeninfo *var, > return 0; > } > > +static int clcdfb_ioctl(struct fb_info *info, > + unsigned int cmd, unsigned long args) > +{ > + struct clcd_fb *fb = to_clcd(info); > + int retval = 0; > + u32 val, ienb_val; > + > + switch (cmd) { > + case FBIO_WAITFORVSYNC:{ In the line above... > + if (fb->lcd_irq <= 0) { > + retval = -EINVAL; > + break; > + } > + /* disable Vcomp interrupts */ > + ienb_val = readl(fb->regs + fb->off_ienb); > + ienb_val &= ~CLCD_PL111_IENB_VCOMP; > + writel(ienb_val, fb->regs + fb->off_ienb); > + > + /* clear Vcomp interrupt */ > + writel(CLCD_PL111_IENB_VCOMP, fb->regs + CLCD_PL111_ICR); > + > + /* Generate Interrupt at the start of Vsync */ > + reinit_completion(&fb->wait); > + val = readl(fb->regs + fb->off_cntl); > + val &= ~(CNTL_LCDVCOMP(3)); > + writel(val, fb->regs + fb->off_cntl); > + > + /* enable Vcomp interrupt */ > + ienb_val = readl(fb->regs + fb->off_ienb); > + ienb_val |= CLCD_PL111_IENB_VCOMP; > + writel(ienb_val, fb->regs + fb->off_ienb); > + if (!wait_for_completion_interruptible_timeout > + (&fb->wait, HZ/10)) > + retval = -ETIMEDOUT; > + break; > + } ... and here. Not sure it's needed? Pawel -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15-03-02 03:27 PM, Rob Herring wrote: > On Mon, Mar 2, 2015 at 10:00 AM, Pawel Moll <pawel.moll@arm.com> wrote: >> On Wed, 2015-02-25 at 21:01 +0000, Arun Ramamurthy wrote: >>> Added ioctl and interrupt handler functions to support FBIO_WAITFORVSYNC >>> Also corrected documentation to make interrupts and interrupt-names >>> optional as they are not required properties. >> >> You may not be aware of this fact, but its the "documentation" what >> defines what properties are required... > > Except when docs are wrong. Then dts files win. > >>> Reviewed-by: Ray Jui <rjui@broadcom.com> >>> Reviewed-by: Scott Branden <sbranden@broadcom.com> >>> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>0 >>> --- >>> .../devicetree/bindings/video/arm,pl11x.txt | 11 +-- >>> drivers/video/fbdev/amba-clcd.c | 82 ++++++++++++++++++++++ >>> include/linux/amba/clcd.h | 4 ++ >>> 3 files changed, 89 insertions(+), 8 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/video/arm,pl11x.txt b/Documentation/devicetree/bindings/video/arm,pl11x.txt >>> index 2262cdb..7d19024 100644 >>> --- a/Documentation/devicetree/bindings/video/arm,pl11x.txt >>> +++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt >>> @@ -10,14 +10,6 @@ Required properties: >>> >>> - reg: base address and size of the control registers block >>> >>> -- interrupt-names: either the single entry "combined" representing a >>> - combined interrupt output (CLCDINTR), or the four entries >>> - "mbe", "vcomp", "lnbu", "fuf" representing the individual >>> - CLCDMBEINTR, CLCDVCOMPINTR, CLCDLNBUINTR, CLCDFUFINTR interrupts >>> - >>> -- interrupts: contains an interrupt specifier for each entry in >>> - interrupt-names >>> - >>> - clock-names: should contain "clcdclk" and "apb_pclk" >>> >>> - clocks: contains phandle and clock specifier pairs for the entries >> >> So no, you can't do that. > > You can't do the other way around (making optional ones required), but > I think this is okay if the h/w interrupt lines are not physically > connected. However, if it is simply because the driver doesn't use > them, then I agree this should not be changed. > Agreed, I will undo this change in V2 > Rob > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15-03-02 03:29 PM, Rob Herring wrote: > On Wed, Feb 25, 2015 at 3:01 PM, Arun Ramamurthy > <arun.ramamurthy@broadcom.com> wrote: >> Added ioctl and interrupt handler functions to support FBIO_WAITFORVSYNC >> Also corrected documentation to make interrupts and interrupt-names >> optional as they are not required properties. >> >> Reviewed-by: Ray Jui <rjui@broadcom.com> >> Reviewed-by: Scott Branden <sbranden@broadcom.com> >> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>0 >> --- >> .../devicetree/bindings/video/arm,pl11x.txt | 11 +-- > > Please split bindings to separate patches. Rob, I looked at the history of patches to this driver and all them included changes to the documentation in the same patch so I followed a similar model. I assumed that a separate patch was only required when the documentation was being added from scratch and not when modifying a few lines. Is this not the case? > >> drivers/video/fbdev/amba-clcd.c | 82 ++++++++++++++++++++++ >> include/linux/amba/clcd.h | 4 ++ >> 3 files changed, 89 insertions(+), 8 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/video/arm,pl11x.txt b/Documentation/devicetree/bindings/video/arm,pl11x.txt >> index 2262cdb..7d19024 100644 >> --- a/Documentation/devicetree/bindings/video/arm,pl11x.txt >> +++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt >> @@ -10,14 +10,6 @@ Required properties: >> >> - reg: base address and size of the control registers block >> >> -- interrupt-names: either the single entry "combined" representing a >> - combined interrupt output (CLCDINTR), or the four entries >> - "mbe", "vcomp", "lnbu", "fuf" representing the individual >> - CLCDMBEINTR, CLCDVCOMPINTR, CLCDLNBUINTR, CLCDFUFINTR interrupts >> - >> -- interrupts: contains an interrupt specifier for each entry in >> - interrupt-names >> - >> - clock-names: should contain "clcdclk" and "apb_pclk" >> >> - clocks: contains phandle and clock specifier pairs for the entries >> @@ -54,6 +46,9 @@ Optional properties: >> It can be used to configure a virtual y resolution. It >> must be a value larger than the actual y resolution. >> >> +- interrupts: contains an interrupt specifier for the clcd vcomp interrupt >> + This is required for the driver to handle FBIO_WAITFORVSYNC ioctls. >> + > > What happened to interrupt-names? As mentioned before, these changes to the documentation will be undone in V2. Thanks > > Rob > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15-03-03 02:01 AM, Pawel Moll wrote: > On Mon, 2015-03-02 at 19:09 +0000, Arun Ramamurthy wrote: >> On 15-03-02 08:00 AM, Pawel Moll wrote: >>> On Wed, 2015-02-25 at 21:01 +0000, Arun Ramamurthy wrote: >>>> Added ioctl and interrupt handler functions to support FBIO_WAITFORVSYNC >>>> Also corrected documentation to make interrupts and interrupt-names >>>> optional as they are not required properties. >>> >>> You may not be aware of this fact, but its the "documentation" what >>> defines what properties are required... >>> >> Pawel, I was not aware of that. Since the driver code did not require >> the interrupts or interrupt-names bindings to load properly, I moved it >> out of the required properties. I can remove that change. > > Cool. Drivers don't have to use all available properties :-) The > interrupt names are required because CLCD can be wired up with a single, > combined interrupt or with 4 separate interrupt lines (see "A.4. On-chip > signals", in the TRM) and the binding has to provide ways of describing > this. Yes, one could say "1 number == combined, 4 numbers == split", but > I personally prefer to be explicit than implicit. > > Just request the interrupt by name and you'll be fine. > Ok got it, will make the necessary changes >> Any other comments on this change? Thanks > > I have no experience with the vsync ioctl, so can't really comment on > it. One minor thing I did spot is your use of curly brackets in one of > the switch cases: > > On Wed, 2015-02-25 at 21:01 +0000, Arun Ramamurthy wrote: > @@ -466,6 +468,73 @@ static int clcdfb_pan_display(struct fb_var_screeninfo *var, >> return 0; >> } >> >> +static int clcdfb_ioctl(struct fb_info *info, >> + unsigned int cmd, unsigned long args) >> +{ >> + struct clcd_fb *fb = to_clcd(info); >> + int retval = 0; >> + u32 val, ienb_val; >> + >> + switch (cmd) { >> + case FBIO_WAITFORVSYNC:{ > > In the line above... > >> + if (fb->lcd_irq <= 0) { >> + retval = -EINVAL; >> + break; >> + } >> + /* disable Vcomp interrupts */ >> + ienb_val = readl(fb->regs + fb->off_ienb); >> + ienb_val &= ~CLCD_PL111_IENB_VCOMP; >> + writel(ienb_val, fb->regs + fb->off_ienb); >> + >> + /* clear Vcomp interrupt */ >> + writel(CLCD_PL111_IENB_VCOMP, fb->regs + CLCD_PL111_ICR); >> + >> + /* Generate Interrupt at the start of Vsync */ >> + reinit_completion(&fb->wait); >> + val = readl(fb->regs + fb->off_cntl); >> + val &= ~(CNTL_LCDVCOMP(3)); >> + writel(val, fb->regs + fb->off_cntl); >> + >> + /* enable Vcomp interrupt */ >> + ienb_val = readl(fb->regs + fb->off_ienb); >> + ienb_val |= CLCD_PL111_IENB_VCOMP; >> + writel(ienb_val, fb->regs + fb->off_ienb); >> + if (!wait_for_completion_interruptible_timeout >> + (&fb->wait, HZ/10)) >> + retval = -ETIMEDOUT; >> + break; >> + } > > ... and here. > > Not sure it's needed? > No its not needed, I can remove it. > Pawel > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/video/arm,pl11x.txt b/Documentation/devicetree/bindings/video/arm,pl11x.txt index 2262cdb..7d19024 100644 --- a/Documentation/devicetree/bindings/video/arm,pl11x.txt +++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt @@ -10,14 +10,6 @@ Required properties: - reg: base address and size of the control registers block -- interrupt-names: either the single entry "combined" representing a - combined interrupt output (CLCDINTR), or the four entries - "mbe", "vcomp", "lnbu", "fuf" representing the individual - CLCDMBEINTR, CLCDVCOMPINTR, CLCDLNBUINTR, CLCDFUFINTR interrupts - -- interrupts: contains an interrupt specifier for each entry in - interrupt-names - - clock-names: should contain "clcdclk" and "apb_pclk" - clocks: contains phandle and clock specifier pairs for the entries @@ -54,6 +46,9 @@ Optional properties: It can be used to configure a virtual y resolution. It must be a value larger than the actual y resolution. +- interrupts: contains an interrupt specifier for the clcd vcomp interrupt + This is required for the driver to handle FBIO_WAITFORVSYNC ioctls. + Required sub-nodes: - port: describes LCD panel signals, following the common binding diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c index 3bc09ad..9f7a58c 100644 --- a/drivers/video/fbdev/amba-clcd.c +++ b/drivers/video/fbdev/amba-clcd.c @@ -30,6 +30,8 @@ #include <linux/of.h> #include <linux/of_address.h> #include <linux/of_graph.h> +#include <linux/of_irq.h> +#include <linux/interrupt.h> #include <video/display_timing.h> #include <video/of_display_timing.h> #include <video/videomode.h> @@ -466,6 +468,73 @@ static int clcdfb_pan_display(struct fb_var_screeninfo *var, return 0; } +static int clcdfb_ioctl(struct fb_info *info, + unsigned int cmd, unsigned long args) +{ + struct clcd_fb *fb = to_clcd(info); + int retval = 0; + u32 val, ienb_val; + + switch (cmd) { + case FBIO_WAITFORVSYNC:{ + if (fb->lcd_irq <= 0) { + retval = -EINVAL; + break; + } + /* disable Vcomp interrupts */ + ienb_val = readl(fb->regs + fb->off_ienb); + ienb_val &= ~CLCD_PL111_IENB_VCOMP; + writel(ienb_val, fb->regs + fb->off_ienb); + + /* clear Vcomp interrupt */ + writel(CLCD_PL111_IENB_VCOMP, fb->regs + CLCD_PL111_ICR); + + /* Generate Interrupt at the start of Vsync */ + reinit_completion(&fb->wait); + val = readl(fb->regs + fb->off_cntl); + val &= ~(CNTL_LCDVCOMP(3)); + writel(val, fb->regs + fb->off_cntl); + + /* enable Vcomp interrupt */ + ienb_val = readl(fb->regs + fb->off_ienb); + ienb_val |= CLCD_PL111_IENB_VCOMP; + writel(ienb_val, fb->regs + fb->off_ienb); + if (!wait_for_completion_interruptible_timeout + (&fb->wait, HZ/10)) + retval = -ETIMEDOUT; + break; + } + default: + retval = -ENOIOCTLCMD; + break; + } + return retval; +} + +static irqreturn_t clcd_interrupt(int irq, void *data) +{ + struct clcd_fb *fb = data; + u32 val; + + /* check for vsync interrupt */ + val = readl(fb->regs + CLCD_PL111_MIS); + + if (val & CLCD_PL111_IENB_VCOMP) { + val = readl(fb->regs + fb->off_ienb); + val &= ~CLCD_PL111_IENB_VCOMP; + + /* disable Vcomp interrupts */ + writel(val, fb->regs + fb->off_ienb); + + /* clear Vcomp interrupt */ + writel(CLCD_PL111_IENB_VCOMP, fb->regs + CLCD_PL111_ICR); + + complete(&fb->wait); + return IRQ_HANDLED; + } + return IRQ_NONE; +} + static struct fb_ops clcdfb_ops = { .owner = THIS_MODULE, .fb_check_var = clcdfb_check_var, @@ -477,6 +546,7 @@ static struct fb_ops clcdfb_ops = { .fb_imageblit = cfb_imageblit, .fb_mmap = clcdfb_mmap, .fb_pan_display = clcdfb_pan_display, + .fb_ioctl = clcdfb_ioctl, }; static int clcdfb_register(struct clcd_fb *fb) @@ -753,6 +823,18 @@ static int clcdfb_of_init_display(struct clcd_fb *fb) else fb->fb.var.yres_virtual = yres_virtual; + fb->lcd_irq = irq_of_parse_and_map(fb->dev->dev.of_node, 0); + if (fb->lcd_irq > 0) { + err = devm_request_irq(&fb->dev->dev, + fb->lcd_irq, clcd_interrupt, + IRQF_SHARED, "clcd", fb); + if (err < 0) { + dev_err(&fb->dev->dev, "unable to register CLCD interrupt\n"); + return err; + } + init_completion(&fb->wait); + } + if (of_property_read_u32_array(endpoint, "arm,pl11x,tft-r0g0b0-pads", tft_r0b0g0, ARRAY_SIZE(tft_r0b0g0)) == 0) diff --git a/include/linux/amba/clcd.h b/include/linux/amba/clcd.h index e82e3ee..6a3bc2d 100644 --- a/include/linux/amba/clcd.h +++ b/include/linux/amba/clcd.h @@ -28,6 +28,8 @@ #define CLCD_PL110_UCUR 0x00000028 #define CLCD_PL110_LCUR 0x0000002C +#define CLCD_PL111_IENB_VCOMP (1 << 3) + #define CLCD_PL111_CNTL 0x00000018 #define CLCD_PL111_IENB 0x0000001c #define CLCD_PL111_RIS 0x00000020 @@ -184,6 +186,8 @@ struct clcd_fb { u32 clcd_cntl; u32 cmap[16]; bool clk_enabled; + struct completion wait; + int lcd_irq; }; static inline void clcdfb_decode(struct clcd_fb *fb, struct clcd_regs *regs)