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@xxxxxxx>
>> ---
>> 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.
>
> 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
