From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Mavrodiev <stefan@olimex.com>
Date: Fri, 14 Dec 2018 16:14:31 +0200
Subject: [U-Boot] [PATCH 1/1] arm: sunxi: Add NULL pointer check
In-Reply-To: <20181214092522.3ev2b4d4q33k6zip@flea>
References: <20181205122757.14523-1-stefan@olimex.com>
 <20181205154619.ffzfl7yt6vfo3qra@flea>
 <9458ed1f-38de-6be0-8e27-a2d6801492a2@gmail.com>
 <449750de-5be4-aeb1-9d5c-43b42f175ce0@olimex.com>
 <20181214092522.3ev2b4d4q33k6zip@flea>
Message-ID: <0e57b561-d56c-440a-74f8-6c35236c40dc@olimex.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
To: u-boot@lists.denx.de


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@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?

>
> Maxime
>