| 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: <8905aad5ba0a37d1c889cfe612b785817d0f1193.camel@nxp.com>
Date: Wed, 31 Mar 2021 14:25:57 +0800
From: Liu Ying <victor.liu@....com>
To: Robert Foss <robert.foss@...aro.org>
Cc: dri-devel <dri-devel@...ts.freedesktop.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>,
"moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE"
<linux-arm-kernel@...ts.infradead.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
linux-media <linux-media@...r.kernel.org>,
David Airlie <airlied@...ux.ie>,
Daniel Vetter <daniel@...ll.ch>,
Rob Herring <robh+dt@...nel.org>, shawnguo@...nel.org,
s.hauer@...gutronix.de, kernel@...gutronix.de,
Fabio Estevam <festevam@...il.com>, linux-imx@....com,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Andrzej Hajda <a.hajda@...sung.com>,
Neil Armstrong <narmstrong@...libre.com>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Jonas Karlman <jonas@...boo.se>,
Jernej Skrabec <jernej.skrabec@...l.net>, kishon@...com,
Vinod Koul <vkoul@...nel.org>, lee.jones@...aro.org
Subject: Re: [PATCH v6 12/14] drm/bridge: imx: Add LDB support for i.MX8qxp
On Tue, 2021-03-30 at 11:54 +0200, Robert Foss wrote:
> Hey Liu,
>
> checkpatch --strict lists some nit and a warning. With those fixed
> feel free to add my r-b.
>
> On Wed, 17 Mar 2021 at 04:57, Liu Ying <victor.liu@....com> wrote:
> > This patch adds a drm bridge driver for i.MX8qxp LVDS display bridge(LDB)
> > which is officially named as pixel mapper. The LDB has two channels.
> > Each of them supports up to 24bpp parallel input color format and can map
> > the input to VESA or JEIDA standards. The two channels cannot be used
> > simultaneously, that is to say, the user should pick one of them to use.
> > Two LDB channels from two LDB instances can work together in LDB split
> > mode to support a dual link LVDS display. The channel indexes have to be
> > different. Channel0 outputs odd pixels and channel1 outputs even pixels.
> > This patch supports the LDB single mode and split mode.
> >
> > Signed-off-by: Liu Ying <victor.liu@....com>
> > ---
> > Note that this patch depends on the patch 'phy: Add LVDS configuration options',
> > which has already been sent with the following series to add Mixel combo PHY
> > found in i.MX8qxp:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg879957.html&data=04%7C01%7Cvictor.liu%40nxp.com%7C7c4f1092633c4687fb3108d8f361ddfd%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637526948949839014%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=iNr6RU0xrhcW8epwIPPJDpayjWxfmWsSjCju3XIcmPM%3D&reserved=0
> >
> > v5->v6:
> > * No change.
> >
> > v4->v5:
> > * Link with the imx-ldb-helper object. (Robert)
> > * Correspondingly, rename 'imx8qxp-ldb.c' to 'imx8qxp-ldb-drv.c'.
> >
> > v3->v4:
> > * No change.
> >
> > v2->v3:
> > * No change.
> >
> > v1->v2:
> > * Drop unnecessary DT validation.
> > * Use of_graph_get_endpoint_by_regs() and of_graph_get_remote_endpoint() to
> > get the input remote endpoint in imx8qxp_ldb_set_di_id().
> > * Avoid using companion_port OF node after putting it in
> > imx8qxp_ldb_parse_dt_companion().
> > * Mention i.MX8qxp LDB official name 'pixel mapper' in the bridge driver
> > and Kconfig help message.
> >
> > drivers/gpu/drm/bridge/imx/Kconfig | 9 +
> > drivers/gpu/drm/bridge/imx/Makefile | 3 +
> > drivers/gpu/drm/bridge/imx/imx8qxp-ldb-drv.c | 720 +++++++++++++++++++++++++++
> > 3 files changed, 732 insertions(+)
> > create mode 100644 drivers/gpu/drm/bridge/imx/imx8qxp-ldb-drv.c
> >
[...]
> > +
> > +static int imx8qxp_ldb_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct imx8qxp_ldb *imx8qxp_ldb;
> > + struct imx8qxp_ldb_channel *imx8qxp_ldb_ch;
> > + struct ldb *ldb;
> > + struct ldb_channel *ldb_ch;
> > + int ret, i;
> > +
> > + imx8qxp_ldb = devm_kzalloc(dev, sizeof(*imx8qxp_ldb), GFP_KERNEL);
> > + if (!imx8qxp_ldb)
> > + return -ENOMEM;
> > +
> > + imx8qxp_ldb->clk_pixel = devm_clk_get(dev, "pixel");
> > + if (IS_ERR(imx8qxp_ldb->clk_pixel)) {
> > + ret = PTR_ERR(imx8qxp_ldb->clk_pixel);
> > + if (ret != -EPROBE_DEFER)
> > + DRM_DEV_ERROR(dev,
> > + "failed to get pixel clock: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + imx8qxp_ldb->clk_bypass = devm_clk_get(dev, "bypass");
> > + if (IS_ERR(imx8qxp_ldb->clk_bypass)) {
> > + ret = PTR_ERR(imx8qxp_ldb->clk_bypass);
> > + if (ret != -EPROBE_DEFER)
> > + DRM_DEV_ERROR(dev,
> > + "failed to get bypass clock: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + imx8qxp_ldb->dev = dev;
> > +
> > + ldb = &imx8qxp_ldb->base;
> > + ldb->dev = dev;
> > + ldb->ctrl_reg = 0xe0;
> > +
> > + for (i = 0; i < MAX_LDB_CHAN_NUM; i++)
> > + ldb->channel[i] = &imx8qxp_ldb->channel[i].base;
> > +
> > + ret = ldb_init_helper(ldb);
> > + if (ret)
> > + return ret;
> > +
> > + if (ldb->available_ch_cnt == 0) {
> > + DRM_DEV_DEBUG_DRIVER(dev, "no available channel\n");
> > + return 0;
> > + } else if (ldb->available_ch_cnt > 1) {
> > + DRM_DEV_ERROR(dev, "invalid available channel number(%u)\n",
> > + ldb->available_ch_cnt);
> > + return -ENOTSUPP;
> > + }
>
> WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP
> #683: FILE: drivers/gpu/drm/bridge/imx/imx8qxp-ldb-drv.c:625:
> + return -ENOTSUPP;
>
> Maybe -EINVAL is a better return value.
Will use -EINVAL in the next version.
Liu Ying
Powered by blists - more mailing lists