Received-SPF: pass (google.com: domain of linux-kernel+bounces-71400-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Content-Type: multipart/signed;
 boundary=4d612e85c44be11edd360cd91b974ed96a303e8b11425e6de3e0efa4c955;
 micalg=pgp-sha256; protocol="application/pgp-signature"
Date: Mon, 19 Feb 2024 14:26:16 +0100
Message-Id: <CZ938PEUZ1JQ.2DJE7C03HI9OO@walle.cc>
Subject: Re: [RFC PATCH] nvmem: core: add sysfs cell write support
Cc: <srinivas.kandagatla@linaro.org>, <gregkh@linuxfoundation.org>,
 <rafal@milecki.pl>, <linux-kernel@vger.kernel.org>, <kernel@pengutronix.de>
From: "Michael Walle" <michael@walle.cc>
To: "Marco Felsch" <m.felsch@pengutronix.de>, "Miquel Raynal"
 <miquel.raynal@bootlin.com>
References: <20240215211401.1201004-1-m.felsch@pengutronix.de>
 <CZ6DFL6061FS.2WMDPMSSBXX8S@walle.cc>
 <20240216100750.zxl4wncbgpulr2cc@pengutronix.de>
 <20240219120414.32395299@xps-13>
 <20240219115358.xui5fpoisvsubdyb@pengutronix.de>
In-Reply-To: <20240219115358.xui5fpoisvsubdyb@pengutronix.de>
Precedence: bulk

--4d612e85c44be11edd360cd91b974ed96a303e8b11425e6de3e0efa4c955
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8

On Mon Feb 19, 2024 at 12:53 PM CET, Marco Felsch wrote:
> On 24-02-19, Miquel Raynal wrote:
> > Hi Marco,
> >=20
> > m.felsch@pengutronix.de wrote on Fri, 16 Feb 2024 11:07:50 +0100:
> >=20
> > > Hi Michael,
> > >=20
> > > On 24-02-16, Michael Walle wrote:
> > > > Hi,
> > > >=20
> > > > On Thu Feb 15, 2024 at 10:14 PM CET, Marco Felsch wrote: =20
> > > > > @@ -432,6 +466,7 @@ static int nvmem_populate_sysfs_cells(struct =
nvmem_device *nvmem)
> > > > >  	struct bin_attribute **cells_attrs, *attrs;
> > > > >  	struct nvmem_cell_entry *entry;
> > > > >  	unsigned int ncells =3D 0, i =3D 0;
> > > > > +	umode_t mode;
> > > > >  	int ret =3D 0;
> > > > > =20
> > > > >  	mutex_lock(&nvmem_mutex);
> > > > > @@ -456,15 +491,18 @@ static int nvmem_populate_sysfs_cells(struc=
t nvmem_device *nvmem)
> > > > >  		goto unlock_mutex;
> > > > >  	}
> > > > > =20
> > > > > +	mode =3D nvmem_bin_attr_get_umode(nvmem);
> > > > > +
> > > > >  	/* Initialize each attribute to take the name and size of the c=
ell */
> > > > >  	list_for_each_entry(entry, &nvmem->cells, node) {
> > > > >  		sysfs_bin_attr_init(&attrs[i]);
> > > > >  		attrs[i].attr.name =3D devm_kasprintf(&nvmem->dev, GFP_KERNEL,
> > > > >  						    "%s@%x", entry->name,
> > > > >  						    entry->offset);
> > > > > -		attrs[i].attr.mode =3D 0444; =20
> > > >=20
> > > > cells are not writable if there is a read post process hook, see
> > > > __nvmem_cell_entry_write().
> > > >=20
> > > > if (entry->read_post_processing)
> > > > 	mode &=3D ~0222; =20
> > >=20
> > > good point, thanks for the hint :) I will add this and send a non-rfc
> > > version if write-support is something you would like to have.
> >=20
> > I like the idea but, what about mtd devices (and soon maybe UBI
> > devices)? This may only work on EEPROM-like devices I guess, where each
> > area is fully independent and where no erasure is actually expected.
>
> For MTD I would say that you need to ensure that you need to align the
> cells correctly. The cell-write should handle the page erase/write cycle
> properly. E.g. an SPI-NOR need to align the cells to erase-page size or
> the nvmem-cell-write need to read-copy-update the cells if they are not
> erase-paged aligned.
>
> Regarding UBI(FS) I'm not sure if this is required at all since you have
> an filesystem. IMHO nvmem-cells are very lowelevel and are not made for
> filesystem backed backends.
>
> That beeing said: I have no problem if we provide write support for
> EEPROMs only and adapt it later on to cover spi-nor/nand devices as
> well.

Agreed. Honestly, I don't know how much sense this makes for MTD
devices. First, the operation itself, seems really dangerous, as
you'll have to delete the whole sector. Second, during initial
provisioning, I don't think it will make much sense to use the sysfs
cells because you cannot combine multiple writes into one. You'll
always end up with unnecessary erases.

What's your use case here?

Just my two cents..
-michael

--4d612e85c44be11edd360cd91b974ed96a303e8b11425e6de3e0efa4c955
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iIcEABYIAC8WIQQCnWSOYTtih6UXaxvNyh2jtWxG+wUCZdNW+BEcbWljaGFlbEB3
YWxsZS5jYwAKCRDNyh2jtWxG+1uSAQCkmDcFY4GzAsY7ha1lKvgSSJRCqLkydobm
qqzwyMMsFwEA65edy4y9PaiLDA1aM8sL5Npr1cskJtyTGNMcmDHKXws=
=bNfX
-----END PGP SIGNATURE-----

--4d612e85c44be11edd360cd91b974ed96a303e8b11425e6de3e0efa4c955--