[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4fe9947f-b190-4dcc-8d1e-f532e6dcb827@collabora.com>
Date: Tue, 6 Feb 2024 14:27:11 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: CK Hu (胡俊光) <ck.hu@...iatek.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
Il 06/02/24 09:57, CK Hu (胡俊光) ha scritto:
> 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.
>
The additional bits are unused on older SoCs and, if set, will be simply ignored;
if we want to prevent setting bits that don't exist on the old ones, that should
be done as a later commit introducing SoC capabilities for those and when the new
capabilities for the new SoCs are introduced anyway.
As of now, this doesn't break anything.
Regards,
Angelo
Powered by blists - more mailing lists