All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Wei Fang <wei.fang@nxp.com>
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, krzk+dt@kernel.org, conor+dt@kernel.org,
	andrew@lunn.ch, f.fainelli@gmail.com, hkallweit1@gmail.com,
	linux@armlinux.org.uk, andrei.botila@oss.nxp.com,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, imx@lists.linux.dev
Subject: Re: [PATCH v3 net-next 1/2] dt-bindings: net: tja11xx: add "nxp,phy-output-refclk" property
Date: Mon, 26 Aug 2024 10:49:58 -0500	[thread overview]
Message-ID: <20240826154958.GA316598-robh@kernel.org> (raw)
In-Reply-To: <20240826052700.232453-2-wei.fang@nxp.com>

On Mon, Aug 26, 2024 at 01:26:59PM +0800, Wei Fang wrote:
> Per the RMII specification, the REF_CLK is sourced from MAC to PHY
> or from an external source. But for TJA11xx PHYs, they support to
> output a 50MHz RMII reference clock on REF_CLK pin. Previously the
> "nxp,rmii-refclk-in" was added to indicate that in RMII mode, if
> this property present, REF_CLK is input to the PHY, otherwise it
> is output. This seems inappropriate now. Because according to the
> RMII specification, the REF_CLK is originally input, so there is
> no need to add an additional "nxp,rmii-refclk-in" property to
> declare that REF_CLK is input.
> Unfortunately, because the "nxp,rmii-refclk-in" property has been
> added for a while, and we cannot confirm which DTS use the TJA1100
> and TJA1101 PHYs, changing it to switch polarity will cause an ABI
> break. But fortunately, this property is only valid for TJA1100 and
> TJA1101. For TJA1103/TJA1104/TJA1120/TJA1121 PHYs, this property is
> invalid because they use the nxp-c45-tja11xx driver, which is a
> different driver from TJA1100/TJA1101. Therefore, for PHYs using
> nxp-c45-tja11xx driver, add "nxp,phy-output-refclk" property to
> support outputting RMII reference clock on REF_CLK pin.
> 
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
> V2 changes:
> 1. Change the property name from "nxp,reverse-mode" to
> "nxp,phy-output-refclk".
> 2. Simplify the description of the property.
> 3. Modify the subject and commit message.
> V3 changes:
> 1. Keep the "nxp,rmii-refclk-in" property for TJA1100 and TJA1101.
> 2. Rephrase the commit message and subject.
> ---
>  Documentation/devicetree/bindings/net/nxp,tja11xx.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)

This binding is completely broken. I challenge you to make it report any 
errors. Those issues need to be addressed before you add more 
properties.

If you want/need custom properties, then you must have a compatible 
string.

> 
> diff --git a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml
> index 85bfa45f5122..f775036a7521 100644
> --- a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml
> +++ b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml
> @@ -48,6 +48,12 @@ patternProperties:
>            reference clock output when RMII mode enabled.
>            Only supported on TJA1100 and TJA1101.
>  
> +      nxp,phy-output-refclk:

Why not "nxp,rmii-refclk-out" if this is just the reverse of the 
existing property.

> +        type: boolean
> +        description: |
> +          Enable 50MHz RMII reference clock output on REF_CLK pin. This
> +          property is only applicable to nxp-c45-tja11xx driver.
> +
>      required:
>        - reg
>  
> -- 
> 2.34.1
> 

  reply	other threads:[~2024-08-26 15:50 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-26  5:26 [PATCH v3 net-next 0/2] make PHY output RMII reference clock Wei Fang
2024-08-26  5:26 ` [PATCH v3 net-next 1/2] dt-bindings: net: tja11xx: add "nxp,phy-output-refclk" property Wei Fang
2024-08-26 15:49   ` Rob Herring [this message]
2024-08-27  3:25     ` Wei Fang
2024-08-27 12:34       ` Andrew Lunn
2024-08-28  7:35         ` Wei Fang
2024-08-27 13:26       ` Krzysztof Kozlowski
2024-08-28  7:37         ` Wei Fang
2024-08-26  5:27 ` [PATCH v3 net-next 2/2] net: phy: c45-tja11xx: add support for outputing RMII reference clock Wei Fang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240826154958.GA316598-robh@kernel.org \
    --to=robh@kernel.org \
    --cc=andrei.botila@oss.nxp.com \
    --cc=andrew@lunn.ch \
    --cc=conor+dt@kernel.org \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=imx@lists.linux.dev \
    --cc=krzk+dt@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=wei.fang@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.