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

[Marek Vasut]

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@xxxxxxx>
> ---
> 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@xxxxxxx>

Thanks!