On 12/17/18 8:37 AM, Stefan Mavrodiev wrote:
On 12/14/18 5:55 PM, Maxime Ripard wrote:
On Fri, Dec 14, 2018 at 04:14:31PM +0200, Stefan Mavrodiev wrote:
On 12/14/18 11:25 AM, Maxime Ripard wrote:
On Thu, Dec 13, 2018 at 09:12:57AM +0200, Stefan Mavrodiev wrote:
On 12/6/18 8:41 AM, Stefan Mavrodiev wrote:
On 12/5/18 5:46 PM, Maxime Ripard wrote:
On Wed, Dec 05, 2018 at 02:27:57PM +0200, Stefan Mavrodiev wrote:
Current driver doesn't check if the destination pointer is NULL.
This cause the data from the FIFO to be stored inside the internal
SDRAM ( address 0 ).
The patch add simple check if the destination pointer is NULL.
Signed-off-by: Stefan Mavrodiev <ste...@olimex.com>
---
drivers/spi/sun4i_spi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/sun4i_spi.c b/drivers/spi/sun4i_spi.c
index b86b5a00ad..38cc743c61 100644
--- a/drivers/spi/sun4i_spi.c
+++ b/drivers/spi/sun4i_spi.c
@@ -129,7 +129,8 @@ static inline void
sun4i_spi_drain_fifo(struct sun4i_spi_priv *priv, int len)
while (len--) {
byte = readb(&priv->regs->rxdata);
- *priv->rx_buf++ = byte;
+ if (priv->rx_buf)
+ *priv->rx_buf++ = byte;
It seems pretty inefficient to test the pointer at each access, it
would be better to check it once before starting the transfer.
I'm not sure if that can even happen?
I've tried to check that before draining the receive fifo, but
then the controller doesn't work. I'm thinking that the fifo must
be drained in any case.
Any further comments?
I was expecting you to comment on whether the FIFO needed to be
drained or not :)
Sorry. I didn't understand that.
Anyway. After some code checking, I found that the FIFO needs to be
drained
because TP_EN (Transmit Pause Enable) bit is set during bus claim.
"....
In master mode, it is used to control transmit state machine to
stop smart burst sending when RX FIFO is full.
..."
Perhaps this bit should be enabled only when we want to read back data?
It's been a while since I last looked at the spi driver. What is linux
doing?
In the kernel version there is the same check, like the one in the patch:
drivers/spi/spi-sun4i.c:
static inline void sun4i_spi_drain_fifo(struct sun4i_spi *sspi, int len)
......
while (len--) {
byte = readb(sspi->base_addr + SUN4I_RXDATA_REG);
if (sspi->rx_buf)
*sspi->rx_buf++ = byte;
}
.....
Guess I've missed this check when I was adopting the driver for u-boot.
Maxime
Any comments?
I think with all said, the patch can be merged as it is.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
