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

[Tudor Ambarus]

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);
> +			break;
> +
>   		default:
>   			break;
>   		}