Re: [PATCH v8 5/9] block: implement NVMEM provider

Christoph Hellwig <[email protected]>
Newsgroups org.kernel.vger.linux-bluetooth,org.infradead.lists.ath10k,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-block,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel,org.kernel.vger.linux-mmc,org.kernel.vger.linux-wireless,org.kernel.vger.netdev
Message-ID <[email protected]>
On Fri, Jul 03, 2026 at 03:45:18PM +0200, Loic Poulain wrote:
> +	  Allow block devices (or partitions) to act as NVMEM providers,

Based on the code below I don't think you actually support partitions.

> +static int blk_nvmem_reg_read(void *priv, unsigned int from, void *val, size_t bytes)

Overly long line.

> +{
> +	dev_t devt = (dev_t)(uintptr_t)priv;
> +	size_t bytes_left = bytes;
> +	loff_t pos = from;
> +	int ret = 0;
> +
> +	struct file *bdev_file __free(fput) =
> +		bdev_file_open_by_dev(devt, BLK_OPEN_READ, NULL, NULL);

Please try to avoid all that magic autfree stuff that makes the code
impossible to read about.

> +	if (IS_ERR(bdev_file))
> +		return PTR_ERR(bdev_file);
> +
> +	while (bytes_left) {

.. and just use __kernel_read instead of badly reimplementing it.

> +	config.name = dev_name(dev);
> +	config.owner = THIS_MODULE;
> +	config.priv = (void *)(uintptr_t)dev->devt;

Why is this storing the dev_t and not the block device itself?

> +
> +		blk_nvmem_add(disk->part0);

Hmm, and you're always doing it for the whole device.  So the intefaces
really should be based on a gendisk, not a block_device.

Also once you register the whole devices as nvmem provider, we really
should keep it from being used a block device, shouldn't we?
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.