Re: [PATCH v6 0/4] dt-bindings: qcom-qce: Convert bindings to yaml & related changes

From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: Bhupesh Sharma <bhupesh.sharma@linaro.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-arm-msm@vger.kernel.org
Cc: agross@kernel.org, linux-kernel@vger.kernel.org,
	robh+dt@kernel.org, thara.gopinath@gmail.com,
	devicetree@vger.kernel.org, robh@kernel.org,
	andersson@kernel.org, bhupesh.linux@gmail.com,
	Jordan Crouse <jorcrous@amazon.com>
Subject: Re: [PATCH v6 0/4] dt-bindings: qcom-qce: Convert bindings to yaml & related changes
Date: Tue, 20 Sep 2022 10:54:37 +0200	[thread overview]
Message-ID: <2ad436c8-8b7a-80ed-9c91-d2293eff70ab@linaro.org> (raw)
In-Reply-To: <dfe1bc9e-2ab9-d2dd-7daa-dddb8d66fd77@linaro.org>

On 20/09/2022 10:48, Bhupesh Sharma wrote:
> 
> On 9/20/22 12:58 PM, Krzysztof Kozlowski wrote:
>> On 20/09/2022 00:08, Bhupesh Sharma wrote:
>>
>> (...)
>>
>>
>>>
>>> Qualcomm crypto engine (qce) is available on several Snapdragon SoCs.
>>> The qce block supports hardware accelerated algorithms for encryption
>>> and authentication. It also provides support for aes, des, 3des
>>> encryption algorithms and sha1, sha256, hmac(sha1), hmac(sha256)
>>> authentication algorithms.
>>>
>>> Note that this patchset is dependent on the dt-bindings patchset (see [1]) sent to devicetree list.
>>>
>>> [1]. https://lore.kernel.org/linux-arm-msm/20220919195618.926227-1-bhupesh.sharma@linaro.org/
>>
>> If it is dependent on the bindings only, keep them together. However I
>> don't think this is the only dependency. You add here several
>> compatibles which are not supported.
> 
> 
> Please go through the cover letter where I mentioned that:
>    'As per Bjorn's suggestion on irc, broke down the patchset into 4
>    separate patchsets, one each for the following areas to allow easier
>    review and handling from the respective maintainer(s):
>          'arm-msm', 'crypto', 'dma' and 'devicetree'
>    This patchset is directed for the 'devicetree' tree / area.'
> 
> Basically now the patchset which had around 23 patches in v5 will send 
> out as 4 separate patchsets one each for 'arm-msm', 'crypto', 'dma' and 
> 'devicetree' trees.
> 
> So when all the respective subsets are picked up, all the compatibles 
> are in place.

and none of reviewers can find them, because you linked only bindings.
Keeping bindings separate from everything is not good approach. Either
they should be with DTS or with driver changes. Otherwise how can we
even look that they are matching DTS?

Keeping them separate even makes impression there are no ABI breaks and
bisectability issues...

Best regards,
Krzysztof