Re: [PATCHv2] arm64: dts: ti: k3-j721e-beagleboneai64: Enable ACSPCIE output for PCIe1

[Krzysztof Kozlowski <krzk@kernel.org>]

On 02/12/2024 15:53, Siddharth Vadapalli wrote:
> On Mon, Dec 02, 2024 at 12:07:17PM +0100, Krzysztof Kozlowski wrote:
> 
> Hello Krzysztof,
> 
>> On 02/12/2024 11:58, Siddharth Vadapalli wrote:
>>> On Mon, Dec 02, 2024 at 11:14:46AM +0100, Krzysztof Kozlowski wrote:
>>>
>>> Hello Krzysztof,
>>>
>>>> On 02/12/2024 11:11, Romain Naour wrote:
>>>>> From: Romain Naour <romain.naour@skf.com>
>>>>>
>>>>> Unlike the SK-TDA4VM (k3-j721e-sk) board, there is no clock generator
>>>>> (CDCI6214RGET) on the BeagleBone AI-64 (k3-j721e-beagleboneai64) to
>>>>> provide PCIe refclk signal to PCIe Endponts. So the ACSPCIE module must
>>>>> provide refclk through PCIe_REFCLK pins.
>>>>>
>>>>> Use the new "ti,syscon-acspcie-proxy-ctrl" property to enable ACSPCIE
>>>>> module's PAD IO Buffers.
>>>>>
>>>>> Reuse the compatible "ti,j784s4-acspcie-proxy-ctrl" since the ACSPCIE
>>>>> buffer and its functionality is the same across all K3 SoCs.
>>>>>
>>>>> Cc: Siddharth Vadapalli <s-vadapalli@ti.com>
>>>>> Signed-off-by: Romain Naour <romain.naour@skf.com>
>>>>> ---
>>>>> With this patch, we can remove "HACK: Sierra: Drive clock out" patch
>>>>> applied on vendor kernel for BeagleBone AI-64:
>>>>> https://openbeagle.org/beagleboard/linux/-/commit/ad65d7ef675966cdbc5d75f2bd545fad1914ba9b
>>>>>
>>>>> v2:
>>>>>  - use generic style comments
>>>>>  - use "syscon" as generic node name for "acspcie0_proxy_ctrl" node
>>>>>  - Keep the compatible "ti,j784s4-acspcie-proxy-ctrl" since the
>>>>>    ACSPCIE buffer and its functionality is the same across all K3 SoCs.
>>>>>    (Siddharth Vadapalli)
>>>>>
>>>>>    "The compatible "ti,j784s4-acspcie-pcie-ctrl" should be reused for
>>>>>    J721E and all other K3 SoCs.
>>>>
>>>> No, it shouldn't and you got comment on this. You always need specific
>>>> compatible, see writing bindings doc.
>>>
>>> Could you please clarify in which cases reusing the compatible is
>>> permissible? The list of compatibles at:
>>
>> Never? You always need specific compatible. Did you read the writing
>> bindings document?
> 
> I went through the bindings document again at:
> https://www.kernel.org/doc/Documentation/devicetree/bindings/writing-bindings.rst
> It mentions:
> - DON'T use 'syscon' alone without a specific compatible string. A 'syscon'
>   hardware block should have a compatible string unique enough to infer the
>   register layout of the entire block (at a minimum).
> 
> The ACSCPCIE Block as well as its integration across all of TI's K3 SoCs
> is the same i.e. same Hardware/IP. The register bits corresponding to


And first rule for compatible property? DT bindings maintainers keep
repeating it over and over - specific means soc as front compatible.

> the feature to be enabled/disabled via the ACSPCIE block are the same as
> well i.e. "register layout can be inferred". The same goes for the
> compatibles listed below in my previous reply i.e. they aren't bugs.
> Same IP and integration across SoCs and hence reused in the sense of
> Hardware and not Software. I hope this clarifies the rationale for the
> "reuse".


You mix re-use with fallback. These are almost never the same blocks,
which you imply here.

Best regards,
Krzysztof