[PATCH v2 0/3] Enable l3gd20 on stm32f429-disco board

[PATCH v2 0/3] Enable l3gd20 on stm32f429-disco board

[dillon.minfei]

From: dillon min <dillon.minfei@gmail.com>

This patchset is for enable l3gd20 on stm32f429-disco board
has following changes:

V2:
    1, insert blank line at stm32f420-disco.dts line 143
    2, add more description for l3gd20 in commit message

V1:
    1, enable spi5 controller on stm32f429-disco (dts)
    2, add spi5 pinmap for stm32f429-disco  (dts)
    3, add SPI_SIMPLEX_RX, SPI_3WIRE_RX support for stm32f4

dillon min (3):
  ARM: dts: stm32: Add pin map for spi5 on stm32f429-disco board
  ARM: dts: stm32: enable l3gd20 on stm32429-disco board
  spi: stm32: Add SPI_SIMPLEX_RX, SPI_3WIRE_RX support for stm32f4

 arch/arm/boot/dts/stm32f4-pinctrl.dtsi | 17 +++++++++++++++++
 arch/arm/boot/dts/stm32f429-disco.dts  | 25 +++++++++++++++++++++++++
 drivers/spi/spi-stm32.c                | 29 +++++++++++++++++++++++++----
 3 files changed, 67 insertions(+), 4 deletions(-)

-- 
2.7.4

[PATCH v2 1/3] ARM: dts: stm32: Add pin map for spi5 on stm32f429-disco board

[dillon.minfei]

From: dillon min <dillon.minfei@gmail.com>

This patch adds the pin configuration for ltdc, spi5 controller
on stm32f429-disco board.

Signed-off-by: dillon min <dillon.minfei@gmail.com>
---
 arch/arm/boot/dts/stm32f4-pinctrl.dtsi | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/stm32f4-pinctrl.dtsi b/arch/arm/boot/dts/stm32f4-pinctrl.dtsi
index 392fa14..54c1b27 100644
--- a/arch/arm/boot/dts/stm32f4-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stm32f4-pinctrl.dtsi
@@ -316,6 +316,23 @@
 				};
 			};
 
+			spi5_pins: spi5-0 {
+				pins1 {
+					pinmux = <STM32_PINMUX('F', 7, AF5)>,
+						/* SPI5_CLK */
+						 <STM32_PINMUX('F', 9, AF5)>;
+						/* SPI5_MOSI */
+					bias-disable;
+					drive-push-pull;
+					slew-rate = <0>;
+				};
+				pins2 {
+					pinmux = <STM32_PINMUX('F', 8, AF5)>;
+						/* SPI5_MISO */
+					bias-disable;
+				};
+			};
+
 			dcmi_pins: dcmi-0 {
 				pins {
 					pinmux = <STM32_PINMUX('A', 4, AF13)>, /* DCMI_HSYNC */
-- 
2.7.4

[PATCH v2 2/3] ARM: dts: stm32: enable l3gd20 on stm32429-disco board

[dillon.minfei]

From: dillon min <dillon.minfei@gmail.com>

L3gd20, st mems motion sensor, 3-axis digital output gyroscope,
connect to stm32f429 via spi5

Signed-off-by: dillon min <dillon.minfei@gmail.com>
---

Hi Alexandre,

V2:
    1, insert blank line at stm32f420-disco.dts line 143
    2, add more description about l3gd20 in commit message

V1:
    enable l3gd20 dts binding on stm32f429-disco

thanks.

dillon,

 arch/arm/boot/dts/stm32f429-disco.dts | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm/boot/dts/stm32f429-disco.dts b/arch/arm/boot/dts/stm32f429-disco.dts
index 30c0f67..1bfb903 100644
--- a/arch/arm/boot/dts/stm32f429-disco.dts
+++ b/arch/arm/boot/dts/stm32f429-disco.dts
@@ -49,6 +49,8 @@
 #include "stm32f429.dtsi"
 #include "stm32f429-pinctrl.dtsi"
 #include <dt-bindings/input/input.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/gpio/gpio.h>
 
 / {
 	model = "STMicroelectronics STM32F429i-DISCO board";
@@ -127,3 +129,26 @@
 	pinctrl-names = "default";
 	status = "okay";
 };
+
+&spi5 {
+	status = "okay";
+	pinctrl-0 = <&spi5_pins>;
+	pinctrl-names = "default";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	cs-gpios = <&gpioc 1 GPIO_ACTIVE_LOW>;
+	dmas = <&dma2 3 2 0x400 0x0>,
+	       <&dma2 4 2 0x400 0x0>;
+	dma-names = "rx", "tx";
+
+	l3gd20: l3gd20@0 {
+		compatible = "st,l3gd20-gyro";
+		spi-max-frequency = <10000000>;
+		st,drdy-int-pin = <2>;
+		interrupt-parent = <&gpioa>;
+		interrupts = <1 IRQ_TYPE_EDGE_RISING>,
+				<2 IRQ_TYPE_EDGE_RISING>;
+		reg = <0>;
+		status = "okay";
+	};
+};
-- 
2.7.4

Re: [PATCH v2 2/3] ARM: dts: stm32: enable l3gd20 on stm32429-disco board

[Alexandre Torgue]

Hi Dillon

On 5/12/20 9:36 AM, dillon.minfei@gmail.com wrote:
> From: dillon min <dillon.minfei@gmail.com>
> 
> L3gd20, st mems motion sensor, 3-axis digital output gyroscope,
> connect to stm32f429 via spi5
> 
> Signed-off-by: dillon min <dillon.minfei@gmail.com>
> ---
> 
> Hi Alexandre,
> 
> V2:
>      1, insert blank line at stm32f420-disco.dts line 143
>      2, add more description about l3gd20 in commit message
> 
> V1:
>      enable l3gd20 dts binding on stm32f429-disco
> 
> thanks.
> 
> dillon,
> 
>   arch/arm/boot/dts/stm32f429-disco.dts | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/stm32f429-disco.dts b/arch/arm/boot/dts/stm32f429-disco.dts
> index 30c0f67..1bfb903 100644
> --- a/arch/arm/boot/dts/stm32f429-disco.dts
> +++ b/arch/arm/boot/dts/stm32f429-disco.dts
> @@ -49,6 +49,8 @@
>   #include "stm32f429.dtsi"
>   #include "stm32f429-pinctrl.dtsi"
>   #include <dt-bindings/input/input.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/gpio/gpio.h>
>   
>   / {
>   	model = "STMicroelectronics STM32F429i-DISCO board";
> @@ -127,3 +129,26 @@
>   	pinctrl-names = "default";
>   	status = "okay";
>   };
> +
> +&spi5 {
> +	status = "okay";
> +	pinctrl-0 = <&spi5_pins>;
> +	pinctrl-names = "default";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	cs-gpios = <&gpioc 1 GPIO_ACTIVE_LOW>;
> +	dmas = <&dma2 3 2 0x400 0x0>,
> +	       <&dma2 4 2 0x400 0x0>;
> +	dma-names = "rx", "tx";
> +

You added this spi5 node in this patch but also in the display series. I 
will have issue to merge. Even if I could fix it easily, as you are 
going to resend, the good practice could be to have several patches in 
one series: one patch for spi5 controller, another for gyro and another 
for display.

And also same remark than Linus did in display series move DMA to soc 
dtsi file please.


> +	l3gd20: l3gd20@0 {
> +		compatible = "st,l3gd20-gyro";
> +		spi-max-frequency = <10000000>;
> +		st,drdy-int-pin = <2>;
> +		interrupt-parent = <&gpioa>;
> +		interrupts = <1 IRQ_TYPE_EDGE_RISING>,
> +				<2 IRQ_TYPE_EDGE_RISING>;
> +		reg = <0>;
> +		status = "okay";
> +	};
> +};
> 

Re: [PATCH v2 2/3] ARM: dts: stm32: enable l3gd20 on stm32429-disco board

[dillon min]

Hi Alexandre,

Alexandre Torgue <alexandre.torgue@st.com> 于2020年5月14日周四 下午10:10写道：
>
> Hi Dillon
>
> On 5/12/20 9:36 AM, dillon.minfei@gmail.com wrote:
> > From: dillon min <dillon.minfei@gmail.com>
> >
> > L3gd20, st mems motion sensor, 3-axis digital output gyroscope,
> > connect to stm32f429 via spi5
> >
> > Signed-off-by: dillon min <dillon.minfei@gmail.com>
> > ---
> >
> > Hi Alexandre,
> >
> > V2:
> >      1, insert blank line at stm32f420-disco.dts line 143
> >      2, add more description about l3gd20 in commit message
> >
> > V1:
> >      enable l3gd20 dts binding on stm32f429-disco
> >
> > thanks.
> >
> > dillon,
> >
> >   arch/arm/boot/dts/stm32f429-disco.dts | 25 +++++++++++++++++++++++++
> >   1 file changed, 25 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/stm32f429-disco.dts b/arch/arm/boot/dts/stm32f429-disco.dts
> > index 30c0f67..1bfb903 100644
> > --- a/arch/arm/boot/dts/stm32f429-disco.dts
> > +++ b/arch/arm/boot/dts/stm32f429-disco.dts
> > @@ -49,6 +49,8 @@
> >   #include "stm32f429.dtsi"
> >   #include "stm32f429-pinctrl.dtsi"
> >   #include <dt-bindings/input/input.h>
> > +#include <dt-bindings/interrupt-controller/irq.h>
> > +#include <dt-bindings/gpio/gpio.h>
> >
> >   / {
> >       model = "STMicroelectronics STM32F429i-DISCO board";
> > @@ -127,3 +129,26 @@
> >       pinctrl-names = "default";
> >       status = "okay";
> >   };
> > +
> > +&spi5 {
> > +     status = "okay";
> > +     pinctrl-0 = <&spi5_pins>;
> > +     pinctrl-names = "default";
> > +     #address-cells = <1>;
> > +     #size-cells = <0>;
> > +     cs-gpios = <&gpioc 1 GPIO_ACTIVE_LOW>;
> > +     dmas = <&dma2 3 2 0x400 0x0>,
> > +            <&dma2 4 2 0x400 0x0>;
> > +     dma-names = "rx", "tx";
> > +
>
> You added this spi5 node in this patch but also in the display series. I
> will have issue to merge. Even if I could fix it easily, as you are
> going to resend, the good practice could be to have several patches in
> one series: one patch for spi5 controller, another for gyro and another
> for display.
>
> And also same remark than Linus did in display series move DMA to soc
> dtsi file please.

Sure, how about this patch sequence:
1 add spi5 dma to soc (stm32f429.dtsi)
2 add pin map for spi5 (stm32f4-pinctrl.dtsi)
3 add spi5 controller with gyro (stm32f429-disco.dts)
4 add spi modification to support gyro (spi-stm32.c)

5 add ltdc pin map for stm32f429-disco board (stm32f4-pinctrl.dtsi)
6 add ilitek-ili9341 dts bindings for disco (stm32f429-disco.dts,
depends on above step 3)
7 add yaml document about ilitek-ili9341 (ilitek,ili9341.yaml)
8 add clk changes to support ltdc driver (clk-stm32f4.c)

so, i will combine gyro and display two patches to one patchset next
time. right ?

thanks.
>
>
> > +     l3gd20: l3gd20@0 {
> > +             compatible = "st,l3gd20-gyro";
> > +             spi-max-frequency = <10000000>;
> > +             st,drdy-int-pin = <2>;
> > +             interrupt-parent = <&gpioa>;
> > +             interrupts = <1 IRQ_TYPE_EDGE_RISING>,
> > +                             <2 IRQ_TYPE_EDGE_RISING>;
> > +             reg = <0>;
> > +             status = "okay";
> > +     };
> > +};
> >

Re: [PATCH v2 2/3] ARM: dts: stm32: enable l3gd20 on stm32429-disco board

[Alexandre Torgue]

On 5/14/20 9:07 AM, dillon min wrote:
> Hi Alexandre,
> 
> Alexandre Torgue <alexandre.torgue@st.com> 于2020年5月14日周四 下午10:10写道：
>>
>> Hi Dillon
>>
>> On 5/12/20 9:36 AM, dillon.minfei@gmail.com wrote:
>>> From: dillon min <dillon.minfei@gmail.com>
>>>
>>> L3gd20, st mems motion sensor, 3-axis digital output gyroscope,
>>> connect to stm32f429 via spi5
>>>
>>> Signed-off-by: dillon min <dillon.minfei@gmail.com>
>>> ---
>>>
>>> Hi Alexandre,
>>>
>>> V2:
>>>       1, insert blank line at stm32f420-disco.dts line 143
>>>       2, add more description about l3gd20 in commit message
>>>
>>> V1:
>>>       enable l3gd20 dts binding on stm32f429-disco
>>>
>>> thanks.
>>>
>>> dillon,
>>>
>>>    arch/arm/boot/dts/stm32f429-disco.dts | 25 +++++++++++++++++++++++++
>>>    1 file changed, 25 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/stm32f429-disco.dts b/arch/arm/boot/dts/stm32f429-disco.dts
>>> index 30c0f67..1bfb903 100644
>>> --- a/arch/arm/boot/dts/stm32f429-disco.dts
>>> +++ b/arch/arm/boot/dts/stm32f429-disco.dts
>>> @@ -49,6 +49,8 @@
>>>    #include "stm32f429.dtsi"
>>>    #include "stm32f429-pinctrl.dtsi"
>>>    #include <dt-bindings/input/input.h>
>>> +#include <dt-bindings/interrupt-controller/irq.h>
>>> +#include <dt-bindings/gpio/gpio.h>
>>>
>>>    / {
>>>        model = "STMicroelectronics STM32F429i-DISCO board";
>>> @@ -127,3 +129,26 @@
>>>        pinctrl-names = "default";
>>>        status = "okay";
>>>    };
>>> +
>>> +&spi5 {
>>> +     status = "okay";
>>> +     pinctrl-0 = <&spi5_pins>;
>>> +     pinctrl-names = "default";
>>> +     #address-cells = <1>;
>>> +     #size-cells = <0>;
>>> +     cs-gpios = <&gpioc 1 GPIO_ACTIVE_LOW>;
>>> +     dmas = <&dma2 3 2 0x400 0x0>,
>>> +            <&dma2 4 2 0x400 0x0>;
>>> +     dma-names = "rx", "tx";
>>> +
>>
>> You added this spi5 node in this patch but also in the display series. I
>> will have issue to merge. Even if I could fix it easily, as you are
>> going to resend, the good practice could be to have several patches in
>> one series: one patch for spi5 controller, another for gyro and another
>> for display.
>>
>> And also same remark than Linus did in display series move DMA to soc
>> dtsi file please.
> 
> Sure, how about this patch sequence:
> 1 add spi5 dma to soc (stm32f429.dtsi)
> 2 add pin map for spi5 (stm32f4-pinctrl.dtsi)
> 3 add spi5 controller with gyro (stm32f429-disco.dts)
> 4 add spi modification to support gyro (spi-stm32.c)
> 
> 5 add ltdc pin map for stm32f429-disco board (stm32f4-pinctrl.dtsi)
> 6 add ilitek-ili9341 dts bindings for disco (stm32f429-disco.dts,
> depends on above step 3)
> 7 add yaml document about ilitek-ili9341 (ilitek,ili9341.yaml)
> 8 add clk changes to support ltdc driver (clk-stm32f4.c)
> 
> so, i will combine gyro and display two patches to one patchset next
> time. right ?
> 

looks good

> thanks.
>>
>>
>>> +     l3gd20: l3gd20@0 {
>>> +             compatible = "st,l3gd20-gyro";
>>> +             spi-max-frequency = <10000000>;
>>> +             st,drdy-int-pin = <2>;
>>> +             interrupt-parent = <&gpioa>;
>>> +             interrupts = <1 IRQ_TYPE_EDGE_RISING>,
>>> +                             <2 IRQ_TYPE_EDGE_RISING>;
>>> +             reg = <0>;
>>> +             status = "okay";
>>> +     };
>>> +};
>>>

[PATCH v2 3/3] spi: stm32: Add SPI_SIMPLEX_RX, SPI_3WIRE_RX support for stm32f4

[dillon.minfei]

From: dillon min <dillon.minfei@gmail.com>

in l3gd20 driver startup, there is a setup failed error return from
stm32 spi driver

    "
    [    2.687630] st-gyro-spi spi0.0: supply vdd not found, using dummy
    regulator
    [    2.696869] st-gyro-spi spi0.0: supply vddio not found, using dummy
    regulator
    [    2.706707] spi_stm32 40015000.spi: SPI transfer setup failed
    [    2.713741] st-gyro-spi spi0.0: SPI transfer failed: -22
    [    2.721096] spi_master spi0: failed to transfer one message from queue
    [    2.729268] iio iio:device0: failed to read Who-Am-I register.
    [    2.737504] st-gyro-spi: probe of spi0.0 failed with error -22
    "

after debug into spi-stm32 driver, st-gyro-spi split two steps to read
l3gd20 id

first: send command to l3gd20 with read id command in tx_buf,
       rx_buf is null.
second: read id with tx_buf is null, rx_buf not null.

so, for second step, stm32 driver recongise this process is SPI_SIMPLE_RX
from stm32_spi_communication_type, but there is no related process for this
type in stm32f4_spi_set_mode, then we get error from
stm32_spi_transfer_one_setup.

we can use two method to fix this bug.
1, use stm32 spi's "In unidirectional receive-only mode (BIDIMODE=0 and
   RXONLY=1)". but as our code running in sdram, the read latency is
   too large to get so many receive overrun error in interrupts handler.

2, use stm32 spi's "In full-duplex (BIDIMODE=0 and RXONLY=0)", as
   tx_buf is null, we must add dummy data sent out before read data.
   so, add stm32f4_spi_tx_dummy to handle this situation.

Signed-off-by: dillon min <dillon.minfei@gmail.com>
---
 drivers/spi/spi-stm32.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 44ac6eb3..bcf1ba7 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -388,6 +388,13 @@ static int stm32h7_spi_get_fifo_size(struct stm32_spi *spi)
 	return count;
 }
 
+static void stm32f4_spi_tx_dummy(struct stm32_spi *spi)
+{
+	if (spi->cur_bpw == 16)
+		writew_relaxed(0x5555, spi->base + STM32F4_SPI_DR);
+	else
+		writeb_relaxed(0x55, spi->base + STM32F4_SPI_DR);
+}
 /**
  * stm32f4_spi_get_bpw_mask - Return bits per word mask
  * @spi: pointer to the spi controller data structure
@@ -811,7 +818,9 @@ static irqreturn_t stm32f4_spi_irq_event(int irq, void *dev_id)
 		mask |= STM32F4_SPI_SR_TXE;
 	}
 
-	if (!spi->cur_usedma && spi->cur_comm == SPI_FULL_DUPLEX) {
+	if (!spi->cur_usedma && (spi->cur_comm == SPI_FULL_DUPLEX ||
+				 spi->cur_comm == SPI_SIMPLEX_RX ||
+				 spi->cur_comm == SPI_3WIRE_RX)) {
 		/* TXE flag is set and is handled when RXNE flag occurs */
 		sr &= ~STM32F4_SPI_SR_TXE;
 		mask |= STM32F4_SPI_SR_RXNE | STM32F4_SPI_SR_OVR;
@@ -850,8 +859,10 @@ static irqreturn_t stm32f4_spi_irq_event(int irq, void *dev_id)
 		stm32f4_spi_read_rx(spi);
 		if (spi->rx_len == 0)
 			end = true;
-		else /* Load data for discontinuous mode */
+		else if (spi->tx_buf)/* Load data for discontinuous mode */
 			stm32f4_spi_write_tx(spi);
+		else if (spi->cur_comm == SPI_SIMPLEX_RX)
+			stm32f4_spi_tx_dummy(spi);
 	}
 
 end_irq:
@@ -1151,7 +1162,9 @@ static int stm32f4_spi_transfer_one_irq(struct stm32_spi *spi)
 	/* Enable the interrupts relative to the current communication mode */
 	if (spi->cur_comm == SPI_SIMPLEX_TX || spi->cur_comm == SPI_3WIRE_TX) {
 		cr2 |= STM32F4_SPI_CR2_TXEIE;
-	} else if (spi->cur_comm == SPI_FULL_DUPLEX) {
+	} else if (spi->cur_comm == SPI_FULL_DUPLEX ||
+				spi->cur_comm == SPI_SIMPLEX_RX ||
+				spi->cur_comm == SPI_3WIRE_RX) {
 		/* In transmit-only mode, the OVR flag is set in the SR register
 		 * since the received data are never read. Therefore set OVR
 		 * interrupt only when rx buffer is available.
@@ -1170,6 +1183,8 @@ static int stm32f4_spi_transfer_one_irq(struct stm32_spi *spi)
 	/* starting data transfer when buffer is loaded */
 	if (spi->tx_buf)
 		stm32f4_spi_write_tx(spi);
+	else if (spi->cur_comm == SPI_SIMPLEX_RX)
+		stm32f4_spi_tx_dummy(spi);
 
 	spin_unlock_irqrestore(&spi->lock, flags);
 
@@ -1462,10 +1477,16 @@ static int stm32f4_spi_set_mode(struct stm32_spi *spi, unsigned int comm_type)
 		stm32_spi_set_bits(spi, STM32F4_SPI_CR1,
 					STM32F4_SPI_CR1_BIDIMODE |
 					STM32F4_SPI_CR1_BIDIOE);
-	} else if (comm_type == SPI_FULL_DUPLEX) {
+	} else if (comm_type == SPI_FULL_DUPLEX ||
+				comm_type == SPI_SIMPLEX_RX) {
 		stm32_spi_clr_bits(spi, STM32F4_SPI_CR1,
 					STM32F4_SPI_CR1_BIDIMODE |
 					STM32F4_SPI_CR1_BIDIOE);
+	} else if (comm_type == SPI_3WIRE_RX) {
+		stm32_spi_set_bits(spi, STM32F4_SPI_CR1,
+					STM32F4_SPI_CR1_BIDIMODE);
+		stm32_spi_clr_bits(spi, STM32F4_SPI_CR1,
+					STM32F4_SPI_CR1_BIDIOE);
 	} else {
 		return -EINVAL;
 	}
-- 
2.7.4