[U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI framework

[Jagan Teki]

On 16/11/18 3:50 PM, Vignesh R wrote:
> 
> 
> On 16/11/18 3:10 PM, Rajat Srivastava wrote:
>>> Hi Rajat,
>>>
>>> On 13/11/18 5:30 PM, Rajat Srivastava wrote:
>>>> Add support for JESD216 rev B standard JEDEC Serial Flash Discoverable
>>>> Parameters (SFDP) tables to dynamically initialize flash size, page
>>>> size and address width of the flash. More parameters can be added as
>>>> per requirement.
>>>> SFDP parsing is made default but already existing method for parsing
>>>> these parameters are not deprecated.
>>>> A flag is created to skip SFDP parsing for a particular flash, if
>>>> required.
>>>>
>>>> SFDP data lets us auto-detect the addressing mode supported by the
>>>> flash which helps us access the flash using 4-byte address.
>>>>
>>>> Add a new argument in spi_flash_addr() function to create commands
>>>> with 3-byte or 4-byte address depending on the SFDP data read. Add
>>>> pointer to struct spi_flash in struct spi_slave so that driver can
>>>> have access to SFDP data.
>>>>
>>>> Introduce new structures and functions to read and parse SFDP data.
>>>> This is loosely based on Linux SFDP framework.
>>>>
>>>> Signed-off-by: Rajat Srivastava <rajat.srivastava at nxp.com>
>>>> ---
>>>> Changes in v2:
>>>> - Make SFDP parsing the default method.
>>>> - Change SPI_FLASH_USE_SFDP flag to SPI_FLASH_USE_SFDP to provide an
>>>> option to skip SFDP parsing for a particular flash.
>>>> ---
>>>
>>> [...]
>>>> +static int spi_flash_parse_bfpt(struct spi_flash *flash,
>>>> +				const struct sfdp_parameter_header
>>> *bfpt_header) {
>>>> +	struct sfdp_bfpt bfpt;
>>>> +	size_t len;
>>>> +	int i, err;
>>>> +	u32 addr;
>>>> +
>>>> +	/* JESD216 Basic Flash Parameter Table length is at least 9 DWORDs.
>>> */
>>>> +	if (bfpt_header->length < BFPT_DWORD_MAX_JESD216)
>>>> +		return -EINVAL;
>>>> +
>>>> +	/* Read the Basic Flash Parameter Table. */
>>>> +	len = min_t(size_t, sizeof(bfpt),
>>>> +		    bfpt_header->length * sizeof(u32));
>>>> +	addr = SFDP_PARAM_HEADER_PTP(bfpt_header);
>>>> +	memset(&bfpt, 0, sizeof(bfpt));
>>>> +	err = spi_flash_read_sfdp(flash, addr, len, &bfpt);
>>>> +	if (err < 0)
>>>> +		return err;
>>>> +
>>>> +	/* Fix endianness of the BFPT DWORDs. */
>>>> +	for (i = 0; i < BFPT_DWORD_MAX; i++)
>>>> +		bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]);
>>>> +
>>>> +	/* Number of address bytes. */
>>>> +	switch (bfpt.dwords[BFPT_DWORD(1)] &
>>> BFPT_DWORD1_ADDRESS_BYTES_MASK) {
>>>> +	case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
>>>> +		flash->addr_width = 3;
>>>> +		break;
>>>> +
>>>> +	case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
>>>> +		printf("SF: Flash defaults to 3-Byte mode; enters 4-Byte ");
>>>> +		printf("mode on command\n");
>>>> +		/*
>>>> +		 * By default, 4-byte addressing mode is set.
>>>> +		 * To enforce 3-byte addressing mode, set addrwd_3_in_use
>>> flag
>>>> +		 * in struct spi_flash for every command.
>>>> +		 */
>>>> +		flash->addr_width = 4;
>>>> +		break;
>>>> +
>>>> +	case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
>>>> +		flash->addr_width = 4;
>>>> +		break;
>>>> +
>>>
>>> I see your are updating flash->addr_width to 4 bytes here. But I don't see
>>> any code in this patch that changes flash->read_cmd (and write/erase
>>> cmds) to use 4B addressing opcodes. Also, I dont see any code that bypasses
>>> BAR register configuration when CONFIG_SPI_FLASH_BAR is defined and
>>> SFDP is available.
>>
>> We don't have any flash that supports CONFIG_SPI_FLASH_BAR. What do you suggest, shall we skip SFDP parsing in case CONFIG_SPI_FLASH_BAR is defined?
>>
> 
> I suggest skipping BAR configuration completely if flash supports
> address width of 4 as per SFDP. BAR configuration is only needed if
> flash does not support 4 byte addressing and flash size is > 16MB

And also some controller do support > 16MB flash, but have in-capable to 
handle 4-byte addressing commands, so for those cases BAR would require.