[PATCH] spi: Correct SPI clock frequency setting in spi_mpc8xxx

[Ernst Schwab]

Grant Likely <grant.likely at secretlab.ca> wrote:

> The change forces the division to always round up instead of down.
> Please describe (for me now, and for people looking at the commit in
> the future) the mathematical reason for the changes.

Ok, here are some infos on the problem I observed, and the fix.

Example:

When specifying spi-max-frequency = <10000000> in the device tree,
the resulting frequency was 11.1 MHz, with spibrg being 133333332. 

According to the freescale datasheet [1], the spi clock rate is
spiclk = spibrg / (4 * (pm+1))

The existing code calculated  
   pm = mpc8xxx_spi->spibrg / (hz * 4); pm--;
resulting in pm = (int) (3.3333) - 1 = 2, 
resulting in spiclk = 133333332/(4*(2+1)) = 11111111

With the fix, 
  pm = (mpc8xxx_spi->spibrg - 1) / (hz * 4) + 1; pm--;
resulting in pm = (int) (4.3333) - 1 = 3,
resulting in spiclk = 133333332/(4*(3+1)) = 8333333

Without the fix, for every desired SPI frequency that 
is not exactly deriveable from spibrg, pm will be too 
small due to rounding down, resulting in a too high SPI clock,
so we need a pm which is one higher.

For values that are exactly deriveable, spibrg will
be divideable by (hz*4) without remainder, and 
(int) ((spibrg-1)/(hz*4)) will be one lower than 
(int) (spibrg)/(hz*4), which is compensated by adding 1.
For these values, the fixed version calculates the same pm 
as the unfixed version.

For all values that are not exactly deriveable,
spibrg will be not divideable by (hz*4) without 
remainder, and (int) ((spibrg-1)/(hz*4)) will be 
the same as (int) (spibrg)/(hz*4), and the calculated pm will
be one higher than calculated by the unfixed version.

Regards
Ernst

References:
[1] http://www.freescale.com/files/32bit/doc/ref_manual/MPC8315ERM.pdf,
    page 22-10 -> 1398
"
pm: Prescale modulus select. Specifies the divide ratio of the 
prescale divider in the SPI clock generator. The SPI baud rate generator
clock source (either system clock or system clock divided by 16, 
depending on DIV16 bit) is divided by 4 * ([PM] + 1), a range from 4 to 64. 
The clock has a 50% duty cycle. For example, if the prescale
modulus is set to PM = 0011 and DIV16 is set, the system/SPICLK clock 
ratio will be 16 * (4 * (0011 + 1)) = 256.
"


> >
> > Signed-off-by: Ernst Schwab <eschwab at online.de>
> > ---
> > Tested on MPC8314.
> >
> > diff -up linux-2.6.33-rc8.orig/drivers/spi/spi_mpc8xxx.c linux-2.6.33-rc8/drivers/spi/spi_mpc8xxx.c
> > --- linux-2.6.33-rc8.orig/drivers/spi/spi_mpc8xxx.c     2010-02-12 20:07:45.000000000 +0100
> > +++ linux-2.6.33-rc8/drivers/spi/spi_mpc8xxx.c  2010-02-15 14:08:33.000000000 +0100
> > @@ -365,7 +365,7 @@ int mpc8xxx_spi_setup_transfer(struct sp
> >
> >        if ((mpc8xxx_spi->spibrg / hz) > 64) {
> >                cs->hw_mode |= SPMODE_DIV16;
> > -               pm = mpc8xxx_spi->spibrg / (hz * 64);
> > +               pm = (mpc8xxx_spi->spibrg - 1) / (hz * 64) + 1;
> >
> >                WARN_ONCE(pm > 16, "%s: Requested speed is too low: %d Hz. "
> >                          "Will use %d Hz instead.\n", dev_name(&spi->dev),
> > @@ -373,7 +373,7 @@ int mpc8xxx_spi_setup_transfer(struct sp
> >                if (pm > 16)
> >                        pm = 16;
> >        } else
> > -               pm = mpc8xxx_spi->spibrg / (hz * 4);
> > +               pm = (mpc8xxx_spi->spibrg - 1) / (hz * 4) + 1;
> >        if (pm)
> >                pm--;
> >
> >
> >
> >
> > --
> >
> >
> 
> 
> 
> -- 
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.


--