From: dillon min <[email protected]>
in l3gd20 driver startup, there is a setup failed error return from
stm32 spi driver
"
[ 2.687630] st-gyro-spi spi0.0: supply vdd not found, using dummy
regulator
[ 2.696869] st-gyro-spi spi0.0: supply vddio not found, using dummy
regulator
[ 2.706707] spi_stm32 40015000.spi: SPI transfer setup failed
[ 2.713741] st-gyro-spi spi0.0: SPI transfer failed: -22
[ 2.721096] spi_master spi0: failed to transfer one message from queue
[ 2.729268] iio iio:device0: failed to read Who-Am-I register.
[ 2.737504] st-gyro-spi: probe of spi0.0 failed with error -22
"
after debug into spi-stm32 driver, st-gyro-spi split two steps to read
l3gd20 id
first: send command to l3gd20 with read id command in tx_buf, rx_buf
is null.
second: read id with tx_buf is null, rx_buf not null.
so, for second step, stm32 driver recongise this process is 'SPI_SIMPLE_RX'
from stm32_spi_communication_type(), but there is no related process for this
type in stm32f4_spi_set_mode(), then we get error from
stm32_spi_transfer_one_setup().
we can use two method to fix this bug.
1, use stm32 spi's "In unidirectional receive-only mode (BIDIMODE=0 and
RXONLY=1)". but as our code running in sdram, the read latency is too large
to get so many receive overrun error in interrupts handler.
2, use stm32 spi's "In full-duplex (BIDIMODE=0 and RXONLY=0)", as tx_buf is
null, we must add dummy data sent out before read data.
so, add stm32f4_spi_tx_dummy() to handle this situation.
Signed-off-by: dillon min <[email protected]>
---
drivers/spi/spi-stm32.c | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 44ac6eb3..72d9387 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -388,6 +388,14 @@ static int stm32h7_spi_get_fifo_size(struct stm32_spi *spi)
return count;
}
+static void stm32f4_spi_tx_dummy(struct stm32_spi *spi)
+{
+ if (spi->cur_bpw == 16)
+ writew_relaxed(0x5555, spi->base + STM32F4_SPI_DR);
+ else
+ writeb_relaxed(0x55, spi->base + STM32F4_SPI_DR);
+}
+
/**
* stm32f4_spi_get_bpw_mask - Return bits per word mask
* @spi: pointer to the spi controller data structure
@@ -811,7 +819,9 @@ static irqreturn_t stm32f4_spi_irq_event(int irq, void *dev_id)
mask |= STM32F4_SPI_SR_TXE;
}
- if (!spi->cur_usedma && spi->cur_comm == SPI_FULL_DUPLEX) {
+ if (!spi->cur_usedma && (spi->cur_comm == SPI_FULL_DUPLEX ||
+ spi->cur_comm == SPI_SIMPLEX_RX ||
+ spi->cur_comm == SPI_3WIRE_RX)) {
/* TXE flag is set and is handled when RXNE flag occurs */
sr &= ~STM32F4_SPI_SR_TXE;
mask |= STM32F4_SPI_SR_RXNE | STM32F4_SPI_SR_OVR;
@@ -850,8 +860,10 @@ static irqreturn_t stm32f4_spi_irq_event(int irq, void *dev_id)
stm32f4_spi_read_rx(spi);
if (spi->rx_len == 0)
end = true;
- else /* Load data for discontinuous mode */
+ else if (spi->tx_buf)/* Load data for discontinuous mode */
stm32f4_spi_write_tx(spi);
+ else if (spi->cur_comm == SPI_SIMPLEX_RX)
+ stm32f4_spi_tx_dummy(spi);
}
end_irq:
@@ -1151,7 +1163,9 @@ static int stm32f4_spi_transfer_one_irq(struct stm32_spi *spi)
/* Enable the interrupts relative to the current communication mode */
if (spi->cur_comm == SPI_SIMPLEX_TX || spi->cur_comm == SPI_3WIRE_TX) {
cr2 |= STM32F4_SPI_CR2_TXEIE;
- } else if (spi->cur_comm == SPI_FULL_DUPLEX) {
+ } else if (spi->cur_comm == SPI_FULL_DUPLEX ||
+ spi->cur_comm == SPI_SIMPLEX_RX ||
+ spi->cur_comm == SPI_3WIRE_RX) {
/* In transmit-only mode, the OVR flag is set in the SR register
* since the received data are never read. Therefore set OVR
* interrupt only when rx buffer is available.
@@ -1170,6 +1184,8 @@ static int stm32f4_spi_transfer_one_irq(struct stm32_spi *spi)
/* starting data transfer when buffer is loaded */
if (spi->tx_buf)
stm32f4_spi_write_tx(spi);
+ else if (spi->cur_comm == SPI_SIMPLEX_RX)
+ stm32f4_spi_tx_dummy(spi);
spin_unlock_irqrestore(&spi->lock, flags);
@@ -1462,10 +1478,16 @@ static int stm32f4_spi_set_mode(struct stm32_spi *spi, unsigned int comm_type)
stm32_spi_set_bits(spi, STM32F4_SPI_CR1,
STM32F4_SPI_CR1_BIDIMODE |
STM32F4_SPI_CR1_BIDIOE);
- } else if (comm_type == SPI_FULL_DUPLEX) {
+ } else if (comm_type == SPI_FULL_DUPLEX ||
+ comm_type == SPI_SIMPLEX_RX) {
stm32_spi_clr_bits(spi, STM32F4_SPI_CR1,
STM32F4_SPI_CR1_BIDIMODE |
STM32F4_SPI_CR1_BIDIOE);
+ } else if (comm_type == SPI_3WIRE_RX) {
+ stm32_spi_set_bits(spi, STM32F4_SPI_CR1,
+ STM32F4_SPI_CR1_BIDIMODE);
+ stm32_spi_clr_bits(spi, STM32F4_SPI_CR1,
+ STM32F4_SPI_CR1_BIDIOE);
} else {
return -EINVAL;
}
--
2.7.4
On Mon, May 18, 2020 at 07:09:20PM +0800, [email protected] wrote:
> 2, use stm32 spi's "In full-duplex (BIDIMODE=0 and RXONLY=0)", as tx_buf is
> null, we must add dummy data sent out before read data.
> so, add stm32f4_spi_tx_dummy() to handle this situation.
There are flags SPI_CONTROLLER_MUST_TX and SPI_CONTROLLER_MUST_RX flags
that the driver can set if it needs to, no need to open code this in the
driver.
hi Mark,
Thanks for reviewing.
On Fri, May 22, 2020 at 7:36 PM Mark Brown <[email protected]> wrote:
>
> On Mon, May 18, 2020 at 07:09:20PM +0800, [email protected] wrote:
>
> > 2, use stm32 spi's "In full-duplex (BIDIMODE=0 and RXONLY=0)", as tx_buf is
> > null, we must add dummy data sent out before read data.
> > so, add stm32f4_spi_tx_dummy() to handle this situation.
>
> There are flags SPI_CONTROLLER_MUST_TX and SPI_CONTROLLER_MUST_RX flags
> that the driver can set if it needs to, no need to open code this in the
> driver.
Yes, after check SPI_CONTROLLER_MUST_TX in drivers/spi/spi.c , it's
indeed to meet
this situation, i will try it and sumbmit a new patch.
thanks.
best regards
Dillon
On Fri, May 22, 2020 at 10:57 PM dillon min <[email protected]> wrote:
>
> hi Mark,
>
> Thanks for reviewing.
>
> On Fri, May 22, 2020 at 7:36 PM Mark Brown <[email protected]> wrote:
> >
> > On Mon, May 18, 2020 at 07:09:20PM +0800, [email protected] wrote:
> >
> > > 2, use stm32 spi's "In full-duplex (BIDIMODE=0 and RXONLY=0)", as tx_buf is
> > > null, we must add dummy data sent out before read data.
> > > so, add stm32f4_spi_tx_dummy() to handle this situation.
> >
> > There are flags SPI_CONTROLLER_MUST_TX and SPI_CONTROLLER_MUST_RX flags
> > that the driver can set if it needs to, no need to open code this in the
> > driver.
>
> Yes, after check SPI_CONTROLLER_MUST_TX in drivers/spi/spi.c , it's
> indeed to meet
> this situation, i will try it and sumbmit a new patch.
>
> thanks.
>
> best regards
>
> Dillon
Hi Mark,
There might be a conflict with 'SPI_CONTROLLER_MUST_TX' and 'SPI_3WIRE' mode,
i need to know the SPI_3WIRE direction, currently i get this
information from 'struct spi_device'
and 'struct spi_transfer'
if ((spi_device->mode & SPI_3WIRE) && (spi_transfer->tx_buf == NULL)
&& (spi_transfer->rx_buf != NULL))
this is a SPI_3WIRE_RX transfer
if ((spi_device->mode & SPI_3WIRE) && (spi_transfer->tx_buf != NULL)
&& (spi_transfer->rx_buf == NULL))
this is a SPI_3WIRE_TX transfer
but, after spi-core create a dummy tx_buf or rx_buf, then i can't get
the correct spi_3wire direction.
actually, this dummy tx_buf is useless for SPI_3WIRE. it's has meaning
for SPI_SIMPLE_RX mode,
simulate SPI_FULL_DUMPLEX
how do you think?
thanks
best regards
Dillon
On Fri, May 22, 2020 at 11:59:25PM +0800, dillon min wrote:
> but, after spi-core create a dummy tx_buf or rx_buf, then i can't get
> the correct spi_3wire direction.
> actually, this dummy tx_buf is useless for SPI_3WIRE. it's has meaning
> for SPI_SIMPLE_RX mode,
> simulate SPI_FULL_DUMPLEX
Oh, that's annoying. I think the fix here is in the core, it should
ignore MUST_TX and MUST_RX in 3WIRE mode since they clearly make no
sense there.
On Sat, May 23, 2020 at 12:29 AM Mark Brown <[email protected]> wrote:
>
> On Fri, May 22, 2020 at 11:59:25PM +0800, dillon min wrote:
>
> > but, after spi-core create a dummy tx_buf or rx_buf, then i can't get
> > the correct spi_3wire direction.
> > actually, this dummy tx_buf is useless for SPI_3WIRE. it's has meaning
> > for SPI_SIMPLE_RX mode,
> > simulate SPI_FULL_DUMPLEX
>
> Oh, that's annoying. I think the fix here is in the core, it should
> ignore MUST_TX and MUST_RX in 3WIRE mode since they clearly make no
> sense there.
How about add below changes to spi-core
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 8994545..bfd465c 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1022,7 +1022,8 @@ static int spi_map_msg(struct spi_controller
*ctlr, struct spi_message *msg)
void *tmp;
unsigned int max_tx, max_rx;
- if (ctlr->flags & (SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX)) {
+ if ((ctlr->flags & (SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX)) &&
+ !(msg->spi->mode & SPI_3WIRE)) {
max_tx = 0;
max_rx = 0;
for my board, lcd panel ilitek ill9341 use 3wire mode, gyro l3gd20 use
simplex rx mode.
it's has benefits to l3gd20, no impact to ili9341.
if it's fine to spi-core, i will include it to my next submits.
thanks
best regards.
Dillon
On Sat, May 23, 2020 at 09:35:06AM +0800, dillon min wrote:
> - if (ctlr->flags & (SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX)) {
> + if ((ctlr->flags & (SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX)) &&
> + !(msg->spi->mode & SPI_3WIRE)) {
> max_tx = 0;
> max_rx = 0;
> for my board, lcd panel ilitek ill9341 use 3wire mode, gyro l3gd20 use
> simplex rx mode.
> it's has benefits to l3gd20, no impact to ili9341.
> if it's fine to spi-core, i will include it to my next submits.
Yes, looks reasonable.