Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <95cbcda8-d1bc-376c-b338-92d1b923f04a@linaro.org>
Date:   Thu, 28 Jul 2022 09:48:13 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.12.0
Subject: Re: [PATCH 4/4] dt-bindings: firmware: Add Qualcomm UEFI Secure
 Application client
Content-Language: en-US
To:     Maximilian Luz <luzmaximilian@gmail.com>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Ard Biesheuvel <ardb@kernel.org>
Cc:     Konrad Dybcio <konrad.dybcio@somainline.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Steev Klimaszewski <steev@kali.org>,
        Shawn Guo <shawn.guo@linaro.org>,
        Sudeep Holla <sudeep.holla@arm.com>,
        Cristian Marussi <cristian.marussi@arm.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-arm-msm@vger.kernel.org, linux-efi@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
        Vinod Koul <vkoul@kernel.org>
References: <20220723224949.1089973-1-luzmaximilian@gmail.com>
 <20220723224949.1089973-5-luzmaximilian@gmail.com>
 <e88d1036-dc58-3fc8-c388-edba9b2d62a7@linaro.org>
 <87c19c5a-d7f4-7183-1322-f62267e01b3b@gmail.com>
 <11e5c369-c0da-7756-b9e2-ac375dc78e9d@linaro.org>
 <2e522bcd-5d55-e87f-126c-514f5edaa560@gmail.com>
 <53a602e2-0590-6c6a-597b-fd55faa3a4ab@linaro.org>
 <acd7b231-3167-e35c-5cdf-8b3127a7d710@gmail.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <acd7b231-3167-e35c-5cdf-8b3127a7d710@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 27/07/2022 15:00, Maximilian Luz wrote:
>>> Then again, if you prefer to name everything based on
>>> "qcom,<device>-<soc>" I don't have any strong arguments against it and
>>> I'm happy to change that. I just think it will unnecessarily introduce
>>> a bunch of compatibles and doesn't reflect the interface "versioning"
>>> situation as I see it.
>>
>> Why bunch? All devices could bind to one specific compatible, as they
>> are compatible.
> 
> Ah, I think I misunderstood you there. I thought you were advocating for
> creating compatibles for each SoC just because it's a new SoC and things
> might be different. I'm not at all against naming this something like
> qcom,tee-uefisecapp-sc8180x then using that on all platforms that work.
> I just didn't like the idea of having a bunch of different
> qcom,tee-uefisecapp-<soc> pointing to the exact same thing without any
> difference at all.

You start with one specific compatible and if needed later either add
more specific upfront (qcom,sc8280x-tee-uefisecapp,
qcom,sc8180x-tee-uefisecapp) or as entirely new one if it is not compatible.

> 
>>>>>>> +
>>>>>>> +required:
>>>>>>> +  - compatible
>>>>>>> +
>>>>>>> +additionalProperties: false
>>>>>>> +
>>>>>>> +examples:
>>>>>>> +  - |
>>>>>>> +    firmware {
>>>>>>> +        scm {
>>>>>>> +            compatible = "qcom,scm-sc8180x", "qcom,scm";
>>>>>>> +        };
>>>>>>> +        tee-uefisecapp {
>>>>>>> +            compatible = "qcom,tee-uefisecapp";
>>>>>>
>>>>>> You did not model here any dependency on SCM. This is not full
>>>>>> description of the firmware/hardware
>>>>>
>>>>> How would I do that? A lot of other stuff also depends on SCM being
>>>>> present (e.g. qcom_q6v5_pas for loading mdt files) and I don't see them
>>>>> declare this in the device tree. As far as I can tell, SCM is pretty
>>>>> much expected to be there at all times (i.e. can't be unloaded) and
>>>>> drivers check for it when probing via qcom_scm_is_available(),
>>>>> deferring probe if not.
>>>>
>>>> It seems this will be opening a can of worms...
>>>
>>> Indeed.
>>>
>>>> The problem with existing approach is:
>>>> 1. Lack of any probe ordering or probe deferral support.
>>>> 2. Lack of any other dependencies, e.g. for PM.
>>>
>>> I'm not entirely sure what you mean by "lack of probe deferral support".
>>> We have qcom_scm_is_available() and defer probe if that fails. So
>>> deferral works, unless I'm misunderstanding something.
>>
>> And how do you differentiate that qcom_scm_is_available() failed because
>> it is not yet available (defer probe) or it is broken and will never
>> load? All regular consumer-provider interfaces have it sorted out.
> 
> Fair point. By shifting that to device links you'll at least know what
> it's waiting for and the driver won't attempt to probe until that's
> resolved. But your question applies to that then as well: How do you
> differentiate between the device link or supplier being broken somehow
> and the supplier being just not ready yet?

For example like tegra_bpmp_get() is doing.

Best regards,
Krzysztof