[U-Boot] [PATCH 1/1] arm: sunxi: Add NULL pointer check

[Maxime Ripard]

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 <stefan at 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?

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181214/a68f5355/attachment.sig>