lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 8 Dec 2023 17:05:34 +0000
From:   "Mahapatra, Amit Kumar" <amit.kumar-mahapatra@....com>
To:     Tudor Ambarus <tudor.ambarus@...aro.org>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "pratyush@...nel.org" <pratyush@...nel.org>,
        "miquel.raynal@...tlin.com" <miquel.raynal@...tlin.com>,
        "richard@....at" <richard@....at>,
        "vigneshr@...com" <vigneshr@...com>,
        "sbinding@...nsource.cirrus.com" <sbinding@...nsource.cirrus.com>,
        "lee@...nel.org" <lee@...nel.org>,
        "james.schulman@...rus.com" <james.schulman@...rus.com>,
        "david.rhodes@...rus.com" <david.rhodes@...rus.com>,
        "rf@...nsource.cirrus.com" <rf@...nsource.cirrus.com>,
        "perex@...ex.cz" <perex@...ex.cz>,
        "tiwai@...e.com" <tiwai@...e.com>
CC:     "linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "michael@...le.cc" <michael@...le.cc>,
        "linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
        "nicolas.ferre@...rochip.com" <nicolas.ferre@...rochip.com>,
        "alexandre.belloni@...tlin.com" <alexandre.belloni@...tlin.com>,
        "claudiu.beznea@...on.dev" <claudiu.beznea@...on.dev>,
        "Simek, Michal" <michal.simek@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
        "patches@...nsource.cirrus.com" <patches@...nsource.cirrus.com>,
        "linux-sound@...r.kernel.org" <linux-sound@...r.kernel.org>,
        "git (AMD-Xilinx)" <git@....com>,
        "amitrkcian2002@...il.com" <amitrkcian2002@...il.com>
Subject: RE: [PATCH v11 07/10] mtd: spi-nor: Add stacked memories support in
 spi-nor

Hello Tudor,

> -----Original Message-----
> From: Tudor Ambarus <tudor.ambarus@...aro.org>
> Sent: Wednesday, December 6, 2023 8:00 PM
> To: Mahapatra, Amit Kumar <amit.kumar-mahapatra@....com>;
> broonie@...nel.org; pratyush@...nel.org; miquel.raynal@...tlin.com;
> richard@....at; vigneshr@...com; sbinding@...nsource.cirrus.com;
> lee@...nel.org; james.schulman@...rus.com; david.rhodes@...rus.com;
> rf@...nsource.cirrus.com; perex@...ex.cz; tiwai@...e.com
> Cc: linux-spi@...r.kernel.org; linux-kernel@...r.kernel.org;
> michael@...le.cc; linux-mtd@...ts.infradead.org;
> nicolas.ferre@...rochip.com; alexandre.belloni@...tlin.com;
> claudiu.beznea@...on.dev; Simek, Michal <michal.simek@....com>; linux-
> arm-kernel@...ts.infradead.org; alsa-devel@...a-project.org;
> patches@...nsource.cirrus.com; linux-sound@...r.kernel.org; git (AMD-
> Xilinx) <git@....com>; amitrkcian2002@...il.com
> Subject: Re: [PATCH v11 07/10] mtd: spi-nor: Add stacked memories support
> in spi-nor
> 
> 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,

No, according to the current implementation, in stacked mode, both flashes 
must be of the same make.

> wouldn't it be simpler to have just an array of flash sizes instead of
> duplicating the entire params struct?

Yes, that is accurate. In alignment with our current stacked support use case we 
can have an array of flash sizes instead.
The primary purpose of having an array of params struct was to facilitate 
potential future extensions, allowing the addition of stacked support for 
different flashes

> 
> >
> > 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@....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]);
> 
> 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 
anticipate that SPI-NOR should throw an error if the second or any 
subsequent flash within the stacked connection is different? Or would you 
prefer it to print a warning and operate in single mode (i.e., only the 
first flash)?

Regards,
Amit
> 
> Cheers,
> ta

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ