On Tue, Feb 16, 2010 at 9:43 AM, Ernst Schwab <esch...@online.de> wrote: > Grant Likely <grant.lik...@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.
Excellent description. Thank you. I'll add this to the commit text when I apply the patch. g. > > 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 <esch...@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. > > > -- > > -- 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