TLDR;
It seems as though the spi-tegra114.c has an issue, patch at the bottom vvv
Longer Version
While working through an IIO driver that uses SPI I found that the CS line did not go high for very long between transactions. I needed the CS to go high for16us but it only went high for 0.5uS
DTS
I tried modifying the values within the DTS including the following:
/ {
spi@3230000{ /* SPI3 in 40 pin conn */
status = "okay";
spi-max-frequency = <50000000>;
/delete-node/ spi@1;
spi@0 {
compatible = "adi,adis16507-3";
reg = <0x0>;
spi-cpha;
spi-cpol;
spi-max-frequency = <2000000>; // 2MHz
interrupt-parent = <&tegra_main_gpio>;
interrupts = <TEGRA194_MAIN_GPIO(Q, 5) IRQ_TYPE_EDGE_RISING>;
reset-gpios = <&tegra_aon_gpio TEGRA194_AON_GPIO(CC, 0) GPIO_ACTIVE_LOW>;
adi,sync-mode = <0x00>;
controller-data {
nvidia,enable-hw-based-cs;
nvidia,rx-clk-tap-delay = <0x10>;
nvidia,tx-clk-tap-delay = <0x0>;
//nvidia,cs-inactive-cycles = <0x08>;
nvidia,cs-inactive-cycles = <0x10>;
nvidia,cs-setup-clk-count = <0x10>;
nvidia,cs-hold-clk-count = <0x10>;
};
};
};
But this had no effect. I then looked at the sp-tegrai114.c and found this function called after a transaction.
SPI Driver
tegra_spi_transfer_delay(xfer->delay_usecs);
This seems to control how long the chip select stays high between transactions. When I added some print statements it kept reporting the ‘delay_usecs == 0’
There are two issues with this.
Issue 1
within the struct spi_transfer there is a member called delay_usecs but there is also a sub structure called struct spi_delay the user can specify either the delay_usecs or the struct spi_delay with a delay and it’s up to the driver to figure out which one to use. A generic call within spi.h handles it.
static inline int
spi_transfer_delay_exec(struct spi_transfer *t)
{
struct spi_delay d;
if (t->delay_usecs) {
d.value = t->delay_usecs;
d.unit = SPI_DELAY_UNIT_USECS;
return spi_delay_exec(&d, NULL);
}
return spi_delay_exec(&t->delay, t);
}
Issue 2
delay or delay_usecs doesn’t specify the delay between transactions, which is determined by the cs_change_delay member.
In order to specify the length of time the CS is high between packets the following should be changed:
Drver Patch for spi-tegra114.c
index 26aa7cb29cbc..8911da4ddab5 100644
--- a/drivers/spi/spi-tegra114.c
+++ b/drivers/spi/spi-tegra114.c
@@ -1366,17 +1366,6 @@ static void tegra_spi_dump_regs(struct tegra_spi_data *tspi)
tegra_spi_readl(tspi, SPI_FIFO_STATUS));
}
-static void tegra_spi_transfer_delay(int delay)
-{
- if (!delay)
- return;
-
- if (delay >= 1000)
- mdelay(delay / 1000);
-
- udelay(delay % 1000);
-}
-
static int tegra_spi_cs_low(struct spi_device *spi, bool state)
{
struct tegra_spi_data *tspi = spi_controller_get_devdata(spi->master);
@@ -1504,7 +1493,7 @@ static int tegra_spi_transfer_one_message(struct spi_controller *ctrl,
if (cstate && cstate->cs_gpio_valid)
gpio_set_value(spi->cs_gpio, gval);
tegra_spi_writel(tspi, cmd1, SPI_COMMAND1);
- tegra_spi_transfer_delay(xfer->delay_usecs);
+ spi_delay_exec(&xfer->cs_change_delay, xfer);
goto exit;
} else if (list_is_last(&xfer->transfer_list,
&msg->transfers)) {
@@ -1514,13 +1503,13 @@ static int tegra_spi_transfer_one_message(struct spi_controller *ctrl,
if (cstate && cstate->cs_gpio_valid)
gpio_set_value(spi->cs_gpio, gval);
tegra_spi_writel(tspi, cmd1, SPI_COMMAND1);
- tegra_spi_transfer_delay(xfer->delay_usecs);
+ spi_delay_exec(&xfer->cs_change_delay, xfer);
}
} else if (xfer->cs_change) {
if (cstate && cstate->cs_gpio_valid)
gpio_set_value(spi->cs_gpio, gval);
tegra_spi_writel(tspi, cmd1, SPI_COMMAND1);
- tegra_spi_transfer_delay(xfer->delay_usecs);
+ spi_delay_exec(&xfer->cs_change_delay, xfer);
}
}
Here is the resulting SPI Transaction; the time isn’t perfect, it’s actually a lot longer than I wanted, perhaps I can bring back the tegra_spi_transfer_delay function with a fix to account for the spi_delay struct as well as Using the ‘udelay’ function instead of the usleep_range might lead to more consistent and accurate timing.
At the moment this seems to be working but if there is anything I am missing please let me know.
Thanks for any feedback,
Dave
