linux-kernel - Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bc772ef1-5f3b-518b-3a02-ffcf7143d36d@exceet.de>
Date:   Tue, 26 Jun 2018 10:58:24 +0200
From:   Frieder Schrempf <frieder.schrempf@...eet.de>
To:     Boris Brezillon <boris.brezillon@...tlin.com>,
        Yogesh Narayan Gaur <yogeshnarayan.gaur@....com>
Cc:     "marek.vasut@...il.com" <marek.vasut@...il.com>,
        "broonie@...nel.org" <broonie@...nel.org>,
        Fabio Estevam <fabio.estevam@....com>,
        David Wolfe <david.wolfe@....com>,
        "dwmw2@...radead.org" <dwmw2@...radead.org>,
        "richard@....at" <richard@....at>,
        Prabhakar Kushwaha <prabhakar.kushwaha@....com>,
        Han Xu <han.xu@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
        "linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
        "miquel.raynal@...tlin.com" <miquel.raynal@...tlin.com>,
        "computersforpeace@...il.com" <computersforpeace@...il.com>
Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI
 controller

Hi Boris, hi Yogesh,

first, thank you for testing and debugging the new driver.

On 19.06.2018 10:46, Boris Brezillon wrote:
> On Tue, 19 Jun 2018 08:31:25 +0000
> Yogesh Narayan Gaur <yogeshnarayan.gaur@....com> wrote:
>
[...]
>>>
>>> On Tue, 19 Jun 2018 07:10:37 +0000
>>> Yogesh Narayan Gaur <yogeshnarayan.gaur@....com> wrote:
>>>    
>>>> Let us take below layout of memory address space map.
>>>> QuadSPI Controller can access range from 0x2000_0000 -
>>>> 0x2FFF_FFFF i.e. 256
>>> MB address space reserved and it is having 4 slave devices
>>> connected.
>>>> These slave devices[of size 64MB, 64MB, 32MB and 64MB ] are
>>>> connected at below address 0x2000_0000, 0x2400_0000, 0x2A00_0000,
>>>> 0x2C00_0000 i.e. there is gap of 32MB from 0x2800_0000 to
>>>> 0x29FF_FFFF.
>>>
>>> Okay, I'm fine with pre-reserving 32MB per chip select.
>>>    
>>>>
>>>> As per my understanding of the controller, flash XX top address,
>>>> register should
>>> have below values:
>>>>    QUADSPI_SFA1AD - 0x0
>>>>    QUADSPI_SFA2AD - 0x400_0000
>>>>    QUADSPI_SFB1AD - 0xA00_0000
>>>>    QUADSPI_SFB2AD - 0xC00_0000
>>>> And Register QUADSPI_SFAR should point to the range for the flash
>>>> in which
>>> operation is happening.
>>
>> My mistake values of these register would be for said case are:
>> QUADSPI_SFA1AD - 0x400_0000
>> QUADSPI_SFA2AD - 0x800_0000
>> QUADSPI_SFB1AD - 0xC00_0000
>> QUADSPI_SFB2AD - 0x1000_0000
>>
>> i.e. as per controller each register is having the Top address for
>> serial flash connected at A1/A2/B1/B2 respectively.
> 
> This is still wrong ;-). I guess you mean:
> 
> QUADSPI_SFA1AD - 0x2400_0000
> QUADSPI_SFA2AD - 0x2800_0000
> QUADSPI_SFB1AD - 0x2C00_0000
> QUADSPI_SFB2AD - 0x3000_0000
> 
>>
>>>
>>> Wait, I thought it was supposed to be an absolute address, not one
>>> relative to the 0x20000000 offset.
>>>    
>>>>
>>>> Please check Table10-32, page 1657, in [1] for more details on
>>>> flash address
>>> assignment.
>>>
>>> Yes, I still don't see where it says that having one of the range
>>> with a zero size is forbidden, or anything mentioning a required
>>> alignment.
>>>>
>>>> But say if I assign address to register QUADSPI_SFA2AD as "0 + 2
>>>> * -
>>>> ahb_buf_size" then this address value is not correct as per the
>>>> value range
>>> explained in above mentioned table.
>>>
>>> Why? If the SFA1AD is set to zero, that should not, right?
>> What this table says that for TOP_ADDR_MEMA1 defines the top address
>> for flash connected at A1 and any address space between
>> TOP_ADDR_MEMA1 and QSPI_AMBA_BASE will be routed to Serial Flash A1.
>> In my example case TOP_ADDR_MEMA1 is 0x400_0000 If assign value to
>> SFAR register is "0 + 2 * ->ahb_buf_size", then this would lie in
>> access range of Serial Flash A1 and access happens to A1 flash
>> whereas we want access to A2 flash.
> 
> No, not if SFA1AD is 0x20000000, because then the address range for CS0
> would be 0x20000000 -> 0x20000000.
> 
> If you look at the code, you'll see that I adjust the CS mapping
> dynamically, making the one being access use the range
> 0x20000000 -> (0x20000000 + 2 * ->ahb_buf_size) and assigning a 0-size
> range for the other ones (either 0x20000000 -> 0x20000000 or
> (0x20000000 + 2 * ->ahb_buf_size) -> (0x20000000 + 2 * ->ahb_buf_size))
> 
>>
>> For access of serial flash A2, any address space access between
>> TOP_ADDR_MEMA2 and TOP_ADDR_MEMA1 would be routed to serial flash A2.
>> Thus to access A2 flash, SFAR would be in range from 0x400_0000 and
>> 0x800_0000
> 
> I understand what you're explaining, what I don't get is why the QSPI
> IP doesn't cope with a 0-size range. If you have SFA1AD set to
> 0x20000000 and SFA2AD set so 0x20000800, I would except any access to
> the 0x20000000 -> 0x20000800 range to be routed to CS1 not CS0. But
> apparently it's not working like that.

But it should definitely be possible to have 0-size ranges in the 
mapping. At least the RM for the i.MX6UL says:

"In case single die flash devices, TOP_ADDR_MEMA2 and
TOP_ADDR_MEMB2 should be initialized/programmed to
TOP_ADDR_MEMA1 and TOP_ADDR_MEMB1
respectively- in effect, setting the size of these devices to 0.
This would ensure that the complete memory map is assigned
to only one flash device."

Yogesh, can you test if it works with the fixed mapping proposed above, 
reserving a fixed range of 2 * ahb_buf_size for each chip?

Thanks,
Frieder