Hi Bin,
On Sun, 20 Oct 2019 at 20:29, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Oct 19, 2019 at 4:37 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Fri, 18 Oct 2019 at 09:38, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Fri, Oct 18, 2019 at 10:14 PM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Thu, 17 Oct 2019 at 20:32, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Fri, Oct 18, 2019 at 10:22 AM Simon Glass sjg@chromium.org wrote:
Hi,
On Thu, 17 Oct 2019 at 08:28, Simon Glass sjg@chromium.org wrote: > > Hi Vignesh, > > On Wed, 16 Oct 2019 at 04:28, Vignesh Raghavendra vigneshr@ti.com wrote: > > > > Hi Simon, > > > > On 12/10/19 10:03 AM, Bin Meng wrote: > > > Hi Simon, > > > > > > On Sat, Oct 12, 2019 at 11:08 AM Simon Glass sjg@chromium.org wrote: > > >> > > >> Hi Bin, > > >> > > >> On Wed, 9 Oct 2019 at 07:55, Bin Meng bmeng.cn@gmail.com wrote: > > >>> > > >>> Hi Simon, > > >>> > > >>> On Wed, Sep 25, 2019 at 10:12 PM Simon Glass sjg@chromium.org wrote: > > >>>> > > >>>> On x86 platforms the SPI flash can be mapped into memory so that the > > >>>> contents can be read with normal memory accesses. > > >>>> > > >>>> Add a new SPI flash method to find the location of the SPI flash in > > >>>> memory. This differs from the existing device-tree "memory-map" mechanism > > >>>> in that the location can be discovered at run-time. > > >>>> > > > > Whats is the usecase? Why can't spi_flash_read() be used instead? > > Flash + Controller driver can underneath take care of using memory > > mapped mode to read data from flash right while making sure that access > > is within valid window? > > I can see spi_flash_read_dm() but it does not support returning a > pointer to the data, only reading it. > > Also I cannot find any documentation about any of this. I've been > looking in the doc/ directory. > > I found the spi_mem.h file but it doesn't mention the meaning of the > in and out buffer pointers so I don't know how to use them. > > Is there an API missing or just comments/documentation?
Apart from this I have found that the ich_spi driver does not work for APL since it apparently only supports 'hardware sequencing'. I did try getting software sequencing to work, but no dice.
In addition, I found that enabling SPI flash, etc. added about 6KB of code!
So I think it might be best to have two SPI drivers for x86 - one for software sequencing and one for hardware?
So if this is true, I think we can create Kconfig options (HW_SEQ and SW_SEQ) for ICH SPI driver, and select either one in the SoC Kconfig file.
I think at least for TPL I'm going to need to bypass all the code as it is just too big for TPL. What do you think?
So what about other boards/drivers? Say we have drv_foo.c, is the best practice to create a separate drv_foo_tpl.c for use with TPL?
Yes, although in general we can export the functions from the core driver, so the TPL one becomes pretty bare-bones - e.g. it handles platdata but uses the operations of the core drivers.
Should we have a separate HW SEQ driver? If you look at the HW SEQ driver I have, it is very little code.
Or are you saying we should have a combined driver with Kconfig options to enable either or both of SW SEQ and HW SEQ?
Yes I was suggesting enable either or both SW and HW SEQ in the same driver.
OK I will take a look at that.
My code-size concern still stands though, but let's see how it looks.
One other wrinkle is that I have to have a separate driver anyway, since of-platdata requires it. It is not possible to use of-platdata in a generic driver since the struct definitions rely on the compatible string.
But I see from the Linux kernel driver, it has:
300static int intel_spi_init(struct intel_spi *ispi) 301{ 302 u32 opmenu0, opmenu1, lvscc, uvscc, val; 303 int i; 304 305 switch (ispi->info->type) { 306 case INTEL_SPI_BYT: 307 ispi->sregs = ispi->base + BYT_SSFSTS_CTL; 308 ispi->pregs = ispi->base + BYT_PR; 309 ispi->nregions = BYT_FREG_NUM; 310 ispi->pr_num = BYT_PR_NUM; 311 ispi->swseq_reg = true; 312 313 if (writeable) { 314 /* Disable write protection */ 315 val = readl(ispi->base + BYT_BCR); 316 if (!(val & BYT_BCR_WPD)) { 317 val |= BYT_BCR_WPD; 318 writel(val, ispi->base + BYT_BCR); 319 val = readl(ispi->base + BYT_BCR); 320 } 321 322 ispi->writeable = !!(val & BYT_BCR_WPD); 323 } 324 325 break; 326 327 case INTEL_SPI_LPT: 328 ispi->sregs = ispi->base + LPT_SSFSTS_CTL; 329 ispi->pregs = ispi->base + LPT_PR; 330 ispi->nregions = LPT_FREG_NUM; 331 ispi->pr_num = LPT_PR_NUM; 332 ispi->swseq_reg = true; 333 break; 334 335 case INTEL_SPI_BXT: 336 ispi->sregs = ispi->base + BXT_SSFSTS_CTL; 337 ispi->pregs = ispi->base + BXT_PR; 338 ispi->nregions = BXT_FREG_NUM; 339 ispi->pr_num = BXT_PR_NUM; 340 ispi->erase_64k = true; 341 break;
So for INTEL_SPI_BXT (which is for ApolloLake I believe) ispi->swseq_reg is not set to true which means it uses hardware sequencer, which seems to contradict with what you found.
I found that it only supports hardware sequencing, so this matches.
OK, I see. So the BXT only supports HW SEQ while LPT and BYT support both SW and HW SEQ.
OK, ta. How do I look up what LPT and BYT are?
I think LPT stands Linx Point (the PCH chipset that desktop processors use) and BYT stands BayTrail which U-Boot already supports.
OK ta.
BTW I'm just preparing the new series. It will be v3 as I have brought in some patches from the other series which was v2.
Regards, Simon