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

Conor Dooley
2026-03-11T18:23:51Z

[-- Attachment #1.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 #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]