2016-11-23 00:10:07

by Tomas Hlavacek

[permalink] [raw]
Subject: [RFC PATCH] ARM: dts: Add support for Turris Omnia

Turris Omnia board by CZ.NIC:

* Marvell Armada 385 SoC
* 1 or 2 GB DDR3
* eMMC
* 8 MB SPI flash (U-Boot and rescue Linux image)
* 88E1514 PHY
* 88E6176 Ethernet switch (not supported)

Supported board revision: CZ11NIC13 (production board).

Signed-off-by: Tomas Hlavacek <[email protected]>
---
Changes since Uwe's version:

- add MBUS regions (needed for Marvell CESA)
- remove rtc disable (WFM with CZ11NIC13 = production board)
- cleanup comments

Unsupported peripherals:
- MV88E7176 switch
- SFP
- LEDs

---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/armada-385-turris-omnia.dts | 279 ++++++++++++++++++++++++++
2 files changed, 280 insertions(+)
create mode 100644 arch/arm/boot/dts/armada-385-turris-omnia.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index befcd26..f1d3b9ff 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -920,6 +920,7 @@ dtb-$(CONFIG_MACH_ARMADA_38X) += \
armada-385-db-ap.dtb \
armada-385-linksys-caiman.dtb \
armada-385-linksys-cobra.dtb \
+ armada-385-turris-omnia.dtb \
armada-388-clearfog.dtb \
armada-388-db.dtb \
armada-388-gp.dtb \
diff --git a/arch/arm/boot/dts/armada-385-turris-omnia.dts b/arch/arm/boot/dts/armada-385-turris-omnia.dts
new file mode 100644
index 0000000..5ef3d62
--- /dev/null
+++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
@@ -0,0 +1,279 @@
+/*
+ * Device Tree file for the Turris Omnia
+ * Schematic available at https://www.turris.cz/doc/_media/rtrom01-schema.pdf
+ *
+ * Copyright (C) 2016 Uwe Kleine-König <[email protected]>
+ * Copyright (C) 2016 Tomas Hlavacek <[email protected]>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without
+ * any warranty of any kind, whether express or implied.
+ *
+ * Or, alternatively,
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include "armada-385.dtsi"
+
+/ {
+ model = "Turris Omnia";
+ compatible = "cznic,turris-omnia", "marvell,armada385", \
+ "marvell,armada380";
+
+ chosen {
+ stdout-path = &uart0;
+ };
+
+ memory {
+ device_type = "memory";
+ reg = <0x00000000 0x40000000>; /* 1024 MB */
+ };
+
+ soc {
+ ranges = <MBUS_ID(0xf0, 0x01) 0 0xf1000000 0x100000
+ MBUS_ID(0x01, 0x1d) 0 0xfff00000 0x100000
+ MBUS_ID(0x09, 0x19) 0 0xf1100000 0x10000
+ MBUS_ID(0x09, 0x15) 0 0xf1110000 0x10000>;
+
+ internal-regs {
+
+ /* USB part of the PCIe2/USB 2.0 port */
+ usb@58000 {
+ status = "okay";
+ };
+
+ sata@a8000 {
+ status = "okay";
+ };
+
+ sdhci@d8000 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&sdhci_pins>;
+ status = "okay";
+
+ bus-width = <8>;
+ no-1-8-v;
+ non-removable;
+ };
+
+ usb3@f0000 {
+ status = "okay";
+ };
+
+ usb3@f8000 {
+ status = "okay";
+ };
+ };
+
+ pcie-controller {
+ status = "okay";
+
+ pcie@1,0 {
+ /* Port 0, Lane 0 */
+ status = "okay";
+ };
+
+ pcie@2,0 {
+ /* Port 1, Lane 0 */
+ status = "okay";
+ };
+
+ pcie@3,0 {
+ /* Port 2, Lane 0 */
+ status = "okay";
+ };
+ };
+ };
+};
+
+/* Connected to 88E6176 switch, port 6 */
+&eth0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&ge0_rgmii_pins>;
+ status = "okay";
+ phy-mode = "rgmii-id";
+
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+};
+
+/* Connected to 88E6176 switch, port 5 */
+&eth1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&ge1_rgmii_pins>;
+ status = "okay";
+ phy-mode = "rgmii-id";
+
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+};
+
+/* WAN port */
+&eth2 {
+ status = "okay";
+ phy-mode = "sgmii";
+ phy = <&phy1>;
+};
+
+&i2c0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c0_pins>;
+ status = "okay";
+
+ i2cmux@70 {
+ compatible = "nxp,pca9547";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x70>;
+ status = "okay";
+
+ i2c@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ status = "okay";
+
+ /* STM32F0 command interface at address 0x2a.
+ * STM32F0 LED interface at address 0x2b.
+ */
+
+ eeprom@54 {
+ compatible = "at,24c64";
+ reg = <0x54>;
+
+ /* The EEPROM contains data for bootloader.
+ * Contents:
+ * struct omnia_eeprom {
+ * u32 magic; (=0x0341a034)
+ * u32 ramsize;
+ * char region[4] (=0x0);
+ * u32 crc32;
+ * };
+ */
+ };
+ };
+
+ /* Channel 1: Routed to PCIe0/mSATA connector (CN7A).
+ * Channel 2: Routed to PCIe1/USB2 connector (CN61A).
+ * Channel 3: Routed to PCIe2 connector (CN62A).
+ * Channel 4: Routed to SFP+.
+ * Channel 5: ATSHA204A at address 0x64.
+ * Channel 6: Routed to user pin header CN11.
+ */
+
+ i2c@7 {
+ /* GPIO expander for SFP+ signals */
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <7>;
+
+ wangpio: gpio@71 {
+ compatible = "nxp,pca9538";
+ reg = <0x71>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <14 IRQ_TYPE_LEVEL_LOW>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+ };
+ };
+};
+
+&mdio {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mdio_pins>;
+ status = "okay";
+
+ phy1: phy@1 {
+ status = "okay";
+ compatible = "ethernet-phy-id0141.0DD1", \
+ "ethernet-phy-ieee802.3-c22";
+ reg = <1>;
+ /* IRQ is connected to PCA9538 pin 7. Currently it
+ * can not be utilized.
+ */
+ };
+
+ /* Switch MV88E7176 at address 0x10. */
+};
+
+&pinctrl {
+ spi0cs1_pins: spi0-pins-0cs1 {
+ marvell,pins = "mpp26";
+ marvell,function = "spi0";
+ };
+};
+
+&spi0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&spi0_pins &spi0cs1_pins>;
+ status = "okay";
+
+ spi-nor@0 {
+ compatible = "spansion,s25fl164k", "jedec,spi-nor";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <0>;
+ spi-max-frequency = <40000000>;
+
+ partition@0 {
+ reg = <0x0 0x00100000>;
+ label = "U-Boot";
+ };
+
+ partition@1 {
+ reg = <0x00100000 0x00700000>;
+ label = "Rescue system";
+ };
+ };
+
+ /* SPI0 + CS1 (MPP26) is routed to a pin header CN11. */
+};
+
+&uart0 {
+ /* Pin header CN10. */
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart0_pins>;
+ status = "okay";
+};
+
+&uart1 {
+ /* Pin header CN11. */
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart1_pins>;
+ status = "okay";
+};
+
--
2.7.4


2016-11-23 00:35:48

by Andrew Lunn

[permalink] [raw]
Subject: Re: [RFC PATCH] ARM: dts: Add support for Turris Omnia

> +++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
> @@ -0,0 +1,279 @@
> +/*
> + * Device Tree file for the Turris Omnia
> + * Schematic available at https://www.turris.cz/doc/_media/rtrom01-schema.pdf

Hi Tomas

Cool that there is a link to the schematics. But please could you put
it lower down. It is more likely to be seen if it comes after the
copyright and license section.

> + sdhci@d8000 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&sdhci_pins>;
> + status = "okay";
> +
> + bus-width = <8>;
> + no-1-8-v;
> + non-removable;
> + };

> +&i2c0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c0_pins>;
> + status = "okay";
> +
> + i2cmux@70 {
> + compatible = "nxp,pca9547";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x70>;
> + status = "okay";
> +
> + /* Channel 1: Routed to PCIe0/mSATA connector (CN7A).
> + * Channel 2: Routed to PCIe1/USB2 connector (CN61A).
> + * Channel 3: Routed to PCIe2 connector (CN62A).
> + * Channel 4: Routed to SFP+.
> + * Channel 5: ATSHA204A at address 0x64.
> + * Channel 6: Routed to user pin header CN11.
> + */

I've not looked at how the pca9547 works.... Will it instantiate a bus
only if there is a node in the device tree with a reg property?

What i'm thinking is that it is possible to indicate to the i2c core
that a device is on a bus using echo to a file. But this only works if
the bus exists. You could for example say using echo that there is an
at24 EEPROM on channel 4 and get access to the EEPROM inside the SFP
module. But that only works if the i2c bus exists. Does it?

No leds? No buttons via gpio-keys?

Andrew

2016-11-23 08:20:09

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [RFC PATCH] ARM: dts: Add support for Turris Omnia

Hello Tomas,

calling it v4 would be nice.

On Wed, Nov 23, 2016 at 01:09:20AM +0100, Tomas Hlavacek wrote:
> Turris Omnia board by CZ.NIC:
>
> * Marvell Armada 385 SoC
> * 1 or 2 GB DDR3
> * eMMC
> * 8 MB SPI flash (U-Boot and rescue Linux image)
> * 88E1514 PHY
> * 88E6176 Ethernet switch (not supported)
>
> Supported board revision: CZ11NIC13 (production board).
>
> Signed-off-by: Tomas Hlavacek <[email protected]>

As you picked my v3, you should keep my S-o-b.

> ---
> Changes since Uwe's version:
>
> - add MBUS regions (needed for Marvell CESA)
> - remove rtc disable (WFM with CZ11NIC13 = production board)

If I do

mw 0xf10184a0 0xfd4d4cfa

in the boot loader, it seems to work for me, too. You don't need that?

> - cleanup comments
>
> Unsupported peripherals:
> - MV88E7176 switch
> - SFP
> - LEDs

LEDs is not that bad IMHO, because they work. You just cannot change
their function, but they blink according to their default trigger.

> ---
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/armada-385-turris-omnia.dts | 279 ++++++++++++++++++++++++++
> 2 files changed, 280 insertions(+)
> create mode 100644 arch/arm/boot/dts/armada-385-turris-omnia.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index befcd26..f1d3b9ff 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -920,6 +920,7 @@ dtb-$(CONFIG_MACH_ARMADA_38X) += \
> armada-385-db-ap.dtb \
> armada-385-linksys-caiman.dtb \
> armada-385-linksys-cobra.dtb \
> + armada-385-turris-omnia.dtb \
> armada-388-clearfog.dtb \
> armada-388-db.dtb \
> armada-388-gp.dtb \
> diff --git a/arch/arm/boot/dts/armada-385-turris-omnia.dts b/arch/arm/boot/dts/armada-385-turris-omnia.dts
> new file mode 100644
> index 0000000..5ef3d62
> --- /dev/null
> +++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
> @@ -0,0 +1,279 @@
> +/*
> + * Device Tree file for the Turris Omnia
> + * Schematic available at https://www.turris.cz/doc/_media/rtrom01-schema.pdf
> + *
> + * Copyright (C) 2016 Uwe Kleine-K?nig <[email protected]>
> + * Copyright (C) 2016 Tomas Hlavacek <[email protected]>
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + * a) This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without
> + * any warranty of any kind, whether express or implied.
> + *
> + * Or, alternatively,
> + *
> + * b) Permission is hereby granted, free of charge, to any person
> + * obtaining a copy of this software and associated documentation
> + * files (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use,
> + * copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following
> + * conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include "armada-385.dtsi"
> +
> +/ {
> + model = "Turris Omnia";
> + compatible = "cznic,turris-omnia", "marvell,armada385", \
> + "marvell,armada380";

You don't need a \ here AFAIK.

> +
> + chosen {
> + stdout-path = &uart0;
> + };
> +
> + memory {
> + device_type = "memory";
> + reg = <0x00000000 0x40000000>; /* 1024 MB */
> + };
> +
> + soc {
> + ranges = <MBUS_ID(0xf0, 0x01) 0 0xf1000000 0x100000
> + MBUS_ID(0x01, 0x1d) 0 0xfff00000 0x100000
> + MBUS_ID(0x09, 0x19) 0 0xf1100000 0x10000
> + MBUS_ID(0x09, 0x15) 0 0xf1110000 0x10000>;
> +
> + internal-regs {
> +
> + /* USB part of the PCIe2/USB 2.0 port */
> + usb@58000 {
> + status = "okay";
> + };
> +
> + sata@a8000 {
> + status = "okay";
> + };
> +
> + sdhci@d8000 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&sdhci_pins>;
> + status = "okay";
> +
> + bus-width = <8>;
> + no-1-8-v;
> + non-removable;
> + };
> +
> + usb3@f0000 {
> + status = "okay";
> + };
> +
> + usb3@f8000 {
> + status = "okay";
> + };
> + };
> +
> + pcie-controller {
> + status = "okay";
> +
> + pcie@1,0 {
> + /* Port 0, Lane 0 */
> + status = "okay";
> + };
> +
> + pcie@2,0 {
> + /* Port 1, Lane 0 */
> + status = "okay";
> + };
> +
> + pcie@3,0 {
> + /* Port 2, Lane 0 */
> + status = "okay";
> + };
> + };
> + };
> +};
> +
> +/* Connected to 88E6176 switch, port 6 */
> +&eth0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&ge0_rgmii_pins>;
> + status = "okay";
> + phy-mode = "rgmii-id";
> +
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> +};
> +
> +/* Connected to 88E6176 switch, port 5 */
> +&eth1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&ge1_rgmii_pins>;
> + status = "okay";
> + phy-mode = "rgmii-id";
> +
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> +};
> +
> +/* WAN port */
> +&eth2 {
> + status = "okay";
> + phy-mode = "sgmii";
> + phy = <&phy1>;
> +};
> +
> +&i2c0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c0_pins>;
> + status = "okay";
> +
> + i2cmux@70 {
> + compatible = "nxp,pca9547";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x70>;
> + status = "okay";
> +
> + i2c@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> + status = "okay";
> +
> + /* STM32F0 command interface at address 0x2a.
> + * STM32F0 LED interface at address 0x2b.
> + */

Should this better be:

/*
* STM32F0 command interface at address 0x2a.
* STM32F0 LED interface at address 0x2b.
*/

As is recommended for comments in .c?

> +
> + eeprom@54 {
> + compatible = "at,24c64";
> + reg = <0x54>;
> +
> + /* The EEPROM contains data for bootloader.
> + * Contents:
> + * struct omnia_eeprom {
> + * u32 magic; (=0x0341a034)
> + * u32 ramsize;
ramsize in GiB?

> + * char region[4] (=0x0);
This is for the WLAN regdomain, right?

> + * u32 crc32;
> + * };
> + */
ditto for the comment format.

> + };
> + };
> +
> + /* Channel 1: Routed to PCIe0/mSATA connector (CN7A).
> + * Channel 2: Routed to PCIe1/USB2 connector (CN61A).
> + * Channel 3: Routed to PCIe2 connector (CN62A).
> + * Channel 4: Routed to SFP+.
> + * Channel 5: ATSHA204A at address 0x64.
> + * Channel 6: Routed to user pin header CN11.
> + */
I'd like to keep the busses as Andrew already pointed out. For example
this might make it possible to use i2c-tools to read out the mac address
from the ATSHA.

> + i2c@7 {
> + /* GPIO expander for SFP+ signals */
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <7>;
> +
> + wangpio: gpio@71 {
> + compatible = "nxp,pca9538";
> + reg = <0x71>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <14 IRQ_TYPE_LEVEL_LOW>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> + };
> + };
> +};
> +
> +&mdio {
> + pinctrl-names = "default";
> + pinctrl-0 = <&mdio_pins>;
> + status = "okay";
> +
> + phy1: phy@1 {
> + status = "okay";
> + compatible = "ethernet-phy-id0141.0DD1", \
> + "ethernet-phy-ieee802.3-c22";

Drop the \

> + reg = <1>;
> + /* IRQ is connected to PCA9538 pin 7. Currently it
> + * can not be utilized.
> + */
> + };
> +
> + /* Switch MV88E7176 at address 0x10. */
> +};
> +
> +&pinctrl {
> + spi0cs1_pins: spi0-pins-0cs1 {
> + marvell,pins = "mpp26";
> + marvell,function = "spi0";
> + };

Why did you drop the pcawan pinctrl?

> +};
> +
> +&spi0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&spi0_pins &spi0cs1_pins>;

Oh, this is wrong (already in my patch): this is cs0 not cs1.

> + status = "okay";
> +
> + spi-nor@0 {
> + compatible = "spansion,s25fl164k", "jedec,spi-nor";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + reg = <0>;
> + spi-max-frequency = <40000000>;
> +
> + partition@0 {
> + reg = <0x0 0x00100000>;
> + label = "U-Boot";
> + };
> +
> + partition@1 {
> + reg = <0x00100000 0x00700000>;
> + label = "Rescue system";
> + };
> + };
> +
> + /* SPI0 + CS1 (MPP26) is routed to a pin header CN11. */

Looks strange. What about

/* MISO, MOSI, SCLK and CS1 are routed to pin header CN11 */

Maybe also add the node for this pin to &pinctrl, but don't use it in
&spi0.pinctrl-0? This would nicely document the MPP26 part.

> +};
> +
> +&uart0 {
> + /* Pin header CN10. */
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart0_pins>;
> + status = "okay";
> +};
> +
> +&uart1 {
> + /* Pin header CN11. */
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart1_pins>;
> + status = "okay";
> +};
> +

Trailing new line

Best regards
Uwe


Attachments:
(No filename) (9.37 kB)
signature.asc (488.00 B)
Download all attachments

2016-11-24 08:37:29

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [RFC PATCH] ARM: dts: Add support for Turris Omnia

On 11/23/2016 01:35 AM, Andrew Lunn wrote:
>> +++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
>> @@ -0,0 +1,279 @@
>> +/*
>> + * Device Tree file for the Turris Omnia
>> + * Schematic available at https://www.turris.cz/doc/_media/rtrom01-schema.pdf
>
> Cool that there is a link to the schematics. But please could you put
> it lower down. It is more likely to be seen if it comes after the
> copyright and license section.

I added to the top because that's where I would look. But checking other
dts files it seems indeed to be more common after the copyright stuff.
I'd suggest to even start a new comment (i.e.

* last blabla of copyright
*/

/*
* Schematic available at ...

) to be more "loud".

@Tomas: I think it doesn't make sense when we alternate sending patches
without prior arrangement. Do you already work on a v5? If not I can do
that to fix the last few comments. Not sure when a submission is too
late to enter v4.10, but I think the window isn't that big any more.

> No leds? No buttons via gpio-keys?

The leds are controlled by a Cortex-M0 and without intervention blink
according to a hardware function (network, power, pci). IMHO that's ok
for an initial setup.

And there are no buttons that are routed to the Armada CPU. Just a reset
button (well, ok, this one is routed to the Armada CPU, but you cannot
make this a gpio-key :-) and the other button is used to control the
brightness of the LEDs and is only routed to the M0.

Best regards
Uwe


Attachments:
signature.asc (488.00 B)
OpenPGP digital signature

2016-11-24 15:07:25

by Andrew Lunn

[permalink] [raw]
Subject: Re: [RFC PATCH] ARM: dts: Add support for Turris Omnia

> @Tomas: I think it doesn't make sense when we alternate sending patches
> without prior arrangement. Do you already work on a v5? If not I can do
> that to fix the last few comments. Not sure when a submission is too
> late to enter v4.10, but I think the window isn't that big any more.

It is getting a bit late. But maybe Linus will add in another -rc
week.

>
> > No leds? No buttons via gpio-keys?
>
> The leds are controlled by a Cortex-M0 and without intervention blink
> according to a hardware function (network, power, pci). IMHO that's ok
> for an initial setup.

Yes. That is fine. It is just unusual. Most boards have gpio-led and
gpio-keys, which are easy to add. That is why i asked. Adding an LED
driver which talks to this M0 can be added later.

Andrew

2016-11-25 13:45:32

by Tomas Hlavacek

[permalink] [raw]
Subject: Re: [RFC PATCH] ARM: dts: Add support for Turris Omnia

Hi!

On Thu, Nov 24, 2016 at 4:07 PM, Andrew Lunn <[email protected]> wrote:
>> @Tomas: I think it doesn't make sense when we alternate sending
>> patches
>> without prior arrangement. Do you already work on a v5? If not I
>> can do
>> that to fix the last few comments. Not sure when a submission is too
>> late to enter v4.10, but I think the window isn't that big any more.
>
> It is getting a bit late. But maybe Linus will add in another -rc
> week.
>
>>
>> > No leds? No buttons via gpio-keys?
>>
>> The leds are controlled by a Cortex-M0 and without intervention
>> blink
>> according to a hardware function (network, power, pci). IMHO that's
>> ok
>> for an initial setup.
>
> Yes. That is fine. It is just unusual. Most boards have gpio-led and
> gpio-keys, which are easy to add. That is why i asked. Adding an LED
> driver which talks to this M0 can be added later.

Actually the WiP driver for MCU LED interface, that we use in our
kernel is here:
https://github.com/tmshlvck/omnia-linux/commit/2121afd8fbd2e4c720edcdd472b11b5303bc0dfb

It definitely needs some cleanup and it adds non-standard features
(main PWM for all LEDs, autonomous blink mode, colors) via custom /sys
files, which I suspect that is not going to be acceptable for upstream.
Let's keep it for the next iteration.

Regarding the button, we actually have one, that is connected to the
MCU and by default it sets LED brigtness autonomously, but it should be
able to generate IRQs via the MCU if we change the mode in the MCU.
I'll look into that.

Tomas

2016-12-10 14:24:27

by Pavel Machek

[permalink] [raw]
Subject: Re: [RFC PATCH] ARM: dts: Add support for Turris Omnia

Hi!

> >Yes. That is fine. It is just unusual. Most boards have gpio-led and
> >gpio-keys, which are easy to add. That is why i asked. Adding an LED
> >driver which talks to this M0 can be added later.
>
> Actually the WiP driver for MCU LED interface, that we use in our
> kernel is here: https://github.com/tmshlvck/omnia-linux/commit/2121afd8fbd2e4c720edcdd472b11b5303bc0dfb
>
> It definitely needs some cleanup and it adds non-standard features
> (main PWM for all LEDs, autonomous blink mode, colors) via custom
> /sys files, which I suspect that is not going to be acceptable for
> upstream. Let's keep it for the next iteration.

Actually, LEDs that can do PWM intensity on their own are common and
supported.

LEDs that can do PWM pretty advanced patterns are common in the cellphones,
as is the color. Unfortunately, good support in kernel is missing. It
would be good to change that.

In n900, I have a LED that can compute prime numbers then blink them,
autonomously. We probably don't need to support _that_, but common
support for patterns would be good.

Best regards,
Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html