linux-fpga.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dt-bindings: fpga: microchip,mpf-spi-fpga-mgr: document CPOL/CPHA support
@ 2024-02-21 19:12 Marco Felsch
  2024-02-22 17:23 ` Conor Dooley
  0 siblings, 1 reply; 4+ messages in thread
From: Marco Felsch @ 2024-02-21 19:12 UTC (permalink / raw)
  To: conor.dooley, v.georgiev, mdf, hao.wu, yilun.xu, trix, robh+dt,
	krzysztof.kozlowski+dt
  Cc: linux-fpga, devicetree, kernel

Microchip FPGAs can communicate in different modes, so document them to
avoid dt-validate warnings.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 .../devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml   | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml b/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml
index a157eecfb5fc..bb9a7d16db60 100644
--- a/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml
+++ b/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml
@@ -22,6 +22,9 @@ properties:
     description: SPI chip select
     maxItems: 1
 
+  spi-cpol: true
+  spi-cpha: true
+
 required:
   - compatible
   - reg
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] dt-bindings: fpga: microchip,mpf-spi-fpga-mgr: document CPOL/CPHA support
  2024-02-21 19:12 [PATCH] dt-bindings: fpga: microchip,mpf-spi-fpga-mgr: document CPOL/CPHA support Marco Felsch
@ 2024-02-22 17:23 ` Conor Dooley
  2024-02-22 20:02   ` Marco Felsch
  0 siblings, 1 reply; 4+ messages in thread
From: Conor Dooley @ 2024-02-22 17:23 UTC (permalink / raw)
  To: Marco Felsch
  Cc: conor.dooley, v.georgiev, mdf, hao.wu, yilun.xu, trix, robh+dt,
	krzysztof.kozlowski+dt, linux-fpga, devicetree, kernel

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

On Wed, Feb 21, 2024 at 08:12:47PM +0100, Marco Felsch wrote:
> Microchip FPGAs can communicate in different modes, so document them to
> avoid dt-validate warnings.

Are you sure it can "communicate in different modes"? The documentation
actually says "Motorla SPI Mode 3 is required to communicate with M2S,
M2GL, and MPF devices using dedicated system controller SPI port" with
mode 3 being SPO = SPH = 1:
https://www.microsemi.com/document-portal/doc_view/137543-spi-directc-sp1-v2-0-user-guide

I suspect the answer is that it can actually communicate in different
modes (because I don't recall setting those options), but the binding
should enforce the correct way of doing it IMO.

Cheers,
Conor.

> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  .../devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml   | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml b/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml
> index a157eecfb5fc..bb9a7d16db60 100644
> --- a/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml
> +++ b/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml
> @@ -22,6 +22,9 @@ properties:
>      description: SPI chip select
>      maxItems: 1
>  
> +  spi-cpol: true
> +  spi-cpha: true
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.39.2
> 
> 

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] dt-bindings: fpga: microchip,mpf-spi-fpga-mgr: document CPOL/CPHA support
  2024-02-22 17:23 ` Conor Dooley
@ 2024-02-22 20:02   ` Marco Felsch
  2024-02-23 19:13     ` Conor Dooley
  0 siblings, 1 reply; 4+ messages in thread
From: Marco Felsch @ 2024-02-22 20:02 UTC (permalink / raw)
  To: Conor Dooley
  Cc: conor.dooley, v.georgiev, mdf, hao.wu, yilun.xu, trix, robh+dt,
	krzysztof.kozlowski+dt, linux-fpga, devicetree, kernel

On 24-02-22, Conor Dooley wrote:
> On Wed, Feb 21, 2024 at 08:12:47PM +0100, Marco Felsch wrote:
> > Microchip FPGAs can communicate in different modes, so document them to
> > avoid dt-validate warnings.
> 
> Are you sure it can "communicate in different modes"?

No I'm not but I didn't found an overview within the FPGA datasheet [1]
which modes are supported. What I did found was an note which says:

"""
1. Parameters are referenced to the active edge of SCK, which depends on
the configured SPI protocol (for example, Motorola SPI mode uses rising
edge as active edge if SPO = 0)
"""

Therefore I thought that this can be configured somehow differently.

[1] https://www.microsemi.com/document-portal/doc_view/136519-ds0141-polarfire-fpga-datasheet

> The documentation actually says "Motorla SPI Mode 3 is required to
> communicate with M2S, M2GL, and MPF devices using dedicated system
> controller SPI port" with mode 3 being SPO = SPH = 1:
> https://www.microsemi.com/document-portal/doc_view/137543-spi-directc-sp1-v2-0-user-guide

Thanks for the Pointer, there are plenty documents for the Polarfire
FPGA.

> I suspect the answer is that it can actually communicate in different
> modes (because I don't recall setting those options), but the binding
> should enforce the correct way of doing it IMO.

Sure, I will rephrase my commit message to:

"""
dt-bindings: fpga: microchip,mpf-spi-fpga-mgr: document CPOL/CPHA

Using the dedicated system controller SPI port requires Motorola SPI
Mode 3 according the SPI-DirectC v2.0 User Guid [1]. So require the
spi-cpol and spi-cpha to be set.

[1] https://www.microsemi.com/document-portal/doc_view/137543-spi-directc-sp1-v2-0-user-guide
"""

Regards,
  Marco



> 
> Cheers,
> Conor.
> 
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  .../devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml   | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml b/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml
> > index a157eecfb5fc..bb9a7d16db60 100644
> > --- a/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml
> > +++ b/Documentation/devicetree/bindings/fpga/microchip,mpf-spi-fpga-mgr.yaml
> > @@ -22,6 +22,9 @@ properties:
> >      description: SPI chip select
> >      maxItems: 1
> >  
> > +  spi-cpol: true
> > +  spi-cpha: true
> > +
> >  required:
> >    - compatible
> >    - reg
> > -- 
> > 2.39.2
> > 
> > 



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] dt-bindings: fpga: microchip,mpf-spi-fpga-mgr: document CPOL/CPHA support
  2024-02-22 20:02   ` Marco Felsch
@ 2024-02-23 19:13     ` Conor Dooley
  0 siblings, 0 replies; 4+ messages in thread
From: Conor Dooley @ 2024-02-23 19:13 UTC (permalink / raw)
  To: Marco Felsch
  Cc: conor.dooley, v.georgiev, mdf, hao.wu, yilun.xu, trix, robh+dt,
	krzysztof.kozlowski+dt, linux-fpga, devicetree, kernel

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

On Thu, Feb 22, 2024 at 09:02:30PM +0100, Marco Felsch wrote:
> On 24-02-22, Conor Dooley wrote:
> > On Wed, Feb 21, 2024 at 08:12:47PM +0100, Marco Felsch wrote:
> > > Microchip FPGAs can communicate in different modes, so document them to
> > > avoid dt-validate warnings.
> > 
> > Are you sure it can "communicate in different modes"?
> 
> No I'm not but I didn't found an overview within the FPGA datasheet [1]
> which modes are supported. What I did found was an note which says:
> 
> """
> 1. Parameters are referenced to the active edge of SCK, which depends on
> the configured SPI protocol (for example, Motorola SPI mode uses rising
> edge as active edge if SPO = 0)
> """
> 
> Therefore I thought that this can be configured somehow differently.
> 
> [1] https://www.microsemi.com/document-portal/doc_view/136519-ds0141-polarfire-fpga-datasheet
> 
> > The documentation actually says "Motorla SPI Mode 3 is required to
> > communicate with M2S, M2GL, and MPF devices using dedicated system
> > controller SPI port" with mode 3 being SPO = SPH = 1:
> > https://www.microsemi.com/document-portal/doc_view/137543-spi-directc-sp1-v2-0-user-guide
> 
> Thanks for the Pointer, there are plenty documents for the Polarfire
> FPGA.
> 
> > I suspect the answer is that it can actually communicate in different
> > modes (because I don't recall setting those options), but the binding
> > should enforce the correct way of doing it IMO.
> 
> Sure, I will rephrase my commit message to:
> 
> """
> dt-bindings: fpga: microchip,mpf-spi-fpga-mgr: document CPOL/CPHA
> 
> Using the dedicated system controller SPI port requires Motorola SPI
> Mode 3 according the SPI-DirectC v2.0 User Guid [1]. So require the
> spi-cpol and spi-cpha to be set.
> 
> [1] https://www.microsemi.com/document-portal/doc_view/137543-spi-directc-sp1-v2-0-user-guide
> """

You say require here, but don't actually make them required.
I think we probably should actually make them required, since the docs
say they should be used, but I'd like to look at it a bit more though
before that, since it does work on the setup I had without them.

I'd say do s/require/allow/ and I'll try to do some testing as to
whether we should actually mark the properties as required.

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-02-23 19:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-21 19:12 [PATCH] dt-bindings: fpga: microchip,mpf-spi-fpga-mgr: document CPOL/CPHA support Marco Felsch
2024-02-22 17:23 ` Conor Dooley
2024-02-22 20:02   ` Marco Felsch
2024-02-23 19:13     ` Conor Dooley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).