[2/3] doc: dt: add cyclone-spi binding document

Commit Message

Joshua Clayton Oct. 6, 2016, 8:34 p.m. UTC

Describe a cyclonespi devicetree entry, required features

Signed-off-by: Joshua Clayton <[e-mail]>
---
 .../bindings/fpga/cyclone-spi-fpga-mgr.txt         | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/cyclone-spi-fpga-mgr.txt

Comments

Moritz Fischer Oct. 7, 2016, 2:53 a.m. UTC | #1

Hi Joshua,

On Thu, Oct 6, 2016 at 1:34 PM, Joshua Clayton <[e-mail]> wrote:
> Describe a cyclonespi devicetree entry, required features
>
> Signed-off-by: Joshua Clayton <[e-mail]>
> ---
>  .../bindings/fpga/cyclone-spi-fpga-mgr.txt         | 23 ++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/fpga/cyclone-spi-fpga-mgr.txt
>
> diff --git a/Documentation/devicetree/bindings/fpga/cyclone-spi-fpga-mgr.txt b/Documentation/devicetree/bindings/fpga/cyclone-spi-fpga-mgr.txt
> new file mode 100644
> index 0000000..8de34db
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fpga/cyclone-spi-fpga-mgr.txt
> @@ -0,0 +1,23 @@
> +Altera SOCFPGA FPGA Manager

Copy & Paste? :)

> +Altera cyclone FPGAs support a method of loading the bitstream over what is

cyclone->Cyclone

> +referred to as "passive serial".
> +The passive serial link is not technically spi, and might require extra
> +circuits in order to play nicely with other spi slaves on the same bus.
> +
> +See https://www.altera.com/literature/hb/cyc/cyc_c51013.pdf
> +
> +Required properties:
> +- compatible  : should contain "altr,cyclonespi-fpga-mgr"

Alan, do you guys have any input on the compat string?

I think generally the bindings should go before the actual usage in
your patch series. Meaning you wanna document the binding
before you use it. I think this patch should be [1/3].

Cheers,

Moritz

Joshua Clayton Oct. 7, 2016, 8:41 p.m. UTC | #2

Moritz,

thank you very much for the review.
 
On 10/06/2016 07:53 PM, Moritz Fischer wrote:
> Hi Joshua,
>
> On Thu, Oct 6, 2016 at 1:34 PM, Joshua Clayton <[e-mail]> wrote:
>> Describe a cyclonespi devicetree entry, required features
>>
>> Signed-off-by: Joshua Clayton <[e-mail]>
>> ---
>>  .../bindings/fpga/cyclone-spi-fpga-mgr.txt         | 23 ++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/fpga/cyclone-spi-fpga-mgr.txt
>>
>> diff --git a/Documentation/devicetree/bindings/fpga/cyclone-spi-fpga-mgr.txt b/Documentation/devicetree/bindings/fpga/cyclone-spi-fpga-mgr.txt
>> new file mode 100644
>> index 0000000..8de34db
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/fpga/cyclone-spi-fpga-mgr.txt
>> @@ -0,0 +1,23 @@
>> +Altera SOCFPGA FPGA Manager
> Copy & Paste? :)
Oops :(
As you might image, documentation was the last item done with the least attention.
Will fix.
>
>> +Altera cyclone FPGAs support a method of loading the bitstream over what is
> cyclone->Cyclone
OK.
>> +referred to as "passive serial".
>> +The passive serial link is not technically spi, and might require extra
>> +circuits in order to play nicely with other spi slaves on the same bus.
>> +
>> +See https://www.altera.com/literature/hb/cyc/cyc_c51013.pdf
>> +
>> +Required properties:
>> +- compatible  : should contain "altr,cyclonespi-fpga-mgr"
> Alan, do you guys have any input on the compat string?
I am open to change if it makes sense. I tried to keep the format similar.
> I think generally the bindings should go before the actual usage in
> your patch series. Meaning you wanna document the binding
> before you use it. I think this patch should be [1/3].
Ah, In my mind I had it backwards.
> Cheers,
>
> Moritz
I'll give Alan a chance to review and then spin a V2

Joshua Clayton

Alan Tull Oct. 10, 2016, 4:24 p.m. UTC | #3

On Fri, 7 Oct 2016, Moritz Fischer wrote:

> > +referred to as "passive serial".
> > +The passive serial link is not technically spi, and might require extra
> > +circuits in order to play nicely with other spi slaves on the same bus.
> > +
> > +See https://www.altera.com/literature/hb/cyc/cyc_c51013.pdf
> > +
> > +Required properties:
> > +- compatible  : should contain "altr,cyclonespi-fpga-mgr"
> 
> Alan, do you guys have any input on the compat string?

We want to make it clear that this is for fpga configuration
using a specific programming method (passive serial).

How about altr,cyclone-ps-spi ?

> 
> I think generally the bindings should go before the actual usage in
> your patch series. Meaning you wanna document the binding
> before you use it. I think this patch should be [1/3].

I agree.

Thanks,
Alan

> 
> Cheers,
> 
> Moritz
>

Patch

diff --git a/Documentation/devicetree/bindings/fpga/cyclone-spi-fpga-mgr.txt b/Documentation/devicetree/bindings/fpga/cyclone-spi-fpga-mgr.txt
new file mode 100644
index 0000000..8de34db
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/cyclone-spi-fpga-mgr.txt
@@ -0,0 +1,23 @@ 
+Altera SOCFPGA FPGA Manager
+
+Altera cyclone FPGAs support a method of loading the bitstream over what is
+referred to as "passive serial".
+The passive serial link is not technically spi, and might require extra
+circuits in order to play nicely with other spi slaves on the same bus.
+
+See https://www.altera.com/literature/hb/cyc/cyc_c51013.pdf
+
+Required properties:
+- compatible  : should contain "altr,cyclonespi-fpga-mgr"
+- reg         : spi slave id of the fpga
+- reset-gpio  : reset pin (referred to as nCONFIG in the cyclone manual)
+- status-gpio : status pin (referred to as nSTATUS in the cyclone manual)
+
+Example:
+	fpga_spi: evi-fpga-spi@0 {
+		compatible = "altr,cyclonespi-fpga-mgr";
+		spi-max-frequency = <20000000>;
+		reg = <0>;
+		reset-gpio = <&gpio4 9 GPIO_ACTIVE_HIGH>;
+		status-gpio = <&gpio4 11 GPIO_ACTIVE_HIGH>;
+	};