Re: [PATCH v4 2/2] cache: add SMCCC-backed cache invalidate provider

["Bence Csókás" <bence.csokas@arm.com>]

Hi,

On 2026. 10. 14. 17:38, Andre Przywara wrote:
>> +static int smccc_cache_probe(struct arm_smccc_device *sdev)
>> +{
>> +    struct arm_smccc_res res = {};
>> +    struct smccc_cache *cache;
>> +    int ret;
>> +
>> +    arm_smccc_1_1_invoke(sdev->func_id, &res);
> 
> I think you should provide the explicit function ID name here, and not 
> rely on which function exactly is used to probe for the interface's 
> existence.

I thought that was the point of the matcher, that you could in theory 
assign the same probe handler to any function ID? For example if there's 
some FW that implements this (or a compatible) function, but at a 
different ID, you would only update the match table? At least that's how 
most subsystems (I2C, PCI, USB) do it.

>> +    if ((s32)res.a0)
>> +        return -ENODEV;
>> +
>> +    cache = cache_coherency_ops_instance_alloc(&smccc_cache_ops,
>> +                           struct smccc_cache, cci);
>> +    if (!cache)
>> +        return -ENOMEM;
>> +
>> +    mutex_init(&cache->lock);
>> +    cache->latency_us = lower_32_bits(res.a2);
>> +    cache->rate_limit = res.a3;
> 
> According to the spec you would need lower_32_bits() for a3 as well - if 
> we are going to use this value. But I don't think we need to, see above.
Where does it say that? My copy of DEN 0028 1.7 H, released 2026/Jul/24, 
only says the following:

   X3 (uint64_t attr_2)
     Rate Limit: The maximum number of SMCCCC_ARCH_CLEAN_INV_MEMREGION 
calls allowed per second. If set to 0, it indicates that this field is 
not reported by the implementation.

See my second-to-last comment on v2 of this patch:

https://lore.kernel.org/lkml/591e7ac9-310e-40f4-ab22-161d6ea8f487@arm.com/

Maybe I have an outdated copy already? ;_;

Bence