[PATCH v5 3/3] arm: dts: ti: Add device tree support for PRU-ICSS on AM335x

Parvathi Pudi posted 3 patches 1 month ago
There is a newer version of this series
[PATCH v5 3/3] arm: dts: ti: Add device tree support for PRU-ICSS on AM335x
Posted by Parvathi Pudi 1 month ago
From: Roger Quadros <rogerq@ti.com>

The TI Sitara AM335x ICE-V2 consists of single PRU-ICSS instance,
This patch adds the new device tree overlay file in-order to enable
PRU-ICSS instance, along with makefile changes.

PRU-ICSS instance consists of two PRU cores along with various
peripherals such as the Interrupt Controller (PRU_INTC), the Industrial
Ethernet Peripheral(IEP), the Real Time Media Independent Interface
controller (MII_RT), and the Enhanced Capture (eCAP) event module.

am33xx-l4.dtsi - Adds IEP and eCAP peripheral as child nodes
of the PRUSS subsystem node.

am335x-icev2-prueth.dtso - Adds PRU-ICSS instance node along with PRU
eth port information and corresponding port configuration. It includes
interrupt mapping for packet reception, HW timestamp collection, and PRU
Ethernet ports in MII mode,

GPIO configuration, boot strapping along with delay configuration for
individual PRU Ethernet port and other required nodes.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Andrew F. Davis <afd@ti.com>
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
Signed-off-by: Basharath Hussain Khaja <basharath@couthit.com>
Signed-off-by: Parvathi Pudi <parvathi@couthit.com>
---
 arch/arm/boot/dts/ti/omap/Makefile            |   5 +
 .../ti/omap/am335x-icev2-prueth-overlay.dtso  | 149 ++++++++++++++++++
 arch/arm/boot/dts/ti/omap/am33xx-l4.dtsi      |  11 ++
 3 files changed, 165 insertions(+)
 create mode 100644 arch/arm/boot/dts/ti/omap/am335x-icev2-prueth-overlay.dtso

diff --git a/arch/arm/boot/dts/ti/omap/Makefile b/arch/arm/boot/dts/ti/omap/Makefile
index 4c577214f512..6fb58fe51648 100644
--- a/arch/arm/boot/dts/ti/omap/Makefile
+++ b/arch/arm/boot/dts/ti/omap/Makefile
@@ -84,6 +84,10 @@ dtb-$(CONFIG_ARCH_OMAP4) += \
 	omap4-samsung-espresso10.dtb \
 	omap4-xyboard-mz609.dtb \
 	omap4-xyboard-mz617.dtb
+
+am335x-icev2-prueth-dtbs := am335x-icev2.dtb \
+	am335x-icev2-prueth-overlay.dtbo
+
 dtb-$(CONFIG_SOC_AM33XX) += \
 	am335x-baltos-ir2110.dtb \
 	am335x-baltos-ir3220.dtb \
@@ -101,6 +105,7 @@ dtb-$(CONFIG_SOC_AM33XX) += \
 	am335x-evmsk.dtb \
 	am335x-guardian.dtb \
 	am335x-icev2.dtb \
+	am335x-icev2-prueth.dtb \
 	am335x-lxm.dtb \
 	am335x-mba335x.dtb \
 	am335x-moxa-uc-2101.dtb \
diff --git a/arch/arm/boot/dts/ti/omap/am335x-icev2-prueth-overlay.dtso b/arch/arm/boot/dts/ti/omap/am335x-icev2-prueth-overlay.dtso
new file mode 100644
index 000000000000..3b82cf719671
--- /dev/null
+++ b/arch/arm/boot/dts/ti/omap/am335x-icev2-prueth-overlay.dtso
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DT overlay for IDK AM335x
+ *
+ * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+ */
+
+/*
+ * AM335x ICE V2 board
+ * http://www.ti.com/tool/tmdsice3359
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/bus/ti-sysc.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pinctrl/am33xx.h>
+#include <dt-bindings/clock/am3.h>
+
+&{/} {
+        /* Dual-MAC Ethernet application node on PRU-ICSS */
+        pruss_eth: pruss-eth {
+                compatible = "ti,am3359-prueth";
+                ti,prus = <&pru0>, <&pru1>;
+                sram = <&ocmcram>;
+                ti,mii-rt = <&pruss_mii_rt>;
+                ti,iep = <&pruss_iep>;
+                ti,ecap = <&pruss_ecap>;
+                interrupts = <20 2 2>, <21 3 3>;
+                interrupt-names = "rx_hp", "rx_lp";
+                interrupt-parent = <&pruss_intc>;
+
+                pinctrl-0 = <&pruss_eth_default>;
+                pinctrl-names = "default";
+
+                ethernet-ports {
+                        #address-cells = <1>;
+                        #size-cells = <0>;
+                        pruss_emac0: ethernet-port@0 {
+                                reg = <0>;
+                                phy-handle = <&pruss_eth0_phy>;
+                                phy-mode = "mii";
+                                interrupts = <20 2 2>, <26 6 6>, <23 6 6>;
+                                interrupt-names = "rx", "emac_ptp_tx",
+                                                  "hsr_ptp_tx";
+                                /* Filled in by bootloader */
+                                local-mac-address = [00 00 00 00 00 00];
+                        };
+
+                        pruss_emac1: ethernet-port@1 {
+                                reg = <1>;
+                                phy-handle = <&pruss_eth1_phy>;
+                                phy-mode = "mii";
+                                interrupts = <21 3 3>, <27 9 7>, <24 9 7>;
+                                interrupt-names = "rx", "emac_ptp_tx",
+                                                  "hsr_ptp_tx";
+                                /* Filled in by bootloader */
+                                local-mac-address = [00 00 00 00 00 00];
+                        };
+                };
+        };
+};
+
+&am33xx_pinmux {
+	/* MDIO node for PRU-ICSS */
+        pruss_mdio_default: pruss-mdio-default {
+                pinctrl-single,pins = <
+			AM33XX_IOPAD(0x88c, PIN_OUTPUT | MUX_MODE5) /* (V12) gpmc_clk.pr1_mdio_mdclk */
+			AM33XX_IOPAD(0x888, PIN_INPUT | MUX_MODE5) /* (T13) gpmc_csn3.pr1_mdio_data */
+                >;
+        };
+
+	/* Pinmux configuration for PRU-ICSS */
+        pruss_eth_default: pruss-eth-default {
+                pinctrl-single,pins = <
+			AM33XX_IOPAD(0x8a0, PIN_INPUT | MUX_MODE2) /* (R1) lcd_data0.pr1_mii_mt0_clk */
+			AM33XX_IOPAD(0x8b4, PIN_OUTPUT | MUX_MODE2) /* (T2) lcd_data5.pr1_mii0_txd0 */
+			AM33XX_IOPAD(0x8b0, PIN_OUTPUT | MUX_MODE2) /* (T1) lcd_data4.pr1_mii0_txd1 */
+			AM33XX_IOPAD(0x8ac, PIN_OUTPUT | MUX_MODE2) /* (R4) lcd_data3.pr1_mii0_txd2 */
+			AM33XX_IOPAD(0x8a8, PIN_OUTPUT | MUX_MODE2) /* (R3) lcd_data2.pr1_mii0_txd3 */
+			AM33XX_IOPAD(0x8cc, PIN_INPUT | MUX_MODE5) /* (U4) lcd_data11.pr1_mii0_rxd0 */
+			AM33XX_IOPAD(0x8c8, PIN_INPUT | MUX_MODE5) /* (U3) lcd_data10.pr1_mii0_rxd1 */
+			AM33XX_IOPAD(0x8c4, PIN_INPUT | MUX_MODE5) /* (U2) lcd_data9.pr1_mii0_rxd2 */
+			AM33XX_IOPAD(0x8c0, PIN_INPUT | MUX_MODE5) /* (U1) lcd_data8.pr1_mii0_rxd3 */
+			AM33XX_IOPAD(0x8a4, PIN_OUTPUT | MUX_MODE2) /* (R2) lcd_data1.pr1_mii0_txen */
+			AM33XX_IOPAD(0x8d8, PIN_INPUT | MUX_MODE5) /* (V4) lcd_data14.pr1_mii_mr0_clk */
+			AM33XX_IOPAD(0x8dc, PIN_INPUT | MUX_MODE5) /* (T5) lcd_data15.pr1_mii0_rxdv */
+			AM33XX_IOPAD(0x8d4, PIN_INPUT | MUX_MODE5) /* (V3) lcd_data13.pr1_mii0_rxer */
+			AM33XX_IOPAD(0x8d0, PIN_INPUT | MUX_MODE5) /* (V2) lcd_data12.pr1_mii0_rxlink */
+			AM33XX_IOPAD(0x8e8, PIN_INPUT | MUX_MODE2) /* (V5) lcd_pclk.pr1_mii0_crs */
+
+			AM33XX_IOPAD(0x840, PIN_INPUT | MUX_MODE5) /* (R13) gpmc_a0.pr1_mii_mt1_clk */
+			AM33XX_IOPAD(0x850, PIN_OUTPUT | MUX_MODE5) /* (R14) gpmc_a4.pr1_mii1_txd0 */
+			AM33XX_IOPAD(0x84c, PIN_OUTPUT | MUX_MODE5) /* (T14) gpmc_a3.pr1_mii1_txd1 */
+			AM33XX_IOPAD(0x848, PIN_OUTPUT | MUX_MODE5) /* (U14) gpmc_a2.pr1_mii1_txd2 */
+			AM33XX_IOPAD(0x844, PIN_OUTPUT | MUX_MODE5) /* (V14) gpmc_a1.pr1_mii1_txd3 */
+			AM33XX_IOPAD(0x860, PIN_INPUT | MUX_MODE5) /* (V16) gpmc_a8.pr1_mii1_rxd0 */
+			AM33XX_IOPAD(0x85c, PIN_INPUT | MUX_MODE5) /* (T15) gpmc_a7.pr1_mii1_rxd1 */
+			AM33XX_IOPAD(0x858, PIN_INPUT | MUX_MODE5) /* (U15) gpmc_a6.pr1_mii1_rxd2 */
+			AM33XX_IOPAD(0x854, PIN_INPUT | MUX_MODE5) /* (V15) gpmc_a5.pr1_mii1_rxd3 */
+			AM33XX_IOPAD(0x874, PIN_OUTPUT | MUX_MODE5) /* (U17) gpmc_wpn.pr1_mii1_txen */
+			AM33XX_IOPAD(0x864, PIN_INPUT | MUX_MODE5) /* (U16) gpmc_a9.pr1_mii_mr1_clk */
+			AM33XX_IOPAD(0x868, PIN_INPUT | MUX_MODE5) /* (T16) gpmc_a10.pr1_mii1_rxdv */
+			AM33XX_IOPAD(0x86c, PIN_INPUT | MUX_MODE5) /* (V17) gpmc_a11.pr1_mii1_rxer */
+			AM33XX_IOPAD(0x878, PIN_INPUT | MUX_MODE5) /* (U18) gpmc_be1n.pr1_mii1_rxlink */
+			AM33XX_IOPAD(0x8ec, PIN_INPUT | MUX_MODE2) /* (R6) lcd_ac_bias_en.pr1_mii1_crs */
+                >;
+        };
+};
+
+&gpio3 {
+        mux-mii-hog {
+                /* ETH1 mux: Low for MII-PRU, high for RMII-CPSW */
+                output-low;
+        };
+};
+
+/*
+ * Disable CPSW switch node and
+ * MDIO configuration to prevent
+ * conflict with PRU-ICSS
+ */
+&mac_sw {
+        status = "disable";
+};
+
+&davinci_mdio_sw {
+        status = "disable";
+};
+
+/* PRU-ICSS MDIO configuration */
+&pruss_mdio {
+        pinctrl-0 = <&pruss_mdio_default>;
+        pinctrl-names = "default";
+        reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
+        reset-delay-us = <2>; /* PHY datasheet states 1uS min */
+        status = "okay";
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pruss_eth0_phy: ethernet-phy@1 {
+                 reg = <1>;
+        };
+
+        pruss_eth1_phy: ethernet-phy@3 {
+                 reg = <3>;
+        };
+};
diff --git a/arch/arm/boot/dts/ti/omap/am33xx-l4.dtsi b/arch/arm/boot/dts/ti/omap/am33xx-l4.dtsi
index 89d16fcc773e..a63ef307d918 100644
--- a/arch/arm/boot/dts/ti/omap/am33xx-l4.dtsi
+++ b/arch/arm/boot/dts/ti/omap/am33xx-l4.dtsi
@@ -896,6 +896,17 @@ pruss_mii_rt: mii-rt@32000 {
 					reg = <0x32000 0x58>;
 				};
 
+				pruss_iep: iep@2e000 {
+					compatible = "ti,am3356-icss-iep";
+					reg = <0x2e000 0x31c>;
+					clocks = <&pruss_iepclk_mux>;
+				};
+
+				pruss_ecap: ecap@30000 {
+					compatible = "ti,pruss-ecap";
+					reg = <0x30000 0x60>;
+				};
+
 				pruss_intc: interrupt-controller@20000 {
 					compatible = "ti,pruss-intc";
 					reg = <0x20000 0x2000>;
-- 
2.43.0
Re: [PATCH v5 3/3] arm: dts: ti: Add device tree support for PRU-ICSS on AM335x
Posted by Kevin Hilman 3 weeks, 6 days ago
Parvathi Pudi <parvathi@couthit.com> writes:

> From: Roger Quadros <rogerq@ti.com>
>
> The TI Sitara AM335x ICE-V2 consists of single PRU-ICSS instance,
> This patch adds the new device tree overlay file in-order to enable
> PRU-ICSS instance, along with makefile changes.
>
> PRU-ICSS instance consists of two PRU cores along with various
> peripherals such as the Interrupt Controller (PRU_INTC), the Industrial
> Ethernet Peripheral(IEP), the Real Time Media Independent Interface
> controller (MII_RT), and the Enhanced Capture (eCAP) event module.
>
> am33xx-l4.dtsi - Adds IEP and eCAP peripheral as child nodes
> of the PRUSS subsystem node.
>
> am335x-icev2-prueth.dtso - Adds PRU-ICSS instance node along with PRU
> eth port information and corresponding port configuration. It includes
> interrupt mapping for packet reception, HW timestamp collection, and PRU
> Ethernet ports in MII mode,
>
> GPIO configuration, boot strapping along with delay configuration for
> individual PRU Ethernet port and other required nodes.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> Signed-off-by: Basharath Hussain Khaja <basharath@couthit.com>
> Signed-off-by: Parvathi Pudi <parvathi@couthit.com>

[...]

> +/*
> + * Disable CPSW switch node and
> + * MDIO configuration to prevent
> + * conflict with PRU-ICSS
> + */
> +&mac_sw {
> +        status = "disable";
> +};
> +
> +&davinci_mdio_sw {
> +        status = "disable";
> +};

I think you need s/disable/disabled/?  (note the trailing 'd').  Without
that, I don't think you're disabling these nodes, so I'm curious how it
is not conflicting with the PRU-ICSS.

Kevin
Re: [PATCH v5 3/3] arm: dts: ti: Add device tree support for PRU-ICSS on AM335x
Posted by Parvathi Pudi 3 weeks, 3 days ago
Hi,

> Parvathi Pudi <parvathi@couthit.com> writes:
> 
>> From: Roger Quadros <rogerq@ti.com>
>>
>> The TI Sitara AM335x ICE-V2 consists of single PRU-ICSS instance,
>> This patch adds the new device tree overlay file in-order to enable
>> PRU-ICSS instance, along with makefile changes.
>>
>> PRU-ICSS instance consists of two PRU cores along with various
>> peripherals such as the Interrupt Controller (PRU_INTC), the Industrial
>> Ethernet Peripheral(IEP), the Real Time Media Independent Interface
>> controller (MII_RT), and the Enhanced Capture (eCAP) event module.
>>
>> am33xx-l4.dtsi - Adds IEP and eCAP peripheral as child nodes
>> of the PRUSS subsystem node.
>>
>> am335x-icev2-prueth.dtso - Adds PRU-ICSS instance node along with PRU
>> eth port information and corresponding port configuration. It includes
>> interrupt mapping for packet reception, HW timestamp collection, and PRU
>> Ethernet ports in MII mode,
>>
>> GPIO configuration, boot strapping along with delay configuration for
>> individual PRU Ethernet port and other required nodes.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>> Signed-off-by: Basharath Hussain Khaja <basharath@couthit.com>
>> Signed-off-by: Parvathi Pudi <parvathi@couthit.com>
> 
> [...]
> 
>> +/*
>> + * Disable CPSW switch node and
>> + * MDIO configuration to prevent
>> + * conflict with PRU-ICSS
>> + */
>> +&mac_sw {
>> +        status = "disable";
>> +};
>> +
>> +&davinci_mdio_sw {
>> +        status = "disable";
>> +};
> 
> I think you need s/disable/disabled/?  (note the trailing 'd').  Without
> that, I don't think you're disabling these nodes, so I'm curious how it
> is not conflicting with the PRU-ICSS.
> 
> Kevin

Thank you for pointing out this typo.

We checked the kernel code as to why this did not create any issue in our
testing.  We found that the device availability check goes through
of_device_is_available(), which only treats "ok" or "okay" as enabled.
Anything else is effectively treated as not enabled.

So even though "disable" isn't the usual DT value, it still prevents the
node from being probed since it doesn't match "ok"/"okay".

We will update the value to "disabled" in the next version, since that is
the standard.

Thanks and Regards,
Parvathi.
Re: [PATCH v5 3/3] arm: dts: ti: Add device tree support for PRU-ICSS on AM335x
Posted by Krzysztof Kozlowski 3 weeks, 2 days ago
On 16/03/2026 13:00, Parvathi Pudi wrote:
> Hi,
> 
>> Parvathi Pudi <parvathi@couthit.com> writes:
>>
>>> From: Roger Quadros <rogerq@ti.com>
>>>
>>> The TI Sitara AM335x ICE-V2 consists of single PRU-ICSS instance,
>>> This patch adds the new device tree overlay file in-order to enable
>>> PRU-ICSS instance, along with makefile changes.
>>>
>>> PRU-ICSS instance consists of two PRU cores along with various
>>> peripherals such as the Interrupt Controller (PRU_INTC), the Industrial
>>> Ethernet Peripheral(IEP), the Real Time Media Independent Interface
>>> controller (MII_RT), and the Enhanced Capture (eCAP) event module.
>>>
>>> am33xx-l4.dtsi - Adds IEP and eCAP peripheral as child nodes
>>> of the PRUSS subsystem node.
>>>
>>> am335x-icev2-prueth.dtso - Adds PRU-ICSS instance node along with PRU
>>> eth port information and corresponding port configuration. It includes
>>> interrupt mapping for packet reception, HW timestamp collection, and PRU
>>> Ethernet ports in MII mode,
>>>
>>> GPIO configuration, boot strapping along with delay configuration for
>>> individual PRU Ethernet port and other required nodes.
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>> Signed-off-by: Basharath Hussain Khaja <basharath@couthit.com>
>>> Signed-off-by: Parvathi Pudi <parvathi@couthit.com>
>>
>> [...]
>>
>>> +/*
>>> + * Disable CPSW switch node and
>>> + * MDIO configuration to prevent
>>> + * conflict with PRU-ICSS
>>> + */
>>> +&mac_sw {
>>> +        status = "disable";
>>> +};
>>> +
>>> +&davinci_mdio_sw {
>>> +        status = "disable";
>>> +};
>>
>> I think you need s/disable/disabled/?  (note the trailing 'd').  Without
>> that, I don't think you're disabling these nodes, so I'm curious how it
>> is not conflicting with the PRU-ICSS.
>>
>> Kevin
> 
> Thank you for pointing out this typo.
> 
> We checked the kernel code as to why this did not create any issue in our
> testing.  We found that the device availability check goes through
> of_device_is_available(), which only treats "ok" or "okay" as enabled.
> Anything else is effectively treated as not enabled.
> 
> So even though "disable" isn't the usual DT value, it still prevents the
> node from being probed since it doesn't match "ok"/"okay".


The question is whether you build tested your code (so dtbs_check). And
if not, why?

Best regards,
Krzysztof
Re: [PATCH v5 3/3] arm: dts: ti: Add device tree support for PRU-ICSS on AM335x
Posted by Parvathi Pudi 2 weeks, 2 days ago
Hi,

> On 16/03/2026 13:00, Parvathi Pudi wrote:
>> Hi,
>> 
>>> Parvathi Pudi <parvathi@couthit.com> writes:
>>>
>>>> From: Roger Quadros <rogerq@ti.com>
>>>>
>>>> The TI Sitara AM335x ICE-V2 consists of single PRU-ICSS instance,
>>>> This patch adds the new device tree overlay file in-order to enable
>>>> PRU-ICSS instance, along with makefile changes.
>>>>
>>>> PRU-ICSS instance consists of two PRU cores along with various
>>>> peripherals such as the Interrupt Controller (PRU_INTC), the Industrial
>>>> Ethernet Peripheral(IEP), the Real Time Media Independent Interface
>>>> controller (MII_RT), and the Enhanced Capture (eCAP) event module.
>>>>
>>>> am33xx-l4.dtsi - Adds IEP and eCAP peripheral as child nodes
>>>> of the PRUSS subsystem node.
>>>>
>>>> am335x-icev2-prueth.dtso - Adds PRU-ICSS instance node along with PRU
>>>> eth port information and corresponding port configuration. It includes
>>>> interrupt mapping for packet reception, HW timestamp collection, and PRU
>>>> Ethernet ports in MII mode,
>>>>
>>>> GPIO configuration, boot strapping along with delay configuration for
>>>> individual PRU Ethernet port and other required nodes.
>>>>
>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>>>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>>> Signed-off-by: Basharath Hussain Khaja <basharath@couthit.com>
>>>> Signed-off-by: Parvathi Pudi <parvathi@couthit.com>
>>>
>>> [...]
>>>
>>>> +/*
>>>> + * Disable CPSW switch node and
>>>> + * MDIO configuration to prevent
>>>> + * conflict with PRU-ICSS
>>>> + */
>>>> +&mac_sw {
>>>> +        status = "disable";
>>>> +};
>>>> +
>>>> +&davinci_mdio_sw {
>>>> +        status = "disable";
>>>> +};
>>>
>>> I think you need s/disable/disabled/?  (note the trailing 'd').  Without
>>> that, I don't think you're disabling these nodes, so I'm curious how it
>>> is not conflicting with the PRU-ICSS.
>>>
>>> Kevin
>> 
>> Thank you for pointing out this typo.
>> 
>> We checked the kernel code as to why this did not create any issue in our
>> testing.  We found that the device availability check goes through
>> of_device_is_available(), which only treats "ok" or "okay" as enabled.
>> Anything else is effectively treated as not enabled.
>> 
>> So even though "disable" isn't the usual DT value, it still prevents the
>> node from being probed since it doesn't match "ok"/"okay".
> 
> 
> The question is whether you build tested your code (so dtbs_check). And
> if not, why?
> 

We did run dtbs_check as part of our internal testing workflow. However, it
is executed within a test framework that runs multiple checks together and is
currently trimming part of the dtbs_check output. Because of this, the schema
validation warning for "disable" was not visible in the logs we reviewed
and we missed catching this error. 

We have verified that dtbs_check does report this issue, and we will update
our test setup to retain full dtbs_check output to avoid missing such warnings
in the future.

Thanks and Regards,
Parvathi.
Re: [PATCH v5 3/3] arm: dts: ti: Add device tree support for PRU-ICSS on AM335x
Posted by Andrew Lunn 2 weeks, 2 days ago
> We have verified that dtbs_check does report this issue, and we will update
> our test setup to retain full dtbs_check output to avoid missing such warnings
> in the future.

I would expect the exit value is set to something other than 0 when it
finds an error. Why not just fail the test based on that?

      Andrew
Re: [PATCH v5 3/3] arm: dts: ti: Add device tree support for PRU-ICSS on AM335x
Posted by Parvathi Pudi 1 week, 3 days ago
Hi,

>> We have verified that dtbs_check does report this issue, and we will update
>> our test setup to retain full dtbs_check output to avoid missing such warnings
>> in the future.
> 
> I would expect the exit value is set to something other than 0 when it
> finds an error. Why not just fail the test based on that?
> 

Earlier, we were using the command "grep -v '^\s\s*'" to capture the logs.
We noticed that this command skips capturing output lines that starts with an
indent. To prevent this, we have temporarily modified the grep command to capture
the full output. The downside is that this results in a bigger log file. 

We will review and try to come up with a better way to handle this issue.

Thanks and Regards,
Parvathi
Re: [PATCH v5 3/3] arm: dts: ti: Add device tree support for PRU-ICSS on AM335x
Posted by Kevin Hilman 3 weeks, 2 days ago
Parvathi Pudi <parvathi@couthit.com> writes:

> Hi,
>
>> Parvathi Pudi <parvathi@couthit.com> writes:
>> 
>>> From: Roger Quadros <rogerq@ti.com>
>>>
>>> The TI Sitara AM335x ICE-V2 consists of single PRU-ICSS instance,
>>> This patch adds the new device tree overlay file in-order to enable
>>> PRU-ICSS instance, along with makefile changes.
>>>
>>> PRU-ICSS instance consists of two PRU cores along with various
>>> peripherals such as the Interrupt Controller (PRU_INTC), the Industrial
>>> Ethernet Peripheral(IEP), the Real Time Media Independent Interface
>>> controller (MII_RT), and the Enhanced Capture (eCAP) event module.
>>>
>>> am33xx-l4.dtsi - Adds IEP and eCAP peripheral as child nodes
>>> of the PRUSS subsystem node.
>>>
>>> am335x-icev2-prueth.dtso - Adds PRU-ICSS instance node along with PRU
>>> eth port information and corresponding port configuration. It includes
>>> interrupt mapping for packet reception, HW timestamp collection, and PRU
>>> Ethernet ports in MII mode,
>>>
>>> GPIO configuration, boot strapping along with delay configuration for
>>> individual PRU Ethernet port and other required nodes.
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>> Signed-off-by: Basharath Hussain Khaja <basharath@couthit.com>
>>> Signed-off-by: Parvathi Pudi <parvathi@couthit.com>
>> 
>> [...]
>> 
>>> +/*
>>> + * Disable CPSW switch node and
>>> + * MDIO configuration to prevent
>>> + * conflict with PRU-ICSS
>>> + */
>>> +&mac_sw {
>>> +        status = "disable";
>>> +};
>>> +
>>> +&davinci_mdio_sw {
>>> +        status = "disable";
>>> +};
>> 
>> I think you need s/disable/disabled/?  (note the trailing 'd').  Without
>> that, I don't think you're disabling these nodes, so I'm curious how it
>> is not conflicting with the PRU-ICSS.
>> 
>> Kevin
>
> Thank you for pointing out this typo.
>
> We checked the kernel code as to why this did not create any issue in our
> testing.  We found that the device availability check goes through
> of_device_is_available(), which only treats "ok" or "okay" as enabled.
> Anything else is effectively treated as not enabled.
>
> So even though "disable" isn't the usual DT value, it still prevents the
> node from being probed since it doesn't match "ok"/"okay".

Yes, but since your node is in an overlay, if a previous .dts[i] had set
this to "ok", then your overlay would not disable it, which would not be
expected behavior.

> We will update the value to "disabled" in the next version, since that is
> the standard.

Thanks.

Kevin
Re: [PATCH v5 3/3] arm: dts: ti: Add device tree support for PRU-ICSS on AM335x
Posted by Parvathi Pudi 2 weeks, 2 days ago
Hi,

> Parvathi Pudi <parvathi@couthit.com> writes:
> 
>> Hi,
>>
>>> Parvathi Pudi <parvathi@couthit.com> writes:
>>> 
>>>> From: Roger Quadros <rogerq@ti.com>
>>>>
>>>> The TI Sitara AM335x ICE-V2 consists of single PRU-ICSS instance,
>>>> This patch adds the new device tree overlay file in-order to enable
>>>> PRU-ICSS instance, along with makefile changes.
>>>>
>>>> PRU-ICSS instance consists of two PRU cores along with various
>>>> peripherals such as the Interrupt Controller (PRU_INTC), the Industrial
>>>> Ethernet Peripheral(IEP), the Real Time Media Independent Interface
>>>> controller (MII_RT), and the Enhanced Capture (eCAP) event module.
>>>>
>>>> am33xx-l4.dtsi - Adds IEP and eCAP peripheral as child nodes
>>>> of the PRUSS subsystem node.
>>>>
>>>> am335x-icev2-prueth.dtso - Adds PRU-ICSS instance node along with PRU
>>>> eth port information and corresponding port configuration. It includes
>>>> interrupt mapping for packet reception, HW timestamp collection, and PRU
>>>> Ethernet ports in MII mode,
>>>>
>>>> GPIO configuration, boot strapping along with delay configuration for
>>>> individual PRU Ethernet port and other required nodes.
>>>>
>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>>>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>>> Signed-off-by: Basharath Hussain Khaja <basharath@couthit.com>
>>>> Signed-off-by: Parvathi Pudi <parvathi@couthit.com>
>>> 
>>> [...]
>>> 
>>>> +/*
>>>> + * Disable CPSW switch node and
>>>> + * MDIO configuration to prevent
>>>> + * conflict with PRU-ICSS
>>>> + */
>>>> +&mac_sw {
>>>> +        status = "disable";
>>>> +};
>>>> +
>>>> +&davinci_mdio_sw {
>>>> +        status = "disable";
>>>> +};
>>> 
>>> I think you need s/disable/disabled/?  (note the trailing 'd').  Without
>>> that, I don't think you're disabling these nodes, so I'm curious how it
>>> is not conflicting with the PRU-ICSS.
>>> 
>>> Kevin
>>
>> Thank you for pointing out this typo.
>>
>> We checked the kernel code as to why this did not create any issue in our
>> testing.  We found that the device availability check goes through
>> of_device_is_available(), which only treats "ok" or "okay" as enabled.
>> Anything else is effectively treated as not enabled.
>>
>> So even though "disable" isn't the usual DT value, it still prevents the
>> node from being probed since it doesn't match "ok"/"okay".
> 
> Yes, but since your node is in an overlay, if a previous .dts[i] had set
> this to "ok", then your overlay would not disable it, which would not be
> expected behavior.
> 
>> We will update the value to "disabled" in the next version, since that is
>> the standard.

Sorry for the delayed response.

In our setup, there is no previous .dts where we set this to "ok". We are working
with a combined DTB (base + overlay applied). When we de-compile the final DTB
back to DTS, we do see that the status property is set to "disable" for the node.
At runtime, the kernel’s of_device_is_available() only treats "ok"/"okay" as enabled,
so "disable" results in the node not being probed, which is why we did not observe
any functional issue during testing. 

Are you referring to anything beyond the final merged DT result?

Thanks and Regards,
Parvathi.