* [PATCH edk2-platforms 1/1] Silicon/SynQuacer: set PHY mode as appropriate in ACPI and DT tables
@ 2020-11-27 14:44 Ard Biesheuvel
2020-11-27 17:33 ` Leif Lindholm
0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2020-11-27 14:44 UTC (permalink / raw)
To: devel; +Cc: leif, masahisa.kojima, ilias.apalodimas, Ard Biesheuvel
As it turns out, the DeveloperBox platform never described its Ethernet
PHY mode correctly: the 'rgmii' value it exposes to the OS was inherited
from the SynQuacer evaluation board, which uses a different PHY, and the
Realtek PHY used on DeveloperBox is integrated on the board with straps
that configure it to 'rgmii-id' mode.
We never noticed because the Realtek PHY driver in Linux ignored the PHY
mode to begin with, and simply used the configuration that was active at
boot. Unfortunately, that has changed, and recent versions of the Linux
kernel (including stable releases) will now honour the firmware provided
PHY mode, and therefore configure the PHY incorrectly on these boards,
resulting in loss of network connectivity.
For ACPI boot, we can fix this by just setting the PHY mode to the empty
string - the Linux driver will be updated (and the change backported) to
ignore it anyway, as ACPI boot implies rich firmware, and it is reasonable
to assume that the PHY will be configured before the OS boots.
For DT, let's fix the description instead. This involves moving the
'phy-mode' property out of the shared .dtsi, as the change should only
apply to DeveloperBox, not to the SynQuacer evaluation board.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl | 2 +-
Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts | 4 ++++
Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 3 +--
Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts | 4 ++++
4 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
index bca484763d2c..3fecc570498b 100644
--- a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
+++ b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
@@ -170,7 +170,7 @@ DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "SNI", "SYNQUACR",
{
ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {
- Package (2) { "phy-mode", "rgmii" },
+ Package (2) { "phy-mode", "" },
Package (2) { "phy-channel", FixedPcdGet32 (PcdNetsecPhyAddress) },
Package (2) { "max-speed", 1000 },
Package (2) { "max-frame-size", 9000 },
diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts b/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts
index 47ac27109929..c9bd436f745c 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts
@@ -44,6 +44,10 @@
"PEC-PD28", "PEC-PD29", "PEC-PD30", "PEC-PD31";
};
+&netsec {
+ phy-mode = "rgmii-id";
+};
+
&mdio_netsec {
phy_netsec: ethernet-phy@7 {
compatible = "ethernet-phy-ieee802.3-c22";
diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
index 2ee3821fca0b..ad418bf292db 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
@@ -418,14 +418,13 @@
#clock-cells = <0>;
};
- ethernet@522d0000 {
+ netsec: ethernet@522d0000 {
compatible = "socionext,synquacer-netsec";
reg = <0 0x522d0000 0x0 0x10000>,
<0 FixedPcdGet32 (PcdNetsecEepromBase) 0x0 0x10000>;
interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk_netsec>;
clock-names = "phy_ref_clk";
- phy-mode = "rgmii";
max-speed = <1000>;
max-frame-size = <9000>;
phy-handle = <&phy_netsec>;
diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts
index f437ee4cccf1..013a3a617748 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts
@@ -24,6 +24,10 @@
"PEC-PD28", "PEC-PD29", "PEC-PD30", "PEC-PD31";
};
+&netsec {
+ phy-mode = "rgmii";
+};
+
&mdio_netsec {
phy_netsec: ethernet-phy@1 {
compatible = "ethernet-phy-ieee802.3-c22";
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH edk2-platforms 1/1] Silicon/SynQuacer: set PHY mode as appropriate in ACPI and DT tables
2020-11-27 14:44 [PATCH edk2-platforms 1/1] Silicon/SynQuacer: set PHY mode as appropriate in ACPI and DT tables Ard Biesheuvel
@ 2020-11-27 17:33 ` Leif Lindholm
2020-11-30 9:13 ` Ard Biesheuvel
0 siblings, 1 reply; 3+ messages in thread
From: Leif Lindholm @ 2020-11-27 17:33 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: devel, masahisa.kojima, ilias.apalodimas
On Fri, Nov 27, 2020 at 15:44:04 +0100, Ard Biesheuvel wrote:
> As it turns out, the DeveloperBox platform never described its Ethernet
> PHY mode correctly: the 'rgmii' value it exposes to the OS was inherited
> from the SynQuacer evaluation board, which uses a different PHY, and the
> Realtek PHY used on DeveloperBox is integrated on the board with straps
> that configure it to 'rgmii-id' mode.
>
> We never noticed because the Realtek PHY driver in Linux ignored the PHY
> mode to begin with, and simply used the configuration that was active at
> boot. Unfortunately, that has changed, and recent versions of the Linux
> kernel (including stable releases) will now honour the firmware provided
> PHY mode, and therefore configure the PHY incorrectly on these boards,
> resulting in loss of network connectivity.
>
> For ACPI boot, we can fix this by just setting the PHY mode to the empty
> string - the Linux driver will be updated (and the change backported) to
> ignore it anyway, as ACPI boot implies rich firmware, and it is reasonable
> to assume that the PHY will be configured before the OS boots.
>
> For DT, let's fix the description instead. This involves moving the
> 'phy-mode' property out of the shared .dtsi, as the change should only
> apply to DeveloperBox, not to the SynQuacer evaluation board.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
> ---
> Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl | 2 +-
> Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts | 4 ++++
> Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 3 +--
> Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts | 4 ++++
> 4 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
> index bca484763d2c..3fecc570498b 100644
> --- a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
> +++ b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
> @@ -170,7 +170,7 @@ DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "SNI", "SYNQUACR",
> {
> ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {
> - Package (2) { "phy-mode", "rgmii" },
> + Package (2) { "phy-mode", "" },
> Package (2) { "phy-channel", FixedPcdGet32 (PcdNetsecPhyAddress) },
> Package (2) { "max-speed", 1000 },
> Package (2) { "max-frame-size", 9000 },
> diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts b/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts
> index 47ac27109929..c9bd436f745c 100644
> --- a/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts
> +++ b/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts
> @@ -44,6 +44,10 @@
> "PEC-PD28", "PEC-PD29", "PEC-PD30", "PEC-PD31";
> };
>
> +&netsec {
> + phy-mode = "rgmii-id";
> +};
> +
> &mdio_netsec {
> phy_netsec: ethernet-phy@7 {
> compatible = "ethernet-phy-ieee802.3-c22";
> diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> index 2ee3821fca0b..ad418bf292db 100644
> --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> @@ -418,14 +418,13 @@
> #clock-cells = <0>;
> };
>
> - ethernet@522d0000 {
> + netsec: ethernet@522d0000 {
> compatible = "socionext,synquacer-netsec";
> reg = <0 0x522d0000 0x0 0x10000>,
> <0 FixedPcdGet32 (PcdNetsecEepromBase) 0x0 0x10000>;
> interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&clk_netsec>;
> clock-names = "phy_ref_clk";
> - phy-mode = "rgmii";
> max-speed = <1000>;
> max-frame-size = <9000>;
> phy-handle = <&phy_netsec>;
> diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts
> index f437ee4cccf1..013a3a617748 100644
> --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts
> +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts
> @@ -24,6 +24,10 @@
> "PEC-PD28", "PEC-PD29", "PEC-PD30", "PEC-PD31";
> };
>
> +&netsec {
> + phy-mode = "rgmii";
> +};
> +
> &mdio_netsec {
> phy_netsec: ethernet-phy@1 {
> compatible = "ethernet-phy-ieee802.3-c22";
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH edk2-platforms 1/1] Silicon/SynQuacer: set PHY mode as appropriate in ACPI and DT tables
2020-11-27 17:33 ` Leif Lindholm
@ 2020-11-30 9:13 ` Ard Biesheuvel
0 siblings, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2020-11-30 9:13 UTC (permalink / raw)
To: Leif Lindholm; +Cc: devel, masahisa.kojima, ilias.apalodimas
On 11/27/20 6:33 PM, Leif Lindholm wrote:
> On Fri, Nov 27, 2020 at 15:44:04 +0100, Ard Biesheuvel wrote:
>> As it turns out, the DeveloperBox platform never described its Ethernet
>> PHY mode correctly: the 'rgmii' value it exposes to the OS was inherited
>> from the SynQuacer evaluation board, which uses a different PHY, and the
>> Realtek PHY used on DeveloperBox is integrated on the board with straps
>> that configure it to 'rgmii-id' mode.
>>
>> We never noticed because the Realtek PHY driver in Linux ignored the PHY
>> mode to begin with, and simply used the configuration that was active at
>> boot. Unfortunately, that has changed, and recent versions of the Linux
>> kernel (including stable releases) will now honour the firmware provided
>> PHY mode, and therefore configure the PHY incorrectly on these boards,
>> resulting in loss of network connectivity.
>>
>> For ACPI boot, we can fix this by just setting the PHY mode to the empty
>> string - the Linux driver will be updated (and the change backported) to
>> ignore it anyway, as ACPI boot implies rich firmware, and it is reasonable
>> to assume that the PHY will be configured before the OS boots.
>>
>> For DT, let's fix the description instead. This involves moving the
>> 'phy-mode' property out of the shared .dtsi, as the change should only
>> apply to DeveloperBox, not to the SynQuacer evaluation board.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>
> Reviewed-by: Leif Lindholm <leif@nuviainc.com>
>
Thanks
Pushed as 3f71a8fb114a..83d38b0b4c0f
>> ---
>> Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl | 2 +-
>> Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts | 4 ++++
>> Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 3 +--
>> Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts | 4 ++++
>> 4 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
>> index bca484763d2c..3fecc570498b 100644
>> --- a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
>> +++ b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
>> @@ -170,7 +170,7 @@ DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "SNI", "SYNQUACR",
>> {
>> ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>> Package () {
>> - Package (2) { "phy-mode", "rgmii" },
>> + Package (2) { "phy-mode", "" },
>> Package (2) { "phy-channel", FixedPcdGet32 (PcdNetsecPhyAddress) },
>> Package (2) { "max-speed", 1000 },
>> Package (2) { "max-frame-size", 9000 },
>> diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts b/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts
>> index 47ac27109929..c9bd436f745c 100644
>> --- a/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts
>> +++ b/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts
>> @@ -44,6 +44,10 @@
>> "PEC-PD28", "PEC-PD29", "PEC-PD30", "PEC-PD31";
>> };
>>
>> +&netsec {
>> + phy-mode = "rgmii-id";
>> +};
>> +
>> &mdio_netsec {
>> phy_netsec: ethernet-phy@7 {
>> compatible = "ethernet-phy-ieee802.3-c22";
>> diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
>> index 2ee3821fca0b..ad418bf292db 100644
>> --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
>> +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
>> @@ -418,14 +418,13 @@
>> #clock-cells = <0>;
>> };
>>
>> - ethernet@522d0000 {
>> + netsec: ethernet@522d0000 {
>> compatible = "socionext,synquacer-netsec";
>> reg = <0 0x522d0000 0x0 0x10000>,
>> <0 FixedPcdGet32 (PcdNetsecEepromBase) 0x0 0x10000>;
>> interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&clk_netsec>;
>> clock-names = "phy_ref_clk";
>> - phy-mode = "rgmii";
>> max-speed = <1000>;
>> max-frame-size = <9000>;
>> phy-handle = <&phy_netsec>;
>> diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts
>> index f437ee4cccf1..013a3a617748 100644
>> --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts
>> +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts
>> @@ -24,6 +24,10 @@
>> "PEC-PD28", "PEC-PD29", "PEC-PD30", "PEC-PD31";
>> };
>>
>> +&netsec {
>> + phy-mode = "rgmii";
>> +};
>> +
>> &mdio_netsec {
>> phy_netsec: ethernet-phy@1 {
>> compatible = "ethernet-phy-ieee802.3-c22";
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-11-30 9:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-27 14:44 [PATCH edk2-platforms 1/1] Silicon/SynQuacer: set PHY mode as appropriate in ACPI and DT tables Ard Biesheuvel
2020-11-27 17:33 ` Leif Lindholm
2020-11-30 9:13 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox