| Message ID | 20200514131935.8583-1-jagan@amarulasolutions.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Jagan, On Thu, 14 May 2020 at 07:19, Jagan Teki <jagan@amarulasolutions.com> wrote: > > UCLASS_SPI_FLASH driver at driver/mtd/spi is a generic > spi flash driver to probe jedec,spi-nor flash chips. > > Technically a probe call in U_BOOT_DRIVER is local to that > driver and not applicable to use it another driver or in > another code. > > The apollolake SPL code using the generic probe by adding > extra SPI flash driver, which make more confusion in terms > of code readability and driver model structure. > > The fact that apollolake SPL requires a separate SPI flash > driver to handle of-platdata via bind call, so move the > bind call in the generic flash driver and drop the driver > from apollolake code. > > I hope this wouldn't break generic code usage flash chips > otherwise, we can handle this via driver data or a separate > spi driver in drivers/mtd/spi. > > Cc: Bin Meng <bmeng.cn@gmail.com> > Cc: Simon Glass <sjg@chromium.org> > Cc: Vignesh R <vigneshr@ti.com> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > --- > arch/x86/cpu/apollolake/spl.c | 60 ----------------------------------- > drivers/mtd/spi/sf_probe.c | 29 ++++++++++++++++- > include/spi_flash.h | 12 ------- > 3 files changed, 28 insertions(+), 73 deletions(-) Unfortunately this stops TPL from booting. Could you expose the 'read' method properly, perhaps? Regards, Simon
On Mon, May 25, 2020 at 10:34 PM Simon Glass <sjg@chromium.org> wrote: > > Hi Jagan, > > On Thu, 14 May 2020 at 07:19, Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > UCLASS_SPI_FLASH driver at driver/mtd/spi is a generic > > spi flash driver to probe jedec,spi-nor flash chips. > > > > Technically a probe call in U_BOOT_DRIVER is local to that > > driver and not applicable to use it another driver or in > > another code. > > > > The apollolake SPL code using the generic probe by adding > > extra SPI flash driver, which make more confusion in terms > > of code readability and driver model structure. > > > > The fact that apollolake SPL requires a separate SPI flash > > driver to handle of-platdata via bind call, so move the > > bind call in the generic flash driver and drop the driver > > from apollolake code. > > > > I hope this wouldn't break generic code usage flash chips > > otherwise, we can handle this via driver data or a separate > > spi driver in drivers/mtd/spi. > > > > Cc: Bin Meng <bmeng.cn@gmail.com> > > Cc: Simon Glass <sjg@chromium.org> > > Cc: Vignesh R <vigneshr@ti.com> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > --- > > arch/x86/cpu/apollolake/spl.c | 60 ----------------------------------- > > drivers/mtd/spi/sf_probe.c | 29 ++++++++++++++++- > > include/spi_flash.h | 12 ------- > > 3 files changed, 28 insertions(+), 73 deletions(-) > > Unfortunately this stops TPL from booting. > > Could you expose the 'read' method properly, perhaps? Not sure why it requires, read call seems very much similar to sf isn't it? Jagan.
On Mon, May 25, 2020 at 11:01 PM Jagan Teki <jagan@amarulasolutions.com> wrote: > > On Mon, May 25, 2020 at 10:34 PM Simon Glass <sjg@chromium.org> wrote: > > > > Hi Jagan, > > > > On Thu, 14 May 2020 at 07:19, Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > > > UCLASS_SPI_FLASH driver at driver/mtd/spi is a generic > > > spi flash driver to probe jedec,spi-nor flash chips. > > > > > > Technically a probe call in U_BOOT_DRIVER is local to that > > > driver and not applicable to use it another driver or in > > > another code. > > > > > > The apollolake SPL code using the generic probe by adding > > > extra SPI flash driver, which make more confusion in terms > > > of code readability and driver model structure. > > > > > > The fact that apollolake SPL requires a separate SPI flash > > > driver to handle of-platdata via bind call, so move the > > > bind call in the generic flash driver and drop the driver > > > from apollolake code. > > > > > > I hope this wouldn't break generic code usage flash chips > > > otherwise, we can handle this via driver data or a separate > > > spi driver in drivers/mtd/spi. > > > > > > Cc: Bin Meng <bmeng.cn@gmail.com> > > > Cc: Simon Glass <sjg@chromium.org> > > > Cc: Vignesh R <vigneshr@ti.com> > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > > --- > > > arch/x86/cpu/apollolake/spl.c | 60 ----------------------------------- > > > drivers/mtd/spi/sf_probe.c | 29 ++++++++++++++++- > > > include/spi_flash.h | 12 ------- > > > 3 files changed, 28 insertions(+), 73 deletions(-) > > > > Unfortunately this stops TPL from booting. > > > > Could you expose the 'read' method properly, perhaps? > > Not sure why it requires, read call seems very much similar to sf isn't it? Any clue? Jagan.
diff --git a/arch/x86/cpu/apollolake/spl.c b/arch/x86/cpu/apollolake/spl.c index d32f2a9898..2e6013d04c 100644 --- a/arch/x86/cpu/apollolake/spl.c +++ b/arch/x86/cpu/apollolake/spl.c @@ -65,66 +65,6 @@ SPL_LOAD_IMAGE_METHOD("Mapped SPI", 2, BOOT_DEVICE_SPI_MMAP, rom_load_image); #if CONFIG_IS_ENABLED(SPI_FLASH_SUPPORT) -static int apl_flash_std_read(struct udevice *dev, u32 offset, size_t len, - void *buf) -{ - struct spi_flash *flash = dev_get_uclass_priv(dev); - struct mtd_info *mtd = &flash->mtd; - size_t retlen; - - return log_ret(mtd->_read(mtd, offset, len, &retlen, buf)); -} - -static int apl_flash_probe(struct udevice *dev) -{ - return spi_flash_std_probe(dev); -} - -/* - * Manually set the parent of the SPI flash to SPI, since dtoc doesn't. We also - * need to allocate the parent_platdata since by the time this function is - * called device_bind() has already gone past that step. - */ -static int apl_flash_bind(struct udevice *dev) -{ - if (CONFIG_IS_ENABLED(OF_PLATDATA)) { - struct dm_spi_slave_platdata *plat; - struct udevice *spi; - int ret; - - ret = uclass_first_device_err(UCLASS_SPI, &spi); - if (ret) - return ret; - dev->parent = spi; - - plat = calloc(sizeof(*plat), 1); - if (!plat) - return -ENOMEM; - dev->parent_platdata = plat; - } - - return 0; -} - -static const struct dm_spi_flash_ops apl_flash_ops = { - .read = apl_flash_std_read, -}; - -static const struct udevice_id apl_flash_ids[] = { - { .compatible = "jedec,spi-nor" }, - { } -}; - -U_BOOT_DRIVER(winbond_w25q128fw) = { - .name = "winbond_w25q128fw", - .id = UCLASS_SPI_FLASH, - .of_match = apl_flash_ids, - .bind = apl_flash_bind, - .probe = apl_flash_probe, - .priv_auto_alloc_size = sizeof(struct spi_flash), - .ops = &apl_flash_ops, -}; - /* This uses a SPI flash device to read the next phase */ static int spl_fast_spi_load_image(struct spl_image_info *spl_image, struct spl_boot_device *bootdev) diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index f167bfab8a..06dac57daf 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -129,7 +129,7 @@ static int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len) return mtd->_erase(mtd, &instr); } -int spi_flash_std_probe(struct udevice *dev) +static int spi_flash_std_probe(struct udevice *dev) { struct spi_slave *slave = dev_get_parent_priv(dev); struct spi_flash *flash; @@ -154,6 +154,32 @@ static const struct dm_spi_flash_ops spi_flash_std_ops = { .erase = spi_flash_std_erase, }; +/* + * Manually set the parent of the SPI flash to SPI, since dtoc doesn't. We also + * need to allocate the parent_platdata since by the time this function is + * called device_bind() has already gone past that step. + */ +static int spi_flash_bind(struct udevice *dev) +{ + if (CONFIG_IS_ENABLED(OF_PLATDATA)) { + struct dm_spi_slave_platdata *plat; + struct udevice *spi; + int ret; + + ret = uclass_first_device_err(UCLASS_SPI, &spi); + if (ret) + return ret; + dev->parent = spi; + + plat = calloc(sizeof(*plat), 1); + if (!plat) + return -ENOMEM; + dev->parent_platdata = plat; + } + + return 0; +} + static const struct udevice_id spi_flash_std_ids[] = { { .compatible = "jedec,spi-nor" }, { } @@ -163,6 +189,7 @@ U_BOOT_DRIVER(spi_flash_std) = { .name = "spi_flash_std", .id = UCLASS_SPI_FLASH, .of_match = spi_flash_std_ids, + .bind = spi_flash_bind, .probe = spi_flash_std_probe, .remove = spi_flash_std_remove, .priv_auto_alloc_size = sizeof(struct spi_flash), diff --git a/include/spi_flash.h b/include/spi_flash.h index d9b2af856c..3b33b970ef 100644 --- a/include/spi_flash.h +++ b/include/spi_flash.h @@ -75,18 +75,6 @@ int spi_flash_write_dm(struct udevice *dev, u32 offset, size_t len, */ int spi_flash_erase_dm(struct udevice *dev, u32 offset, size_t len); -/** - * spi_flash_std_probe() - Probe a SPI flash device - * - * This is the standard internal method for probing a SPI flash device to - * determine its type. It can be used in chip-specific drivers which need to - * do this, typically with of-platdata - * - * @dev: SPI-flash device to probe - * @return 0 if OK, -ve on error - */ -int spi_flash_std_probe(struct udevice *dev); - int spi_flash_probe_bus_cs(unsigned int busnum, unsigned int cs, unsigned int max_hz, unsigned int spi_mode, struct udevice **devp);
UCLASS_SPI_FLASH driver at driver/mtd/spi is a generic spi flash driver to probe jedec,spi-nor flash chips. Technically a probe call in U_BOOT_DRIVER is local to that driver and not applicable to use it another driver or in another code. The apollolake SPL code using the generic probe by adding extra SPI flash driver, which make more confusion in terms of code readability and driver model structure. The fact that apollolake SPL requires a separate SPI flash driver to handle of-platdata via bind call, so move the bind call in the generic flash driver and drop the driver from apollolake code. I hope this wouldn't break generic code usage flash chips otherwise, we can handle this via driver data or a separate spi driver in drivers/mtd/spi. Cc: Bin Meng <bmeng.cn@gmail.com> Cc: Simon Glass <sjg@chromium.org> Cc: Vignesh R <vigneshr@ti.com> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- arch/x86/cpu/apollolake/spl.c | 60 ----------------------------------- drivers/mtd/spi/sf_probe.c | 29 ++++++++++++++++- include/spi_flash.h | 12 ------- 3 files changed, 28 insertions(+), 73 deletions(-)