diff mbox

[v4,6/6] ARM: zynq: DT: Add OCM controller node

Message ID 0a88d52b5e71fb220ca619180ec9ca746edad2b1.1415962281.git.michal.simek@xilinx.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michal Simek Nov. 14, 2014, 10:52 a.m. UTC
Add the on-chip-memory controller node to the Zynq devicetree.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v4:
- Use memory-controller@... instead of ocmc@...

Changes in v3:
- Extract from OCM driver

Changes in v2: None

 arch/arm/boot/dts/zynq-7000.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

--
1.8.2.3

Comments

Andreas Färber Nov. 16, 2014, 10:51 a.m. UTC | #1
Hi Michal,

Am 14.11.2014 um 11:52 schrieb Michal Simek:
> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> index ce2ef5bec4f2..e217fb1c1169 100644
> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> @@ -150,6 +150,13 @@
>  			reg = <0xf8006000 0x1000>;
>  		};
> 
> +		ocmc: memory-controller@f800c000 {
> +			compatible = "xlnx,zynq-ocmc-1.0";
> +			interrupt-parent = <&intc>;
> +			interrupts = <0 3 4>;
> +			reg = <0xf800c000 0x1000>;
> +		};
> +
>  		uart0: serial@e0000000 {
>  			compatible = "xlnx,xuartps", "cdns,uart-r1p8";
>  			status = "disabled";

Not directly related to this patch: As one can see here, the node order
is quite a mess... According to Olof, nodes should be ordered by unit
address, whereas here some but not all seem ordered by node name. Would
you welcome a cleanup patch, or can you fix that yourself?

Regards,
Andreas
Soren Brinkmann Nov. 16, 2014, 7:32 p.m. UTC | #2
On Sun, 2014-11-16 at 11:51AM +0100, Andreas Färber wrote:
> Hi Michal,
> 
> Am 14.11.2014 um 11:52 schrieb Michal Simek:
> > diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> > index ce2ef5bec4f2..e217fb1c1169 100644
> > --- a/arch/arm/boot/dts/zynq-7000.dtsi
> > +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> > @@ -150,6 +150,13 @@
> >  			reg = <0xf8006000 0x1000>;
> >  		};
> > 
> > +		ocmc: memory-controller@f800c000 {
> > +			compatible = "xlnx,zynq-ocmc-1.0";
> > +			interrupt-parent = <&intc>;
> > +			interrupts = <0 3 4>;
> > +			reg = <0xf800c000 0x1000>;
> > +		};
> > +
> >  		uart0: serial@e0000000 {
> >  			compatible = "xlnx,xuartps", "cdns,uart-r1p8";
> >  			status = "disabled";
> 
> Not directly related to this patch: As one can see here, the node order
> is quite a mess... According to Olof, nodes should be ordered by unit
> address, whereas here some but not all seem ordered by node name. Would
> you welcome a cleanup patch, or can you fix that yourself?

I wouldn't say it's a mess, just a different property to sort the nodes
by. For humans reading the DT, searching for nodes, alphabetical order
helps finding the right node, IMHO. What advantage would sorting by
address have?

	Thanks,
	Sören
Peter Crosthwaite Nov. 16, 2014, 11 p.m. UTC | #3
On Mon, Nov 17, 2014 at 5:32 AM, Sören Brinkmann
<soren.brinkmann@xilinx.com> wrote:
> On Sun, 2014-11-16 at 11:51AM +0100, Andreas Färber wrote:
>> Hi Michal,
>>
>> Am 14.11.2014 um 11:52 schrieb Michal Simek:
>> > diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>> > index ce2ef5bec4f2..e217fb1c1169 100644
>> > --- a/arch/arm/boot/dts/zynq-7000.dtsi
>> > +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>> > @@ -150,6 +150,13 @@
>> >                     reg = <0xf8006000 0x1000>;
>> >             };
>> >
>> > +           ocmc: memory-controller@f800c000 {
>> > +                   compatible = "xlnx,zynq-ocmc-1.0";
>> > +                   interrupt-parent = <&intc>;
>> > +                   interrupts = <0 3 4>;
>> > +                   reg = <0xf800c000 0x1000>;
>> > +           };
>> > +
>> >             uart0: serial@e0000000 {
>> >                     compatible = "xlnx,xuartps", "cdns,uart-r1p8";
>> >                     status = "disabled";
>>
>> Not directly related to this patch: As one can see here, the node order
>> is quite a mess... According to Olof, nodes should be ordered by unit
>> address, whereas here some but not all seem ordered by node name. Would
>> you welcome a cleanup patch, or can you fix that yourself?
>
> I wouldn't say it's a mess, just a different property to sort the nodes
> by. For humans reading the DT, searching for nodes, alphabetical order
> helps finding the right node, IMHO.

I do generally find myself asking "whats that thing at that address"
more than I find myself asking the "wheres that piece of hardware" so
Andreas' sorting scheme makes more sense to me. Vertically scanning a
DT to give yourself an overview of the system level address map is
good too. Wheras alphabetic sorting doesn't mean to much.

Regards,
Peter

What advantage would sorting by
> address have?
>
>         Thanks,
>         Sören
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
Michal Simek Nov. 18, 2014, 7:56 a.m. UTC | #4
On 11/17/2014 12:00 AM, Peter Crosthwaite wrote:
> On Mon, Nov 17, 2014 at 5:32 AM, Sören Brinkmann
> <soren.brinkmann@xilinx.com> wrote:
>> On Sun, 2014-11-16 at 11:51AM +0100, Andreas Färber wrote:
>>> Hi Michal,
>>>
>>> Am 14.11.2014 um 11:52 schrieb Michal Simek:
>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>> index ce2ef5bec4f2..e217fb1c1169 100644
>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>> @@ -150,6 +150,13 @@
>>>>                     reg = <0xf8006000 0x1000>;
>>>>             };
>>>>
>>>> +           ocmc: memory-controller@f800c000 {
>>>> +                   compatible = "xlnx,zynq-ocmc-1.0";
>>>> +                   interrupt-parent = <&intc>;
>>>> +                   interrupts = <0 3 4>;
>>>> +                   reg = <0xf800c000 0x1000>;
>>>> +           };
>>>> +
>>>>             uart0: serial@e0000000 {
>>>>                     compatible = "xlnx,xuartps", "cdns,uart-r1p8";
>>>>                     status = "disabled";
>>>
>>> Not directly related to this patch: As one can see here, the node order
>>> is quite a mess... According to Olof, nodes should be ordered by unit
>>> address, whereas here some but not all seem ordered by node name. Would
>>> you welcome a cleanup patch, or can you fix that yourself?
>>
>> I wouldn't say it's a mess, just a different property to sort the nodes
>> by. For humans reading the DT, searching for nodes, alphabetical order
>> helps finding the right node, IMHO.
> 
> I do generally find myself asking "whats that thing at that address"
> more than I find myself asking the "wheres that piece of hardware" so
> Andreas' sorting scheme makes more sense to me. Vertically scanning a
> DT to give yourself an overview of the system level address map is
> good too. Wheras alphabetic sorting doesn't mean to much.

IMHO the reason why we have names in DT is that it is easily to read/understand them
that's why name sorting seems to me more reasonable.
Something like machine code and assembler - asm is also sorted by names not by opcode.

Is this strict rule?

Thanks,
Michal
diff mbox

Patch

diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index ce2ef5bec4f2..e217fb1c1169 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -150,6 +150,13 @@ 
 			reg = <0xf8006000 0x1000>;
 		};

+		ocmc: memory-controller@f800c000 {
+			compatible = "xlnx,zynq-ocmc-1.0";
+			interrupt-parent = <&intc>;
+			interrupts = <0 3 4>;
+			reg = <0xf800c000 0x1000>;
+		};
+
 		uart0: serial@e0000000 {
 			compatible = "xlnx,xuartps", "cdns,uart-r1p8";
 			status = "disabled";