[PATCH v3 0/5] mtd: mchp23k256: device tree and mchp23lcv1024

[PATCH v3 0/5] mtd: mchp23k256: device tree and mchp23lcv1024

[Chris Packham]

This series adds device tree support to the mchp23k256 driver and
support for the mchp23lcv1024 chip. I suspect there are more compatible
variants that we could now enumerate if desired.

Chris Packham (5):
  mtd: mchp23k256: Add OF device ID table
  mtd: mchp23k256: switch to mtd_device_register()
  mtd: handle partitioning on devices with 0 erasesize
  mtd: mchp23k256: add partitioning support
  mtd: mchp23k256: Add support for mchp23lcv1024

 .../bindings/mtd/microchip,mchp23k256.txt          | 18 ++++++
 drivers/mtd/devices/mchp23k256.c                   | 65 ++++++++++++++++++----
 drivers/mtd/mtdpart.c                              | 25 ++++++---
 3 files changed, 87 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt

-- 
2.11.0.24.ge6920cf

[PATCH v3 1/5] mtd: mchp23k256: Add OF device ID table

[Chris Packham]

This allows registering of this device via a Device Tree.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Andrew Lunn <andrew@lunn.ch>
---
Changes in v2:
- collect review/test from Andrew
Changes in v3:
- None

 .../devicetree/bindings/mtd/microchip,mchp23k256.txt   | 18 ++++++++++++++++++
 drivers/mtd/devices/mchp23k256.c                       |  8 ++++++++
 2 files changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt

diff --git a/Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt b/Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt
new file mode 100644
index 000000000000..25e5ad38b0f0
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt
@@ -0,0 +1,18 @@
+* MTD SPI driver for Microchip 23K256 (and similar) serial SRAM
+
+Required properties:
+- #address-cells, #size-cells : Must be present if the device has sub-nodes
+  representing partitions.
+- compatible : Must be "microchip,mchp23k256"
+- reg : Chip-Select number
+- spi-max-frequency : Maximum frequency of the SPI bus the chip can operate at
+
+Example:
+
+	spi-sram@0 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "microchip,mchp23k256";
+		reg = <0>;
+		spi-max-frequency = <20000000>;
+	};
diff --git a/drivers/mtd/devices/mchp23k256.c b/drivers/mtd/devices/mchp23k256.c
index e237db9f1bdb..9d8306a15833 100644
--- a/drivers/mtd/devices/mchp23k256.c
+++ b/drivers/mtd/devices/mchp23k256.c
@@ -19,6 +19,7 @@
 #include <linux/sizes.h>
 #include <linux/spi/flash.h>
 #include <linux/spi/spi.h>
+#include <linux/of_device.h>
 
 struct mchp23k256_flash {
 	struct spi_device	*spi;
@@ -166,9 +167,16 @@ static int mchp23k256_remove(struct spi_device *spi)
 	return mtd_device_unregister(&flash->mtd);
 }
 
+static const struct of_device_id mchp23k256_of_table[] = {
+	{ .compatible = "microchip,mchp23k256" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, mchp23k256_of_table);
+
 static struct spi_driver mchp23k256_driver = {
 	.driver = {
 		.name	= "mchp23k256",
+		.of_match_table = of_match_ptr(mchp23k256_of_table),
 	},
 	.probe		= mchp23k256_probe,
 	.remove		= mchp23k256_remove,
-- 
2.11.0.24.ge6920cf

[PATCH v3 2/5] mtd: mchp23k256: switch to mtd_device_register()

[Chris Packham]

Use mtd_device_register() instead of mtd_device_parse_register() to
eliminate two unused parameters.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Andrew Lunn <andrew@lunn.ch>
---
Changes in v2
- collect review/test from Andrew
Changes in v3:
- None

 drivers/mtd/devices/mchp23k256.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/devices/mchp23k256.c b/drivers/mtd/devices/mchp23k256.c
index 9d8306a15833..2542f5b8b63f 100644
--- a/drivers/mtd/devices/mchp23k256.c
+++ b/drivers/mtd/devices/mchp23k256.c
@@ -151,9 +151,8 @@ static int mchp23k256_probe(struct spi_device *spi)
 	flash->mtd._read	= mchp23k256_read;
 	flash->mtd._write	= mchp23k256_write;
 
-	err = mtd_device_parse_register(&flash->mtd, NULL, NULL,
-					data ? data->parts : NULL,
-					data ? data->nr_parts : 0);
+	err = mtd_device_register(&flash->mtd, data ? data->parts : NULL,
+				  data ? data->nr_parts : 0);
 	if (err)
 		return err;
 
-- 
2.11.0.24.ge6920cf

[PATCH v3 3/5] mtd: handle partitioning on devices with 0 erasesize

[Chris Packham]

erasesize is meaningful for flash devices but for SRAM there is no
concept of an erase block so erasesize is set to 0. When partitioning
these devices instead of ensuring partitions fall on erasesize
boundaries we ensure they fall on writesize boundaries.

Helped-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
Changes in v3:
- new

 drivers/mtd/mtdpart.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index ea5e5307f667..5cef1247806c 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -393,8 +393,11 @@ static struct mtd_part *allocate_partition(struct mtd_info *master,
 			const struct mtd_partition *part, int partno,
 			uint64_t cur_offset)
 {
+	int wr_alignment = master->erasesize ? : master->writesize;
 	struct mtd_part *slave;
+	u32 remainder;
 	char *name;
+	u64 tmp;
 
 	/* allocate the partition structure */
 	slave = kzalloc(sizeof(*slave), GFP_KERNEL);
@@ -499,10 +502,11 @@ static struct mtd_part *allocate_partition(struct mtd_info *master,
 	if (slave->offset == MTDPART_OFS_APPEND)
 		slave->offset = cur_offset;
 	if (slave->offset == MTDPART_OFS_NXTBLK) {
+		tmp = cur_offset;
 		slave->offset = cur_offset;
-		if (mtd_mod_by_eb(cur_offset, master) != 0) {
-			/* Round up to next erasesize */
-			slave->offset = (mtd_div_by_eb(cur_offset, master) + 1) * master->erasesize;
+		remainder = do_div(tmp, wr_alignment);
+		if (remainder) {
+			slave->offset += wr_alignment - remainder;
 			printk(KERN_NOTICE "Moving partition %d: "
 			       "0x%012llx -> 0x%012llx\n", partno,
 			       (unsigned long long)cur_offset, (unsigned long long)slave->offset);
@@ -567,19 +571,22 @@ static struct mtd_part *allocate_partition(struct mtd_info *master,
 		slave->mtd.erasesize = master->erasesize;
 	}
 
-	if ((slave->mtd.flags & MTD_WRITEABLE) &&
-	    mtd_mod_by_eb(slave->offset, &slave->mtd)) {
+	tmp = slave->offset;
+	remainder = do_div(tmp, wr_alignment);
+	if ((slave->mtd.flags & MTD_WRITEABLE) && remainder) {
 		/* Doesn't start on a boundary of major erase size */
 		/* FIXME: Let it be writable if it is on a boundary of
 		 * _minor_ erase size though */
 		slave->mtd.flags &= ~MTD_WRITEABLE;
-		printk(KERN_WARNING"mtd: partition \"%s\" doesn't start on an erase block boundary -- force read-only\n",
+		printk(KERN_WARNING"mtd: partition \"%s\" doesn't start on an erase/write block boundary -- force read-only\n",
 			part->name);
 	}
-	if ((slave->mtd.flags & MTD_WRITEABLE) &&
-	    mtd_mod_by_eb(slave->mtd.size, &slave->mtd)) {
+
+	tmp = slave->mtd.size;
+	remainder = do_div(tmp, wr_alignment);
+	if ((slave->mtd.flags & MTD_WRITEABLE) && remainder) {
 		slave->mtd.flags &= ~MTD_WRITEABLE;
-		printk(KERN_WARNING"mtd: partition \"%s\" doesn't end on an erase block -- force read-only\n",
+		printk(KERN_WARNING"mtd: partition \"%s\" doesn't end on an erase/write block -- force read-only\n",
 			part->name);
 	}
 
-- 
2.11.0.24.ge6920cf

[PATCH v3 4/5] mtd: mchp23k256: add partitioning support

[Chris Packham]

Setting the of_node for the mtd device allows the generic mtd code to
setup the partitions. Additionally we must specify a non-zero erasesize
for the partitions to be writeable.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Andrew Lunn <andrew@lunn.ch>
---
Changes in v2
- collect revew/test from Andrew
Changes in v3:
- remove setting of erasesize

 drivers/mtd/devices/mchp23k256.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/devices/mchp23k256.c b/drivers/mtd/devices/mchp23k256.c
index 2542f5b8b63f..3e5feb454644 100644
--- a/drivers/mtd/devices/mchp23k256.c
+++ b/drivers/mtd/devices/mchp23k256.c
@@ -143,6 +143,7 @@ static int mchp23k256_probe(struct spi_device *spi)
 
 	data = dev_get_platdata(&spi->dev);
 
+	mtd_set_of_node(&flash->mtd, spi->dev.of_node);
 	flash->mtd.dev.parent	= &spi->dev;
 	flash->mtd.type		= MTD_RAM;
 	flash->mtd.flags	= MTD_CAP_RAM;
-- 
2.11.0.24.ge6920cf

[PATCH v3 5/5] mtd: mchp23k256: Add support for mchp23lcv1024

[Chris Packham]

The mchp23lcv1024 is software compatible with the mchp23k256, the
only difference (from a software point of view) is the size. There
is no way to detect the size so we must be told via a Device Tree.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
Changes in v2:
- fix formatting in switch statement
- add support for 24-bit addressing
Changes in v3:
- None

 .../bindings/mtd/microchip,mchp23k256.txt          |  2 +-
 drivers/mtd/devices/mchp23k256.c                   | 53 ++++++++++++++++++----
 2 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt b/Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt
index 25e5ad38b0f0..7328eb92a03c 100644
--- a/Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt
+++ b/Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt
@@ -3,7 +3,7 @@
 Required properties:
 - #address-cells, #size-cells : Must be present if the device has sub-nodes
   representing partitions.
-- compatible : Must be "microchip,mchp23k256"
+- compatible : Must be one of "microchip,mchp23k256" or "microchip,mchp23lcv1024"
 - reg : Chip-Select number
 - spi-max-frequency : Maximum frequency of the SPI bus the chip can operate at
 
diff --git a/drivers/mtd/devices/mchp23k256.c b/drivers/mtd/devices/mchp23k256.c
index 3e5feb454644..72ecf374a06a 100644
--- a/drivers/mtd/devices/mchp23k256.c
+++ b/drivers/mtd/devices/mchp23k256.c
@@ -21,10 +21,14 @@
 #include <linux/spi/spi.h>
 #include <linux/of_device.h>
 
+#define MAX_CMD_SIZE		4
+enum chips { mchp23k256, mchp23lcv1024 };
+
 struct mchp23k256_flash {
 	struct spi_device	*spi;
 	struct mutex		lock;
 	struct mtd_info		mtd;
+	u8			addr_width;
 };
 
 #define MCHP23K256_CMD_WRITE_STATUS	0x01
@@ -34,22 +38,35 @@ struct mchp23k256_flash {
 
 #define to_mchp23k256_flash(x) container_of(x, struct mchp23k256_flash, mtd)
 
+static void mchp23k256_addr2cmd(struct mchp23k256_flash *flash,
+				unsigned int addr, u8 *cmd)
+{
+	/* cmd[0] has opcode */
+	cmd[1] = addr >> (flash->addr_width * 8 -  8);
+	cmd[2] = addr >> (flash->addr_width * 8 - 16);
+	cmd[3] = addr >> (flash->addr_width * 8 - 24);
+}
+
+static int mchp23k256_cmdsz(struct mchp23k256_flash *flash)
+{
+	return 1 + flash->addr_width;
+}
+
 static int mchp23k256_write(struct mtd_info *mtd, loff_t to, size_t len,
 			    size_t *retlen, const unsigned char *buf)
 {
 	struct mchp23k256_flash *flash = to_mchp23k256_flash(mtd);
 	struct spi_transfer transfer[2] = {};
 	struct spi_message message;
-	unsigned char command[3];
+	unsigned char command[MAX_CMD_SIZE];
 
 	spi_message_init(&message);
 
 	command[0] = MCHP23K256_CMD_WRITE;
-	command[1] = to >> 8;
-	command[2] = to;
+	mchp23k256_addr2cmd(flash, to, command);
 
 	transfer[0].tx_buf = command;
-	transfer[0].len = sizeof(command);
+	transfer[0].len = mchp23k256_cmdsz(flash);
 	spi_message_add_tail(&transfer[0], &message);
 
 	transfer[1].tx_buf = buf;
@@ -73,17 +90,16 @@ 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[MAX_CMD_SIZE];
 
 	spi_message_init(&message);
 
 	memset(&transfer, 0, sizeof(transfer));
 	command[0] = MCHP23K256_CMD_READ;
-	command[1] = from >> 8;
-	command[2] = from;
+	mchp23k256_addr2cmd(flash, from, command);
 
 	transfer[0].tx_buf = command;
-	transfer[0].len = sizeof(command);
+	transfer[0].len = mchp23k256_cmdsz(flash);
 	spi_message_add_tail(&transfer[0], &message);
 
 	transfer[1].rx_buf = buf;
@@ -128,6 +144,7 @@ static int mchp23k256_probe(struct spi_device *spi)
 	struct mchp23k256_flash *flash;
 	struct flash_platform_data *data;
 	int err;
+	enum chips chip;
 
 	flash = devm_kzalloc(&spi->dev, sizeof(*flash), GFP_KERNEL);
 	if (!flash)
@@ -143,15 +160,30 @@ static int mchp23k256_probe(struct spi_device *spi)
 
 	data = dev_get_platdata(&spi->dev);
 
+	if (spi->dev.of_node)
+		chip = (enum chips)of_device_get_match_data(&spi->dev);
+	else
+		chip = mchp23k256;
+
 	mtd_set_of_node(&flash->mtd, spi->dev.of_node);
 	flash->mtd.dev.parent	= &spi->dev;
 	flash->mtd.type		= MTD_RAM;
 	flash->mtd.flags	= MTD_CAP_RAM;
 	flash->mtd.writesize	= 1;
-	flash->mtd.size		= SZ_32K;
 	flash->mtd._read	= mchp23k256_read;
 	flash->mtd._write	= mchp23k256_write;
 
+	switch (chip) {
+	case mchp23lcv1024:
+		flash->mtd.size		= SZ_128K;
+		flash->addr_width	= 3;
+		break;
+	default:
+		flash->mtd.size		= SZ_32K;
+		flash->addr_width	= 2;
+		break;
+	}
+
 	err = mtd_device_register(&flash->mtd, data ? data->parts : NULL,
 				  data ? data->nr_parts : 0);
 	if (err)
@@ -168,7 +200,8 @@ static int mchp23k256_remove(struct spi_device *spi)
 }
 
 static const struct of_device_id mchp23k256_of_table[] = {
-	{ .compatible = "microchip,mchp23k256" },
+	{ .compatible = "microchip,mchp23k256", .data = (void *)mchp23k256 },
+	{ .compatible = "microchip,mchp23lcv1024", .data = (void *)mchp23lcv1024 },
 	{}
 };
 MODULE_DEVICE_TABLE(of, mchp23k256_of_table);
-- 
2.11.0.24.ge6920cf

Re: [PATCH v3 5/5] mtd: mchp23k256: Add support for mchp23lcv1024

[Andrew Lunn]

On Tue, May 23, 2017 at 12:43:17PM +1200, Chris Packham wrote:
> The mchp23lcv1024 is software compatible with the mchp23k256, the
> only difference (from a software point of view) is the size.

This is not really true. The size of the address is also different,
and the point of the v2 change.

> There
> is no way to detect the size so we must be told via a Device Tree.

or defaults to 256K when device tree is not used.

> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v3 5/5] mtd: mchp23k256: Add support for mchp23lcv1024

[Andrew Lunn]

On Tue, May 23, 2017 at 12:43:17PM +1200, Chris Packham wrote:
> The mchp23lcv1024 is software compatible with the mchp23k256, the
> only difference (from a software point of view) is the size.

This is not really true. The size of the address is also different,
and the point of the v2 change.

> There
> is no way to detect the size so we must be told via a Device Tree.

or defaults to 256K when device tree is not used.

> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

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

Re: [PATCH v3 5/5] mtd: mchp23k256: Add support for mchp23lcv1024

[Chris Packham]

On 23/05/17 12:58, Andrew Lunn wrote:
> On Tue, May 23, 2017 at 12:43:17PM +1200, Chris Packham wrote:
>> The mchp23lcv1024 is software compatible with the mchp23k256, the
>> only difference (from a software point of view) is the size.
> 
> This is not really true. The size of the address is also different,
> and the point of the v2 change.

Agreed. How about this revised commit message

--- 8< ---
mtd: mchp23k256: Add support for mchp23lcv1024

The mchp23lcv1024 is similar to the mchp23k256, the differences (from a 
software point of view) are the capacity of the chip and the size of the 
addresses used.

There is no way to detect the specific chip so we must be told via a 
Device Tree or default to mchp23k256 when device tree is not used.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
--- >8 ---

Can someone fixup the commit message or should I re-send?

> 
>> There
>> is no way to detect the size so we must be told via a Device Tree.
> 
> or defaults to 256K when device tree is not used.
> 
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
>      Andrew
> 

Re: [PATCH v3 5/5] mtd: mchp23k256: Add support for mchp23lcv1024

[Chris Packham]

On 23/05/17 12:58, Andrew Lunn wrote:
> On Tue, May 23, 2017 at 12:43:17PM +1200, Chris Packham wrote:
>> The mchp23lcv1024 is software compatible with the mchp23k256, the
>> only difference (from a software point of view) is the size.
> 
> This is not really true. The size of the address is also different,
> and the point of the v2 change.

Agreed. How about this revised commit message

--- 8< ---
mtd: mchp23k256: Add support for mchp23lcv1024

The mchp23lcv1024 is similar to the mchp23k256, the differences (from a 
software point of view) are the capacity of the chip and the size of the 
addresses used.

There is no way to detect the specific chip so we must be told via a 
Device Tree or default to mchp23k256 when device tree is not used.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
--- >8 ---

Can someone fixup the commit message or should I re-send?

> 
>> There
>> is no way to detect the size so we must be told via a Device Tree.
> 
> or defaults to 256K when device tree is not used.
> 
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
>      Andrew
> 

Re: [PATCH v3 5/5] mtd: mchp23k256: Add support for mchp23lcv1024

[Andrew Lunn]

> Agreed. How about this revised commit message
> 
> --- 8< ---
> mtd: mchp23k256: Add support for mchp23lcv1024
> 
> The mchp23lcv1024 is similar to the mchp23k256, the differences (from a 
> software point of view) are the capacity of the chip and the size of the 
> addresses used.
> 
> There is no way to detect the specific chip so we must be told via a 
> Device Tree or default to mchp23k256 when device tree is not used.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> --- >8 ---

Yes, that is good.

> Can someone fixup the commit message or should I re-send?

The less the maintainer has to do, the more likely he will do it.

But hold on a few days, and see if you get any more comments.

    Andrew

Re: [PATCH v3 5/5] mtd: mchp23k256: Add support for mchp23lcv1024

[Andrew Lunn]

> Agreed. How about this revised commit message
> 
> --- 8< ---
> mtd: mchp23k256: Add support for mchp23lcv1024
> 
> The mchp23lcv1024 is similar to the mchp23k256, the differences (from a 
> software point of view) are the capacity of the chip and the size of the 
> addresses used.
> 
> There is no way to detect the specific chip so we must be told via a 
> Device Tree or default to mchp23k256 when device tree is not used.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> --- >8 ---

Yes, that is good.

> Can someone fixup the commit message or should I re-send?

The less the maintainer has to do, the more likely he will do it.

But hold on a few days, and see if you get any more comments.

    Andrew

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

Re: [PATCH v3 5/5] mtd: mchp23k256: Add support for mchp23lcv1024

[Andrew Lunn]

> Agreed. How about this revised commit message
> 
> --- 8< ---
> mtd: mchp23k256: Add support for mchp23lcv1024
> 
> The mchp23lcv1024 is similar to the mchp23k256, the differences (from a 
> software point of view) are the capacity of the chip and the size of the 
> addresses used.
> 
> There is no way to detect the specific chip so we must be told via a 
> Device Tree or default to mchp23k256 when device tree is not used.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> --- >8 ---

Yes, that is good.

> Can someone fixup the commit message or should I re-send?

The less the maintainer has to do, the more likely he will do it.

But hold on a few days, and see if you get any more comments.

    Andrew

Re: [PATCH v3 5/5] mtd: mchp23k256: Add support for mchp23lcv1024

[Boris Brezillon]

Le Tue, 23 May 2017 12:43:17 +1200,
Chris Packham <chris.packham@alliedtelesis.co.nz> a écrit :

> The mchp23lcv1024 is software compatible with the mchp23k256, the
> only difference (from a software point of view) is the size. There
> is no way to detect the size so we must be told via a Device Tree.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> Changes in v2:
> - fix formatting in switch statement
> - add support for 24-bit addressing
> Changes in v3:
> - None
> 
>  .../bindings/mtd/microchip,mchp23k256.txt          |  2 +-
>  drivers/mtd/devices/mchp23k256.c                   | 53 ++++++++++++++++++----
>  2 files changed, 44 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt b/Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt
> index 25e5ad38b0f0..7328eb92a03c 100644
> --- a/Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt
> +++ b/Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt
> @@ -3,7 +3,7 @@
>  Required properties:
>  - #address-cells, #size-cells : Must be present if the device has sub-nodes
>    representing partitions.
> -- compatible : Must be "microchip,mchp23k256"
> +- compatible : Must be one of "microchip,mchp23k256" or "microchip,mchp23lcv1024"
>  - reg : Chip-Select number
>  - spi-max-frequency : Maximum frequency of the SPI bus the chip can operate at
>  
> diff --git a/drivers/mtd/devices/mchp23k256.c b/drivers/mtd/devices/mchp23k256.c
> index 3e5feb454644..72ecf374a06a 100644
> --- a/drivers/mtd/devices/mchp23k256.c
> +++ b/drivers/mtd/devices/mchp23k256.c
> @@ -21,10 +21,14 @@
>  #include <linux/spi/spi.h>
>  #include <linux/of_device.h>
>  
> +#define MAX_CMD_SIZE		4
> +enum chips { mchp23k256, mchp23lcv1024 };
> +
>  struct mchp23k256_flash {
>  	struct spi_device	*spi;
>  	struct mutex		lock;
>  	struct mtd_info		mtd;
> +	u8			addr_width;
>  };

How about creating a struct mchp23_caps or mchp23_specs struct
containing the SRAM specs.

struct mchp23_caps {
	u8 addr_width;
	unsigned int size;
}

and then

struct mchp23k256_flash {
	...
	const struct mchp32_cap	*caps;
};

This way you can get rid of the enum and add new fields to the caps
struct if needed.

BTW, it's really weird to have the _flash extension in the struct name,
while we're actually dealing with SRAMs.

>  
>  #define MCHP23K256_CMD_WRITE_STATUS	0x01
> @@ -34,22 +38,35 @@ struct mchp23k256_flash {
>  
>  #define to_mchp23k256_flash(x) container_of(x, struct mchp23k256_flash, mtd)
>  
> +static void mchp23k256_addr2cmd(struct mchp23k256_flash *flash,
> +				unsigned int addr, u8 *cmd)
> +{
> +	/* cmd[0] has opcode */
> +	cmd[1] = addr >> (flash->addr_width * 8 -  8);
> +	cmd[2] = addr >> (flash->addr_width * 8 - 16);
> +	cmd[3] = addr >> (flash->addr_width * 8 - 24);

or

	int i;

	/*
	 * Address is sent in big endian (MSB first) and we skip
	 * the first entry of the cmd array which contains the cmd
	 * opcode.
	 */
	for (i = flash->caps->addr_width; i--, addr >>= 8; i > 0)
		cmd[i] = addr;

> +}
> +
> +static int mchp23k256_cmdsz(struct mchp23k256_flash *flash)
> +{
> +	return 1 + flash->addr_width;
> +}
> +
>  static int mchp23k256_write(struct mtd_info *mtd, loff_t to, size_t len,
>  			    size_t *retlen, const unsigned char *buf)
>  {
>  	struct mchp23k256_flash *flash = to_mchp23k256_flash(mtd);
>  	struct spi_transfer transfer[2] = {};
>  	struct spi_message message;
> -	unsigned char command[3];
> +	unsigned char command[MAX_CMD_SIZE];
>  
>  	spi_message_init(&message);
>  
>  	command[0] = MCHP23K256_CMD_WRITE;
> -	command[1] = to >> 8;
> -	command[2] = to;
> +	mchp23k256_addr2cmd(flash, to, command);
>  
>  	transfer[0].tx_buf = command;
> -	transfer[0].len = sizeof(command);
> +	transfer[0].len = mchp23k256_cmdsz(flash);
>  	spi_message_add_tail(&transfer[0], &message);
>  
>  	transfer[1].tx_buf = buf;
> @@ -73,17 +90,16 @@ 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[MAX_CMD_SIZE];
>  
>  	spi_message_init(&message);
>  
>  	memset(&transfer, 0, sizeof(transfer));
>  	command[0] = MCHP23K256_CMD_READ;
> -	command[1] = from >> 8;
> -	command[2] = from;
> +	mchp23k256_addr2cmd(flash, from, command);
>  
>  	transfer[0].tx_buf = command;
> -	transfer[0].len = sizeof(command);
> +	transfer[0].len = mchp23k256_cmdsz(flash);
>  	spi_message_add_tail(&transfer[0], &message);
>  
>  	transfer[1].rx_buf = buf;

static const struct mchp23_caps mchp23k256_caps = {
	.size = SZ_32K,
	.addr_width = 2;
};

static const struct mchp23_caps mchp23lcv1024_caps = {
	.size = SZ_128K,
	.addr_width = 3;
};

> @@ -128,6 +144,7 @@ static int mchp23k256_probe(struct spi_device *spi)
>  	struct mchp23k256_flash *flash;
>  	struct flash_platform_data *data;
>  	int err;
> +	enum chips chip;
>  
>  	flash = devm_kzalloc(&spi->dev, sizeof(*flash), GFP_KERNEL);
>  	if (!flash)
> @@ -143,15 +160,30 @@ static int mchp23k256_probe(struct spi_device *spi)
>  
>  	data = dev_get_platdata(&spi->dev);
>  
> +	if (spi->dev.of_node)
> +		chip = (enum chips)of_device_get_match_data(&spi->dev);
> +	else
> +		chip = mchp23k256;

	flash->caps = of_device_get_match_data(&spi->dev);
	if (!flash->caps)
		flash->caps = mchp23k256_caps;

> +
>  	mtd_set_of_node(&flash->mtd, spi->dev.of_node);
>  	flash->mtd.dev.parent	= &spi->dev;
>  	flash->mtd.type		= MTD_RAM;
>  	flash->mtd.flags	= MTD_CAP_RAM;
>  	flash->mtd.writesize	= 1;
> -	flash->mtd.size		= SZ_32K;
>  	flash->mtd._read	= mchp23k256_read;
>  	flash->mtd._write	= mchp23k256_write;
>  
> +	switch (chip) {
> +	case mchp23lcv1024:
> +		flash->mtd.size		= SZ_128K;
> +		flash->addr_width	= 3;
> +		break;
> +	default:
> +		flash->mtd.size		= SZ_32K;
> +		flash->addr_width	= 2;
> +		break;
> +	}
> +
>  	err = mtd_device_register(&flash->mtd, data ? data->parts : NULL,
>  				  data ? data->nr_parts : 0);
>  	if (err)
> @@ -168,7 +200,8 @@ static int mchp23k256_remove(struct spi_device *spi)
>  }
>  
>  static const struct of_device_id mchp23k256_of_table[] = {
> -	{ .compatible = "microchip,mchp23k256" },
> +	{ .compatible = "microchip,mchp23k256", .data = (void *)mchp23k256 },
> +	{ .compatible = "microchip,mchp23lcv1024", .data = (void *)mchp23lcv1024 },

	{
		.compatible = "microchip,mchp23k256",
		.data = &mchp23k256_caps,
	},
	{
		.compatible = "microchip,mchp23lcv1024",
		.data = &mchp23lcv1024_caps,
	},
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, mchp23k256_of_table);

Re: [PATCH v3 5/5] mtd: mchp23k256: Add support for mchp23lcv1024

[Boris Brezillon]

Le Tue, 23 May 2017 12:43:17 +1200,
Chris Packham <chris.packham-6g8wRflRTwXFdCa3tKVlE6U/zSkkHjvu@public.gmane.org> a écrit :

> The mchp23lcv1024 is software compatible with the mchp23k256, the
> only difference (from a software point of view) is the size. There
> is no way to detect the size so we must be told via a Device Tree.
> 
> Signed-off-by: Chris Packham <chris.packham-6g8wRflRTwXFdCa3tKVlE6U/zSkkHjvu@public.gmane.org>
> ---
> Changes in v2:
> - fix formatting in switch statement
> - add support for 24-bit addressing
> Changes in v3:
> - None
> 
>  .../bindings/mtd/microchip,mchp23k256.txt          |  2 +-
>  drivers/mtd/devices/mchp23k256.c                   | 53 ++++++++++++++++++----
>  2 files changed, 44 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt b/Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt
> index 25e5ad38b0f0..7328eb92a03c 100644
> --- a/Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt
> +++ b/Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt
> @@ -3,7 +3,7 @@
>  Required properties:
>  - #address-cells, #size-cells : Must be present if the device has sub-nodes
>    representing partitions.
> -- compatible : Must be "microchip,mchp23k256"
> +- compatible : Must be one of "microchip,mchp23k256" or "microchip,mchp23lcv1024"
>  - reg : Chip-Select number
>  - spi-max-frequency : Maximum frequency of the SPI bus the chip can operate at
>  
> diff --git a/drivers/mtd/devices/mchp23k256.c b/drivers/mtd/devices/mchp23k256.c
> index 3e5feb454644..72ecf374a06a 100644
> --- a/drivers/mtd/devices/mchp23k256.c
> +++ b/drivers/mtd/devices/mchp23k256.c
> @@ -21,10 +21,14 @@
>  #include <linux/spi/spi.h>
>  #include <linux/of_device.h>
>  
> +#define MAX_CMD_SIZE		4
> +enum chips { mchp23k256, mchp23lcv1024 };
> +
>  struct mchp23k256_flash {
>  	struct spi_device	*spi;
>  	struct mutex		lock;
>  	struct mtd_info		mtd;
> +	u8			addr_width;
>  };

How about creating a struct mchp23_caps or mchp23_specs struct
containing the SRAM specs.

struct mchp23_caps {
	u8 addr_width;
	unsigned int size;
}

and then

struct mchp23k256_flash {
	...
	const struct mchp32_cap	*caps;
};

This way you can get rid of the enum and add new fields to the caps
struct if needed.

BTW, it's really weird to have the _flash extension in the struct name,
while we're actually dealing with SRAMs.

>  
>  #define MCHP23K256_CMD_WRITE_STATUS	0x01
> @@ -34,22 +38,35 @@ struct mchp23k256_flash {
>  
>  #define to_mchp23k256_flash(x) container_of(x, struct mchp23k256_flash, mtd)
>  
> +static void mchp23k256_addr2cmd(struct mchp23k256_flash *flash,
> +				unsigned int addr, u8 *cmd)
> +{
> +	/* cmd[0] has opcode */
> +	cmd[1] = addr >> (flash->addr_width * 8 -  8);
> +	cmd[2] = addr >> (flash->addr_width * 8 - 16);
> +	cmd[3] = addr >> (flash->addr_width * 8 - 24);

or

	int i;

	/*
	 * Address is sent in big endian (MSB first) and we skip
	 * the first entry of the cmd array which contains the cmd
	 * opcode.
	 */
	for (i = flash->caps->addr_width; i--, addr >>= 8; i > 0)
		cmd[i] = addr;

> +}
> +
> +static int mchp23k256_cmdsz(struct mchp23k256_flash *flash)
> +{
> +	return 1 + flash->addr_width;
> +}
> +
>  static int mchp23k256_write(struct mtd_info *mtd, loff_t to, size_t len,
>  			    size_t *retlen, const unsigned char *buf)
>  {
>  	struct mchp23k256_flash *flash = to_mchp23k256_flash(mtd);
>  	struct spi_transfer transfer[2] = {};
>  	struct spi_message message;
> -	unsigned char command[3];
> +	unsigned char command[MAX_CMD_SIZE];
>  
>  	spi_message_init(&message);
>  
>  	command[0] = MCHP23K256_CMD_WRITE;
> -	command[1] = to >> 8;
> -	command[2] = to;
> +	mchp23k256_addr2cmd(flash, to, command);
>  
>  	transfer[0].tx_buf = command;
> -	transfer[0].len = sizeof(command);
> +	transfer[0].len = mchp23k256_cmdsz(flash);
>  	spi_message_add_tail(&transfer[0], &message);
>  
>  	transfer[1].tx_buf = buf;
> @@ -73,17 +90,16 @@ 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[MAX_CMD_SIZE];
>  
>  	spi_message_init(&message);
>  
>  	memset(&transfer, 0, sizeof(transfer));
>  	command[0] = MCHP23K256_CMD_READ;
> -	command[1] = from >> 8;
> -	command[2] = from;
> +	mchp23k256_addr2cmd(flash, from, command);
>  
>  	transfer[0].tx_buf = command;
> -	transfer[0].len = sizeof(command);
> +	transfer[0].len = mchp23k256_cmdsz(flash);
>  	spi_message_add_tail(&transfer[0], &message);
>  
>  	transfer[1].rx_buf = buf;

static const struct mchp23_caps mchp23k256_caps = {
	.size = SZ_32K,
	.addr_width = 2;
};

static const struct mchp23_caps mchp23lcv1024_caps = {
	.size = SZ_128K,
	.addr_width = 3;
};

> @@ -128,6 +144,7 @@ static int mchp23k256_probe(struct spi_device *spi)
>  	struct mchp23k256_flash *flash;
>  	struct flash_platform_data *data;
>  	int err;
> +	enum chips chip;
>  
>  	flash = devm_kzalloc(&spi->dev, sizeof(*flash), GFP_KERNEL);
>  	if (!flash)
> @@ -143,15 +160,30 @@ static int mchp23k256_probe(struct spi_device *spi)
>  
>  	data = dev_get_platdata(&spi->dev);
>  
> +	if (spi->dev.of_node)
> +		chip = (enum chips)of_device_get_match_data(&spi->dev);
> +	else
> +		chip = mchp23k256;

	flash->caps = of_device_get_match_data(&spi->dev);
	if (!flash->caps)
		flash->caps = mchp23k256_caps;

> +
>  	mtd_set_of_node(&flash->mtd, spi->dev.of_node);
>  	flash->mtd.dev.parent	= &spi->dev;
>  	flash->mtd.type		= MTD_RAM;
>  	flash->mtd.flags	= MTD_CAP_RAM;
>  	flash->mtd.writesize	= 1;
> -	flash->mtd.size		= SZ_32K;
>  	flash->mtd._read	= mchp23k256_read;
>  	flash->mtd._write	= mchp23k256_write;
>  
> +	switch (chip) {
> +	case mchp23lcv1024:
> +		flash->mtd.size		= SZ_128K;
> +		flash->addr_width	= 3;
> +		break;
> +	default:
> +		flash->mtd.size		= SZ_32K;
> +		flash->addr_width	= 2;
> +		break;
> +	}
> +
>  	err = mtd_device_register(&flash->mtd, data ? data->parts : NULL,
>  				  data ? data->nr_parts : 0);
>  	if (err)
> @@ -168,7 +200,8 @@ static int mchp23k256_remove(struct spi_device *spi)
>  }
>  
>  static const struct of_device_id mchp23k256_of_table[] = {
> -	{ .compatible = "microchip,mchp23k256" },
> +	{ .compatible = "microchip,mchp23k256", .data = (void *)mchp23k256 },
> +	{ .compatible = "microchip,mchp23lcv1024", .data = (void *)mchp23lcv1024 },

	{
		.compatible = "microchip,mchp23k256",
		.data = &mchp23k256_caps,
	},
	{
		.compatible = "microchip,mchp23lcv1024",
		.data = &mchp23lcv1024_caps,
	},
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, mchp23k256_of_table);

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/5] mtd: mchp23k256: device tree and mchp23lcv1024

[Boris Brezillon]

On Tue, 23 May 2017 12:43:12 +1200
Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:

> This series adds device tree support to the mchp23k256 driver and
> support for the mchp23lcv1024 chip. I suspect there are more compatible
> variants that we could now enumerate if desired.
> 
> Chris Packham (5):
>   mtd: mchp23k256: Add OF device ID table
>   mtd: mchp23k256: switch to mtd_device_register()
>   mtd: handle partitioning on devices with 0 erasesize
>   mtd: mchp23k256: add partitioning support
>   mtd: mchp23k256: Add support for mchp23lcv1024

Shouldn't we add a spi_device_id table to allow non-DT users to probe
mchp23lcv1024 chips?

> 
>  .../bindings/mtd/microchip,mchp23k256.txt          | 18 ++++++
>  drivers/mtd/devices/mchp23k256.c                   | 65 ++++++++++++++++++----
>  drivers/mtd/mtdpart.c                              | 25 ++++++---
>  3 files changed, 87 insertions(+), 21 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt
> 

Re: [PATCH v3 0/5] mtd: mchp23k256: device tree and mchp23lcv1024

[Andrew Lunn]

On Tue, May 23, 2017 at 09:39:57AM +0200, Boris Brezillon wrote:
> On Tue, 23 May 2017 12:43:12 +1200
> Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:
> 
> > This series adds device tree support to the mchp23k256 driver and
> > support for the mchp23lcv1024 chip. I suspect there are more compatible
> > variants that we could now enumerate if desired.
> > 
> > Chris Packham (5):
> >   mtd: mchp23k256: Add OF device ID table
> >   mtd: mchp23k256: switch to mtd_device_register()
> >   mtd: handle partitioning on devices with 0 erasesize
> >   mtd: mchp23k256: add partitioning support
> >   mtd: mchp23k256: Add support for mchp23lcv1024
> 
> Shouldn't we add a spi_device_id table to allow non-DT users to probe
> mchp23lcv1024 chips?

Hi Boris

This is something which can be added at any time. Currently there is
no need for it, so i personally would wait until somebody does need
it.

	Andrew

Re: [PATCH v3 5/5] mtd: mchp23k256: Add support for mchp23lcv1024

[Chris Packham]

On 23/05/17 19:19, Boris Brezillon wrote:
> BTW, it's really weird to have the _flash extension in the struct name,
> while we're actually dealing with SRAMs.

Agreed. After the dust settles on this series I'll send a trivial patch 
to change "flash" to something sensible like "sram" or "chip".

Re: [PATCH v3 5/5] mtd: mchp23k256: Add support for mchp23lcv1024

[Chris Packham]

On 23/05/17 19:19, Boris Brezillon wrote:
> BTW, it's really weird to have the _flash extension in the struct name,
> while we're actually dealing with SRAMs.

Agreed. After the dust settles on this series I'll send a trivial patch 
to change "flash" to something sensible like "sram" or "chip".

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