mbox series

[0/7] spi: stm32: add spi slave mode

Message ID 20230428121524.2125832-1-valentin.caron@foss.st.com
Headers show
Series spi: stm32: add spi slave mode | expand

Message

Valentin Caron April 28, 2023, 12:15 p.m. UTC
STM32 SPI can operate in slave mode.
This series add this functionnality in spi-stm32 driver.

The device-tree property st,spi-slave-underrun can configure STM32 SPI
reaction to an underrun.

Alain Volmat (3):
  spi: stm32: renaming of spi_master into spi_controller
  spi: stm32: use dmaengine_terminate_{a}sync instead of _all
  dt-bindings: spi: stm32: add bindings regarding stm32h7 spi slave

Valentin Caron (4):
  dt-bindings: spi: stm32: add address-cells and size-cells into yaml
  spi: stm32: introduction of stm32h7 SPI slave support
  dt-bindings: spi: stm32: add stm32h7 st,spi-slave-underrun property
  spi: stm32: add support for stm32h7 SPI slave underrun detection

 .../devicetree/bindings/spi/st,stm32-spi.yaml |  19 +
 MAINTAINERS                                   |   1 +
 drivers/spi/Kconfig                           |   1 +
 drivers/spi/spi-stm32.c                       | 386 ++++++++++++------
 include/dt-bindings/spi/spi-stm32.h           |  15 +
 5 files changed, 307 insertions(+), 115 deletions(-)
 create mode 100644 include/dt-bindings/spi/spi-stm32.h

Comments

Rob Herring April 28, 2023, 9:41 p.m. UTC | #1
On Fri, Apr 28, 2023 at 02:15:18PM +0200, Valentin Caron wrote:
> Theses properties need to be described to satisfy dtbs_check.

No, they are defined in spi-controller.yaml, so they should not be 
needed here.

> 
> Signed-off-by: Valentin Caron <valentin.caron@foss.st.com>
> ---
>  Documentation/devicetree/bindings/spi/st,stm32-spi.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
> index 9ca1a843c820..c599eb359d56 100644
> --- a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
> +++ b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
> @@ -29,6 +29,9 @@ allOf:
>          st,spi-midi-ns: false
>  
>  properties:
> +  "#address-cells": true
> +  "#size-cells": true
> +
>    compatible:
>      enum:
>        - st,stm32f4-spi
> -- 
> 2.25.1
>
Rob Herring April 28, 2023, 9:56 p.m. UTC | #2
On Fri, Apr 28, 2023 at 02:15:23PM +0200, Valentin Caron wrote:
> This property is used to enable and configure stm32h7 SPI controller to
> handle underrun that could appear in slave mode.
> 
> Signed-off-by: Valentin Caron <valentin.caron@foss.st.com>
> ---
>  .../devicetree/bindings/spi/st,stm32-spi.yaml     |  8 ++++++++
>  MAINTAINERS                                       |  1 +
>  include/dt-bindings/spi/spi-stm32.h               | 15 +++++++++++++++
>  3 files changed, 24 insertions(+)
>  create mode 100644 include/dt-bindings/spi/spi-stm32.h
> 
> diff --git a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
> index 1d26fa2658c5..e946ea71a247 100644
> --- a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
> +++ b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
> @@ -28,6 +28,7 @@ allOf:
>        properties:
>          st,spi-midi-ns: false
>          spi-slave: false
> +        st,spi-slave-underrun: false
>  
>  properties:
>    "#address-cells": true
> @@ -70,6 +71,13 @@ properties:
>        In case of spi-slave defined, if <0>, indicate that SS should be
>        detected via the dedicated HW pin
>  
> +  st,spi-slave-underrun:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description:
> +      First parameter enables and selects slave underrun reaction.
> +      Refer to "dt-bindings/spi/spi-stm32.h" for the supported values.
> +      Second parameter is the pattern in case of SPI_SEND_PATTERN mode.

So, max 2 cells? Then:

minItems: 1
maxItems: 2

Though I don't really think this belongs in DT. The driver implementing 
the SPI slave function defines all the rest of the protocol the slave 
implements. Why not this little bit? Perhaps there is no way for a SPI 
slave driver to tell the SPI controller which controller specific mode 
to use, so you abuse DT to configure the SPI controller. Also, with a 
controller specific response, then the slave driver is coupled to that 
SPI controller which isn't great either.

Rob
Valentin Caron May 4, 2023, 5:13 p.m. UTC | #3
On 4/28/23 23:56, Rob Herring wrote:
> On Fri, Apr 28, 2023 at 02:15:23PM +0200, Valentin Caron wrote:
>> This property is used to enable and configure stm32h7 SPI controller to
>> handle underrun that could appear in slave mode.
>>
>> Signed-off-by: Valentin Caron <valentin.caron@foss.st.com>
>> ---
>>   .../devicetree/bindings/spi/st,stm32-spi.yaml     |  8 ++++++++
>>   MAINTAINERS                                       |  1 +
>>   include/dt-bindings/spi/spi-stm32.h               | 15 +++++++++++++++
>>   3 files changed, 24 insertions(+)
>>   create mode 100644 include/dt-bindings/spi/spi-stm32.h
>>
>> diff --git a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
>> index 1d26fa2658c5..e946ea71a247 100644
>> --- a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
>> +++ b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml
>> @@ -28,6 +28,7 @@ allOf:
>>         properties:
>>           st,spi-midi-ns: false
>>           spi-slave: false
>> +        st,spi-slave-underrun: false
>>   
>>   properties:
>>     "#address-cells": true
>> @@ -70,6 +71,13 @@ properties:
>>         In case of spi-slave defined, if <0>, indicate that SS should be
>>         detected via the dedicated HW pin
>>   
>> +  st,spi-slave-underrun:
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    description:
>> +      First parameter enables and selects slave underrun reaction.
>> +      Refer to "dt-bindings/spi/spi-stm32.h" for the supported values.
>> +      Second parameter is the pattern in case of SPI_SEND_PATTERN mode.
> So, max 2 cells? Then:
>
> minItems: 1
> maxItems: 2
>
> Though I don't really think this belongs in DT. The driver implementing
> the SPI slave function defines all the rest of the protocol the slave
> implements. Why not this little bit? Perhaps there is no way for a SPI
> slave driver to tell the SPI controller which controller specific mode
> to use, so you abuse DT to configure the SPI controller. Also, with a
> controller specific response, then the slave driver is coupled to that
> SPI controller which isn't great either.
>
> Rob
I basically made this DT property to configure the controler and doesn't
give attention to the fact that this parameter can came from framework.

I will not rework this functionality, but simply let it down as we has
no demands on it.

Thanks for all your remarks,
Valentin