* [PATCH v2 0/2] spi: fsl-dspi (vf610) clock fixes
@ 2015-03-30 17:49 Aaron Brice
2015-03-30 17:49 ` Aaron Brice
2015-03-30 17:49 ` [PATCH v2 2/2] spi: fsl-dspi: Add ~50ns delay between cs and sck Aaron Brice
0 siblings, 2 replies; 9+ messages in thread
From: Aaron Brice @ 2015-03-30 17:49 UTC (permalink / raw)
To: broonie; +Cc: linux-spi, linux-kernel, stefan
We were having intermittent problems writing to SRAM chip on SPI bus on vf610
SoM. Added support for CS setup and hold times to meet the SRAM spec. In the
process noticed that the baud rate was a little high.
Changes since v1:
* More detail in commit message for clock rate fix
Aaron Brice (2):
spi: fsl-dspi: Fix clock rate scale values
spi: fsl-dspi: Add ~50ns delay between cs and sck
drivers/spi/spi-fsl-dspi.c | 84 ++++++++++++++++++++++++++++++++++++++--------
1 file changed, 70 insertions(+), 14 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] spi: fsl-dspi: Fix clock rate scale values
@ 2015-03-30 17:49 ` Aaron Brice
0 siblings, 0 replies; 9+ messages in thread
From: Aaron Brice @ 2015-03-30 17:49 UTC (permalink / raw)
To: broonie; +Cc: linux-spi, linux-kernel, stefan
Previous algorithm had an outer loop with the values {2,3,5,7} and an
inner loop with {2,4,6,8,16,32,...,32768}, and would pick the first
value over the required scaling value (where the total scale was the two
numbers multiplied).
Since the inner loop went up to 32768 it would always pick a value of 2
for PBR and a much higher than necessary value for BR. The desired
scale factor was being divided by two I believe to compensate for the
much higher scale factors (the divide by two not specified in the
reference manual).
Updated to check all values and find the smallest scale factor possible
without going over the desired clock rate.
Signed-off-by: Aaron Brice <aaron.brice@datasoft.com>
---
drivers/spi/spi-fsl-dspi.c | 35 +++++++++++++++++++++--------------
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index d1a3924..96cac87 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -148,23 +148,30 @@ static void hz_to_spi_baud(char *pbr, char *br, int speed_hz,
16, 32, 64, 128,
256, 512, 1024, 2048,
4096, 8192, 16384, 32768 };
- int temp, i = 0, j = 0;
-
- temp = clkrate / 2 / speed_hz;
-
- for (i = 0; i < ARRAY_SIZE(pbr_tbl); i++)
- for (j = 0; j < ARRAY_SIZE(brs); j++) {
- if (pbr_tbl[i] * brs[j] >= temp) {
- *pbr = i;
- *br = j;
- return;
+ int scale_needed, scale, minscale = INT_MAX;
+ int i, j;
+
+ scale_needed = clkrate / speed_hz;
+
+ for (i = 0; i < ARRAY_SIZE(brs); i++)
+ for (j = 0; j < ARRAY_SIZE(pbr_tbl); j++) {
+ scale = brs[i] * pbr_tbl[j];
+ if (scale >= scale_needed) {
+ if (scale < minscale) {
+ minscale = scale;
+ *br = i;
+ *pbr = j;
+ }
+ break;
}
}
- pr_warn("Can not find valid baud rate,speed_hz is %d,clkrate is %ld\
- ,we use the max prescaler value.\n", speed_hz, clkrate);
- *pbr = ARRAY_SIZE(pbr_tbl) - 1;
- *br = ARRAY_SIZE(brs) - 1;
+ if (minscale == INT_MAX) {
+ pr_warn("Can not find valid baud rate,speed_hz is %d,clkrate is %ld, we use the max prescaler value.\n",
+ speed_hz, clkrate);
+ *pbr = ARRAY_SIZE(pbr_tbl) - 1;
+ *br = ARRAY_SIZE(brs) - 1;
+ }
}
static int dspi_transfer_write(struct fsl_dspi *dspi)
--
2.1.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] spi: fsl-dspi: Fix clock rate scale values
@ 2015-03-30 17:49 ` Aaron Brice
0 siblings, 0 replies; 9+ messages in thread
From: Aaron Brice @ 2015-03-30 17:49 UTC (permalink / raw)
To: broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, stefan-XLVq0VzYD2Y
Previous algorithm had an outer loop with the values {2,3,5,7} and an
inner loop with {2,4,6,8,16,32,...,32768}, and would pick the first
value over the required scaling value (where the total scale was the two
numbers multiplied).
Since the inner loop went up to 32768 it would always pick a value of 2
for PBR and a much higher than necessary value for BR. The desired
scale factor was being divided by two I believe to compensate for the
much higher scale factors (the divide by two not specified in the
reference manual).
Updated to check all values and find the smallest scale factor possible
without going over the desired clock rate.
Signed-off-by: Aaron Brice <aaron.brice-vSOi0k5x9wFWk0Htik3J/w@public.gmane.org>
---
drivers/spi/spi-fsl-dspi.c | 35 +++++++++++++++++++++--------------
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index d1a3924..96cac87 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -148,23 +148,30 @@ static void hz_to_spi_baud(char *pbr, char *br, int speed_hz,
16, 32, 64, 128,
256, 512, 1024, 2048,
4096, 8192, 16384, 32768 };
- int temp, i = 0, j = 0;
-
- temp = clkrate / 2 / speed_hz;
-
- for (i = 0; i < ARRAY_SIZE(pbr_tbl); i++)
- for (j = 0; j < ARRAY_SIZE(brs); j++) {
- if (pbr_tbl[i] * brs[j] >= temp) {
- *pbr = i;
- *br = j;
- return;
+ int scale_needed, scale, minscale = INT_MAX;
+ int i, j;
+
+ scale_needed = clkrate / speed_hz;
+
+ for (i = 0; i < ARRAY_SIZE(brs); i++)
+ for (j = 0; j < ARRAY_SIZE(pbr_tbl); j++) {
+ scale = brs[i] * pbr_tbl[j];
+ if (scale >= scale_needed) {
+ if (scale < minscale) {
+ minscale = scale;
+ *br = i;
+ *pbr = j;
+ }
+ break;
}
}
- pr_warn("Can not find valid baud rate,speed_hz is %d,clkrate is %ld\
- ,we use the max prescaler value.\n", speed_hz, clkrate);
- *pbr = ARRAY_SIZE(pbr_tbl) - 1;
- *br = ARRAY_SIZE(brs) - 1;
+ if (minscale == INT_MAX) {
+ pr_warn("Can not find valid baud rate,speed_hz is %d,clkrate is %ld, we use the max prescaler value.\n",
+ speed_hz, clkrate);
+ *pbr = ARRAY_SIZE(pbr_tbl) - 1;
+ *br = ARRAY_SIZE(brs) - 1;
+ }
}
static int dspi_transfer_write(struct fsl_dspi *dspi)
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] spi: fsl-dspi: Add ~50ns delay between cs and sck
2015-03-30 17:49 [PATCH v2 0/2] spi: fsl-dspi (vf610) clock fixes Aaron Brice
2015-03-30 17:49 ` Aaron Brice
@ 2015-03-30 17:49 ` Aaron Brice
2015-03-30 22:13 ` Stefan Agner
1 sibling, 1 reply; 9+ messages in thread
From: Aaron Brice @ 2015-03-30 17:49 UTC (permalink / raw)
To: broonie; +Cc: linux-spi, linux-kernel, stefan
Add delay between chip select and clock signals, before clock starts and
after clock stops.
Signed-off-by: Aaron Brice <aaron.brice@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
+
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));
+
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);
--
2.1.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] spi: fsl-dspi: Add ~50ns delay between cs and sck
@ 2015-03-30 22:13 ` Stefan Agner
0 siblings, 0 replies; 9+ messages in thread
From: Stefan Agner @ 2015-03-30 22:13 UTC (permalink / raw)
To: Aaron Brice; +Cc: broonie, linux-spi, linux-kernel
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.brice@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);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] spi: fsl-dspi: Add ~50ns delay between cs and sck
@ 2015-03-30 22:13 ` Stefan Agner
0 siblings, 0 replies; 9+ messages in thread
From: Stefan Agner @ 2015-03-30 22:13 UTC (permalink / raw)
To: Aaron Brice
Cc: broonie-DgEjT+Ai2ygdnm+yROfE0A, linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
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.brice-vSOi0k5x9wFWk0Htik3J/w@public.gmane.org>
> ---
> 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-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] spi: fsl-dspi: Fix clock rate scale values
2015-03-30 17:49 ` Aaron Brice
(?)
@ 2015-03-31 11:13 ` Mark Brown
-1 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2015-03-31 11:13 UTC (permalink / raw)
To: Aaron Brice; +Cc: linux-spi, linux-kernel, stefan
[-- Attachment #1: Type: text/plain, Size: 319 bytes --]
On Mon, Mar 30, 2015 at 10:49:15AM -0700, Aaron Brice wrote:
> Previous algorithm had an outer loop with the values {2,3,5,7} and an
> inner loop with {2,4,6,8,16,32,...,32768}, and would pick the first
> value over the required scaling value (where the total scale was the two
> numbers multiplied).
Applied, thanks.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] spi: fsl-dspi: Add ~50ns delay between cs and sck
@ 2015-03-31 19:14 ` Aaron Brice
0 siblings, 0 replies; 9+ messages in thread
From: Aaron Brice @ 2015-03-31 19:14 UTC (permalink / raw)
To: Stefan Agner, broonie; +Cc: linux-spi, linux-kernel
On 03/30/2015 03:13 PM, Stefan Agner wrote:
> Hi Aaron,
>
> Thanks for the fixes! Some comments below:
>
Thanks for the review!
> 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
>
Thanks for the link. Those properties didn't make it into the kernel,
Mark said the chip select delays were covered by "the standard delays"
but I'm not sure what that means?
The minimum 50ns was specified in the SRAM datasheet, but I haven't
worked with a lot of SPI devices, I don't know if this varies widely or
if it's more or less standard?
I hard-coded this based on spi-bitbang having a hard-coded ndelay(100)
after chipselect, although there is a FIXME comment above it.. Should I
add this as a standard spi-bus slave node binding and update spi-bitbang
also, or just fsl-dspi?
>>
>> Signed-off-by: Aaron Brice <aaron.brice@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)
>
Ok.
>> 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?
>
Ok.
>> 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);
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] spi: fsl-dspi: Add ~50ns delay between cs and sck
@ 2015-03-31 19:14 ` Aaron Brice
0 siblings, 0 replies; 9+ messages in thread
From: Aaron Brice @ 2015-03-31 19:14 UTC (permalink / raw)
To: Stefan Agner, broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA
On 03/30/2015 03:13 PM, Stefan Agner wrote:
> Hi Aaron,
>
> Thanks for the fixes! Some comments below:
>
Thanks for the review!
> 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
>
Thanks for the link. Those properties didn't make it into the kernel,
Mark said the chip select delays were covered by "the standard delays"
but I'm not sure what that means?
The minimum 50ns was specified in the SRAM datasheet, but I haven't
worked with a lot of SPI devices, I don't know if this varies widely or
if it's more or less standard?
I hard-coded this based on spi-bitbang having a hard-coded ndelay(100)
after chipselect, although there is a FIXME comment above it.. Should I
add this as a standard spi-bus slave node binding and update spi-bitbang
also, or just fsl-dspi?
>>
>> Signed-off-by: Aaron Brice <aaron.brice-vSOi0k5x9wFWk0Htik3J/w@public.gmane.org>
>> ---
>> 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)
>
Ok.
>> 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?
>
Ok.
>> 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-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-03-31 19:21 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-30 17:49 [PATCH v2 0/2] spi: fsl-dspi (vf610) clock fixes Aaron Brice
2015-03-30 17:49 ` [PATCH v2 1/2] spi: fsl-dspi: Fix clock rate scale values Aaron Brice
2015-03-30 17:49 ` Aaron Brice
2015-03-31 11:13 ` Mark Brown
2015-03-30 17:49 ` [PATCH v2 2/2] spi: fsl-dspi: Add ~50ns delay between cs and sck Aaron Brice
2015-03-30 22:13 ` Stefan Agner
2015-03-30 22:13 ` Stefan Agner
2015-03-31 19:14 ` Aaron Brice
2015-03-31 19:14 ` Aaron Brice
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.