* Re: [PATCH v4 0/5] simplefb: add clock handling code
From: Geert Uytterhoeven @ 2014-11-12 10:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACxGe6sMF2C6hvRNNnB_i6a44fh0WC7JjjisFOh5GMV66mC6Fw@mail.gmail.com>
Hi Grant,
On Wed, Nov 12, 2014 at 10:57 AM, Grant Likely <grant.likely@linaro.org> wrote:
> On Wednesday, November 12, 2014, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Tue, Nov 11, 2014 at 10:49 PM, Grant Likely <grant.likely@linaro.org> wrote:
>> > However, I am concerned about handover. I've lost track over the entire
>> > thread on whether the handover mechanism has been resolved, and I would
>> > really like to have a proposed solution to this documented in the
>> > binding. The fact that there is nothing tying the simple framebuffer to
>> > the actual hardware backing the framebuffer is concerning. It means the
>> > kernel needs to guess which graphics device is associated with the
>> > framebuffer.
>>
>> We did discuss handover in Düsseldorf, and concluded that the simplefb's
>> regs property can be used for this.
>>
>> While on a modern system with unified memory this association cannot be
>> derived in a generic way, a device-specific driver for the graphics hardware
>> can if the regs property of the simplefb node matches the address the CRTC
>> engine is configured for.
>
> ???
>
> Right, I'm going to be blunt here: That's just dumb. All the
> capability needed is there in the DT to associate a simple FB to a
> display controller, and the solution chosen is to use a heuristic?
>
> The association needs to be explicit. I strongly prefer putting the
> simple FB directly into the display controller node, but I would
> consider phandle linkage also.
IFF there's a display controller node, you can put it there.
I actually proposed to have a minimal/preliminary display controller node,
but people countered that for various reasons (too many components
with multiple nodes on many systems, bindings not yet defined, etc.).
And if there's no graphics driver/bindings yet at the time the bootloader
is written, it doesn't know how to link simplefb with it in DT.
Hence the heuristic to match regs... Does that make sense?
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 v4 0/5] simplefb: add clock handling code
From: Grant Likely @ 2014-11-12 9:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuHMdUoPK_s7OP1Aop4eOyhec+abFij7ZoAAdA5fqpJJy-Dgg@mail.gmail.com>
On Wednesday, November 12, 2014, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Grant,
>
> On Tue, Nov 11, 2014 at 10:49 PM, Grant Likely <grant.likely@linaro.org> wrote:
> > However, I am concerned about handover. I've lost track over the entire
> > thread on whether the handover mechanism has been resolved, and I would
> > really like to have a proposed solution to this documented in the
> > binding. The fact that there is nothing tying the simple framebuffer to
> > the actual hardware backing the framebuffer is concerning. It means the
> > kernel needs to guess which graphics device is associated with the
> > framebuffer.
>
> We did discuss handover in Düsseldorf, and concluded that the simplefb's
> regs property can be used for this.
>
> While on a modern system with unified memory this association cannot be
> derived in a generic way, a device-specific driver for the graphics hardware
> can if the regs property of the simplefb node matches the address the CRTC
> engine is configured for.
???
Right, I'm going to be blunt here: That's just dumb. All the
capability needed is there in the DT to associate a simple FB to a
display controller, and the solution chosen is to use a heuristic?
The association needs to be explicit. I strongly prefer putting the
simple FB directly into the display controller node, but I would
consider phandle linkage also.
g.
>
> 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: [linux-sunxi] Re: [PATCH v4 0/5] simplefb: add clock handling code
From: Geert Uytterhoeven @ 2014-11-12 8:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141111214950.7FCABC41610@trevor.secretlab.ca>
Hi Grant,
On Tue, Nov 11, 2014 at 10:49 PM, Grant Likely <grant.likely@linaro.org> wrote:
> However, I am concerned about handover. I've lost track over the entire
> thread on whether the handover mechanism has been resolved, and I would
> really like to have a proposed solution to this documented in the
> binding. The fact that there is nothing tying the simple framebuffer to
> the actual hardware backing the framebuffer is concerning. It means the
> kernel needs to guess which graphics device is associated with the
> framebuffer.
We did discuss handover in Düsseldorf, and concluded that the simplefb's
regs property can be used for this.
While on a modern system with unified memory this association cannot be
derived in a generic way, a device-specific driver for the graphics hardware
can if the regs property of the simplefb node matches the address the CRTC
engine is configured for.
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] video: fbdev: Add CVT timing calculations.
From: David Ung @ 2014-11-11 22:54 UTC (permalink / raw)
To: linux-fbdev
Currently fbmon is still relying on the old GTF timings when parsing
the standard timings of the EDID.
This causes problem with some monitor eg DELL U2410 which advertises
high resolutions like 1920x1200@60 and GTF timing gives it 193mhz
clock which is out of spec of the monitor which has dclkmax of 170mhz.
This patch address the above problem by adding support for CVT with
reduced timings.
Signed-off-by: David Ung <davidu@nvidia.com>
---
drivers/video/fbdev/core/fbmon.c | 92 +++++++++++++++++++++++++++++++++-------
1 file changed, 77 insertions(+), 15 deletions(-)
diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
index 5b0e313..8d4ec9f 100644
--- a/drivers/video/fbdev/core/fbmon.c
+++ b/drivers/video/fbdev/core/fbmon.c
@@ -21,6 +21,11 @@
* GTF Spreadsheet by Andy Morrish (1/5/97)
* available at http://www.vesa.org
*
+ * Coordinated Video Timing is derived from:
+ *
+ * CVT Spreadsheet by Graham Loveridge (9/Apr/2003)
+ * available at http://www.vesa.org
+ *
* This file is subject to the terms and conditions of the GNU General Public
* License. See the file COPYING in the main directory of this archive
* for more details.
@@ -383,17 +388,21 @@ static void get_chroma(unsigned char *block, struct fb_monspecs *specs)
}
static void calc_mode_timings(int xres, int yres, int refresh,
- struct fb_videomode *mode)
+ struct fb_videomode *mode,
+ struct fb_monspecs *specs)
{
struct fb_var_screeninfo *var;
+ struct fb_info info;
var = kzalloc(sizeof(struct fb_var_screeninfo), GFP_KERNEL);
+ if (specs)
+ memcpy(&info.monspecs, specs, sizeof(struct fb_monspecs));
if (var) {
var->xres = xres;
var->yres = yres;
- fb_get_mode(FB_VSYNCTIMINGS | FB_IGNOREMON,
- refresh, var, NULL);
+ fb_get_mode(FB_VSYNCTIMINGS | FB_IGNOREMON, refresh, var,
+ specs && !specs->gtf ? &info : NULL);
mode->xres = xres;
mode->yres = yres;
mode->pixclock = var->pixclock;
@@ -417,12 +426,12 @@ static int get_est_timing(unsigned char *block, struct fb_videomode *mode)
c = block[0];
if (c&0x80) {
- calc_mode_timings(720, 400, 70, &mode[num]);
+ calc_mode_timings(720, 400, 70, &mode[num], NULL);
mode[num++].flag = FB_MODE_IS_CALCULATED;
DPRINTK(" 720x400@70Hz\n");
}
if (c&0x40) {
- calc_mode_timings(720, 400, 88, &mode[num]);
+ calc_mode_timings(720, 400, 88, &mode[num], NULL);
mode[num++].flag = FB_MODE_IS_CALCULATED;
DPRINTK(" 720x400@88Hz\n");
}
@@ -431,7 +440,7 @@ static int get_est_timing(unsigned char *block, struct fb_videomode *mode)
DPRINTK(" 640x480@60Hz\n");
}
if (c&0x10) {
- calc_mode_timings(640, 480, 67, &mode[num]);
+ calc_mode_timings(640, 480, 67, &mode[num], NULL);
mode[num++].flag = FB_MODE_IS_CALCULATED;
DPRINTK(" 640x480@67Hz\n");
}
@@ -462,7 +471,7 @@ static int get_est_timing(unsigned char *block, struct fb_videomode *mode)
DPRINTK(" 800x600@75Hz\n");
}
if (c&0x20) {
- calc_mode_timings(832, 624, 75, &mode[num]);
+ calc_mode_timings(832, 624, 75, &mode[num], NULL);
mode[num++].flag = FB_MODE_IS_CALCULATED;
DPRINTK(" 832x624@75Hz\n");
}
@@ -496,7 +505,7 @@ static int get_est_timing(unsigned char *block, struct fb_videomode *mode)
}
static int get_std_timing(unsigned char *block, struct fb_videomode *mode,
- int ver, int rev)
+ int ver, int rev, struct fb_monspecs *specs)
{
int xres, yres = 0, refresh, ratio, i;
@@ -535,7 +544,7 @@ static int get_std_timing(unsigned char *block, struct fb_videomode *mode,
return 1;
}
}
- calc_mode_timings(xres, yres, refresh, mode);
+ calc_mode_timings(xres, yres, refresh, mode, specs);
return 1;
}
@@ -545,7 +554,7 @@ static int get_dst_timing(unsigned char *block,
int j, num = 0;
for (j = 0; j < 6; j++, block += STD_TIMING_DESCRIPTION_SIZE)
- num += get_std_timing(block, &mode[num], ver, rev);
+ num += get_std_timing(block, &mode[num], ver, rev, NULL);
return num;
}
@@ -601,7 +610,8 @@ static void get_detailed_timing(unsigned char *block,
* This function builds a mode database using the contents of the EDID
* data
*/
-static struct fb_videomode *fb_create_modedb(unsigned char *edid, int *dbsize)
+static struct fb_videomode *fb_create_modedb(unsigned char *edid, int *dbsize,
+ struct fb_monspecs *specs)
{
struct fb_videomode *mode, *m;
unsigned char *block;
@@ -643,7 +653,7 @@ static struct fb_videomode *fb_create_modedb(unsigned char *edid, int *dbsize)
DPRINTK(" Standard Timings\n");
block = edid + STD_TIMING_DESCRIPTIONS_START;
for (i = 0; i < STD_TIMING; i++, block += STD_TIMING_DESCRIPTION_SIZE)
- num += get_std_timing(block, &mode[num], ver, rev);
+ num += get_std_timing(block, &mode[num], ver, rev, specs);
block = edid + DETAILED_TIMING_DESCRIPTIONS_START;
for (i = 0; i < 4; i++, block+= DETAILED_TIMING_DESCRIPTION_SIZE) {
@@ -707,7 +717,7 @@ static int fb_get_monitor_limits(unsigned char *edid, struct fb_monspecs *specs)
int num_modes, hz, hscan, pixclock;
int vtotal, htotal;
- modes = fb_create_modedb(edid, &num_modes);
+ modes = fb_create_modedb(edid, &num_modes, NULL);
if (!modes) {
DPRINTK("None Available\n");
return 1;
@@ -964,7 +974,7 @@ void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs)
DPRINTK(" Display Characteristics:\n");
get_monspecs(edid, specs);
- specs->modedb = fb_create_modedb(edid, &specs->modedb_len);
+ specs->modedb = fb_create_modedb(edid, &specs->modedb_len, specs);
/*
* Workaround for buggy EDIDs that sets that the first
@@ -1255,6 +1265,55 @@ static void fb_timings_dclk(struct __fb_timings *timings)
}
/*
+ * All calculations are based on the VESA CVT Spreadsheet
+ * available at VESA's public ftp (http://www.vesa.org).
+ *
+ * The names of variables are kept to match as closely to the names
+ * that appears in the spreadsheet.
+ */
+#define RB_MIN_V_BLANK 460
+#define RB_H_BLANK 160
+#define CELL_GRAN 8
+#define CLOCK_STEP 250
+#define RB_V_FPORCH 3
+#define V_SYNC_RND 6
+#define MIN_V_BPORCH 6
+
+static void fb_timings_vfreq_cvt(struct __fb_timings *timings)
+{
+ u32 v_lines_rnd, h_period_est, vbi_line, rb_min_vbi, act_vbi_lines;
+ u32 total_v_lines, h_pixels_rnd, total_active_pixels, total_pixels;
+ u32 act_pixel_freq;
+ u32 v_field_rate_rqd = timings->vfreq;
+ u32 h_pixels = timings->hactive;
+ u32 v_lines = timings->vactive;
+
+ v_lines_rnd = v_lines;
+ h_period_est = 1000000 / v_field_rate_rqd - RB_MIN_V_BLANK;
+ h_period_est = h_period_est / v_lines_rnd;
+ vbi_line = RB_MIN_V_BLANK / h_period_est;
+ rb_min_vbi = RB_V_FPORCH + V_SYNC_RND + MIN_V_BPORCH;
+ if (vbi_line < rb_min_vbi)
+ act_vbi_lines = rb_min_vbi;
+ else
+ act_vbi_lines = vbi_line;
+ total_v_lines = act_vbi_lines + v_lines_rnd;
+ h_pixels_rnd = (h_pixels / CELL_GRAN) * CELL_GRAN;
+ total_active_pixels = h_pixels_rnd;
+ total_pixels = RB_H_BLANK + total_active_pixels;
+ act_pixel_freq = v_field_rate_rqd * total_v_lines * total_pixels;
+ act_pixel_freq /= 1000;
+ act_pixel_freq = act_pixel_freq / CLOCK_STEP * CLOCK_STEP * 1000;
+
+ timings->hfreq = act_pixel_freq / total_pixels;
+ timings->vblank = act_vbi_lines;
+ timings->vtotal = timings->vactive + timings->vblank;
+ timings->hblank = RB_H_BLANK;
+ timings->htotal = timings->hactive + timings->hblank;
+ timings->dclk = timings->htotal * timings->hfreq;
+}
+
+/*
* fb_get_mode - calculates video mode using VESA GTF
* @flags: if: 0 - maximize vertical refresh rate
* 1 - vrefresh-driven calculation;
@@ -1347,7 +1406,10 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
break;
case FB_VSYNCTIMINGS: /* vrefresh driven */
timings->vfreq = val;
- fb_timings_vfreq(timings);
+ if (info && !info->monspecs.gtf)
+ fb_timings_vfreq_cvt(timings);
+ else
+ fb_timings_vfreq(timings);
break;
case FB_HSYNCTIMINGS: /* hsync driven */
timings->hfreq = val;
--
1.8.1.5
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
^ permalink raw reply related
* Re: [linux-sunxi] Re: [PATCH v4 0/5] simplefb: add clock handling code
From: Grant Likely @ 2014-11-11 21:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <545B8E29.90807@redhat.com>
On Thu, 06 Nov 2014 16:05:13 +0100
, Hans de Goede <hdegoede@redhat.com>
wrote:
> Hi,
>
> On 11/06/2014 03:18 PM, Tomi Valkeinen wrote:
> > On 31/10/14 10:15, Tomi Valkeinen wrote:
> >> Hi Hans,
> >>
> >> On 22/10/14 19:45, Hans de Goede wrote:
> >>
> >>> -We will add a clocks property to the simplefb devicetree bindings, so
> >>> that u-boot setup framebuffers passed to the kernel (for early console
> >>> support) can properly list the clocks used, and simplefb can claim them to
> >>> avoid them getting turned off, thereby breaking the early console
> >>
> >> Perhaps this has been discussed earlier, but I started to wonder if
> >> managing clocks is enough.
> >>
> >> What about regulators (to power the LCD), gpios (to keep the LCD
> >> enabled) and pinctrl (for video signals)? I guess any those could be
> >> reset by the kernel if no driver uses them.
I'm okay with extending the binding for each of these dependencies
/providing/ it can be argued that the way simplefb uses it is merely to
inhibit the kernel from making any changes to the resource, and then
releasing the resource when it is done. Each resource type will need to
be taken on a case-by-case basis since each has it's own behaviour. I'll
want to see that the simplefb driver doesn't need to do anything special
per device, and that the resource binding is well established and
generic.
g.
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH v4 0/5] simplefb: add clock handling code
From: Grant Likely @ 2014-11-11 21:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAL_JsqKWqq+H5ymUW3n8tkmLJHEUGaKDLX5+orJmGUQ3ECQwUQ@mail.gmail.com>
On Sat, 1 Nov 2014 04:47:56 +0800
, Rob Herring <robherring2@gmail.com>
wrote:
> On Wed, Oct 29, 2014 at 7:08 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > Hi Hans, Rob,
> >
> > On 28/10/14 13:30, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 10/28/2014 12:11 PM, Rob Herring wrote:
> >
> >>> Yes, I object to the binding still as it has not changed from what was
> >>> previously posted.
> >>
> >> It would be helpful if you could explain why you object. Last time you
> >> said: " You are mixing in a hardware description that is simply inaccurate."
> >>
> >> I then explained that this is not hardware description, but runtime state
> >> information, as it tells the kernel which clocks were chosen to drive the
> >> display (out of typically a list of possible options, depending on which
> >> output is used, etc.). Just like which memory address the bootloader has
> >> chosen to scan out the video image from.
> >>
> >> Then you got quiet, so sorry, but this time your objection really is too
> >> late. You cannot simply go quiet halfway through a discussion and then pop
> >> up again when a new version is posted to say "I object" yet another time,
> >> you've had your chance to make your arguments last time, and chose to stay
> >> quiet after I explained in detail that this is not hardware description but
> >> state information, so now it is simply too late.
> >>
> >> These bindings have been discussed at Plumbers with various interested people
> >> present, and the conclusion was that this really is the best way to handle this,
> >> so this patch is:
> >>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> Reviewed-by: Mike Turquette <mturquette@linaro.org>
> >> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> >> Reviewed-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >>
> >> And David Herrman who is working on simpledrm, which will be merged soon, which
> >> will also use the simplefb bindings also agrees. So we have the simplefb maintainer,
> >> simpledrm maintainer, and the clk subsystem maintainer + 2 other maintainers all
> >> agreeing on a way forward, the time for bikeshedding now really really really is
> >> over.
> >>
> >> Tomi, can you please let us know how you plan to proceed with this ?
> >
> > I won't merge DT bindings via fbdev tree, if a DT maintainer says no.
> >
> > I took Rob's silence to the earlier series as a silent ack for your
> > explanation. Obviously that was not the case.
> >
> > Rob, please advice asap what should be done to the bindings to get your
> > ack. As Hans explained above, this discussion has been going on for a
> > long time, and afaik this series is the best way forward of all the
> > options discussed.
>
> I still think for the most part this is a kernel problem. It is a
> kernel policy to turn off unused clocks. The clock framework could
> just as easily decide that any clocks enabled at boot and left
> un-managed (i.e. w/o a driver) are kept on until they are managed. I'm
> not saying this can't be in DT, only that DT is not the only solution
> here. This problem is not unique to simplefb. A serial console could
> stop working if no serial driver is loaded before unused clocks are
> disabled. CPU core clocks have a similar issue as well (often enabled
> in platform code). I want to see this solved in a generic way for any
> clock.
I've spent most of today rereading the entire thread and thinking about
this issue. Given that the clock binding is a well established core
binding at this point, it is entirely reasonable for a generic driver to
have part of the binding specify the clock resources that it depends on
to keep working. The binding needs to be very clear that it is a mere
dependency, and that the OS must not make any attempt to modify the
clock settings, but it is reasonable indicate to the OS that the clocks
are used by the firmware framebuffer, and turning them off will cause
the framebuffer will break. The best place to provide that guidance is
directly in the node.
If we were talking about a resource that didn't have a well established
binding, ie, the video path configuration, then I'd still say no way
because there is no way to do it generically. Claiming clocks on the
other hand is straight forward and simple.
Aside from wanting the documentation to be more explicit about what the
driver does with the clocks, I don't have a problem with doing it.
However, I am concerned about handover. I've lost track over the entire
thread on whether the handover mechanism has been resolved, and I would
really like to have a proposed solution to this documented in the
binding. The fact that there is nothing tying the simple framebuffer to
the actual hardware backing the framebuffer is concerning. It means the
kernel needs to guess which graphics device is associated with the
framebuffer.
The best solution in my opinion is to embed the simplefb properties
directly into the graphics device node (Yes, I know some have argued
that the graphics subsystem is composed of multiple devices; That
doesn't matter. Just choose one of the device nodes; the device that
actually DMAs the video buffer is probably the correct choice). That
makes it crystal clear which graphics controller owns the framebuffer.
There is precedence in DT bindings for putting current state into the
tree. offb.c is trying to solve the exact same problem. It also means
that we can use the driver model to provide mutual exclusion between
probing simplefb and a full-featured driver.
That approach would require some modifications of the binding to make
sure properties don't conflict with the device specific properties. The
easiest way is to allow the properties to live in a sub node so that the
reg property doesn't conflict.
However, I'm not stuck on this approach. The solution must be
unambiguious, and the handover mechanism must be thought through, but it
doesn't /have/ to be what I suggested above if a proposal that can be
well argued as better.
Hans, let's talk about this tomorrow. #devicetree on freenode? It looks like
I'm committed to volunteer at my daughters school tomorrow morning, but
the afternoon looks good.
g.
^ permalink raw reply
* Re: [PATCH v4 2/5] dt-bindings: Add a clocks property to the simple-framebuffer binding
From: Grant Likely @ 2014-11-11 21:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1413996311-4287-3-git-send-email-hdegoede@redhat.com>
On Wed, 22 Oct 2014 18:45:08 +0200
, Hans de Goede <hdegoede@redhat.com>
wrote:
> A simple-framebuffer node represents a framebuffer setup by the firmware /
> bootloader. Such a framebuffer may have a number of clocks in use, add a
> property to communicate this to the OS.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Mike Turquette <mturquette@linaro.org>
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Reviewed-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>
> --
> Changes in v2:
> -Added Reviewed-by: Mike Turquette <mturquette@linaro.org>
> Changes in v3:
> -Updated description to make clear simplefb deals with more then just memory
> ---
> Documentation/devicetree/bindings/video/simple-framebuffer.txt | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
> index 70c26f3..172ad5f 100644
> --- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt
> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
> @@ -1,8 +1,8 @@
> Simple Framebuffer
>
> -A simple frame-buffer describes a raw memory region that may be rendered to,
> -with the assumption that the display hardware has already been set up to scan
> -out from that buffer.
> +A simple frame-buffer describes a frame-buffer setup by firmware or
> +the bootloader, with the assumption that the display hardware has already
> +been set up to scan out from the memory pointed to by the reg property.
>
> Required properties:
> - compatible: "simple-framebuffer"
> @@ -14,6 +14,9 @@ Required properties:
> - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b).
> - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r).
>
> +Optional properties:
> +- clocks : List of clocks used by the framebuffer
Please change to the following:
- clocks : List of clocks used by the framebuffer. Clocks listed here
are expected to already be configured correctly. The OS must
ensure these clocks are not modified or disabled while the
simple framebuffer remains active.
g.
^ permalink raw reply
* Re: [PATCH v4 3/5] simplefb: formalize pseudo palette handling
From: Grant Likely @ 2014-11-11 16:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1413996311-4287-4-git-send-email-hdegoede@redhat.com>
On Wed, 22 Oct 2014 18:45:09 +0200
, Hans de Goede <hdegoede@redhat.com>
wrote:
> From: Luc Verhaegen <libv@skynet.be>
>
Patch description? Write a proper changelog before merging please.
Describe why the change is necessary and what was done. You need to give
future readers some information about this patch. Heck, you're listed in
MAINTAINERS, you should be *enforcing* proper changelogs.
The code looks fine though.
Acked-by: Grant Likely <grant.likely@linaro.org>
> Signed-off-by: Luc Verhaegen <libv@skynet.be>
> Acked-by: Stephen Warren <swarren@nvidia.com>
> [hdegoede@redhat.com: drop unnecessary void * cast]
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Reviewed-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> ---
> drivers/video/fbdev/simplefb.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
> index 210f3a0..ec112c1 100644
> --- a/drivers/video/fbdev/simplefb.c
> +++ b/drivers/video/fbdev/simplefb.c
> @@ -41,6 +41,8 @@ static struct fb_var_screeninfo simplefb_var = {
> .vmode = FB_VMODE_NONINTERLACED,
> };
>
> +#define PSEUDO_PALETTE_SIZE 16
> +
> static int simplefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
> u_int transp, struct fb_info *info)
> {
> @@ -50,7 +52,7 @@ static int simplefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
> u32 cb = blue >> (16 - info->var.blue.length);
> u32 value;
>
> - if (regno >= 16)
> + if (regno >= PSEUDO_PALETTE_SIZE)
> return -EINVAL;
>
> value = (cr << info->var.red.offset) |
> @@ -163,11 +165,16 @@ static int simplefb_parse_pd(struct platform_device *pdev,
> return 0;
> }
>
> +struct simplefb_par {
> + u32 palette[PSEUDO_PALETTE_SIZE];
> +};
> +
> static int simplefb_probe(struct platform_device *pdev)
> {
> int ret;
> struct simplefb_params params;
> struct fb_info *info;
> + struct simplefb_par *par;
> struct resource *mem;
>
> if (fb_get_options("simplefb", NULL))
> @@ -188,11 +195,13 @@ static int simplefb_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> - info = framebuffer_alloc(sizeof(u32) * 16, &pdev->dev);
> + info = framebuffer_alloc(sizeof(struct simplefb_par), &pdev->dev);
> if (!info)
> return -ENOMEM;
> platform_set_drvdata(pdev, info);
>
> + par = info->par;
> +
> info->fix = simplefb_fix;
> info->fix.smem_start = mem->start;
> info->fix.smem_len = resource_size(mem);
> @@ -225,7 +234,7 @@ static int simplefb_probe(struct platform_device *pdev)
> framebuffer_release(info);
> return -ENODEV;
> }
> - info->pseudo_palette = (void *)(info + 1);
> + info->pseudo_palette = par->palette;
>
> dev_info(&pdev->dev, "framebuffer at 0x%lx, 0x%x bytes, mapped to 0x%p\n",
> info->fix.smem_start, info->fix.smem_len,
> --
> 2.1.0
>
^ permalink raw reply
* Re: [PATCH v4 1/5] simplefb: Add simplefb MAINTAINERS entry
From: Grant Likely @ 2014-11-11 16:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1413996311-4287-2-git-send-email-hdegoede@redhat.com>
On Wed, 22 Oct 2014 18:45:07 +0200
, Hans de Goede <hdegoede@redhat.com>
wrote:
> During the discussion about adding clock handling code to simplefb, it became
> clear that simplefb currently does not have an active maintainer.
>
> I've discussed this with Stephen Warren <swarren@wwwdotorg.org>, the original
> author of simplefb, and with his permisson I'm picking up maintainership of
> simplefb.
>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
> Reviewed-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Acked-by: Grant Likely <grant.likely@linaro.org>
> ---
> MAINTAINERS | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 73d1aef..6e92e73 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8416,6 +8416,14 @@ F: drivers/media/usb/siano/
> F: drivers/media/usb/siano/
> F: drivers/media/mmc/siano/
>
> +SIMPLEFB FB DRIVER
> +M: Hans de Goede <hdegoede@redhat.com>
> +L: linux-fbdev@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/video/simple-framebuffer.txt
> +F: drivers/video/fbdev/simplefb.c
> +F: include/linux/platform_data/simplefb.h
> +
> SH_VEU V4L2 MEM2MEM DRIVER
> L: linux-media@vger.kernel.org
> S: Orphan
> --
> 2.1.0
>
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Grant Likely @ 2014-11-11 16:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <542E56AB.40208@redhat.com>
On Fri, 03 Oct 2014 09:56:27 +0200
, Hans de Goede <hdegoede@redhat.com>
wrote:
> On 10/03/2014 01:31 AM, Julian Calaby wrote:
> > On Fri, Oct 3, 2014 at 1:14 AM, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
> >> Because you are creating two different device tree nodes describing a
> >> single piece of hardware and that's not suppose to happen in a device
> >> tree. The accurate description of the hardware is being perverted to
> >> solve a software problem.
> >>
> >> One node describes the hardware in a format to make simplefb happy.
> >> Another node describes the same hardware in a format to make the
> >> device specific driver happy.
> >
> > Stupid question: What about mangling an existing device node to be
> > simplefb compatible, and writing simplefb to be binding agnostic?
>
> That will not work, with simplefb a single node represents the currently
> active video output. While in real hardware that may involve multiple
> blocks, e.g on sunxi for hdmi out this involves the compositor (which
> generates video data from 1 or more layers) which feeds into the
> lcd-controller (which in this case is only used to generate hsync + vsync)
> signals really, which feeds into the hdmi encoder, all 3 of which are
> separate hardware blocks with their own clocks, etc.
The answer here should be, 'whoever does the DMA'.
Julian's suggestion is actually the sanest approach, and there is
precedence for doing exactly that in DT, both for serial devices
(of_serial.c) and framebuffers (offb.c).
g.
^ permalink raw reply
* Re: [PATCH 3/4] arm: dts: omap3-gta04: Add handling for tv output
From: Tony Lindgren @ 2014-11-11 15:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5461BDC6.7090308@ti.com>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [141110 23:44]:
> Tony,
>
> On 11/11/14 01:30, Tony Lindgren wrote:
> > * Marek Belisko <marek@goldelico.com> [141103 14:01]:
> >> Add handling for gta04 tv out chain:
> >> venc -> opa362 -> svideo
> >>
> >> Signed-off-by: Marek Belisko <marek@goldelico.com>
> >> ---
> >> arch/arm/boot/dts/omap3-gta04.dtsi | 48 ++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 48 insertions(+)
> >
> > Applying this patch into omap-for-v3.19/dt thanks.
>
> There are changes needed for this series, so don't apply yet.
>
> I'll try to find the time to do a review and send comments soon.
I've already pushed out the omap-for-v3.19/dt branch with
this patch buried in it. I'd rather not start redoing the
branch if the $subject patch is mostly correct and usable
with changes to the driver patches.
However, if the $subject patch needs to be redone for
the driver changes, let me know and I'll redo the branch.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH 02/15] GPIO: port LoCoMo gpio support from old driver
From: Mark Brown @ 2014-11-11 13:23 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: Linus Walleij,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux Input,
linux-leds-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org, Andrea Adami,
Russell King, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Richard Purdie,
Samuel Ortiz, Lee Jones
In-Reply-To: <CALT56yPr42FV66USngocw=eWPt81d5R2MJxmzBnv02HOMmXAkA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 397 bytes --]
On Tue, Nov 11, 2014 at 05:16:38PM +0400, Dmitry Eremin-Solenikov wrote:
> Just to better understand your suggestions: do you want me to convert
> to regmap only gpio driver or do you suggest to convert all LoCoMo drivers?
> That is doable, of course, but the amount of changes is overwhelming.
> Also I'm concerned about the performance impact from going through
> regmap layers.
I don't care.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH 02/15] GPIO: port LoCoMo gpio support from old driver
From: Dmitry Eremin-Solenikov @ 2014-11-11 13:16 UTC (permalink / raw)
To: Mark Brown
Cc: Linus Walleij, linux-arm-kernel@lists.infradead.org,
linux-gpio@vger.kernel.org, Linux Input,
linux-leds@vger.kernel.org, linux-spi@vger.kernel.org,
linux-fbdev@vger.kernel.org, alsa-devel@alsa-project.org,
Andrea Adami, Russell King, Daniel Mack, Haojian Zhuang,
Robert Jarzmik, Alexandre Courbot, Dmitry Torokhov, Bryan Wu,
Richard Purdie, Samuel Ortiz, Lee Jones
In-Reply-To: <20141106060335.GR8509@sirena.org.uk>
2014-11-06 9:03 GMT+03:00 Mark Brown <broonie@kernel.org>:
> On Thu, Nov 06, 2014 at 01:33:24AM +0400, Dmitry Eremin-Solenikov wrote:
>> 2014-11-03 16:43 GMT+03:00 Linus Walleij <linus.walleij@linaro.org>:
>
>> > Yes that's the point: if you use regmap mmio you get the RMW-locking
>> > for free, with the regmap implementation.
>
>> Just to be more concrete. Currently locomo_gpio_ack_irq() function uses
>> one lock and one unlock for doing 3 consecutive RMW I I convert locomo
>> to regmap, will that be 3 lock/unlock calls or still one? (Or maybe I'm
>> trying to be over-protective here and adding more lock/unlock cycles
>> won't matter that much?)
>
> You'll get three lock/unlocks, we could add an interface for bulk
> updates under one lock if it's important though.
>
>> Next question: if I have to export regmap to several subdrivers, is it better
>> to have one big regmap or to have one-map-per-driver approach?
>
> One regmap for the physical register map which is shared between all the
> users.
Mark, Linus,
Just to better understand your suggestions: do you want me to convert
to regmap only gpio driver or do you suggest to convert all LoCoMo drivers?
That is doable, of course, but the amount of changes is overwhelming.
Also I'm concerned about the performance impact from going through
regmap layers.
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 3/4] arm: dts: omap3-gta04: Add handling for tv output
From: Tomi Valkeinen @ 2014-11-11 7:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141110233050.GC31454@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 570 bytes --]
Tony,
On 11/11/14 01:30, Tony Lindgren wrote:
> * Marek Belisko <marek@goldelico.com> [141103 14:01]:
>> Add handling for gta04 tv out chain:
>> venc -> opa362 -> svideo
>>
>> Signed-off-by: Marek Belisko <marek@goldelico.com>
>> ---
>> arch/arm/boot/dts/omap3-gta04.dtsi | 48 ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 48 insertions(+)
>
> Applying this patch into omap-for-v3.19/dt thanks.
There are changes needed for this series, so don't apply yet.
I'll try to find the time to do a review and send comments soon.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 4/4] arm: dts: omap3-gta04: Add static configuration for devconf1 register
From: Tony Lindgren @ 2014-11-10 23:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1415051968-4878-5-git-send-email-marek@goldelico.com>
* Marek Belisko <marek@goldelico.com> [141103 14:01]:
> gta04 board need for tvout enabled 2 bits in devconf1 register (tvbypass and acbias).
> Add single pinmux entry and enable it.
>
> Signed-off-by: Marek Belisko <marek@goldelico.com>
> ---
> arch/arm/boot/dts/omap3-gta04.dtsi | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi
> index e4d05f0..a456d37 100644
> --- a/arch/arm/boot/dts/omap3-gta04.dtsi
> +++ b/arch/arm/boot/dts/omap3-gta04.dtsi
> @@ -118,6 +118,17 @@
> };
> };
> };
> +
> + /* pinmux for devconf1 */
> + control_devconf1: pinmux@480022d8 {
> + compatible = "pinctrl-single";
> + reg = <0x480022d8 4>; /* single register */
> + #address-cells = <1>;
> + #size-cells = <0>;
> + pinctrl-single,bit-per-mux;
> + pinctrl-single,register-width = <32>;
> + pinctrl-single,function-mask = <0xfc0bd5>;
> + };
> };
>
The pinctrl-single entry should be in omap3.dtsi as the mux register is there
for all the omap3 devices, can you please update the patch for that?
Regards,
Tony
> &omap3_pmx_core {
> @@ -497,3 +508,14 @@
> };
> };
> };
> +
> +&control_devconf1 {
> + pinctrl-name = "default";
> + pinctrl-0 = < &tv_acbias_pins>;
> +
> + tv_acbias_pins: pinmux_tv_acbias_pins {
> + pinctrl-single,bits = <
> + 0 0x40800 0x40800
> + >;
> + };
> +};
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH 3/4] arm: dts: omap3-gta04: Add handling for tv output
From: Tony Lindgren @ 2014-11-10 23:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1415051968-4878-4-git-send-email-marek@goldelico.com>
* Marek Belisko <marek@goldelico.com> [141103 14:01]:
> Add handling for gta04 tv out chain:
> venc -> opa362 -> svideo
>
> Signed-off-by: Marek Belisko <marek@goldelico.com>
> ---
> arch/arm/boot/dts/omap3-gta04.dtsi | 48 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
Applying this patch into omap-for-v3.19/dt thanks.
Tony
^ permalink raw reply
* Re: [PATCH 2/2] backlight: pwm: clean up pwm requested using legacy API
From: Lee Jones @ 2014-11-10 10:01 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: linux-fbdev, linux-pwm, Thierry Reding, Jingoo Han, Bryan Wu,
stable
In-Reply-To: <1413035186-11771-3-git-send-email-vladimir_zapolskiy@mentor.com>
On Sat, 11 Oct 2014, Vladimir Zapolskiy wrote:
> If PWM device is requested by means of legacy API pwm_request(), its
> resources are not freed on module unbind, which may cause an oops on
> access, e.g. by reading /sys/kernel/debug/pwm.
>
> Reported-by: Dmitry Eremin-Solenikov <dmitry_eremin@mentor.com>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> ---
> drivers/video/backlight/pwm_bl.c | 4 ++++
> 1 file changed, 4 insertions(+)
Applied with Thierry's Ack.
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index dd7aaf7..40770dd 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -34,6 +34,7 @@ struct pwm_bl_data {
> struct regulator *power_supply;
> struct gpio_desc *enable_gpio;
> unsigned int scale;
> + bool legacy;
> int (*notify)(struct device *,
> int brightness);
> void (*notify_after)(struct device *,
> @@ -274,6 +275,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> pb->pwm = devm_pwm_get(&pdev->dev, NULL);
> if (IS_ERR(pb->pwm) && !pdev->dev.of_node) {
> dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
> + pb->legacy = true;
> pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");
> }
>
> @@ -339,6 +341,8 @@ static int pwm_backlight_remove(struct platform_device *pdev)
>
> if (pb->exit)
> pb->exit(&pdev->dev);
> + if (pb->legacy)
> + pwm_free(pb->pwm);
>
> return 0;
> }
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Loan Empowerment
From: ALLIANCE WEST FINANCE. @ 2014-11-10 8:07 UTC (permalink / raw)
To: linux-fbdev
hallo,
Willkommen in der Kreditvergabe Firma Allianz Westen Finanz gestalten wir geschäftliche oder persönliche Darlehen in Höhe von 10.000 € und mehr als 100 Mio. € auf einem erschwinglichen Zinssatz von 2% pro Jahr? reagieren freundlich heute zu leihen.
Grüße,
Mr. Freeman Paulson
Allianz Westen Financial.
Tel: +447937438531
Tel: +447937438531
---
This email has been checked for viruses by Avast antivirus software.
http://www.avast.com
^ permalink raw reply
* Re: [PATCH 1/2] backlight: pwm: don't call legacy pwm request for device defined in dt
From: Vladimir Zapolskiy @ 2014-11-07 14:57 UTC (permalink / raw)
To: Thierry Reding; +Cc: linux-fbdev, linux-pwm, Jingoo Han, Bryan Wu, Lee Jones
In-Reply-To: <545CD4DC.4030409@mentor.com>
Thierry,
On 07.11.2014 16:19, Vladimir Zapolskiy wrote:
> Hi Thierry,
>
> On 07.11.2014 15:48, Thierry Reding wrote:
>> On Sat, Oct 11, 2014 at 04:46:25PM +0300, Vladimir Zapolskiy wrote:
>>> Platform PWM backlight data provided by board's device tree should be
>>> complete enough to successfully request a pwm device using pwm_get() API.
>>>
>>> Based on initial implementation done by Dmitry Eremin-Solenikov.
>>>
>>> Reported-by: Dmitry Eremin-Solenikov <dmitry_eremin@mentor.com>
>>> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
>>> Cc: Thierry Reding <thierry.reding@gmail.com>
>>> Cc: Jingoo Han <jg1.han@samsung.com>
>>> Cc: Bryan Wu <cooloney@gmail.com>
>>> Cc: Lee Jones <lee.jones@linaro.org>
>>> ---
>>> drivers/video/backlight/pwm_bl.c | 14 +++++++-------
>>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> I don't really understand what this is supposed to do. The commit
>> message doesn't make a very good job of explaining it either.
>>
>> Can you describe in more detail what problem this fixes and why it
>> should be merged?
>
> thank you for review.
>
> As it is shown by the code this particular change rejects fallback to
> legacy PWM device request (which itself in turn is fixed in the next
> commit) for boards with supplied DTS, "pwm-backlight" compatible node
> and unregistered corresponding PWM device in that node.
>
> I don't know if there is a good enough reason to register PWM backlight
> device connected to some quite arbitrary PWM device, if no PWM device
> information is given in the "pwm-backlight" compatible node, so I think
> it makes sense to change the default policy.
>
also please note that
Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
quite fairly describes "pwms" as a required property, but right now this
statement from the documentation is wrong, it is possible to register
pwm-backlight device driver (using notorious pwm_request() legacy API)
connected to some unspecified pwm device.
I don't think that the current registration policy is correct, that's
why I propose to fix the logic instead of making a documentation update.
--
With best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH 1/2] backlight: pwm: don't call legacy pwm request for device defined in dt
From: Vladimir Zapolskiy @ 2014-11-07 14:19 UTC (permalink / raw)
To: Thierry Reding; +Cc: linux-fbdev, linux-pwm, Jingoo Han, Bryan Wu, Lee Jones
In-Reply-To: <20141107134848.GB31950@ulmo>
Hi Thierry,
On 07.11.2014 15:48, Thierry Reding wrote:
> On Sat, Oct 11, 2014 at 04:46:25PM +0300, Vladimir Zapolskiy wrote:
>> Platform PWM backlight data provided by board's device tree should be
>> complete enough to successfully request a pwm device using pwm_get() API.
>>
>> Based on initial implementation done by Dmitry Eremin-Solenikov.
>>
>> Reported-by: Dmitry Eremin-Solenikov <dmitry_eremin@mentor.com>
>> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
>> Cc: Thierry Reding <thierry.reding@gmail.com>
>> Cc: Jingoo Han <jg1.han@samsung.com>
>> Cc: Bryan Wu <cooloney@gmail.com>
>> Cc: Lee Jones <lee.jones@linaro.org>
>> ---
>> drivers/video/backlight/pwm_bl.c | 14 +++++++-------
>> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> I don't really understand what this is supposed to do. The commit
> message doesn't make a very good job of explaining it either.
>
> Can you describe in more detail what problem this fixes and why it
> should be merged?
thank you for review.
As it is shown by the code this particular change rejects fallback to
legacy PWM device request (which itself in turn is fixed in the next
commit) for boards with supplied DTS, "pwm-backlight" compatible node
and unregistered corresponding PWM device in that node.
I don't know if there is a good enough reason to register PWM backlight
device connected to some quite arbitrary PWM device, if no PWM device
information is given in the "pwm-backlight" compatible node, so I think
it makes sense to change the default policy.
--
With best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH 1/2] backlight: pwm: don't call legacy pwm request for device defined in dt
From: Thierry Reding @ 2014-11-07 13:48 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: linux-fbdev, linux-pwm, Jingoo Han, Bryan Wu, Lee Jones
In-Reply-To: <1413035186-11771-2-git-send-email-vladimir_zapolskiy@mentor.com>
[-- Attachment #1: Type: text/plain, Size: 947 bytes --]
On Sat, Oct 11, 2014 at 04:46:25PM +0300, Vladimir Zapolskiy wrote:
> Platform PWM backlight data provided by board's device tree should be
> complete enough to successfully request a pwm device using pwm_get() API.
>
> Based on initial implementation done by Dmitry Eremin-Solenikov.
>
> Reported-by: Dmitry Eremin-Solenikov <dmitry_eremin@mentor.com>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Bryan Wu <cooloney@gmail.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> ---
> drivers/video/backlight/pwm_bl.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
I don't really understand what this is supposed to do. The commit
message doesn't make a very good job of explaining it either.
Can you describe in more detail what problem this fixes and why it
should be merged?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] backlight: pwm: clean up pwm requested using legacy API
From: Thierry Reding @ 2014-11-07 13:47 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: linux-fbdev, linux-pwm, Jingoo Han, Bryan Wu, Lee Jones, stable
In-Reply-To: <1413035186-11771-3-git-send-email-vladimir_zapolskiy@mentor.com>
[-- Attachment #1: Type: text/plain, Size: 652 bytes --]
On Sat, Oct 11, 2014 at 04:46:26PM +0300, Vladimir Zapolskiy wrote:
> If PWM device is requested by means of legacy API pwm_request(), its
> resources are not freed on module unbind, which may cause an oops on
> access, e.g. by reading /sys/kernel/debug/pwm.
>
> Reported-by: Dmitry Eremin-Solenikov <dmitry_eremin@mentor.com>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> ---
> drivers/video/backlight/pwm_bl.c | 4 ++++
> 1 file changed, 4 insertions(+)
Hopefully this can soon go away when all users of the legacy API have
been converted. Until then:
Acked-by: Thierry Reding <thierry.reding@gmail.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] backlight: pwm: clean up pwm requested using legacy API
From: Lee Jones @ 2014-11-07 11:50 UTC (permalink / raw)
To: Greg KH
Cc: Vladimir Zapolskiy, Thierry Reding, linux-fbdev, linux-pwm,
Jingoo Han, Bryan Wu, Dmitry Eremin-Solenikov
In-Reply-To: <20141106225420.GA2770@kroah.com>
On Thu, 06 Nov 2014, Greg KH wrote:
> On Fri, Nov 07, 2014 at 12:10:25AM +0200, Vladimir Zapolskiy wrote:
> > Hello Greg, Thierry,
> >
> > On 11.10.2014 17:21, Greg KH wrote:
> > > On Sat, Oct 11, 2014 at 04:46:26PM +0300, Vladimir Zapolskiy wrote:
> > >> If PWM device is requested by means of legacy API pwm_request(), its
> > >> resources are not freed on module unbind, which may cause an oops on
> > >> access, e.g. by reading /sys/kernel/debug/pwm.
> > >>
> > >> Reported-by: Dmitry Eremin-Solenikov <dmitry_eremin@mentor.com>
> > >> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> > >> ---
> > >> drivers/video/backlight/pwm_bl.c | 4 ++++
> > >> 1 file changed, 4 insertions(+)
> > >
> > > <formletter>
> > >
> > > This is not the correct way to submit patches for inclusion in the
> > > stable kernel tree. Please read Documentation/stable_kernel_rules.txt
> > > for how to do this properly.
> > >
> > > </formletter>
> > > --
> >
> > could you please review the change? I believe it would be nice to have
> > oops fix in v3.18.
>
> drivers/video/ is not under my control, so there's nothing I can do
> here, sorry...
I can apply this with Thierry's Ack.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH 2/2] backlight: pwm: clean up pwm requested using legacy API
From: Greg KH @ 2014-11-06 22:54 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Thierry Reding, linux-fbdev, linux-pwm, Jingoo Han, Bryan Wu,
Lee Jones, Dmitry Eremin-Solenikov
In-Reply-To: <545BF1D1.30304@mentor.com>
On Fri, Nov 07, 2014 at 12:10:25AM +0200, Vladimir Zapolskiy wrote:
> Hello Greg, Thierry,
>
> On 11.10.2014 17:21, Greg KH wrote:
> > On Sat, Oct 11, 2014 at 04:46:26PM +0300, Vladimir Zapolskiy wrote:
> >> If PWM device is requested by means of legacy API pwm_request(), its
> >> resources are not freed on module unbind, which may cause an oops on
> >> access, e.g. by reading /sys/kernel/debug/pwm.
> >>
> >> Reported-by: Dmitry Eremin-Solenikov <dmitry_eremin@mentor.com>
> >> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> >> ---
> >> drivers/video/backlight/pwm_bl.c | 4 ++++
> >> 1 file changed, 4 insertions(+)
> >
> > <formletter>
> >
> > This is not the correct way to submit patches for inclusion in the
> > stable kernel tree. Please read Documentation/stable_kernel_rules.txt
> > for how to do this properly.
> >
> > </formletter>
> > --
>
> could you please review the change? I believe it would be nice to have
> oops fix in v3.18.
drivers/video/ is not under my control, so there's nothing I can do
here, sorry...
^ permalink raw reply
* Re: [PATCH 2/2] backlight: pwm: clean up pwm requested using legacy API
From: Vladimir Zapolskiy @ 2014-11-06 22:10 UTC (permalink / raw)
To: Greg KH, Thierry Reding
Cc: linux-fbdev, linux-pwm, Jingoo Han, Bryan Wu, Lee Jones,
Dmitry Eremin-Solenikov
In-Reply-To: <20141011142156.GC11456@kroah.com>
Hello Greg, Thierry,
On 11.10.2014 17:21, Greg KH wrote:
> On Sat, Oct 11, 2014 at 04:46:26PM +0300, Vladimir Zapolskiy wrote:
>> If PWM device is requested by means of legacy API pwm_request(), its
>> resources are not freed on module unbind, which may cause an oops on
>> access, e.g. by reading /sys/kernel/debug/pwm.
>>
>> Reported-by: Dmitry Eremin-Solenikov <dmitry_eremin@mentor.com>
>> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
>> ---
>> drivers/video/backlight/pwm_bl.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>
> <formletter>
>
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree. Please read Documentation/stable_kernel_rules.txt
> for how to do this properly.
>
> </formletter>
> --
could you please review the change? I believe it would be nice to have
oops fix in v3.18.
--
With best wishes,
Vladimir
^ 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