On 19/08/2022 15:44, Andrew Lunn wrote: > 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. IOW, all DTSes would have a fixed clock stub without any logic usable by Common CF (like enabling)? > 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. Indeed. Anyway the property name and typo need fixes. Best regards, Krzysztof