On 3/18/10, Mika Westerberg <mika.westerb...@iki.fi> wrote: > This patch adds an SPI master driver for the Cirrus EP93xx SPI controller > found > in EP93xx chips (EP9301, EP9302, EP9307, EP9312 and EP9315). > > Driver currently supports only interrupt driven mode but in future we may add > polling mode support as well. > > Signed-off-by: Mika Westerberg <mika.westerb...@iki.fi> > ---
> diff --git a/drivers/spi/ep93xx_spi.c b/drivers/spi/ep93xx_spi.c > new file mode 100644 > index 0000000..17935f3 > --- /dev/null > +++ b/drivers/spi/ep93xx_spi.c > +/** > + * ep93xx_spi_calc_divisors() - calculates SPI clock divisors > + * @espi: ep93xx SPI controller struct > + * @chip: divisors are calculated for this chip > + * @rate: maximum rate (in Hz) that this chip supports Isn't that > + * @rate: desired SPI output clock rate > + * > + * Function calculates cpsr (clock pre-scaler) and scr divisors based on > + * given @rate and places them to @chip->div_cpsr and @chip->div_scr. If, > + * for some reason, divisors cannot be calculated nothing is stored and > + * %-EINVAL is returned. > + */ > +static int ep93xx_spi_calc_divisors(const struct ep93xx_spi *espi, > + struct ep93xx_spi_chip *chip, > + unsigned long rate) > +{ ... > + /* > + * Calculate divisors so that we can get speed according the > + * following formula: > + * rate = max_speed / (cpsr * (1 + scr)) > + * where max_speed is same as SPI clock rate. > + * > + * cpsr must be even number and starts from 2, scr can be any number > + * between 0 and 255. > + */ > + for (cpsr = 2; cpsr <= 254; cpsr += 2) { > + for (scr = 0; scr <= 255; scr++) { > + if ((espi->max_rate / (cpsr * (scr + 1))) < rate) { Shouldn't that be > + if (clk_get_rate(espi->clk) / (cpsr * (scr + 1)) <= > rate) { since max_rate is the maximum usable rate, which is sspclk / 2, so the existing code seems like it would set the divisors to give twice the requested frequency. Conversely, if an exact divisor of the master clock rate is requested (such as 1843200), the "<" would give the next lower rate instead of the requested rate so for the highest rates the doubling and this halving would cancel out, while for lower rates you could get up to twice the requested clock frequecy. > +static int __init ep93xx_spi_probe(struct platform_device *pdev) > +{ ... > + espi->max_rate = clk_get_rate(espi->clk) / 2; > + espi->min_rate = clk_get_rate(espi->clk) / (254 * 255); Isn't that > + espi->min_rate = clk_get_rate(espi->clk) / (254 * 256); since the output SPI clock frequency is sspclk / (cpsr * (1 + scr)) where cpsr = 2..254 step 2, scr = 1..255 I also notice that the code recalculates the clock divisors, programs them ad resotres them for every transfer, even if the frequency etc is always the same. I don't know if this makes much difference to speed in normal operation but it does make the debug output incredibly verbose, halving transfer speeds when SPI debug is enabled. Is there a suitable place to be lazy about this, so as to avoid doing this if the transfer parameters are the same on successive operations? Thanks M ------------------------------------------------------------------------------ Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev _______________________________________________ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general