On Sat, Dec 26, 2015 at 04:53:05PM +0100, Marcus Weseloh wrote: > This patch fixes multiple problems with the current clock calculations: > > 1. The A10/A20 datasheet contains the formula AHB_CLK / (2^(n+1)) to > calculate SPI_CLK from CDR1, but this formula is wrong. The actual > formula - determined by analyzing the actual waveforms - is > AHB_CLK / (2^n). > > 2. The divisor calculations for CDR1 and CDR2 both rounded to the > nearest integer. This could lead to a transfer speed that is higher than > the requested speed. This patch changes both calculations to always > round down. > > 3. The mclk frequency was only ever increased, never decreased. This could > lead to unpredictable transfer speeds, depending on the order in which > transfers with different speeds where serviced by the SPI driver.
These 3 should probably be separate patches (and be Cc'd to stable > Signed-off-by: Marcus Weseloh <[email protected]> > --- > drivers/spi/spi-sun4i.c | 26 ++++++++++++++++---------- > 1 file changed, 16 insertions(+), 10 deletions(-) > > diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c > index f60a6d6..d67e142 100644 > --- a/drivers/spi/spi-sun4i.c > +++ b/drivers/spi/spi-sun4i.c > @@ -79,6 +79,9 @@ struct sun4i_spi { > struct clk *hclk; > struct clk *mclk; > > + int cur_max_speed; > + int cur_mclk; > + > struct completion done; > > const u8 *tx_buf; > @@ -227,11 +230,17 @@ static int sun4i_spi_transfer_one(struct spi_master > *master, > > sun4i_spi_write(sspi, SUN4I_CTL_REG, reg); > > - /* Ensure that we have a parent clock fast enough */ > + /* > + * Ensure that the parent clock is set to twice the max speed > + * of the spi device (possibly rounded up by the clk driver) > + */ > mclk_rate = clk_get_rate(sspi->mclk); > - if (mclk_rate < (2 * tfr->speed_hz)) { > - clk_set_rate(sspi->mclk, 2 * tfr->speed_hz); > + if (spi->max_speed_hz != sspi->cur_max_speed || > + mclk_rate != sspi->cur_mclk) { Do you need to cache the values? As far as I'm aware, you end up doing the computation all the time anyway. > + clk_set_rate(sspi->mclk, 2 * spi->max_speed_hz); > mclk_rate = clk_get_rate(sspi->mclk); > + sspi->cur_mclk = mclk_rate; > + sspi->cur_max_speed = spi->max_speed_hz; > } > > /* > @@ -239,7 +248,7 @@ static int sun4i_spi_transfer_one(struct spi_master > *master, > * > * We have two choices there. Either we can use the clock > * divide rate 1, which is calculated thanks to this formula: > - * SPI_CLK = MOD_CLK / (2 ^ (cdr + 1)) > + * SPI_CLK = MOD_CLK / (2 ^ cdr) > > * Or we can use CDR2, which is calculated with the formula: > * SPI_CLK = MOD_CLK / (2 * (cdr + 1)) > * Wether we use the former or the latter is set through the > @@ -248,14 +257,11 @@ static int sun4i_spi_transfer_one(struct spi_master > *master, > * First try CDR2, and if we can't reach the expected > * frequency, fall back to CDR1. > */ > - div = mclk_rate / (2 * tfr->speed_hz); > - if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) { > - if (div > 0) > - div--; > - > + div = DIV_ROUND_UP(mclk_rate, 2 * tfr->speed_hz) - 1; Isn't it exactly the same thing as mclk_rate / (2 * tfr->speed_hz) ? Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
signature.asc
Description: Digital signature
