On 11/26/22 15:09, Lad, Prabhakar wrote:
>>> + if (!ax45mp_priv->l2c_base) {
>>> + ret = -ENOMEM;
>>> + goto l2c_err;
>>> + }
>>> +
>>> + ret = ax45mp_configure_l2_cache(np);
>>> + if (ret)
>>> + goto l2c_err;
>>> +
>>> + ret = ax45mp_configure_pma_regions(np);
>>> + if (ret)
>>> + goto l2c_err;
>>> +
>>> + static_branch_disable(&ax45mp_l2c_configured);
>>
>> Instead of enabling this before the probe function, and disabling it
>> afterward, just enable it once here, in the success case. Then you can
>> drop the !ax45mp_priv check in the functions above.
>>
> I think I had tried it but static_branch_unlikely() was always returning true.
You use DEFINE_STATIC_KEY_FALSE above, so static_branch_unlikely()
should return false until you call static_branch_enable().
>> And none of the functions would get called anyway if the alternative is
>> not applied. I suppose it's not possible to do some of this probe logic
>> in the alternative check function?
>>
> you mean to check in the vendor errata patch function to see if this
> driver has probed?
I meant to do the equivalent of:
+ ax45mp_priv->ucctl_ok = ax45mp_cpu_cache_controlable();
+ ax45mp_priv->l2cache_enabled = ax45mp_cpu_l2c_ctl_status() &
AX45MP_L2_CACHE_CTL_CEN_MASK;
in the errata function, since that decides if the cache maintenance
functions actually do anything. But ax45mp_cpu_l2c_ctl_status() gets the
MMIO address from the DT, and trying to do that from the errata function
could get ugly, so maybe it is not a good suggestion.
Regards,
Samuel
