On 11/05/2014 03:57 AM, Andy Shevchenko wrote:
On Tue, 2014-11-04 at 16:36 -0600, tthayer@xxxxxxxxxxxxxxxxxxxxx wrote:
From: Thor Thayer <tthayer@xxxxxxxxxxxxxxxxxxxxx>
Changing SPI transfer speed using a utility such as spi_config with
spidev updates chip->speed_hz which is compared to the next transfer
speed.
When speed_hz is not declared in a SPI transfer, the transfer speed is
not updated for the next read/write on /dev/spidevX.Y. The element
spi_transfer->speed_hz is filled with spi->max_speed_hz. The test of
if (transfer->speed_hz != speed) doesn't work because the chip->speed_hz
matches transfer->speed_hz and the clock divider is not updated.
This fix: On each transfer update the clock divider, compare to the
previous clock divider and update if necessary. This fixes another
bug where the clock divider calculation at the top of the
pump_transfers() function could be an odd-number.
My intention is to use SPI core API as much as possible. Thus,
pump_transfers() I think should be gone in future. Instead of doing an
additional work can you provide a helper function to set speed_hz and
call it from pump_transfers()?
Hi. I see your point but this patch may change significantly as a result
of discussion. If some form of clock divider changes are implemented,
I'm happy to make a helper function.
Thanks for reviewing!
Reported-by: Vlastimil Setka <setka@xxxxxxx>
Signed-off-by: Vlastimil Setka <setka@xxxxxxx>
Signed-off-by: Thor Thayer <tthayer@xxxxxxxxxxxxxxxxxxxxx>
---
drivers/spi/spi-dw.c | 33 ++++++++++++++++++---------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index 72e12ba..3456b34 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -376,9 +376,6 @@ static void pump_transfers(unsigned long data)
chip = dws->cur_chip;
spi = message->spi;
- if (unlikely(!chip->clk_div))
- chip->clk_div = dws->max_freq / chip->speed_hz;
-
if (message->state == ERROR_STATE) {
message->status = -EIO;
goto early_exit;
@@ -415,21 +412,27 @@ static void pump_transfers(unsigned long data)
cr0 = chip->cr0;
- /* Handle per transfer options for bpw and speed */
- if (transfer->speed_hz) {
- speed = chip->speed_hz;
+ /* Always calculate the desired clock divider */
+ speed = transfer->speed_hz ? transfer->speed_hz : chip->speed_hz;
+
+ if (speed > dws->max_freq) {
+ dev_err(&spi->dev, "Unsupported SPI freq: %d Hz\n", speed);
+ message->status = -EIO;
+ goto early_exit;
+ }
+
+ /* clk_div doesn't support odd number */
+ clk_div = dws->max_freq / speed;
+ clk_div = (clk_div + 1) & 0xfffe;
- if (transfer->speed_hz != speed) {
- speed = transfer->speed_hz;
+ /* Determine if the clock divider changed, if so update chip struct */
Maybe "…update speed_hz" ?
+ if (clk_div != chip->clk_div)
+ chip->clk_div = clk_div;
+ else
+ clk_div = 0; /* Prevent register programming below */
- /* clk_div doesn't support odd number */
- clk_div = dws->max_freq / speed;
- clk_div = (clk_div + 1) & 0xfffe;
+ chip->speed_hz = speed;
- chip->speed_hz = speed;
- chip->clk_div = clk_div;
- }
- }
if (transfer->bits_per_word) {
bits = transfer->bits_per_word;
dws->n_bytes = dws->dma_width = bits >> 3;
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
