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

[Tudor Ambarus]

On 4/7/23 03:23, Takahiro Kuwano wrote:
> Hi Tudor,
> 

Hi!

> On 4/6/2023 7:44 PM, Tudor Ambarus wrote:
>>
>>
>> On 4/6/23 07:18, 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 | 73 ++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 73 insertions(+)
>>>
>>> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
>>> index 9d43e18d3770..00a40bd8aa1e 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
>>>  
>>> @@ -1264,6 +1269,70 @@ static int spi_nor_parse_sccr(struct spi_nor *nor,
>>>  	return ret;
>>>  }
>>>  
>>> +#define SCCR_MC_MAX_DICE		255
>>> +
>>> +/**
>>> + * 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;
>>> +	u8 i, n_dice;
>>> +	size_t len;
>>> +	int ret;
>>> +
>>> +	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).
>>> +	 */
>>> +	n_dice = 1 + sccr_mc_header->length / 2;
>>> +	if (params->n_dice > SCCR_MC_MAX_DICE) {
> s/params->n_dice/n_dice
> 
>>
>> smatch warnings:
>> drivers/mtd/spi-nor/sfdp.c:1310 spi_nor_parse_sccr_mc() warn: impossible
>> condition '(params->n_dice > 255) => (0-255 > 255)'
>>
>> declaring the local variable n_dice as unsigned int should solve the
>> problem.
>>
> 'sccr_mc_header->length' is u8 so n_dice can be 1-128. We can keep
> n_dice as u8.
> 
> (n_dice > SCCR_MC_MAX_DICE) is impossible condition anyway...
> We don't need n_dice check. Otherwise define SCCR_MC_MAX_DICE as smaller
> value something like 16. What do you think?
> 

Indeed. Let's get rid of the check entirely. Keep n_dice as u8, remove
SCCR_MC_MAX_DICE and the check.