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

[Parvathi Pudi <parvathi@couthit.com>]

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.