Message-ID: <8cf36a4b-2070-2e79-c06d-b0ec06d8b9f7@linaro.org>
Date:   Thu, 2 Feb 2023 15:20:27 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH v8 9/9] crypto: qce: core: Add new compatibles for qce
 crypto driver
Content-Language: en-US
To:     Vladimir Zapolskiy <vladimir.zapolskiy@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>
References: <20230202135036.2635376-1-vladimir.zapolskiy@linaro.org>
 <20230202135036.2635376-10-vladimir.zapolskiy@linaro.org>
 <6577abf2-7717-b952-13d7-9143200f24fc@linaro.org>
 <397bcc25-dd5e-808f-a38b-15e6c18db669@linaro.org>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <397bcc25-dd5e-808f-a38b-15e6c18db669@linaro.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 02/02/2023 15:15, Vladimir Zapolskiy wrote:
> Hi Krzysztof,
> 
> On 2/2/23 16:01, Krzysztof Kozlowski wrote:
>> On 02/02/2023 14:50, Vladimir Zapolskiy wrote:
>>> From: Bhupesh Sharma <bhupesh.sharma@linaro.org>
>>>
>>> Since we decided to use soc specific compatibles for describing
>>> the qce crypto IP nodes in the device-trees, adapt the driver
>>> now to handle the same.
>>>
>>> Keep the old deprecated compatible strings still in the driver,
>>> to ensure backward compatibility.
>>>
>>> Cc: Bjorn Andersson <andersson@kernel.org>
>>> Cc: Rob Herring <robh@kernel.org>
>>> Cc: herbert@gondor.apana.org.au
>>> Tested-by: Jordan Crouse <jorcrous@amazon.com>
>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
>>> [vladimir: added more SoC specfic compatibles]
>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>> ---
>>>   drivers/crypto/qce/core.c | 12 ++++++++++++
>>>   1 file changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
>>> index 8e496fb2d5e2..2420a5ff44d1 100644
>>> --- a/drivers/crypto/qce/core.c
>>> +++ b/drivers/crypto/qce/core.c
>>> @@ -291,8 +291,20 @@ static int qce_crypto_remove(struct platform_device *pdev)
>>>   }
>>>   
>>>   static const struct of_device_id qce_crypto_of_match[] = {
>>> +	/* Following two entries are deprecated (kept only for backward compatibility) */
>>>   	{ .compatible = "qcom,crypto-v5.1", },
>>>   	{ .compatible = "qcom,crypto-v5.4", },
>>> +	/* Add compatible strings as per updated dt-bindings, here: */
>>> +	{ .compatible = "qcom,ipq4019-qce", },
>>> +	{ .compatible = "qcom,ipq6018-qce", },
>>> +	{ .compatible = "qcom,ipq8074-qce", },
>>> +	{ .compatible = "qcom,msm8996-qce", },
>>> +	{ .compatible = "qcom,sdm845-qce", },
>>> +	{ .compatible = "qcom,sm8150-qce", },
>>> +	{ .compatible = "qcom,sm8250-qce", },
>>> +	{ .compatible = "qcom,sm8350-qce", },
>>> +	{ .compatible = "qcom,sm8450-qce", },
>>> +	{ .compatible = "qcom,sm8550-qce", },
>> I did not agree with this at v7 and I still do not agree. We already did
>> some effort to clean this pattern in other drivers, so to make it clear
>> - driver does not need 10 compatibles because they are the same.
> 
> Here is a misunderstanding, the compatibles are not the same and it shall
> not be assumed this way, only the current support of the IP on different SoCs
> in the driver is the same.

They are the same for the driver. It's the same what we fixed for SDHCI
and other cases. Why this should be treated differently?

> 
> Later on every minor found difference among IPs will require to break DTB ABI,
> if all of the particular SoC specific comaptibles are not listed.

No, why? Why SDHCI and hundreds of other devices are not affected and
this one is?

Best regards,
Krzysztof