Re: [PATCH v3 1/5] dt-bindings: spi: Add SSTE support for DWC SSI controller

From: Serge Semin <fancer.lancer@gmail.com>
To: nandhini.srikandan@intel.com, broonie@kernel.org
Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>,
	robh+dt@kernel.org, linux-spi@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	mgross@linux.intel.com, kris.pan@intel.com,
	kenchappa.demakkanavar@intel.com, furong.zhou@intel.com,
	mallikarjunappa.sangannavar@intel.com, mahesh.r.vaidya@intel.com,
	rashmi.a@intel.com
Subject: Re: [PATCH v3 1/5] dt-bindings: spi: Add SSTE support for DWC SSI controller
Date: Thu, 11 Nov 2021 17:31:08 +0300	[thread overview]
Message-ID: <20211111143108.pxovseqvm2ywmoc2@mobilestation> (raw)
In-Reply-To: <20211111065201.10249-2-nandhini.srikandan@intel.com>

Hello Nandhini, Mark

On Thu, Nov 11, 2021 at 02:51:57PM +0800, nandhini.srikandan@intel.com wrote:
> From: Nandhini Srikandan <nandhini.srikandan@intel.com>
> 

> Add Slave Select Toggle Enable(SSTE) support for DWC SSI controller.

Nandhini, as Mark said this is no need in this new property since that
feature is supposed to be enabled by the client drivers by means of
setting the SPI_CS_WORD flag in the spi_device->mode field. (See its
usage for reference.)

BTW Mark, why not to have a generic DT-property which would set that
flag automatically by the SPI-core subsystem seeing it's indeed a
client device-property? For instance there can be some property like
"spi-cs-toggle" DT-property which when specified for the particular
SPI-client DT-node will make the SPI-core subsystem to set the
SPI_CS_WORD flag of the device mode? Like it has already been done for
"spi-cs-high"/"spi-lsb-first"/etc.
In this case Nandhini would need to just convert this patch a bit so
to be fixing the Documentation/devicetree/bindings/spi/spi-controller.yaml
bindings instead.

-Sergey

> 
> Signed-off-by: Nandhini Srikandan <nandhini.srikandan@intel.com>
> ---
>  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> index ca91201a9926..866416d01e94 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -149,6 +149,12 @@ patternProperties:
>            is an optional feature of the designware controller, and the
>            upper limit is also subject to controller configuration.
>  
> +      snps,sste:
> +        description: Slave select line will toggle between consecutive
> +          data frames, with the serial clock being held to its default
> +          value while slave select line is high.
> +        type: boolean
> +
>  unevaluatedProperties: false
>  
>  required:
> -- 
> 2.17.1
> 