Hello Tudor,
-----Original Message----- From: Tudor Ambarus tudor.ambarus@linaro.org Sent: Tuesday, December 12, 2023 8:33 PM To: Mahapatra, Amit Kumar amit.kumar-mahapatra@amd.com; broonie@kernel.org; pratyush@kernel.org; miquel.raynal@bootlin.com; richard@nod.at; vigneshr@ti.com; sbinding@opensource.cirrus.com; lee@kernel.org; james.schulman@cirrus.com; david.rhodes@cirrus.com; rf@opensource.cirrus.com; perex@perex.cz; tiwai@suse.com Cc: linux-spi@vger.kernel.org; linux-kernel@vger.kernel.org; michael@walle.cc; linux-mtd@lists.infradead.org; nicolas.ferre@microchip.com; alexandre.belloni@bootlin.com; claudiu.beznea@tuxon.dev; Simek, Michal michal.simek@amd.com; linux- arm-kernel@lists.infradead.org; alsa-devel@alsa-project.org; patches@opensource.cirrus.com; linux-sound@vger.kernel.org; git (AMD- Xilinx) git@amd.com; amitrkcian2002@gmail.com Subject: Re: [PATCH v11 07/10] mtd: spi-nor: Add stacked memories support in spi-nor
On 12/11/23 13:37, Mahapatra, Amit Kumar wrote:
Hi!
cut
>> drivers/mtd/spi-nor/core.c | 272 >> +++++++++++++++++++++++++++++-----
--
>> drivers/mtd/spi-nor/core.h | 4 + >> include/linux/mtd/spi-nor.h | 15 +- >> 3 files changed, 240 insertions(+), 51 deletions(-) >> >> diff --git a/drivers/mtd/spi-nor/core.c >> b/drivers/mtd/spi-nor/core.c index 93ae69b7ff83..e990be7c7eb6 >> 100644 >> --- a/drivers/mtd/spi-nor/core.c >> +++ b/drivers/mtd/spi-nor/core.c > > cut > >> @@ -2905,7 +3007,10 @@ static void >> spi_nor_init_fixup_flags(struct spi_nor *nor) static int >> spi_nor_late_init_params(struct spi_nor >> *nor) { >> struct spi_nor_flash_parameter *params = >> spi_nor_get_params(nor, > 0); >> - int ret; >> + struct device_node *np = spi_nor_get_flash_node(nor); >> + u64 flash_size[SNOR_FLASH_CNT_MAX]; >> + u32 idx = 0; >> + int rc, ret; >> >> if (nor->manufacturer && nor->manufacturer->fixups && >> nor->manufacturer->fixups->late_init) { @@ -2937,6 >> +3042,44 @@ static int spi_nor_late_init_params(struct spi_nor
*nor)
>> if (params->n_banks > 1) >> params->bank_size = div64_u64(params->size,
params-
n_banks);
>> >> + nor->num_flash = 0; >> + >> + /* >> + * The flashes that are connected in stacked mode should be
of
>> +same > make. >> + * Except the flash size all other properties are identical >> +for all
the
>> + * flashes connected in stacked mode. >> + * The flashes that are connected in parallel mode should be
identical.
>> + */ >> + while (idx < SNOR_FLASH_CNT_MAX) { >> + rc = of_property_read_u64_index(np, "stacked-
memories",
> idx, >> +&flash_size[idx]); > > This is a little late in my opinion, as we don't have any sanity > check on the flashes that are stacked on top of the first. We > shall at least read and compare the ID for all.
Alright, I will incorporate a sanity check for reading and comparing the ID of the stacked flash. Subsequently, I believe this stacked logic should be relocated to spi_nor_get_flash_info() where we identify the first flash. Please share your thoughts on this. Additionally, do you
I'm wondering whether we can add a layer on top of the flash type to handle
When you mention "on top," are you referring to incorporating it into the MTD layer? Initially, Miquel had submitted this patch to address
I mean something above SPI MEM flashes, be it NOR, NANDs or whatever. Instead of treating the stacked flashes as a monolithic device and treat in SPI NOR some array of flashes, to have a layer above which probes the SPI MEM flash driver for each stacked flash. In your case SPI NOR would be probed twice, as you use 2 SPI NOR flashes.
stacked/parallel handling in the MTD layer. https://lore.kernel.org/linux-mtd/20200114191052.0a16d116@xps13/t/ However, the Device Tree bindings were initially not accepted.
Okay, thanks for the pointer. I'll take a look.
haven't yet read the thread from above, but I skimmed over the AMD controller datasheet.
Following a series of discussions, the below bindings were eventually merged. https://lore.kernel.org/all/20220126112608.955728-4-miquel.raynal@bo ot lin.com/
I saw, thanks.
the stacked/parallel modes. This way everything will become flash type independent. Would it be possible to stack 2 SPI NANDs? How about a SPI NOR and a SPI NAND?
Is the datasheet of the controller public?
Yes, https://docs.xilinx.com/r/en-US/am011-versal-acap-trm/Quad-SPI-Contr ol ler
Wonderful, I'll take a look. I see two clocks there. Does this mean that the stacked flashes can be operated at different frequencies? Do you know if we
In stacked configuration, both flashes utilize a common clock (single clock line). In a parallel setup, the flashes share the same clock but have distinct clock lines. Please refer the diagrams in the sections below for reference. https://docs.xilinx.com/r/en-US/am011-versal-acap-trm/QSPI-Flash-Inter face-Diagrams
Thanks! Can you share with us what flashes you used for testing in the stacked and parallel configurations?
I used SPI-NOR QSPI flashes for testing stacked and parallel.
can combine a SPI NOR with a SPI NAND in stacked configuration?
No, Xilinx/AMD QSPI controllers doesn't support this configuration.
2 SPI NANDs shall work with the AMD controller, right?
We never tested 2 SPI-NAND with AMD controller.
I skimmed over the QSPI controller datasheet and wondered why one would get complicated with 2 Quad Flashes when we have Octal. But then I saw that the same SoC can configure an Octal controller (the Octal and Quad controllers seems mutual exclusive) and that the Octal one can operate 2 octal flashes in stacked mode :).
Thats correct .
Please let me know how you want me to proceed on this.
Regards, Amit
Cheers, ta