Re: [PATCH v5 1/2] iommu/arm-smmu-v3: Add a cmdq_max_entries module parameter

[Nicolin Chen]

On Mon, Sep 07, 2026 at 10:58:34AM +0100, Kiryl Shutsemau (Meta) wrote:

I still think that cmdq_max_n_shift can slightly tidy things here.

> +static u32 arm_smmu_queue_max_n_shift(u32 ceiling, u32 ent_sz_shift,
> +				      u32 entries)

Here, all three inputs would have been "shifts", instead of two
"shifts" and one "number of entries".

> +{
> +	u32 floor = PAGE_SHIFT - ent_sz_shift;
> +
> +	if (!entries)
> +		return ceiling;
> +
> +	return min(ceiling, max(ilog2(entries), floor));

And I see Sashiko keeps complaining against the ilog2 here:
"
Does this trigger a build failure due to strict type checking?
The ilog2(entries) expression returns a signed int when entries is derived
from a runtime value like the module parameter, while floor is explicitly
declared as an unsigned u32.
Since the kernel's max() macro enforces strict type compatibility, this
mismatch evaluates to 0 in the type checking logic, triggering a signedness
error during compilation.
Should this use max_t(u32, ilog2(entries), floor) instead?
"

Though Sashiko complained about the min_t() in v4 as well..

With cmdq_max_n_shift, there is no ilog2 and no type mismatch.

If you can address or justify the Sashiko finding, I am fine with
the cmdq_max_entries though..

So, in either way, the patch looks good to me,

Reviewed-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
Tested-by: Nicolin Chen <nicolinc@xxxxxxxxxx>