2017-06-02 03:22:18

by Chris Packham

[permalink] [raw]
Subject: [PATCH v5 0/5] mtd: mchp23k256: device tree and mchp23lcv1024

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.

Note: I've included 2 patches that have already been applied to l2-mtd.git for
context and so as not to upset the kbuild bot.

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 | 78 ++++++++++++++++++----
drivers/mtd/mtdpart.c | 25 ++++---
3 files changed, 100 insertions(+), 21 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt

--
2.13.0


2017-06-02 03:21:44

by Chris Packham

[permalink] [raw]
Subject: [PATCH v5 2/5] mtd: mchp23k256: switch to mtd_device_register()

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

Signed-off-by: Chris Packham <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Tested-by: Andrew Lunn <[email protected]>
---
Changes in v2
- collect review/test from Andrew
Changes in v3:
- None
Changes in v4:
- None
Changes in v5:
- None (note this has already been applied to l2-mtd.git)

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.13.0

2017-06-02 03:21:34

by Chris Packham

[permalink] [raw]
Subject: [PATCH v5 1/5] mtd: mchp23k256: Add OF device ID table

This allows registering of this device via a Device Tree.

Signed-off-by: Chris Packham <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Tested-by: Andrew Lunn <[email protected]>
---
Changes in v2:
- collect review/test from Andrew
Changes in v3:
- None
Changes in v4:
- None
Changes in v5:
- None (note this has already been applied to l2-mtd.git)

.../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.13.0

2017-06-02 03:21:32

by Chris Packham

[permalink] [raw]
Subject: [PATCH v5 5/5] 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 <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
Changes in v2:
- fix formatting in switch statement
- add support for 24-bit addressing
Changes in v3:
- None
Changes in v4:
- reword commit message (thanks Andrew)
- introduce mchp23_caps struct (thanks Boris)
Changes in v5:
- collect ack from Rob

.../bindings/mtd/microchip,mchp23k256.txt | 2 +-
drivers/mtd/devices/mchp23k256.c | 66 ++++++++++++++++++----
2 files changed, 57 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..8956b7dcc984 100644
--- a/drivers/mtd/devices/mchp23k256.c
+++ b/drivers/mtd/devices/mchp23k256.c
@@ -21,10 +21,18 @@
#include <linux/spi/spi.h>
#include <linux/of_device.h>

+#define MAX_CMD_SIZE 4
+
+struct mchp23_caps {
+ u8 addr_width;
+ unsigned int size;
+};
+
struct mchp23k256_flash {
struct spi_device *spi;
struct mutex lock;
struct mtd_info mtd;
+ const struct mchp23_caps *caps;
};

#define MCHP23K256_CMD_WRITE_STATUS 0x01
@@ -34,22 +42,40 @@ 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)
+{
+ 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 > 0; i--, addr >>= 8)
+ cmd[i] = addr;
+}
+
+static int mchp23k256_cmdsz(struct mchp23k256_flash *flash)
+{
+ return 1 + flash->caps->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 +99,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;
@@ -123,6 +148,16 @@ static int mchp23k256_set_mode(struct spi_device *spi)
return spi_sync(spi, &message);
}

+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,
+};
+
static int mchp23k256_probe(struct spi_device *spi)
{
struct mchp23k256_flash *flash;
@@ -143,12 +178,16 @@ static int mchp23k256_probe(struct spi_device *spi)

data = dev_get_platdata(&spi->dev);

+ 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.size = flash->caps->size;
flash->mtd._read = mchp23k256_read;
flash->mtd._write = mchp23k256_write;

@@ -168,7 +207,14 @@ static int mchp23k256_remove(struct spi_device *spi)
}

static const struct of_device_id mchp23k256_of_table[] = {
- { .compatible = "microchip,mchp23k256" },
+ {
+ .compatible = "microchip,mchp23k256",
+ .data = &mchp23k256_caps,
+ },
+ {
+ .compatible = "microchip,mchp23lcv1024",
+ .data = &mchp23lcv1024_caps,
+ },
{}
};
MODULE_DEVICE_TABLE(of, mchp23k256_of_table);
--
2.13.0

2017-06-02 03:21:31

by Chris Packham

[permalink] [raw]
Subject: [PATCH v5 3/5] mtd: handle partitioning on devices with 0 erasesize

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 <[email protected]>
Signed-off-by: Chris Packham <[email protected]>
---
Changes in v3:
- new
Changes in v4:
- None
Changes in v5:
- None (yet). There is some active discussion on this so it may change.
patch 4/5 is somewhat dependent on this but only if partitions are
specified on the dt node.

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.13.0

2017-06-02 03:22:31

by Chris Packham

[permalink] [raw]
Subject: [PATCH v5 4/5] mtd: mchp23k256: add partitioning support

Setting the of_node for the mtd device allows the generic mtd code to
setup the partitions.

Signed-off-by: Chris Packham <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Tested-by: Andrew Lunn <[email protected]>
---
Changes in v2
- collect revew/test from Andrew
Changes in v3:
- remove setting of erasesize
Changes in v4:
- none
Changes in v5:
- update commit message

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.13.0

2017-06-02 05:36:44

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH v5 0/5] mtd: mchp23k256: device tree and mchp23lcv1024

Hi Chris,

Le Fri, 2 Jun 2017 15:21:14 +1200,
Chris Packham <[email protected]> a écrit :

> 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.
>
> Note: I've included 2 patches that have already been applied to l2-mtd.git for
> context and so as not to upset the kbuild bot.
>
> Chris Packham (5):
> mtd: mchp23k256: Add OF device ID table
> mtd: mchp23k256: switch to mtd_device_register()

You should drop patches that have already been applied when sending a
new version.

> 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 | 78 ++++++++++++++++++----
> drivers/mtd/mtdpart.c | 25 ++++---
> 3 files changed, 100 insertions(+), 21 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt
>

2017-06-08 23:27:02

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v5 3/5] mtd: handle partitioning on devices with 0 erasesize

On Fri, Jun 02, 2017 at 03:21:17PM +1200, Chris Packham wrote:
> 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 <[email protected]>
> Signed-off-by: Chris Packham <[email protected]>
> ---
> Changes in v3:
> - new
> Changes in v4:
> - None
> Changes in v5:
> - None (yet). There is some active discussion on this so it may change.
> patch 4/5 is somewhat dependent on this but only if partitions are
> specified on the dt node.
>
> 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;

I'd prefer this be checked based on the MTD_NO_ERASE flag, as the
erasesize is essentially undefined. That might also help other drivers,
which might currently fake an erasesize just to satisfy this check. Now,
they won't have to.

i.e.

int wr_alignment = (master->flags & MTD_NO_ERASE) ? master->erasesize :
master->writesize;

or similar.

> struct mtd_part *slave;
> + u32 remainder;
> char *name;
> + u64 tmp;
>
> /* allocate the partition structure */
> slave = kzalloc(sizeof(*slave), GFP_KERNEL);

...

Brian

2017-06-08 23:33:51

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v5 4/5] mtd: mchp23k256: add partitioning support

On Fri, Jun 02, 2017 at 03:21:18PM +1200, Chris Packham wrote:
> Setting the of_node for the mtd device allows the generic mtd code to
> setup the partitions.
>
> Signed-off-by: Chris Packham <[email protected]>
> Reviewed-by: Andrew Lunn <[email protected]>
> Tested-by: Andrew Lunn <[email protected]>

Applied patches 4 and 5, with an editorial note on this one, to note the
outstanding issue about fixing up the eraseblock-boundary check in the
partitioning code.

1 and 2 were already applied, and I had a change request on patch 3.

Regards,
Brian

2017-06-08 23:40:30

by Chris Packham

[permalink] [raw]
Subject: Re: [PATCH v5 4/5] mtd: mchp23k256: add partitioning support

On 09/06/17 11:33, Brian Norris wrote:
> On Fri, Jun 02, 2017 at 03:21:18PM +1200, Chris Packham wrote:
>> Setting the of_node for the mtd device allows the generic mtd code to
>> setup the partitions.
>>
>> Signed-off-by: Chris Packham <[email protected]>
>> Reviewed-by: Andrew Lunn <[email protected]>
>> Tested-by: Andrew Lunn <[email protected]>
>
> Applied patches 4 and 5, with an editorial note on this one, to note the
> outstanding issue about fixing up the eraseblock-boundary check in the
> partitioning code.
>
> 1 and 2 were already applied, and I had a change request on patch 3.

Thanks Brian,

I'll take a look at the suggestions on patch 3.


2017-07-04 03:23:00

by Chris Packham

[permalink] [raw]
Subject: Re: [PATCH v5 3/5] mtd: handle partitioning on devices with 0 erasesize

Hi,

On 02/06/17 15:21, Chris Packham wrote:
> 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 <[email protected]>
> Signed-off-by: Chris Packham <[email protected]>

I had someone mention to me in passing that mtdinfo was failing for them
(crashing with some floating point error). I'm wondering if we've
created a divide-by-zero problem by reporting 0 erase size in /proc/mtd.
I don't have any other info and right now I don't have access to the
system I had with the mchp23lcv1024 sram.

Andrew, do you still have access to your device?

2017-07-08 01:14:23

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v5 3/5] mtd: handle partitioning on devices with 0 erasesize

On Tue, Jul 04, 2017 at 03:22:52AM +0000, Chris Packham wrote:
> I had someone mention to me in passing that mtdinfo was failing for them
> (crashing with some floating point error). I'm wondering if we've
> created a divide-by-zero problem by reporting 0 erase size in /proc/mtd.
> I don't have any other info and right now I don't have access to the
> system I had with the mchp23lcv1024 sram.

Wouldn't be that surprising. mtdinfo is actually developed under the
"ubi-utils" directory of mtd-utils, and UBI wouldn't know what to do
with and SRAM like that.

Brian