Re: [PATCH 2/2] dt-bindings: net: phy: vsc8531: document 'vsc8531,clkout-freq-mhz' property

[Alexandru Ardelean]

On Sat, Jul 8, 2023 at 9:25 PM Andrew Lunn <andrew@xxxxxxx> wrote:
>
> On Thu, Jul 06, 2023 at 11:15:54AM +0300, Alexandru Ardelean wrote:
> > For VSC8351 and similar PHYs, a new property was added to generate a clock
> > signal on the CLKOUT pin.
> > This change documents the change in the device-tree bindings doc.
> >
> > Signed-off-by: Alexandru Ardelean <alex@xxxxxxxxxxx>
> > ---
> >  Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> > index 0a3647fe331b..133bdd644618 100644
> > --- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> > +++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> > @@ -31,6 +31,10 @@ Optional properties:
> >                         VSC8531_LINK_100_ACTIVITY (2),
> >                         VSC8531_LINK_ACTIVITY (0) and
> >                         VSC8531_DUPLEX_COLLISION (8).
> > +- vsc8531,clkout-freq-mhz : For VSC8531 and similar PHYs, this will output
> > +                       a clock signal on the CLKOUT pin of the chip.
> > +                       The supported values are 25, 50 & 125 Mhz.
> > +                       Default value is no clock signal on the CLKOUT pin.
>
> It is possible this could cause regressions. The bootloader could
> turned the clock on, and then Linux leaves it alone. Now, it will get
> turned off unless a DT property is added.
>
> I prefer to explicitly have the property, so there is no dependency on
> the bootloader, so lets leave it like this. But if we do get
> regressions reported, this might need to change.

Well, we could also need add a "mscc,clkout-freq-mhz = <0>" handling
where the CLKOUT pin gets disabled explicitly (if needed, after the
bootloade), for some weird corner cases.
Though, to-be-honest, I can't think of any (remotely) reasonable ones.

It would definitely be simple to just make sure that Linux does not do
any changes if this property isn't present.

If you're on board about having this as-is, I will keep it; and spin a
V2 just with 'vsc8531,clkout-freq-mhz ' -> 'mscc,clkout-freq-mhz' as
Rob requested.

Thanks
Alex

>
>    Andrew