Re: [PATCH v2 02/11] soc: qcom: mdtloader: Add context aware qcom_mdt_pas_load() helper

[Bryan O'Donoghue <bryan.odonoghue@linaro.org>]

On 04/09/2025 10:52, Mukesh Ojha wrote:
>> So is it really the intention of this patch to change the callsites where
>> qcom_mdt_pas_load() away from the init version to the no_init version ?
>>
>> Maybe its a symptom of patch collision but hmm, having a hard time
>> cherry-picking this to test.
> My intention is to unify all subsystems whether it's remoteproc, video,
> or others using Secure PIL, so that they all use the same set of APIs
> via context (cxt).
> 
> Like, we first initialize the context, and then use it for other PIL-related
> SMC calls such as pas_init, mem_setup, auth_and_reset, or even for the
> new rsc_table SMC call. This way, everything is connected, and it
> becomes clear which functions need to be called and it will also be
> extensible via a small handling for SHMbridge on gunyah absence. Similar
> changes would also apply to the MDT loader APIs.
> 
> That's why I asked here after "---" in this patch if this approach is
> preferred, I will apply it consistently and eliminate redundant APIs.
> 
> Let me know your thought.

For me its a question of digesting the change.

Your series says "Add context aware qcom_mdt_pas_load()" but, it does 
more than add - it changes logic.

At a minimum I'd suggest splitting the addition of the function from the 
changing of the existing logic so that the two could be disambiguated.

The other comment I have is, is this change really required to enable 
pil loading @ EL2 ?

You could for example structure this series to implement the changes you 
need for EL2 - and then have a last patch at the end to make the code 
"more beautiful" or even a second series to do that.

So I'd suggest one of

1. Splitting the addition of the new helper and its use into
    separate patches in the same series.

or

2. Doing the full EL2 conversion and then applying the
    "make the code look nice" patch last.
    So that we could for example take 11 of 13 patches

or

3. Making the EL2 conversion and the posting a second series
    with your proposed tidy up

Either way for me splicing both the addition and the usage here is a bit 
hard to parse, especially since I can't seem to find a public SHA where 
this series cleanly applies.

---
bod