From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Mavrodiev Date: Thu, 20 Dec 2018 13:19:49 +0200 Subject: [U-Boot] [PATCH 1/1] arm: sunxi: Add NULL pointer check In-Reply-To: References: <20181205122757.14523-1-stefan@olimex.com> <18e25666-911a-8dc6-f8f5-223f1761eafa@olimex.com> Message-ID: <14480b84-da66-885b-38a0-6f414719ee01@olimex.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On 12/20/18 12:56 PM, Jagan Teki wrote: > On Thu, Dec 20, 2018 at 4:18 PM Stefan Mavrodiev wrote: >> >> On 12/20/18 12:45 PM, Jagan Teki wrote: >>> On Thu, Dec 20, 2018 at 3:59 PM Stefan Mavrodiev wrote: >>>> On 12/20/18 12:14 PM, Jagan Teki wrote: >>>>> On Fri, Dec 14, 2018 at 3:48 PM Jagan Teki wrote: >>>>>> On Wed, Dec 5, 2018 at 5:58 PM 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 >>>>>>> --- >>>>>>> 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; >>>>>> Acked-by: Jagan Teki >>>>>> >>>>>> But, have you tested how much data in the fifo before drained? It's >>>>>> better we can get the available data before reading via fifo_sta >>>> I don't understand what's the point of doing this? >>> Didn't get? don't you understand what I'm saying or it not require >>> from point of you? >> Maybe I don't understand correctly what you're saying. > You comment now and previous mail doesn't match. better be specific. > > For draining fifo. > We can find how much data available before reading from fifo and > assign to local rx. > > static inline void sun4i_spi_drain_fifo(struct sun4i_spi_priv *priv, int len) > { > u32 reg, cnt > u8 byte; > > /* See how much data is available */ > reg = readl(&priv->regs->fifo_sta); > reg &= SUN4I_FIFO_STA_RF_CNT_MASK; > cnt = reg >> SUN4I_FIFO_STA_RF_CNT_BITS; > > if (len > cnt) > len = cnt; > > while (len--) { > byte = readb(&priv->regs->rxdata); > *priv->rx_buf++ = byte; > } > } > > This can be perfect drain fifo, and this is what I'm trying to test > you with existing code and after your patch and verify whether all the > data perfectly drain or not before and after. > > Hope you understand, this time. Hope I understood this time... I've made some modification to sun4i_spi_xfer: static int sun4i_spi_xfer(struct udevice *dev, unsigned int bitlen,               const void *dout, void *din, unsigned long flags) {     struct udevice *bus = dev->parent;     struct sun4i_spi_priv *priv = dev_get_priv(bus);     struct dm_spi_slave_platdata *slave_plat = dev_get_parent_platdata(dev);     u32 len = bitlen / 8;     u32 reg, cnt;     u8 nbytes;     int ret;     priv->tx_buf = dout;     priv->rx_buf = din;     if (bitlen % 8) {         debug("%s: non byte-aligned SPI transfer.\n", __func__);         return -ENAVAIL;     }     if (flags & SPI_XFER_BEGIN)         sun4i_spi_set_cs(bus, slave_plat->cs, true);     reg = readl(&priv->regs->ctl);     /* Reset FIFOs */     writel(reg | SUN4I_CTL_RF_RST | SUN4I_CTL_TF_RST, &priv->regs->ctl);     while (len) {         /* Setup the transfer now... */         nbytes = min(len, (u32)(SUN4I_FIFO_DEPTH - 1));         if (!priv->rx_buf)             printf("%s: Sending %d bytes, ", __func__, nbytes);         /* Setup the counters */         writel(SUN4I_BURST_CNT(nbytes), &priv->regs->bc);         writel(SUN4I_XMIT_CNT(nbytes), &priv->regs->tc);         /* Fill the TX FIFO */         sun4i_spi_fill_fifo(priv, nbytes);         /* Start the transfer */         reg = readl(&priv->regs->ctl);         writel(reg | SUN4I_CTL_XCH, &priv->regs->ctl);         /* Wait transfer to complete */         ret = wait_for_bit_le32(&priv->regs->ctl, SUN4I_CTL_XCH_MASK,                     false, SUN4I_SPI_TIMEOUT_US, false);         if (ret) {             printf("ERROR: sun4i_spi: Timeout transferring data\n");             sun4i_spi_set_cs(bus, slave_plat->cs, false);             return ret;         }         /* Drain the RX FIFO */         if (!priv->rx_buf) {             reg = readl(&priv->regs->fifo_sta);             reg &= SUN4I_FIFO_STA_RF_CNT_MASK;             cnt = reg >> SUN4I_FIFO_STA_RF_CNT_BITS;             printf("rx fifo: before: %d, ", cnt);         }         sun4i_spi_drain_fifo(priv, nbytes);         if (!priv->rx_buf) {             reg = readl(&priv->regs->fifo_sta);             reg &= SUN4I_FIFO_STA_RF_CNT_MASK;             cnt = reg >> SUN4I_FIFO_STA_RF_CNT_BITS;             printf("after: %d\n", cnt);         }         len -= nbytes;     }     if (flags & SPI_XFER_END)         sun4i_spi_set_cs(bus, slave_plat->cs, false);     return 0; } This will print only when destination buffer is NULL. This gave me the following output: sun4i_spi_xfer: Sending 1 bytes, rx fifo: before: 1, after: 0 sun4i_spi_xfer: Sending 1 bytes, rx fifo: before: 1, after: 0 sun4i_spi_xfer: Sending 1 bytes, rx fifo: before: 1, after: 0 sun4i_spi_xfer: Sending 4 bytes, rx fifo: before: 4, after: 0 sun4i_spi_xfer: Sending 63 bytes, rx fifo: before: 63, after: 0 sun4i_spi_xfer: Sending 63 bytes, rx fifo: before: 63, after: 0 sun4i_spi_xfer: Sending 63 bytes, rx fifo: before: 63, after: 0 sun4i_spi_xfer: Sending 63 bytes, rx fifo: before: 63, after: 0 sun4i_spi_xfer: Sending 4 bytes, rx fifo: before: 4, after: 0 sun4i_spi_xfer: Sending 1 bytes, rx fifo: before: 1, after: 0 sun4i_spi_xfer: Sending 1 bytes, rx fifo: before: 1, after: 0 sun4i_spi_xfer: Sending 4 bytes, rx fifo: before: 4, after: 0 sun4i_spi_xfer: Sending 63 bytes, rx fifo: before: 63, after: 0 sun4i_spi_xfer: Sending 63 bytes, rx fifo: before: 63, after: 0 sun4i_spi_xfer: Sending 63 bytes, rx fifo: before: 63, after: 0 sun4i_spi_xfer: Sending 63 bytes, rx fifo: before: 63, after: 0 .... etc As you can see the fifo is drained.