[U-Boot] [PATCH v7 08/34] sf: Use flash function pointers in dm_spi_flash_ops

[Jagan Teki]

Hi Simon,

On 26 November 2015 at 23:20, Simon Glass <sjg at chromium.org> wrote:
> Hi Jagan,
>
> On 26 November 2015 at 04:03, Jagan Teki <jteki at openedev.com> wrote:
>> flash operations are defined as static and reuse them
>> with function-pointers so call them with generic
>> function pounters instead of calling like normal functions.
>>
>> Signed-off-by: Jagan Teki <jteki at openedev.com>
>> ---
>>  drivers/mtd/spi/sf_ops.c   |  2 --
>>  drivers/mtd/spi/sf_probe.c | 15 +++------------
>>  include/spi_flash.h        | 13 -------------
>>  3 files changed, 3 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
>> index 853759e..ab21f02 100644
>> --- a/drivers/mtd/spi/sf_ops.c
>> +++ b/drivers/mtd/spi/sf_ops.c
>> @@ -958,7 +958,6 @@ int spi_flash_scan(struct spi_slave *spi, struct spi_flash *flash)
>>                 flash->flags |= SNOR_F_SST_WR;
>>
>>         /* Assign spi_flash ops */
>> -#ifndef CONFIG_DM_SPI_FLASH
>>         flash->write = spi_flash_cmd_write_ops;
>>  #if defined(CONFIG_SPI_FLASH_SST)
>>         if (flash->flags & SNOR_F_SST_WR) {
>> @@ -970,7 +969,6 @@ int spi_flash_scan(struct spi_slave *spi, struct spi_flash *flash)
>>  #endif
>>         flash->erase = spi_flash_cmd_erase_ops;
>>         flash->read = spi_flash_cmd_read_ops;
>> -#endif
>>
>>         /* lock hooks are flash specific - assign them based on idcode0 */
>>         switch (idcode[0]) {
>> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
>> index e35b917..678b81c 100644
>> --- a/drivers/mtd/spi/sf_probe.c
>> +++ b/drivers/mtd/spi/sf_probe.c
>> @@ -120,7 +120,7 @@ static int spi_flash_std_read(struct udevice *dev, u32 offset, size_t len,
>>  {
>>         struct spi_flash *flash = dev_get_uclass_priv(dev);
>>
>> -       return spi_flash_cmd_read_ops(flash, offset, len, buf);
>> +       return flash->read(flash, offset, len, buf);
>>  }
>>
>>  int spi_flash_std_write(struct udevice *dev, u32 offset, size_t len,
>> @@ -128,23 +128,14 @@ int spi_flash_std_write(struct udevice *dev, u32 offset, size_t len,
>>  {
>>         struct spi_flash *flash = dev_get_uclass_priv(dev);
>>
>> -#if defined(CONFIG_SPI_FLASH_SST)
>> -       if (flash->flags & SNOR_F_SST_WR) {
>> -               if (flash->spi->op_mode_tx & SPI_OPM_TX_BP)
>> -                       return sst_write_bp(flash, offset, len, buf);
>> -               else
>> -                       return sst_write_wp(flash, offset, len, buf);
>> -       }
>> -#endif
>> -
>> -       return spi_flash_cmd_write_ops(flash, offset, len, buf);
>> +       return flash->write(flash, offset, len, buf);
>>  }
>>
>>  int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len)
>>  {
>>         struct spi_flash *flash = dev_get_uclass_priv(dev);
>>
>> -       return spi_flash_cmd_erase_ops(flash, offset, len);
>> +       return flash->erase(flash, offset, len);
>>  }
>>
>>  int spi_flash_std_probe(struct udevice *dev)
>> diff --git a/include/spi_flash.h b/include/spi_flash.h
>> index f25b3e7..721b358 100644
>> --- a/include/spi_flash.h
>> +++ b/include/spi_flash.h
>> @@ -94,23 +94,10 @@ struct spi_flash {
>>         int (*flash_lock)(struct spi_flash *flash, u32 ofs, size_t len);
>>         int (*flash_unlock)(struct spi_flash *flash, u32 ofs, size_t len);
>>         int (*flash_is_locked)(struct spi_flash *flash, u32 ofs, size_t len);
>> -#ifndef CONFIG_DM_SPI_FLASH
>> -       /*
>> -        * These are not strictly needed for driver model, but keep them here
>> -        * while the transition is in progress.
>> -        *
>> -        * Normally each driver would provide its own operations, but for
>> -        * SPI flash most chips use the same algorithms. One approach is
>> -        * to create a 'common' SPI flash device which knows how to talk
>> -        * to most devices, and then allow other drivers to be used instead
>> -        * if required, perhaps with a way of scanning through the list to
>> -        * find the driver that matches the device.
>> -        */
>>         int (*read)(struct spi_flash *flash, u32 offset, size_t len, void *buf);
>>         int (*write)(struct spi_flash *flash, u32 offset, size_t len,
>>                         const void *buf);
>>         int (*erase)(struct spi_flash *flash, u32 offset, size_t len);
>> -#endif
>
> No this should go the other way. The flash_...() methods introduced
> above should be removed with driver model.

The lock methods above flash_..() are common to both dm and non-dm
there is no separate lock ops for dm now.

>
> If we don't move things forward towards driver model for all platforms
> we are going to create a fork.
>
>>  };
>>
>>  struct dm_spi_flash_ops {
>> --

thanks!
-- 
Jagan.