linux-kernel - Re: [PATCH v3 1/7] 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: <082bc4d9efd0746d7ec25eab0b3bf96018e997e5.camel@mediatek.com>
Date: Tue, 6 Feb 2024 08:57:08 +0000
From: CK Hu (胡俊光) <ck.hu@...iatek.com>
To: "angelogioacchino.delregno@...labora.com"
	<angelogioacchino.delregno@...labora.com>, "chunkuang.hu@...nel.org"
	<chunkuang.hu@...nel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"kernel@...labora.com" <kernel@...labora.com>, "daniel@...ll.ch"
	<daniel@...ll.ch>, "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	"airlied@...il.com" <airlied@...il.com>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "matthias.bgg@...il.com"
	<matthias.bgg@...il.com>
Subject: Re: [PATCH v3 1/7] drm/mediatek: dsi: Use GENMASK() for register mask
 definitions

Hi, Angelo:

On Wed, 2024-01-31 at 12:34 +0100, AngeloGioacchino Del Regno wrote:
> Change magic numerical masks with usage of the GENMASK() macro
> to improve readability.
> 
> While at it, also fix the DSI_PS_SEL mask to include all bits instead
> of just a subset of them.
> 
> This commit brings no functional changes.
> 
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@...labora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c | 45 +++++++++++++++-------------
> --
>  1 file changed, 23 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index a2fdfc8ddb15..3b7392c03b4d 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)

The original definition of DSI_PS_WC/DSI_PS_SEL is correct in MT8173.
So both need two definition and let each SoC select its own definition.

Regards,
CK

>  #define PACKED_PS_16BIT_RGB565		(0 << 16)
>  #define LOOSELY_PS_18BIT_RGB666		(1 << 16)
>  #define PACKED_PS_18BIT_RGB666		(2 << 16)
> @@ -109,26 +109,26 @@
>  #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 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 +138,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))
>