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

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

[Romain Naour]

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.
   For example, see:
   https://lore.kernel.org/r/20240402105708.4114146-1-s-vadapalli@ti.com/
   which introduced "ti,am62p-cpsw-mac-efuse" compatible.

   The same compatible is reused across all K3 SoCs:
   https://lore.kernel.org/r/20240628151518.40100-1-afd@ti.com/ "
---
 arch/arm64/boot/dts/ti/k3-j721e-beagleboneai64.dts |  5 +++++
 arch/arm64/boot/dts/ti/k3-j721e-main.dtsi          | 10 ++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-j721e-beagleboneai64.dts b/arch/arm64/boot/dts/ti/k3-j721e-beagleboneai64.dts
index fb899c99753e..741ad2ba6fdb 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e-beagleboneai64.dts
+++ b/arch/arm64/boot/dts/ti/k3-j721e-beagleboneai64.dts
@@ -859,6 +859,11 @@ &pcie1_rc {
 	num-lanes = <2>;
 	max-link-speed = <3>;
 	reset-gpios = <&main_gpio0 22 GPIO_ACTIVE_HIGH>;
+	/*
+	 * There is no on-board or external reference clock generators,
+	 * use refclk from the ACSPCIE module's PAD IO Buffers.
+	 */
+	ti,syscon-acspcie-proxy-ctrl = <&acspcie0_proxy_ctrl 0x3>;
 };
 
 &ufs_wrapper {
diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
index af3d730154ac..b9bb59ce4ed2 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
@@ -5,6 +5,7 @@
  * Copyright (C) 2016-2024 Texas Instruments Incorporated - https://www.ti.com/
  */
 #include <dt-bindings/phy/phy.h>
+#include <dt-bindings/phy/phy-cadence.h>
 #include <dt-bindings/phy/phy-ti.h>
 #include <dt-bindings/mux/mux.h>
 
@@ -82,6 +83,11 @@ ehrpwm_tbclk: clock-controller@4140 {
 			reg = <0x4140 0x18>;
 			#clock-cells = <1>;
 		};
+
+		acspcie0_proxy_ctrl: syscon@18090 {
+			compatible = "ti,j784s4-acspcie-proxy-ctrl", "syscon";
+			reg = <0x18090 0x4>;
+		};
 	};
 
 	main_ehrpwm0: pwm@3000000 {
@@ -979,8 +985,8 @@ pcie1_rc: pcie@2910000 {
 		max-link-speed = <3>;
 		num-lanes = <2>;
 		power-domains = <&k3_pds 240 TI_SCI_PD_EXCLUSIVE>;
-		clocks = <&k3_clks 240 1>;
-		clock-names = "fck";
+		clocks = <&k3_clks 240 1>, <&serdes1 CDNS_SIERRA_DERIVED_REFCLK>;
+		clock-names = "fck", "pcie_refclk";
 		#address-cells = <3>;
 		#size-cells = <2>;
 		bus-range = <0x0 0xff>;
-- 
2.45.0

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

[Krzysztof Kozlowski]

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.

Best regards,
Krzysztof

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

[Siddharth Vadapalli]

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:
https://github.com/torvalds/linux/blob/v6.12/Documentation/devicetree/bindings/mfd/syscon.yaml#L112
namely,
          - ti,am62-opp-efuse-table
          - ti,am62-usb-phy-ctrl
          - ti,am625-dss-oldi-io-ctrl
          - ti,am62p-cpsw-mac-efuse
          - ti,am654-dss-oldi-io-ctrl
          - ti,j784s4-pcie-ctrl
have all been reused for different TI SoCs since they all correspond to the
device functionality enabled via the CTRL_MMR System Controller registers.
The compatible "ti,j784s4-acspcie-pcie-ctrl" has also been added to the
list:
https://github.com/torvalds/linux/blob/v6.12/Documentation/devicetree/bindings/mfd/syscon.yaml#L117
with the intent of reusing it the same way that other compatibles have
been reused.

Regards,
Siddharth.

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

[Krzysztof Kozlowski]

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?

> https://github.com/torvalds/linux/blob/v6.12/Documentation/devicetree/bindings/mfd/syscon.yaml#L112
> namely,
>           - ti,am62-opp-efuse-table
>           - ti,am62-usb-phy-ctrl
>           - ti,am625-dss-oldi-io-ctrl
>           - ti,am62p-cpsw-mac-efuse
>           - ti,am654-dss-oldi-io-ctrl
>           - ti,j784s4-pcie-ctrl
> have all been reused for different TI SoCs since they all correspond to the
> device functionality enabled via the CTRL_MMR System Controller registers.

If you find a bug, does it mean you can send new patch introducing the
same bug?

> The compatible "ti,j784s4-acspcie-pcie-ctrl" has also been added to the
> list:
> https://github.com/torvalds/linux/blob/v6.12/Documentation/devicetree/bindings/mfd/syscon.yaml#L117
> with the intent of reusing it the same way that other compatibles have
> been reused.

And?

Best regards,
Krzysztof

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

[Siddharth Vadapalli]

On Mon, Dec 02, 2024 at 12:07:17PM +0100, Krzysztof Kozlowski wrote:
> 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?
> 
> > https://github.com/torvalds/linux/blob/v6.12/Documentation/devicetree/bindings/mfd/syscon.yaml#L112
> > namely,
> >           - ti,am62-opp-efuse-table
> >           - ti,am62-usb-phy-ctrl
> >           - ti,am625-dss-oldi-io-ctrl
> >           - ti,am62p-cpsw-mac-efuse
> >           - ti,am654-dss-oldi-io-ctrl
> >           - ti,j784s4-pcie-ctrl
> > have all been reused for different TI SoCs since they all correspond to the
> > device functionality enabled via the CTRL_MMR System Controller registers.
> 
> If you find a bug, does it mean you can send new patch introducing the
> same bug?

Thank you for clarifying. Since the above compatibles have been reused,
they served as an example and appeared as the norm. But as you said, the
reference should be the bindings document and not the patches that got
through and ended up in the bindings. I will make sure that I add new
compatibles for each SoC going forward, despite what the existing
implementation suggests.

Regards,
Siddharth.

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

[Siddharth Vadapalli]

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
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".

> 
> > https://github.com/torvalds/linux/blob/v6.12/Documentation/devicetree/bindings/mfd/syscon.yaml#L112
> > namely,
> >           - ti,am62-opp-efuse-table
> >           - ti,am62-usb-phy-ctrl
> >           - ti,am625-dss-oldi-io-ctrl
> >           - ti,am62p-cpsw-mac-efuse
> >           - ti,am654-dss-oldi-io-ctrl
> >           - ti,j784s4-pcie-ctrl
> > have all been reused for different TI SoCs since they all correspond to the
> > device functionality enabled via the CTRL_MMR System Controller registers.
> 
> If you find a bug, does it mean you can send new patch introducing the
> same bug?

[...]

Regards,
Siddharth.

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

[Krzysztof Kozlowski]

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

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

[Siddharth Vadapalli]

On Mon, Dec 02, 2024 at 04:09:51PM +0100, Krzysztof Kozlowski wrote:
> 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.

I know that the IP is the same, the bits are the same and those bits enable
the same functionality of the IP across the SoCs. If you still insist that
they are not same, I don't know what to say anymore.

Regards,
Siddharth.

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

[Krzysztof Kozlowski]

On 02/12/2024 16:45, Siddharth Vadapalli wrote:
>>> 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.
> 
> I know that the IP is the same, the bits are the same and those bits enable
> the same functionality of the IP across the SoCs. If you still insist that
> they are not same, I don't know what to say anymore.


You can say what we have been saying on mailing lists all the time:
hardware datasheets lie and sometimes you find one, tiny tiny
difference. If you are uncertain, please consult your SoC maintainer on
this matter.

Best regards,
Krzysztof