dt-bindings: spi: dw: add cs-override property

Update the dw-apb-ssi binding document, to add an optional 'cs-override' devicetree property.

This property adds the ability for dw spi controller driver to work with the dw spi controller found on Alpine chips.

The dw spi controller has an auto-deselect of Chip-Select, in case there is no data inside the Tx FIFO. While working on platforms with Alpine chips, auto-deselect mode causes an issue for some spi devices that can't handle the Chip-Select deselect in the middle of a transaction. It is a normal behavior for a Tx FIFO to be empty in the middle of a transaction, due to busy cpu. In the Alpine chip family an option to change the default behavior was added to the original dw spi controller to prevent this issue of de-asserting Chip-Select once TX FIFO is empty. The change was to allow SW to force the Chip-Select. With this change, as long as the Slave Enable Register is asserted, the Chip-Select will be asserted. As a result, it is necessary to deselect the Slave Select Register once the transaction is done. This feature is enabled via a device property called 'cs-override'. Once the driver identifies the 'cs-override' property, it enables the hw fixup logic, by writing to a dedicated register found in the IP reserved area and will starts deselecting the Slave Select Register when the transfer ends.

---
 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 2 ++
 1 file changed, 2 insertions(+)

index 642d3fb..dd366f5 100644
@@ -14,6 +14,8 @@ Optional properties:
 - num-cs : The number of chipselects. If omitted, this will default to 4.
 - reg-io-width : The I/O register width (in bytes) implemented by this
   device.  Supported values are 2 or 4 (the default).
+- cs-override: Enable explicit Chip-Select deselect during transfer instead of
+  default auto-deselect upon tx FIFO becoming empty.
 
 Child nodes as per the generic SPI binding.
 

2.7.4

This compatible adds the ability for dw spi controller driver to work with the dw spi controller found on Alpine chips.

The dw spi controller has an auto-deselect of Chip-Select, in case there is no data inside the Tx FIFO. While working on platforms with Alpine chips, auto-deselect mode causes an issue for some spi devices that can't handle the Chip-Select deselect in the middle of a transaction. It is a normal behavior for a Tx FIFO to be empty in the middle of a transaction, due to busy cpu. In the Alpine chip family an option to change the default behavior was added to the original dw spi controller to prevent this issue of de-asserting Chip-Select once TX FIFO is empty. The change was to allow SW manual control of the Chip-Select. With this change, as long as the Slave Enable Register is asserted, the Chip-Select will be asserted. As a result, it is necessary to deselect the Slave Select Register once the transaction is done. This feature is enabled via a new device compatible string called 'al,alpine-dw-apb-ssi'. Once the driver identifies the new compatible string, it enables the hw fixup logic, by writing to a dedicated register found in the IP reserved area and will start manual deselecting the Slave Select Register when the transfer ends.

---
 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 642d3fb..d25b1f8 100644
@@ -2,7 +2,7 @@ Synopsys DesignWare AMBA 2.0 Synchronous Serial Interface.
 
 Required properties:
 - compatible : "snps,dw-apb-ssi" or "mscc,<soc>-spi", where soc is "ocelot" or
-  "jaguar2"
+  "jaguar2", or "al,alpine-dw-apb-ssi"
 - reg : The register base for the controller. For "mscc,<soc>-spi", a second
   register set is required (named ICPU_CFG:SPI_MST)
 - interrupts : One interrupt, used by the controller.

2.7.4

Add support for a new devicetree compatible string called 'al,alpine-apb-ssi', which is necessary for the Amazon Alpine spi controller. 'al,alpine-dw-apb-ssi' is used in the dw spi driver if specified in the devicetree. Otherwise, fall back to driver default behavior, i.e. original dw IP hw driver behavior.

---
 drivers/spi/spi-dw-mmio.c | 9 +++++++++
 drivers/spi/spi-dw.c      | 6 ++++++
 drivers/spi/spi-dw.h      | 2 ++
 3 files changed, 17 insertions(+)

index a768461..88b04d2 100644
@@ -126,6 +126,14 @@ static int dw_spi_mscc_jaguar2_init(struct platform_device *pdev,
 				JAGUAR2_IF_SI_OWNER_OFFSET);
 }
 
+static int dw_spi_alpine_init(struct platform_device *pdev,
+			      struct dw_spi_mmio *dwsmmio)
+{
+	dwsmmio->dws.cs_override = 1;
+
+	return 0;
+}
+
 static int dw_spi_mmio_probe(struct platform_device *pdev)
 {
 	int (*init_func)(struct platform_device *pdev,

@@ -230,6 +238,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
 	{ .compatible = "snps,dw-apb-ssi", },
 	{ .compatible = "mscc,ocelot-spi", .data = dw_spi_mscc_ocelot_init},
 	{ .compatible = "mscc,jaguar2-spi", .data = dw_spi_mscc_jaguar2_init},
+	{ .compatible = "al,alpine-dw-apb-ssi", .data = dw_spi_alpine_init},
 	{ /* end of table */}
 };
 MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);

index 3e205ab..b705f2b 100644
@@ -144,6 +144,8 @@ void dw_spi_set_cs(struct spi_device *spi, bool enable)
 
 	if (!enable)
 		dw_writel(dws, DW_SPI_SER, BIT(spi->chip_select));
+	else if (dws->cs_override)
+		dw_writel(dws, DW_SPI_SER, 0);
 }
 EXPORT_SYMBOL_GPL(dw_spi_set_cs);
 

@@ -463,6 +465,10 @@ static void spi_hw_init(struct device *dev, struct dw_spi *dws)
 		dws->fifo_len = (fifo == 1) ? 0 : fifo;
 		dev_dbg(dev, "Detected FIFO size: %u bytes\n", dws->fifo_len);
 	}
+
+	/* enable HW fixup for explicit CS deselect for Amazon's alpine chip */
+	if (dws->cs_override)
+		dw_writel(dws, DW_SPI_CS_OVERRIDE, 0xF);
 }
 
 int dw_spi_add_host(struct device *dev, struct dw_spi *dws)

index 0168b08..c9c1588 100644
@@ -32,6 +32,7 @@
 #define DW_SPI_IDR			0x58
 #define DW_SPI_VERSION			0x5c
 #define DW_SPI_DR			0x60
+#define DW_SPI_CS_OVERRIDE		0xf4
 
 /* Bit fields in CTRLR0 */
 #define SPI_DFS_OFFSET			0

@@ -109,6 +110,7 @@ struct dw_spi {
 	u32			fifo_len;	/* depth of the FIFO buffer */
 	u32			max_freq;	/* max bus freq supported */
 
+	int			cs_override;
 	u32			reg_io_width;	/* DR I/O width in bytes */
 	u16			bus_num;
 	u16			num_cs;		/* supported slave numbers */

2.7.4

Shouldn't this be "amzn,alpine-apb-ssi"? The convention, and this IS documented, is to the use company's stock ticker symbol as the prefix, so as to have something grounded in the real world. I don't know of anyone else using a product line name as the company name in their dt bindings. Example, "snps,dw-apb-ssi", is named for Synopsis, not the Designware line.

This compatible adds the ability for dw spi controller driver to work with the dw spi controller found on Alpine chips.

The dw spi controller has an auto-deselect of Chip-Select, in case there is no data inside the Tx FIFO. While working on platforms with Alpine chips, auto-deselect mode causes an issue for some spi devices that can't handle the Chip-Select deselect in the middle of a transaction. It is a normal behavior for a Tx FIFO to be empty in the middle of a transaction, due to busy cpu. In the Alpine chip family an option to change the default behavior was added to the original dw spi controller to prevent this issue of de-asserting Chip-Select once TX FIFO is empty. The change was to allow SW manual control of the Chip-Select. With this change, as long as the Slave Enable Register is asserted, the Chip-Select will be asserted. As a result, it is necessary to deselect the Slave Select Register once the transaction is done. This feature is enabled via a new device compatible string called 'amazon,alpine-dw-apb-ssi'. Once the driver identifies the new compatible string, it enables the hw fixup logic, by writing to a dedicated register found in the IP reserved area and will start manual deselecting the Slave Select Register when the transfer ends.

---
 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 642d3fb..2864bc6 100644
@@ -2,7 +2,7 @@ Synopsys DesignWare AMBA 2.0 Synchronous Serial Interface.
 
 Required properties:
 - compatible : "snps,dw-apb-ssi" or "mscc,<soc>-spi", where soc is "ocelot" or
-  "jaguar2"
+  "jaguar2", or "amazon,alpine-dw-apb-ssi"
 - reg : The register base for the controller. For "mscc,<soc>-spi", a second
   register set is required (named ICPU_CFG:SPI_MST)
 - interrupts : One interrupt, used by the controller.

2.7.4

Add support for a new devicetree compatible string called 'amazon,alpine-apb-ssi', which is necessary for the Amazon Alpine spi controller. 'amazon,alpine-dw-apb-ssi' is used in the dw spi driver if specified in the devicetree. Otherwise, fall back to driver default behavior, i.e. original dw IP hw driver behavior.

---
 drivers/spi/spi-dw-mmio.c | 9 +++++++++
 drivers/spi/spi-dw.c      | 6 ++++++
 drivers/spi/spi-dw.h      | 2 ++
 3 files changed, 17 insertions(+)

index a768461..3ffb6a40 100644
@@ -126,6 +126,14 @@ static int dw_spi_mscc_jaguar2_init(struct platform_device *pdev,
 				JAGUAR2_IF_SI_OWNER_OFFSET);
 }
 
+static int dw_spi_alpine_init(struct platform_device *pdev,
+			      struct dw_spi_mmio *dwsmmio)
+{
+	dwsmmio->dws.cs_override = 1;
+
+	return 0;
+}
+
 static int dw_spi_mmio_probe(struct platform_device *pdev)
 {
 	int (*init_func)(struct platform_device *pdev,

@@ -230,6 +238,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
 	{ .compatible = "snps,dw-apb-ssi", },
 	{ .compatible = "mscc,ocelot-spi", .data = dw_spi_mscc_ocelot_init},
 	{ .compatible = "mscc,jaguar2-spi", .data = dw_spi_mscc_jaguar2_init},
+	{ .compatible = "amazon,alpine-dw-apb-ssi", .data = dw_spi_alpine_init},
 	{ /* end of table */}
 };
 MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);

index 3e205ab..b705f2b 100644
@@ -144,6 +144,8 @@ void dw_spi_set_cs(struct spi_device *spi, bool enable)
 
 	if (!enable)
 		dw_writel(dws, DW_SPI_SER, BIT(spi->chip_select));
+	else if (dws->cs_override)
+		dw_writel(dws, DW_SPI_SER, 0);
 }
 EXPORT_SYMBOL_GPL(dw_spi_set_cs);
 

@@ -463,6 +465,10 @@ static void spi_hw_init(struct device *dev, struct dw_spi *dws)
 		dws->fifo_len = (fifo == 1) ? 0 : fifo;
 		dev_dbg(dev, "Detected FIFO size: %u bytes\n", dws->fifo_len);
 	}
+
+	/* enable HW fixup for explicit CS deselect for Amazon's alpine chip */
+	if (dws->cs_override)
+		dw_writel(dws, DW_SPI_CS_OVERRIDE, 0xF);
 }
 
 int dw_spi_add_host(struct device *dev, struct dw_spi *dws)

index 0168b08..c9c1588 100644
@@ -32,6 +32,7 @@
 #define DW_SPI_IDR			0x58
 #define DW_SPI_VERSION			0x5c
 #define DW_SPI_DR			0x60
+#define DW_SPI_CS_OVERRIDE		0xf4
 
 /* Bit fields in CTRLR0 */
 #define SPI_DFS_OFFSET			0

@@ -109,6 +110,7 @@ struct dw_spi {
 	u32			fifo_len;	/* depth of the FIFO buffer */
 	u32			max_freq;	/* max bus freq supported */
 
+	int			cs_override;
 	u32			reg_io_width;	/* DR I/O width in bytes */
 	u16			bus_num;
 	u16			num_cs;		/* supported slave numbers */

2.7.4

The patch

dw: spi: add support for Amazon's Alpine spi controller

has been applied to the spi tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

From: Talel Shenhar <[email protected]>
Date: Thu, 11 Oct 2018 14:20:07 +0300
Subject: [PATCH] dw: spi: add support for Amazon's Alpine spi controller

Add support for a new devicetree compatible string called 'amazon,alpine-apb-ssi', which is necessary for the Amazon Alpine spi controller. 'amazon,alpine-dw-apb-ssi' is used in the dw spi driver if specified in the devicetree. Otherwise, fall back to driver default behavior, i.e. original dw IP hw driver behavior.

---
 drivers/spi/spi-dw-mmio.c | 9 +++++++++
 drivers/spi/spi-dw.c      | 6 ++++++
 drivers/spi/spi-dw.h      | 2 ++
 3 files changed, 17 insertions(+)

index a768461614a0..3ffb6a40fe0c 100644
@@ -126,6 +126,14 @@ static int dw_spi_mscc_jaguar2_init(struct platform_device *pdev,
 				JAGUAR2_IF_SI_OWNER_OFFSET);
 }
 
+static int dw_spi_alpine_init(struct platform_device *pdev,
+			      struct dw_spi_mmio *dwsmmio)
+{
+	dwsmmio->dws.cs_override = 1;
+
+	return 0;
+}
+
 static int dw_spi_mmio_probe(struct platform_device *pdev)
 {
 	int (*init_func)(struct platform_device *pdev,

@@ -230,6 +238,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
 	{ .compatible = "snps,dw-apb-ssi", },
 	{ .compatible = "mscc,ocelot-spi", .data = dw_spi_mscc_ocelot_init},
 	{ .compatible = "mscc,jaguar2-spi", .data = dw_spi_mscc_jaguar2_init},
+	{ .compatible = "amazon,alpine-dw-apb-ssi", .data = dw_spi_alpine_init},
 	{ /* end of table */}
 };
 MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);

index 3e205ab60cd4..b705f2bdb8b9 100644
@@ -144,6 +144,8 @@ void dw_spi_set_cs(struct spi_device *spi, bool enable)
 
 	if (!enable)
 		dw_writel(dws, DW_SPI_SER, BIT(spi->chip_select));
+	else if (dws->cs_override)
+		dw_writel(dws, DW_SPI_SER, 0);
 }
 EXPORT_SYMBOL_GPL(dw_spi_set_cs);
 

@@ -463,6 +465,10 @@ static void spi_hw_init(struct device *dev, struct dw_spi *dws)
 		dws->fifo_len = (fifo == 1) ? 0 : fifo;
 		dev_dbg(dev, "Detected FIFO size: %u bytes\n", dws->fifo_len);
 	}
+
+	/* enable HW fixup for explicit CS deselect for Amazon's alpine chip */
+	if (dws->cs_override)
+		dw_writel(dws, DW_SPI_CS_OVERRIDE, 0xF);
 }
 
 int dw_spi_add_host(struct device *dev, struct dw_spi *dws)

index 0168b08364d5..c9c15881e982 100644
@@ -32,6 +32,7 @@
 #define DW_SPI_IDR			0x58
 #define DW_SPI_VERSION			0x5c
 #define DW_SPI_DR			0x60
+#define DW_SPI_CS_OVERRIDE		0xf4
 
 /* Bit fields in CTRLR0 */
 #define SPI_DFS_OFFSET			0

@@ -109,6 +110,7 @@ struct dw_spi {
 	u32			fifo_len;	/* depth of the FIFO buffer */
 	u32			max_freq;	/* max bus freq supported */
 
+	int			cs_override;
 	u32			reg_io_width;	/* DR I/O width in bytes */
 	u16			bus_num;
 	u16			num_cs;		/* supported slave numbers */

2.19.0.rc2

Please don't send new series in reply to old ones, especially if you're not versioning things, as it makes it much harder to track what's going on.

The patch

spi: dw: add compatible for Amazon's Alpine spi controller

has been applied to the spi tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

From: Talel Shenhar <[email protected]>
Date: Thu, 11 Oct 2018 14:20:06 +0300
Subject: [PATCH] spi: dw: add compatible for Amazon's Alpine spi controller

This compatible adds the ability for dw spi controller driver to work with the dw spi controller found on Alpine chips.

The dw spi controller has an auto-deselect of Chip-Select, in case there is no data inside the Tx FIFO. While working on platforms with Alpine chips, auto-deselect mode causes an issue for some spi devices that can't handle the Chip-Select deselect in the middle of a transaction. It is a normal behavior for a Tx FIFO to be empty in the middle of a transaction, due to busy cpu. In the Alpine chip family an option to change the default behavior was added to the original dw spi controller to prevent this issue of de-asserting Chip-Select once TX FIFO is empty. The change was to allow SW manual control of the Chip-Select. With this change, as long as the Slave Enable Register is asserted, the Chip-Select will be asserted. As a result, it is necessary to deselect the Slave Select Register once the transaction is done. This feature is enabled via a new device compatible string called 'amazon,alpine-dw-apb-ssi'. Once the driver identifies the new compatible string, it enables the hw fixup logic, by writing to a dedicated register found in the IP reserved area and will start manual deselecting the Slave Select Register when the transfer ends.

---
 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 642d3fb1ef85..2864bc6b659c 100644
@@ -2,7 +2,7 @@ Synopsys DesignWare AMBA 2.0 Synchronous Serial Interface.
 
 Required properties:
 - compatible : "snps,dw-apb-ssi" or "mscc,<soc>-spi", where soc is "ocelot" or
-  "jaguar2"
+  "jaguar2", or "amazon,alpine-dw-apb-ssi"
 - reg : The register base for the controller. For "mscc,<soc>-spi", a second
   register set is required (named ICPU_CFG:SPI_MST)
 - interrupts : One interrupt, used by the controller.

2.19.0.rc2

updated the vendor prefix to "amazon" as described in bindings/vendor-prefixes.txt.

Why would we ever want to use this behaviour? It will be broken for any non-trivial SPI message such as those made with multiple transfers anyway. Why not just unconditionally control it manually?

Thank you for your prompt response.

This behavior (auto-deselect of Chip-Select) is the default behavior of dw spi controller hw.
On Alpine chip there is additional behavior added to the dw spi controller hw that allows the sw to disable this behavior. This patch allows the dw driver to enable this hw workaround and add the needed sw manual control for it.

In particular, that would have been an incompatible change and would render the hardware non-function with existing drivers. By making it opt-in, old drivers work.... as long as the FIFO doesn't stall in the middle of a transaction, which no worse than with the "standard" DW implementation anyway.

This behavior (auto-deselect of Chip-Select) is the default behavior of dw spi controller hw.
On Alpine chip there is additional behavior added to the dw spi controller hw that allows the sw to disable this behavior. This patch allows the dw driver to enable this hw workaround and add the needed sw manual control for it.

If this is a modified IP with additional features then it should be given a new compatible string rather than having a property - it's not just configuration of the existing IP, it's a new thing and we may find there are other quirks that have to be taken care of for it.

No.

It's an extension of the existing IP which is explicitly designed to be compatible with existing drivers via an opt-in feature.

Which is exactly what we've spent the last decade or two trying to beat hardware designers into doing, instead of randomly breaking compatibility for no good reason.

Amazon Development Centre (London) Ltd. Registered in England and Wales with registration number 04543232 with its registered office at 1 Principal Place, Worship Street, London EC2A 2FA, United Kingdom.

That's totally normal and something that there's already infrastructure to handle, plenty of other IPs have new versions with new features - you can just add a new compatible string and use that to decide which features to enable, and if it's backwards compatible with old versions of the driver supporting older hardware then you can list multiple compatibles. As we just discussed on IRC this is partly a policy thing now I've been told that it's a modified version of the hardware rather than a configuration or integration option, it's easier than having implementations of new features trickle out from the vendor and needing to enable them all one by one in device trees (which does happen).

That's great and we get to reuse all the driver code with a quirk (a quirk which fixes the hardware to be more compatible with devices, this is a really good hardware change). Ideally we'd be able to enumerate things like IP versions and options from hardware but that's a more entertaining problem.

Having said all this if there are production systems using this property, especially production systems where people other than the system integrator can realistically deploy their own kernel separate to the device tree, then supporting those existing DTs even if they're not doing the ideal thing might be the best thing. You mentioned that this might be the case, can you check what the status is there please?

Might well be possible here. There are an id code and version registers that could be used. Did anyone have the foresight to change them would modifying the IP is the question.

I've developed systems that used chips with this SPI master. I used GPIO chip selects to work around the existing behavior and would not be negatively affected if someone fixed this bug.

There's also another quirk where certain phase/polarity combination make it generate a CS pulse between each word.

IMHO, Linux SPI has a specification for how it works. Setup an xfer a certain way and you get a certain waveform from the master. If the master generates the wrong waveform then it's a bug. Designers should know better than to design a system that depends on a bug never being fixed and be caught off guard when it is. If you want CS to turn off whenever the FIFO is empty (why???) then extend the spec to encompass the behavior you want. Like how "pulse between words" can be specified now.

Add support for an optional devicetree property called 'cs-override', which is necessary for the Amazon Alpine spi controller. 'cs-override' is used in the dw spi driver if specified in the devicetree. Otherwise, fall back to driver default behavior, i.e., original dw IP hw driver behavior.

---
 drivers/spi/spi-dw-mmio.c | 2 ++
 drivers/spi/spi-dw.c      | 6 ++++++
 drivers/spi/spi-dw.h      | 2 ++
 3 files changed, 10 insertions(+)

index a768461..a2f7359 100644
@@ -195,6 +195,8 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
 					goto out;
 			}
 		}
+		dws->cs_override = of_property_read_bool(pdev->dev.of_node,
+							 "cs-override");
 	}
 
 	init_func = device_get_match_data(&pdev->dev);

index 3e205ab..eb4c3d1 100644
@@ -144,6 +144,8 @@ void dw_spi_set_cs(struct spi_device *spi, bool enable)
 
 	if (!enable)
 		dw_writel(dws, DW_SPI_SER, BIT(spi->chip_select));
+	else
+		dw_writel(dws, DW_SPI_SER, 0);
 }
 EXPORT_SYMBOL_GPL(dw_spi_set_cs);
 

@@ -463,6 +465,10 @@ static void spi_hw_init(struct device *dev, struct dw_spi *dws)
 		dws->fifo_len = (fifo == 1) ? 0 : fifo;
 		dev_dbg(dev, "Detected FIFO size: %u bytes\n", dws->fifo_len);
 	}
+
+	/* enable HW fixup for explicit CS deselect for Amazon's alpine chip */
+	if (dws->cs_override)
+		dw_writel(dws, DW_SPI_CS_OVERRIDE, 0xF);
 }
 
 int dw_spi_add_host(struct device *dev, struct dw_spi *dws)

index 0168b08..c9c1588 100644
@@ -32,6 +32,7 @@
 #define DW_SPI_IDR			0x58
 #define DW_SPI_VERSION			0x5c
 #define DW_SPI_DR			0x60
+#define DW_SPI_CS_OVERRIDE		0xf4
 
 /* Bit fields in CTRLR0 */
 #define SPI_DFS_OFFSET			0

@@ -109,6 +110,7 @@ struct dw_spi {
 	u32			fifo_len;	/* depth of the FIFO buffer */
 	u32			max_freq;	/* max bus freq supported */
 
+	int			cs_override;
 	u32			reg_io_width;	/* DR I/O width in bytes */
 	u16			bus_num;
 	u16			num_cs;		/* supported slave numbers */

2.7.4

So that's the problem! I, like everyone else I suspect, switched to using GPIO chip selects with this driver because of this. I narrowed it down to a CS de-assert when the bus switched from TX to RX, which of course makes a SPI register read fail on most devices. The TX FIFO would empty at this point, so that would explain it.

Did the designers of this IP ever read a SPI device datasheet???

Got to agree with Mark Brown, why would anyone ever want to NOT have it work properly? The previous behavior is not "alternate correct", it's Broken.

This patch allow the Amazon changed hw to work in a correct way. Unfortunately, the original hw doesn't support auto-deselect disable. auto-deselect disable is a hw fixup Amazon hw engineers added on top of the original dw IP.
The fix was to disable the auto-deselect and to allow sw to manually control the chip-select.
This patch enables the above described Amazon hw fixup and adds manual control over chip-select.

That's generally a good idea.

This isn't even that unusual an innovation for hardware to have, for some reason automatic chip select management is *really* popular and rarely helpful. It's far from the most entertaining thing I've seen hardware do.