| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20170206100301.fatil7cvasuyhnqw@lukather>
Date: Mon, 6 Feb 2017 11:03:01 +0100
From: Maxime Ripard <maxime.ripard@...e-electrons.com>
To: Rob Herring <robh@...nel.org>
Cc: David Airlie <airlied@...ux.ie>,
Daniel Vetter <daniel.vetter@...el.com>,
Sean Paul <seanpaul@...omium.org>,
dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org,
Frank Rowand <frowand.list@...il.com>,
Boris Brezillon <boris.brezillon@...e-electrons.com>,
Archit Taneja <architt@...eaurora.org>,
Jingoo Han <jingoohan1@...il.com>,
Inki Dae <inki.dae@...sung.com>,
Joonyoung Shim <jy0922.shim@...sung.com>,
Seung-Woo Kim <sw0312.kim@...sung.com>,
Kyungmin Park <kyungmin.park@...sung.com>,
Kukjin Kim <kgene@...nel.org>,
Krzysztof Kozlowski <krzk@...nel.org>,
Javier Martinez Canillas <javier@....samsung.com>,
Stefan Agner <stefan@...er.ch>,
Alison Wang <alison.wang@...escale.com>,
Xinliang Liu <z.liuxinliang@...ilicon.com>,
Rongrong Zou <zourongrong@...il.com>,
Xinwei Kong <kong.kongxinwei@...ilicon.com>,
Chen Feng <puck.chen@...ilicon.com>,
Philipp Zabel <p.zabel@...gutronix.de>,
CK Hu <ck.hu@...iatek.com>,
Matthias Brugger <matthias.bgg@...il.com>,
Marek Vasut <marex@...x.de>,
Mark Yao <mark.yao@...k-chips.com>,
Heiko Stuebner <heiko@...ech.de>, Chen-Yu Tsai <wens@...e.org>,
Liviu Dudau <liviu.dudau@....com>,
Mali DP Maintainers <malidp@...s.arm.com>,
Neil Armstrong <narmstrong@...libre.com>,
Carlo Caione <carlo@...one.org>,
Kevin Hilman <khilman@...libre.com>,
Rob Clark <robdclark@...il.com>, Jyri Sarha <jsarha@...com>,
Tomi Valkeinen <tomi.valkeinen@...com>,
Eric Anholt <eric@...olt.net>,
Russell King <rmk+kernel@...linux.org.uk>
Subject: Re: [PATCH 4/5] drm: convert drivers to use
drm_of_find_panel_or_bridge
Hi Rob,
On Fri, Feb 03, 2017 at 09:36:34PM -0600, Rob Herring wrote:
> Similar to the previous commit, convert drivers open coding OF graph
> parsing to use drm_of_find_panel_or_bridge instead.
>
> This changes some error messages to debug messages (in the graph core).
> Graph connections are often "no connects" depending on the particular
> board, so we want to avoid spurious messages. Plus the kernel is not a
> DT validator.
>
> Signed-off-by: Rob Herring <robh@...nel.org>
> ---
[..]
> diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> index f5e86fe7750e..4720725b0fb0 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> @@ -15,6 +15,7 @@
> #include <drm/drmP.h>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_of.h>
> #include <drm/drm_panel.h>
>
> #include "sun4i_drv.h"
> @@ -217,12 +218,10 @@ int sun4i_rgb_init(struct drm_device *drm)
> rgb->drv = drv;
> encoder = &rgb->encoder;
>
> - tcon->panel = sun4i_tcon_find_panel(tcon->dev->of_node);
> - encoder->bridge = sun4i_tcon_find_bridge(tcon->dev->of_node);
> - if (IS_ERR(tcon->panel) && IS_ERR(encoder->bridge)) {
> - dev_info(drm->dev, "No panel or bridge found... RGB output disabled\n");
> - return 0;
> - }
> + ret = drm_of_find_panel_or_bridge(tcon->dev->of_node, 1, 0,
> + &tcon->panel, &encoder->bridge);
> + if (ret)
> + return ret;
It used to ignore the error if it couldn't find the bridge. This will
break the probe.
>
> drm_encoder_helper_add(&rgb->encoder,
> &sun4i_rgb_enc_helper_funcs);
> @@ -239,7 +238,7 @@ int sun4i_rgb_init(struct drm_device *drm)
> /* The RGB encoder can only work with the TCON channel 0 */
> rgb->encoder.possible_crtcs = BIT(0);
>
> - if (!IS_ERR(tcon->panel)) {
> + if (tcon->panel) {
> drm_connector_helper_add(&rgb->connector,
> &sun4i_rgb_con_helper_funcs);
> ret = drm_connector_init(drm, &rgb->connector,
> @@ -260,7 +259,7 @@ int sun4i_rgb_init(struct drm_device *drm)
> }
> }
>
> - if (!IS_ERR(encoder->bridge)) {
> + if (encoder->bridge) {
> encoder->bridge->encoder = &rgb->encoder;
>
> ret = drm_bridge_attach(drm, encoder->bridge);
> @@ -268,8 +267,6 @@ int sun4i_rgb_init(struct drm_device *drm)
> dev_err(drm->dev, "Couldn't attach our bridge\n");
> goto err_cleanup_connector;
> }
> - } else {
> - encoder->bridge = NULL;
> }
>
> return 0;
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index ea2906f87cb9..2e4e365cecf9 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -15,13 +15,12 @@
> #include <drm/drm_crtc.h>
> #include <drm/drm_crtc_helper.h>
> #include <drm/drm_modes.h>
> -#include <drm/drm_panel.h>
> +#include <drm/drm_of.h>
>
> #include <linux/component.h>
> #include <linux/ioport.h>
> #include <linux/of_address.h>
> #include <linux/of_device.h>
> -#include <linux/of_graph.h>
> #include <linux/of_irq.h>
> #include <linux/regmap.h>
> #include <linux/reset.h>
> @@ -405,74 +404,6 @@ static int sun4i_tcon_init_regmap(struct device *dev,
> return 0;
> }
>
> -struct drm_panel *sun4i_tcon_find_panel(struct device_node *node)
> -{
> - struct device_node *port, *remote, *child;
> - struct device_node *end_node = NULL;
> -
> - /* Inputs are listed first, then outputs */
> - port = of_graph_get_port_by_id(node, 1);
> -
> - /*
> - * Our first output is the RGB interface where the panel will
> - * be connected.
> - */
> - for_each_child_of_node(port, child) {
> - u32 reg;
> -
> - of_property_read_u32(child, "reg", ®);
> - if (reg == 0)
> - end_node = child;
> - }
> -
> - if (!end_node) {
> - DRM_DEBUG_DRIVER("Missing panel endpoint\n");
> - return ERR_PTR(-ENODEV);
> - }
> -
> - remote = of_graph_get_remote_port_parent(end_node);
> - if (!remote) {
> - DRM_DEBUG_DRIVER("Unable to parse remote node\n");
> - return ERR_PTR(-EINVAL);
> - }
> -
> - return of_drm_find_panel(remote) ?: ERR_PTR(-EPROBE_DEFER);
And the panel is only one of our endpoints, which is optional, while
other endpoints are mandatory. This means that we might very well have
an endpoint that is not a panel or a bridge. In this case, I think
your function will return an error and will be treated as such, while
it's really the expected behaviour.
I think it's better to leave this driver alone for now, it's not as
trivial as it looks, and will require some testing to get things
right. I'll try to get my head around how to use your new (very
welcome) helpers.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)
Powered by blists - more mailing lists