Some of slow SPI devices can not handle 32 bits transfers or need
delay between words/transfers.
Series is tested with ATSAMD20J15 slave device which is running @8Mhz.
Limiting bits per word to 16 bits and adding delay between transfers,
gives slave device enough extra time to process reply.
Oskari Lemmela (2):
spi: ar934x: fix transfer size
spi: ar934x: fix transfer and word delays
drivers/spi/spi-ar934x.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
--
2.25.1
If bits_per_word is configured, transfer only word amount
of data per iteration.
Signed-off-by: Oskari Lemmela <[email protected]>
---
drivers/spi/spi-ar934x.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/spi/spi-ar934x.c b/drivers/spi/spi-ar934x.c
index def32e0aaefe..a2cf37aca234 100644
--- a/drivers/spi/spi-ar934x.c
+++ b/drivers/spi/spi-ar934x.c
@@ -82,7 +82,7 @@ static int ar934x_spi_transfer_one_message(struct spi_controller *master,
struct spi_device *spi = m->spi;
unsigned long trx_done, trx_cur;
int stat = 0;
- u8 term = 0;
+ u8 bpw, term = 0;
int div, i;
u32 reg;
const u8 *tx_buf;
@@ -90,6 +90,11 @@ static int ar934x_spi_transfer_one_message(struct spi_controller *master,
m->actual_length = 0;
list_for_each_entry(t, &m->transfers, transfer_list) {
+ if (t->bits_per_word >= 8 && t->bits_per_word < 32)
+ bpw = t->bits_per_word >> 3;
+ else
+ bpw = 4;
+
if (t->speed_hz)
div = ar934x_spi_clk_div(sp, t->speed_hz);
else
@@ -105,10 +110,10 @@ static int ar934x_spi_transfer_one_message(struct spi_controller *master,
iowrite32(reg, sp->base + AR934X_SPI_REG_CTRL);
iowrite32(0, sp->base + AR934X_SPI_DATAOUT);
- for (trx_done = 0; trx_done < t->len; trx_done += 4) {
+ for (trx_done = 0; trx_done < t->len; trx_done += bpw) {
trx_cur = t->len - trx_done;
- if (trx_cur > 4)
- trx_cur = 4;
+ if (trx_cur > bpw)
+ trx_cur = bpw;
else if (list_is_last(&t->transfer_list, &m->transfers))
term = 1;
@@ -191,7 +196,8 @@ static int ar934x_spi_probe(struct platform_device *pdev)
ctlr->mode_bits = SPI_LSB_FIRST;
ctlr->setup = ar934x_spi_setup;
ctlr->transfer_one_message = ar934x_spi_transfer_one_message;
- ctlr->bits_per_word_mask = SPI_BPW_MASK(8);
+ ctlr->bits_per_word_mask = SPI_BPW_MASK(32) | SPI_BPW_MASK(24) |
+ SPI_BPW_MASK(16) | SPI_BPW_MASK(8);
ctlr->dev.of_node = pdev->dev.of_node;
ctlr->num_chipselect = 3;
--
2.25.1
Add missing delay between transferred messages and words.
Signed-off-by: Oskari Lemmela <[email protected]>
---
drivers/spi/spi-ar934x.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/spi/spi-ar934x.c b/drivers/spi/spi-ar934x.c
index a2cf37aca234..ec7250c4c810 100644
--- a/drivers/spi/spi-ar934x.c
+++ b/drivers/spi/spi-ar934x.c
@@ -142,8 +142,10 @@ static int ar934x_spi_transfer_one_message(struct spi_controller *master,
reg >>= 8;
}
}
+ spi_delay_exec(&t->word_delay, t);
}
m->actual_length += t->len;
+ spi_transfer_delay_exec(t);
}
msg_done:
--
2.25.1
On Wed, Dec 22, 2021 at 07:59:57AM +0200, Oskari Lemmela wrote:
> If bits_per_word is configured, transfer only word amount
> of data per iteration.
Does this actually materially affect what the hardware does? How much
data is transferred in an internal loop in the driver is completely
immaterial, bits per word only matters for formatting of the transferred
data.
On Wed, 22 Dec 2021 07:59:56 +0200, Oskari Lemmela wrote:
> Some of slow SPI devices can not handle 32 bits transfers or need
> delay between words/transfers.
>
> Series is tested with ATSAMD20J15 slave device which is running @8Mhz.
> Limiting bits per word to 16 bits and adding delay between transfers,
> gives slave device enough extra time to process reply.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Thanks!
[2/2] spi: ar934x: fix transfer and word delays
commit: c70282457c380db7deb57c81a6894debc8f88efa
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
On 22.12.2021 14.32, Mark Brown wrote:
> On Wed, Dec 22, 2021 at 07:59:57AM +0200, Oskari Lemmela wrote:
>> If bits_per_word is configured, transfer only word amount
>> of data per iteration.
> Does this actually materially affect what the hardware does? How much
> data is transferred in an internal loop in the driver is completely
> immaterial, bits per word only matters for formatting of the transferred
> data.
I don't have logic analyzator to verify what hardware actual does.
I tested this with transferring 32bits to ATSAMD20J15 slave.
Running loop in 8bits or 16bits, transfer is done correctly without
any errors. When running loop in 24bits or 32bits directly I got
error from spi_sync_transfer.
Oskari
On Wed, Dec 22, 2021 at 04:27:36PM +0200, Oskari Lemmel? wrote:
> On 22.12.2021 14.32, Mark Brown wrote:
> > Does this actually materially affect what the hardware does? How much
> > data is transferred in an internal loop in the driver is completely
> > immaterial, bits per word only matters for formatting of the transferred
> > data.
> I don't have logic analyzator to verify what hardware actual does.
> I tested this with transferring 32bits to ATSAMD20J15 slave.
> Running loop in 8bits or 16bits, transfer is done correctly without
> any errors. When running loop in 24bits or 32bits directly I got
> error from spi_sync_transfer.
This doesn't inspire confidence TBH. Given the lack of any change in
the interaction with the hardware it doesn't seem likely that the word
length is being changed at any point. Possibly there's a bug somewhere
that needs fixing but it's been misdiagnosed.
Note also that the commit log is not good here, now I look at the code
the driver only supports 8 bits per word at the minute and the change
adds support for higher word lengths. If you are seeing an issue that
might point towards what it is.
On 22.12.2021 16.59, Mark Brown wrote:
> On Wed, Dec 22, 2021 at 04:27:36PM +0200, Oskari Lemmel? wrote:
>> On 22.12.2021 14.32, Mark Brown wrote:
>>> Does this actually materially affect what the hardware does? How much
>>> data is transferred in an internal loop in the driver is completely
>>> immaterial, bits per word only matters for formatting of the transferred
>>> data.
>> I don't have logic analyzator to verify what hardware actual does.
>> I tested this with transferring 32bits to ATSAMD20J15 slave.
>> Running loop in 8bits or 16bits, transfer is done correctly without
>> any errors. When running loop in 24bits or 32bits directly I got
>> error from spi_sync_transfer.
> This doesn't inspire confidence TBH. Given the lack of any change in
> the interaction with the hardware it doesn't seem likely that the word
> length is being changed at any point. Possibly there's a bug somewhere
> that needs fixing but it's been misdiagnosed.
I did find datasheet for AR9344 and hardware supports shifting bits.
8.25.6 SPI Content to Shift Out or In (SPI_SHIFT_CNT_ADDR)
Address: 0x1FFF0014
Access: Read/Write
Reset: 0x0
-------------------------------------------
Bit? | Bit Name???? | Desc
31?? | SHIFT_EN???? | Enables shifting data out
30?? | SHIFT_CHNL?? | If set to 1, enables chip select 2
29?? | ? ? ? ? ???? | If set to 1, enables chip select 1
28?? | ? ? ? ? ???? | If set to 1, enables chip select 0
27?? | SHIFT_CLKOUT | Initial value of the clock signal
26?? | TERMINATE??? | When set to 1, deassert the chip select
25:7 | RES????????? | Reserved
6:0? | SHIFT_COUNT? | The number of bits to be shifted out or shifted in
on the data line
This is currently implemented in defines
#define AR934X_SPI_REG_SHIFT_CTRL?????? 0x14
#define AR934X_SPI_SHIFT_EN???????????? BIT(31)
#define AR934X_SPI_SHIFT_CS(n)????????? BIT(28 + (n))
#define AR934X_SPI_SHIFT_TERM?????????? 26
#define AR934X_SPI_SHIFT_VAL(cs, term, count)?????????????????? \
??????? (AR934X_SPI_SHIFT_EN | AR934X_SPI_SHIFT_CS(cs) |??????? \
??????? (term) << AR934X_SPI_SHIFT_TERM | (count))
In the transfer loop count value is set to number of bits per word.
reg = AR934X_SPI_SHIFT_VAL(spi->chip_select, term, trx_cur * 8);
iowrite32(reg, sp->base + AR934X_SPI_REG_SHIFT_CTRL);
So actually hardware support any word size between 1-32bits.
> Note also that the commit log is not good here, now I look at the code
> the driver only supports 8 bits per word at the minute and the change
> adds support for higher word lengths. If you are seeing an issue that
> might point towards what it is.
Should I split this in two commits? First one fixing SPI_BPW_MASK(32) typo.
Then second commit which implements 8bits, 16bits and 24bits word sizes?
Or should driver implement support for any word size between 1-32bits?
Oskari
On Wed, 22 Dec 2021 07:59:56 +0200, Oskari Lemmela wrote:
> Some of slow SPI devices can not handle 32 bits transfers or need
> delay between words/transfers.
>
> Series is tested with ATSAMD20J15 slave device which is running @8Mhz.
> Limiting bits per word to 16 bits and adding delay between transfers,
> gives slave device enough extra time to process reply.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Thanks!
[1/2] spi: ar934x: fix transfer size
commit: ebe33e5a98dcf14a9630845f3f10c193584ac054
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark