diff mbox series

[v2,10/11] ARM: dts: stm32: add ethernet1 and ethernet2 for STM32MP135F-DK board

Message ID 20240426125707.585269-11-christophe.roullier@foss.st.com (mailing list archive)
State Changes Requested
Headers show
Series Series to deliver Ethernets for STM32MP13 | expand

Checks

Context Check Description
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Christophe Roullier April 26, 2024, 12:57 p.m. UTC
Add dual Ethernet:
-Ethernet1: RMII with crystal
-Ethernet2: RMII without crystal
PHYs used are SMSC (LAN8742A)

With Ethernet1, we can performed WoL from PHY instead of GMAC point
of view.
(in this case IRQ for WoL is managed as wakeup pin and configured
in OS secure).

Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
---
 arch/arm/boot/dts/st/stm32mp135f-dk.dts | 48 +++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

Comments

Marek Vasut April 26, 2024, 3:44 p.m. UTC | #1
On 4/26/24 2:57 PM, Christophe Roullier wrote:
> Add dual Ethernet:
> -Ethernet1: RMII with crystal
> -Ethernet2: RMII without crystal
> PHYs used are SMSC (LAN8742A)
> 
> With Ethernet1, we can performed WoL from PHY instead of GMAC point
> of view.
> (in this case IRQ for WoL is managed as wakeup pin and configured
> in OS secure).

How does the Linux PHY driver process such a PHY IRQ ?

Or is Linux unaware of the PHY IRQ ? Doesn't that cause issues ?

> diff --git a/arch/arm/boot/dts/st/stm32mp135f-dk.dts b/arch/arm/boot/dts/st/stm32mp135f-dk.dts
> index 567e53ad285f..3b8eb0ab9ab9 100644
> --- a/arch/arm/boot/dts/st/stm32mp135f-dk.dts
> +++ b/arch/arm/boot/dts/st/stm32mp135f-dk.dts
> @@ -19,6 +19,8 @@ / {
>   	compatible = "st,stm32mp135f-dk", "st,stm32mp135";
>   
>   	aliases {
> +		ethernet0 = &ethernet1;
> +		ethernet1 = &ethernet2;
>   		serial0 = &uart4;
>   		serial1 = &usart1;
>   		serial2 = &uart8;
> @@ -141,6 +143,52 @@ &cryp {
>   	status = "okay";
>   };
>   
> +&ethernet1 {
> +	status = "okay";
> +	pinctrl-0 = <&eth1_rmii_pins_a>;
> +	pinctrl-1 = <&eth1_rmii_sleep_pins_a>;
> +	pinctrl-names = "default", "sleep";
> +	phy-mode = "rmii";
> +	max-speed = <100>;
> +	phy-handle = <&phy0_eth1>;

Keep the list sorted please (is the max-speed even needed? if not, drop it)

> +	mdio {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "snps,dwmac-mdio";
> +
> +		phy0_eth1: ethernet-phy@0 {
> +			compatible = "ethernet-phy-id0007.c131";
> +			reset-gpios =  <&mcp23017 9 GPIO_ACTIVE_LOW>;
> +			reg = <0>;
> +			wakeup-source;
> +		};
> +	};
> +};
> +
> +&ethernet2 {
> +	status = "okay";
> +	pinctrl-0 = <&eth2_rmii_pins_a>;
> +	pinctrl-1 = <&eth2_rmii_sleep_pins_a>;
> +	pinctrl-names = "default", "sleep";
> +	phy-mode = "rmii";
> +	max-speed = <100>;
> +	phy-handle = <&phy0_eth2>;
> +	st,ext-phyclk;
> +	phy-supply = <&scmi_v3v3_sw>;

Sort please

[...]
Alexandre TORGUE May 13, 2024, 4:01 p.m. UTC | #2
Hi Marek

On 4/26/24 17:44, Marek Vasut wrote:
> On 4/26/24 2:57 PM, Christophe Roullier wrote:
>> Add dual Ethernet:
>> -Ethernet1: RMII with crystal
>> -Ethernet2: RMII without crystal
>> PHYs used are SMSC (LAN8742A)
>>
>> With Ethernet1, we can performed WoL from PHY instead of GMAC point
>> of view.
>> (in this case IRQ for WoL is managed as wakeup pin and configured
>> in OS secure).
> 
> How does the Linux PHY driver process such a PHY IRQ ?
> 
> Or is Linux unaware of the PHY IRQ ? Doesn't that cause issues ?

In this case, we want to have an example to wakeup the system from 
Standby low power mode (VDDCPU and VDD_CORE off) thanks to a magic 
packet detected by the PHY. The PHY then assert his interrupt output signal.
On MP13 DK platform, this PHY signal is connected to a specific GPIO
aka "Wakeup pins" (only 6 wakeup pins an MP13). Those specific GPIOs are 
handled by the PWR peripheral which is controlled by the secure OS.

On WoL packet, the Secure OS catches the PHY interrupt and uses 
asynchronous notification mechanism to warn Linux (on our platform we 
use a PPI). On Linux side, Optee core driver creates an irq 
domain/irqchip triggered on the asynchronous notification. Each device 
which use a wakeup pin need then to request an IRQ on this "Optee irq 
domain".

This OPTEE irq domain will be pushed soon.

cheers
Alex

> 
>> diff --git a/arch/arm/boot/dts/st/stm32mp135f-dk.dts 
>> b/arch/arm/boot/dts/st/stm32mp135f-dk.dts
>> index 567e53ad285f..3b8eb0ab9ab9 100644
>> --- a/arch/arm/boot/dts/st/stm32mp135f-dk.dts
>> +++ b/arch/arm/boot/dts/st/stm32mp135f-dk.dts
>> @@ -19,6 +19,8 @@ / {
>>       compatible = "st,stm32mp135f-dk", "st,stm32mp135";
>>       aliases {
>> +        ethernet0 = &ethernet1;
>> +        ethernet1 = &ethernet2;
>>           serial0 = &uart4;
>>           serial1 = &usart1;
>>           serial2 = &uart8;
>> @@ -141,6 +143,52 @@ &cryp {
>>       status = "okay";
>>   };
>> +&ethernet1 {
>> +    status = "okay";
>> +    pinctrl-0 = <&eth1_rmii_pins_a>;
>> +    pinctrl-1 = <&eth1_rmii_sleep_pins_a>;
>> +    pinctrl-names = "default", "sleep";
>> +    phy-mode = "rmii";
>> +    max-speed = <100>;
>> +    phy-handle = <&phy0_eth1>;
> 
> Keep the list sorted please (is the max-speed even needed? if not, drop it)
> 
>> +    mdio {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        compatible = "snps,dwmac-mdio";
>> +
>> +        phy0_eth1: ethernet-phy@0 {
>> +            compatible = "ethernet-phy-id0007.c131";
>> +            reset-gpios =  <&mcp23017 9 GPIO_ACTIVE_LOW>;
>> +            reg = <0>;
>> +            wakeup-source;
>> +        };
>> +    };
>> +};
>> +
>> +&ethernet2 {
>> +    status = "okay";
>> +    pinctrl-0 = <&eth2_rmii_pins_a>;
>> +    pinctrl-1 = <&eth2_rmii_sleep_pins_a>;
>> +    pinctrl-names = "default", "sleep";
>> +    phy-mode = "rmii";
>> +    max-speed = <100>;
>> +    phy-handle = <&phy0_eth2>;
>> +    st,ext-phyclk;
>> +    phy-supply = <&scmi_v3v3_sw>;
> 
> Sort please
> 
> [...]
Marek Vasut May 16, 2024, 12:23 a.m. UTC | #3
On 5/13/24 6:01 PM, Alexandre TORGUE wrote:
> Hi Marek

Hi,

> On 4/26/24 17:44, Marek Vasut wrote:
>> On 4/26/24 2:57 PM, Christophe Roullier wrote:
>>> Add dual Ethernet:
>>> -Ethernet1: RMII with crystal
>>> -Ethernet2: RMII without crystal
>>> PHYs used are SMSC (LAN8742A)
>>>
>>> With Ethernet1, we can performed WoL from PHY instead of GMAC point
>>> of view.
>>> (in this case IRQ for WoL is managed as wakeup pin and configured
>>> in OS secure).
>>
>> How does the Linux PHY driver process such a PHY IRQ ?
>>
>> Or is Linux unaware of the PHY IRQ ? Doesn't that cause issues ?
> 
> In this case, we want to have an example to wakeup the system from 
> Standby low power mode (VDDCPU and VDD_CORE off) thanks to a magic 
> packet detected by the PHY. The PHY then assert his interrupt output 
> signal.
> On MP13 DK platform, this PHY signal is connected to a specific GPIO
> aka "Wakeup pins" (only 6 wakeup pins an MP13). Those specific GPIOs are 
> handled by the PWR peripheral which is controlled by the secure OS.

What does configure the PHY for this wakeup mode ?

> On WoL packet, the Secure OS catches the PHY interrupt and uses 
> asynchronous notification mechanism to warn Linux (on our platform we 
> use a PPI). On Linux side, Optee core driver creates an irq 
> domain/irqchip triggered on the asynchronous notification. Each device 
> which use a wakeup pin need then to request an IRQ on this "Optee irq 
> domain".
> 
> This OPTEE irq domain will be pushed soon.

I suspect it might make sense to add this WoL part separately from the 
actual ethernet DT nodes, so ethernet could land and the WoL 
functionality can be added when it is ready ?
Alexandre TORGUE May 16, 2024, 7:58 a.m. UTC | #4
Hi

On 5/16/24 02:23, Marek Vasut wrote:
> On 5/13/24 6:01 PM, Alexandre TORGUE wrote:
>> Hi Marek
> 
> Hi,
> 
>> On 4/26/24 17:44, Marek Vasut wrote:
>>> On 4/26/24 2:57 PM, Christophe Roullier wrote:
>>>> Add dual Ethernet:
>>>> -Ethernet1: RMII with crystal
>>>> -Ethernet2: RMII without crystal
>>>> PHYs used are SMSC (LAN8742A)
>>>>
>>>> With Ethernet1, we can performed WoL from PHY instead of GMAC point
>>>> of view.
>>>> (in this case IRQ for WoL is managed as wakeup pin and configured
>>>> in OS secure).
>>>
>>> How does the Linux PHY driver process such a PHY IRQ ?
>>>
>>> Or is Linux unaware of the PHY IRQ ? Doesn't that cause issues ?
>>
>> In this case, we want to have an example to wakeup the system from 
>> Standby low power mode (VDDCPU and VDD_CORE off) thanks to a magic 
>> packet detected by the PHY. The PHY then assert his interrupt output 
>> signal.
>> On MP13 DK platform, this PHY signal is connected to a specific GPIO
>> aka "Wakeup pins" (only 6 wakeup pins an MP13). Those specific GPIOs 
>> are handled by the PWR peripheral which is controlled by the secure OS.
> 
> What does configure the PHY for this wakeup mode ?

Linux device tree.

> 
>> On WoL packet, the Secure OS catches the PHY interrupt and uses 
>> asynchronous notification mechanism to warn Linux (on our platform we 
>> use a PPI). On Linux side, Optee core driver creates an irq 
>> domain/irqchip triggered on the asynchronous notification. Each device 
>> which use a wakeup pin need then to request an IRQ on this "Optee irq 
>> domain".
>>
>> This OPTEE irq domain will be pushed soon.
> 
> I suspect it might make sense to add this WoL part separately from the 
> actual ethernet DT nodes, so ethernet could land and the WoL 
> functionality can be added when it is ready ?

If at the end we want to have this Wol from PHY then I agree we need to 
wait. We could push a WoL from MAC for this node before optee driver 
patches merge but not sure it makes sens.

Alex
Andrew Lunn May 16, 2024, 12:22 p.m. UTC | #5
> > I suspect it might make sense to add this WoL part separately from the
> > actual ethernet DT nodes, so ethernet could land and the WoL
> > functionality can be added when it is ready ?
> 
> If at the end we want to have this Wol from PHY then I agree we need to
> wait. We could push a WoL from MAC for this node before optee driver patches
> merge but not sure it makes sens.

In general, it is better if the PHY does WoL, since the MAC can then
be powered down. MAC WoL should only be used when the PHY does not
support the requested WoL configuration, but the MAC can. And
sometimes you need to spread it over both the PHY and the MAC.

	Andrew
Alexandre TORGUE May 17, 2024, 7:30 a.m. UTC | #6
On 5/16/24 14:22, Andrew Lunn wrote:
>>> I suspect it might make sense to add this WoL part separately from the
>>> actual ethernet DT nodes, so ethernet could land and the WoL
>>> functionality can be added when it is ready ?
>>
>> If at the end we want to have this Wol from PHY then I agree we need to
>> wait. We could push a WoL from MAC for this node before optee driver patches
>> merge but not sure it makes sens.
> 
> In general, it is better if the PHY does WoL, since the MAC can then
> be powered down. MAC WoL should only be used when the PHY does not
> support the requested WoL configuration, but the MAC can. And
> sometimes you need to spread it over both the PHY and the MAC.
>

thanks Andrew. So lets wait the optee driver missing part.

alex


> 	Andrew
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/st/stm32mp135f-dk.dts b/arch/arm/boot/dts/st/stm32mp135f-dk.dts
index 567e53ad285f..3b8eb0ab9ab9 100644
--- a/arch/arm/boot/dts/st/stm32mp135f-dk.dts
+++ b/arch/arm/boot/dts/st/stm32mp135f-dk.dts
@@ -19,6 +19,8 @@  / {
 	compatible = "st,stm32mp135f-dk", "st,stm32mp135";
 
 	aliases {
+		ethernet0 = &ethernet1;
+		ethernet1 = &ethernet2;
 		serial0 = &uart4;
 		serial1 = &usart1;
 		serial2 = &uart8;
@@ -141,6 +143,52 @@  &cryp {
 	status = "okay";
 };
 
+&ethernet1 {
+	status = "okay";
+	pinctrl-0 = <&eth1_rmii_pins_a>;
+	pinctrl-1 = <&eth1_rmii_sleep_pins_a>;
+	pinctrl-names = "default", "sleep";
+	phy-mode = "rmii";
+	max-speed = <100>;
+	phy-handle = <&phy0_eth1>;
+
+	mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "snps,dwmac-mdio";
+
+		phy0_eth1: ethernet-phy@0 {
+			compatible = "ethernet-phy-id0007.c131";
+			reset-gpios =  <&mcp23017 9 GPIO_ACTIVE_LOW>;
+			reg = <0>;
+			wakeup-source;
+		};
+	};
+};
+
+&ethernet2 {
+	status = "okay";
+	pinctrl-0 = <&eth2_rmii_pins_a>;
+	pinctrl-1 = <&eth2_rmii_sleep_pins_a>;
+	pinctrl-names = "default", "sleep";
+	phy-mode = "rmii";
+	max-speed = <100>;
+	phy-handle = <&phy0_eth2>;
+	st,ext-phyclk;
+	phy-supply = <&scmi_v3v3_sw>;
+
+	mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "snps,dwmac-mdio";
+		phy0_eth2: ethernet-phy@0 {
+			compatible = "ethernet-phy-id0007.c131";
+			reset-gpios = <&mcp23017 10 GPIO_ACTIVE_LOW>;
+			reg = <0>;
+		};
+	};
+};
+
 &i2c1 {
 	pinctrl-names = "default", "sleep";
 	pinctrl-0 = <&i2c1_pins_a>;