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

[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 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

______________________________________________________
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

[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

[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 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/