netdev - Re: [PATCH v2 1/5] ethernet: add sun8i-emac driver

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] [day] [month] [year] [list]

Message-ID: <20160826204914.GD3165@lukather>
Date:   Fri, 26 Aug 2016 22:49:14 +0200
From:   Maxime Ripard <maxime.ripard@...e-electrons.com>
To:     LABBE Corentin <clabbe.montjoie@...il.com>
Cc:     robh+dt@...nel.org, mark.rutland@....com, wens@...e.org,
        linux@...linux.org.uk, davem@...emloft.net, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, linux-sunxi@...glegroups.com
Subject: Re: [PATCH v2 1/5] ethernet: add sun8i-emac driver

Hi,

On Wed, Aug 24, 2016 at 02:02:21PM +0200, LABBE Corentin wrote:
> > > +/* Set Management Data Clock, must be call after device reset */
> > > +static void sun8i_emac_set_mdc(struct net_device *ndev)
> > > +{
> > > +	struct sun8i_emac_priv *priv = netdev_priv(ndev);
> > > +	unsigned long rate;
> > > +	u32 reg;
> > > +
> > > +	rate = clk_get_rate(priv->ahb_clk);
> > > +	if (rate > 160000000)
> > > +		reg = 0x3 << 20; /* AHB / 128 */
> > > +	else if (rate > 80000000)
> > > +		reg = 0x2 << 20; /* AHB / 64 */
> > > +	else if (rate > 40000000)
> > > +		reg = 0x1 << 20; /* AHB / 32 */
> > > +	else
> > > +		reg = 0x0 << 20; /* AHB / 16 */
> > > +	netif_dbg(priv, link, ndev, "MDC auto : %x\n", reg);
> > > +	writel(reg, priv->base + SUN8I_EMAC_MDIO_CMD);
> > 
> > You could also expose that as a clock.
> > 
> 
> For which purpose ?
> No ethernet driver expose the MDC as clock and I dont see any interest:
> - I dont think that tuning it give any gain
> - Knowing it's value is of little interest

You don't have to implement anything, you can just register a clk_div
driver, and everything works, and you would use the proper clock APIs
(ie. clk_set_rate, and that's it).

That would be exposed just like any other clock, including in debugfs,
which would remove the need for the debug call.

But this really was just a suggestion.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)