Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 19 Aug 2022 14:44:42 +0200
From:   Andrew Lunn <andrew@lunn.ch>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc:     Wei Fang <wei.fang@nxp.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "krzysztof.kozlowski+dt@linaro.org" 
        <krzysztof.kozlowski+dt@linaro.org>,
        "f.fainelli@gmail.com" <f.fainelli@gmail.com>,
        "hkallweit1@gmail.com" <hkallweit1@gmail.com>,
        "linux@armlinux.org.uk" <linux@armlinux.org.uk>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next 1/2] dt-bindings: net: tja11xx: add
 nxp,refclk_in property
Message-ID: <Yv+FuiUoTjpoUZ32@lunn.ch>
References: <20220819074729.1496088-1-wei.fang@nxp.com>
 <20220819074729.1496088-2-wei.fang@nxp.com>
 <f0f6e8af-4006-e0e8-544b-f2f892d79a1f@linaro.org>
 <DB9PR04MB81064199835C0E44B997DE06886C9@DB9PR04MB8106.eurprd04.prod.outlook.com>
 <9ec575ba-784d-74f7-8861-da2f62fe0773@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <9ec575ba-784d-74f7-8861-da2f62fe0773@linaro.org>
Precedence: bulk

On Fri, Aug 19, 2022 at 02:37:36PM +0300, Krzysztof Kozlowski wrote:
> On 19/08/2022 12:37, Wei Fang wrote:
> >>
> >>> +          in RMII mode. This clock signal is provided by the PHY and is
> >>> +          typically derived from an external 25MHz crystal. Alternatively,
> >>> +          a 50MHz clock signal generated by an external oscillator can be
> >>> +          connected to pin REF_CLK. A third option is to connect a 25MHz
> >>> +          clock to pin CLK_IN_OUT. So, the REF_CLK should be configured
> >>> +          as input or output according to the actual circuit connection.
> >>> +          If present, indicates that the REF_CLK will be configured as
> >>> +          interface reference clock input when RMII mode enabled.
> >>> +          If not present, the REF_CLK will be configured as interface
> >>> +          reference clock output when RMII mode enabled.
> >>> +          Only supported on TJA1100 and TJA1101.
> >>
> >> Then disallow it on other variants.
> >>
> >> Shouldn't this be just "clocks" property?
> >>
> >>
> > This property is to configure the pin REF_CLK of PHY as a input pin through phy register,
> > indicates that the REF_CLK signal is provided by an external oscillator. so I don't think it's a
> > "clock" property.
> 
> clocks, not clock.
> 
> You just repeated pieces of description as an counter-argument, so this
> does not explain anything.
> 
> If it is external oscillator shouldn't it be represented in DTS and then
> obtained by driver (clk_get + clk_prepare_enable)? Otherwise how are you
> sure that clock is actually enabled? And the lack of presence of the
> external clock means it is derived from PHY?

Using the common clock framework has been discussed in the past. But
no PHY actually does this. When the SoC provides the clock, a few PHYs
do make use of the common clock framework as clock consumers to ensure
the clock is ticking.

Plus, as the description says, this pin can be either a clock producer
or a consumer. I don't think the common clock code allows this. It is
also not something you negotiate between the MAC and PHY. The hardware
designer typically decides based on the MAC and PHY actually used. So
this is a fixed hardware property.

     Andrew