Re: [RFC PATCH] mtd: spi: Drop redundent SPI flash driver

2020-07-08 Thread Jagan Teki
On Mon, May 25, 2020 at 11:01 PM Jagan Teki  wrote:
>
> On Mon, May 25, 2020 at 10:34 PM Simon Glass  wrote:
> >
> > Hi Jagan,
> >
> > On Thu, 14 May 2020 at 07:19, Jagan Teki  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 
> > > Cc: Simon Glass 
> > > Cc: Vignesh R 
> > > Signed-off-by: Jagan Teki 
> > > ---
> > >  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.


Re: [RFC PATCH] mtd: spi: Drop redundent SPI flash driver

2020-05-25 Thread Jagan Teki
On Mon, May 25, 2020 at 10:34 PM Simon Glass  wrote:
>
> Hi Jagan,
>
> On Thu, 14 May 2020 at 07:19, Jagan Teki  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 
> > Cc: Simon Glass 
> > Cc: Vignesh R 
> > Signed-off-by: Jagan Teki 
> > ---
> >  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.


Re: [RFC PATCH] mtd: spi: Drop redundent SPI flash driver

2020-05-25 Thread Simon Glass
Hi Jagan,

On Thu, 14 May 2020 at 07:19, Jagan Teki  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 
> Cc: Simon Glass 
> Cc: Vignesh R 
> Signed-off-by: Jagan Teki 
> ---
>  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


[RFC PATCH] mtd: spi: Drop redundent SPI flash driver

2020-05-14 Thread Jagan Teki
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 
Cc: Simon Glass 
Cc: Vignesh R 
Signed-off-by: Jagan Teki 
---
 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(-)

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 = >mtd;
-   size_t retlen;
-
-   return log_ret(mtd->_read(mtd, offset, len, , 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, );
-   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= _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, );
 }
 
-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, );
+   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