On Thu, Jul 16, 2009 at 3:42 PM, Wolfgang Denk<w...@denx.de> 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 <w...@denx.de> > Cc: Grant Likely <grant.lik...@secretlab.ca> > Cc: Kumar Gala <ga...@kernel.crashing.org> > Cc: <net...@vger.kernel.org>
Mostly good. One comment below. When it's resolved, feel free to add my acked-by line. Thanks for getting this done. > @@ -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; > + } Just thought of something. If it cannot find the clock, then wouldn't it be better to just use the maximum divider and print a warning instead of bailing completely? This goes for the other patch as well. > + > + /* 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 > > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev