| 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: <20170208074632.n35dqoz2rz24ncx3@lukather>
Date: Wed, 8 Feb 2017 08:46:32 +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 Mon, Feb 06, 2017 at 11:32:01AM -0600, Rob Herring wrote:
> On Mon, Feb 06, 2017 at 11:03:01AM +0100, Maxime Ripard wrote:
> > 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.
>
> Well, I got it half right. :) The probe does that, but this needs to
> too.
>
> > >
> > > 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.
>
> Well, certainly it needs testing. But this is one of the easier examples
> because you are requesting a specific port/endpoint number. It's the
> drivers that just loop over all the endpoints that give me more
> problems.
Oh, right... I missed that you were giving the port and endpoint
numbers..
> Anyway, this is 4.12 material, so I'll make the fix above and want
> to leave this in for now.
Ok, I'll test it, thanks!
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