linux-kernel - Re: [PATCH v1 5/6] soc: mediatek: mutex: Add mtk_mutex_set

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>] [day] [month] [year] [list]

Message-ID: <42ab09ef-427d-f2dd-c480-f3e11f4aaa79@collabora.com>
Date:   Tue, 4 Oct 2022 14:38:02 +0200
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>
To:     Moudy Ho <moudy.ho@...iatek.com>, Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Chun-Kuang Hu <chunkuang.hu@...nel.org>
Cc:     linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org,
        Project_Global_Chrome_Upstream_Group@...iatek.com,
        "Roy-CW.Yeh" <roy-cw.yeh@...iatek.com>
Subject: Re: [PATCH v1 5/6] soc: mediatek: mutex: Add mtk_mutex_set_mod
 support to set MOD1

Il 04/10/22 11:33, Moudy Ho ha scritto:
> From: "Roy-CW.Yeh" <roy-cw.yeh@...iatek.com>
> 
> Add mtk_mutex_set_mod support to set MOD1
> 
> Signed-off-by: Roy-CW.Yeh <roy-cw.yeh@...iatek.com>
> ---
>   drivers/soc/mediatek/mtk-mutex.c | 27 +++++++++++++++++++--------
>   1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-mutex.c b/drivers/soc/mediatek/mtk-mutex.c
> index c1a33d52038e..5dcbd61fe42c 100644
> --- a/drivers/soc/mediatek/mtk-mutex.c
> +++ b/drivers/soc/mediatek/mtk-mutex.c
> @@ -23,6 +23,7 @@
>   #define DISP_REG_MUTEX(n)			(0x24 + 0x20 * (n))
>   #define DISP_REG_MUTEX_RST(n)			(0x28 + 0x20 * (n))
>   #define DISP_REG_MUTEX_MOD(mutex_mod_reg, n)	(mutex_mod_reg + 0x20 * (n))
> +#define DISP_REG_MUTEX_MOD1(mutex_mod_reg, n)	((mutex_mod_reg) + 0x20 * (n) + 0x4)
>   #define DISP_REG_MUTEX_SOF(mutex_sof_reg, n)	(mutex_sof_reg + 0x20 * (n))
>   #define DISP_REG_MUTEX_MOD2(n)			(0x34 + 0x20 * (n))
>   
> @@ -750,14 +751,24 @@ int mtk_mutex_write_mod(struct mtk_mutex *mutex,
>   		return -EINVAL;
>   	}
>   
> -	offset = DISP_REG_MUTEX_MOD(mtx->data->mutex_mod_reg,
> -				    mutex->id);
> -	reg = readl_relaxed(mtx->regs + offset);
> -
> -	if (clear)
> -		reg &= ~BIT(mtx->data->mutex_table_mod[idx]);
> -	else
> -		reg |= BIT(mtx->data->mutex_table_mod[idx]);
> +	if (mtx->data->mutex_table_mod[idx] < 32) {

What if we do...

	u8 id_offset = 0;

	/*
	 * Some SoCs may have multiple MUTEX_MOD registers as more than 32 mods
	 * are present, hence requiring multiple 32-bits registers.
	 *
	 * The mutex_table_mod fully represents that by defining the number of
	 * the mod sequentially, later used as a bit number, which can be more
	 * than 0..31.
	 *
	 * In order to retain compatibility with older SoCs, we perform R/W on
	 * the single 32 bits registers, but this requires us to translate the
	 * mutex ID bit accordingly.
	 */
	if (mtx->data->mutex_table_mod[idx] < 32) {
		reg_offset = DISP_REG_MUTEX_MOD0(mtx->data->mutex_mod_reg,
						 mutex->id);
	} else {
		reg_offset = DISP_REG_MUTEX_MOD1(mtx->data->mutex_mod_reg,
						 mutex->id);
		id_offset = 32;
	}

	reg = readl_relaxed(mtx->regs + offset);
	if (clear)
		reg &= ~BIT(mtx->data->mutex_table_mod[idx] - id_offset);
	else
		reg |= BIT(mtx->data->mutex_table_mod[idx] - id_offset);

	writel_relaxed(reg, mtx->regs + offset);

...like this, we give good documentation and also keep the code "short" :-)

Regards,
Angelo


> +		offset = DISP_REG_MUTEX_MOD(mtx->data->mutex_mod_reg,
> +					    mutex->id);
> +		reg = readl_relaxed(mtx->regs + offset);
> +		if (clear)
> +			reg &= ~BIT(mtx->data->mutex_table_mod[idx]);
> +		else
> +			reg |= BIT(mtx->data->mutex_table_mod[idx]);
> +
> +	} else {
> +		offset = DISP_REG_MUTEX_MOD1(mtx->data->mutex_mod_reg,
> +					     mutex->id);
> +		reg = readl_relaxed(mtx->regs + offset);
> +		if (clear)
> +			reg &= ~BIT(mtx->data->mutex_table_mod[idx] - 32);
> +		else
> +			reg |= BIT(mtx->data->mutex_table_mod[idx] - 32);
> +	}
>   
>   	writel_relaxed(reg, mtx->regs + offset);
>