On Fri, Jul 14, 2023 at 8:24 PM Rob Herring <robh@xxxxxxxxxx> wrote: > > On Thu, Jul 13, 2023 at 11:21:23PM +0300, Alexandru Ardelean wrote: > > For VSC8351 and similar PHYs, a new property was added to generate a clock > > signal on the CLKOUT pin. > > Sorry, didn't think about it on v1, but I would imagine other vendors' > PHYs have similar functionality. We should have something common. We > have the clock binding for clocks already, so we should consider if > that should be used here. It may look like an overkill for what you > need, but things always start out that way. What if you want to turn the > clock on and off as well? So, there's the adin.c PHY driver which has a similar functionality with the adin_config_clk_out(). Something in the micrel.c PHY driver (with micrel,rmii-reference-clock-select-25-mhz); hopefully I did not misread the code about that one. And the at803x.c PHY driver has a 'qca,clk-out-frequency' property too. Now with the mscc.c driver, there is a common-ality that could use a framework. @Rob are you suggesting something like registering a clock provider (somewhere in the PHY framework) and let the PHY drivers use it? Usually, these clock signals (once enabled on startup), don't get turned off; but I've worked mostly on reference designs; somewhere down the line some people get different requirements. These clocks get connected back to the MAC (usually), and are usually like a "fixed-clock" driver. In our case, turning off the clock would be needed if the PHY negotiates a non-gigabit link; i.e 100 or 10 Mbps; in that case, the CLKOUT signal is not needed and it can be turned off. Maybe start out with a hook in 'struct phy_driver'? Like "int (*config_clk_out)(struct phy_device *dev);" or something? And underneath, this delegates to the CLK framework? I'd let Andrew (or someone in netdev) have a final feedback here. I can (probably) try to allocate some time to do this change based on the MSCC driver in the next weeks, if there's a consensus. Thanks Alex > > > This change documents the change in the device-tree bindings doc. > > That's obvious. > > > > > Signed-off-by: Alexandru Ardelean <alex@xxxxxxxxxxx> > > --- > > > > Changelog v1 -> v2: > > * https://lore.kernel.org/netdev/20230706081554.1616839-2-alex@xxxxxxxxxxx/ > > * changed property name 'vsc8531,clkout-freq-mhz' -> 'mscc,clkout-freq-mhz' > > as requested by Rob > > * added 'net-next' tag as requested by Andrew > > > > Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt > > index 0a3647fe331b..085d0e8a834e 100644 > > --- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt > > +++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt > > @@ -31,6 +31,10 @@ Optional properties: > > VSC8531_LINK_100_ACTIVITY (2), > > VSC8531_LINK_ACTIVITY (0) and > > VSC8531_DUPLEX_COLLISION (8). > > +- mscc,clkout-freq-mhz : For VSC8531 and similar PHYs, this will output > > + a clock signal on the CLKOUT pin of the chip. > > + The supported values are 25, 50 & 125 Mhz. > > + Default value is no clock signal on the CLKOUT pin. > > - load-save-gpios : GPIO used for the load/save operation of the PTP > > hardware clock (PHC). > > > > @@ -69,5 +73,6 @@ Example: > > vsc8531,edge-slowdown = <7>; > > vsc8531,led-0-mode = <VSC8531_LINK_1000_ACTIVITY>; > > vsc8531,led-1-mode = <VSC8531_LINK_100_ACTIVITY>; > > + mscc,clkout-freq-mhz = <50>; > > load-save-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>; > > }; > > -- > > 2.41.0 > >
