Re: [PATCH 2/3] mtd: spi-nor: Add a driver for the VIA/WonderMedia serial flash controller

From: Pratyush Yadav <pratyush@kernel.org>
To: Alexey Charkov <alchark@gmail.com>
Cc: Pratyush Yadav <pratyush@kernel.org>,
	 Mark Brown <broonie@kernel.org>, Rob Herring <robh@kernel.org>,
	 Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	 Tudor Ambarus <tudor.ambarus@linaro.org>,
	 Michael Walle <mwalle@kernel.org>,
	 Miquel Raynal <miquel.raynal@bootlin.com>,
	 Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	 Krzysztof Kozlowski <krzk@kernel.org>,
	 linux-spi@vger.kernel.org, devicetree@vger.kernel.org,
	 linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org,
	 linux-arm-kernel@lists.infradead.org,
	Mika Westerberg <mika.westerberg@linux.intel.com>
Subject: Re: [PATCH 2/3] mtd: spi-nor: Add a driver for the VIA/WonderMedia serial flash controller
Date: Thu, 24 Jul 2025 15:51:08 +0200	[thread overview]
Message-ID: <mafs0h5z1snn7.fsf@kernel.org> (raw)
In-Reply-To: <CABjd4YyRScBgDbi8Sk0D3vxcmLF8+YBetUdkfhrS_4Y7M+gS1g@mail.gmail.com>

Hi Alexey,

This email got buried in my inbox somewhere...

On Mon, May 12 2025, Alexey Charkov wrote:

> Hi Pratyush,
>
> On Mon, May 12, 2025 at 1:20 PM Pratyush Yadav <pratyush@kernel.org> wrote:
>>
>> Hi Alexey,
>>
>> On Sat, May 10 2025, Alexey Charkov wrote:
>>
>> > The controller is tailored to SPI NOR flash memory and abstracts away all
>> > SPI communications behind a small set of MMIO registers and a physical
>> > memory mapping of the actual chip contents.
>> >
>> > It doesn't expose chip probing functions beyond reading the ID though, so
>> > use lower level chip opcodes via the "programmable command mode" of the
>> > controller and the kernel's SPI NOR framework to avoid hard-coding chip
>> > parameters for each ID the way the vendor kernel does it.
>> >
>> > Currently tested on a WonderMedia WM8950 SoC with a Macronix MX25L4005A
>> > flash chip (APC Rock board), but should work on all VIA/WonderMedia SoCs
>> >
>> > Signed-off-by: Alexey Charkov <alchark@gmail.com>
>> > ---
>> >  MAINTAINERS                                  |   1 +
>> >  drivers/mtd/spi-nor/controllers/Kconfig      |  14 +
>> >  drivers/mtd/spi-nor/controllers/Makefile     |   1 +
>> >  drivers/mtd/spi-nor/controllers/wmt-sflash.c | 525 +++++++++++++++++++++++++++
>>
>> Drivers in drivers/mtd/spi-nor/controllers/ are deprecated, and we want
>> to eventually get rid of the API. The expected way is for drivers to use
>> SPI MEM (drivers/spi/spi-mem.c). SPI MEM drivers are usually more
>> general and not tailored specifically to SPI NOR flashes, so it might be
>> a bit tricky to write drivers for specialized hardware with it. But I
>> think the drivers/spi/spi-intel.c driver is written for similar kind of
>> hardware so it should be possible.
>
> Oops. I've had a look at spi-mem, and it seems like it's not a
> particularly fitting abstraction for this controller.
>
> From what I understood, spi-mem primarily expects to be talking SPI
> opcodes to the controller, and for the controller/driver to bring
> their own chip probing routines. This controller on the other hand
> abstracts the opcodes away, and wants someone to tell it what its
> flash chip can do (the controller itself can only get a chip ID in
> "normal" mode, and it needs to somehow know the chip size and
> standard/fast read capability of the chip). So pretty much the
> opposite, huh.

Does it use SFDP to figure out which opcodes to use? Then it feels very
similar to intel-spi. See [0] for example. I know this is fitting a
square peg in a round hole, but if it isn't too painful then it would
make maintenance on SPI NOR end a bit easier.

Mika (+Cc), you did the conversion of intel-spi to SPI MEM. Maybe you
can share how painful/easy the conversion was, and if it ended up being
maintainable?

>
> In the end, I only need something like spi_nor_detect() and can do the
> rest directly on top of the MTD framework without touching any SPI
> opcodes after the detection is done. Is there any other non-deprecated
> framework that can provide something like this? Maybe physmap? It
> looks even older than SPI NOR though :)

[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/spi/spi-intel.c#n905

-- 
Regards,
Pratyush Yadav