Subject: Re: [PATCH v4 1/3] dt-bindings: net: adin: document phy clock

From: Jakub Kicinski <[email-masked]>
Date: Wed, 11 May 2022 12:42:41 -0700

On Wed, 11 May 2022 19:10:36 +0200 Michael Walle wrote:
> Am 2022-05-11 18:11, schrieb Jakub Kicinski:
> > On Wed, 11 May 2022 14:58:55 +0200 Michael Walle wrote:  
> >> FWIW, the recovered clock only works if there is a link. AFAIR on the
> >> AR8031 you can have the free-running one enabled even if there is no
> >> link, which might sometimes be useful.  
> > 
> > Is that true for all PHYs? I've seen "larger" devices mention holdover
> > or some other form of automatic fallback in the DPLL if input clock is
> > lost.  
> 
> I certainly can't speak of 'all' PHYs, who can ;) But how is the
> switchover for example? hitless? will there be a brief period of
> no clock at all?
> 
> The point I wanted to add is that the user should have the choice or
> at least you should clearly mention that. If you drop the suffix and 
> just
> use "25mhz" is that now the recovered one or the free-running one. And
> why is that one preferred over the other? Eg. if I were a designer for a
> cheapo board and I'd need a 125MHz clock and want to save some bucks
> for the 125MHz osc and the PHY could supply one, I'd use the
> free-running mode. Just to avoid any surprises with a switchover
> or whatever.

It's pure speculation on my side. I don't even know if PHYs use 
the recovered clock to clock its output towards the MAC or that's 
a different clock domain.

My concern is that people will start to use DT to configure SyncE which
is entirely a runtime-controllable thing, and doesn't belong. Hence
my preference to hide the recovered vs free-running detail if we can
pick one that makes most sense for now.

Again, if you feel strongly that the current design is indeed needed 
to configure pure SOC<>PHY / MAC<>PHY clocking, just send a review tag
and I'll apply :)

> > I thought that's the case here, too:
> 
> I read "reference" as being the 25MHz (maybe when the PHY is in 10Mbit
> mode? I didn't read the datasheet) because the first mode is called
> 25mhz-reference. So it might be switching between 25MHz and 125MHz?
> I don't know.

I couldn't grok that from the datasheet. Anyway, it'd be good to make
it clearer that the second sentence refers to the "adaptive" mode and
not the behavior of the recovered clock when lock is lost. Just put
(adaptive) in the sentence somewhere.