[PATCH 00/15] MIPS: pistachio: Creator Ci40 aka Marduk SPI-UART

[PATCH 00/15] MIPS: pistachio: Creator Ci40 aka Marduk SPI-UART

[Andreas Färber]

Hello,

This patchset enables SPI and UARTs for the Ci40's mikroBUS and Raspberry Pi B+
connectors, as well as the remaining seven LEDs and as dependency the 802.15.4.

I also dug out some related driver fixes from their pistachio-4.9-lede branch:
https://github.com/CreatorDev/linux/commits/pistachio-4.9-lede

Tested with a LoRa click expansion board. [*]
Note that a number of drivers are missing in the pistachio defconfig. This
should probably be remedied as follow-up.

While all creatordev.io websites appear to be gone, I still had a local copy
of the Ci40 schematics document.

This patchset: https://github.com/afaerber/linux/commits/ci40-spi-uart.v1

Some more potential patches for missing features appear to be here:
https://github.com/CreatorDev/linux/commits/pistachio-4.9-wip

Have a lot of fun!

Cheers,
Andreas

P.S. As follow-up to this series I've reported on linux-serial that sc16is7xx
and serdev drivers don't appear to play nicely together:
https://marc.info/?l=linux-serial&m=153228314300681&w=2

[*] Cf. netdev https://patchwork.ozlabs.org/cover/937545/

Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org

Cc: Mark Brown <broonie@kernel.org>
Cc: linux-spi@vger.kernel.org

Cc: Vinod Koul <vkoul@kernel.org>
Cc: dmaengine@vger.kernel.org

Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: linux-clk@vger.kernel.org

Andreas Färber (7):
  MIPS: dts: img: pistachio_marduk: Reorder nodes
  MIPS: dts: img: pistachio_marduk: Cleanups
  MIPS: dts: img: pistachio: Rename spim0-clk pin node label
  MIPS: dts: img: pistachio_marduk: Enable SPIM0
  MIPS: dts: img: pistachio_marduk: Add 6Lowpan node
  MIPS: dts: img: pistachio_marduk: Add SPI UART node
  MIPS: dts: img: pistachio_marduk: Add user LEDs

Damien Horsley (1):
  dmaengine: img-mdc: Handle early status read

Govindraj Raja (1):
  clk: pistachio: Fix wrong SDHost card speed

Ian Pozella (1):
  MIPS: dts: img: pistachio_marduk: Switch mmc to 1 bit mode

Ionela Voinescu (5):
  spi: img-spfi: Implement dual and quad mode
  spi: img-spfi: Set device select bits for SPFI port state
  spi: img-spfi: Use device 0 configuration for all devices
  spi: img-spfi: RX maximum burst size for DMA is 8
  spi: img-spfi: Finish every transfer cleanly

 arch/mips/boot/dts/img/pistachio.dtsi       |   2 +-
 arch/mips/boot/dts/img/pistachio_marduk.dts | 174 ++++++++++++++++++++++------
 drivers/clk/pistachio/clk-pistachio.c       |   3 +-
 drivers/dma/img-mdc-dma.c                   |  40 ++++---
 drivers/spi/spi-img-spfi.c                  | 165 +++++++++++++++++++++-----
 include/dt-bindings/clock/pistachio-clk.h   |   1 +
 6 files changed, 299 insertions(+), 86 deletions(-)

-- 
2.16.4

[PATCH 01/15] MIPS: dts: img: pistachio_marduk: Reorder nodes

[Andreas Färber]

Consistently order nodes referenced by label alphabetically.
No functional changes. This prepares for adding nodes.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 arch/mips/boot/dts/img/pistachio_marduk.dts | 76 ++++++++++++++---------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts b/arch/mips/boot/dts/img/pistachio_marduk.dts
index cf9cebd52294..f03f4114e645 100644
--- a/arch/mips/boot/dts/img/pistachio_marduk.dts
+++ b/arch/mips/boot/dts/img/pistachio_marduk.dts
@@ -74,40 +74,34 @@
 	};
 };
 
-&internal_dac {
-	VDD-supply = <&internal_dac_supply>;
-};
-
-&spfi1 {
+&adc {
 	status = "okay";
-
-	pinctrl-0 = <&spim1_pins>, <&spim1_quad_pins>, <&spim1_cs0_pin>,
-		    <&spim1_cs1_pin>;
-	pinctrl-names = "default";
-	cs-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>, <&gpio0 1 GPIO_ACTIVE_HIGH>;
-
-	flash@0 {
-		compatible = "spansion,s25fl016k", "jedec,spi-nor";
-		reg = <0>;
-		spi-max-frequency = <50000000>;
-	};
+	vref-supply = <&reg_1v8>;
+	adc-reserved-channels = <0x10>;
 };
 
-&uart0 {
+&enet {
 	status = "okay";
-	assigned-clock-rates = <114278400>, <1843200>;
 };
 
-&uart1 {
+&i2c2 {
 	status = "okay";
+	clock-frequency = <400000>;
+
+	tpm@20 {
+		compatible = "infineon,slb9645tt";
+		reg = <0x20>;
+	};
+
 };
 
-&usb {
+&i2c3 {
 	status = "okay";
+	clock-frequency = <400000>;
 };
 
-&enet {
-	status = "okay";
+&internal_dac {
+	VDD-supply = <&internal_dac_supply>;
 };
 
 &pin_enet {
@@ -118,12 +112,6 @@
 	drive-strength = <2>;
 };
 
-&sdhost {
-	status = "okay";
-	bus-width = <4>;
-	disable-wp;
-};
-
 &pin_sdhost_cmd {
 	drive-strength = <2>;
 };
@@ -140,24 +128,36 @@
 	pinctrl-names = "default";
 };
 
-&adc {
+&sdhost {
 	status = "okay";
-	vref-supply = <&reg_1v8>;
-	adc-reserved-channels = <0x10>;
+	bus-width = <4>;
+	disable-wp;
 };
 
-&i2c2 {
+&spfi1 {
 	status = "okay";
-	clock-frequency = <400000>;
 
-	tpm@20 {
-		compatible = "infineon,slb9645tt";
-		reg = <0x20>;
+	pinctrl-0 = <&spim1_pins>, <&spim1_quad_pins>, <&spim1_cs0_pin>,
+		    <&spim1_cs1_pin>;
+	pinctrl-names = "default";
+	cs-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>, <&gpio0 1 GPIO_ACTIVE_HIGH>;
+
+	flash@0 {
+		compatible = "spansion,s25fl016k", "jedec,spi-nor";
+		reg = <0>;
+		spi-max-frequency = <50000000>;
 	};
+};
 
+&uart0 {
+	status = "okay";
+	assigned-clock-rates = <114278400>, <1843200>;
 };
 
-&i2c3 {
+&uart1 {
+	status = "okay";
+};
+
+&usb {
 	status = "okay";
-	clock-frequency = <400000>;
 };
-- 
2.16.4

[PATCH 02/15] MIPS: dts: img: pistachio_marduk: Cleanups

[Andreas Färber]

Add and remove some white lines for consistency.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 arch/mips/boot/dts/img/pistachio_marduk.dts | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts b/arch/mips/boot/dts/img/pistachio_marduk.dts
index f03f4114e645..29358d1f7027 100644
--- a/arch/mips/boot/dts/img/pistachio_marduk.dts
+++ b/arch/mips/boot/dts/img/pistachio_marduk.dts
@@ -51,6 +51,7 @@
 
 	leds {
 		compatible = "pwm-leds";
+
 		heartbeat {
 			label = "marduk:red:heartbeat";
 			pwms = <&pwm 3 300000>;
@@ -61,11 +62,13 @@
 
 	keys {
 		compatible = "gpio-keys";
+
 		button@1 {
 			label = "Button 1";
 			linux,code = <0x101>; /* BTN_1 */
 			gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
 		};
+
 		button@2 {
 			label = "Button 2";
 			linux,code = <0x102>; /* BTN_2 */
@@ -92,7 +95,6 @@
 		compatible = "infineon,slb9645tt";
 		reg = <0x20>;
 	};
-
 };
 
 &i2c3 {
-- 
2.16.4

[PATCH 03/15] MIPS: dts: img: pistachio: Rename spim0-clk pin node label

[Andreas Färber]

Renaming will allow consistent ordering when referenced.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 arch/mips/boot/dts/img/pistachio.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/boot/dts/img/pistachio.dtsi b/arch/mips/boot/dts/img/pistachio.dtsi
index f8d7e6f622cb..1adef5bc740d 100644
--- a/arch/mips/boot/dts/img/pistachio.dtsi
+++ b/arch/mips/boot/dts/img/pistachio.dtsi
@@ -406,7 +406,7 @@
 				function = "spim0";
 				drive-strength = <4>;
 			};
-			spim0_clk: spim0-clk {
+			pin_spim0_clk: spim0-clk {
 				pins = "mfio8";
 				function = "spim0";
 				drive-strength = <4>;
-- 
2.16.4

[PATCH 04/15] MIPS: dts: img: pistachio_marduk: Switch mmc to 1 bit mode

[Andreas Färber]

From: Ian Pozella <Ian.Pozella@imgtec.com>

The mmc block in Pistachio allows 1 to 8 data bits to be used.
Marduk uses 4 bits allowing the upper 4 bits to be allocated
to the Mikrobus ports. However these bits are still connected
internally meaning the mmc block recieves signals on all data lines
and seems the internal HW CRC checks get corrupted by this erroneous
data.

We cannot control what data is sent on these lines because they go
to external ports. 1 bit mode does not exhibit the issue hence the
safe default is to use this. If a user knows that in their use case
they will not use the upper bits then they can set to 4 bit mode in
order to improve performance.

Also make sure that the upper 4 bits don't get allocated to the mmc
driver (the default is to assign all 8 pins) so they can be allocated
to other drivers. Allocating all 4 despite setting 1 bit mode as this
matches what is there in hardware.

Signed-off-by: Ian Pozella <Ian.Pozella@imgtec.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 arch/mips/boot/dts/img/pistachio_marduk.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts b/arch/mips/boot/dts/img/pistachio_marduk.dts
index 29358d1f7027..5557a6ad61c3 100644
--- a/arch/mips/boot/dts/img/pistachio_marduk.dts
+++ b/arch/mips/boot/dts/img/pistachio_marduk.dts
@@ -120,6 +120,7 @@
 
 &pin_sdhost_data {
 	drive-strength = <2>;
+	pins = "mfio17", "mfio18", "mfio19", "mfio20";
 };
 
 &pwm {
@@ -132,7 +133,7 @@
 
 &sdhost {
 	status = "okay";
-	bus-width = <4>;
+	bus-width = <1>;
 	disable-wp;
 };
 
-- 
2.16.4

Re: [PATCH 04/15] MIPS: dts: img: pistachio_marduk: Switch mmc to 1 bit mode

[Andreas Färber]

Am 22.07.2018 um 23:19 schrieb Andreas Färber:
> From: Ian Pozella <Ian.Pozella@imgtec.com>
> 
> The mmc block in Pistachio allows 1 to 8 data bits to be used.
> Marduk uses 4 bits allowing the upper 4 bits to be allocated
> to the Mikrobus ports. However these bits are still connected
> internally meaning the mmc block recieves signals on all data lines

"receives"

I had fixed a number of typos from the downstream patches already, but
this one slipped through.

Regards,
Andreas

> and seems the internal HW CRC checks get corrupted by this erroneous
> data.
> 
> We cannot control what data is sent on these lines because they go
> to external ports. 1 bit mode does not exhibit the issue hence the
> safe default is to use this. If a user knows that in their use case
> they will not use the upper bits then they can set to 4 bit mode in
> order to improve performance.
> 
> Also make sure that the upper 4 bits don't get allocated to the mmc
> driver (the default is to assign all 8 pins) so they can be allocated
> to other drivers. Allocating all 4 despite setting 1 bit mode as this
> matches what is there in hardware.
> 
> Signed-off-by: Ian Pozella <Ian.Pozella@imgtec.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
[snip]

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

[PATCH 05/15] MIPS: dts: img: pistachio_marduk: Enable SPIM0

[Andreas Färber]

SPIM0 supplies SPI pins on the mikroBUS and Raspberry Pi B+ connectors.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 arch/mips/boot/dts/img/pistachio_marduk.dts | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts b/arch/mips/boot/dts/img/pistachio_marduk.dts
index 5557a6ad61c3..d723b68084c9 100644
--- a/arch/mips/boot/dts/img/pistachio_marduk.dts
+++ b/arch/mips/boot/dts/img/pistachio_marduk.dts
@@ -123,6 +123,14 @@
 	pins = "mfio17", "mfio18", "mfio19", "mfio20";
 };
 
+&pin_spim0 {
+	drive-strength = <2>;
+};
+
+&pin_spim0_clk {
+	drive-strength = <2>;
+};
+
 &pwm {
 	status = "okay";
 
@@ -137,6 +145,21 @@
 	disable-wp;
 };
 
+&spfi0 {
+	status = "okay";
+
+	pinctrl-0 = <&spim0_pins>,
+		    <&spim0_cs0_alt_pin>,
+		    <&spim0_cs2_alt_pin>,
+		    <&spim0_cs3_alt_pin>,
+		    <&spim0_cs4_alt_pin>;
+	pinctrl-names = "default";
+	cs-gpios = <&gpio0 2 GPIO_ACTIVE_HIGH>, <0>,
+		   <&gpio1 12 GPIO_ACTIVE_HIGH>,
+		   <&gpio1 13 GPIO_ACTIVE_HIGH>,
+		   <&gpio1 14 GPIO_ACTIVE_HIGH>;
+};
+
 &spfi1 {
 	status = "okay";
 
-- 
2.16.4

[PATCH 06/15] MIPS: dts: img: pistachio_marduk: Add 6Lowpan node

[Andreas Färber]

The CA8210's clock output is needed for the SPI-UART bridge.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 arch/mips/boot/dts/img/pistachio_marduk.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts b/arch/mips/boot/dts/img/pistachio_marduk.dts
index d723b68084c9..b0b6b534a41f 100644
--- a/arch/mips/boot/dts/img/pistachio_marduk.dts
+++ b/arch/mips/boot/dts/img/pistachio_marduk.dts
@@ -158,6 +158,20 @@
 		   <&gpio1 12 GPIO_ACTIVE_HIGH>,
 		   <&gpio1 13 GPIO_ACTIVE_HIGH>,
 		   <&gpio1 14 GPIO_ACTIVE_HIGH>;
+
+	ca8210: sixlowpan@4 {
+		compatible = "cascoda,ca8210";
+		reg = <4>;
+		spi-max-frequency = <3000000>;
+		spi-cpol;
+		reset-gpio = <&gpio0 12 GPIO_ACTIVE_HIGH>;
+		irq-gpio = <&gpio2 12 GPIO_ACTIVE_HIGH>;
+
+		extclock-enable;
+		extclock-freq = <16000000>;
+		extclock-gpio = <2>; /* spiuart_clk */
+		#clock-cells = <0>;
+	};
 };
 
 &spfi1 {
-- 
2.16.4

Re: [PATCH 06/15] MIPS: dts: img: pistachio_marduk: Add 6Lowpan node

[Paul Burton]

Hi Andreas,

On Sun, Jul 22, 2018 at 11:20:01PM +0200, Andreas Färber wrote:
> diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts b/arch/mips/boot/dts/img/pistachio_marduk.dts
> index d723b68084c9..b0b6b534a41f 100644
> --- a/arch/mips/boot/dts/img/pistachio_marduk.dts
> +++ b/arch/mips/boot/dts/img/pistachio_marduk.dts
> @@ -158,6 +158,20 @@
>  		   <&gpio1 12 GPIO_ACTIVE_HIGH>,
>  		   <&gpio1 13 GPIO_ACTIVE_HIGH>,
>  		   <&gpio1 14 GPIO_ACTIVE_HIGH>;
> +
> +	ca8210: sixlowpan@4 {
> +		compatible = "cascoda,ca8210";
> +		reg = <4>;
> +		spi-max-frequency = <3000000>;
> +		spi-cpol;
> +		reset-gpio = <&gpio0 12 GPIO_ACTIVE_HIGH>;
> +		irq-gpio = <&gpio2 12 GPIO_ACTIVE_HIGH>;
> +
> +		extclock-enable;
> +		extclock-freq = <16000000>;
> +		extclock-gpio = <2>; /* spiuart_clk */
> +		#clock-cells = <0>;
> +	};

dtc complains about the extclock-gpio property because it expects a
property named *-gpio to contain a gpio-list:

    DTC     arch/mips/boot/dts/img/pistachio_marduk.dtb
  arch/mips/boot/dts/img/pistachio_marduk.dtb: Warning (gpios_property):
    /spi@18100f00/sixlowpan@4: Missing property '#gpio-cells' in node
    /clk@18144000 or bad phandle (referred from extclock-gpio[0])

Rob, perhaps this should be added as a second false-positive case in
dtc's prop_is_gpio()?

Thanks,
    Paul

[PATCH 07/15] MIPS: dts: img: pistachio_marduk: Add SPI UART node

[Andreas Färber]

The mikroBUS and Raspberry Pi B+ connector UARTs are behind an SC16IS752
SPI-UART bridge. Add it in order to be able to use these connectors.

Note: For UART flow control two pairs of jumpers need to be configured.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 arch/mips/boot/dts/img/pistachio_marduk.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts b/arch/mips/boot/dts/img/pistachio_marduk.dts
index b0b6b534a41f..f682d0a5a3d9 100644
--- a/arch/mips/boot/dts/img/pistachio_marduk.dts
+++ b/arch/mips/boot/dts/img/pistachio_marduk.dts
@@ -159,6 +159,18 @@
 		   <&gpio1 13 GPIO_ACTIVE_HIGH>,
 		   <&gpio1 14 GPIO_ACTIVE_HIGH>;
 
+	sc16is752: uart@0 {
+		compatible = "nxp,sc16is752";
+		reg = <0>;
+		spi-max-frequency = <4000000>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
+		clocks = <&ca8210>;
+
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
+
 	ca8210: sixlowpan@4 {
 		compatible = "cascoda,ca8210";
 		reg = <4>;
-- 
2.16.4

[PATCH 08/15] MIPS: dts: img: pistachio_marduk: Add user LEDs

[Andreas Färber]

Add the LEDs that are connected to the SPI UART bridge.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 arch/mips/boot/dts/img/pistachio_marduk.dts | 46 +++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts b/arch/mips/boot/dts/img/pistachio_marduk.dts
index f682d0a5a3d9..6984933b3cdc 100644
--- a/arch/mips/boot/dts/img/pistachio_marduk.dts
+++ b/arch/mips/boot/dts/img/pistachio_marduk.dts
@@ -60,6 +60,52 @@
 		};
 	};
 
+	gpio-leds {
+		compatible = "gpio-leds";
+
+		led0 {
+			label = "marduk:red:user0";
+			gpios = <&sc16is752 0 GPIO_ACTIVE_LOW>;
+			default-state = "off";
+		};
+
+		led1 {
+			label = "marduk:red:user1";
+			gpios = <&sc16is752 1 GPIO_ACTIVE_LOW>;
+			default-state = "off";
+		};
+
+		led2 {
+			label = "marduk:red:user2";
+			gpios = <&sc16is752 2 GPIO_ACTIVE_LOW>;
+			default-state = "off";
+		};
+
+		led3 {
+			label = "marduk:red:user3";
+			gpios = <&sc16is752 3 GPIO_ACTIVE_LOW>;
+			default-state = "off";
+		};
+
+		led4 {
+			label = "marduk:red:user4";
+			gpios = <&sc16is752 4 GPIO_ACTIVE_LOW>;
+			default-state = "off";
+		};
+
+		led5 {
+			label = "marduk:red:user5";
+			gpios = <&sc16is752 5 GPIO_ACTIVE_LOW>;
+			default-state = "off";
+		};
+
+		led6 {
+			label = "marduk:red:user6";
+			gpios = <&sc16is752 6 GPIO_ACTIVE_LOW>;
+			default-state = "off";
+		};
+	};
+
 	keys {
 		compatible = "gpio-keys";
 
-- 
2.16.4

[PATCH 09/15] dmaengine: img-mdc: Handle early status read

[Andreas Färber]

From: Damien Horsley <damien.horsley@imgtec.com>

It is possible that mdc_tx_status may be called before the first
node has been read from memory.

In this case, the residue value stored in the register is undefined.
Return the transfer size instead.

Signed-off-by: Damien Horsley <damien.horsley@imgtec.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 drivers/dma/img-mdc-dma.c | 40 ++++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/img-mdc-dma.c b/drivers/dma/img-mdc-dma.c
index 25cec9c243e1..0f2f0f52d83a 100644
--- a/drivers/dma/img-mdc-dma.c
+++ b/drivers/dma/img-mdc-dma.c
@@ -621,25 +621,33 @@ static enum dma_status mdc_tx_status(struct dma_chan *chan,
 			(MDC_CMDS_PROCESSED_CMDS_DONE_MASK + 1);
 
 		/*
-		 * If the command loaded event hasn't been processed yet, then
-		 * the difference above includes an extra command.
+		 * If the first node has not yet been read from memory,
+		 * the residue register value is undefined
 		 */
-		if (!mdesc->cmd_loaded)
-			cmds--;
-		else
-			cmds += mdesc->list_cmds_done;
-
-		bytes = mdesc->list_xfer_size;
-		ldesc = mdesc->list;
-		for (i = 0; i < cmds; i++) {
-			bytes -= ldesc->xfer_size + 1;
-			ldesc = ldesc->next_desc;
-		}
-		if (ldesc) {
-			if (residue != MDC_TRANSFER_SIZE_MASK)
-				bytes -= ldesc->xfer_size - residue;
+		if (!mdesc->cmd_loaded && !cmds) {
+			bytes = mdesc->list_xfer_size;
+		} else {
+			/*
+			 * If the command loaded event hasn't been processed yet, then
+			 * the difference above includes an extra command.
+			 */
+			if (!mdesc->cmd_loaded)
+				cmds--;
 			else
+				cmds += mdesc->list_cmds_done;
+
+			bytes = mdesc->list_xfer_size;
+			ldesc = mdesc->list;
+			for (i = 0; i < cmds; i++) {
 				bytes -= ldesc->xfer_size + 1;
+				ldesc = ldesc->next_desc;
+			}
+			if (ldesc) {
+				if (residue != MDC_TRANSFER_SIZE_MASK)
+					bytes -= ldesc->xfer_size - residue;
+				else
+					bytes -= ldesc->xfer_size + 1;
+			}
 		}
 	}
 	spin_unlock_irqrestore(&mchan->vc.lock, flags);
-- 
2.16.4

Re: [PATCH 09/15] dmaengine: img-mdc: Handle early status read

[Vinod Koul]

On 22-07-18, 23:20, Andreas Färber wrote:
> From: Damien Horsley <damien.horsley@imgtec.com>
> 
> It is possible that mdc_tx_status may be called before the first
> node has been read from memory.
> 
> In this case, the residue value stored in the register is undefined.
> Return the transfer size instead.
> 
> Signed-off-by: Damien Horsley <damien.horsley@imgtec.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  drivers/dma/img-mdc-dma.c | 40 ++++++++++++++++++++++++----------------
>  1 file changed, 24 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/dma/img-mdc-dma.c b/drivers/dma/img-mdc-dma.c
> index 25cec9c243e1..0f2f0f52d83a 100644
> --- a/drivers/dma/img-mdc-dma.c
> +++ b/drivers/dma/img-mdc-dma.c
> @@ -621,25 +621,33 @@ static enum dma_status mdc_tx_status(struct dma_chan *chan,
>  			(MDC_CMDS_PROCESSED_CMDS_DONE_MASK + 1);
>  
>  		/*
> -		 * If the command loaded event hasn't been processed yet, then
> -		 * the difference above includes an extra command.
> +		 * If the first node has not yet been read from memory,
> +		 * the residue register value is undefined

so is this the case when transfer is not started ?

>  		 */
> -		if (!mdesc->cmd_loaded)
> -			cmds--;
> -		else
> -			cmds += mdesc->list_cmds_done;
> -
> -		bytes = mdesc->list_xfer_size;
> -		ldesc = mdesc->list;
> -		for (i = 0; i < cmds; i++) {
> -			bytes -= ldesc->xfer_size + 1;
> -			ldesc = ldesc->next_desc;
> -		}
> -		if (ldesc) {
> -			if (residue != MDC_TRANSFER_SIZE_MASK)
> -				bytes -= ldesc->xfer_size - residue;
> +		if (!mdesc->cmd_loaded && !cmds) {
> +			bytes = mdesc->list_xfer_size;
> +		} else {
> +			/*
> +			 * If the command loaded event hasn't been processed yet, then
> +			 * the difference above includes an extra command.
> +			 */
> +			if (!mdesc->cmd_loaded)
> +				cmds--;
>  			else
> +				cmds += mdesc->list_cmds_done;
> +
> +			bytes = mdesc->list_xfer_size;
> +			ldesc = mdesc->list;
> +			for (i = 0; i < cmds; i++) {
>  				bytes -= ldesc->xfer_size + 1;
> +				ldesc = ldesc->next_desc;
> +			}
> +			if (ldesc) {
> +				if (residue != MDC_TRANSFER_SIZE_MASK)
> +					bytes -= ldesc->xfer_size - residue;
> +				else
> +					bytes -= ldesc->xfer_size + 1;
> +			}
>  		}
>  	}
>  	spin_unlock_irqrestore(&mchan->vc.lock, flags);
> -- 
> 2.16.4

-- 
~Vinod

[PATCH 10/15] spi: img-spfi: Implement dual and quad mode

[Andreas Färber]

From: Ionela Voinescu <ionela.voinescu@imgtec.com>

For dual and quad modes to work, the SPFI controller needs
to have information about command/address/dummy bytes in the
transaction register. This information is not relevant for
single mode, and therefore it can have any value in the
allowed range. Therefore, for any read or write transfers of less
than 8 bytes (cmd = 1 byte, addr up to 7 bytes), SPFI will be
configured, but not enabled (unless it is the last transfer in
the queue). The transfer will be enabled by the subsequent transfer.
A pending transfer is determined by the content of the transaction
register: if command part is set and tsize is not.

This way we ensure that for dual and quad transactions
the command request size will appear in the command/address part
of the transaction register, while the data size will be in
tsize, all data being sent/received in the same transaction (as
set up in the transaction register).

Signed-off-by: Ionela Voinescu <ionela.voinescu@imgtec.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 drivers/spi/spi-img-spfi.c | 96 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 85 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c
index 7a37090dabbe..c845a505bae6 100644
--- a/drivers/spi/spi-img-spfi.c
+++ b/drivers/spi/spi-img-spfi.c
@@ -40,7 +40,8 @@
 #define SPFI_CONTROL_SOFT_RESET			BIT(11)
 #define SPFI_CONTROL_SEND_DMA			BIT(10)
 #define SPFI_CONTROL_GET_DMA			BIT(9)
-#define SPFI_CONTROL_SE			BIT(8)
+#define SPFI_CONTROL_SE				BIT(8)
+#define SPFI_CONTROL_TX_RX			BIT(1)
 #define SPFI_CONTROL_TMODE_SHIFT		5
 #define SPFI_CONTROL_TMODE_MASK			0x7
 #define SPFI_CONTROL_TMODE_SINGLE		0
@@ -51,6 +52,10 @@
 #define SPFI_TRANSACTION			0x18
 #define SPFI_TRANSACTION_TSIZE_SHIFT		16
 #define SPFI_TRANSACTION_TSIZE_MASK		0xffff
+#define SPFI_TRANSACTION_CMD_SHIFT		13
+#define SPFI_TRANSACTION_CMD_MASK		0x7
+#define SPFI_TRANSACTION_ADDR_SHIFT		10
+#define SPFI_TRANSACTION_ADDR_MASK		0x7
 
 #define SPFI_PORT_STATE				0x1c
 #define SPFI_PORT_STATE_DEV_SEL_SHIFT		20
@@ -87,6 +92,7 @@
  */
 #define SPFI_32BIT_FIFO_SIZE			64
 #define SPFI_8BIT_FIFO_SIZE			16
+#define SPFI_DATA_REQUEST_MAX_SIZE		8
 
 struct img_spfi {
 	struct device *dev;
@@ -103,6 +109,8 @@ struct img_spfi {
 	struct dma_chan *tx_ch;
 	bool tx_dma_busy;
 	bool rx_dma_busy;
+
+	bool complete;
 };
 
 struct img_spfi_device_data {
@@ -123,9 +131,11 @@ static inline void spfi_start(struct img_spfi *spfi)
 {
 	u32 val;
 
-	val = spfi_readl(spfi, SPFI_CONTROL);
-	val |= SPFI_CONTROL_SPFI_EN;
-	spfi_writel(spfi, val, SPFI_CONTROL);
+	if (spfi->complete) {
+		val = spfi_readl(spfi, SPFI_CONTROL);
+		val |= SPFI_CONTROL_SPFI_EN;
+		spfi_writel(spfi, val, SPFI_CONTROL);
+	}
 }
 
 static inline void spfi_reset(struct img_spfi *spfi)
@@ -138,12 +148,21 @@ static int spfi_wait_all_done(struct img_spfi *spfi)
 {
 	unsigned long timeout = jiffies + msecs_to_jiffies(50);
 
+	if (!(spfi->complete))
+		return 0;
+
 	while (time_before(jiffies, timeout)) {
 		u32 status = spfi_readl(spfi, SPFI_INTERRUPT_STATUS);
 
 		if (status & SPFI_INTERRUPT_ALLDONETRIG) {
 			spfi_writel(spfi, SPFI_INTERRUPT_ALLDONETRIG,
 				    SPFI_INTERRUPT_CLEAR);
+			/*
+			 * Disable SPFI for it not to interfere with
+			 * pending transactions
+			 */
+			spfi_writel(spfi, spfi_readl(spfi, SPFI_CONTROL)
+			& ~SPFI_CONTROL_SPFI_EN, SPFI_CONTROL);
 			return 0;
 		}
 		cpu_relax();
@@ -494,9 +513,32 @@ static void img_spfi_config(struct spi_master *master, struct spi_device *spi,
 			    struct spi_transfer *xfer)
 {
 	struct img_spfi *spfi = spi_master_get_devdata(spi->master);
-	u32 val, div;
+	u32 val, div, transact;
+	bool is_pending;
 
 	/*
+	 * For read or write transfers of less than 8 bytes (cmd = 1 byte,
+	 * addr up to 7 bytes), SPFI will be configured, but not enabled
+	 * (unless it is the last transfer in the queue).The transfer will
+	 * be enabled by the subsequent transfer.
+	 * A pending transfer is determined by the content of the
+	 * transaction register: if command part is set and tsize
+	 * is not
+	 */
+	transact = spfi_readl(spfi, SPFI_TRANSACTION);
+	is_pending = ((transact >> SPFI_TRANSACTION_CMD_SHIFT) &
+			SPFI_TRANSACTION_CMD_MASK) &&
+			(!((transact >> SPFI_TRANSACTION_TSIZE_SHIFT) &
+			SPFI_TRANSACTION_TSIZE_MASK));
+
+	/* If there are no pending transactions it's OK to soft reset */
+	if (!is_pending) {
+		/* Start the transaction from a known (reset) state */
+		spfi_reset(spfi);
+	}
+
+	/*
+	 * Before anything else, set up parameters.
 	 * output = spfi_clk * (BITCLK / 512), where BITCLK must be a
 	 * power of 2 up to 128
 	 */
@@ -509,20 +551,52 @@ static void img_spfi_config(struct spi_master *master, struct spi_device *spi,
 	val |= div << SPFI_DEVICE_PARAMETER_BITCLK_SHIFT;
 	spfi_writel(spfi, val, SPFI_DEVICE_PARAMETER(spi->chip_select));
 
-	spfi_writel(spfi, xfer->len << SPFI_TRANSACTION_TSIZE_SHIFT,
-		    SPFI_TRANSACTION);
+	if (!list_is_last(&xfer->transfer_list, &master->cur_msg->transfers) &&
+		/*
+		 * For duplex mode (both the tx and rx buffers are !NULL) the
+		 * CMD, ADDR, and DUMMY byte parts of the transaction register
+		 * should always be 0 and therefore the pending transfer
+		 * technique cannot be used.
+		 */
+		(xfer->tx_buf) && (!xfer->rx_buf) &&
+		(xfer->len <= SPFI_DATA_REQUEST_MAX_SIZE) && !is_pending) {
+		transact = (1 & SPFI_TRANSACTION_CMD_MASK) <<
+			SPFI_TRANSACTION_CMD_SHIFT;
+		transact |= ((xfer->len - 1) & SPFI_TRANSACTION_ADDR_MASK) <<
+			SPFI_TRANSACTION_ADDR_SHIFT;
+		spfi->complete = false;
+	} else {
+		spfi->complete = true;
+		if (is_pending) {
+			/* Keep setup from pending transfer */
+			transact |= ((xfer->len & SPFI_TRANSACTION_TSIZE_MASK) <<
+				SPFI_TRANSACTION_TSIZE_SHIFT);
+		} else {
+			transact = ((xfer->len & SPFI_TRANSACTION_TSIZE_MASK) <<
+				SPFI_TRANSACTION_TSIZE_SHIFT);
+		}
+	}
+	spfi_writel(spfi, transact, SPFI_TRANSACTION);
 
 	val = spfi_readl(spfi, SPFI_CONTROL);
 	val &= ~(SPFI_CONTROL_SEND_DMA | SPFI_CONTROL_GET_DMA);
-	if (xfer->tx_buf)
+	/*
+	 * We set up send DMA for pending transfers also, as
+	 * those are always send transfers
+	 */
+	if ((xfer->tx_buf) || is_pending)
 		val |= SPFI_CONTROL_SEND_DMA;
-	if (xfer->rx_buf)
+	if (xfer->tx_buf)
+		val |= SPFI_CONTROL_TX_RX;
+	if (xfer->rx_buf) {
 		val |= SPFI_CONTROL_GET_DMA;
+		val &= ~SPFI_CONTROL_TX_RX;
+	}
 	val &= ~(SPFI_CONTROL_TMODE_MASK << SPFI_CONTROL_TMODE_SHIFT);
-	if (xfer->tx_nbits == SPI_NBITS_DUAL &&
+	if (xfer->tx_nbits == SPI_NBITS_DUAL ||
 	    xfer->rx_nbits == SPI_NBITS_DUAL)
 		val |= SPFI_CONTROL_TMODE_DUAL << SPFI_CONTROL_TMODE_SHIFT;
-	else if (xfer->tx_nbits == SPI_NBITS_QUAD &&
+	else if (xfer->tx_nbits == SPI_NBITS_QUAD ||
 		 xfer->rx_nbits == SPI_NBITS_QUAD)
 		val |= SPFI_CONTROL_TMODE_QUAD << SPFI_CONTROL_TMODE_SHIFT;
 	val |= SPFI_CONTROL_SE;
-- 
2.16.4

Re: [PATCH 10/15] spi: img-spfi: Implement dual and quad mode

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1308 bytes --]

On Sun, Jul 22, 2018 at 11:20:05PM +0200, Andreas Färber wrote:

>  #define SPFI_CONTROL_GET_DMA			BIT(9)
> -#define SPFI_CONTROL_SE			BIT(8)
> +#define SPFI_CONTROL_SE				BIT(8)
> +#define SPFI_CONTROL_TX_RX			BIT(1)

Random reindent of _SE there?

> +			/*
> +			 * Disable SPFI for it not to interfere with
> +			 * pending transactions
> +			 */
> +			spfi_writel(spfi, spfi_readl(spfi, SPFI_CONTROL)
> +			& ~SPFI_CONTROL_SPFI_EN, SPFI_CONTROL);
>  			return 0;

The indentation on the second line of the write is very confusing, it
should be indented relative to the first line.

> +	if (!list_is_last(&xfer->transfer_list, &master->cur_msg->transfers) &&
> +		/*
> +		 * For duplex mode (both the tx and rx buffers are !NULL) the
> +		 * CMD, ADDR, and DUMMY byte parts of the transaction register
> +		 * should always be 0 and therefore the pending transfer
> +		 * technique cannot be used.
> +		 */
> +		(xfer->tx_buf) && (!xfer->rx_buf) &&
> +		(xfer->len <= SPFI_DATA_REQUEST_MAX_SIZE) && !is_pending) {
> +		transact = (1 & SPFI_TRANSACTION_CMD_MASK) <<

This is again *really* hard to read - having the comment in the middle
of the condidional for the if statement, then indenting the code within
the if statement to the same depth is just super confusing.  

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 11/15] spi: img-spfi: Set device select bits for SPFI port state

[Andreas Färber]

From: Ionela Voinescu <ionela.voinescu@imgtec.com>

Even if the chip select line is not controlled by the SPFI
hardware, the device select bits need to be set to specify
the chip select line in use for the hardware to know what
parameters to use for the current transfer.

Signed-off-by: Ionela Voinescu <ionela.voinescu@imgtec.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 drivers/spi/spi-img-spfi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c
index c845a505bae6..0d73d31a6a2b 100644
--- a/drivers/spi/spi-img-spfi.c
+++ b/drivers/spi/spi-img-spfi.c
@@ -438,6 +438,9 @@ static int img_spfi_prepare(struct spi_master *master, struct spi_message *msg)
 	u32 val;
 
 	val = spfi_readl(spfi, SPFI_PORT_STATE);
+	val &= ~(SPFI_PORT_STATE_DEV_SEL_MASK <<
+		 SPFI_PORT_STATE_DEV_SEL_SHIFT);
+	val |= msg->spi->chip_select << SPFI_PORT_STATE_DEV_SEL_SHIFT;
 	if (msg->spi->mode & SPI_CPHA)
 		val |= SPFI_PORT_STATE_CK_PHASE(msg->spi->chip_select);
 	else
-- 
2.16.4

[PATCH 12/15] spi: img-spfi: Use device 0 configuration for all devices

[Andreas Färber]

From: Ionela Voinescu <ionela.voinescu@imgtec.com>

Given that we control the chip select line externally,
we can use only one parameter register (device 0 parameter
register) and one set of configuration bits (port configuration
bits for device 0) for all devices (all chip select lines).

Signed-off-by: Ionela Voinescu <ionela.voinescu@imgtec.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 drivers/spi/spi-img-spfi.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c
index 0d73d31a6a2b..231b59c1ab60 100644
--- a/drivers/spi/spi-img-spfi.c
+++ b/drivers/spi/spi-img-spfi.c
@@ -437,18 +437,23 @@ static int img_spfi_prepare(struct spi_master *master, struct spi_message *msg)
 	struct img_spfi *spfi = spi_master_get_devdata(master);
 	u32 val;
 
+	/*
+	 * The chip select line is controlled externally so
+	 * we can use the CS0 configuration for all devices
+	 */
 	val = spfi_readl(spfi, SPFI_PORT_STATE);
+
+	/* 0 for device selection */
 	val &= ~(SPFI_PORT_STATE_DEV_SEL_MASK <<
 		 SPFI_PORT_STATE_DEV_SEL_SHIFT);
-	val |= msg->spi->chip_select << SPFI_PORT_STATE_DEV_SEL_SHIFT;
 	if (msg->spi->mode & SPI_CPHA)
-		val |= SPFI_PORT_STATE_CK_PHASE(msg->spi->chip_select);
+		val |= SPFI_PORT_STATE_CK_PHASE(0);
 	else
-		val &= ~SPFI_PORT_STATE_CK_PHASE(msg->spi->chip_select);
+		val &= ~SPFI_PORT_STATE_CK_PHASE(0);
 	if (msg->spi->mode & SPI_CPOL)
-		val |= SPFI_PORT_STATE_CK_POL(msg->spi->chip_select);
+		val |= SPFI_PORT_STATE_CK_POL(0);
 	else
-		val &= ~SPFI_PORT_STATE_CK_POL(msg->spi->chip_select);
+		val &= ~SPFI_PORT_STATE_CK_POL(0);
 	spfi_writel(spfi, val, SPFI_PORT_STATE);
 
 	return 0;
@@ -548,11 +553,15 @@ static void img_spfi_config(struct spi_master *master, struct spi_device *spi,
 	div = DIV_ROUND_UP(clk_get_rate(spfi->spfi_clk), xfer->speed_hz);
 	div = clamp(512 / (1 << get_count_order(div)), 1, 128);
 
-	val = spfi_readl(spfi, SPFI_DEVICE_PARAMETER(spi->chip_select));
+	/*
+	 * The chip select line is controlled externally so
+	 * we can use the CS0 parameters for all devices
+	 */
+	val = spfi_readl(spfi, SPFI_DEVICE_PARAMETER(0));
 	val &= ~(SPFI_DEVICE_PARAMETER_BITCLK_MASK <<
 		 SPFI_DEVICE_PARAMETER_BITCLK_SHIFT);
 	val |= div << SPFI_DEVICE_PARAMETER_BITCLK_SHIFT;
-	spfi_writel(spfi, val, SPFI_DEVICE_PARAMETER(spi->chip_select));
+	spfi_writel(spfi, val, SPFI_DEVICE_PARAMETER(0));
 
 	if (!list_is_last(&xfer->transfer_list, &master->cur_msg->transfers) &&
 		/*
-- 
2.16.4

[PATCH 13/15] spi: img-spfi: RX maximum burst size for DMA is 8

[Andreas Färber]

From: Ionela Voinescu <ionela.voinescu@imgtec.com>

The depth of the FIFOs is 16 bytes. The DMA request line is tied
to the half full/empty (depending on the use of the TX or RX FIFO)
threshold. For the TX FIFO, if you set a burst size of 8 (equal to
half the depth) the first burst goes into FIFO without any issues,
but due the latency involved (the time the data leaves  the DMA
engine to the time it arrives at the FIFO), the DMA might trigger
another burst of 8. But given that there is no space for 2 additonal
bursts of 8, this would result in a failure. Therefore, we have to
keep the burst size for TX to 4 to accomodate for an extra burst.

For the read (RX) scenario, the DMA request line goes high when
there is at least 8 entries in the FIFO (half full), and we can
program the burst size to be 8 because the risk of accidental burst
does not exist. The DMA engine will not trigger another read until
the read data for all the burst it has sent out has been received.

While here, move the burst size setting outside of the if/else branches
as they have the same value for both 8 and 32 bit data widths.

Signed-off-by: Ionela Voinescu <ionela.voinescu@imgtec.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 drivers/spi/spi-img-spfi.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c
index 231b59c1ab60..8ad6c75d0af5 100644
--- a/drivers/spi/spi-img-spfi.c
+++ b/drivers/spi/spi-img-spfi.c
@@ -346,12 +346,11 @@ static int img_spfi_start_dma(struct spi_master *master,
 		if (xfer->len % 4 == 0) {
 			rxconf.src_addr = spfi->phys + SPFI_RX_32BIT_VALID_DATA;
 			rxconf.src_addr_width = 4;
-			rxconf.src_maxburst = 4;
 		} else {
 			rxconf.src_addr = spfi->phys + SPFI_RX_8BIT_VALID_DATA;
 			rxconf.src_addr_width = 1;
-			rxconf.src_maxburst = 4;
 		}
+		rxconf.src_maxburst = 8;
 		dmaengine_slave_config(spfi->rx_ch, &rxconf);
 
 		rxdesc = dmaengine_prep_slave_sg(spfi->rx_ch, xfer->rx_sg.sgl,
@@ -370,12 +369,11 @@ static int img_spfi_start_dma(struct spi_master *master,
 		if (xfer->len % 4 == 0) {
 			txconf.dst_addr = spfi->phys + SPFI_TX_32BIT_VALID_DATA;
 			txconf.dst_addr_width = 4;
-			txconf.dst_maxburst = 4;
 		} else {
 			txconf.dst_addr = spfi->phys + SPFI_TX_8BIT_VALID_DATA;
 			txconf.dst_addr_width = 1;
-			txconf.dst_maxburst = 4;
 		}
+		txconf.dst_maxburst = 4;
 		dmaengine_slave_config(spfi->tx_ch, &txconf);
 
 		txdesc = dmaengine_prep_slave_sg(spfi->tx_ch, xfer->tx_sg.sgl,
-- 
2.16.4

Re: [PATCH 13/15] spi: img-spfi: RX maximum burst size for DMA is 8

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]

On Sun, Jul 22, 2018 at 11:20:08PM +0200, Andreas Färber wrote:
> From: Ionela Voinescu <ionela.voinescu@imgtec.com>
> 
> The depth of the FIFOs is 16 bytes. The DMA request line is tied
> to the half full/empty (depending on the use of the TX or RX FIFO)
> threshold. For the TX FIFO, if you set a burst size of 8 (equal to
> half the depth) the first burst goes into FIFO without any issues,
> but due the latency involved (the time the data leaves  the DMA
> engine to the time it arrives at the FIFO), the DMA might trigger
> another burst of 8. But given that there is no space for 2 additonal
> bursts of 8, this would result in a failure. Therefore, we have to
> keep the burst size for TX to 4 to accomodate for an extra burst.

This seems like something that should be sent as a bug fix - doesn't it
fix anything?  If it is a fix then it should've gone at the start of the
series so it's got no dependencies and could be sent to Linus for the
current release.

> While here, move the burst size setting outside of the if/else branches
> as they have the same value for both 8 and 32 bit data widths.

Though this refactoring probably less so.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 14/15] spi: img-spfi: Finish every transfer cleanly

[Andreas Färber]

From: Ionela Voinescu <ionela.voinescu@imgtec.com>

Before this change, the interrupt status bit that signaled
the end of a transfer was cleared in the wait_all_done
function. That functionality triggered issues for DMA
duplex transactions where the wait function was called
twice, in both the TX and RX callbacks.

In order to fix the issue, clear all interrupt data bits
at the end of a PIO transfer or at the end of both TX and RX
duplex transfers, if the transfer is not a pending transfer
(command waiting for data). After that, the status register
is checked for new incoming data or new data requests to be
signaled. If SPFI finished cleanly, no new interrupt data
bits should be set.

Signed-off-by: Ionela Voinescu <ionela.voinescu@imgtec.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 drivers/spi/spi-img-spfi.c | 49 +++++++++++++++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c
index 8ad6c75d0af5..a1244234daa5 100644
--- a/drivers/spi/spi-img-spfi.c
+++ b/drivers/spi/spi-img-spfi.c
@@ -83,6 +83,14 @@
 #define SPFI_INTERRUPT_SDE			BIT(1)
 #define SPFI_INTERRUPT_SDTRIG			BIT(0)
 
+#define SPFI_INTERRUPT_DATA_BITS		(SPFI_INTERRUPT_SDHF |\
+						SPFI_INTERRUPT_SDFUL |\
+						SPFI_INTERRUPT_GDEX32BIT |\
+						SPFI_INTERRUPT_GDHF |\
+						SPFI_INTERRUPT_GDFUL |\
+						SPFI_INTERRUPT_ALLDONETRIG |\
+						SPFI_INTERRUPT_GDEX8BIT)
+
 /*
  * There are four parallel FIFOs of 16 bytes each.  The word buffer
  * (*_32BIT_VALID_DATA) accesses all four FIFOs at once, resulting in an
@@ -144,6 +152,23 @@ static inline void spfi_reset(struct img_spfi *spfi)
 	spfi_writel(spfi, 0, SPFI_CONTROL);
 }
 
+static inline void spfi_finish(struct img_spfi *spfi)
+{
+	if (!(spfi->complete))
+		return;
+
+	/* Clear data bits as all transfers(TX and RX) have finished */
+	spfi_writel(spfi, SPFI_INTERRUPT_DATA_BITS, SPFI_INTERRUPT_CLEAR);
+	if (spfi_readl(spfi, SPFI_INTERRUPT_STATUS) & SPFI_INTERRUPT_DATA_BITS) {
+		dev_err(spfi->dev, "SPFI did not finish transfer cleanly.\n");
+		spfi_reset(spfi);
+	}
+	/* Disable SPFI for it not to interfere with pending transactions */
+	spfi_writel(spfi,
+		    spfi_readl(spfi, SPFI_CONTROL) & ~SPFI_CONTROL_SPFI_EN,
+		    SPFI_CONTROL);
+}
+
 static int spfi_wait_all_done(struct img_spfi *spfi)
 {
 	unsigned long timeout = jiffies + msecs_to_jiffies(50);
@@ -152,19 +177,9 @@ static int spfi_wait_all_done(struct img_spfi *spfi)
 		return 0;
 
 	while (time_before(jiffies, timeout)) {
-		u32 status = spfi_readl(spfi, SPFI_INTERRUPT_STATUS);
-
-		if (status & SPFI_INTERRUPT_ALLDONETRIG) {
-			spfi_writel(spfi, SPFI_INTERRUPT_ALLDONETRIG,
-				    SPFI_INTERRUPT_CLEAR);
-			/*
-			 * Disable SPFI for it not to interfere with
-			 * pending transactions
-			 */
-			spfi_writel(spfi, spfi_readl(spfi, SPFI_CONTROL)
-			& ~SPFI_CONTROL_SPFI_EN, SPFI_CONTROL);
+		if (spfi_readl(spfi, SPFI_INTERRUPT_STATUS) &
+		    SPFI_INTERRUPT_ALLDONETRIG)
 			return 0;
-		}
 		cpu_relax();
 	}
 
@@ -296,6 +311,8 @@ static int img_spfi_start_pio(struct spi_master *master,
 	}
 
 	ret = spfi_wait_all_done(spfi);
+	spfi_finish(spfi);
+
 	if (ret < 0)
 		return ret;
 
@@ -311,8 +328,10 @@ static void img_spfi_dma_rx_cb(void *data)
 
 	spin_lock_irqsave(&spfi->lock, flags);
 	spfi->rx_dma_busy = false;
-	if (!spfi->tx_dma_busy)
+	if (!spfi->tx_dma_busy) {
+		spfi_finish(spfi);
 		spi_finalize_current_transfer(spfi->master);
+	}
 	spin_unlock_irqrestore(&spfi->lock, flags);
 }
 
@@ -325,8 +344,10 @@ static void img_spfi_dma_tx_cb(void *data)
 
 	spin_lock_irqsave(&spfi->lock, flags);
 	spfi->tx_dma_busy = false;
-	if (!spfi->rx_dma_busy)
+	if (!spfi->rx_dma_busy) {
+		spfi_finish(spfi);
 		spi_finalize_current_transfer(spfi->master);
+	}
 	spin_unlock_irqrestore(&spfi->lock, flags);
 }
 
-- 
2.16.4

Re: [PATCH 14/15] spi: img-spfi: Finish every transfer cleanly

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 430 bytes --]

On Sun, Jul 22, 2018 at 11:20:09PM +0200, Andreas Färber wrote:
> From: Ionela Voinescu <ionela.voinescu@imgtec.com>
> 
> Before this change, the interrupt status bit that signaled
> the end of a transfer was cleared in the wait_all_done
> function. That functionality triggered issues for DMA
> duplex transactions where the wait function was called
> twice, in both the TX and RX callbacks.

Again, is this not a fix?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 15/15] clk: pistachio: Fix wrong SDHost card speed

[Andreas Färber]

From: Govindraj Raja <Govindraj.Raja@imgtec.com>

The SDHost currently clocks the card 4x slower than it
should do, because there is a fixed divide by 4 in the
sdhost wrapper that is not present in the clock tree.
To model this, add a fixed divide by 4 clock node in
the SDHost clock path.

This will ensure the right clock frequency is selected when
the mmc driver tries to configure frequency on card insert.

Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 drivers/clk/pistachio/clk-pistachio.c     | 3 ++-
 include/dt-bindings/clock/pistachio-clk.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/pistachio/clk-pistachio.c b/drivers/clk/pistachio/clk-pistachio.c
index c4ceb5eaf46c..1c968d9a6e17 100644
--- a/drivers/clk/pistachio/clk-pistachio.c
+++ b/drivers/clk/pistachio/clk-pistachio.c
@@ -44,7 +44,7 @@ static struct pistachio_gate pistachio_gates[] __initdata = {
 	GATE(CLK_AUX_ADC_INTERNAL, "aux_adc_internal", "sys_internal_div",
 	     0x104, 22),
 	GATE(CLK_AUX_ADC, "aux_adc", "aux_adc_div", 0x104, 23),
-	GATE(CLK_SD_HOST, "sd_host", "sd_host_div", 0x104, 24),
+	GATE(CLK_SD_HOST, "sd_host", "sd_host_div4", 0x104, 24),
 	GATE(CLK_BT, "bt", "bt_div", 0x104, 25),
 	GATE(CLK_BT_DIV4, "bt_div4", "bt_div4_div", 0x104, 26),
 	GATE(CLK_BT_DIV8, "bt_div8", "bt_div8_div", 0x104, 27),
@@ -54,6 +54,7 @@ static struct pistachio_gate pistachio_gates[] __initdata = {
 static struct pistachio_fixed_factor pistachio_ffs[] __initdata = {
 	FIXED_FACTOR(CLK_WIFI_DIV4, "wifi_div4", "wifi_pll", 4),
 	FIXED_FACTOR(CLK_WIFI_DIV8, "wifi_div8", "wifi_pll", 8),
+	FIXED_FACTOR(CLK_SDHOST_DIV4, "sd_host_div4", "sd_host_div", 4),
 };
 
 static struct pistachio_div pistachio_divs[] __initdata = {
diff --git a/include/dt-bindings/clock/pistachio-clk.h b/include/dt-bindings/clock/pistachio-clk.h
index 039f83facb68..77b92aed241d 100644
--- a/include/dt-bindings/clock/pistachio-clk.h
+++ b/include/dt-bindings/clock/pistachio-clk.h
@@ -21,6 +21,7 @@
 /* Fixed-factor clocks */
 #define CLK_WIFI_DIV4			16
 #define CLK_WIFI_DIV8			17
+#define CLK_SDHOST_DIV4			18
 
 /* Gate clocks */
 #define CLK_MIPS			32
-- 
2.16.4

Re: [PATCH 15/15] clk: pistachio: Fix wrong SDHost card speed

[Stephen Boyd]

Quoting Andreas Färber (2018-07-22 14:20:10)
> From: Govindraj Raja <Govindraj.Raja@imgtec.com>
> 
> The SDHost currently clocks the card 4x slower than it
> should do, because there is a fixed divide by 4 in the
> sdhost wrapper that is not present in the clock tree.
> To model this, add a fixed divide by 4 clock node in
> the SDHost clock path.
> 
> This will ensure the right clock frequency is selected when
> the mmc driver tries to configure frequency on card insert.
> 
> Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---

Acked-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH 15/15] clk: pistachio: Fix wrong SDHost card speed

[Rob Herring]

On Sun, Jul 22, 2018 at 11:20:10PM +0200, Andreas Färber wrote:
> From: Govindraj Raja <Govindraj.Raja@imgtec.com>
> 
> The SDHost currently clocks the card 4x slower than it
> should do, because there is a fixed divide by 4 in the
> sdhost wrapper that is not present in the clock tree.
> To model this, add a fixed divide by 4 clock node in
> the SDHost clock path.
> 
> This will ensure the right clock frequency is selected when
> the mmc driver tries to configure frequency on card insert.
> 
> Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  drivers/clk/pistachio/clk-pistachio.c     | 3 ++-
>  include/dt-bindings/clock/pistachio-clk.h | 1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/pistachio/clk-pistachio.c b/drivers/clk/pistachio/clk-pistachio.c
> index c4ceb5eaf46c..1c968d9a6e17 100644
> --- a/drivers/clk/pistachio/clk-pistachio.c
> +++ b/drivers/clk/pistachio/clk-pistachio.c
> @@ -44,7 +44,7 @@ static struct pistachio_gate pistachio_gates[] __initdata = {
>  	GATE(CLK_AUX_ADC_INTERNAL, "aux_adc_internal", "sys_internal_div",
>  	     0x104, 22),
>  	GATE(CLK_AUX_ADC, "aux_adc", "aux_adc_div", 0x104, 23),
> -	GATE(CLK_SD_HOST, "sd_host", "sd_host_div", 0x104, 24),
> +	GATE(CLK_SD_HOST, "sd_host", "sd_host_div4", 0x104, 24),
>  	GATE(CLK_BT, "bt", "bt_div", 0x104, 25),
>  	GATE(CLK_BT_DIV4, "bt_div4", "bt_div4_div", 0x104, 26),
>  	GATE(CLK_BT_DIV8, "bt_div8", "bt_div8_div", 0x104, 27),
> @@ -54,6 +54,7 @@ static struct pistachio_gate pistachio_gates[] __initdata = {
>  static struct pistachio_fixed_factor pistachio_ffs[] __initdata = {
>  	FIXED_FACTOR(CLK_WIFI_DIV4, "wifi_div4", "wifi_pll", 4),
>  	FIXED_FACTOR(CLK_WIFI_DIV8, "wifi_div8", "wifi_pll", 8),
> +	FIXED_FACTOR(CLK_SDHOST_DIV4, "sd_host_div4", "sd_host_div", 4),
>  };
>  
>  static struct pistachio_div pistachio_divs[] __initdata = {
> diff --git a/include/dt-bindings/clock/pistachio-clk.h b/include/dt-bindings/clock/pistachio-clk.h
> index 039f83facb68..77b92aed241d 100644
> --- a/include/dt-bindings/clock/pistachio-clk.h
> +++ b/include/dt-bindings/clock/pistachio-clk.h
> @@ -21,6 +21,7 @@
>  /* Fixed-factor clocks */
>  #define CLK_WIFI_DIV4			16
>  #define CLK_WIFI_DIV8			17
> +#define CLK_SDHOST_DIV4			18

Does this clock really need to be exposed in DT?

>  
>  /* Gate clocks */
>  #define CLK_MIPS			32
> -- 
> 2.16.4
>