Re: [PATCH v3 11/25] ARM: dts: Add Actions Semi S500 and LeMaker Guitar

[Mark Rutland]

On Tue, Feb 28, 2017 at 04:13:00PM +0100, Andreas Färber wrote:
> Am 28.02.2017 um 13:32 schrieb Mark Rutland:
> > On Tue, Feb 28, 2017 at 07:35:21AM +0100, Andreas Färber wrote:

> >> +#include "skeleton.dtsi"
> > 
> > Please don't include skeleton.dtsi.
> 
> Wasn't aware that was permissible. I now see it has been updated with a
> textual comment, which predates my past uses of skeleton.dtsi - for
> other pending work such as FM4 and XMC4500 I only noticed it was moved
> out of armv7m.dtsi so that dtc started spewing scary messages. ;)
> 
> A #warning might be a more effective way to make people aware of its
> deprecation - but sadly we have users that predate #include:
> 
> $ git grep /include/ -- arch/arm/boot/dts/ | grep skeleton.dtsi | wc --lines
> 32
> $ git grep /include/ -- arch/arm/boot/dts/ | grep skeleton64.dtsi | wc
> --lines
> 0
> $ git grep skeleton -- arch/arm/boot/dts/ | wc --lines
> 117

I deliberately didn't go that route, since people would blindly fix up
the warning and mess it up (e.g. forgetting to add the required nodes).

While it should be avoided for new dts, it's largely not problematic for
old dts, and we can fix those up at our leisure.

> > Please add the relevant nodes and
> > properties explciitly to your dts{i,} files.
> 
> Great, will do.
> 
> But this series has grown too large to resend just for such nits. Is
> there a chance we can get initial patches queued on some arm-soc
> for-next branch soonish? It seems -rc1 pulls were merged 5 days ago?

There seem to be other comments on this series, and I was under the
impression that arm-soc generally pulled branches.

Even if you're not going to repost, surely you can fold in the fixup
before sending the pull request?

Thanks,
Mark.

Re: [PATCH v3 11/25] ARM: dts: Add Actions Semi S500 and LeMaker Guitar

[Mark Rutland]

On Tue, Feb 28, 2017 at 04:13:00PM +0100, Andreas Färber wrote:
> Am 28.02.2017 um 13:32 schrieb Mark Rutland:
> > On Tue, Feb 28, 2017 at 07:35:21AM +0100, Andreas Färber wrote:

> >> +#include "skeleton.dtsi"
> > 
> > Please don't include skeleton.dtsi.
> 
> Wasn't aware that was permissible. I now see it has been updated with a
> textual comment, which predates my past uses of skeleton.dtsi - for
> other pending work such as FM4 and XMC4500 I only noticed it was moved
> out of armv7m.dtsi so that dtc started spewing scary messages. ;)
> 
> A #warning might be a more effective way to make people aware of its
> deprecation - but sadly we have users that predate #include:
> 
> $ git grep /include/ -- arch/arm/boot/dts/ | grep skeleton.dtsi | wc --lines
> 32
> $ git grep /include/ -- arch/arm/boot/dts/ | grep skeleton64.dtsi | wc
> --lines
> 0
> $ git grep skeleton -- arch/arm/boot/dts/ | wc --lines
> 117

I deliberately didn't go that route, since people would blindly fix up
the warning and mess it up (e.g. forgetting to add the required nodes).

While it should be avoided for new dts, it's largely not problematic for
old dts, and we can fix those up at our leisure.

> > Please add the relevant nodes and
> > properties explciitly to your dts{i,} files.
> 
> Great, will do.
> 
> But this series has grown too large to resend just for such nits. Is
> there a chance we can get initial patches queued on some arm-soc
> for-next branch soonish? It seems -rc1 pulls were merged 5 days ago?

There seem to be other comments on this series, and I was under the
impression that arm-soc generally pulled branches.

Even if you're not going to repost, surely you can fold in the fixup
before sending the pull request?

Thanks,
Mark.

Re: [PATCH v3 11/25] ARM: dts: Add Actions Semi S500 and LeMaker Guitar

[Andreas Färber]

Hi Mark,

Am 28.02.2017 um 13:32 schrieb Mark Rutland:
> On Tue, Feb 28, 2017 at 07:35:21AM +0100, Andreas Färber wrote:
>> +/dts-v1/;
>> +
>> +#include "s500-guitar.dtsi"
>> +
>> +/ {
>> +compatible = "lemaker,guitar-bb-rev-b", "lemaker,guitar", 
>> "actions,s500";
>> +model = "LeMaker Guitar Base Board rev. B";
>> +
>> +aliases {
>> +serial3 = 
>> +};
>> +
>> +chosen {
>> +stdout-path = "serial3:115200n8";
>> +};
>> +};
>> +
>> + {
>> +status = "okay";
>> +};
> 
>> +#include "s500.dtsi"
>> +
>> +/ {
>> +compatible = "lemaker,guitar", "actions,s500";
>> +
>> +/delete-node/ memory;
> 
> Eww. Is this just bodging around skeleton.dtsi?

I guess so: Working around its hardcoded name without unit address, not
finding a /rename-node/ to that effect.

>> +
>> +memory@0 {
>> +device_type = "memory";
>> +reg = <0x0 0x4000>;
>> +};
>> +};
> 
>> +#include "skeleton.dtsi"
> 
> Please don't include skeleton.dtsi.

Wasn't aware that was permissible. I now see it has been updated with a
textual comment, which predates my past uses of skeleton.dtsi - for
other pending work such as FM4 and XMC4500 I only noticed it was moved
out of armv7m.dtsi so that dtc started spewing scary messages. ;)

A #warning might be a more effective way to make people aware of its
deprecation - but sadly we have users that predate #include:

$ git grep /include/ -- arch/arm/boot/dts/ | grep skeleton.dtsi | wc --lines
32
$ git grep /include/ -- arch/arm/boot/dts/ | grep skeleton64.dtsi | wc
--lines
0
$ git grep skeleton -- arch/arm/boot/dts/ | wc --lines
117

Might updating those 32 users be a suitable Outreachy project (CC'ed),
to then get the remaining 85 occurrences to be eliminated by their
maintainers, so that it doesn't get copied for new SoCs any more? :)

> Please add the relevant nodes and
> properties explciitly to your dts{i,} files.

Great, will do.

But this series has grown too large to resend just for such nits. Is
there a chance we can get initial patches queued on some arm-soc
for-next branch soonish? It seems -rc1 pulls were merged 5 days ago?

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v3 11/25] ARM: dts: Add Actions Semi S500 and LeMaker Guitar

[Andreas Färber]

Hi Mark,

Am 28.02.2017 um 13:32 schrieb Mark Rutland:
> On Tue, Feb 28, 2017 at 07:35:21AM +0100, Andreas Färber wrote:
>> +/dts-v1/;
>> +
>> +#include "s500-guitar.dtsi"
>> +
>> +/ {
>> +compatible = "lemaker,guitar-bb-rev-b", "lemaker,guitar", 
>> "actions,s500";
>> +model = "LeMaker Guitar Base Board rev. B";
>> +
>> +aliases {
>> +serial3 = 
>> +};
>> +
>> +chosen {
>> +stdout-path = "serial3:115200n8";
>> +};
>> +};
>> +
>> + {
>> +status = "okay";
>> +};
> 
>> +#include "s500.dtsi"
>> +
>> +/ {
>> +compatible = "lemaker,guitar", "actions,s500";
>> +
>> +/delete-node/ memory;
> 
> Eww. Is this just bodging around skeleton.dtsi?

I guess so: Working around its hardcoded name without unit address, not
finding a /rename-node/ to that effect.

>> +
>> +memory@0 {
>> +device_type = "memory";
>> +reg = <0x0 0x4000>;
>> +};
>> +};
> 
>> +#include "skeleton.dtsi"
> 
> Please don't include skeleton.dtsi.

Wasn't aware that was permissible. I now see it has been updated with a
textual comment, which predates my past uses of skeleton.dtsi - for
other pending work such as FM4 and XMC4500 I only noticed it was moved
out of armv7m.dtsi so that dtc started spewing scary messages. ;)

A #warning might be a more effective way to make people aware of its
deprecation - but sadly we have users that predate #include:

$ git grep /include/ -- arch/arm/boot/dts/ | grep skeleton.dtsi | wc --lines
32
$ git grep /include/ -- arch/arm/boot/dts/ | grep skeleton64.dtsi | wc
--lines
0
$ git grep skeleton -- arch/arm/boot/dts/ | wc --lines
117

Might updating those 32 users be a suitable Outreachy project (CC'ed),
to then get the remaining 85 occurrences to be eliminated by their
maintainers, so that it doesn't get copied for new SoCs any more? :)

> Please add the relevant nodes and
> properties explciitly to your dts{i,} files.

Great, will do.

But this series has grown too large to resend just for such nits. Is
there a chance we can get initial patches queued on some arm-soc
for-next branch soonish? It seems -rc1 pulls were merged 5 days ago?

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v3 11/25] ARM: dts: Add Actions Semi S500 and LeMaker Guitar

[Mark Rutland]

Hi,

On Tue, Feb 28, 2017 at 07:35:21AM +0100, Andreas Färber wrote:
> +/dts-v1/;
> +
> +#include "s500-guitar.dtsi"
> +
> +/ {
> + compatible = "lemaker,guitar-bb-rev-b", "lemaker,guitar", 
> "actions,s500";
> + model = "LeMaker Guitar Base Board rev. B";
> +
> + aliases {
> + serial3 = 
> + };
> +
> + chosen {
> + stdout-path = "serial3:115200n8";
> + };
> +};
> +
> + {
> + status = "okay";
> +};

> +#include "s500.dtsi"
> +
> +/ {
> + compatible = "lemaker,guitar", "actions,s500";
> +
> + /delete-node/ memory;

Eww. Is this just bodging around skeleton.dtsi?

> +
> + memory@0 {
> + device_type = "memory";
> + reg = <0x0 0x4000>;
> + };
> +};

> +#include "skeleton.dtsi"

Please don't include skeleton.dtsi. Please add the relevant nodes and
properties explciitly to your dts{i,} files.

> +
> +#include 
> +
> +/ {

Please define your #address-cells and #size-cells here.

If you want, add an empty /chosen node (not strictly necessary since
your dts file has a non-empty one anyhow).

Thanks,
Mark.

Re: [PATCH v3 11/25] ARM: dts: Add Actions Semi S500 and LeMaker Guitar

[Mark Rutland]

Hi,

On Tue, Feb 28, 2017 at 07:35:21AM +0100, Andreas Färber wrote:
> +/dts-v1/;
> +
> +#include "s500-guitar.dtsi"
> +
> +/ {
> + compatible = "lemaker,guitar-bb-rev-b", "lemaker,guitar", 
> "actions,s500";
> + model = "LeMaker Guitar Base Board rev. B";
> +
> + aliases {
> + serial3 = 
> + };
> +
> + chosen {
> + stdout-path = "serial3:115200n8";
> + };
> +};
> +
> + {
> + status = "okay";
> +};

> +#include "s500.dtsi"
> +
> +/ {
> + compatible = "lemaker,guitar", "actions,s500";
> +
> + /delete-node/ memory;

Eww. Is this just bodging around skeleton.dtsi?

> +
> + memory@0 {
> + device_type = "memory";
> + reg = <0x0 0x4000>;
> + };
> +};

> +#include "skeleton.dtsi"

Please don't include skeleton.dtsi. Please add the relevant nodes and
properties explciitly to your dts{i,} files.

> +
> +#include 
> +
> +/ {

Please define your #address-cells and #size-cells here.

If you want, add an empty /chosen node (not strictly necessary since
your dts file has a non-empty one anyhow).

Thanks,
Mark.

[PATCH v3 11/25] ARM: dts: Add Actions Semi S500 and LeMaker Guitar

[Andreas Färber]

Signed-off-by: Andreas Färber 
---
 v2 -> v3:
 * Fixed uart2 reg offset
 * Enlarged timer reg size
 * Added 2 Hz timer interrupts, interrupt-names
 * Disabled CPUs 1-3 for now (cf. later patches)
 * Added Cortex-A9 SCU, global timer, TWD timer/wdt nodes
 * Added HOSC clock for timer
 
 v1 -> v2:
 * Reworded subject
 * Updated ARCH_OWL to ARCH_ACTIONS (Arnd)
 * Adopted "actions" vendor prefix
 * Dropped irq.h include
 * Added memory@0 node for Guitar SoM
 * Dropped bogus uart3 clock-frequency
 * Added device_type for CPU nodes
 * Fixed UART reg size
 * Adopted SPDX-License-Identifier (Rob)
 * Added remaining UART nodes
 * Added timer node
 
 arch/arm/boot/dts/Makefile |   2 +
 arch/arm/boot/dts/s500-guitar-bb-rev-b.dts |  26 +
 arch/arm/boot/dts/s500-guitar.dtsi |  24 
 arch/arm/boot/dts/s500.dtsi| 170 +
 4 files changed, 222 insertions(+)
 create mode 100644 arch/arm/boot/dts/s500-guitar-bb-rev-b.dts
 create mode 100644 arch/arm/boot/dts/s500-guitar.dtsi
 create mode 100644 arch/arm/boot/dts/s500.dtsi

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 0118084..578a20b 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -638,6 +638,8 @@ dtb-$(CONFIG_ARCH_ORION5X) += \
orion5x-maxtor-shared-storage-2.dtb \
orion5x-netgear-wnr854t.dtb \
orion5x-rd88f5182-nas.dtb
+dtb-$(CONFIG_ARCH_ACTIONS) += \
+   s500-guitar-bb-rev-b.dtb
 dtb-$(CONFIG_ARCH_PRIMA2) += \
prima2-evb.dtb
 dtb-$(CONFIG_ARCH_OXNAS) += \
diff --git a/arch/arm/boot/dts/s500-guitar-bb-rev-b.dts 
b/arch/arm/boot/dts/s500-guitar-bb-rev-b.dts
new file mode 100644
index 000..834b71d
--- /dev/null
+++ b/arch/arm/boot/dts/s500-guitar-bb-rev-b.dts
@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2016-2017 Andreas Färber
+ *
+ * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+ */
+
+/dts-v1/;
+
+#include "s500-guitar.dtsi"
+
+/ {
+   compatible = "lemaker,guitar-bb-rev-b", "lemaker,guitar", 
"actions,s500";
+   model = "LeMaker Guitar Base Board rev. B";
+
+   aliases {
+   serial3 = 
+   };
+
+   chosen {
+   stdout-path = "serial3:115200n8";
+   };
+};
+
+ {
+   status = "okay";
+};
diff --git a/arch/arm/boot/dts/s500-guitar.dtsi 
b/arch/arm/boot/dts/s500-guitar.dtsi
new file mode 100644
index 000..31c5def
--- /dev/null
+++ b/arch/arm/boot/dts/s500-guitar.dtsi
@@ -0,0 +1,24 @@
+/*
+ * LeMaker Guitar SoM
+ *
+ * Copyright (c) 2016-2017 Andreas Färber
+ *
+ * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+ */
+
+#include "s500.dtsi"
+
+/ {
+   compatible = "lemaker,guitar", "actions,s500";
+
+   /delete-node/ memory;
+
+   memory@0 {
+   device_type = "memory";
+   reg = <0x0 0x4000>;
+   };
+};
+
+ {
+   clocks = <>;
+};
diff --git a/arch/arm/boot/dts/s500.dtsi b/arch/arm/boot/dts/s500.dtsi
new file mode 100644
index 000..1973181
--- /dev/null
+++ b/arch/arm/boot/dts/s500.dtsi
@@ -0,0 +1,170 @@
+/*
+ * Actions Semi S500 SoC
+ *
+ * Copyright (c) 2016-2017 Andreas Färber
+ *
+ * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+ */
+
+#include "skeleton.dtsi"
+
+#include 
+
+/ {
+   compatible = "actions,s500";
+   interrupt-parent = <>;
+
+   cpus {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   cpu0: cpu@0 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a9";
+   reg = <0x0>;
+   };
+
+   cpu1: cpu@1 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a9";
+   reg = <0x1>;
+   };
+
+   cpu2: cpu@2 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a9";
+   reg = <0x2>;
+   };
+
+   cpu3: cpu@3 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a9";
+   reg = <0x3>;
+   };
+   };
+
+   arm-pmu {
+   compatible = "arm,cortex-a9-pmu";
+   interrupts = ,
+,
+,
+;
+   interrupt-affinity = <>, <>, <>, <>;
+   };
+
+   hosc: hosc {
+   compatible = "fixed-clock";
+   clock-frequency = <2400>;
+   #clock-cells = <0>;
+   };
+
+   soc {
+   compatible = "simple-bus";
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+
+   scu: scu@b002 {
+   compatible = "arm,cortex-a9-scu";
+   reg = <0xb002 0x100>;
+   };
+
+   global_timer: timer@b0020200 {
+   compatible = 

[PATCH v3 11/25] ARM: dts: Add Actions Semi S500 and LeMaker Guitar

[Andreas Färber]

Signed-off-by: Andreas Färber 
---
 v2 -> v3:
 * Fixed uart2 reg offset
 * Enlarged timer reg size
 * Added 2 Hz timer interrupts, interrupt-names
 * Disabled CPUs 1-3 for now (cf. later patches)
 * Added Cortex-A9 SCU, global timer, TWD timer/wdt nodes
 * Added HOSC clock for timer
 
 v1 -> v2:
 * Reworded subject
 * Updated ARCH_OWL to ARCH_ACTIONS (Arnd)
 * Adopted "actions" vendor prefix
 * Dropped irq.h include
 * Added memory@0 node for Guitar SoM
 * Dropped bogus uart3 clock-frequency
 * Added device_type for CPU nodes
 * Fixed UART reg size
 * Adopted SPDX-License-Identifier (Rob)
 * Added remaining UART nodes
 * Added timer node
 
 arch/arm/boot/dts/Makefile |   2 +
 arch/arm/boot/dts/s500-guitar-bb-rev-b.dts |  26 +
 arch/arm/boot/dts/s500-guitar.dtsi |  24 
 arch/arm/boot/dts/s500.dtsi| 170 +
 4 files changed, 222 insertions(+)
 create mode 100644 arch/arm/boot/dts/s500-guitar-bb-rev-b.dts
 create mode 100644 arch/arm/boot/dts/s500-guitar.dtsi
 create mode 100644 arch/arm/boot/dts/s500.dtsi

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 0118084..578a20b 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -638,6 +638,8 @@ dtb-$(CONFIG_ARCH_ORION5X) += \
orion5x-maxtor-shared-storage-2.dtb \
orion5x-netgear-wnr854t.dtb \
orion5x-rd88f5182-nas.dtb
+dtb-$(CONFIG_ARCH_ACTIONS) += \
+   s500-guitar-bb-rev-b.dtb
 dtb-$(CONFIG_ARCH_PRIMA2) += \
prima2-evb.dtb
 dtb-$(CONFIG_ARCH_OXNAS) += \
diff --git a/arch/arm/boot/dts/s500-guitar-bb-rev-b.dts 
b/arch/arm/boot/dts/s500-guitar-bb-rev-b.dts
new file mode 100644
index 000..834b71d
--- /dev/null
+++ b/arch/arm/boot/dts/s500-guitar-bb-rev-b.dts
@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2016-2017 Andreas Färber
+ *
+ * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+ */
+
+/dts-v1/;
+
+#include "s500-guitar.dtsi"
+
+/ {
+   compatible = "lemaker,guitar-bb-rev-b", "lemaker,guitar", 
"actions,s500";
+   model = "LeMaker Guitar Base Board rev. B";
+
+   aliases {
+   serial3 = 
+   };
+
+   chosen {
+   stdout-path = "serial3:115200n8";
+   };
+};
+
+ {
+   status = "okay";
+};
diff --git a/arch/arm/boot/dts/s500-guitar.dtsi 
b/arch/arm/boot/dts/s500-guitar.dtsi
new file mode 100644
index 000..31c5def
--- /dev/null
+++ b/arch/arm/boot/dts/s500-guitar.dtsi
@@ -0,0 +1,24 @@
+/*
+ * LeMaker Guitar SoM
+ *
+ * Copyright (c) 2016-2017 Andreas Färber
+ *
+ * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+ */
+
+#include "s500.dtsi"
+
+/ {
+   compatible = "lemaker,guitar", "actions,s500";
+
+   /delete-node/ memory;
+
+   memory@0 {
+   device_type = "memory";
+   reg = <0x0 0x4000>;
+   };
+};
+
+ {
+   clocks = <>;
+};
diff --git a/arch/arm/boot/dts/s500.dtsi b/arch/arm/boot/dts/s500.dtsi
new file mode 100644
index 000..1973181
--- /dev/null
+++ b/arch/arm/boot/dts/s500.dtsi
@@ -0,0 +1,170 @@
+/*
+ * Actions Semi S500 SoC
+ *
+ * Copyright (c) 2016-2017 Andreas Färber
+ *
+ * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+ */
+
+#include "skeleton.dtsi"
+
+#include 
+
+/ {
+   compatible = "actions,s500";
+   interrupt-parent = <>;
+
+   cpus {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   cpu0: cpu@0 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a9";
+   reg = <0x0>;
+   };
+
+   cpu1: cpu@1 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a9";
+   reg = <0x1>;
+   };
+
+   cpu2: cpu@2 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a9";
+   reg = <0x2>;
+   };
+
+   cpu3: cpu@3 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a9";
+   reg = <0x3>;
+   };
+   };
+
+   arm-pmu {
+   compatible = "arm,cortex-a9-pmu";
+   interrupts = ,
+,
+,
+;
+   interrupt-affinity = <>, <>, <>, <>;
+   };
+
+   hosc: hosc {
+   compatible = "fixed-clock";
+   clock-frequency = <2400>;
+   #clock-cells = <0>;
+   };
+
+   soc {
+   compatible = "simple-bus";
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+
+   scu: scu@b002 {
+   compatible = "arm,cortex-a9-scu";
+   reg = <0xb002 0x100>;
+   };
+
+   global_timer: timer@b0020200 {
+   compatible =