linux-kernel - Re: [PATCH v2 2/2] drm/bridge: fsl-ldb: Add i.MX93 LDB support

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <a5f8ec67-5888-3ee8-a1b1-32fef386d6cb@denx.de>
Date:   Sun, 22 Jan 2023 18:14:28 +0100
From:   Marek Vasut <marex@...x.de>
To:     Liu Ying <victor.liu@....com>, dri-devel@...ts.freedesktop.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     andrzej.hajda@...el.com, neil.armstrong@...aro.org,
        robert.foss@...aro.org, Laurent.pinchart@...asonboard.com,
        jonas@...boo.se, jernej.skrabec@...il.com, airlied@...il.com,
        daniel@...ll.ch, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, linux-imx@....com,
        krzysztof.kozlowski@...aro.org
Subject: Re: [PATCH v2 2/2] drm/bridge: fsl-ldb: Add i.MX93 LDB support

On 1/22/23 13:18, Liu Ying wrote:
> Same to i.MX8mp LDB, i.MX93 LDB is controlled by mediamix blk-ctrl
> through LDB_CTRL and LVDS_CTRL registers.  i.MX93 LDB supports only
> one LVDS channel(channel 0) and it's LVDS_CTRL register bit1 is used
> as LVDS_EN instead of CH1_EN.  Add i.MX93 LDB support in the existing
> i.MX8mp LDB bridge driver by adding i.MX93 LDB compatible string and
> device data(to reflect different register offsets and LVDS_CTRL register
> bit1 definition).
> 
> Signed-off-by: Liu Ying <victor.liu@....com>
> ---
> v1->v2:
> * No change.
> 
>   drivers/gpu/drm/bridge/fsl-ldb.c | 53 ++++++++++++++++++++++++++------
>   1 file changed, 44 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
> index 9bcba8fc57e7..6ad63ac7367c 100644
> --- a/drivers/gpu/drm/bridge/fsl-ldb.c
> +++ b/drivers/gpu/drm/bridge/fsl-ldb.c
> @@ -18,7 +18,6 @@
>   #include <drm/drm_of.h>
>   #include <drm/drm_panel.h>
>   
> -#define LDB_CTRL				0x5c
>   #define LDB_CTRL_CH0_ENABLE			BIT(0)
>   #define LDB_CTRL_CH0_DI_SELECT			BIT(1)
>   #define LDB_CTRL_CH1_ENABLE			BIT(2)
> @@ -35,9 +34,9 @@
>   #define LDB_CTRL_ASYNC_FIFO_ENABLE		BIT(24)
>   #define LDB_CTRL_ASYNC_FIFO_THRESHOLD_MASK	GENMASK(27, 25)
>   
> -#define LVDS_CTRL				0x128
>   #define LVDS_CTRL_CH0_EN			BIT(0)
>   #define LVDS_CTRL_CH1_EN			BIT(1)

It would be good to add a comment here that the bit is poorly named and 
that LVDS_CTRL_LVDS_EN=1 means DISABLE, while LVDS_CTRL_LVDS_EN=0 means 
ENABLE .

> +#define LVDS_CTRL_LVDS_EN			BIT(1)

[...]

With that fixed:

Reviewed-by: Marek Vasut <marex@...x.de>

Thanks!