| Message ID | 20230202135036.2635376-7-vladimir.zapolskiy@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | crypto: qcom-qce: Add YAML bindings & support for newer SoCs | expand |
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. Best regards, Krzysztof
On 02/02/2023 15:09, Vladimir Zapolskiy wrote: > 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. Right, the driver keeps the old one compatible, I missed that, so it actually implements what I asked for in v7. It's fine then from ABI point of view. The remaining trouble is that any user of DTS (other systems, firmware, bootloader) is going to be broken when taking the new DTS. But it's not critical. Best regards, Krzysztof
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: + - 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 reg: maxItems: 1 @@ -68,7 +83,7 @@ examples: - | #include <dt-bindings/clock/qcom,gcc-apq8084.h> crypto-engine@fd45a000 { - compatible = "qcom,crypto-v5.1"; + compatible = "qcom,ipq6018-qce"; reg = <0xfd45a000 0x6000>; clocks = <&gcc GCC_CE2_AHB_CLK>, <&gcc GCC_CE2_AXI_CLK>,