public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <leif@nuviainc.com>
To: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: devel@edk2.groups.io, masahisa.kojima@linaro.org,
	ilias.apalodimas@linaro.org
Subject: Re: [PATCH edk2-platforms 1/1] Silicon/SynQuacer: set PHY mode as appropriate in ACPI and DT tables
Date: Fri, 27 Nov 2020 17:33:11 +0000	[thread overview]
Message-ID: <20201127173311.GB1664@vanye> (raw)
In-Reply-To: <20201127144404.21386-1-ard.biesheuvel@arm.com>

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
> 

  reply	other threads:[~2020-11-27 17:33 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2020-11-30  9:13   ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201127173311.GB1664@vanye \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox