-----Original Message-----
From: Boris Brezillon [mailto:boris.brezil...@bootlin.com]
Sent: Friday, June 15, 2018 7:26 PM
To: Yogesh Narayan Gaur <yogeshnarayan.g...@nxp.com>; Fabio Estevam
<fabio.este...@nxp.com>; David Wolfe <david.wo...@nxp.com>; dw...@infradead.org
Cc: rich...@nod.at; Prabhakar Kushwaha <prabhakar.kushw...@nxp.com>; Han Xu
<han...@nxp.com>; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org;
marek.va...@gmail.com; Frieder Schrempf <frieder.schre...@exceet.de>;
broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com;
computersforpe...@gmail.com
Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI
controller
On Fri, 15 Jun 2018 13:42:12 +0000
Yogesh Narayan Gaur <yogeshnarayan.g...@nxp.com> wrote:
> Hi Boris,
>
> I am still debugging the issue.
> With some analysis, able to check that proper values are not being written
> for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register.
>
> In current code, value of map_addr are being assigned to these register.
> map_addr = q->memmap_phy +
> 2 * q->devtype_data->ahb_buf_size;
>
> qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4));
>
> But instead of "q->devtype_data->ahb_buf_size" it should be flash size.
No, because we're only using 2 * ->ahb_buf_size in the direct mapping for each
device, and we're modifying the mapping dynamically based on the selected
device. Maybe we got the logic wrong though.
Yes, for register QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD, we need to
save starting actual address from where this flash is getting started.
Thus, if my first flash size is 64MB, then register QUADSPI_SFA2AD would have
value of q->memmap_phy + 0x4000000 i.e. (QUADSPI_SFA1AD + sizeof First Flash)
If second flash is of size 32MB, then register QUADSPI_SFB1AD would have value
of value of QUADSPI_SFA2AD + sizeof second flash.
> For my case flash size is 0x4000000 and with this hard coded value I am able
> to perform Write and Erase operation.
> One more change, I have to do is adding the flash_size when writing the
> base_address in SFAR register for case when "mem->spi->chip_select == 1"
> qspi_writel(q, q->memmap_phy + 0x4000000, base + QUADSPI_SFAR);
I don't want to expose the full device in the direct mapping yet (that's part
of the direct-mapping API I posted here [1]). What this version of the driver
does is, map only 2 time the ahb_size so that we can bypass the internal cache
of the QSPI engine.
To perform any operation on second flash, we need to provide it's base address
should be saved in SFAR register for this particular operation.
Exposing only 2 time of ahb_size is design decision but value in SFAR register
should be correct.
>
> Thus, there should be mechanism or the entry in structure where we can have
> the information of the size of the connected slave device.
Because that's exactly the kind of thing I'd like to avoid. What if the device
is bigger than the reserved memory region? What if the sum of all devices does
not fit in there? Here I tried to support all cases by just mapping the portion
of memory we need.
So IMO, there should be mechanism to have value of start address of each slave
device. This might can be done from DTS entry of each slave device connected to
the controller.
