Hi Marek,
On Tue, Oct 22, 2024 at 4:07 AM Marek Vasut <marex@xxxxxxx> wrote:
> On 10/21/24 9:13 AM, Geert Uytterhoeven wrote:
> > On Mon, Oct 21, 2024 at 2:13 AM Marek Vasut <marex@xxxxxxx> wrote:
> >> On 10/15/24 4:48 PM, Niklas Söderlund wrote:
> >>>>> However, the reset signal may be in asserted state when the PHY is
> >>>>> probed (e.g. after unbind from the Ethernet driver, or during kexec).
> >>>>> Identifying the PHY by reading the ID register requires deasserting
> >>>>> the reset first.
> >>>> That may not be the entire precondition. For example the SMSC LAN87xx PHYs
> >>>> also require PHY clock to be enabled before the reset is toggled, but such
> >>>> information is available only to the specific PHY driver.
> >>>>
> >>>> The MDIO-level reset GPIO handling, as far as I understand it, applies in
> >>>> case there are more PHYs on the MDIO bus which share the same reset GPIO
> >>>> line.
> >>>>
> >>>> In this case there is only one PHY on the MDIO bus, so the only bit which
> >>>> applies is the potential PHY-specific reset requirement handling. If the PHY
> >>>> driver ever gets extended with such a thing in the future, then having the
> >>>> reset-gpios in the PHY node is beneficial over having it in MDIO node.
> >>>>
> >>>> It will always be a compromise between the above and best-effort PHY
> >>>> auto-detection though.
> >>>
> >>> I agree this is not needed if the PHY is identified by the compatible
> >>> string, but might be if it is not. In this case it works and the reason
> >>> for this patch was just to align the style used here.
> >>>
> >>> I'm happy to drop this patch, or send a rebased version that applies
> >>> since the context changed ;-) Marek, Geert what is your view? I'm happy
> >>> with either option.
> >>
> >> I was hoping Geert would comment on this first, but seems like maybe no.
> >> I think, since the PHY node does have a compatible string AND the reset
> >> is connected to the PHY, I would keep the reset property in the PHY
> >> node. Sorry.
> >
> > You are inverting the reasoning ;-) The compatible strings were added
> > because otherwise the PHY core can not identify the PHY when the
> > reset is asserted (e.g. after kexec).
>
> ... or because the PHY requires some complex sequence to bring it up, it
> is not just reset.
That is your hypothetical case, but not the reason behind commit
722d55f3a9bd810f ("arm64: dts: renesas: Add compatible properties to
KSZ9031 Ethernet PHYs").
> > If possible, I'd rather remove
> > the compatible strings again, as different PHYs may be mounted on
> > different PHY revisions, causing a headache for DTB management.
>
> Will that ever be the case with this hardware ?
Dunno. It did happen with the Beacon boards.
> > So, what would you suggest when the PHY nodes would not have compatible
> > strings?
> I would suggest keep the PHY compatible strings, because that is the
> most accurate method to describe the hardware and fulfill the PHY bring
> up requirements. If the PHY changes on this hardware in some future
That issue is moot for KSZ9031.
> revision, we can revisit this discussion ? Maybe bootloader-applied DTOs
> could work then ?
So, what would you suggest when the PHY nodes would not have compatible
strings?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
