cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 
slocke
Observer
Observer
208 Views
Registered: ‎01-26-2018

Linux QSPI driver and 4 Byte address

I've ran into an issue with the linux QSPI driver under the xilinx 2020.2 release.  It seems spi_nor.c has changed enough between the 4.x kernels and the 5.4 release that the "workaround" for the lack of 4 byte address support on the Zynq QSPI does not catch all cases.

I have a system that uses an S25FL512S flash.  This is a part that is greater than 16MB and supports 4 byte addressing, but moreover, it also supports SFDP.  Both kernels have a Xilinx workaround in the spi_nor driver to limit the addressing to 3 byte mode, however the new kernel is pulling the 4 byte address support from the SFDP, and dropping out of the if statement at the first clause below.  This means that the "workaround" code is not executed and linux tries to use 4-byte addressing (which obviously doesn't turn out well).

static int spi_nor_set_addr_width(struct spi_nor *nor)
{
	struct device_node *np = spi_nor_get_flash_node(nor);
	struct device_node *np_spi;
	dev_warn(nor->dev, "Setting NOR addr width\n");
	if (nor->addr_width) {
		/* already configured from SFDP */
		dev_warn(nor->dev, "Done in SFDP\n");
	} else if (nor->info->addr_width) {
		dev_warn(nor->dev, "Set in info\n");
		nor->addr_width = nor->info->addr_width;
	} else if (nor->mtd.size > 0x1000000) {
		dev_warn(nor->dev, "Bigger than 16MiB\n");
#ifdef CONFIG_OF
		dev_warn(nor->dev, "Checking compatible string\n");
		np_spi = of_get_next_parent(np);
		if (of_property_match_string(np_spi, "compatible",
					     "xlnx,zynq-qspi-1.0") >= 0) {
			int status;
			dev_warn(nor->dev, "Limiting to 3 byte address for zynq\n");
			nor->addr_width = 3;
			nor->params.set_4byte(nor, false);
			status = read_ear(nor, (struct flash_info *)nor->info);
			if (status < 0)
				dev_warn(nor->dev, "failed to read ear reg\n");
			else
				nor->curbank = status & EAR_SEGMENT_MASK;
		} else {
#endif

The older kernels also read SFDP, but do not parse the 4 Byte address instruction type (SFDP_4BAIT_ID).  The new kernel does in spi_nor_parse_sfdp:

	/* Parse optional parameter tables. */
	for (i = 0; i < header.nph; i++) {
		param_header = &param_headers[i];

		switch (SFDP_PARAM_HEADER_ID(param_header)) {
		case SFDP_SECTOR_MAP_ID:
			err = spi_nor_parse_smpt(nor, param_header, params);
			break;

		case SFDP_4BAIT_ID:
			err = spi_nor_parse_4bait(nor, param_header, params);
			break;

		default:
			break;
		}

		if (err) {
			dev_warn(dev, "Failed to parse optional parameter table: %04x\n",
				 SFDP_PARAM_HEADER_ID(param_header));
			/*
			 * Let's not drop all information we extracted so far
			 * if optional table parsers fail. In case of failing,
			 * each optional parser is responsible to roll back to
			 * the previously known spi_nor data.
			 */
			err = 0;
		}
	}

I think this means that any quad (or dual) NOR that is larger than 16MB and reports 4 byte addressing support in SFDP will not work with the Zynq QSPI driver in linux (without modifying the kernel).

Since the SFDP info (spi_nor_parse_4bait) sets up all of the 4-byte addressing parameters, even forcing execution of the xilinx "workaround" doesn't help (as the read/write/erase commands are all set up for the NOR to use 4 byte addressing versions).  What really has to be done is to ignore the 4 byte info from SFDP altogether.  I'm not sure what the most graceful (and universally useful) way to accomplish this is, but the entire "spi_nor_parse_4bait()" call above could probably be removed for any NOR attached to a Zynq QSPI (as anything reported about 4B mode in the flash would probably break this configuration anyway).  This is what I did to "fix" the issue:

	/* Parse optional parameter tables. */
	for (i = 0; i < header.nph; i++) {
		param_header = &param_headers[i];

		switch (SFDP_PARAM_HEADER_ID(param_header)) {
		case SFDP_SECTOR_MAP_ID:
			err = spi_nor_parse_smpt(nor, param_header, params);
			break;

		case SFDP_4BAIT_ID:
#ifdef CONFIG_OF
			np_spi = of_get_next_parent(np);
			if (of_property_match_string(np_spi, "compatible",
						     "xlnx,zynq-qspi-1.0") < 0)
#endif
				err = spi_nor_parse_4bait(nor, param_header, params);
			break;

		default:
			break;
		}

		if (err) {
			dev_warn(dev, "Failed to parse optional parameter table: %04x\n",
				 SFDP_PARAM_HEADER_ID(param_header));
			/*
			 * Let's not drop all information we extracted so far
			 * if optional table parsers fail. In case of failing,
			 * each optional parser is responsible to roll back to
			 * the previously known spi_nor data.
			 */
			err = 0;
		}
	}

Hopefully it is useful to someone else until Xilinx officially fixes the issue.

0 Kudos
0 Replies