Hi Aaron,

Thanks for the fixes! Some comments below:

On 2015-03-30 19:49, Aaron Brice wrote:
> Add delay between chip select and clock signals, before clock starts and
> after clock stops.

This 50ns are specifc to the SPI slave at hand (SRAM) is this correct?
If yes, this would probably need new slave properties. I found a TI
controller which also supports such properties:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/285761.html

> 
> Signed-off-by: Aaron Brice <aaron.br...@datasoft.com>
> ---
>  drivers/spi/spi-fsl-dspi.c | 53 
> ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 51 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
> index 96cac87..41284cc 100644
> --- a/drivers/spi/spi-fsl-dspi.c
> +++ b/drivers/spi/spi-fsl-dspi.c
> @@ -20,6 +20,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/kernel.h>
> +#include <linux/math64.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> @@ -51,7 +52,7 @@
>  #define SPI_CTAR_CPOL(x)     ((x) << 26)
>  #define SPI_CTAR_CPHA(x)     ((x) << 25)
>  #define SPI_CTAR_LSBFE(x)    ((x) << 24)
> -#define SPI_CTAR_PCSSCR(x)   (((x) & 0x00000003) << 22)
> +#define SPI_CTAR_PCSSCK(x)   (((x) & 0x00000003) << 22)
>  #define SPI_CTAR_PASC(x)     (((x) & 0x00000003) << 20)
>  #define SPI_CTAR_PDT(x)      (((x) & 0x00000003) << 18)
>  #define SPI_CTAR_PBR(x)      (((x) & 0x00000003) << 16)
> @@ -59,6 +60,7 @@
>  #define SPI_CTAR_ASC(x)      (((x) & 0x0000000f) << 8)
>  #define SPI_CTAR_DT(x)               (((x) & 0x0000000f) << 4)
>  #define SPI_CTAR_BR(x)               ((x) & 0x0000000f)
> +#define SPI_CTAR_SCALE_BITS  0xf
>  
>  #define SPI_CTAR0_SLAVE      0x0c
>  
> @@ -99,6 +101,8 @@
>  #define SPI_CS_ASSERT                0x02
>  #define SPI_CS_DROP          0x04
>  
> +#define NSEC_PER_SEC         1000000000L
> +

Avoid redefine this, should be available from headers (e.g. time.h)

>  struct chip_data {
>       u32 mcr_val;
>       u32 ctar_val;
> @@ -174,6 +178,40 @@ static void hz_to_spi_baud(char *pbr, char *br,
> int speed_hz,
>       }
>  }
>  
> +static void ns_delay_scale(char *psc, char *sc, int delay_ns,
> +             unsigned long clkrate)
> +{
> +     int pscale_tbl[4] = {1, 3, 5, 7};
> +     int scale_needed, scale, minscale = INT_MAX;
> +     int i, j;
> +     u32 remainder;
> +
> +     scale_needed = div_u64_rem((u64)delay_ns * clkrate, NSEC_PER_SEC,
> +                     &remainder);
> +     if (remainder)
> +             scale_needed++;
> +
> +     for (i = 0; i < ARRAY_SIZE(pscale_tbl); i++)
> +             for (j = 0; j <= SPI_CTAR_SCALE_BITS; j++) {
> +                     scale = pscale_tbl[i] * (2 << j);
> +                     if (scale >= scale_needed) {
> +                             if (scale < minscale) {
> +                                     minscale = scale;
> +                                     *psc = i;
> +                                     *sc = j;
> +                             }
> +                             break;
> +                     }
> +             }
> +
> +     if (minscale == INT_MAX) {
> +             pr_warn("Cannot find correct scale values for %dns delay at 
> clkrate
> %ld, using max prescaler value",
> +                     delay_ns, clkrate);
> +             *psc = ARRAY_SIZE(pscale_tbl) - 1;
> +             *sc = SPI_CTAR_SCALE_BITS;
> +     }
> +}
> +
>  static int dspi_transfer_write(struct fsl_dspi *dspi)
>  {
>       int tx_count = 0;
> @@ -352,7 +390,8 @@ static int dspi_setup(struct spi_device *spi)
>  {
>       struct chip_data *chip;
>       struct fsl_dspi *dspi = spi_master_get_devdata(spi->master);
> -     unsigned char br = 0, pbr = 0, fmsz = 0;
> +     unsigned char br = 0, pbr = 0, pcssck = 0, cssck = 0;
> +     unsigned char pasc = 0, asc = 0, fmsz = 0;
>  
>       if ((spi->bits_per_word >= 4) && (spi->bits_per_word <= 16)) {
>               fmsz = spi->bits_per_word - 1;
> @@ -377,10 +416,20 @@ static int dspi_setup(struct spi_device *spi)
>       hz_to_spi_baud(&pbr, &br,
>                       spi->max_speed_hz, clk_get_rate(dspi->clk));
>  
> +     /* Set PCS to SCK delay scale values for 50ns delay*/
> +     ns_delay_scale(&pcssck, &cssck, 50, clk_get_rate(dspi->clk));
> +
> +     /* Set After SCK delay scale values for 50ns delay*/
> +     ns_delay_scale(&pasc, &asc, 50, clk_get_rate(dspi->clk));
> +

This functions calls clk_get_rate three times, can you add a local
variable for it?

>       chip->ctar_val =  SPI_CTAR_FMSZ(fmsz)
>               | SPI_CTAR_CPOL(spi->mode & SPI_CPOL ? 1 : 0)
>               | SPI_CTAR_CPHA(spi->mode & SPI_CPHA ? 1 : 0)
>               | SPI_CTAR_LSBFE(spi->mode & SPI_LSB_FIRST ? 1 : 0)
> +             | SPI_CTAR_PCSSCK(pcssck)
> +             | SPI_CTAR_CSSCK(cssck)
> +             | SPI_CTAR_PASC(pasc)
> +             | SPI_CTAR_ASC(asc)
>               | SPI_CTAR_PBR(pbr)
>               | SPI_CTAR_BR(br);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to