linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* mtd: mchp23k256: How to follow a more generic approach?
@ 2019-02-18 12:33 Heinrich.Toews
  2019-02-18 13:02 ` Schrempf Frieder
  0 siblings, 1 reply; 6+ messages in thread
From: Heinrich.Toews @ 2019-02-18 12:33 UTC (permalink / raw)
  To: linux-mtd; +Cc: Oleg.Karfich

Hi altogether,

I'm using currently the CONFIG_MTD_MCHP23K256 driver to access an 
ANV32AA1W 1Mb Serial SPI nvSRAM from Anvo-Systems Dresden.

I did some changes to the driver as seen below to make it work.


diff --git a/drivers/mtd/devices/mchp23k256.c 
b/drivers/mtd/devices/mchp23k256.c
index 9d8306a..6140973 100644
--- a/drivers/mtd/devices/mchp23k256.c
+++ b/drivers/mtd/devices/mchp23k256.c
@@ -28,6 +28,7 @@ struct mchp23k256_flash {
  };

  #define MCHP23K256_CMD_WRITE_STATUS    0x01
+#define MCHP23K256_CMD_WREN            0x06
  #define MCHP23K256_CMD_WRITE           0x02
  #define MCHP23K256_CMD_READ            0x03
  #define MCHP23K256_MODE_SEQ            BIT(6)
@@ -40,13 +41,14 @@ static int mchp23k256_write(struct mtd_info *mtd, 
loff_t to, size_t len,
         struct mchp23k256_flash *flash = to_mchp23k256_flash(mtd);
         struct spi_transfer transfer[2] = {};
         struct spi_message message;
-       unsigned char command[3];
+       unsigned char command[4];

         spi_message_init(&message);

         command[0] = MCHP23K256_CMD_WRITE;
-       command[1] = to >> 8;
-       command[2] = to;
+       command[1] = to >> 16;
+       command[2] = to >> 8;
+       command[3] = to;

         transfer[0].tx_buf = command;
         transfer[0].len = sizeof(command);
@@ -73,14 +75,15 @@ static int mchp23k256_read(struct mtd_info *mtd, 
loff_t from, size_t len,
         struct mchp23k256_flash *flash = to_mchp23k256_flash(mtd);
         struct spi_transfer transfer[2] = {};
         struct spi_message message;
-       unsigned char command[3];
+       unsigned char command[4];

         spi_message_init(&message);

         memset(&transfer, 0, sizeof(transfer));
         command[0] = MCHP23K256_CMD_READ;
-       command[1] = from >> 8;
-       command[2] = from;
+       command[1] = from >> 16;
+       command[2] = from >> 8;
+       command[3] = from;

         transfer[0].tx_buf = command;
         transfer[0].len = sizeof(command);
@@ -104,17 +107,18 @@ static int mchp23k256_read(struct mtd_info *mtd, 
loff_t from, size_t len,
  /*
   * Set the device into sequential mode. This allows read/writes to the
   * entire SRAM in a single operation
+ *
+ * CHANGE: Enable Write Mode in the device
   */
  static int mchp23k256_set_mode(struct spi_device *spi)
  {
         struct spi_transfer transfer = {};
         struct spi_message message;
-       unsigned char command[2];
+       unsigned char command[1];

         spi_message_init(&message);

-       command[0] = MCHP23K256_CMD_WRITE_STATUS;
-       command[1] = MCHP23K256_MODE_SEQ;
+       command[0] = MCHP23K256_CMD_WREN;

         transfer.tx_buf = command;
         transfer.len = sizeof(command);
@@ -147,7 +151,7 @@ static int mchp23k256_probe(struct spi_device *spi)
         flash->mtd.type         = MTD_RAM;
         flash->mtd.flags        = MTD_CAP_RAM;
         flash->mtd.writesize    = 1;
-       flash->mtd.size         = SZ_32K;
+       flash->mtd.size         = SZ_128K;
         flash->mtd._read        = mchp23k256_read;
         flash->mtd._write       = mchp23k256_write;


What would be the best approach to add a more generic solution to the 
kernel in order to be able to address different SPI SRAM devices?

Thanks.


Greetings,

	Heinrich Toews
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: mtd: mchp23k256: How to follow a more generic approach?
  2019-02-18 12:33 mtd: mchp23k256: How to follow a more generic approach? Heinrich.Toews
@ 2019-02-18 13:02 ` Schrempf Frieder
  2019-02-19  8:15   ` Heinrich.Toews
  0 siblings, 1 reply; 6+ messages in thread
From: Schrempf Frieder @ 2019-02-18 13:02 UTC (permalink / raw)
  To: Heinrich.Toews, linux-mtd; +Cc: Oleg.Karfich

Hi Heinrich,

On 18.02.19 13:33, Heinrich.Toews@wago.com wrote:
> Hi altogether,
> 
> I'm using currently the CONFIG_MTD_MCHP23K256 driver to access an
> ANV32AA1W 1Mb Serial SPI nvSRAM from Anvo-Systems Dresden.

We also have a ANV32 nvSRAM chip on some of our boards and so far we 
used misc/eeprom/at25.c to access the memory.
It would be nice to use the chip with the MTD framework too, so it's 
interesting to see, that it's possible with a modified version of 
mchp23k256.c.

> 
> I did some changes to the driver as seen below to make it work.
> 
> 
> diff --git a/drivers/mtd/devices/mchp23k256.c
> b/drivers/mtd/devices/mchp23k256.c
> index 9d8306a..6140973 100644
> --- a/drivers/mtd/devices/mchp23k256.c
> +++ b/drivers/mtd/devices/mchp23k256.c
> @@ -28,6 +28,7 @@ struct mchp23k256_flash {
>    };
> 
>    #define MCHP23K256_CMD_WRITE_STATUS    0x01
> +#define MCHP23K256_CMD_WREN            0x06
>    #define MCHP23K256_CMD_WRITE           0x02
>    #define MCHP23K256_CMD_READ            0x03
>    #define MCHP23K256_MODE_SEQ            BIT(6)
> @@ -40,13 +41,14 @@ static int mchp23k256_write(struct mtd_info *mtd,
> loff_t to, size_t len,
>           struct mchp23k256_flash *flash = to_mchp23k256_flash(mtd);
>           struct spi_transfer transfer[2] = {};
>           struct spi_message message;
> -       unsigned char command[3];
> +       unsigned char command[4];
> 
>           spi_message_init(&message);
> 
>           command[0] = MCHP23K256_CMD_WRITE;
> -       command[1] = to >> 8;
> -       command[2] = to;
> +       command[1] = to >> 16;
> +       command[2] = to >> 8;
> +       command[3] = to;
> 
>           transfer[0].tx_buf = command;
>           transfer[0].len = sizeof(command);
> @@ -73,14 +75,15 @@ static int mchp23k256_read(struct mtd_info *mtd,
> loff_t from, size_t len,
>           struct mchp23k256_flash *flash = to_mchp23k256_flash(mtd);
>           struct spi_transfer transfer[2] = {};
>           struct spi_message message;
> -       unsigned char command[3];
> +       unsigned char command[4];
> 
>           spi_message_init(&message);
> 
>           memset(&transfer, 0, sizeof(transfer));
>           command[0] = MCHP23K256_CMD_READ;
> -       command[1] = from >> 8;
> -       command[2] = from;
> +       command[1] = from >> 16;
> +       command[2] = from >> 8;
> +       command[3] = from;
> 
>           transfer[0].tx_buf = command;
>           transfer[0].len = sizeof(command);
> @@ -104,17 +107,18 @@ static int mchp23k256_read(struct mtd_info *mtd,
> loff_t from, size_t len,
>    /*
>     * Set the device into sequential mode. This allows read/writes to the
>     * entire SRAM in a single operation
> + *
> + * CHANGE: Enable Write Mode in the device
>     */
>    static int mchp23k256_set_mode(struct spi_device *spi)
>    {
>           struct spi_transfer transfer = {};
>           struct spi_message message;
> -       unsigned char command[2];
> +       unsigned char command[1];
> 
>           spi_message_init(&message);
> 
> -       command[0] = MCHP23K256_CMD_WRITE_STATUS;
> -       command[1] = MCHP23K256_MODE_SEQ;
> +       command[0] = MCHP23K256_CMD_WREN;
> 
>           transfer.tx_buf = command;
>           transfer.len = sizeof(command);
> @@ -147,7 +151,7 @@ static int mchp23k256_probe(struct spi_device *spi)
>           flash->mtd.type         = MTD_RAM;
>           flash->mtd.flags        = MTD_CAP_RAM;
>           flash->mtd.writesize    = 1;
> -       flash->mtd.size         = SZ_32K;
> +       flash->mtd.size         = SZ_128K;
>           flash->mtd._read        = mchp23k256_read;
>           flash->mtd._write       = mchp23k256_write;
> 
> 
> What would be the best approach to add a more generic solution to the
> kernel in order to be able to address different SPI SRAM devices?

That's an interesting question. mchp23k256.c registers a RAM device 
(MTD_RAM), but nvSRAM contains RAM and flash and to the outside it acts 
more like flash as data is persistent.
The instructions seem to be very similar to EEPROM and SPI NOR devices.
So I'm curious about what others propose how to support these devices.

Regards,
Frieder
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: mtd: mchp23k256: How to follow a more generic approach?
  2019-02-18 13:02 ` Schrempf Frieder
@ 2019-02-19  8:15   ` Heinrich.Toews
  2019-02-19 14:47     ` Schrempf Frieder
  0 siblings, 1 reply; 6+ messages in thread
From: Heinrich.Toews @ 2019-02-19  8:15 UTC (permalink / raw)
  To: frieder.schrempf, linux-mtd; +Cc: Oleg.Karfich

Hi Frieder,

On 18.02.19 14:02, Schrempf Frieder wrote:
> Hi Heinrich,
> 
> On 18.02.19 13:33, Heinrich.Toews@wago.com wrote:
>> Hi altogether,
>>
>> I'm using currently the CONFIG_MTD_MCHP23K256 driver to access an
>> ANV32AA1W 1Mb Serial SPI nvSRAM from Anvo-Systems Dresden.
> 
> We also have a ANV32 nvSRAM chip on some of our boards and so far we
> used misc/eeprom/at25.c to access the memory.
> It would be nice to use the chip with the MTD framework too, so it's
> interesting to see, that it's possible with a modified version of
> mchp23k256.c.

We saw that the AT25 misc driver is limiting the access to a maximum of 
4k (PAGE_SIZE) data blocks and by this is increasing the data transfer 
overhead. In order to minimize the CPU utilization we considered to 
write the whole 128k of data during one SPI transfer. By this using the 
MCHP23K256 we reached 70ms for one mtdchar_write operation which is 
quite good for the driver spends most of the time in DMA (68ms).

>>
>> I did some changes to the driver as seen below to make it work.
>>
>>
>> diff --git a/drivers/mtd/devices/mchp23k256.c
>> b/drivers/mtd/devices/mchp23k256.c
>> index 9d8306a..6140973 100644
>> --- a/drivers/mtd/devices/mchp23k256.c
>> +++ b/drivers/mtd/devices/mchp23k256.c
>> @@ -28,6 +28,7 @@ struct mchp23k256_flash {
>>     };
>>
>>     #define MCHP23K256_CMD_WRITE_STATUS    0x01
>> +#define MCHP23K256_CMD_WREN            0x06
>>     #define MCHP23K256_CMD_WRITE           0x02
>>     #define MCHP23K256_CMD_READ            0x03
>>     #define MCHP23K256_MODE_SEQ            BIT(6)
>> @@ -40,13 +41,14 @@ static int mchp23k256_write(struct mtd_info *mtd,
>> loff_t to, size_t len,
>>            struct mchp23k256_flash *flash = to_mchp23k256_flash(mtd);
>>            struct spi_transfer transfer[2] = {};
>>            struct spi_message message;
>> -       unsigned char command[3];
>> +       unsigned char command[4];
>>
>>            spi_message_init(&message);
>>
>>            command[0] = MCHP23K256_CMD_WRITE;
>> -       command[1] = to >> 8;
>> -       command[2] = to;
>> +       command[1] = to >> 16;
>> +       command[2] = to >> 8;
>> +       command[3] = to;
>>
>>            transfer[0].tx_buf = command;
>>            transfer[0].len = sizeof(command);
>> @@ -73,14 +75,15 @@ static int mchp23k256_read(struct mtd_info *mtd,
>> loff_t from, size_t len,
>>            struct mchp23k256_flash *flash = to_mchp23k256_flash(mtd);
>>            struct spi_transfer transfer[2] = {};
>>            struct spi_message message;
>> -       unsigned char command[3];
>> +       unsigned char command[4];
>>
>>            spi_message_init(&message);
>>
>>            memset(&transfer, 0, sizeof(transfer));
>>            command[0] = MCHP23K256_CMD_READ;
>> -       command[1] = from >> 8;
>> -       command[2] = from;
>> +       command[1] = from >> 16;
>> +       command[2] = from >> 8;
>> +       command[3] = from;
>>
>>            transfer[0].tx_buf = command;
>>            transfer[0].len = sizeof(command);
>> @@ -104,17 +107,18 @@ static int mchp23k256_read(struct mtd_info *mtd,
>> loff_t from, size_t len,
>>     /*
>>      * Set the device into sequential mode. This allows read/writes to the
>>      * entire SRAM in a single operation
>> + *
>> + * CHANGE: Enable Write Mode in the device
>>      */
>>     static int mchp23k256_set_mode(struct spi_device *spi)
>>     {
>>            struct spi_transfer transfer = {};
>>            struct spi_message message;
>> -       unsigned char command[2];
>> +       unsigned char command[1];
>>
>>            spi_message_init(&message);
>>
>> -       command[0] = MCHP23K256_CMD_WRITE_STATUS;
>> -       command[1] = MCHP23K256_MODE_SEQ;
>> +       command[0] = MCHP23K256_CMD_WREN;
>>
>>            transfer.tx_buf = command;
>>            transfer.len = sizeof(command);
>> @@ -147,7 +151,7 @@ static int mchp23k256_probe(struct spi_device *spi)
>>            flash->mtd.type         = MTD_RAM;
>>            flash->mtd.flags        = MTD_CAP_RAM;
>>            flash->mtd.writesize    = 1;
>> -       flash->mtd.size         = SZ_32K;
>> +       flash->mtd.size         = SZ_128K;
>>            flash->mtd._read        = mchp23k256_read;
>>            flash->mtd._write       = mchp23k256_write;
>>
>>
>> What would be the best approach to add a more generic solution to the
>> kernel in order to be able to address different SPI SRAM devices?
> 
> That's an interesting question. mchp23k256.c registers a RAM device
> (MTD_RAM), but nvSRAM contains RAM and flash and to the outside it acts
> more like flash as data is persistent.

The data is transferred through SPI directly into a SRAM and written 
only into the flash on power loss. So its basically a RAM connected by 
SPI and so MTD_RAM seems for me to be the right choice here.

> The instructions seem to be very similar to EEPROM and SPI NOR devices.
> So I'm curious about what others propose how to support these devices.

I'm planning to write the driver for the ANV32AA1W during the next weeks 
and it would be really great to have some feedback on the approach here.


Thanks,
Heinrich.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: mtd: mchp23k256: How to follow a more generic approach?
  2019-02-19  8:15   ` Heinrich.Toews
@ 2019-02-19 14:47     ` Schrempf Frieder
  2019-02-20  7:52       ` Boris Brezillon
  0 siblings, 1 reply; 6+ messages in thread
From: Schrempf Frieder @ 2019-02-19 14:47 UTC (permalink / raw)
  To: Heinrich.Toews, linux-mtd; +Cc: Oleg.Karfich

On 19.02.19 09:15, Heinrich.Toews@wago.com wrote:
> Hi Frieder,
> 
> On 18.02.19 14:02, Schrempf Frieder wrote:
>> Hi Heinrich,
>>
>> On 18.02.19 13:33, Heinrich.Toews@wago.com wrote:
>>> Hi altogether,
>>>
>>> I'm using currently the CONFIG_MTD_MCHP23K256 driver to access an
>>> ANV32AA1W 1Mb Serial SPI nvSRAM from Anvo-Systems Dresden.
>>
>> We also have a ANV32 nvSRAM chip on some of our boards and so far we
>> used misc/eeprom/at25.c to access the memory.
>> It would be nice to use the chip with the MTD framework too, so it's
>> interesting to see, that it's possible with a modified version of
>> mchp23k256.c.
> 
> We saw that the AT25 misc driver is limiting the access to a maximum of
> 4k (PAGE_SIZE) data blocks and by this is increasing the data transfer
> overhead.

Right, this is something that we have observed, too.

> In order to minimize the CPU utilization we considered to
> write the whole 128k of data during one SPI transfer. By this using the
> MCHP23K256 we reached 70ms for one mtdchar_write operation which is
> quite good for the driver spends most of the time in DMA (68ms).
> 
>>>
>>> I did some changes to the driver as seen below to make it work.
>>>
>>>
>>> diff --git a/drivers/mtd/devices/mchp23k256.c
>>> b/drivers/mtd/devices/mchp23k256.c
>>> index 9d8306a..6140973 100644
>>> --- a/drivers/mtd/devices/mchp23k256.c
>>> +++ b/drivers/mtd/devices/mchp23k256.c
>>> @@ -28,6 +28,7 @@ struct mchp23k256_flash {
>>>      };
>>>
>>>      #define MCHP23K256_CMD_WRITE_STATUS    0x01
>>> +#define MCHP23K256_CMD_WREN            0x06
>>>      #define MCHP23K256_CMD_WRITE           0x02
>>>      #define MCHP23K256_CMD_READ            0x03
>>>      #define MCHP23K256_MODE_SEQ            BIT(6)
>>> @@ -40,13 +41,14 @@ static int mchp23k256_write(struct mtd_info *mtd,
>>> loff_t to, size_t len,
>>>             struct mchp23k256_flash *flash = to_mchp23k256_flash(mtd);
>>>             struct spi_transfer transfer[2] = {};
>>>             struct spi_message message;
>>> -       unsigned char command[3];
>>> +       unsigned char command[4];
>>>
>>>             spi_message_init(&message);
>>>
>>>             command[0] = MCHP23K256_CMD_WRITE;
>>> -       command[1] = to >> 8;
>>> -       command[2] = to;
>>> +       command[1] = to >> 16;
>>> +       command[2] = to >> 8;
>>> +       command[3] = to;
>>>
>>>             transfer[0].tx_buf = command;
>>>             transfer[0].len = sizeof(command);
>>> @@ -73,14 +75,15 @@ static int mchp23k256_read(struct mtd_info *mtd,
>>> loff_t from, size_t len,
>>>             struct mchp23k256_flash *flash = to_mchp23k256_flash(mtd);
>>>             struct spi_transfer transfer[2] = {};
>>>             struct spi_message message;
>>> -       unsigned char command[3];
>>> +       unsigned char command[4];
>>>
>>>             spi_message_init(&message);
>>>
>>>             memset(&transfer, 0, sizeof(transfer));
>>>             command[0] = MCHP23K256_CMD_READ;
>>> -       command[1] = from >> 8;
>>> -       command[2] = from;
>>> +       command[1] = from >> 16;
>>> +       command[2] = from >> 8;
>>> +       command[3] = from;
>>>
>>>             transfer[0].tx_buf = command;
>>>             transfer[0].len = sizeof(command);
>>> @@ -104,17 +107,18 @@ static int mchp23k256_read(struct mtd_info *mtd,
>>> loff_t from, size_t len,
>>>      /*
>>>       * Set the device into sequential mode. This allows read/writes to the
>>>       * entire SRAM in a single operation
>>> + *
>>> + * CHANGE: Enable Write Mode in the device
>>>       */
>>>      static int mchp23k256_set_mode(struct spi_device *spi)
>>>      {
>>>             struct spi_transfer transfer = {};
>>>             struct spi_message message;
>>> -       unsigned char command[2];
>>> +       unsigned char command[1];
>>>
>>>             spi_message_init(&message);
>>>
>>> -       command[0] = MCHP23K256_CMD_WRITE_STATUS;
>>> -       command[1] = MCHP23K256_MODE_SEQ;
>>> +       command[0] = MCHP23K256_CMD_WREN;
>>>
>>>             transfer.tx_buf = command;
>>>             transfer.len = sizeof(command);
>>> @@ -147,7 +151,7 @@ static int mchp23k256_probe(struct spi_device *spi)
>>>             flash->mtd.type         = MTD_RAM;
>>>             flash->mtd.flags        = MTD_CAP_RAM;
>>>             flash->mtd.writesize    = 1;
>>> -       flash->mtd.size         = SZ_32K;
>>> +       flash->mtd.size         = SZ_128K;
>>>             flash->mtd._read        = mchp23k256_read;
>>>             flash->mtd._write       = mchp23k256_write;
>>>
>>>
>>> What would be the best approach to add a more generic solution to the
>>> kernel in order to be able to address different SPI SRAM devices?
>>
>> That's an interesting question. mchp23k256.c registers a RAM device
>> (MTD_RAM), but nvSRAM contains RAM and flash and to the outside it acts
>> more like flash as data is persistent.
> 
> The data is transferred through SPI directly into a SRAM and written
> only into the flash on power loss. So its basically a RAM connected by
> SPI and so MTD_RAM seems for me to be the right choice here.

Hm, sounds reasonable.

> 
>> The instructions seem to be very similar to EEPROM and SPI NOR devices.
>> So I'm curious about what others propose how to support these devices.
> 
> I'm planning to write the driver for the ANV32AA1W during the next weeks
> and it would be really great to have some feedback on the approach here.

Maybe you can create a new generic SPI RAM driver (I don't think 
something like this exists yet), that can be used with things like SRAM, 
nvSRAM, FRAM, etc.?

At the time when a generic driver exists, we could use it to replace the 
mchp23k256 driver.

If you write a new driver or convert an existing one to support the 
nvSRAM chips, please use the SPI MEM interface (drivers/spi/spi-mem.c). 
This would enable us to share some code and connect those chips to all 
kinds of controllers that support this interface (which are not only 
generic SPI controllers, but also some QSPI controllers, that don't 
support generic SPI transfers).

Thanks,
Frieder
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: mtd: mchp23k256: How to follow a more generic approach?
  2019-02-19 14:47     ` Schrempf Frieder
@ 2019-02-20  7:52       ` Boris Brezillon
  2019-02-20  8:07         ` Schrempf Frieder
  0 siblings, 1 reply; 6+ messages in thread
From: Boris Brezillon @ 2019-02-20  7:52 UTC (permalink / raw)
  To: Schrempf Frieder; +Cc: Heinrich.Toews, Oleg.Karfich, linux-mtd

On Tue, 19 Feb 2019 14:47:08 +0000
Schrempf Frieder <frieder.schrempf@kontron.de> wrote:

> >   
> >> The instructions seem to be very similar to EEPROM and SPI NOR devices.
> >> So I'm curious about what others propose how to support these devices.  
> > 
> > I'm planning to write the driver for the ANV32AA1W during the next weeks
> > and it would be really great to have some feedback on the approach here.  
> 
> Maybe you can create a new generic SPI RAM driver (I don't think 
> something like this exists yet), that can be used with things like SRAM, 
> nvSRAM, FRAM, etc.?

Creating a new framework makes sense if you have enough to share
between all those drivers. I might be wrong (didn't look at the code)
but I think SRAM devices are simple enough to not require a new
mid-layer (especially if they use a different cmdset). What we could do
though is group them in drivers/mtd/sram/.

> 
> At the time when a generic driver exists, we could use it to replace the 
> mchp23k256 driver.
> 
> If you write a new driver or convert an existing one to support the 
> nvSRAM chips, please use the SPI MEM interface (drivers/spi/spi-mem.c). 
> This would enable us to share some code and connect those chips to all 
> kinds of controllers that support this interface (which are not only 
> generic SPI controllers, but also some QSPI controllers, that don't 
> support generic SPI transfers).

Yes, please use the spi-mem layer (note that I already started the
conversion of at25 here [1]).

[1]https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1904223.html

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: mtd: mchp23k256: How to follow a more generic approach?
  2019-02-20  7:52       ` Boris Brezillon
@ 2019-02-20  8:07         ` Schrempf Frieder
  0 siblings, 0 replies; 6+ messages in thread
From: Schrempf Frieder @ 2019-02-20  8:07 UTC (permalink / raw)
  To: Boris Brezillon; +Cc: Heinrich.Toews, Oleg.Karfich, linux-mtd

On 20.02.19 08:52, Boris Brezillon wrote:
> On Tue, 19 Feb 2019 14:47:08 +0000
> Schrempf Frieder <frieder.schrempf@kontron.de> wrote:
> 
>>>    
>>>> The instructions seem to be very similar to EEPROM and SPI NOR devices.
>>>> So I'm curious about what others propose how to support these devices.
>>>
>>> I'm planning to write the driver for the ANV32AA1W during the next weeks
>>> and it would be really great to have some feedback on the approach here.
>>
>> Maybe you can create a new generic SPI RAM driver (I don't think
>> something like this exists yet), that can be used with things like SRAM,
>> nvSRAM, FRAM, etc.?
> 
> Creating a new framework makes sense if you have enough to share
> between all those drivers. I might be wrong (didn't look at the code)
> but I think SRAM devices are simple enough to not require a new
> mid-layer (especially if they use a different cmdset). What we could do
> though is group them in drivers/mtd/sram/.

It's just that it seems they are using a common basic command set (at 
least the ones that I have looked at) which seems to consist of READ 
(0x03), WRITE (0x02), READ_STATUS (0x05), WRITE_STATUS (0x01).
Some of them might provide additional commands for write protection or 
dual/quad access.
So I wouldn't consider it a "framework", but more a generic driver that 
supports this basic command set to connect to all of those chips.

> 
>>
>> At the time when a generic driver exists, we could use it to replace the
>> mchp23k256 driver.
>>
>> If you write a new driver or convert an existing one to support the
>> nvSRAM chips, please use the SPI MEM interface (drivers/spi/spi-mem.c).
>> This would enable us to share some code and connect those chips to all
>> kinds of controllers that support this interface (which are not only
>> generic SPI controllers, but also some QSPI controllers, that don't
>> support generic SPI transfers).
> 
> Yes, please use the spi-mem layer (note that I already started the
> conversion of at25 here [1]).
> 
> [1]https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1904223.html
> 
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

end of thread, other threads:[~2019-02-20  8:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-18 12:33 mtd: mchp23k256: How to follow a more generic approach? Heinrich.Toews
2019-02-18 13:02 ` Schrempf Frieder
2019-02-19  8:15   ` Heinrich.Toews
2019-02-19 14:47     ` Schrempf Frieder
2019-02-20  7:52       ` Boris Brezillon
2019-02-20  8:07         ` Schrempf Frieder

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).