linux-kernel - Re: [PATCH v2 1/4] drm/mediatek: dsi: Use GENMASK() for register mask definitions

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] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAC=S1ng+kkJS-g3FHe4SC-r1a6hegj3=Any07TCdvTU_apvZNg@mail.gmail.com>
Date: Tue, 26 Dec 2023 18:46:18 +0800
From: Fei Shao <fshao@...omium.org>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: chunkuang.hu@...nel.org, p.zabel@...gutronix.de, airlied@...il.com, 
	daniel@...ll.ch, matthias.bgg@...il.com, dri-devel@...ts.freedesktop.org, 
	linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, kernel@...labora.com
Subject: Re: [PATCH v2 1/4] drm/mediatek: dsi: Use GENMASK() for register mask definitions

Hi Angelo,

On Wed, Dec 20, 2023 at 9:57 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com> wrote:
>
> Change magic numerical masks with usage of the GENMASK() macro
> to improve readability.
>
> This commit brings no functional changes.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c | 46 ++++++++++++++++--------------
>  1 file changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index a2fdfc8ddb15..23d2c5be8dbb 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -58,18 +58,18 @@
>
>  #define DSI_TXRX_CTRL          0x18
>  #define VC_NUM                         BIT(1)
> -#define LANE_NUM                       (0xf << 2)
> +#define LANE_NUM                       GENMASK(5, 2)
>  #define DIS_EOT                                BIT(6)
>  #define NULL_EN                                BIT(7)
>  #define TE_FREERUN                     BIT(8)
>  #define EXT_TE_EN                      BIT(9)
>  #define EXT_TE_EDGE                    BIT(10)
> -#define MAX_RTN_SIZE                   (0xf << 12)
> +#define MAX_RTN_SIZE                   GENMASK(15, 12)
>  #define HSTX_CKLP_EN                   BIT(16)
>
>  #define DSI_PSCTRL             0x1c
> -#define DSI_PS_WC                      0x3fff
> -#define DSI_PS_SEL                     (3 << 16)
> +#define DSI_PS_WC                      GENMASK(14, 0)
> +#define DSI_PS_SEL                     GENMASK(19, 16)

GENMASK(17, 16)
>
>  #define PACKED_PS_16BIT_RGB565         (0 << 16)
>  #define LOOSELY_PS_18BIT_RGB666                (1 << 16)
>  #define PACKED_PS_18BIT_RGB666         (2 << 16)
> @@ -109,26 +109,27 @@
>  #define LD0_WAKEUP_EN                  BIT(2)
>
>  #define DSI_PHY_TIMECON0       0x110
> -#define LPX                            (0xff << 0)
> -#define HS_PREP                                (0xff << 8)
> -#define HS_ZERO                                (0xff << 16)
> -#define HS_TRAIL                       (0xff << 24)
> +#define LPX                            GENMASK(7, 0)
> +#define HS_PREP                                GENMASK(15, 8)
> +#define HS_ZERO                                GENMASK(23, 16)
> +#define HS_TRAIL                       GENMASK(31, 24)
>
>  #define DSI_PHY_TIMECON1       0x114
> -#define TA_GO                          (0xff << 0)
> -#define TA_SURE                                (0xff << 8)
> -#define TA_GET                         (0xff << 16)
> -#define DA_HS_EXIT                     (0xff << 24)
> +#define TA_GO                          GENMASK(7, 0)
> +#define TA_SURE                                GENMASK(15, 8)
> +#define TA_GET                         GENMASK(23, 16)
> +#define DA_HS_EXIT                     GENMASK(31, 24)
>
>  #define DSI_PHY_TIMECON2       0x118
> -#define CONT_DET                       (0xff << 0)
> -#define CLK_ZERO                       (0xff << 16)
> -#define CLK_TRAIL                      (0xff << 24)
> +#define CONT_DET                       GENMASK(7, 0)
> +#define DA_HS_SYNC                     GENMASK(15, 8)

This is new, so please introduce it in a separate patch if intended.

The rest looks good to me.

Regards,
Fei


>
> +#define CLK_ZERO                       GENMASK(23, 16)
> +#define CLK_TRAIL                      GENMASK(31, 24)
>
>  #define DSI_PHY_TIMECON3       0x11c
> -#define CLK_HS_PREP                    (0xff << 0)
> -#define CLK_HS_POST                    (0xff << 8)
> -#define CLK_HS_EXIT                    (0xff << 16)
> +#define CLK_HS_PREP                    GENMASK(7, 0)
> +#define CLK_HS_POST                    GENMASK(15, 8)
> +#define CLK_HS_EXIT                    GENMASK(23, 16)
>
>  #define DSI_VM_CMD_CON         0x130
>  #define VM_CMD_EN                      BIT(0)
> @@ -138,13 +139,14 @@
>  #define FORCE_COMMIT                   BIT(0)
>  #define BYPASS_SHADOW                  BIT(1)
>
> -#define CONFIG                         (0xff << 0)
> +/* CMDQ related bits */
> +#define CONFIG                         GENMASK(7, 0)
>  #define SHORT_PACKET                   0
>  #define LONG_PACKET                    2
>  #define BTA                            BIT(2)
> -#define DATA_ID                                (0xff << 8)
> -#define DATA_0                         (0xff << 16)
> -#define DATA_1                         (0xff << 24)
> +#define DATA_ID                                GENMASK(15, 8)
> +#define DATA_0                         GENMASK(23, 16)
> +#define DATA_1                         GENMASK(31, 24)
>
>  #define NS_TO_CYCLE(n, c)    ((n) / (c) + (((n) % (c)) ? 1 : 0))
>
> --
> 2.43.0
>
>