Messages in this thread | ![/](/images/icornerl.gif) | | From | "Mahapatra, Amit Kumar" <> | Subject | RE: [PATCH v11 07/10] mtd: spi-nor: Add stacked memories support in spi-nor | Date | Fri, 8 Dec 2023 17:06:24 +0000 |
| |
Hello Tudor,
> -----Original Message----- > From: Tudor Ambarus <tudor.ambarus@linaro.org> > Sent: Wednesday, December 6, 2023 8:14 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/6/23 14:30, Tudor Ambarus wrote: > > Hi, Amit, > > > > On 11/25/23 09:21, Amit Kumar Mahapatra wrote: > >> Each flash that is connected in stacked mode should have a separate > >> parameter structure. So, the flash parameter member(*params) of the > >> spi_nor structure is changed to an array (*params[2]). The array is > >> used to store the parameters of each flash connected in stacked > configuration. > >> > >> The current implementation assumes that a maximum of two flashes are > >> connected in stacked mode and both the flashes are of same make but > >> can differ in sizes. So, except the sizes all other flash parameters > >> of both the flashes are identical. > > > > Do you plan to add support for different flashes in stacked mode? If > > not, wouldn't it be simpler to have just an array of flash sizes > > instead of duplicating the entire params struct? > > > >> > >> SPI-NOR is not aware of the chip_select values, for any incoming > >> request SPI-NOR will decide the flash index with the help of > >> individual flash size and the configuration type (single/stacked). > >> SPI-NOR will pass on the flash index information to the SPI core & > >> SPI driver by setting the appropriate bit in > >> nor->spimem->spi->cs_index_mask. For example, if nth bit of > >> nor->spimem->spi->cs_index_mask is set then the driver would > >> assert/de-assert spi->chip_slect[n]. > >> > >> Signed-off-by: Amit Kumar Mahapatra <amit.kumar- > mahapatra@amd.com> > >> --- > >> 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]); > > also, it's not clear to me why you read this property multiple times. > Have you sent a device tree patch somewhere? It will help me understand > what you're trying to achieve.
Miquel submitted the device tree patch; here is the series. https://lore.kernel.org/all/20220126112608.955728-1-miquel.raynal@bootlin.com/
Regards, Amit > > > > > 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. > > > > Cheers, > > ta
| ![\](/images/icornerr.gif) |