[U-Boot] [PATCH 1/2] dm: sf: Make SST flash write op work again

With SPI flash moving to driver model, commit fbb0991 "dm: Convert spi_flash_probe() and 'sf probe' to use driver model" ignored the SST flash-specific write op (byte program & word program), which actually broke the SST flash from wroking.
This commit makes SST flash work again under driver model, by adding a new SST flash-specific driver to handle the different write op from the standard one.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
drivers/mtd/spi/sf_probe.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index d19138d..47438d2 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -511,4 +511,35 @@ U_BOOT_DRIVER(spi_flash_std) = { .ops = &spi_flash_std_ops, };
+int spi_flash_sst_write(struct udevice *dev, u32 offset, size_t len, + const void *buf) +{ + struct spi_flash *flash = dev_get_uclass_priv(dev); + + 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); +} + +static const struct dm_spi_flash_ops spi_flash_sst_ops = { + .read = spi_flash_std_read, + .write = spi_flash_sst_write, + .erase = spi_flash_std_erase, +}; + +static const struct udevice_id spi_flash_sst_ids[] = { + { .compatible = "spi-flash-sst" }, + { } +}; + +U_BOOT_DRIVER(spi_flash_sst) = { + .name = "spi_flash_sst", + .id = UCLASS_SPI_FLASH, + .of_match = spi_flash_sst_ids, + .probe = spi_flash_std_probe, + .priv_auto_alloc_size = sizeof(struct spi_flash), + .ops = &spi_flash_sst_ops, +}; + #endif /* CONFIG_DM_SPI_FLASH */

On 23 April 2015 at 03:00, Bin Meng bmeng.cn@gmail.com wrote:
With SPI flash moving to driver model, commit fbb0991 "dm: Convert spi_flash_probe() and 'sf probe' to use driver model" ignored the SST flash-specific write op (byte program & word program), which actually broke the SST flash from wroking.
This commit makes SST flash work again under driver model, by adding a new SST flash-specific driver to handle the different write op from the standard one.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/mtd/spi/sf_probe.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

Hi Bin,
On 23 April 2015 at 14:30, Bin Meng bmeng.cn@gmail.com wrote:
With SPI flash moving to driver model, commit fbb0991 "dm: Convert spi_flash_probe() and 'sf probe' to use driver model" ignored the SST flash-specific write op (byte program & word program), which actually broke the SST flash from wroking.
This commit makes SST flash work again under driver model, by adding a new SST flash-specific driver to handle the different write op from the standard one.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/mtd/spi/sf_probe.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index d19138d..47438d2 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -511,4 +511,35 @@ U_BOOT_DRIVER(spi_flash_std) = { .ops = &spi_flash_std_ops, };
+int spi_flash_sst_write(struct udevice *dev, u32 offset, size_t len,
const void *buf)
+{
struct spi_flash *flash = dev_get_uclass_priv(dev);
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);
+}
+static const struct dm_spi_flash_ops spi_flash_sst_ops = {
.read = spi_flash_std_read,
.write = spi_flash_sst_write,
.erase = spi_flash_std_erase,
+};
+static const struct udevice_id spi_flash_sst_ids[] = {
{ .compatible = "spi-flash-sst" },
{ }
+};
+U_BOOT_DRIVER(spi_flash_sst) = {
.name = "spi_flash_sst",
.id = UCLASS_SPI_FLASH,
.of_match = spi_flash_sst_ids,
.probe = spi_flash_std_probe,
.priv_auto_alloc_size = sizeof(struct spi_flash),
.ops = &spi_flash_sst_ops,
+};
#endif /* CONFIG_DM_SPI_FLASH */
1.8.2.1
I'm just curiosity to see different approach of being code duplicate with just for sst write call.
What about this- int spi_flash_std_write(struct udevice *dev, u32 offset, size_t len, const void *buf) { struct spi_flash *flash = dev_get_uclass_priv(dev);
if defined(CONFIG_SPI_FLASH_SST) if (flash->flags & 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); }
Of course this requires extra flags member in spi_flash, any other thoughts?
thanks!

Hi Jagan,
On Fri, Apr 24, 2015 at 4:07 PM, Jagan Teki jagannadh.teki@gmail.com wrote:
Hi Bin,
On 23 April 2015 at 14:30, Bin Meng bmeng.cn@gmail.com wrote:
With SPI flash moving to driver model, commit fbb0991 "dm: Convert spi_flash_probe() and 'sf probe' to use driver model" ignored the SST flash-specific write op (byte program & word program), which actually broke the SST flash from wroking.
This commit makes SST flash work again under driver model, by adding a new SST flash-specific driver to handle the different write op from the standard one.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/mtd/spi/sf_probe.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index d19138d..47438d2 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -511,4 +511,35 @@ U_BOOT_DRIVER(spi_flash_std) = { .ops = &spi_flash_std_ops, };
+int spi_flash_sst_write(struct udevice *dev, u32 offset, size_t len,
const void *buf)
+{
struct spi_flash *flash = dev_get_uclass_priv(dev);
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);
+}
+static const struct dm_spi_flash_ops spi_flash_sst_ops = {
.read = spi_flash_std_read,
.write = spi_flash_sst_write,
.erase = spi_flash_std_erase,
+};
+static const struct udevice_id spi_flash_sst_ids[] = {
{ .compatible = "spi-flash-sst" },
{ }
+};
+U_BOOT_DRIVER(spi_flash_sst) = {
.name = "spi_flash_sst",
.id = UCLASS_SPI_FLASH,
.of_match = spi_flash_sst_ids,
.probe = spi_flash_std_probe,
.priv_auto_alloc_size = sizeof(struct spi_flash),
.ops = &spi_flash_sst_ops,
+};
#endif /* CONFIG_DM_SPI_FLASH */
1.8.2.1
I'm just curiosity to see different approach of being code duplicate with just for sst write call.
What about this- int spi_flash_std_write(struct udevice *dev, u32 offset, size_t len, const void *buf) { struct spi_flash *flash = dev_get_uclass_priv(dev);
if defined(CONFIG_SPI_FLASH_SST) if (flash->flags & 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);
}
Of course this requires extra flags member in spi_flash, any other thoughts?
Yep, this way works too. Let me know which way you prefer and I can respin a v2.
Regards, Bin

On 24 April 2015 at 14:12, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Fri, Apr 24, 2015 at 4:07 PM, Jagan Teki jagannadh.teki@gmail.com wrote:
Hi Bin,
On 23 April 2015 at 14:30, Bin Meng bmeng.cn@gmail.com wrote:
With SPI flash moving to driver model, commit fbb0991 "dm: Convert spi_flash_probe() and 'sf probe' to use driver model" ignored the SST flash-specific write op (byte program & word program), which actually broke the SST flash from wroking.
This commit makes SST flash work again under driver model, by adding a new SST flash-specific driver to handle the different write op from the standard one.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/mtd/spi/sf_probe.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index d19138d..47438d2 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -511,4 +511,35 @@ U_BOOT_DRIVER(spi_flash_std) = { .ops = &spi_flash_std_ops, };
+int spi_flash_sst_write(struct udevice *dev, u32 offset, size_t len,
const void *buf)
+{
struct spi_flash *flash = dev_get_uclass_priv(dev);
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);
+}
+static const struct dm_spi_flash_ops spi_flash_sst_ops = {
.read = spi_flash_std_read,
.write = spi_flash_sst_write,
.erase = spi_flash_std_erase,
+};
+static const struct udevice_id spi_flash_sst_ids[] = {
{ .compatible = "spi-flash-sst" },
{ }
+};
+U_BOOT_DRIVER(spi_flash_sst) = {
.name = "spi_flash_sst",
.id = UCLASS_SPI_FLASH,
.of_match = spi_flash_sst_ids,
.probe = spi_flash_std_probe,
.priv_auto_alloc_size = sizeof(struct spi_flash),
.ops = &spi_flash_sst_ops,
+};
#endif /* CONFIG_DM_SPI_FLASH */
1.8.2.1
I'm just curiosity to see different approach of being code duplicate with just for sst write call.
What about this- int spi_flash_std_write(struct udevice *dev, u32 offset, size_t len, const void *buf) { struct spi_flash *flash = dev_get_uclass_priv(dev);
if defined(CONFIG_SPI_FLASH_SST) if (flash->flags & 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);
}
Of course this requires extra flags member in spi_flash, any other thoughts?
Yep, this way works too. Let me know which way you prefer and I can respin a v2.
I preferred second.
thanks!

Hi Jagan,
On Fri, Apr 24, 2015 at 5:25 PM, Jagan Teki jagannadh.teki@gmail.com wrote:
On 24 April 2015 at 14:12, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Fri, Apr 24, 2015 at 4:07 PM, Jagan Teki jagannadh.teki@gmail.com wrote:
Hi Bin,
On 23 April 2015 at 14:30, Bin Meng bmeng.cn@gmail.com wrote:
With SPI flash moving to driver model, commit fbb0991 "dm: Convert spi_flash_probe() and 'sf probe' to use driver model" ignored the SST flash-specific write op (byte program & word program), which actually broke the SST flash from wroking.
This commit makes SST flash work again under driver model, by adding a new SST flash-specific driver to handle the different write op from the standard one.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/mtd/spi/sf_probe.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index d19138d..47438d2 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -511,4 +511,35 @@ U_BOOT_DRIVER(spi_flash_std) = { .ops = &spi_flash_std_ops, };
+int spi_flash_sst_write(struct udevice *dev, u32 offset, size_t len,
const void *buf)
+{
struct spi_flash *flash = dev_get_uclass_priv(dev);
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);
+}
+static const struct dm_spi_flash_ops spi_flash_sst_ops = {
.read = spi_flash_std_read,
.write = spi_flash_sst_write,
.erase = spi_flash_std_erase,
+};
+static const struct udevice_id spi_flash_sst_ids[] = {
{ .compatible = "spi-flash-sst" },
{ }
+};
+U_BOOT_DRIVER(spi_flash_sst) = {
.name = "spi_flash_sst",
.id = UCLASS_SPI_FLASH,
.of_match = spi_flash_sst_ids,
.probe = spi_flash_std_probe,
.priv_auto_alloc_size = sizeof(struct spi_flash),
.ops = &spi_flash_sst_ops,
+};
#endif /* CONFIG_DM_SPI_FLASH */
1.8.2.1
I'm just curiosity to see different approach of being code duplicate with just for sst write call.
What about this- int spi_flash_std_write(struct udevice *dev, u32 offset, size_t len, const void *buf) { struct spi_flash *flash = dev_get_uclass_priv(dev);
if defined(CONFIG_SPI_FLASH_SST) if (flash->flags & 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);
}
Of course this requires extra flags member in spi_flash, any other thoughts?
Yep, this way works too. Let me know which way you prefer and I can respin a v2.
I preferred second.
OK, will respin a v2 soon. Thanks,
Regards, Bin
participants (3)
-
Bin Meng
-
Jagan Teki
-
Simon Glass