Re: [PATCH v3 1/7] drm/mediatek: dsi: Use GENMASK() for register mask definitions

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

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@collabora.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