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: <0255ff85-71e2-2174-2188-82899b6e9678@linaro.org>
Date:   Tue, 11 Apr 2023 19:17:16 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.1
Subject: Re: [PATCH 1/2] dt-bindings: firmware: arm,scmi: support parameter
 passing in smc/hvc
Content-Language: en-US
To:     Nikunj Kela <quic_nkela@quicinc.com>, sudeep.holla@arm.com
Cc:     cristian.marussi@arm.com, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org,
        linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20230409181918.29270-1-quic_nkela@quicinc.com>
 <20230409181918.29270-2-quic_nkela@quicinc.com>
 <5d264f46-b9e8-62ec-9bce-9d666fd1eaa8@linaro.org>
 <74551421-56df-73d3-3f4d-1708768e207a@quicinc.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <74551421-56df-73d3-3f4d-1708768e207a@quicinc.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 10/04/2023 19:33, Nikunj Kela wrote:
> 
> On 4/10/2023 10:20 AM, Krzysztof Kozlowski wrote:
>> On 09/04/2023 20:19, Nikunj Kela wrote:
>>> Currently, smc/hvc calls are made with smc-id only. The parameters are
>>> all set to zeros. This patch defines two optional device tree bindings,
>>> that can be used to pass parameters in smc/hvc calls.
>>>
>>> This is useful when multiple scmi instances are used with common smc-id.
>>>
>>> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
>>> ---
>>>   .../devicetree/bindings/firmware/arm,scmi.yaml   | 16 ++++++++++++++++
>>>   1 file changed, 16 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>>> index 5824c43e9893..08c331a79b80 100644
>>> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>>> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>>> @@ -115,6 +115,22 @@ properties:
>>>       description:
>>>         SMC id required when using smc or hvc transports
>>>   
>>> +  arm,smc32-params:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +    description:
>>> +      An optional parameter list passed in smc32 or hvc32 calls
>>> +    default: 0
>>> +    minItems: 1
>>> +    maxItems: 6
>>> +
>>> +  arm,smc64-params:
>>> +    $ref: /schemas/types.yaml#/definitions/uint64-array
>>> +    description:
>>> +      An optional parameter list passed in smc64 or hvc64 calls
>>> +    default: 0
>>> +    minItems: 1
>>> +    maxItems: 6
>> These do not look like hardware properties and the fact that you need
>> two properties for the same also points that you tied it to specific SW
>> interface.
> 
> This is certainly not the H/W property but then smc-id is also not H/W 
> property either

Yep, maybe it should be also removed?

> 
> but that gets passed via DTB. I could use the same property for both 
> however I wasn't sure
> 
> which datatype should be used, uint32-array/uint64-array. Moreover, I 
> thought if users are
> 
> passing parameters, they better know which SMC convention they are using 
> hence used two
> 
> explicit properties.

Does not solve my concern.

> 
>> Why this should be board-specific? Actually better question - why this
>> should be fixed per board? Doesn't my software want to have different
>> parameters, depending on some other condition?
> 
> Not sure I follow, I didn't say this is board specific.

But putting it in DTS which is customized per board is then board specific.

>  People can use 
> the parameters to pass
> 
> whatever their S/W demands. SMC/HVC calls are made by passing parameters 
> which is what this patch is enabling.

I cannot follow your paragraphs. You cut middle of sentence.

Anyway, if this is to match whatever their SW demands, it is not
Devicetree property.

> 
>>
>> You also did not provide any DTS user for this, so difficult to judge
>> usefulness.
> 
> The work is still on going and we will push the dts in few months, 
> however that shouldn't stop
> 
> making changes in advance.

With a proper DTS maybe it would be easier to convince us why SW
interface should be put to Devicetree... but sure, we can skip DTS and
answer is - this looks like SW property and not a DT.

Best regards,
Krzysztof