<html><body><p> <pre> Hi, Angelo: On Tue, 2024-02-06 at 14:27 +0100, AngeloGioacchino Del Regno wrote: > 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_CTRL0x18 > > > #define VC_NUMBIT(1) > > > -#define LANE_NUM(0xf << 2) > > > +#define LANE_NUMGENMASK(5, 2) > > > #define DIS_EOTBIT(6) > > > #define NULL_ENBIT(7) > > > #define TE_FREERUNBIT(8) > > > #define EXT_TE_ENBIT(9) > > > #define EXT_TE_EDGEBIT(10) > > > -#define MAX_RTN_SIZE(0xf << 12) > > > +#define MAX_RTN_SIZEGENMASK(15, 12) > > > #define HSTX_CKLP_ENBIT(16) > > > > > > #define DSI_PSCTRL0x1c > > > -#define DSI_PS_WC0x3fff > > > -#define DSI_PS_SEL(3 << 16) > > > +#define DSI_PS_WCGENMASK(14, 0) > > > +#define DSI_PS_SELGENMASK(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. The title of this patch is only to use GENMASK(), but here does more things. I agree this does not break anything, but I would like to separate this to an independent patch just for new bits. In your later patch, DSI_PS_WC is not used any more. So maybe after that patch, you could define as: #define DSI_PS_WC_MT8173 GENMASK(13, 0) #define DSI_PS_WC_MT8xxx GENMASK(14, 0) DSI_PS_SEL is not used now, so it could also define as: #define DSI_PS_SEL_MT8137 GENMASK(17, 16) #define DSI_PS_SEL_MT8xxx GENMASK(19, 16) And add definition of value 4 ~ 15. Regards, CK > > Regards, > Angelo > > </pre> </p></body></html><pre>************* MEDIATEK Confidentiality Notice ******************** The information contained in this e-mail message (including any attachments) may be confidential, proprietary, privileged, or otherwise exempt from disclosure under applicable laws. It is intended to be conveyed only to the designated recipient(s). Any use, dissemination, distribution, printing, retaining or copying of this e-mail (including its attachments) by unintended recipient(s) is strictly prohibited and may be unlawful. If you are not an intended recipient of this e-mail, or believe that you have received this e-mail in error, please notify the sender immediately (by replying to this e-mail), delete any and all copies of this e-mail (including any attachments) from your system, and do not disclose the content of this e-mail to any other person. Thank you! </pre>