lkml.org 
[lkml]   [2023]   [Dec]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH v11 07/10] mtd: spi-nor: Add stacked memories support in spi-nor
    Date
    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
    \
     
     \ /
      Last update: 2023-12-15 08:56    [W:6.229 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site