lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7fa2d457-4ae9-42f5-be73-80549aae558c@lunn.ch>
Date:   Sun, 16 Jul 2023 16:44:40 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Alexandru Ardelean <alex@...uggie.ro>
Cc:     Rob Herring <robh@...nel.org>, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, krzysztof.kozlowski+dt@...aro.org,
        conor+dt@...nel.org, hkallweit1@...il.com, linux@...linux.org.uk,
        olteanv@...il.com, marius.muresan@....ro
Subject: Re: [PATCH v2 2/2] dt-bindings: net: phy: vsc8531: document
 'vsc8531,clkout-freq-mhz' property

> 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.

They are not necessarily fixed clocks. The clock you are adding here
has three frequencies. Two frequencies is common for PHY devices. So
you need to use something more than clk-fixed-rate.c. Also, mostly
PHYs allows the clock to be gated.

> 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.

Who does not need it? The PHY, or the MAC? If it is the MAC, it should
really be the MAC driver which uses the common clock API to turn it
off. Just watch out for deadlocks with phydev->lock.

> 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?

Yes, have phy_device.c implement that registration/unregister of the
clock, deal with locking, and call into the PHY driver to actually
manipulate the clock. You missed the requested frequency in the
function prototype. I would also call it refclk. Three is sometimes
confusion about the different clocks.

Traditionally, clk_enable() can be called in atomic context, but that
is not allowed with phylib, it always assume thread context. I don't
know if the clock framework has some helpers for that, but i also
don't see there being a real need for MAC to enable the clock in
atomic context.

	Andrew

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ