diff mbox

[1/3] doc: dt: fpga: Added Documentation for Xilinx Zynq FPGA manager.

Message ID 1444344307-22509-2-git-send-email-moritz.fischer@ettus.com (mailing list archive)
State New, archived
Headers show

Commit Message

Moritz Fischer Oct. 8, 2015, 10:45 p.m. UTC
Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
---
 .../bindings/fpga/xilinx-zynq-fpga-mgr.txt         | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt

Comments

Josh Cartwright Oct. 9, 2015, 4:04 p.m. UTC | #1
On Fri, Oct 09, 2015 at 12:45:05AM +0200, Moritz Fischer wrote:
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> ---
>  .../bindings/fpga/xilinx-zynq-fpga-mgr.txt         | 26 ++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
> 
> diff --git a/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt b/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
> new file mode 100644
> index 0000000..82ffda8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
> @@ -0,0 +1,26 @@
> +Xilinx Zynq FPGA Manager
> +
> +Required properties:
> +- compatible:		should contain "xlnx,zynq-devcfg-1.0"
> +- reg:			base address and size for memory mapped io
> +- interrupt parent:	interrupt source phandle

I think you mean 'interrupt-parent', with the hyphen.

Actually, this isn't really a 'required' property of this node, as it
could be specified in a parent node.

  Josh
Michal Simek Oct. 12, 2015, 9:31 a.m. UTC | #2
On 10/09/2015 06:04 PM, Josh Cartwright wrote:
> On Fri, Oct 09, 2015 at 12:45:05AM +0200, Moritz Fischer wrote:
>> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
>> ---
>>  .../bindings/fpga/xilinx-zynq-fpga-mgr.txt         | 26 ++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
>>
>> diff --git a/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt b/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
>> new file mode 100644
>> index 0000000..82ffda8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
>> @@ -0,0 +1,26 @@
>> +Xilinx Zynq FPGA Manager
>> +
>> +Required properties:
>> +- compatible:		should contain "xlnx,zynq-devcfg-1.0"
>> +- reg:			base address and size for memory mapped io
>> +- interrupt parent:	interrupt source phandle
> 
> I think you mean 'interrupt-parent', with the hyphen.

yes.

> 
> Actually, this isn't really a 'required' property of this node, as it
> could be specified in a parent node.

yes. Normally interrupt-parent is not listed as required property in the
binding doc. Some docs listed it as optional property.

Thanks,
Michal
Soren Brinkmann Oct. 12, 2015, 4:33 p.m. UTC | #3
On Fri, 2015-10-09 at 12:45AM +0200, Moritz Fischer wrote:
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> ---
>  .../bindings/fpga/xilinx-zynq-fpga-mgr.txt         | 26 ++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
> 
> diff --git a/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt b/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
> new file mode 100644
> index 0000000..82ffda8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
> @@ -0,0 +1,26 @@
> +Xilinx Zynq FPGA Manager
> +
> +Required properties:
> +- compatible:		should contain "xlnx,zynq-devcfg-1.0"
> +- reg:			base address and size for memory mapped io
> +- interrupt parent:	interrupt source phandle
> +- interrupts:		interrupt for the FPGA manager device
> +- clocks:		phandle for clocks required operation
> +- syscon:		phandle for access to SLCR registers
> +
> +Optional properties:
> +- clock-names:		names for clocks

Is it optional? Currently, there is only one clock input, so a match
without specifying a clock name should work making this optional. But in
your implementation, you do specify a clock name in devm_clk_get(). I'm
not entirely sure, but that call might fail if it doesn't find the
corresponding clock-names property.
I think, either we should make this required and list the required
entries here. Or the implementation probably needs to drop the clock
name when looking up its input clock.

	Sören
Moritz Fischer Oct. 12, 2015, 8:41 p.m. UTC | #4
Hi Michal, Josh,

On Mon, Oct 12, 2015 at 2:31 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> On 10/09/2015 06:04 PM, Josh Cartwright wrote:
>> On Fri, Oct 09, 2015 at 12:45:05AM +0200, Moritz Fischer wrote:
>>> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
>>> ---
>>>  .../bindings/fpga/xilinx-zynq-fpga-mgr.txt         | 26 ++++++++++++++++++++++
>>>  1 file changed, 26 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt b/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
>>> new file mode 100644
>>> index 0000000..82ffda8
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
>>> @@ -0,0 +1,26 @@
>>> +Xilinx Zynq FPGA Manager
>>> +
>>> +Required properties:
>>> +- compatible:               should contain "xlnx,zynq-devcfg-1.0"
>>> +- reg:                      base address and size for memory mapped io
>>> +- interrupt parent: interrupt source phandle
>>
>> I think you mean 'interrupt-parent', with the hyphen.
>
> yes.
>
>>
>> Actually, this isn't really a 'required' property of this node, as it
>> could be specified in a parent node.
>
> yes. Normally interrupt-parent is not listed as required property in the
> binding doc. Some docs listed it as optional property.

Alright, will correct typo, and mark it as optional unless someone
feels strongly against having the optional.
>
> Thanks,
> Michal

Thanks for the feedback,

Moritz
Moritz Fischer Oct. 12, 2015, 9:24 p.m. UTC | #5
Soeren,

On Mon, Oct 12, 2015 at 9:33 AM, Sören Brinkmann
<soren.brinkmann@xilinx.com> wrote:
> On Fri, 2015-10-09 at 12:45AM +0200, Moritz Fischer wrote:
>> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
>> ---
>>  .../bindings/fpga/xilinx-zynq-fpga-mgr.txt         | 26 ++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
>>
>> diff --git a/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt b/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
>> new file mode 100644
>> index 0000000..82ffda8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
>> @@ -0,0 +1,26 @@
>> +Xilinx Zynq FPGA Manager
>> +
>> +Required properties:
>> +- compatible:                should contain "xlnx,zynq-devcfg-1.0"
>> +- reg:                       base address and size for memory mapped io
>> +- interrupt parent:  interrupt source phandle
>> +- interrupts:                interrupt for the FPGA manager device
>> +- clocks:            phandle for clocks required operation
>> +- syscon:            phandle for access to SLCR registers
>> +
>> +Optional properties:
>> +- clock-names:               names for clocks
>
> Is it optional? Currently, there is only one clock input, so a match
> without specifying a clock name should work making this optional. But in
> your implementation, you do specify a clock name in devm_clk_get(). I'm
> not entirely sure, but that call might fail if it doesn't find the
> corresponding clock-names property.
> I think, either we should make this required and list the required
> entries here. Or the implementation probably needs to drop the clock
> name when looking up its input clock.

I kinda prefer the named one, so I'll probaly adapt the
implementation, any feelings either way anyone?
>someone
>         Sören

Thanks,

Moritz
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt b/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
new file mode 100644
index 0000000..82ffda8
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
@@ -0,0 +1,26 @@ 
+Xilinx Zynq FPGA Manager
+
+Required properties:
+- compatible:		should contain "xlnx,zynq-devcfg-1.0"
+- reg:			base address and size for memory mapped io
+- interrupt parent:	interrupt source phandle
+- interrupts:		interrupt for the FPGA manager device
+- clocks:		phandle for clocks required operation
+- syscon:		phandle for access to SLCR registers
+
+Optional properties:
+- clock-names:		names for clocks
+
+
+
+Example:
+	devcfg: devcfg@f8007000 {
+		compatible = "xlnx,zynq-devcfg-1.0";
+		reg = <0xf8007000 0x100>;
+		interrupt-parent = <&intc>;
+		interrupts = <0 8 4>;
+		clocks = <&clkc 12>;
+		clock-names = "ref_clk";
+		syscon = <&slcr>;
+	};
+