diff mbox series

[V2,3/4] spi: dt-bindings: Add Tegra TPM wait polling flag

Message ID 20230203130133.32901-4-kyarlagadda@nvidia.com
State New
Headers
Series Tegra TPM driver with hw flow control |

Commit Message

Krishna Yarlagadda Feb. 3, 2023, 1:01 p.m. UTC 
  Add "nvidia,wait-polling" flag to enable TCG TIS hardware flow control.

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 .../bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Rob Herring Feb. 3, 2023, 7:49 p.m. UTC | #1 
On Fri, Feb 3, 2023 at 7:02 AM Krishna Yarlagadda
<kyarlagadda@nvidia.com> wrote:
>
> Add "nvidia,wait-polling" flag to enable TCG TIS hardware flow control.

Tell me something that the diff doesn't.

>
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  .../bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml
> index 2c3cada75339..19d2b30cadbf 100644
> --- a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml
> +++ b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml
> @@ -29,4 +29,10 @@ properties:
>      minimum: 0
>      maximum: 255
>
> +  nvidia,wait-polling:
> +    description:
> +      Enable TPM wait polling feature for QSPI as specified in TCG PC Client
> +      Specific TPM Interface Specification (TIS).
> +    $ref: /schemas/types.yaml#/definitions/flag

Why do you need this flag when you have a compatible that also
indicates you have a quirk.

If this a TPM feature, why is it enabled for every single SPI slave device?

If the fundamental issue is the controller only supports half-duplex,
why can't you just check that from the driver? Can't the SPI subsystem
tell you that the host controller is half-duplex? Though sometimes
that may be board level property I suppose. If so, define the h/w
quirk, not the driver mode in DT. Half-duplex is probably something
everyone could use, not just Nvidia.

Please discuss this series internally with the folks you marked as
maintainers. It has issues I'm sure they would have also pointed out.

Rob
Krishna Yarlagadda Feb. 23, 2023, 4:26 p.m. UTC | #2 
> -----Original Message-----
> From: Rob Herring <robh+dt@kernel.org>
> Sent: 04 February 2023 01:20
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Cc: broonie@kernel.org; peterhuewe@gmx.de; jgg@ziepe.ca;
> jarkko@kernel.org; krzysztof.kozlowski+dt@linaro.org; linux-
> spi@vger.kernel.org; linux-tegra@vger.kernel.org; linux-
> integrity@vger.kernel.org; linux-kernel@vger.kernel.org;
> thierry.reding@gmail.com; Jonathan Hunter <jonathanh@nvidia.com>;
> Sowjanya Komatineni <skomatineni@nvidia.com>; Laxman Dewangan
> <ldewangan@nvidia.com>
> Subject: Re: [Patch V2 3/4] spi: dt-bindings: Add Tegra TPM wait polling flag
> 
> External email: Use caution opening links or attachments
> 
> 
> On Fri, Feb 3, 2023 at 7:02 AM Krishna Yarlagadda
> <kyarlagadda@nvidia.com> wrote:
> >
> > Add "nvidia,wait-polling" flag to enable TCG TIS hardware flow control.
> 
> Tell me something that the diff doesn't.
> 
> >
> > Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> > ---
> >  .../bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-
> peripheral-props.yaml
> b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-
> props.yaml
> > index 2c3cada75339..19d2b30cadbf 100644
> > --- a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-
> peripheral-props.yaml
> > +++ b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-
> peripheral-props.yaml
> > @@ -29,4 +29,10 @@ properties:
> >      minimum: 0
> >      maximum: 255
> >
> > +  nvidia,wait-polling:
> > +    description:
> > +      Enable TPM wait polling feature for QSPI as specified in TCG PC Client
> > +      Specific TPM Interface Specification (TIS).
> > +    $ref: /schemas/types.yaml#/definitions/flag
> 
> Why do you need this flag when you have a compatible that also
> indicates you have a quirk.
> 
> If this a TPM feature, why is it enabled for every single SPI slave device?
> 
> If the fundamental issue is the controller only supports half-duplex,
> why can't you just check that from the driver? Can't the SPI subsystem
> tell you that the host controller is half-duplex? Though sometimes
> that may be board level property I suppose. If so, define the h/w
> quirk, not the driver mode in DT. Half-duplex is probably something
> everyone could use, not just Nvidia.
> 
> Please discuss this series internally with the folks you marked as
> maintainers. It has issues I'm sure they would have also pointed out.
> 
> Rob
QSPI is a multi-chip-select controller and HW wait polling is only for TPM
Both controller and device would need a flag/setting to identify support
for this feature. Using SPI controller flags and SPI device mode flags to
avoid device tree flags. Moved HW/alternate TPM flow control into existing
driver. No need of new compatible now.

KY
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml
index 2c3cada75339..19d2b30cadbf 100644
--- a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml
+++ b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml
@@ -29,4 +29,10 @@  properties:
     minimum: 0
     maximum: 255
 
+  nvidia,wait-polling:
+    description:
+      Enable TPM wait polling feature for QSPI as specified in TCG PC Client
+      Specific TPM Interface Specification (TIS).
+    $ref: /schemas/types.yaml#/definitions/flag
+
 additionalProperties: true