Received-SPF: pass (google.com: domain of linux-kernel+bounces-161893-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <57529d24-8b46-4eed-95ed-463ad3f4190e@collabora.com>
Date: Mon, 29 Apr 2024 10:08:20 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v5 4/7] soc: mediatek: Add MediaTek DVFS Resource
 Collector (DVFSRC) driver
To: Nathan Chancellor <nathan@kernel.org>
Cc: djakov@kernel.org, robh@kernel.org, krzysztof.kozlowski+dt@linaro.org,
 conor+dt@kernel.org, matthias.bgg@gmail.com, lgirdwood@gmail.com,
 broonie@kernel.org, keescook@chromium.org, gustavoars@kernel.org,
 henryc.chen@mediatek.com, linux-pm@vger.kernel.org,
 devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
 linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org,
 kernel@collabora.com, wenst@chromium.org, amergnat@baylibre.com,
 Dawei Chien <dawei.chien@mediatek.com>
References: <20240424095416.1105639-1-angelogioacchino.delregno@collabora.com>
 <20240424095416.1105639-5-angelogioacchino.delregno@collabora.com>
 <20240424190405.GA2803128@dev-arch.thelio-3990X>
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Content-Language: en-US
In-Reply-To: <20240424190405.GA2803128@dev-arch.thelio-3990X>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Il 24/04/24 21:04, Nathan Chancellor ha scritto:
> Hi Angelo,
> 
> On Wed, Apr 24, 2024 at 11:54:13AM +0200, AngeloGioacchino Del Regno wrote:
>> The Dynamic Voltage and Frequency Scaling Resource Collector (DVFSRC) is a
>> Hardware module used to collect all the requests from both software and the
>> various remote processors embedded into the SoC and decide about a minimum
>> operating voltage and a minimum DRAM frequency to fulfill those requests in
>> an effort to provide the best achievable performance per watt.
>>
>> This hardware IP is capable of transparently performing direct register R/W
>> on all of the DVFSRC-controlled regulators and SoC bandwidth knobs.
>>
>> This driver includes support for MT8183, MT8192 and MT8195.
>>
>> Co-Developed-by: Dawei Chien <dawei.chien@mediatek.com>
>> [Angelo: Partial refactoring and cleanups]
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ...
>>   drivers/soc/mediatek/mtk-dvfsrc.c        | 551 +++++++++++++++++++++++
> ...
>> +#define KBPS_TO_MBPS(x)			((x) / 1000)
> ...
>> +static void __dvfsrc_set_dram_bw_v1(struct mtk_dvfsrc *dvfsrc, u32 reg,
>> +				    u16 max_bw, u16 min_bw, u64 bw)
>> +{
>> +	u32 new_bw = (u32)div_u64(KBPS_TO_MBPS(bw), 100);
>> +
>> +	/* If bw constraints (in mbps) are defined make sure to respect them */
>> +	if (max_bw)
>> +		new_bw = min(new_bw, max_bw);
>> +	if (min_bw && new_bw > 0)
>> +		new_bw = max(new_bw, min_bw);
>> +
>> +	dvfsrc_writel(dvfsrc, reg, new_bw);
>> +}
> 
> Using KBPS_TO_MBPS here results in
> 
>    ERROR: modpost: "__aeabi_uldivmod" [drivers/soc/mediatek/mtk-dvfsrc.ko] undefined!
> 
> when building ARCH=arm allmodconfig with clang. I did not check to see
> if this is visible with GCC but if it is not, it is only because GCC
> implements certain transformations for constant division that clang may
> or may not have implemented (there was some work on getting all
> transformations that GCC has supported in clang as well but I do not
> think was ever completed). Perhaps KBPS_TO_MBPS() should be dropped and
> the new_bw assignement turned into
> 
>    u32 new_bw = (u32)div_u64(bw, 100 * 1000); /* Multiply divisor by 1000 to convert bw from Kbps to Mbps */

Thanks, I honestly didn't check clang.

Your suggestion looks good, I'll do exactly that.

Cheers!
Angelo

> 
> or something like that.
> 
> Cheers,
> Nathan