netdev - Re: [PATCH 2/2 v2] MPC52xx FEC: be more conservative when setting MII

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: <20090717094725.GC3150@pengutronix.de>
Date:	Fri, 17 Jul 2009 11:47:25 +0200
From:	Wolfram Sang <w.sang@...gutronix.de>
To:	Wolfgang Denk <wd@...x.de>
Cc:	linuxppc-dev@...abs.org, netdev@...r.kernel.org
Subject: Re: [PATCH 2/2 v2] MPC52xx FEC: be more conservative when setting
	MII_SPEED register


Hi Wolfgang,

On Thu, Jul 16, 2009 at 11:42:26PM +0200, Wolfgang Denk wrote:
> This patch adds error checking and prevents clobbering unrelated bits
> (reserved bits or the DIS_PREAMBLE bit) when writing the MII_SPEED
> register on MPC52xx systems.
> 
> Signed-off-by: Wolfgang Denk <wd@...x.de>
> Cc: Grant Likely <grant.likely@...retlab.ca>
> Cc: Kumar Gala <galak@...nel.crashing.org>
> Cc: <netdev@...r.kernel.org>
> ---
>  drivers/net/fec_mpc52xx.c     |    2 +-
>  drivers/net/fec_mpc52xx_phy.c |   21 ++++++++++++++++++---
>  2 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
> index cc78633..b69d440 100644
> --- a/drivers/net/fec_mpc52xx.c
> +++ b/drivers/net/fec_mpc52xx.c
> @@ -639,7 +639,7 @@ static void mpc52xx_fec_hw_init(struct net_device *dev)
>  	/* set phy speed.
>  	 * this can't be done in phy driver, since it needs to be called
>  	 * before fec stuff (even on resume) */
> -	out_be32(&fec->mii_speed, priv->mdio_speed);
> +	clrsetbits_be32(&fec->mii_speed, 0x7E, priv->mdio_speed);
>  }

In the probe-function when mdio_speed is set, there is still the old formula
used. Wouldn't that be better in sync?

>  
>  /**
> diff --git a/drivers/net/fec_mpc52xx_phy.c b/drivers/net/fec_mpc52xx_phy.c
> index 31e6d62..4c33dc5 100644
> --- a/drivers/net/fec_mpc52xx_phy.c
> +++ b/drivers/net/fec_mpc52xx_phy.c
> @@ -70,7 +70,7 @@ static int mpc52xx_fec_mdio_probe(struct of_device *of,
>  	struct mpc52xx_fec_mdio_priv *priv;
>  	struct resource res = {};
>  	int err;
> -	int i;
> +	int i, clock, speed;
>  
>  	bus = mdiobus_alloc();
>  	if (bus == NULL)
> @@ -105,8 +105,23 @@ static int mpc52xx_fec_mdio_probe(struct of_device *of,
>  	dev_set_drvdata(dev, bus);
>  
>  	/* set MII speed */
> -	out_be32(&priv->regs->mii_speed,
> -		((mpc5xxx_get_bus_frequency(of->node) >> 20) / 5) << 1);
> +	clock = mpc5xxx_get_bus_frequency(of->node);
> +	if (!clock) {
> +		dev_err(&of->dev, "could not determine IPS/IPB clock\n");
> +		goto out_unmap;
> +	}
> +
> +	/* scale for a MII clock <= 2.5 MHz */
> +	speed = (clock + 2499999) / 2500000;
> +
> +	/* only 6 bits (25:30) available for MII speed */
> +	if (speed > 0x3F) {
> +		speed = 0x3F;
> +		dev_err(&of->dev, "MII clock (%d Hz) exceeds max (2.5 MHz)\n",
> +			clock / speed);
> +	}
> +
> +	clrsetbits_be32(&priv->regs->mii_speed, 0x7E, speed << 1);
>  
>  	err = of_mdiobus_register(bus, np);
>  	if (err)
> -- 
> 1.6.0.6
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@...ts.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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