diff mbox series

[2/4] Revert "arm64: dts: renesas: Add compatible properties to AR8031 Ethernet PHYs"

Message ID 20230104141245.8407-2-aford173@gmail.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series [1/4] arm64: dts: beacon-renesom: Fix gpio expander reference | expand

Commit Message

Adam Ford Jan. 4, 2023, 2:12 p.m. UTC 
This reverts commit 18a2427146bf8a3da8fc7825051d6aadb9c2d8fb.

Due to the part shortage, the AR8031 PHY was replaced with a
Micrel KSZ9131.  Hard-coding the ID of the PHY makes this new
PHY non-operational.  Since previous hardware had shipped,
it's not as simple as just replacing the ID number as it would
break the older hardware.  Since the generic mode can correctly
identify both versions of hardware, it seems safer to revert
this patch.

Signed-off-by: Adam Ford <aford173@gmail.com>

Comments

Geert Uytterhoeven Jan. 6, 2023, 2:27 p.m. UTC | #1 
Hi Adam,

CC Ethernet phy

On Wed, Jan 4, 2023 at 3:12 PM Adam Ford <aford173@gmail.com> wrote:
> This reverts commit 18a2427146bf8a3da8fc7825051d6aadb9c2d8fb.
>
> Due to the part shortage, the AR8031 PHY was replaced with a
> Micrel KSZ9131.  Hard-coding the ID of the PHY makes this new
> PHY non-operational.  Since previous hardware had shipped,
> it's not as simple as just replacing the ID number as it would
> break the older hardware.  Since the generic mode can correctly
> identify both versions of hardware, it seems safer to revert
> this patch.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
> +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
> @@ -59,8 +59,6 @@ &avb {
>         status = "okay";
>
>         phy0: ethernet-phy@0 {
> -               compatible = "ethernet-phy-id004d.d074",
> -                            "ethernet-phy-ieee802.3-c22";
>                 reg = <0>;
>                 interrupt-parent = <&gpio2>;
>                 interrupts = <11 IRQ_TYPE_LEVEL_LOW>;

The next line:

                reset-gpios = <&gpio2 10 GPIO_ACTIVE_LOW>;

Unfortunately, removing the compatible value will cause regressions
for kexec/kdump and for Ethernet driver unbind, as the PHY reset will
be asserted before starting the new kernel, or on driver unbind.
Due to a deficiency in the Ethernet PHY subsystem, the PHY will be
probed while the reset is still asserted, and thus fail probing[1].

Is there a (new) proper way to handle this?
Perhaps the issue has been fixed in the PHY subsystem meanwhile?

Thanks!

[1] https://lore.kernel.org/all/cover.1631174218.git.geert+renesas@glider.be

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Adam Ford Jan. 6, 2023, 2:32 p.m. UTC | #2 
On Fri, Jan 6, 2023 at 8:28 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> CC Ethernet phy
>
> On Wed, Jan 4, 2023 at 3:12 PM Adam Ford <aford173@gmail.com> wrote:
> > This reverts commit 18a2427146bf8a3da8fc7825051d6aadb9c2d8fb.
> >
> > Due to the part shortage, the AR8031 PHY was replaced with a
> > Micrel KSZ9131.  Hard-coding the ID of the PHY makes this new
> > PHY non-operational.  Since previous hardware had shipped,
> > it's not as simple as just replacing the ID number as it would
> > break the older hardware.  Since the generic mode can correctly
> > identify both versions of hardware, it seems safer to revert
> > this patch.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Thanks for your patch!
>
> > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
> > @@ -59,8 +59,6 @@ &avb {
> >         status = "okay";
> >
> >         phy0: ethernet-phy@0 {
> > -               compatible = "ethernet-phy-id004d.d074",
> > -                            "ethernet-phy-ieee802.3-c22";
> >                 reg = <0>;
> >                 interrupt-parent = <&gpio2>;
> >                 interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
>
> The next line:
>
>                 reset-gpios = <&gpio2 10 GPIO_ACTIVE_LOW>;
>
> Unfortunately, removing the compatible value will cause regressions
> for kexec/kdump and for Ethernet driver unbind, as the PHY reset will
> be asserted before starting the new kernel, or on driver unbind.
> Due to a deficiency in the Ethernet PHY subsystem, the PHY will be
> probed while the reset is still asserted, and thus fail probing[1].

FWIW, the bootloader brings the device out of reset.  Would it be
sufficient to keep  "ethernet-phy-ieee802.3-c22" and drop the
hard-coded ID?

thanks,

adam
>
> Is there a (new) proper way to handle this?
> Perhaps the issue has been fixed in the PHY subsystem meanwhile?
>
> Thanks!
>
> [1] https://lore.kernel.org/all/cover.1631174218.git.geert+renesas@glider.be
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
Geert Uytterhoeven Jan. 6, 2023, 2:45 p.m. UTC | #3 
Hi Adam,

On Fri, Jan 6, 2023 at 3:35 PM Adam Ford <aford173@gmail.com> wrote:
> On Fri, Jan 6, 2023 at 8:28 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Wed, Jan 4, 2023 at 3:12 PM Adam Ford <aford173@gmail.com> wrote:
> > > This reverts commit 18a2427146bf8a3da8fc7825051d6aadb9c2d8fb.
> > >
> > > Due to the part shortage, the AR8031 PHY was replaced with a
> > > Micrel KSZ9131.  Hard-coding the ID of the PHY makes this new
> > > PHY non-operational.  Since previous hardware had shipped,
> > > it's not as simple as just replacing the ID number as it would
> > > break the older hardware.  Since the generic mode can correctly
> > > identify both versions of hardware, it seems safer to revert
> > > this patch.
> > >
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > Thanks for your patch!
> >
> > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
> > > @@ -59,8 +59,6 @@ &avb {
> > >         status = "okay";
> > >
> > >         phy0: ethernet-phy@0 {
> > > -               compatible = "ethernet-phy-id004d.d074",
> > > -                            "ethernet-phy-ieee802.3-c22";
> > >                 reg = <0>;
> > >                 interrupt-parent = <&gpio2>;
> > >                 interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> >
> > The next line:
> >
> >                 reset-gpios = <&gpio2 10 GPIO_ACTIVE_LOW>;
> >
> > Unfortunately, removing the compatible value will cause regressions
> > for kexec/kdump and for Ethernet driver unbind, as the PHY reset will
> > be asserted before starting the new kernel, or on driver unbind.
> > Due to a deficiency in the Ethernet PHY subsystem, the PHY will be
> > probed while the reset is still asserted, and thus fail probing[1].
>
> FWIW, the bootloader brings the device out of reset.  Would it be

The bootloader is not involved when using kexec/kdump, or when
unbinding the Ethernet driver.

> sufficient to keep  "ethernet-phy-ieee802.3-c22" and drop the
> hard-coded ID?

I am afraid not, as that still requires actual probing to determine
the PHY ID.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Adam Ford Jan. 6, 2023, 2:49 p.m. UTC | #4 
On Fri, Jan 6, 2023 at 8:45 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> On Fri, Jan 6, 2023 at 3:35 PM Adam Ford <aford173@gmail.com> wrote:
> > On Fri, Jan 6, 2023 at 8:28 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Wed, Jan 4, 2023 at 3:12 PM Adam Ford <aford173@gmail.com> wrote:
> > > > This reverts commit 18a2427146bf8a3da8fc7825051d6aadb9c2d8fb.
> > > >
> > > > Due to the part shortage, the AR8031 PHY was replaced with a
> > > > Micrel KSZ9131.  Hard-coding the ID of the PHY makes this new
> > > > PHY non-operational.  Since previous hardware had shipped,
> > > > it's not as simple as just replacing the ID number as it would
> > > > break the older hardware.  Since the generic mode can correctly
> > > > identify both versions of hardware, it seems safer to revert
> > > > this patch.
> > > >
> > > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
> > > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
> > > > @@ -59,8 +59,6 @@ &avb {
> > > >         status = "okay";
> > > >
> > > >         phy0: ethernet-phy@0 {
> > > > -               compatible = "ethernet-phy-id004d.d074",
> > > > -                            "ethernet-phy-ieee802.3-c22";
> > > >                 reg = <0>;
> > > >                 interrupt-parent = <&gpio2>;
> > > >                 interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> > >
> > > The next line:
> > >
> > >                 reset-gpios = <&gpio2 10 GPIO_ACTIVE_LOW>;
> > >
> > > Unfortunately, removing the compatible value will cause regressions
> > > for kexec/kdump and for Ethernet driver unbind, as the PHY reset will
> > > be asserted before starting the new kernel, or on driver unbind.
> > > Due to a deficiency in the Ethernet PHY subsystem, the PHY will be
> > > probed while the reset is still asserted, and thus fail probing[1].
> >
> > FWIW, the bootloader brings the device out of reset.  Would it be
>
> The bootloader is not involved when using kexec/kdump, or when
> unbinding the Ethernet driver.
>
> > sufficient to keep  "ethernet-phy-ieee802.3-c22" and drop the
> > hard-coded ID?
>
> I am afraid not, as that still requires actual probing to determine
> the PHY ID.

OK.  I'll try to find out how many of the older versions of the board
shipped. I don't really want to maintain two device trees for a small
population of boards.  Even those customers with early hardware won't
be getting the same versions going forward and Qualcomm/Atheros told
us it's an EOL part and cancelled our orders.  If there are no
objections, I might just change the ID to the new PHY.  The customers
who received the older hardware should have already been notified of
the hardware change and the fact they won't get any more with that
PHY.

adam
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
index d3fc8ffd5b4c..1eb713530878 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
@@ -59,8 +59,6 @@  &avb {
 	status = "okay";
 
 	phy0: ethernet-phy@0 {
-		compatible = "ethernet-phy-id004d.d074",
-			     "ethernet-phy-ieee802.3-c22";
 		reg = <0>;
 		interrupt-parent = <&gpio2>;
 		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;