Re: [PATCH v8 6/9] dt-bindings: qcom-qce: Add new SoC compatible strings for qcom-qce

From: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Thara Gopinath <thara.gopinath@gmail.com>,
	Bhupesh Sharma <bhupesh.sharma@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	Andy Gross <agross@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-crypto@vger.kernel.org, Rob Herring <robh@kernel.org>,
	Jordan Crouse <jorcrous@amazon.com>
Subject: Re: [PATCH v8 6/9] dt-bindings: qcom-qce: Add new SoC compatible strings for qcom-qce
Date: Thu, 2 Feb 2023 16:09:44 +0200	[thread overview]
Message-ID: <65aefb8a-7384-ce0c-9aab-cb8fd38bc1c6@linaro.org> (raw)
In-Reply-To: <0fc4c509-2db4-0bce-75c6-11835d6987d0@linaro.org>

Hi Krzysztof,

On 2/2/23 15:57, Krzysztof Kozlowski wrote:
> On 02/02/2023 14:50, Vladimir Zapolskiy wrote:
>> From: Bhupesh Sharma <bhupesh.sharma@linaro.org>
>>
>> Newer Qualcomm chips support newer versions of the qce crypto IP, so add
>> soc specific compatible strings for qcom-qce instead of using crypto
>> IP version specific ones.
>>
>> Keep the old strings for backward-compatibility, but mark them as
>> deprecated.
>>
>> Cc: Bjorn Andersson <andersson@kernel.org>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> Tested-by: Jordan Crouse <jorcrous@amazon.com>
>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> ---
>>   .../devicetree/bindings/crypto/qcom-qce.yaml  | 19 +++++++++++++++++--
>>   1 file changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
>> index a159089e8a6a..4e0b63b85267 100644
>> --- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
>> +++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
>> @@ -15,7 +15,22 @@ description:
>>   
>>   properties:
>>     compatible:
>> -    const: qcom,crypto-v5.1
>> +    oneOf:
>> +      - const: qcom,crypto-v5.1
>> +        deprecated: true
>> +        description: Kept only for ABI backward compatibility
>> +      - items:
> 
> Drop items.
> 
>> +          - enum:
>> +              - qcom,ipq4019-qce
>> +              - qcom,ipq6018-qce
>> +              - qcom,ipq8074-qce
>> +              - qcom,msm8996-qce
>> +              - qcom,sdm845-qce
>> +              - qcom,sm8150-qce
>> +              - qcom,sm8250-qce
>> +              - qcom,sm8350-qce
>> +              - qcom,sm8450-qce
>> +              - qcom,sm8550-qce
> 
> Unfortunately my comments from v6 was not addressed, nor responded to.
> 
> We already got a public comment from community that we handle Qualcomm
> bindings in a too loose way. I don't think we should be doing this (so
> keep ignoring ABI), just for the sanity of cleanup.
> 
> It's fine to discuss it with me, but since v6 there was no discussion,
> so let's be clear here - NAK on ABI break.

Can you please elaborate, what is the ABI break you find here?

As for me it looks like an incremental change, thus I don't understand
your comment why ABI is broken.

--
Best wishes,
Vladimir