On Thu, Sep 04, 2025 at 11:15:29AM +0100, Bryan O'Donoghue wrote: > 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. I agree, I did more than just add even used in current patch itself. Will split it. > > The other comment I have is, is this change really required to enable pil > loading @ EL2 ? Not exactly, but to looks things cleaner.. As the other way was carry extra boolean "rproc->has_iommu" in qcom_mdt_pas_init() for rproc and qcom_mdt_load() for video and other smc function to tell what needs to be done extra when Linux at 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. I'm fine with 2 and 3 as well only if non-cleaner way with EL2 enablement gets accepted which may look ugly with lots of if's in the code or just do 1 for now. > > --- > bod -- -Mukesh Ojha
