All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/2] dm: sf: Make SST flash write op work again
@ 2015-04-23  9:00 Bin Meng
  2015-04-24  3:35 ` Simon Glass
  2015-04-24  8:07 ` Jagan Teki
  0 siblings, 2 replies; 6+ messages in thread
From: Bin Meng @ 2015-04-23  9:00 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 1/2] dm: sf: Make SST flash write op work again
  2015-04-23  9:00 [U-Boot] [PATCH 1/2] dm: sf: Make SST flash write op work again Bin Meng
@ 2015-04-24  3:35 ` Simon Glass
  2015-04-24  8:07 ` Jagan Teki
  1 sibling, 0 replies; 6+ messages in thread
From: Simon Glass @ 2015-04-24  3:35 UTC (permalink / raw)
  To: u-boot

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>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 1/2] dm: sf: Make SST flash write op work again
  2015-04-23  9:00 [U-Boot] [PATCH 1/2] dm: sf: Make SST flash write op work again Bin Meng
  2015-04-24  3:35 ` Simon Glass
@ 2015-04-24  8:07 ` Jagan Teki
  2015-04-24  8:42   ` Bin Meng
  1 sibling, 1 reply; 6+ messages in thread
From: Jagan Teki @ 2015-04-24  8:07 UTC (permalink / raw)
  To: u-boot

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!
-- 
Jagan.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 1/2] dm: sf: Make SST flash write op work again
  2015-04-24  8:07 ` Jagan Teki
@ 2015-04-24  8:42   ` Bin Meng
  2015-04-24  9:25     ` Jagan Teki
  0 siblings, 1 reply; 6+ messages in thread
From: Bin Meng @ 2015-04-24  8:42 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 1/2] dm: sf: Make SST flash write op work again
  2015-04-24  8:42   ` Bin Meng
@ 2015-04-24  9:25     ` Jagan Teki
  2015-04-24  9:31       ` Bin Meng
  0 siblings, 1 reply; 6+ messages in thread
From: Jagan Teki @ 2015-04-24  9:25 UTC (permalink / raw)
  To: u-boot

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!
-- 
Jagan.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 1/2] dm: sf: Make SST flash write op work again
  2015-04-24  9:25     ` Jagan Teki
@ 2015-04-24  9:31       ` Bin Meng
  0 siblings, 0 replies; 6+ messages in thread
From: Bin Meng @ 2015-04-24  9:31 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-04-24  9:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-23  9:00 [U-Boot] [PATCH 1/2] dm: sf: Make SST flash write op work again Bin Meng
2015-04-24  3:35 ` Simon Glass
2015-04-24  8:07 ` Jagan Teki
2015-04-24  8:42   ` Bin Meng
2015-04-24  9:25     ` Jagan Teki
2015-04-24  9:31       ` Bin Meng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.