[PATCH v2 3/8] mtd: spi-nor: sfdp: Add support for SCCR map for multi-chip device

[Tudor Ambarus]

On 02.03.2023 07:41, Tudor Ambarus wrote:
> 
> 
> On 08.02.2023 07:53, tkuw584924 at gmail.com wrote:
>> From: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
>>
>> SCCR map for multi-chip devices contains the number of additional dice in
>> the device and register offset values for each additional dice.
>>
>> spi_nor_parse_sccr_mc() is added to determine the number of dice and
>> volatile register offset for each die. The volatile register offset table
>> may already be allocated and contains offset value for die-0 via SCCR map
>> parse. So, we should use devm_krealloc() to expand the table with
>> preserving die-0 offset.
>>
>> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
>> ---
>>   drivers/mtd/spi-nor/sfdp.c | 64 ++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 64 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
>> index e5b5776d8172..b7c08fe86239 100644
>> --- a/drivers/mtd/spi-nor/sfdp.c
>> +++ b/drivers/mtd/spi-nor/sfdp.c
>> @@ -26,6 +26,11 @@
>>                        * Status, Control and Configuration
>>                        * Register Map.
>>                        */
>> +#define SFDP_SCCR_MAP_MC_ID    0xff88    /*
>> +                     * Status, Control and Configuration
>> +                     * Register Map Offsets for Multi-Chip
>> +                     * SPI Memory Devices.
>> +                     */
>>   #define SFDP_SIGNATURE        0x50444653U
>> @@ -1252,6 +1257,61 @@ static int spi_nor_parse_sccr(struct spi_nor *nor,
>>       return ret;
>>   }
>> +/**
>> + * spi_nor_parse_sccr_mc() - Parse the Status, Control and Configuration
>> + *                           Register Map Offsets for Multi-Chip SPI 
>> Memory
>> + *                           Devices.
>> + * @nor:        pointer to a 'struct spi_nor'
>> + * @sccr_mc_header:    pointer to the 'struct sfdp_parameter_header' 
>> describing
>> + *            the SCCR Map offsets table length and version.
>> + *
>> + * Return: 0 on success, -errno otherwise.
>> + */
>> +static int spi_nor_parse_sccr_mc(struct spi_nor *nor,
>> +                 const struct sfdp_parameter_header *sccr_mc_header)
>> +{
>> +    struct spi_nor_flash_parameter *params = nor->params;
>> +    u32 *dwords, addr;
>> +    size_t len;
>> +    int ret;
>> +    u8 i;
>> +
>> +    len = sccr_mc_header->length * sizeof(*dwords);
>> +    dwords = kmalloc(len, GFP_KERNEL);
>> +    if (!dwords)
>> +        return -ENOMEM;
>> +
>> +    addr = SFDP_PARAM_HEADER_PTP(sccr_mc_header);
>> +    ret = spi_nor_read_sfdp(nor, addr, len, dwords);
>> +    if (ret)
>> +        goto out;
>> +
>> +    le32_to_cpu_array(dwords, sccr_mc_header->length);
>> +
>> +    /*
>> +     * Pair of DOWRDs (volatile and non-volatile register offsets) per
>> +     * additional die. Hence, length = 2 * (number of additional dice).
>> +     */
>> +    params->num_of_dice += sccr_mc_header->length / 2;
>> +
>> +    /* Address offset for volatile registers of additional dice */
>> +    params->vreg_offset =
>> +            devm_krealloc(nor->dev, params->vreg_offset,
>> +                      params->num_of_dice * sizeof(*dwords),
>> +                      GFP_KERNEL);
>> +    if (!params->vreg_offset) {
>> +        ret = -ENOMEM;
>> +        goto out;
>> +    }
>> +
>> +    for (i = 1; i < params->num_of_dice; i++)
> 
> here you limit the number of dices to 256, while the jesd216 says "any
> number from 2 and up may be used". While I agree that having 256 dices
> is improbable we'll have to decide whether:
> 1/ keep using u8 for the number of dices, thus limiting them to 256,
> assuming that the table length is wrong. In this case you'll have to
> check sccr_mc_header->length and return an error for no-of-dices > 256
> 2/ use unsigned int for the number of dices and hope you'll get an
> ENOMEM sooner or later
> 
>> +        params->vreg_offset[i] = dwords[(i - 1) * 2];
> 
> use SFDP_DWORD() please
>> +
>> +out:
>> +    kfree(dwords);
>> +    return ret;
>> +}
>> +
>>   /**
>>    * spi_nor_post_sfdp_fixups() - Updates the flash's parameters and 
>> settings
>>    * after SFDP has been parsed. Called only for flashes that define 
>> JESD216 SFDP
>> @@ -1461,6 +1521,10 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
>>               err = spi_nor_parse_sccr(nor, param_header);
>>               break;
>> +        case SFDP_SCCR_MAP_MC_ID:
>> +            err = spi_nor_parse_sccr_mc(nor, param_header);

what happens if you get an error here? Will the flash work fine just for
the first dice or it will not work at all?

>> +            break;
>> +
>>           default:
>>               break;
>>           }