Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 23 Nov 2018 11:29:03 +0100
From:   Boris Brezillon <boris.brezillon@bootlin.com>
To:     "Sverdlin, Alexander (Nokia - DE/Ulm)" <alexander.sverdlin@nokia.com>
Cc:     "Tudor.Ambarus@microchip.com" <Tudor.Ambarus@microchip.com>,
        "marek.vasut@gmail.com" <marek.vasut@gmail.com>,
        "dwmw2@infradead.org" <dwmw2@infradead.org>,
        "computersforpeace@gmail.com" <computersforpeace@gmail.com>,
        "richard@nod.at" <richard@nod.at>,
        "linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mtd: spi-nor: fix erase_type array to indicate current
 map conf
Message-ID: <20181123112903.56393f28@bbrezillon>
In-Reply-To: <20181123111729.1d5b6e57@bbrezillon>
References: <20181122123552.15756-1-tudor.ambarus@microchip.com>
        <3d8a2b83-282d-7cb2-3c8b-addae3c2af7f@nokia.com>
        <20181123111729.1d5b6e57@bbrezillon>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, 23 Nov 2018 11:17:29 +0100
Boris Brezillon <boris.brezillon@bootlin.com> wrote:

> On Fri, 23 Nov 2018 09:42:55 +0000
> "Sverdlin, Alexander (Nokia - DE/Ulm)" <alexander.sverdlin@nokia.com>
> wrote:
> 
> > Hello Tudor,
> > 
> > On 22/11/2018 13:36, Tudor.Ambarus@microchip.com wrote:
> > > From: Tudor Ambarus <tudor.ambarus@microchip.com>
> > > 
> > > Bug reported for the out-of-tree S25FS128S flash memory.
> > > 
> > > BFPT table advertises all the erase types supported by all the
> > > possible map configurations. Update the erase_type array to indicate
> > > which erase types are applicable to the current map configuration.
> > > 
> > > Backward compatibility test done on sst26vf064b.
> > > 
> > > Fixes: b038e8e3be72 ("mtd: spi-nor: parse SFDP Sector Map Parameter Table")
> > > Reported-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
> > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>  
> > 
> > I've tested this patch and it fixes the erasesize for S25FS128S and
> > our 128k partitions are writeable again with this patch.
> > 
> > Nevertheless, I think this is coincidence. I don't think that it
> > makes sense to OR all the erase types from all regions in one
> > bitmask and derive any uniform erasesize out of it.
> > This makes little sense for me in case of non-uniform maps.
> > 
> > I believe, the culprit here is one level higher, in the MTD partitioning
> > code (mtdpart.c) which has to be taught about non-uniform maps
> > but there is no infrastructure for this currently.
> 
> Keep in mind that mtdpart is only one part of the issue. As I said
> previously, some MTD users (UBI) expect a single eraseblock size, so
> it's not only mtdpart that you'd need to fix, but basically all MTD
> users that don't check ->eraseregions.

Just checked, and it seems mtdpart already supports eraseregions [1].
It picks the biggest erasesize of the regions covered by the partition,
which is exactly what we want. So all we'd have to do is make spi-nor.c
define those regions.

[1]https://elixir.bootlin.com/linux/v4.20-rc3/source/drivers/mtd/mtdpart.c#L482