Re: [PATCH net-next v3 03/10] dt-bindings: net: cdns,macb: replace cdns,refclk-ext with cdns,refclk-source

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3740 bytes --]

On Wed, Mar 11, 2026 at 07:10:09PM +0100, Krzysztof Kozlowski wrote:
> On 11/03/2026 19:08, Conor Dooley wrote:
> > On Wed, Mar 11, 2026 at 08:00:54AM +0100, Krzysztof Kozlowski wrote:
> >> On Tue, Mar 10, 2026 at 05:17:10PM +0000, Conor Dooley wrote:
> >>> From: Conor Dooley <conor.dooley@microchip.com>
> >>>
> >>> Ryan added cdns,refclk-ext with the intent of decoupling the source of
> >>> the reference clock on sama7g5 (and related platforms) from the
> >>> compatible. Unfortunately, the default for sama7g5-emac is an external
> >>> reference clock, so this property had no effect there, so that
> >>> compatibility with older devicetrees is preserved.
> >>> Replace cdns,refclk-ext with one that supports both default states and
> >>> therefore is usable for sama7g5-emac.
> >>>
> >>> For now, limit it to only the platforms that have USRIO controlled
> >>> reference clock selection, but this could be generalised in the future.
> >>>
> >>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> >>> ---
> >>>  .../devicetree/bindings/net/cdns,macb.yaml    | 39 ++++++++++++++++---
> >>>  1 file changed, 34 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> >>> index cb14c35ba9969..a492357570edd 100644
> >>> --- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
> >>> +++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> >>> @@ -120,12 +120,14 @@ properties:
> >>>    power-domains:
> >>>      maxItems: 1
> >>>  
> >>> -  cdns,refclk-ext:
> >>> -    type: boolean
> >>
> >> This is already an implemented ABI, so you cannot drop it even if some
> >> devices works fine on default. Instead please deprecate it and keep
> >> drivers backwards compatible for whoever is using it.
> > 
> > The driver still will support it for those devices, but I don't see any
> > value in keeping the property about, even deprecated, when it doesn't
> > work properly. If you insist that it be kept and marked deprecated, it
> 
> There cannot be an undocumented ABI, so if driver has it, it must stay
> in the bindings.

Right that's something I should have considered.
I'll keep it so.

> > needs conditional rules to only permit it on devices that it actually
> > does something. I'm surprised you even considered that I would drop
> > support for it from the driver!
> 
> Commit msg suggested it is no-op, thus could be dropped without having
> impact?

Nah, it's not an NOP. Commit message maybe reads that way because I was
trying to avoid mentioning the driver implementation details. I think I
could do a better job talking about sama7g5-emac versus sama7g5-gem in
the commit message, which would show that the current property has a use
case, just not as broad as it was intended to be.

The existing property was added along with a driver change that modified
sama7g5-emac which had a default of external to default to internal instead,
and the property could be used to switch to external. sama7g5-gem had a
default of internal, so this change was done to make things uniform and
allow a single property to work for both devices.
That driver change was reverted to maintain backwards compatibility, so
the property became useless on sama7g5-emac, and there's currently no
way to make that device use an internal reference.

What I am doing here is probably what cdns,refclk-ext should have been
originally, but when the driver's defaults got reverted the whole series
probably should have been reverted so that Ryan could try again with a
new approach. Instead, the property went into a release but only works
on sama7g5-gem.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]