Re: [PATCH v3 2/3] mtd: spi-nor: Move m25p80 code in spi-nor.c

2019-08-01 Thread Vignesh Raghavendra



On 01/08/19 11:22 AM, Boris Brezillon wrote:
> On Thu, 1 Aug 2019 10:00:51 +0530
> Vignesh Raghavendra  wrote:
> 
>> From: Boris Brezillon 
>>
>> The m25p80 driver is actually a generic wrapper around the spi-mem
>> layer. Not only the driver name is misleading, but we'd expect such a
>> common logic to be directly available in the core. Another reason for
>> moving this code is that SPI NOR controller drivers should
>> progressively be replaced by SPI controller drivers implementing the
>> spi_mem_ops interface, and when the conversion is done, we should have
>> a single spi-nor driver directly interfacing with the spi-mem layer.
>>
>> While moving the code we also fix a longstanding issue when
>> non-DMA-able buffers are passed by the MTD layer.
>>
>> Signed-off-by: Boris Brezillon 
>> Signed-off-by: Vignesh Raghavendra 
>> ---
>> v3:
>> Simplify register read/write by dropping spi_nor_exec_op() and using
>> spi_mem_exec_op() directly
>> Modify spi_nor_spimem_xfer_data() to drop "enum spi_nor_protocol proto"
>> Fix misc coding style comments by Tudor
>>
>> v2:
>> Add docs for new functions added
>> Add spi_nor_ prefix to new functions
>> Incorporate Andrey's patches https://lkml.org/lkml/2019/4/1/32
>> to avoid looping spi_nor_spimem_* APIs
>>
>>  drivers/mtd/devices/Kconfig   |  18 -
>>  drivers/mtd/devices/Makefile  |   1 -
>>  drivers/mtd/devices/m25p80.c  | 347 ---
>>  drivers/mtd/spi-nor/Kconfig   |   2 +
>>  drivers/mtd/spi-nor/spi-nor.c | 632 --
>>  include/linux/mtd/spi-nor.h   |   3 +
>>  6 files changed, 604 insertions(+), 399 deletions(-)
>>  delete mode 100644 drivers/mtd/devices/m25p80.c
>>
> 
> [...]
> 
> 
>> @@ -348,6 +530,16 @@ static int read_cr(struct spi_nor *nor)
>>   */
>>  static int write_sr(struct spi_nor *nor, u8 val)
>>  {
>> +if (nor->spimem) {
>> +struct spi_mem_op op =
>> +SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRSR, 1),
>> +   SPI_MEM_OP_NO_ADDR,
>> +   SPI_MEM_OP_NO_DUMMY,
>> +   SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
>> +
>> +return spi_mem_exec_op(nor->spimem, );
>> +}
>> +
>>  nor->bouncebuf[0] = val;
> 
> The above line should be moved at the beginning of the function if you
> want the spimem path to work correctly.


Good catch! will send v4 with this fixed


-- 
Regards
Vignesh


Re: [PATCH v3 2/3] mtd: spi-nor: Move m25p80 code in spi-nor.c

2019-07-31 Thread Boris Brezillon
On Thu, 1 Aug 2019 10:00:51 +0530
Vignesh Raghavendra  wrote:

> From: Boris Brezillon 
> 
> The m25p80 driver is actually a generic wrapper around the spi-mem
> layer. Not only the driver name is misleading, but we'd expect such a
> common logic to be directly available in the core. Another reason for
> moving this code is that SPI NOR controller drivers should
> progressively be replaced by SPI controller drivers implementing the
> spi_mem_ops interface, and when the conversion is done, we should have
> a single spi-nor driver directly interfacing with the spi-mem layer.
> 
> While moving the code we also fix a longstanding issue when
> non-DMA-able buffers are passed by the MTD layer.
> 
> Signed-off-by: Boris Brezillon 
> Signed-off-by: Vignesh Raghavendra 
> ---
> v3:
> Simplify register read/write by dropping spi_nor_exec_op() and using
> spi_mem_exec_op() directly
> Modify spi_nor_spimem_xfer_data() to drop "enum spi_nor_protocol proto"
> Fix misc coding style comments by Tudor
> 
> v2:
> Add docs for new functions added
> Add spi_nor_ prefix to new functions
> Incorporate Andrey's patches https://lkml.org/lkml/2019/4/1/32
> to avoid looping spi_nor_spimem_* APIs
> 
>  drivers/mtd/devices/Kconfig   |  18 -
>  drivers/mtd/devices/Makefile  |   1 -
>  drivers/mtd/devices/m25p80.c  | 347 ---
>  drivers/mtd/spi-nor/Kconfig   |   2 +
>  drivers/mtd/spi-nor/spi-nor.c | 632 --
>  include/linux/mtd/spi-nor.h   |   3 +
>  6 files changed, 604 insertions(+), 399 deletions(-)
>  delete mode 100644 drivers/mtd/devices/m25p80.c
> 

[...]


> @@ -348,6 +530,16 @@ static int read_cr(struct spi_nor *nor)
>   */
>  static int write_sr(struct spi_nor *nor, u8 val)
>  {
> + if (nor->spimem) {
> + struct spi_mem_op op =
> + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRSR, 1),
> +SPI_MEM_OP_NO_ADDR,
> +SPI_MEM_OP_NO_DUMMY,
> +SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
> +
> + return spi_mem_exec_op(nor->spimem, );
> + }
> +
>   nor->bouncebuf[0] = val;

The above line should be moved at the beginning of the function if you
want the spimem path to work correctly.

>   return nor->write_reg(nor, SPINOR_OP_WRSR, nor->bouncebuf, 1);
>  }




[PATCH v3 2/3] mtd: spi-nor: Move m25p80 code in spi-nor.c

2019-07-31 Thread Vignesh Raghavendra
From: Boris Brezillon 

The m25p80 driver is actually a generic wrapper around the spi-mem
layer. Not only the driver name is misleading, but we'd expect such a
common logic to be directly available in the core. Another reason for
moving this code is that SPI NOR controller drivers should
progressively be replaced by SPI controller drivers implementing the
spi_mem_ops interface, and when the conversion is done, we should have
a single spi-nor driver directly interfacing with the spi-mem layer.

While moving the code we also fix a longstanding issue when
non-DMA-able buffers are passed by the MTD layer.

Signed-off-by: Boris Brezillon 
Signed-off-by: Vignesh Raghavendra 
---
v3:
Simplify register read/write by dropping spi_nor_exec_op() and using
spi_mem_exec_op() directly
Modify spi_nor_spimem_xfer_data() to drop "enum spi_nor_protocol proto"
Fix misc coding style comments by Tudor

v2:
Add docs for new functions added
Add spi_nor_ prefix to new functions
Incorporate Andrey's patches https://lkml.org/lkml/2019/4/1/32
to avoid looping spi_nor_spimem_* APIs

 drivers/mtd/devices/Kconfig   |  18 -
 drivers/mtd/devices/Makefile  |   1 -
 drivers/mtd/devices/m25p80.c  | 347 ---
 drivers/mtd/spi-nor/Kconfig   |   2 +
 drivers/mtd/spi-nor/spi-nor.c | 632 --
 include/linux/mtd/spi-nor.h   |   3 +
 6 files changed, 604 insertions(+), 399 deletions(-)
 delete mode 100644 drivers/mtd/devices/m25p80.c

diff --git a/drivers/mtd/devices/Kconfig b/drivers/mtd/devices/Kconfig
index 49abbc52457d..f96287c4b789 100644
--- a/drivers/mtd/devices/Kconfig
+++ b/drivers/mtd/devices/Kconfig
@@ -79,24 +79,6 @@ config MTD_DATAFLASH_OTP
  other key product data.  The second half is programmed with a
  unique-to-each-chip bit pattern at the factory.
 
-config MTD_M25P80
-   tristate "Support most SPI Flash chips (AT26DF, M25P, W25X, ...)"
-   depends on SPI_MASTER && MTD_SPI_NOR
-   select SPI_MEM
-   help
- This enables access to most modern SPI flash chips, used for
- program and data storage.   Series supported include Atmel AT26DF,
- Spansion S25SL, SST 25VF, ST M25P, and Winbond W25X.  Other chips
- are supported as well.  See the driver source for the current list,
- or to add other chips.
-
- Note that the original DataFlash chips (AT45 series, not AT26DF),
- need an entirely different driver.
-
- Set up your spi devices with the right board-specific platform data,
- if you want to specify device partitioning or to use a device which
- doesn't support the JEDEC ID instruction.
-
 config MTD_MCHP23K256
tristate "Microchip 23K256 SRAM"
depends on SPI_MASTER
diff --git a/drivers/mtd/devices/Makefile b/drivers/mtd/devices/Makefile
index 94895eab3066..991c8d12c016 100644
--- a/drivers/mtd/devices/Makefile
+++ b/drivers/mtd/devices/Makefile
@@ -12,7 +12,6 @@ obj-$(CONFIG_MTD_MTDRAM)  += mtdram.o
 obj-$(CONFIG_MTD_LART) += lart.o
 obj-$(CONFIG_MTD_BLOCK2MTD)+= block2mtd.o
 obj-$(CONFIG_MTD_DATAFLASH)+= mtd_dataflash.o
-obj-$(CONFIG_MTD_M25P80)   += m25p80.o
 obj-$(CONFIG_MTD_MCHP23K256)   += mchp23k256.o
 obj-$(CONFIG_MTD_SPEAR_SMI)+= spear_smi.o
 obj-$(CONFIG_MTD_SST25L)   += sst25l.o
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
deleted file mode 100644
index c50888670250..
--- a/drivers/mtd/devices/m25p80.c
+++ /dev/null
@@ -1,347 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * MTD SPI driver for ST M25Pxx (and similar) serial flash chips
- *
- * Author: Mike Lavender, m...@steroidmicros.com
- *
- * Copyright (c) 2005, Intec Automation Inc.
- *
- * Some parts are based on lart.c by Abraham Van Der Merwe
- *
- * Cleaned up and generalized based on mtd_dataflash.c
- */
-
-#include 
-#include 
-#include 
-#include 
-
-#include 
-#include 
-
-#include 
-#include 
-#include 
-#include 
-
-struct m25p {
-   struct spi_mem  *spimem;
-   struct spi_nor  spi_nor;
-};
-
-static int m25p80_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len)
-{
-   struct m25p *flash = nor->priv;
-   struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(code, 1),
- SPI_MEM_OP_NO_ADDR,
- SPI_MEM_OP_NO_DUMMY,
- SPI_MEM_OP_DATA_IN(len, NULL, 1));
-   void *scratchbuf;
-   int ret;
-
-   scratchbuf = kmalloc(len, GFP_KERNEL);
-   if (!scratchbuf)
-   return -ENOMEM;
-
-   op.data.buf.in = scratchbuf;
-   ret = spi_mem_exec_op(flash->spimem, );
-   if (ret < 0)
-   dev_err(>spimem->spi->dev, "error %d reading %x\n", ret,
-   code);
-   else
-   memcpy(val, scratchbuf, len);
-
-   kfree(scratchbuf);
-
-   return ret;
-}
-
-static int m25p80_write_reg(struct spi_nor