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

[PATCH 0/4] 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 (4):
  mtd: mchp23k256: Add OF device ID table
  mtd: mchp23k256: switch to mtd_device_register()
  mtd: mchp23k256: add partitioning support
  mtd: mchp23k256: Add support for mchp23lcv1024

 .../bindings/mtd/microchip,mchp23k256.txt          | 18 +++++++++++
 drivers/mtd/devices/mchp23k256.c                   | 37 +++++++++++++++++++---
 2 files changed, 51 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/microchip,mchp23k256.txt

-- 
2.11.0.24.ge6920cf

[PATCH 1/4] 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>
---
 .../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

Re: [PATCH 1/4] mtd: mchp23k256: Add OF device ID table

[Andrew Lunn]

On Wed, May 17, 2017 at 05:39:05PM +1200, Chris Packham wrote:
> 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>

    Andrew

[PATCH 2/4] 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>
---
 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

Re: [PATCH 2/4] mtd: mchp23k256: switch to mtd_device_register()

[Andrew Lunn]

On Wed, May 17, 2017 at 05:39:06PM +1200, Chris Packham wrote:
> 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>

    Andrew

[PATCH 3/4] 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>
---
 drivers/mtd/devices/mchp23k256.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mtd/devices/mchp23k256.c b/drivers/mtd/devices/mchp23k256.c
index 2542f5b8b63f..02c6b9dcbd3e 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;
@@ -151,6 +152,10 @@ static int mchp23k256_probe(struct spi_device *spi)
 	flash->mtd._read	= mchp23k256_read;
 	flash->mtd._write	= mchp23k256_write;
 
+	flash->mtd.erasesize = PAGE_SIZE;
+	while (flash->mtd.size & (flash->mtd.erasesize - 1))
+		flash->mtd.erasesize >>= 1;
+
 	err = mtd_device_register(&flash->mtd, data ? data->parts : NULL,
 				  data ? data->nr_parts : 0);
 	if (err)
-- 
2.11.0.24.ge6920cf

Re: [PATCH 3/4] mtd: mchp23k256: add partitioning support

[Andrew Lunn]

On Wed, May 17, 2017 at 05:39:07PM +1200, Chris Packham wrote:
> 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>

    Andrew

Re: [PATCH 3/4] mtd: mchp23k256: add partitioning support

[Boris Brezillon]

Hi Chris,

On Wed, 17 May 2017 17:39:07 +1200
Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:

> 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>
> ---
>  drivers/mtd/devices/mchp23k256.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/mtd/devices/mchp23k256.c b/drivers/mtd/devices/mchp23k256.c
> index 2542f5b8b63f..02c6b9dcbd3e 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;
> @@ -151,6 +152,10 @@ static int mchp23k256_probe(struct spi_device *spi)
>  	flash->mtd._read	= mchp23k256_read;
>  	flash->mtd._write	= mchp23k256_write;
>  
> +	flash->mtd.erasesize = PAGE_SIZE;
> +	while (flash->mtd.size & (flash->mtd.erasesize - 1))
> +		flash->mtd.erasesize >>= 1;
> +

Can we fix allocate_partition() to properly handle the
master->erasesize == 0 case instead of doing that?

Thanks,

Boris

Re: [PATCH 3/4] mtd: mchp23k256: add partitioning support

[Chris Packham]

On 18/05/17 03:29, Boris Brezillon wrote:
> Hi Chris,
> 
> On Wed, 17 May 2017 17:39:07 +1200
> Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:
> 
>> 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>
>> ---
>>   drivers/mtd/devices/mchp23k256.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/mtd/devices/mchp23k256.c b/drivers/mtd/devices/mchp23k256.c
>> index 2542f5b8b63f..02c6b9dcbd3e 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;
>> @@ -151,6 +152,10 @@ static int mchp23k256_probe(struct spi_device *spi)
>>   	flash->mtd._read	= mchp23k256_read;
>>   	flash->mtd._write	= mchp23k256_write;
>>   
>> +	flash->mtd.erasesize = PAGE_SIZE;
>> +	while (flash->mtd.size & (flash->mtd.erasesize - 1))
>> +		flash->mtd.erasesize >>= 1;
>> +
> 
> Can we fix allocate_partition() to properly handle the
> master->erasesize == 0 case instead of doing that?
> 

Do you mean something like this?

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index ea5e5307f667..0cd20ed6b374 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -577,6 +577,7 @@ static struct mtd_part *allocate_partition(struct 
mtd_info *master,
                         part->name);
         }
         if ((slave->mtd.flags & MTD_WRITEABLE) &&
+           master->erasesize != 0 &&
             mtd_mod_by_eb(slave->mtd.size, &slave->mtd)) {
                 slave->mtd.flags &= ~MTD_WRITEABLE;


I'm happy to submit this as a formal patch but it could potentially 
affect a number of devices. Whereas the snippet I initially added is 
consistent with drivers/mtd/chips/map_ram.c.

For now I'll leave v2 as-is but I can send a v3 if needed.

Re: [PATCH 3/4] mtd: mchp23k256: add partitioning support

[Boris Brezillon]

Hi Chris,

On Mon, 22 May 2017 04:52:34 +0000
Chris Packham <Chris.Packham@alliedtelesis.co.nz> wrote:

> On 18/05/17 03:29, Boris Brezillon wrote:
> > Hi Chris,
> > 
> > On Wed, 17 May 2017 17:39:07 +1200
> > Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:
> >   
> >> 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>
> >> ---
> >>   drivers/mtd/devices/mchp23k256.c | 5 +++++
> >>   1 file changed, 5 insertions(+)
> >>
> >> diff --git a/drivers/mtd/devices/mchp23k256.c b/drivers/mtd/devices/mchp23k256.c
> >> index 2542f5b8b63f..02c6b9dcbd3e 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;
> >> @@ -151,6 +152,10 @@ static int mchp23k256_probe(struct spi_device *spi)
> >>   	flash->mtd._read	= mchp23k256_read;
> >>   	flash->mtd._write	= mchp23k256_write;
> >>   
> >> +	flash->mtd.erasesize = PAGE_SIZE;
> >> +	while (flash->mtd.size & (flash->mtd.erasesize - 1))
> >> +		flash->mtd.erasesize >>= 1;
> >> +  
> > 
> > Can we fix allocate_partition() to properly handle the
> > master->erasesize == 0 case instead of doing that?
> >   
> 
> Do you mean something like this?

I had something slightly different in mind (see below).

> 
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index ea5e5307f667..0cd20ed6b374 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -577,6 +577,7 @@ static struct mtd_part *allocate_partition(struct 
> mtd_info *master,
>                          part->name);
>          }
>          if ((slave->mtd.flags & MTD_WRITEABLE) &&
> +           master->erasesize != 0 &&
>              mtd_mod_by_eb(slave->mtd.size, &slave->mtd)) {
>                  slave->mtd.flags &= ~MTD_WRITEABLE;
> 
> 
> I'm happy to submit this as a formal patch but it could potentially 
> affect a number of devices. Whereas the snippet I initially added is 
> consistent with drivers/mtd/chips/map_ram.c.

Well, if you're duplicating a workaround that's a good sign this should
actually be handled in the core.

> 
> For now I'll leave v2 as-is but I can send a v3 if needed.
> 

--->8---
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index ea5e5307f667..378ff4a9174e 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -393,7 +393,9 @@ 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;
 
        /* allocate the partition structure */
@@ -499,10 +501,12 @@ 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) {
+               u64 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);
        }
 

Re: [PATCH 3/4] mtd: mchp23k256: add partitioning support

[Brian Norris]

On Wed, May 17, 2017 at 05:29:11PM +0200, Boris Brezillon wrote:
> Hi Chris,
> 
> On Wed, 17 May 2017 17:39:07 +1200
> Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:
> 
> > 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>
> > ---
> >  drivers/mtd/devices/mchp23k256.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/mtd/devices/mchp23k256.c b/drivers/mtd/devices/mchp23k256.c
> > index 2542f5b8b63f..02c6b9dcbd3e 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;
> > @@ -151,6 +152,10 @@ static int mchp23k256_probe(struct spi_device *spi)
> >  	flash->mtd._read	= mchp23k256_read;
> >  	flash->mtd._write	= mchp23k256_write;
> >  
> > +	flash->mtd.erasesize = PAGE_SIZE;
> > +	while (flash->mtd.size & (flash->mtd.erasesize - 1))
> > +		flash->mtd.erasesize >>= 1;
> > +
> 
> Can we fix allocate_partition() to properly handle the
> master->erasesize == 0 case instead of doing that?

Is everything actually ready for the eraseblock size to be 0? That would
seem surprising to many applications, I would think. Can you, for
instance, even use UBI on such a device?

BTW, I feel like this check is a little more natural to do with
'mtd->flags & MTD_NO_ERASE', rather than checking the (apparently
meaningless) erasesize.

(I realize there's a later version of these patches, but I figured I'd
put my comments where the suggestion was brought up.)

Brian

Re: [PATCH 3/4] mtd: mchp23k256: add partitioning support

[Boris Brezillon]

Le Thu, 1 Jun 2017 11:43:40 -0700,
Brian Norris <computersforpeace@gmail.com> a écrit :

> On Wed, May 17, 2017 at 05:29:11PM +0200, Boris Brezillon wrote:
> > Hi Chris,
> > 
> > On Wed, 17 May 2017 17:39:07 +1200
> > Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:
> >   
> > > 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>
> > > ---
> > >  drivers/mtd/devices/mchp23k256.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/drivers/mtd/devices/mchp23k256.c b/drivers/mtd/devices/mchp23k256.c
> > > index 2542f5b8b63f..02c6b9dcbd3e 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;
> > > @@ -151,6 +152,10 @@ static int mchp23k256_probe(struct spi_device *spi)
> > >  	flash->mtd._read	= mchp23k256_read;
> > >  	flash->mtd._write	= mchp23k256_write;
> > >  
> > > +	flash->mtd.erasesize = PAGE_SIZE;
> > > +	while (flash->mtd.size & (flash->mtd.erasesize - 1))
> > > +		flash->mtd.erasesize >>= 1;
> > > +  
> > 
> > Can we fix allocate_partition() to properly handle the
> > master->erasesize == 0 case instead of doing that?  
> 
> Is everything actually ready for the eraseblock size to be 0? That would
> seem surprising to many applications, I would think. Can you, for
> instance, even use UBI on such a device?

Well, I think it's already broken. AFAICT this driver does not
implement ->_erase(), and mtd_erase() does not check if MTD_NO_ERASE is
set before calling mtd->_erase(), neither UBI does before calling
mtd_erase().

Between a NULL pointer exception and a div-by-zero exception, I can't
decide what is better :-).

IMO, we'd better add a check in UBI to refuse to attach a device with
MTD_NO_ERASE or mtd->erasesize == 0, and fix other places that don't
check erasesize value instead of putting a fake erasesize and using a
dummy ->_erase() implementation for those devices that simply can't be
erased.

We should also probably complain with -ENOTSUPP when someone calls
mtd_erase() on a device with MTD_NO_ERASE and add more checks in the
add_mtd_device() to detect drivers that don't have MTD_NO_ERASE set
and do not implement ->_erase() or leave ->erasesize to 0. 

> 
> BTW, I feel like this check is a little more natural to do with
> 'mtd->flags & MTD_NO_ERASE', rather than checking the (apparently
> meaningless) erasesize.

Fair enough.

Re: [PATCH 3/4] mtd: mchp23k256: add partitioning support

[Brian Norris]

On Thu, Jun 01, 2017 at 10:47:12PM +0200, Boris Brezillon wrote:
> Le Thu, 1 Jun 2017 11:43:40 -0700,
> Brian Norris <computersforpeace@gmail.com> a écrit :
> > On Wed, May 17, 2017 at 05:29:11PM +0200, Boris Brezillon wrote:
> > > On Wed, 17 May 2017 17:39:07 +1200
> > > Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:
> > > > @@ -151,6 +152,10 @@ static int mchp23k256_probe(struct spi_device *spi)
> > > >  	flash->mtd._read	= mchp23k256_read;
> > > >  	flash->mtd._write	= mchp23k256_write;
> > > >  
> > > > +	flash->mtd.erasesize = PAGE_SIZE;
> > > > +	while (flash->mtd.size & (flash->mtd.erasesize - 1))
> > > > +		flash->mtd.erasesize >>= 1;
> > > > +  
> > > 
> > > Can we fix allocate_partition() to properly handle the
> > > master->erasesize == 0 case instead of doing that?  
> > 
> > Is everything actually ready for the eraseblock size to be 0? That would
> > seem surprising to many applications, I would think. Can you, for
> > instance, even use UBI on such a device?
> 
> Well, I think it's already broken. AFAICT this driver does not
> implement ->_erase(), and mtd_erase() does not check if MTD_NO_ERASE is
> set before calling mtd->_erase(), neither UBI does before calling
> mtd_erase().

Sure.

> Between a NULL pointer exception and a div-by-zero exception, I can't
> decide what is better :-).

Well, there are other potential problems than that. What if someone was
iterating over the device size, by increments of erasesize? Infinite
loop! Or what about anything that might have assumed
'writesize < erasesize'?

I'm mostly thinking out loud, because I'm not sure there's a really good
way to handle this, other than stop making those assumptions.

(A *possible* solution would be to have MTD enforce a fake erasesize for
NO_ERASE flash, instead of making drivers do it, like Chris was trying.
But I'm not sure that's a good one.)

> IMO, we'd better add a check in UBI to refuse to attach a device with
> MTD_NO_ERASE or mtd->erasesize == 0, and fix other places that don't
> check erasesize value instead of putting a fake erasesize and using a
> dummy ->_erase() implementation for those devices that simply can't be
> erased.

That's probably a good idea.

> We should also probably complain with -ENOTSUPP when someone calls
> mtd_erase() on a device with MTD_NO_ERASE and add more checks in the
> add_mtd_device() to detect drivers that don't have MTD_NO_ERASE set
> and do not implement ->_erase() or leave ->erasesize to 0. 

Yep.

> > BTW, I feel like this check is a little more natural to do with
> > 'mtd->flags & MTD_NO_ERASE', rather than checking the (apparently
> > meaningless) erasesize.
> 
> Fair enough.

OK, well I'll take another look at v4, but that might be my only
criticism then.

Overall though, a "NO_ERASE" MTD makes me wonder why it's an MTD in the
first place. I guess we're kinda the wild west of things that don't fit
into the block subsystem...

Brian

Re: [PATCH 3/4] mtd: mchp23k256: add partitioning support

[Boris Brezillon]

On Thu, 1 Jun 2017 15:01:56 -0700
Brian Norris <computersforpeace@gmail.com> wrote:

> On Thu, Jun 01, 2017 at 10:47:12PM +0200, Boris Brezillon wrote:
> > Le Thu, 1 Jun 2017 11:43:40 -0700,
> > Brian Norris <computersforpeace@gmail.com> a écrit :  
> > > On Wed, May 17, 2017 at 05:29:11PM +0200, Boris Brezillon wrote:  
> > > > On Wed, 17 May 2017 17:39:07 +1200
> > > > Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:  
> > > > > @@ -151,6 +152,10 @@ static int mchp23k256_probe(struct spi_device *spi)
> > > > >  	flash->mtd._read	= mchp23k256_read;
> > > > >  	flash->mtd._write	= mchp23k256_write;
> > > > >  
> > > > > +	flash->mtd.erasesize = PAGE_SIZE;
> > > > > +	while (flash->mtd.size & (flash->mtd.erasesize - 1))
> > > > > +		flash->mtd.erasesize >>= 1;
> > > > > +    
> > > > 
> > > > Can we fix allocate_partition() to properly handle the
> > > > master->erasesize == 0 case instead of doing that?    
> > > 
> > > Is everything actually ready for the eraseblock size to be 0? That would
> > > seem surprising to many applications, I would think. Can you, for
> > > instance, even use UBI on such a device?  
> > 
> > Well, I think it's already broken. AFAICT this driver does not
> > implement ->_erase(), and mtd_erase() does not check if MTD_NO_ERASE is
> > set before calling mtd->_erase(), neither UBI does before calling
> > mtd_erase().  
> 
> Sure.
> 
> > Between a NULL pointer exception and a div-by-zero exception, I can't
> > decide what is better :-).  
> 
> Well, there are other potential problems than that. What if someone was
> iterating over the device size, by increments of erasesize? Infinite
> loop! Or what about anything that might have assumed
> 'writesize < erasesize'?

Absolutely, div-by-zero is not the only problem we can face.

> 
> I'm mostly thinking out loud, because I'm not sure there's a really good
> way to handle this, other than stop making those assumptions.
> 
> (A *possible* solution would be to have MTD enforce a fake erasesize for
> NO_ERASE flash, instead of making drivers do it, like Chris was trying.
> But I'm not sure that's a good one.)

Actually, by doing that we keep encouraging people to not do the right
thing :-).

> 
> > IMO, we'd better add a check in UBI to refuse to attach a device with
> > MTD_NO_ERASE or mtd->erasesize == 0, and fix other places that don't
> > check erasesize value instead of putting a fake erasesize and using a
> > dummy ->_erase() implementation for those devices that simply can't be
> > erased.  
> 
> That's probably a good idea.

I'll try to post patches soon, but they definitely won't fix all
problems. For example, we just can't catch the infinite loop issue you
are mentioning above.

> 
> > We should also probably complain with -ENOTSUPP when someone calls
> > mtd_erase() on a device with MTD_NO_ERASE and add more checks in the
> > add_mtd_device() to detect drivers that don't have MTD_NO_ERASE set
> > and do not implement ->_erase() or leave ->erasesize to 0.   
> 
> Yep.
> 
> > > BTW, I feel like this check is a little more natural to do with
> > > 'mtd->flags & MTD_NO_ERASE', rather than checking the (apparently
> > > meaningless) erasesize.  
> > 
> > Fair enough.  
> 
> OK, well I'll take another look at v4, but that might be my only
> criticism then.
> 
> Overall though, a "NO_ERASE" MTD makes me wonder why it's an MTD in the
> first place. I guess we're kinda the wild west of things that don't fit
> into the block subsystem...

Yes, that's exactly what the MTD subsystem is: a place where you can
put drivers for memory devices that are not block devices :-). Don't
know if it's a good thing or not, but it definitely makes MTD users
life harder.

BTW, MTD_NO_ERASE is not the only problem we have with UBI or JFFS2.
Are we guaranteed that an erase operation fills an eraseblock with
ones? Don't we have mem technologies that are filling them with zeros?
Note that mtdram is artificially setting the mem-region to 0xff in its
dummy erase operation, so maybe it's a implicit rule that ->_erase() is
supposed to fill eraseblocks with 0xff.

Re: [PATCH 3/4] mtd: mchp23k256: add partitioning support

[Brian Norris]

On Fri, Jun 02, 2017 at 11:04:06AM +0200, Boris Brezillon wrote:
> BTW, MTD_NO_ERASE is not the only problem we have with UBI or JFFS2.
> Are we guaranteed that an erase operation fills an eraseblock with
> ones? Don't we have mem technologies that are filling them with zeros?
> Note that mtdram is artificially setting the mem-region to 0xff in its
> dummy erase operation, so maybe it's a implicit rule that ->_erase() is
> supposed to fill eraseblocks with 0xff.

I've wondered about the general assumption. But mtdram isn't really a
good example, because it clearly calls itself a "test mtd device". So it
makes sense it would emulate common MTDs (i.e., flash memory).

Brian

Re: [PATCH 3/4] mtd: mchp23k256: add partitioning support

[Chris Packham]

On 02/06/17 06:43, Brian Norris wrote:
> On Wed, May 17, 2017 at 05:29:11PM +0200, Boris Brezillon wrote:
>> Hi Chris,
>>
>> On Wed, 17 May 2017 17:39:07 +1200
>> Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:
>>
>>> 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>
>>> ---
>>>  drivers/mtd/devices/mchp23k256.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/mtd/devices/mchp23k256.c b/drivers/mtd/devices/mchp23k256.c
>>> index 2542f5b8b63f..02c6b9dcbd3e 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;
>>> @@ -151,6 +152,10 @@ static int mchp23k256_probe(struct spi_device *spi)
>>>  	flash->mtd._read	= mchp23k256_read;
>>>  	flash->mtd._write	= mchp23k256_write;
>>>
>>> +	flash->mtd.erasesize = PAGE_SIZE;
>>> +	while (flash->mtd.size & (flash->mtd.erasesize - 1))
>>> +		flash->mtd.erasesize >>= 1;
>>> +
>>
>> Can we fix allocate_partition() to properly handle the
>> master->erasesize == 0 case instead of doing that?
>
> Is everything actually ready for the eraseblock size to be 0?

That was my initial motivation for faking it.

> That would
> seem surprising to many applications, I would think. Can you, for
> instance, even use UBI on such a device?

I've tried ext2 and I believe Andrew has tried minix fs. We're talking 
SRAM so UBI/UBIFS doesn't really provide much benefit for this use-case.

> BTW, I feel like this check is a little more natural to do with
> 'mtd->flags & MTD_NO_ERASE', rather than checking the (apparently
> meaningless) erasesize.
>
> (I realize there's a later version of these patches, but I figured I'd
> put my comments where the suggestion was brought up.)
>
> Brian
>

Re: [PATCH 3/4] mtd: mchp23k256: add partitioning support

[Brian Norris]

On Thu, Jun 01, 2017 at 09:30:07PM +0000, Chris Packham wrote:
> On 02/06/17 06:43, Brian Norris wrote:
> > On Wed, May 17, 2017 at 05:29:11PM +0200, Boris Brezillon wrote:
> >> Can we fix allocate_partition() to properly handle the
> >> master->erasesize == 0 case instead of doing that?
> >
> > Is everything actually ready for the eraseblock size to be 0?
> 
> That was my initial motivation for faking it.

Understood. I think it's probably better to avoid hacking drivers like
you were about to, but I was also curious if anyone had thought through
the implications of *not* forcing a non-zero size.

> > That would
> > seem surprising to many applications, I would think. Can you, for
> > instance, even use UBI on such a device?
> 
> I've tried ext2 and I believe Andrew has tried minix fs. We're talking 
> SRAM so UBI/UBIFS doesn't really provide much benefit for this use-case.

Right. But that's not necessarily true for all NO_ERASE devices, so we'd
probably want to think about that before allowing it.

Brian

Re: [PATCH 3/4] mtd: mchp23k256: add partitioning support

[Chris Packham]

On 02/06/17 10:23, Brian Norris wrote:
> On Thu, Jun 01, 2017 at 09:30:07PM +0000, Chris Packham wrote:
>> On 02/06/17 06:43, Brian Norris wrote:
>>> On Wed, May 17, 2017 at 05:29:11PM +0200, Boris Brezillon wrote:
>>>> Can we fix allocate_partition() to properly handle the
>>>> master->erasesize == 0 case instead of doing that?
>>>
>>> Is everything actually ready for the eraseblock size to be 0?
>>
>> That was my initial motivation for faking it.
> 
> Understood. I think it's probably better to avoid hacking drivers like
> you were about to, but I was also curious if anyone had thought through
> the implications of *not* forcing a non-zero size.
> 
>>> That would
>>> seem surprising to many applications, I would think. Can you, for
>>> instance, even use UBI on such a device?
>>
>> I've tried ext2 and I believe Andrew has tried minix fs. We're talking
>> SRAM so UBI/UBIFS doesn't really provide much benefit for this use-case.
> 
> Right. But that's not necessarily true for all NO_ERASE devices, so we'd
> probably want to think about that before allowing it.

Do we need a flag to indicate SRAM-like properties? I assume there is a 
difference between NO_ERASE on ROM devices where there is just no way of 
erasing the data. For {S,F,M}RAM there is no block erase operation but 
you can overwrite data to destroy it (which is actually my use-case with 
this SPI SRAM). I was tempted to set erase_size = 1 at one point which 
in my mind was technically accurate but would probably upset the mtd 
layer just as much as 0.

Re: [PATCH 3/4] mtd: mchp23k256: add partitioning support

[Brian Norris]

On Thu, Jun 01, 2017 at 11:08:08PM +0000, Chris Packham wrote:
> Do we need a flag to indicate SRAM-like properties? I assume there is a 
> difference between NO_ERASE on ROM devices where there is just no way of 
> erasing the data. For {S,F,M}RAM there is no block erase operation but 

I think we already have that:

#define MTD_CAP_ROM             0
#define MTD_CAP_RAM             (MTD_WRITEABLE | MTD_BIT_WRITEABLE | MTD_NO_ERASE)

The key signifier for ROM would be !MTD_WRITEABLE.

> you can overwrite data to destroy it (which is actually my use-case with 
> this SPI SRAM). I was tempted to set erase_size = 1 at one point which 
> in my mind was technically accurate but would probably upset the mtd 
> layer just as much as 0.

I'm not sure what erasesize should be here. I suppose 0, but really, I
think the MTD_NO_ERASE flag is the clearer indication that erase is not
needed, and that one should ignore the erasesize.

Brian

[PATCH 4/4] 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>
---
 .../bindings/mtd/microchip,mchp23k256.txt           |  2 +-
 drivers/mtd/devices/mchp23k256.c                    | 21 +++++++++++++++++++--
 2 files changed, 20 insertions(+), 3 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 02c6b9dcbd3e..d1eba587633c 100644
--- a/drivers/mtd/devices/mchp23k256.c
+++ b/drivers/mtd/devices/mchp23k256.c
@@ -21,6 +21,8 @@
 #include <linux/spi/spi.h>
 #include <linux/of_device.h>
 
+enum chips { mchp23k256, mchp23lcv1024 };
+
 struct mchp23k256_flash {
 	struct spi_device	*spi;
 	struct mutex		lock;
@@ -128,6 +130,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 +146,28 @@ 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;
+		break;
+	default:
+		flash->mtd.size		= SZ_32K;
+		break;
+	}
+
 	flash->mtd.erasesize = PAGE_SIZE;
 	while (flash->mtd.size & (flash->mtd.erasesize - 1))
 		flash->mtd.erasesize >>= 1;
@@ -172,7 +188,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 4/4] mtd: mchp23k256: Add support for mchp23lcv1024

[Andrew Lunn]

> +	switch (chip){

Space please, between ) and {

      Andrew

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

[Chris Packham]

On 17/05/17 17:39, Chris Packham wrote:
> 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.


I've just noticed that another difference between the 2 chips is 16-bit 
vs 24-bit addressing. So that will also need to be handled.


> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>   .../bindings/mtd/microchip,mchp23k256.txt           |  2 +-
>   drivers/mtd/devices/mchp23k256.c                    | 21 +++++++++++++++++++--
>   2 files changed, 20 insertions(+), 3 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 02c6b9dcbd3e..d1eba587633c 100644
> --- a/drivers/mtd/devices/mchp23k256.c
> +++ b/drivers/mtd/devices/mchp23k256.c
> @@ -21,6 +21,8 @@
>   #include <linux/spi/spi.h>
>   #include <linux/of_device.h>
>   
> +enum chips { mchp23k256, mchp23lcv1024 };
> +
>   struct mchp23k256_flash {
>   	struct spi_device	*spi;
>   	struct mutex		lock;
> @@ -128,6 +130,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 +146,28 @@ 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;
> +		break;
> +	default:
> +		flash->mtd.size		= SZ_32K;
> +		break;
> +	}
> +
>   	flash->mtd.erasesize = PAGE_SIZE;
>   	while (flash->mtd.size & (flash->mtd.erasesize - 1))
>   		flash->mtd.erasesize >>= 1;
> @@ -172,7 +188,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);
> 

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

[Andrew Lunn]

On Wed, May 17, 2017 at 05:39:04PM +1200, Chris Packham 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.

Hi Chris

Cool. I only have a mchp23k256, so that is what i targeted. And it is
used on an Intel board, via a USB-SPI dongle, so i didn't need
platform data. I considered partition support, but decided against it,
for my use case. I can just about get a usable minix filesystem in
256K. But anything smaller seems silly to have a real filesystem on
it.

	Andrew

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

[Andrew Lunn]

On Wed, May 17, 2017 at 05:39:04PM +1200, Chris Packham 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.

Hi Chris

I tested my user case. No obvious regressions.

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

    Andrew