* [PATCH 0/4] video: of: display_timing: Adjust err printing of of_get_display_timing()
@ 2019-07-22 18:24 Douglas Anderson
2019-07-22 18:24 ` [PATCH 1/4] video: of: display_timing: Add of_node_put() in of_get_display_timing() Douglas Anderson
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Douglas Anderson @ 2019-07-22 18:24 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz, Thierry Reding, Sam Ravnborg
Cc: David Airlie, linux-fbdev, Philipp Zabel, Tomi Valkeinen,
dri-devel, Laurent Pinchart, Russell King, Daniel Vetter,
Douglas Anderson, linux-kernel, Linus Walleij
As reported by Sam Ravnborg [1], after commit b8a2948fa2b3
("drm/panel: simple: Add ability to override typical timing") we now
see a pointless error message printed on every boot for many systems.
Let's fix that by adjusting who is responsible for printing error
messages when of_get_display_timing() is used.
Most certainly we can bikeshed the topic about whether this is the
right fix or we should instead add logic to panel_simple_probe() to
avoid calling of_get_display_timing() in the case where there is no
"panel-timing" sub-node. If there is consensus that I should move the
fix to panel_simple_probe() I'm happy to spin this series. In that
case we probably should _remove_ the extra prints that were already
present in the other two callers of of_get_display_timing().
While at it, fix a missing of_node_put() found by code inspection.
NOTE: amba-clcd and panel-lvds were only compile-tested.
[1] https://lkml.kernel.org/r/20190721093815.GA4375@ravnborg.org
Douglas Anderson (4):
video: of: display_timing: Add of_node_put() in
of_get_display_timing()
video: of: display_timing: Don't yell if no timing node is present
drm: panel-lvds: Spout an error if of_get_display_timing() gives an
error
video: amba-clcd: Spout an error if of_get_display_timing() gives an
error
drivers/gpu/drm/panel/panel-lvds.c | 5 ++++-
drivers/video/fbdev/amba-clcd.c | 4 +++-
drivers/video/of_display_timing.c | 11 +++++++----
3 files changed, 14 insertions(+), 6 deletions(-)
--
2.22.0.657.g960e92d24f-goog
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/4] video: of: display_timing: Add of_node_put() in of_get_display_timing()
2019-07-22 18:24 [PATCH 0/4] video: of: display_timing: Adjust err printing of of_get_display_timing() Douglas Anderson
@ 2019-07-22 18:24 ` Douglas Anderson
2019-07-27 2:51 ` Laurent Pinchart
2019-07-22 18:24 ` [PATCH 2/4] video: of: display_timing: Don't yell if no timing node is present Douglas Anderson
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Douglas Anderson @ 2019-07-22 18:24 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz, Thierry Reding, Sam Ravnborg
Cc: David Airlie, linux-fbdev, Philipp Zabel, Tomi Valkeinen,
dri-devel, Laurent Pinchart, Russell King, Daniel Vetter,
Douglas Anderson, linux-kernel
From code inspection it can be seen that of_get_display_timing() is
lacking an of_node_put(). Add it.
Fixes: ffa3fd21de8a ("videomode: implement public of_get_display_timing()")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
drivers/video/of_display_timing.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
index f5c1c469c0af..5eedae0799f0 100644
--- a/drivers/video/of_display_timing.c
+++ b/drivers/video/of_display_timing.c
@@ -119,6 +119,7 @@ int of_get_display_timing(const struct device_node *np, const char *name,
struct display_timing *dt)
{
struct device_node *timing_np;
+ int ret;
if (!np)
return -EINVAL;
@@ -129,7 +130,11 @@ int of_get_display_timing(const struct device_node *np, const char *name,
return -ENOENT;
}
- return of_parse_display_timing(timing_np, dt);
+ ret = of_parse_display_timing(timing_np, dt);
+
+ of_node_put(timing_np);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(of_get_display_timing);
--
2.22.0.657.g960e92d24f-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/4] video: of: display_timing: Don't yell if no timing node is present
2019-07-22 18:24 [PATCH 0/4] video: of: display_timing: Adjust err printing of of_get_display_timing() Douglas Anderson
2019-07-22 18:24 ` [PATCH 1/4] video: of: display_timing: Add of_node_put() in of_get_display_timing() Douglas Anderson
@ 2019-07-22 18:24 ` Douglas Anderson
2019-07-22 18:24 ` [PATCH 3/4] drm: panel-lvds: Spout an error if of_get_display_timing() gives an error Douglas Anderson
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Douglas Anderson @ 2019-07-22 18:24 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz, Thierry Reding, Sam Ravnborg
Cc: David Airlie, linux-fbdev, Philipp Zabel, Tomi Valkeinen,
dri-devel, Laurent Pinchart, Russell King, Daniel Vetter,
Douglas Anderson, linux-kernel
There may be cases (like in panel-simple.c) where we have a sane
fallback if no timings are specified in the device tree. Let's get
rid of the unconditional pr_err(). We can add error messages in
individual drivers if it makes sense.
NOTE: we'll still print errors if the node is present but there are
problems parsing the timings.
Fixes: b8a2948fa2b3 ("drm/panel: simple: Add ability to override typical timing")
Reported-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
drivers/video/of_display_timing.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
index 5eedae0799f0..abc9ada798ee 100644
--- a/drivers/video/of_display_timing.c
+++ b/drivers/video/of_display_timing.c
@@ -125,10 +125,8 @@ int of_get_display_timing(const struct device_node *np, const char *name,
return -EINVAL;
timing_np = of_get_child_by_name(np, name);
- if (!timing_np) {
- pr_err("%pOF: could not find node '%s'\n", np, name);
+ if (!timing_np)
return -ENOENT;
- }
ret = of_parse_display_timing(timing_np, dt);
--
2.22.0.657.g960e92d24f-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/4] drm: panel-lvds: Spout an error if of_get_display_timing() gives an error
2019-07-22 18:24 [PATCH 0/4] video: of: display_timing: Adjust err printing of of_get_display_timing() Douglas Anderson
2019-07-22 18:24 ` [PATCH 1/4] video: of: display_timing: Add of_node_put() in of_get_display_timing() Douglas Anderson
2019-07-22 18:24 ` [PATCH 2/4] video: of: display_timing: Don't yell if no timing node is present Douglas Anderson
@ 2019-07-22 18:24 ` Douglas Anderson
2019-07-22 18:24 ` [PATCH 4/4] video: amba-clcd: " Douglas Anderson
2019-07-23 8:38 ` [PATCH 0/4] video: of: display_timing: Adjust err printing of of_get_display_timing() Sam Ravnborg
4 siblings, 0 replies; 9+ messages in thread
From: Douglas Anderson @ 2019-07-22 18:24 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz, Thierry Reding, Sam Ravnborg
Cc: David Airlie, linux-fbdev, Philipp Zabel, Tomi Valkeinen,
dri-devel, Laurent Pinchart, Russell King, Daniel Vetter,
Douglas Anderson, linux-kernel
In the patch ("video: of: display_timing: Don't yell if no timing node
is present") we'll stop spouting an error directly in
of_get_display_timing() if no node is present. Presumably panel-lvds
should take charge of spouting its own error now.
NOTE: we'll print two errors if the node was present but there were
problems parsing the timing node (one in of_parse_display_timing() and
this new one). Since this is a fatal error for the driver's probe
(and presumably someone will be debugging), this should be OK.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
drivers/gpu/drm/panel/panel-lvds.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-lvds.c b/drivers/gpu/drm/panel/panel-lvds.c
index 1ec57d0806a8..ad47cc95459e 100644
--- a/drivers/gpu/drm/panel/panel-lvds.c
+++ b/drivers/gpu/drm/panel/panel-lvds.c
@@ -147,8 +147,11 @@ static int panel_lvds_parse_dt(struct panel_lvds *lvds)
int ret;
ret = of_get_display_timing(np, "panel-timing", &timing);
- if (ret < 0)
+ if (ret < 0) {
+ dev_err(lvds->dev, "%pOF: problems parsing panel-timing (%d)\n",
+ np, ret);
return ret;
+ }
videomode_from_timing(&timing, &lvds->video_mode);
--
2.22.0.657.g960e92d24f-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/4] video: amba-clcd: Spout an error if of_get_display_timing() gives an error
2019-07-22 18:24 [PATCH 0/4] video: of: display_timing: Adjust err printing of of_get_display_timing() Douglas Anderson
` (2 preceding siblings ...)
2019-07-22 18:24 ` [PATCH 3/4] drm: panel-lvds: Spout an error if of_get_display_timing() gives an error Douglas Anderson
@ 2019-07-22 18:24 ` Douglas Anderson
2019-07-23 8:27 ` Linus Walleij
2019-07-23 8:38 ` [PATCH 0/4] video: of: display_timing: Adjust err printing of of_get_display_timing() Sam Ravnborg
4 siblings, 1 reply; 9+ messages in thread
From: Douglas Anderson @ 2019-07-22 18:24 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz, Thierry Reding, Sam Ravnborg
Cc: David Airlie, linux-fbdev, Philipp Zabel, Tomi Valkeinen,
dri-devel, Laurent Pinchart, Russell King, Daniel Vetter,
Douglas Anderson, Linus Walleij, linux-kernel
In the patch ("video: of: display_timing: Don't yell if no timing node
is present") we'll stop spouting an error directly in
of_get_display_timing() if no node is present. Presumably amba-clcd
should take charge of spouting its own error now.
NOTE: we'll print two errors if the node was present but there were
problems parsing the timing node (one in of_parse_display_timing() and
this new one). Since this is a fatal error for the driver's probe
(and presumably someone will be debugging), this should be OK.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
drivers/video/fbdev/amba-clcd.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index 89324e42a033..7de43be6ef2c 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -561,8 +561,10 @@ static int clcdfb_of_get_dpi_panel_mode(struct device_node *node,
struct videomode video;
err = of_get_display_timing(node, "panel-timing", &timing);
- if (err)
+ if (err) {
+ pr_err("%pOF: problems parsing panel-timing (%d)\n", node, err);
return err;
+ }
videomode_from_timing(&timing, &video);
--
2.22.0.657.g960e92d24f-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 4/4] video: amba-clcd: Spout an error if of_get_display_timing() gives an error
2019-07-22 18:24 ` [PATCH 4/4] video: amba-clcd: " Douglas Anderson
@ 2019-07-23 8:27 ` Linus Walleij
0 siblings, 0 replies; 9+ messages in thread
From: Linus Walleij @ 2019-07-23 8:27 UTC (permalink / raw)
To: Douglas Anderson
Cc: Bartlomiej Zolnierkiewicz, Thierry Reding, Sam Ravnborg,
David Airlie, linux-fbdev, Philipp Zabel, Tomi Valkeinen,
open list:DRM PANEL DRIVERS, Laurent Pinchart, Russell King,
Daniel Vetter, linux-kernel
On Mon, Jul 22, 2019 at 8:25 PM Douglas Anderson <dianders@chromium.org> wrote:
> In the patch ("video: of: display_timing: Don't yell if no timing node
> is present") we'll stop spouting an error directly in
> of_get_display_timing() if no node is present. Presumably amba-clcd
> should take charge of spouting its own error now.
>
> NOTE: we'll print two errors if the node was present but there were
> problems parsing the timing node (one in of_parse_display_timing() and
> this new one). Since this is a fatal error for the driver's probe
> (and presumably someone will be debugging), this should be OK.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/4] video: of: display_timing: Adjust err printing of of_get_display_timing()
2019-07-22 18:24 [PATCH 0/4] video: of: display_timing: Adjust err printing of of_get_display_timing() Douglas Anderson
` (3 preceding siblings ...)
2019-07-22 18:24 ` [PATCH 4/4] video: amba-clcd: " Douglas Anderson
@ 2019-07-23 8:38 ` Sam Ravnborg
2019-07-26 14:42 ` Bartlomiej Zolnierkiewicz
4 siblings, 1 reply; 9+ messages in thread
From: Sam Ravnborg @ 2019-07-23 8:38 UTC (permalink / raw)
To: Douglas Anderson
Cc: Bartlomiej Zolnierkiewicz, Thierry Reding, David Airlie,
linux-fbdev, Philipp Zabel, Tomi Valkeinen, dri-devel,
Laurent Pinchart, Russell King, Daniel Vetter, linux-kernel,
Linus Walleij
Hi Dough.
On Mon, Jul 22, 2019 at 11:24:35AM -0700, Douglas Anderson wrote:
> As reported by Sam Ravnborg [1], after commit b8a2948fa2b3
> ("drm/panel: simple: Add ability to override typical timing") we now
> see a pointless error message printed on every boot for many systems.
> Let's fix that by adjusting who is responsible for printing error
> messages when of_get_display_timing() is used.
>
> Most certainly we can bikeshed the topic about whether this is the
> right fix or we should instead add logic to panel_simple_probe() to
> avoid calling of_get_display_timing() in the case where there is no
> "panel-timing" sub-node. If there is consensus that I should move the
> fix to panel_simple_probe() I'm happy to spin this series. In that
> case we probably should _remove_ the extra prints that were already
> present in the other two callers of of_get_display_timing().
>
> While at it, fix a missing of_node_put() found by code inspection.
>
> NOTE: amba-clcd and panel-lvds were only compile-tested.
>
> [1] https://lkml.kernel.org/r/20190721093815.GA4375@ravnborg.org
>
>
> Douglas Anderson (4):
> video: of: display_timing: Add of_node_put() in
> of_get_display_timing()
> video: of: display_timing: Don't yell if no timing node is present
> drm: panel-lvds: Spout an error if of_get_display_timing() gives an
> error
> video: amba-clcd: Spout an error if of_get_display_timing() gives an
> error
Series looks good - thanks.
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
You could consider silencing display_timing as the last patch, but thats
a very small detail.
How do we apply these fixes - to drm-misc-next? Bartlomiej?
No need to go in via drm-misc-fixes as the offending commit is only in
drm-misc-next.
Sam
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/4] video: of: display_timing: Adjust err printing of of_get_display_timing()
2019-07-23 8:38 ` [PATCH 0/4] video: of: display_timing: Adjust err printing of of_get_display_timing() Sam Ravnborg
@ 2019-07-26 14:42 ` Bartlomiej Zolnierkiewicz
0 siblings, 0 replies; 9+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2019-07-26 14:42 UTC (permalink / raw)
To: Sam Ravnborg, Douglas Anderson
Cc: Thierry Reding, David Airlie, linux-fbdev, Philipp Zabel,
Tomi Valkeinen, dri-devel, Laurent Pinchart, Russell King,
Daniel Vetter, linux-kernel, Linus Walleij
Hi,
On 7/23/19 10:38 AM, Sam Ravnborg wrote:
> Hi Dough.
>
> On Mon, Jul 22, 2019 at 11:24:35AM -0700, Douglas Anderson wrote:
>> As reported by Sam Ravnborg [1], after commit b8a2948fa2b3
>> ("drm/panel: simple: Add ability to override typical timing") we now
>> see a pointless error message printed on every boot for many systems.
>> Let's fix that by adjusting who is responsible for printing error
>> messages when of_get_display_timing() is used.
>>
>> Most certainly we can bikeshed the topic about whether this is the
>> right fix or we should instead add logic to panel_simple_probe() to
>> avoid calling of_get_display_timing() in the case where there is no
>> "panel-timing" sub-node. If there is consensus that I should move the
>> fix to panel_simple_probe() I'm happy to spin this series. In that
>> case we probably should _remove_ the extra prints that were already
>> present in the other two callers of of_get_display_timing().
>>
>> While at it, fix a missing of_node_put() found by code inspection.
>>
>> NOTE: amba-clcd and panel-lvds were only compile-tested.
>>
>> [1] https://lkml.kernel.org/r/20190721093815.GA4375@ravnborg.org
>>
>>
>> Douglas Anderson (4):
>> video: of: display_timing: Add of_node_put() in
>> of_get_display_timing()
>> video: of: display_timing: Don't yell if no timing node is present
>> drm: panel-lvds: Spout an error if of_get_display_timing() gives an
>> error
>> video: amba-clcd: Spout an error if of_get_display_timing() gives an
>> error
>
> Series looks good - thanks.
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
>
> You could consider silencing display_timing as the last patch, but thats
> a very small detail.
>
> How do we apply these fixes - to drm-misc-next? Bartlomiej?
>
> No need to go in via drm-misc-fixes as the offending commit is only in
> drm-misc-next.
I've merged the whole series to drm-misc-next, thanks!
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] video: of: display_timing: Add of_node_put() in of_get_display_timing()
2019-07-22 18:24 ` [PATCH 1/4] video: of: display_timing: Add of_node_put() in of_get_display_timing() Douglas Anderson
@ 2019-07-27 2:51 ` Laurent Pinchart
0 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2019-07-27 2:51 UTC (permalink / raw)
To: Douglas Anderson
Cc: Bartlomiej Zolnierkiewicz, Thierry Reding, Sam Ravnborg,
David Airlie, linux-fbdev, Philipp Zabel, Tomi Valkeinen,
dri-devel, Russell King, Daniel Vetter, linux-kernel
Hi Douglas,
Thank you for the patch.
On Mon, Jul 22, 2019 at 11:24:36AM -0700, Douglas Anderson wrote:
> From code inspection it can be seen that of_get_display_timing() is
> lacking an of_node_put(). Add it.
>
> Fixes: ffa3fd21de8a ("videomode: implement public of_get_display_timing()")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>
> drivers/video/of_display_timing.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
> index f5c1c469c0af..5eedae0799f0 100644
> --- a/drivers/video/of_display_timing.c
> +++ b/drivers/video/of_display_timing.c
> @@ -119,6 +119,7 @@ int of_get_display_timing(const struct device_node *np, const char *name,
> struct display_timing *dt)
> {
> struct device_node *timing_np;
> + int ret;
>
> if (!np)
> return -EINVAL;
> @@ -129,7 +130,11 @@ int of_get_display_timing(const struct device_node *np, const char *name,
> return -ENOENT;
> }
>
> - return of_parse_display_timing(timing_np, dt);
> + ret = of_parse_display_timing(timing_np, dt);
> +
> + of_node_put(timing_np);
> +
> + return ret;
> }
> EXPORT_SYMBOL_GPL(of_get_display_timing);
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-07-27 2:52 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-22 18:24 [PATCH 0/4] video: of: display_timing: Adjust err printing of of_get_display_timing() Douglas Anderson
2019-07-22 18:24 ` [PATCH 1/4] video: of: display_timing: Add of_node_put() in of_get_display_timing() Douglas Anderson
2019-07-27 2:51 ` Laurent Pinchart
2019-07-22 18:24 ` [PATCH 2/4] video: of: display_timing: Don't yell if no timing node is present Douglas Anderson
2019-07-22 18:24 ` [PATCH 3/4] drm: panel-lvds: Spout an error if of_get_display_timing() gives an error Douglas Anderson
2019-07-22 18:24 ` [PATCH 4/4] video: amba-clcd: " Douglas Anderson
2019-07-23 8:27 ` Linus Walleij
2019-07-23 8:38 ` [PATCH 0/4] video: of: display_timing: Adjust err printing of of_get_display_timing() Sam Ravnborg
2019-07-26 14:42 ` Bartlomiej Zolnierkiewicz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).