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
State Under Review, 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
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>;
+	};
+