2018-03-15 16:28:34

by Harald Geyer

[permalink] [raw]
Subject: [PATCHv2 0/5] arm64: allwinner: Add support for TERES-I laptop

This series adds support for the TERES I open hardware laptop produced
by olimex. With these patches and a bootloader capable of setting up
simple framebuffer the laptop is quite useable.

Patches 1-4 add some pin descriptions and devices in the A64 dtsi, which
are useful to have on the teres.
Patch 5 finally adds the board dts itself.

Changes to the previous version are listed along the individual patches.

best regards,
Harald


Harald Geyer (5):
arm64: dts: allwinner: a64: Add i2c0 pins
arm64: dts: allwinner: a64: Add watchdog
arm64: dts: allwinner: a64: add simplefb for A64 SoC
arm64: dts: allwinner: a64: Add pwm device
arm64: allwinner: a64: Add support for TERES-I laptop

.../devicetree/bindings/pwm/pwm-sun4i.txt | 1 +
.../devicetree/bindings/watchdog/sunxi-wdt.txt | 6 +-
arch/arm64/boot/dts/allwinner/Makefile | 1 +
.../boot/dts/allwinner/sun50i-a64-teres-i.dts | 279 +++++++++++++++++++++
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 46 ++++
5 files changed, 331 insertions(+), 2 deletions(-)
create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts

--
2.11.0



2018-03-15 16:27:22

by Harald Geyer

[permalink] [raw]
Subject: [PATCHv2 5/5] arm64: allwinner: a64: Add support for TERES-I laptop

The TERES-I is an open hardware laptop built by Olimex using the
Allwinner A64 SoC.

Add the board specific .dts file, which includes the A64 .dtsi and
enables the peripherals that we support so far.

Signed-off-by: Harald Geyer <[email protected]>
---
changes since v1:
* use SPDX header instead of license text
* change model string to match compatible string
* removed node labes from leds
* added label properties ot led nodes
* add a comment about the purpose of i2c0

arch/arm64/boot/dts/allwinner/Makefile | 1 +
.../boot/dts/allwinner/sun50i-a64-teres-i.dts | 279 +++++++++++++++++++++
2 files changed, 280 insertions(+)
create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts

diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
index f505227b0250..5f073f7423b7 100644
--- a/arch/arm64/boot/dts/allwinner/Makefile
+++ b/arch/arm64/boot/dts/allwinner/Makefile
@@ -5,6 +5,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-olinuxino.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-orangepi-win.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-pine64-plus.dtb sun50i-a64-pine64.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-sopine-baseboard.dtb
+dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-teres-i.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-pc2.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-prime.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus2.dtb
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
new file mode 100644
index 000000000000..b105430e0368
--- /dev/null
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
@@ -0,0 +1,279 @@
+/*
+ * Copyright (C) Harald Geyer <[email protected]>
+ * based on sun50i-a64-olinuxino.dts by Jagan Teki <[email protected]>
+ *
+ * SPDX-License-Identifier: (GPL-2.0 OR MIT)
+ */
+
+/dts-v1/;
+
+#include "sun50i-a64.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/pwm/pwm.h>
+
+/ {
+ model = "Olimex A64 Teres-I";
+ compatible = "olimex,a64-teres-i", "allwinner,sun50i-a64";
+
+ aliases {
+ serial0 = &uart0;
+ };
+
+ backlight: backlight {
+ compatible = "pwm-backlight";
+ pwms = <&pwm 0 50000 0>;
+ brightness-levels = <0 10 20 30 40 50 60 70 100>;
+ default-brightness-level = <3>;
+ enable-gpios = <&pio 3 23 GPIO_ACTIVE_HIGH>; /* PD23 */
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+
+ framebuffer-lcd {
+ eDP25-supply = <&reg_dldo2>;
+ eDP12-supply = <&reg_dldo3>;
+ };
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+
+ lid-switch {
+ label = "Lid Switch";
+ gpios = <&r_pio 0 8 GPIO_ACTIVE_LOW>; /* PL8 */
+ linux,input-type = <EV_SW>;
+ linux,code = <SW_LID>;
+ };
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ capslock {
+ label = "leds:green:capslock";
+ gpios = <&pio 2 7 GPIO_ACTIVE_HIGH>; /* PC7 */
+ };
+
+ numlock {
+ label = "leds:green:numlock";
+ gpios = <&pio 2 4 GPIO_ACTIVE_HIGH>; /* PC4 */
+ };
+ };
+
+ reg_usb1_vbus: usb1-vbus {
+ compatible = "regulator-fixed";
+ regulator-name = "usb1-vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ enable-active-high;
+ gpio = <&r_pio 0 7 GPIO_ACTIVE_HIGH>; /* PL7 */
+ status = "okay";
+ };
+
+ wifi_pwrseq: wifi_pwrseq {
+ compatible = "mmc-pwrseq-simple";
+ reset-gpios = <&r_pio 0 2 GPIO_ACTIVE_LOW>; /* PL2 */
+ };
+};
+
+&ehci1 {
+ status = "okay";
+};
+
+
+/* The ANX6345 eDP-bridge is on i2c0. There is no linux (mainline)
+ * driver for this chip at the moment, the bootloader initializes it.
+ * However it can be accessed with the i2c-dev driver from user space.
+ */
+&i2c0 {
+ clock-frequency = <100000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c0_pins>;
+ status = "okay";
+};
+
+&mmc0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mmc0_pins>;
+ vmmc-supply = <&reg_dcdc1>;
+ cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>;
+ disable-wp;
+ bus-width = <4>;
+ status = "okay";
+};
+
+&mmc1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mmc1_pins>;
+ vmmc-supply = <&reg_aldo2>;
+ vqmmc-supply = <&reg_dldo4>;
+ mmc-pwrseq = <&wifi_pwrseq>;
+ bus-width = <4>;
+ non-removable;
+ status = "okay";
+
+ rtl8723bs: wifi@1 {
+ reg = <1>;
+ interrupt-parent = <&r_pio>;
+ interrupts = <0 3 IRQ_TYPE_LEVEL_LOW>; /* PL3 */
+ interrupt-names = "host-wake";
+ };
+};
+
+&mmc2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mmc2_pins>;
+ vmmc-supply = <&reg_dcdc1>;
+ vqmmc-supply = <&reg_dcdc1>;
+ bus-width = <8>;
+ non-removable;
+ cap-mmc-hw-reset;
+ status = "okay";
+};
+
+&pwm {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm_pin>;
+ status = "okay";
+};
+
+&ohci1 {
+ status = "okay";
+};
+
+&r_rsb {
+ status = "okay";
+
+ axp803: pmic@3a3 {
+ compatible = "x-powers,axp803";
+ reg = <0x3a3>;
+ interrupt-parent = <&r_intc>;
+ interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+ wakeup-source;
+ };
+};
+
+#include "axp803.dtsi"
+
+&reg_aldo1 {
+ regulator-always-on;
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ regulator-name = "vcc-pe";
+};
+
+&reg_aldo2 {
+ regulator-always-on;
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-name = "vcc-pl";
+};
+
+&reg_aldo3 {
+ regulator-always-on;
+ regulator-min-microvolt = <3000000>;
+ regulator-max-microvolt = <3000000>;
+ regulator-name = "vcc-pll-avcc";
+};
+
+&reg_dcdc1 {
+ regulator-always-on;
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-name = "vcc-3v3";
+};
+
+&reg_dcdc2 {
+ regulator-always-on;
+ regulator-min-microvolt = <1040000>;
+ regulator-max-microvolt = <1300000>;
+ regulator-name = "vdd-cpux";
+};
+
+/* DCDC3 is polyphased with DCDC2 */
+
+&reg_dcdc5 {
+ regulator-always-on;
+ regulator-min-microvolt = <1500000>;
+ regulator-max-microvolt = <1500000>;
+ regulator-name = "vcc-ddr3";
+};
+
+&reg_dcdc6 {
+ regulator-always-on;
+ regulator-min-microvolt = <1100000>;
+ regulator-max-microvolt = <1100000>;
+ regulator-name = "vdd-sys";
+};
+
+&reg_dldo1 {
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-name = "vcc-hdmi";
+};
+
+&reg_dldo2 {
+ regulator-min-microvolt = <2500000>;
+ regulator-max-microvolt = <2500000>;
+ regulator-name = "vcc-pd";
+};
+
+&reg_dldo3 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-name = "eDP12";
+};
+
+&reg_dldo4 {
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-name = "vcc-wifi-io";
+};
+
+&reg_eldo1 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-name = "cpvdd";
+};
+
+&reg_eldo2 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-name = "vcc-dvdd-csi";
+};
+
+&reg_fldo1 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-name = "vcc-1v2-hsic";
+};
+
+/*
+ * The A64 chip cannot work without this regulator off, although
+ * it seems to be only driving the AR100 core.
+ * Maybe we don't still know well about CPUs domain.
+ */
+&reg_fldo2 {
+ regulator-always-on;
+ regulator-min-microvolt = <1100000>;
+ regulator-max-microvolt = <1100000>;
+ regulator-name = "vdd-cpus";
+};
+
+&reg_rtc_ldo {
+ regulator-name = "vcc-rtc";
+};
+
+&uart0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart0_pins_a>;
+ status = "okay";
+};
+
+&usbphy {
+ usb1_vbus-supply = <&reg_usb1_vbus>;
+ status = "okay";
+};
--
2.11.0


2018-03-15 16:27:56

by Harald Geyer

[permalink] [raw]
Subject: [PATCHv2 1/5] arm64: dts: allwinner: a64: Add i2c0 pins

Add the proper pin group node to reference in board files.

Reviewed-by: Andre Przywara <[email protected]>
Signed-off-by: Harald Geyer <[email protected]>
---
changes since v1: none

arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 1b6dc31e7d91..64e452a758fa 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -309,6 +309,11 @@
interrupt-controller;
#interrupt-cells = <3>;

+ i2c0_pins: i2c0_pins {
+ pins = "PH0", "PH1";
+ function = "i2c0";
+ };
+
i2c1_pins: i2c1_pins {
pins = "PH2", "PH3";
function = "i2c1";
--
2.11.0


2018-03-15 16:27:56

by Harald Geyer

[permalink] [raw]
Subject: [PATCHv2 4/5] arm64: dts: allwinner: a64: Add pwm device

This device is compatible with A13, so no new driver is needed.
A new compatible string is reserved in the binding documentation, to be
used together with the proper fall back. Tested on Teres-I.

Signed-off-by: Harald Geyer <[email protected]>
---
changes since v1:
* add and use an A64 specific compatible string
* claim the full memory range

I saw that Andre Przywara has been working on A64 pwm too and has
submitted some patches a few days ago. I think his patches are functionally
equivalent to this one here, but clean up things a bit and additionally
add support for r_pwm and thus are preferable. See:

https://groups.google.com/forum/#!topic/linux-sunxi/hQFeteP591k

I'm including my patch here mostly to have a consistent series for others
to test. OTOH you might merge the device tree changes here and pick up
the cleanup patches from him. Either way should work fine.

Documentation/devicetree/bindings/pwm/pwm-sun4i.txt | 1 +
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 14 ++++++++++++++
2 files changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt b/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
index 51ff54c8b8ef..5986a3b2a504 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
@@ -5,6 +5,7 @@ Required properties:
- "allwinner,sun4i-a10-pwm"
- "allwinner,sun5i-a10s-pwm"
- "allwinner,sun5i-a13-pwm"
+ - "allwinner,sun50i-a64-pwm","allwinner,sun5i-a13-pwm"
- "allwinner,sun7i-a20-pwm"
- "allwinner,sun8i-h3-pwm"
- reg: physical base address and length of the controller's registers
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 1b2ef28c42bd..7e72eadd07b1 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -364,6 +364,11 @@
bias-pull-up;
};

+ pwm_pin: pwm_pin {
+ pins = "PD22";
+ function = "pwm";
+ };
+
rmii_pins: rmii_pins {
pins = "PD10", "PD11", "PD13", "PD14", "PD17",
"PD18", "PD19", "PD20", "PD22", "PD23";
@@ -629,6 +634,15 @@
#interrupt-cells = <3>;
};

+ pwm: pwm@1c21400 {
+ compatible = "allwinner,sun50i-a64-pwm",
+ "allwinner,sun5i-a13-pwm";
+ reg = <0x01c21400 0x400>;
+ clocks = <&osc24M>;
+ #pwm-cells = <3>;
+ status = "disabled";
+ };
+
rtc: rtc@1f00000 {
compatible = "allwinner,sun6i-a31-rtc";
reg = <0x01f00000 0x54>;
--
2.11.0


2018-03-15 16:27:58

by Harald Geyer

[permalink] [raw]
Subject: [PATCHv2 3/5] arm64: dts: allwinner: a64: add simplefb for A64 SoC

The A64 SoC features two display pipelines, one has a LCD output, the
other has a HDMI output.

Add support for simplefb for the LCD output. Tested on Teres I.

This patch was inspired by work of Icenowy Zheng.

Signed-off-by: Harald Geyer <[email protected]>
---
changes since v1: none

arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 8401e7f887ff..1b2ef28c42bd 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -52,6 +52,26 @@
#address-cells = <1>;
#size-cells = <1>;

+ chosen {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+/*
+ * The pipeline mixer0-lcd0 depends on clock CLK_MIXER0 from DE2 CCU.
+ * However there is no support for this clock on A64 yet, so we depend
+ * on the upstream clocks here to keep them (and thus CLK_MIXER0) up.
+ */
+ simplefb_lcd: framebuffer-lcd {
+ compatible = "allwinner,simple-framebuffer",
+ "simple-framebuffer";
+ allwinner,pipeline = "mixer0-lcd0";
+ clocks = <&ccu CLK_TCON0>,
+ <&ccu CLK_DE>, <&ccu CLK_BUS_DE>;
+ status = "disabled";
+ };
+ };
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
--
2.11.0


2018-03-15 16:28:16

by Harald Geyer

[permalink] [raw]
Subject: [PATCHv2 2/5] arm64: dts: allwinner: a64: Add watchdog

Add a watchdog node for the A64, automatically enabled on all boards.
Since the device is compatible with an existing driver, we only reserve
a new compatible string to be used together with the fall back.
Tested on Olimex Teres-I.

Signed-off-by: Harald Geyer <[email protected]>
---
changes since v1:
* add and use a specific compatible string

Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt | 6 ++++--
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 7 +++++++
2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt b/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
index 62dd5baad70e..04fc368d828f 100644
--- a/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
@@ -2,8 +2,10 @@ Allwinner SoCs Watchdog timer

Required properties:

-- compatible : should be either "allwinner,sun4i-a10-wdt" or
- "allwinner,sun6i-a31-wdt"
+- compatible : should be one of
+ "allwinner,sun4i-a10-wdt"
+ "allwinner,sun6i-a31-wdt"
+ "allwinner,sun50i-a64-wdt","allwinner,sun6i-a31-wdt"
- reg : Specifies base physical address and size of the registers.

Example:
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 64e452a758fa..8401e7f887ff 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -665,5 +665,12 @@
#address-cells = <1>;
#size-cells = <0>;
};
+
+ wdt0: watchdog@1c20ca0 {
+ compatible = "allwinner,sun50i-a64-wdt",
+ "allwinner,sun6i-a31-wdt";
+ reg = <0x01c20ca0 0x20>;
+ interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
+ };
};
};
--
2.11.0


2018-03-16 06:39:46

by afzal mohammed

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] arm64: allwinner: a64: Add support for TERES-I laptop

Hi,

On Thu, Mar 15, 2018 at 04:25:10PM +0000, Harald Geyer wrote:
> The TERES-I is an open hardware laptop built by Olimex using the
> Allwinner A64 SoC.
>
> Add the board specific .dts file, which includes the A64 .dtsi and
> enables the peripherals that we support so far.
>
> Signed-off-by: Harald Geyer <[email protected]>

Received only patch 4 & 5 in my inbox, receive path was via
linux-kernel rather than linux-arm-kernel, but in both archives all
patches are seen (though threading seems not right), probably missing
patches are due to issue gmail have with LKML,

so had to pull the series from patchwork, for the series,

Tested-by: afzal mohammed <[email protected]>

afzal

2018-03-16 17:52:00

by afzal mohammed

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] arm64: allwinner: a64: Add support for TERES-I laptop

Hi,

On Fri, Mar 16, 2018 at 12:07:53PM +0530, afzal mohammed wrote:

> Received only patch 4 & 5 in my inbox, receive path was via
> linux-kernel rather than linux-arm-kernel, but in both archives all
> patches are seen (though threading seems not right), probably missing
> patches are due to issue gmail have with LKML,

Cover letter plus 1-3 patches was swallowed by spam filter, even your
reply to me on v1 cover letter subthread was so, dunno whether it has
something to do with your mail header contents.

afzal

2018-03-18 12:55:26

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCHv2 4/5] arm64: dts: allwinner: a64: Add pwm device

On Thu, Mar 15, 2018 at 04:25:09PM +0000, Harald Geyer wrote:
> This device is compatible with A13, so no new driver is needed.
> A new compatible string is reserved in the binding documentation, to be
> used together with the proper fall back. Tested on Teres-I.
>
> Signed-off-by: Harald Geyer <[email protected]>
> ---
> changes since v1:
> * add and use an A64 specific compatible string
> * claim the full memory range
>
> I saw that Andre Przywara has been working on A64 pwm too and has
> submitted some patches a few days ago. I think his patches are functionally
> equivalent to this one here, but clean up things a bit and additionally
> add support for r_pwm and thus are preferable. See:
>
> https://groups.google.com/forum/#!topic/linux-sunxi/hQFeteP591k
>
> I'm including my patch here mostly to have a consistent series for others
> to test. OTOH you might merge the device tree changes here and pick up
> the cleanup patches from him. Either way should work fine.

I would sync up with Andre and send out a combined series. Don't make
the maintainer(s) have to sort this out.

> Documentation/devicetree/bindings/pwm/pwm-sun4i.txt | 1 +
> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 14 ++++++++++++++
> 2 files changed, 15 insertions(+)

Reviewed-by: Rob Herring <[email protected]>

2018-03-18 12:57:41

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCHv2 2/5] arm64: dts: allwinner: a64: Add watchdog

On Thu, Mar 15, 2018 at 04:25:07PM +0000, Harald Geyer wrote:
> Add a watchdog node for the A64, automatically enabled on all boards.
> Since the device is compatible with an existing driver, we only reserve
> a new compatible string to be used together with the fall back.
> Tested on Olimex Teres-I.
>
> Signed-off-by: Harald Geyer <[email protected]>
> ---
> changes since v1:
> * add and use a specific compatible string
>
> Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt | 6 ++++--
> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 7 +++++++
> 2 files changed, 11 insertions(+), 2 deletions(-)

Reviewed-by: Rob Herring <[email protected]>

2018-03-18 13:52:35

by Andre Przywara

[permalink] [raw]
Subject: Re: [PATCHv2 4/5] arm64: dts: allwinner: a64: Add pwm device

On 18/03/18 12:52, Rob Herring wrote:
> On Thu, Mar 15, 2018 at 04:25:09PM +0000, Harald Geyer wrote:
>> This device is compatible with A13, so no new driver is needed.
>> A new compatible string is reserved in the binding documentation, to be
>> used together with the proper fall back. Tested on Teres-I.
>>
>> Signed-off-by: Harald Geyer <[email protected]>
>> ---
>> changes since v1:
>> * add and use an A64 specific compatible string
>> * claim the full memory range
>>
>> I saw that Andre Przywara has been working on A64 pwm too and has
>> submitted some patches a few days ago. I think his patches are functionally
>> equivalent to this one here, but clean up things a bit and additionally
>> add support for r_pwm and thus are preferable. See:
>>
>> https://groups.google.com/forum/#!topic/linux-sunxi/hQFeteP591k
>>
>> I'm including my patch here mostly to have a consistent series for others
>> to test. OTOH you might merge the device tree changes here and pick up
>> the cleanup patches from him. Either way should work fine.
>
> I would sync up with Andre and send out a combined series. Don't make
> the maintainer(s) have to sort this out.

I have something ready, but need to test it. Will try to send an updated
series later tonight.

Thanks,
Andre.

>
>> Documentation/devicetree/bindings/pwm/pwm-sun4i.txt | 1 +
>> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 14 ++++++++++++++
>> 2 files changed, 15 insertions(+)
>
> Reviewed-by: Rob Herring <[email protected]>
>


2018-03-18 20:24:17

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] arm64: allwinner: a64: Add support for TERES-I laptop

On Thu, Mar 15, 2018 at 04:25:10PM +0000, Harald Geyer wrote:
> + leds {
> + compatible = "gpio-leds";
> +
> + capslock {
> + label = "leds:green:capslock";

The first part is supposed to be the name of the boards. I did sed
s/leds/teres-i/, and applied, together with all the patches but the
PWM (so I had to drop the backlight node as well).

Please coordinate with Andre about who should send the PWM support.

Maxime

--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (556.00 B)
signature.asc (849.00 B)
Download all attachments

2018-03-19 07:19:29

by afzal mohammed

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] arm64: allwinner: a64: Add support for TERES-I laptop

Hi Maxime,

On Sun, Mar 18, 2018 at 09:22:51PM +0100, Maxime Ripard wrote:
> The first part is supposed to be the name of the boards. I did sed
> s/leds/teres-i/, and applied, together with all the patches but the
> PWM (so I had to drop the backlight node as well).
>
> Please coordinate with Andre about who should send the PWM support.

Assuming that these patches were applied to your sunxi/dt64-for-4.17
branch, since PWM support patch is missing, there is a build error,

arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts:129.1-5 Label or path pwm not found

Diff at the end cures it.

(there is another H6 pine 64 DT build error related to header file
missing)

afzal


--->8---
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
index b3c7ef6b6fe5..d9baab3dc96b 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
@@ -126,12 +126,6 @@
status = "okay";
};

-&pwm {
- pinctrl-names = "default";
- pinctrl-0 = <&pwm_pin>;
- status = "okay";
-};
-
&ohci1 {
status = "okay";
};


2018-03-19 15:29:23

by Harald Geyer

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] arm64: allwinner: a64: Add support for TERES-I laptop

Maxime Ripard writes:
> On Thu, Mar 15, 2018 at 04:25:10PM +0000, Harald Geyer wrote:
> > + leds {
> > + compatible = "gpio-leds";
> > +
> > + capslock {
> > + label = "leds:green:capslock";
>
> The first part is supposed to be the name of the boards. I did sed
> s/leds/teres-i/, and applied,

I'm not sure what good this convention would do, if anything it seems
to make it harder to write portable scripts, but in the end I don't care.

> together with all the patches but the
> PWM (so I had to drop the backlight node as well).
>
> Please coordinate with Andre about who should send the PWM support.

Seems the patch got broken because only the backlight node but not the
pwm node was removed. Anyway, since Andre has already sent an updated
version of his series, maybe just revert the broken patch, merge his
series and then apply the original teres-i patch again?

I'm going to test his patches soon, but I don't expect any problems.

Thanks,
Harald

2018-03-20 01:56:00

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] arm64: allwinner: a64: Add support for TERES-I laptop

hi,

On Mon, Mar 19, 2018 at 12:47:23PM +0530, afzal mohammed wrote:
> On Sun, Mar 18, 2018 at 09:22:51PM +0100, Maxime Ripard wrote:
> > The first part is supposed to be the name of the boards. I did sed
> > s/leds/teres-i/, and applied, together with all the patches but the
> > PWM (so I had to drop the backlight node as well).
> >
> > Please coordinate with Andre about who should send the PWM support.
>
> Assuming that these patches were applied to your sunxi/dt64-for-4.17
> branch, since PWM support patch is missing, there is a build error,
>
> arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts:129.1-5 Label or path pwm not found
>
> Diff at the end cures it.
>
> (there is another H6 pine 64 DT build error related to header file
> missing)
>
> afzal
>
>
> --->8---
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> index b3c7ef6b6fe5..d9baab3dc96b 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> @@ -126,12 +126,6 @@
> status = "okay";
> };
>
> -&pwm {
> - pinctrl-names = "default";
> - pinctrl-0 = <&pwm_pin>;
> - status = "okay";
> -};
> -
> &ohci1 {
> status = "okay";
> };
>

Thanks for noticing and sending a patch. I've fixed the original
commit.

Maxime

--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (1.51 kB)
signature.asc (849.00 B)
Download all attachments

2018-03-20 14:19:50

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] arm64: allwinner: a64: Add support for TERES-I laptop

On Mon, Mar 19, 2018 at 04:27:36PM +0100, Harald Geyer wrote:
> > together with all the patches but the
> > PWM (so I had to drop the backlight node as well).
> >
> > Please coordinate with Andre about who should send the PWM support.
>
> Seems the patch got broken because only the backlight node but not the
> pwm node was removed. Anyway, since Andre has already sent an updated
> version of his series, maybe just revert the broken patch, merge his
> series and then apply the original teres-i patch again?

Unfortunately, there's dependencies on the PWM driver itself, and the
maintainer hasn't replied yet.

Maxime

--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

2018-03-20 17:11:20

by Andre Przywara

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] arm64: allwinner: a64: Add support for TERES-I laptop

Hi,

On 20/03/18 14:13, Maxime Ripard wrote:
> On Mon, Mar 19, 2018 at 04:27:36PM +0100, Harald Geyer wrote:
>>> together with all the patches but the
>>> PWM (so I had to drop the backlight node as well).
>>>
>>> Please coordinate with Andre about who should send the PWM support.
>>
>> Seems the patch got broken because only the backlight node but not the
>> pwm node was removed. Anyway, since Andre has already sent an updated
>> version of his series, maybe just revert the broken patch, merge his
>> series and then apply the original teres-i patch again?
>
> Unfortunately, there's dependencies on the PWM driver itself, and the
> maintainer hasn't replied yet.

But those dependencies are purely "administrative", not technical,
aren't they? As the existing driver worked already with the DT changes,
it's just the listing of the compatible strings in the binding doc that
is missing? IIRC we added those later on in the past already.

So I think it's safe to merge them independently:
"[PATCH v2 1/4] pwm: sun4i: drop unused .has_rdy member" and
"[PATCH v2 2/4] pwm: sun4i: simplify controller mapping" are
PWM fixes and go via Thierry, I guess.

"[PATCH v2 3/4] dt-bindings: pwm: sunxi: add new compatible strings" is
just Documentation of existing behaviour, and independent from 1/4 and 2/4.

"[PATCH v2 4/4] dts: sunxi: A64: Add PWM controllers" just "softly"
depends on the introduction of the compatible strings in 3/4, but has no
real technical dependency. It can go in any time on its own without
breaking the build or functionality.

Or am I too sloppy here?

Cheers,
Andre.

2018-03-22 18:00:16

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] arm64: allwinner: a64: Add support for TERES-I laptop

On Tue, Mar 20, 2018 at 05:09:47PM +0000, Andre Przywara wrote:
> Hi,
>
> On 20/03/18 14:13, Maxime Ripard wrote:
> > On Mon, Mar 19, 2018 at 04:27:36PM +0100, Harald Geyer wrote:
> >>> together with all the patches but the
> >>> PWM (so I had to drop the backlight node as well).
> >>>
> >>> Please coordinate with Andre about who should send the PWM support.
> >>
> >> Seems the patch got broken because only the backlight node but not the
> >> pwm node was removed. Anyway, since Andre has already sent an updated
> >> version of his series, maybe just revert the broken patch, merge his
> >> series and then apply the original teres-i patch again?
> >
> > Unfortunately, there's dependencies on the PWM driver itself, and the
> > maintainer hasn't replied yet.
>
> But those dependencies are purely "administrative", not technical,
> aren't they? As the existing driver worked already with the DT changes,
> it's just the listing of the compatible strings in the binding doc that
> is missing? IIRC we added those later on in the past already.
>
> So I think it's safe to merge them independently:
> "[PATCH v2 1/4] pwm: sun4i: drop unused .has_rdy member" and
> "[PATCH v2 2/4] pwm: sun4i: simplify controller mapping" are
> PWM fixes and go via Thierry, I guess.
>
> "[PATCH v2 3/4] dt-bindings: pwm: sunxi: add new compatible strings" is
> just Documentation of existing behaviour, and independent from 1/4 and 2/4.
>
> "[PATCH v2 4/4] dts: sunxi: A64: Add PWM controllers" just "softly"
> depends on the introduction of the compatible strings in 3/4, but has no
> real technical dependency. It can go in any time on its own without
> breaking the build or functionality.
>
> Or am I too sloppy here?

As far as I know, Thierry never commented on any version of these
patches, so I'd still like to get his review first.

Maxime

--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (1.95 kB)
signature.asc (849.00 B)
Download all attachments

2018-06-22 16:32:02

by Icenowy Zheng

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] arm64: allwinner: a64: Add support for TERES-I laptop

在 2018-03-15四的 16:25 +0000,Harald Geyer写道:
> The TERES-I is an open hardware laptop built by Olimex using the
> Allwinner A64 SoC.
>
> Add the board specific .dts file, which includes the A64 .dtsi and
> enables the peripherals that we support so far.
>
> Signed-off-by: Harald Geyer <[email protected]>
> ---
> changes since v1:
> * use SPDX header instead of license text
> * change model string to match compatible string
> * removed node labes from leds
> * added label properties ot led nodes
> * add a comment about the purpose of i2c0
>
> arch/arm64/boot/dts/allwinner/Makefile | 1 +
> .../boot/dts/allwinner/sun50i-a64-teres-i.dts | 279
> +++++++++++++++++++++
> 2 files changed, 280 insertions(+)
> create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a64-teres-
> i.dts
>
> diff --git a/arch/arm64/boot/dts/allwinner/Makefile
> b/arch/arm64/boot/dts/allwinner/Makefile
> index f505227b0250..5f073f7423b7 100644
> --- a/arch/arm64/boot/dts/allwinner/Makefile
> +++ b/arch/arm64/boot/dts/allwinner/Makefile
> @@ -5,6 +5,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-olinuxino.dtb
> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-orangepi-win.dtb
> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-pine64-plus.dtb sun50i-a64-
> pine64.dtb
> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-sopine-baseboard.dtb
> +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-teres-i.dtb
> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-pc2.dtb
> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-prime.dtb
> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus2.dtb
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> new file mode 100644
> index 000000000000..b105430e0368
> --- /dev/null
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> @@ -0,0 +1,279 @@
> +/*
> + * Copyright (C) Harald Geyer <[email protected]>
> + * based on sun50i-a64-olinuxino.dts by Jagan Teki <[email protected]
> om>
> + *
> + * SPDX-License-Identifier: (GPL-2.0 OR MIT)
> + */
> +
> +/dts-v1/;
> +
> +#include "sun50i-a64.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/pwm/pwm.h>
> +
> +/ {
> + model = "Olimex A64 Teres-I";
> + compatible = "olimex,a64-teres-i", "allwinner,sun50i-a64";
> +
> + aliases {
> + serial0 = &uart0;
> + };
> +
> + backlight: backlight {
> + compatible = "pwm-backlight";
> + pwms = <&pwm 0 50000 0>;
> + brightness-levels = <0 10 20 30 40 50 60 70 100>;
> + default-brightness-level = <3>;
> + enable-gpios = <&pio 3 23 GPIO_ACTIVE_HIGH>; /* PD23
> */
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> +
> + framebuffer-lcd {
> + eDP25-supply = <&reg_dldo2>;
> + eDP12-supply = <&reg_dldo3>;
> + };
> + };
> +
> + gpio-keys {
> + compatible = "gpio-keys";
> +
> + lid-switch {
> + label = "Lid Switch";
> + gpios = <&r_pio 0 8 GPIO_ACTIVE_LOW>; /* PL8
> */
> + linux,input-type = <EV_SW>;
> + linux,code = <SW_LID>;
> + };
> + };
> +
> + leds {
> + compatible = "gpio-leds";
> +
> + capslock {
> + label = "leds:green:capslock";
> + gpios = <&pio 2 7 GPIO_ACTIVE_HIGH>; /* PC7
> */
> + };
> +
> + numlock {
> + label = "leds:green:numlock";
> + gpios = <&pio 2 4 GPIO_ACTIVE_HIGH>; /* PC4
> */
> + };
> + };
> +
> + reg_usb1_vbus: usb1-vbus {
> + compatible = "regulator-fixed";
> + regulator-name = "usb1-vbus";
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + enable-active-high;
> + gpio = <&r_pio 0 7 GPIO_ACTIVE_HIGH>; /* PL7 */
> + status = "okay";
> + };
> +
> + wifi_pwrseq: wifi_pwrseq {
> + compatible = "mmc-pwrseq-simple";
> + reset-gpios = <&r_pio 0 2 GPIO_ACTIVE_LOW>; /* PL2
> */
> + };
> +};
> +
> +&ehci1 {
> + status = "okay";
> +};
> +
> +
> +/* The ANX6345 eDP-bridge is on i2c0. There is no linux (mainline)
> + * driver for this chip at the moment, the bootloader initializes
> it.
> + * However it can be accessed with the i2c-dev driver from user
> space.
> + */
> +&i2c0 {
> + clock-frequency = <100000>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c0_pins>;
> + status = "okay";
> +};
> +
> +&mmc0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&mmc0_pins>;
> + vmmc-supply = <&reg_dcdc1>;
> + cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>;
> + disable-wp;
> + bus-width = <4>;
> + status = "okay";
> +};
> +
> +&mmc1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&mmc1_pins>;
> + vmmc-supply = <&reg_aldo2>;
> + vqmmc-supply = <&reg_dldo4>;
> + mmc-pwrseq = <&wifi_pwrseq>;
> + bus-width = <4>;
> + non-removable;
> + status = "okay";
> +
> + rtl8723bs: wifi@1 {
> + reg = <1>;
> + interrupt-parent = <&r_pio>;
> + interrupts = <0 3 IRQ_TYPE_LEVEL_LOW>; /* PL3 */
> + interrupt-names = "host-wake";
> + };

I think this node has some problem:

- This device node has no binding. The "host-wake" interrupt is part of
Broadcom SDIO Wi-Fi binding, rather than a generic one.
- Without the interrupt this device node isn't needed, as RTL8723BS has
MAC eFUSE and doesn't need a MAC in device tree.

In order to solve the problems. I suggest either drop this device node
or make a generic "sdio-wifi" device tree binding. Personally I prefer
the latter, as it's more accurate device representation.

If such a device tree binding is added, I think it should contain the
"host-wake" interrupt and a "local-mac-address" property. Both can be
ignored by the driver. (This interrupt can be needed if a more card-
ventor-neutral name is found.)

> +};
> +
> +&mmc2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&mmc2_pins>;
> + vmmc-supply = <&reg_dcdc1>;
> + vqmmc-supply = <&reg_dcdc1>;
> + bus-width = <8>;
> + non-removable;
> + cap-mmc-hw-reset;
> + status = "okay";
> +};
> +
> +&pwm {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pwm_pin>;
> + status = "okay";
> +};
> +
> +&ohci1 {
> + status = "okay";
> +};
> +
> +&r_rsb {
> + status = "okay";
> +
> + axp803: pmic@3a3 {
> + compatible = "x-powers,axp803";
> + reg = <0x3a3>;
> + interrupt-parent = <&r_intc>;
> + interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> + wakeup-source;
> + };
> +};
> +
> +#include "axp803.dtsi"
> +
> +&reg_aldo1 {
> + regulator-always-on;
> + regulator-min-microvolt = <2800000>;
> + regulator-max-microvolt = <2800000>;
> + regulator-name = "vcc-pe";
> +};
> +
> +&reg_aldo2 {
> + regulator-always-on;
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "vcc-pl";
> +};
> +
> +&reg_aldo3 {
> + regulator-always-on;
> + regulator-min-microvolt = <3000000>;
> + regulator-max-microvolt = <3000000>;
> + regulator-name = "vcc-pll-avcc";
> +};
> +
> +&reg_dcdc1 {
> + regulator-always-on;
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "vcc-3v3";
> +};
> +
> +&reg_dcdc2 {
> + regulator-always-on;
> + regulator-min-microvolt = <1040000>;
> + regulator-max-microvolt = <1300000>;
> + regulator-name = "vdd-cpux";
> +};
> +
> +/* DCDC3 is polyphased with DCDC2 */
> +
> +&reg_dcdc5 {
> + regulator-always-on;
> + regulator-min-microvolt = <1500000>;
> + regulator-max-microvolt = <1500000>;
> + regulator-name = "vcc-ddr3";
> +};
> +
> +&reg_dcdc6 {
> + regulator-always-on;
> + regulator-min-microvolt = <1100000>;
> + regulator-max-microvolt = <1100000>;
> + regulator-name = "vdd-sys";
> +};
> +
> +&reg_dldo1 {
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "vcc-hdmi";
> +};
> +
> +&reg_dldo2 {
> + regulator-min-microvolt = <2500000>;
> + regulator-max-microvolt = <2500000>;
> + regulator-name = "vcc-pd";
> +};
> +
> +&reg_dldo3 {
> + regulator-min-microvolt = <1200000>;
> + regulator-max-microvolt = <1200000>;
> + regulator-name = "eDP12";
> +};
> +
> +&reg_dldo4 {
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "vcc-wifi-io";
> +};
> +
> +&reg_eldo1 {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-name = "cpvdd";
> +};
> +
> +&reg_eldo2 {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-name = "vcc-dvdd-csi";
> +};
> +
> +&reg_fldo1 {
> + regulator-min-microvolt = <1200000>;
> + regulator-max-microvolt = <1200000>;
> + regulator-name = "vcc-1v2-hsic";
> +};
> +
> +/*
> + * The A64 chip cannot work without this regulator off, although
> + * it seems to be only driving the AR100 core.
> + * Maybe we don't still know well about CPUs domain.
> + */
> +&reg_fldo2 {
> + regulator-always-on;
> + regulator-min-microvolt = <1100000>;
> + regulator-max-microvolt = <1100000>;
> + regulator-name = "vdd-cpus";
> +};
> +
> +&reg_rtc_ldo {
> + regulator-name = "vcc-rtc";
> +};
> +
> +&uart0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart0_pins_a>;
> + status = "okay";
> +};
> +
> +&usbphy {
> + usb1_vbus-supply = <&reg_usb1_vbus>;
> + status = "okay";
> +};

2018-06-24 16:42:37

by Harald Geyer

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] arm64: allwinner: a64: Add support for TERES-I laptop

Icenowy Zheng writes:
> 在 2018-03-15四的 16:25 +0000,Harald Geyer写道:
> > +&mmc1 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&mmc1_pins>;
> > + vmmc-supply = <&reg_aldo2>;
> > + vqmmc-supply = <&reg_dldo4>;
> > + mmc-pwrseq = <&wifi_pwrseq>;
> > + bus-width = <4>;
> > + non-removable;
> > + status = "okay";
> > +
> > + rtl8723bs: wifi@1 {
> > + reg = <1>;
> > + interrupt-parent = <&r_pio>;
> > + interrupts = <0 3 IRQ_TYPE_LEVEL_LOW>; /* PL3 */
> > + interrupt-names = "host-wake";
> > + };
>
> I think this node has some problem:

Thanks for the heads up! Admittedly, I simply copied this node from
sun50i-a64-olinuxino.dts and since it worked, didn't look into it in
too much detail.

> - This device node has no binding. The "host-wake" interrupt is part of
> Broadcom SDIO Wi-Fi binding, rather than a generic one.

I think the general mmc and interrupts bindings apply. And the mmc binding
clearly states that for sub-nodes a compatible string is optional.

However I just realized that the 'interrupt-names' property is not part
of the general interrupts binding, so I guess at least this property should
be removed.

> - Without the interrupt this device node isn't needed, as RTL8723BS has
> MAC eFUSE and doesn't need a MAC in device tree.

Indeed. I wasn't aware of this, but I just tested and the device probes
fine without the subnode present. I think the devicetree is mainly for
information which cannot be probed, so maybe the subnode should just get
removed.

> In order to solve the problems. I suggest either drop this device node
> or make a generic "sdio-wifi" device tree binding. Personally I prefer
> the latter, as it's more accurate device representation.
>
> If such a device tree binding is added, I think it should contain the
> "host-wake" interrupt and a "local-mac-address" property. Both can be
> ignored by the driver. (This interrupt can be needed if a more card-
> ventor-neutral name is found.)

I don't feel qualified to comment on this. If you want to propose such
a patch and fix above node accordingly, I won't object. Otherwise I'll
just send a patch to remove the subnode.

Thanks,
Harald

2018-06-25 07:44:51

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] arm64: allwinner: a64: Add support for TERES-I laptop

On 6/24/2018 6:34 PM, Harald Geyer wrote:
> Icenowy Zheng writes:
>> >在 2018-03-15四的 16:25 +0000,Harald Geyer写道:
>>> > >+&mmc1 {
>>> > >+ pinctrl-names = "default";
>>> > >+ pinctrl-0 = <&mmc1_pins>;
>>> > >+ vmmc-supply = <&reg_aldo2>;
>>> > >+ vqmmc-supply = <&reg_dldo4>;
>>> > >+ mmc-pwrseq = <&wifi_pwrseq>;
>>> > >+ bus-width = <4>;
>>> > >+ non-removable;
>>> > >+ status = "okay";
>>> > >+
>>> > >+ rtl8723bs: wifi@1 {
>>> > >+ reg = <1>;
>>> > >+ interrupt-parent = <&r_pio>;
>>> > >+ interrupts = <0 3 IRQ_TYPE_LEVEL_LOW>; /* PL3 */
>>> > >+ interrupt-names = "host-wake";
>>> > >+ };

[...]

>> >- This device node has no binding. The "host-wake" interrupt is part of
>> > Broadcom SDIO Wi-Fi binding, rather than a generic one.
> I think the general mmc and interrupts bindings apply. And the mmc binding
> clearly states that for sub-nodes a compatible string is optional.
>
> However I just realized that the 'interrupt-names' property is not part
> of the general interrupts binding, so I guess at least this property should
> be removed.

Indeed. If the device just used the SDIO interrupt this is not needed.
The Broadcom device can use either SDIO interrupt or a so-called
out-of-band host-wake interrupt, which is what the above represents.

Regards,
Arend

2018-06-25 07:53:55

by Icenowy Zheng

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] arm64: allwinner: a64: Add support for TERES-I laptop



于 2018年6月25日 GMT+08:00 下午3:43:51, Arend van Spriel <[email protected]> 写到:
>On 6/24/2018 6:34 PM, Harald Geyer wrote:
>> Icenowy Zheng writes:
>>> >在 2018-03-15四的 16:25 +0000,Harald Geyer写道:
>>>> > >+&mmc1 {
>>>> > >+ pinctrl-names = "default";
>>>> > >+ pinctrl-0 = <&mmc1_pins>;
>>>> > >+ vmmc-supply = <&reg_aldo2>;
>>>> > >+ vqmmc-supply = <&reg_dldo4>;
>>>> > >+ mmc-pwrseq = <&wifi_pwrseq>;
>>>> > >+ bus-width = <4>;
>>>> > >+ non-removable;
>>>> > >+ status = "okay";
>>>> > >+
>>>> > >+ rtl8723bs: wifi@1 {
>>>> > >+ reg = <1>;
>>>> > >+ interrupt-parent = <&r_pio>;
>>>> > >+ interrupts = <0 3 IRQ_TYPE_LEVEL_LOW>; /* PL3 */
>>>> > >+ interrupt-names = "host-wake";
>>>> > >+ };
>
>[...]
>
>>> >- This device node has no binding. The "host-wake" interrupt is
>part of
>>> > Broadcom SDIO Wi-Fi binding, rather than a generic one.
>> I think the general mmc and interrupts bindings apply. And the mmc
>binding
>> clearly states that for sub-nodes a compatible string is optional.
>>
>> However I just realized that the 'interrupt-names' property is not
>part
>> of the general interrupts binding, so I guess at least this property
>should
>> be removed.
>
>Indeed. If the device just used the SDIO interrupt this is not needed.
>The Broadcom device can use either SDIO interrupt or a so-called
>out-of-band host-wake interrupt, which is what the above represents.

RTL8....S is also capable of use OOB interrupt.

>
>Regards,
>Arend

2018-06-25 08:14:00

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] arm64: allwinner: a64: Add support for TERES-I laptop

On 6/25/2018 9:47 AM, Icenowy Zheng wrote:
>
>
> 于 2018年6月25日 GMT+08:00 下午3:43:51, Arend van Spriel <[email protected]> 写到:
>> On 6/24/2018 6:34 PM, Harald Geyer wrote:
>>> Icenowy Zheng writes:
>>>>> 在 2018-03-15四的 16:25 +0000,Harald Geyer写道:
>>>>>>> +&mmc1 {
>>>>>>> + pinctrl-names = "default";
>>>>>>> + pinctrl-0 = <&mmc1_pins>;
>>>>>>> + vmmc-supply = <&reg_aldo2>;
>>>>>>> + vqmmc-supply = <&reg_dldo4>;
>>>>>>> + mmc-pwrseq = <&wifi_pwrseq>;
>>>>>>> + bus-width = <4>;
>>>>>>> + non-removable;
>>>>>>> + status = "okay";
>>>>>>> +
>>>>>>> + rtl8723bs: wifi@1 {
>>>>>>> + reg = <1>;
>>>>>>> + interrupt-parent = <&r_pio>;
>>>>>>> + interrupts = <0 3 IRQ_TYPE_LEVEL_LOW>; /* PL3 */
>>>>>>> + interrupt-names = "host-wake";
>>>>>>> + };
>>
>> [...]
>>
>>>>> - This device node has no binding. The "host-wake" interrupt is
>> part of
>>>>> Broadcom SDIO Wi-Fi binding, rather than a generic one.
>>> I think the general mmc and interrupts bindings apply. And the mmc
>> binding
>>> clearly states that for sub-nodes a compatible string is optional.
>>>
>>> However I just realized that the 'interrupt-names' property is not
>> part
>>> of the general interrupts binding, so I guess at least this property
>> should
>>> be removed.
>>
>> Indeed. If the device just used the SDIO interrupt this is not needed.
>> The Broadcom device can use either SDIO interrupt or a so-called
>> out-of-band host-wake interrupt, which is what the above represents.
>
> RTL8....S is also capable of use OOB interrupt.

Ok. Is it also in-place in this TERES-I laptop? Anyway, if RTL8...S does
not have a binding specification there is not much to do about it. In my
opinion it does not make sense to add it to the generic mmc/sdio binding
as this interrupt does not involve the mmc/sdio hardware hence the term
OOB. There is generic wifi binding net/wireless/ieee80211.txt in which
this could be added. Obviously it would just be a binding and no
guarantee that the actual device driver supports it so the RTL driver
would need modification for that.

Regards,
Arend


2018-06-25 10:44:02

by Icenowy Zheng

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] arm64: allwinner: a64: Add support for TERES-I laptop



于 2018年6月25日 GMT+08:00 下午4:13:01, Arend van Spriel <[email protected]> 写到:
>On 6/25/2018 9:47 AM, Icenowy Zheng wrote:
>>
>>
>> 于 2018年6月25日 GMT+08:00 下午3:43:51, Arend van Spriel
><[email protected]> 写到:
>>> On 6/24/2018 6:34 PM, Harald Geyer wrote:
>>>> Icenowy Zheng writes:
>>>>>> 在 2018-03-15四的 16:25 +0000,Harald Geyer写道:
>>>>>>>> +&mmc1 {
>>>>>>>> + pinctrl-names = "default";
>>>>>>>> + pinctrl-0 = <&mmc1_pins>;
>>>>>>>> + vmmc-supply = <&reg_aldo2>;
>>>>>>>> + vqmmc-supply = <&reg_dldo4>;
>>>>>>>> + mmc-pwrseq = <&wifi_pwrseq>;
>>>>>>>> + bus-width = <4>;
>>>>>>>> + non-removable;
>>>>>>>> + status = "okay";
>>>>>>>> +
>>>>>>>> + rtl8723bs: wifi@1 {
>>>>>>>> + reg = <1>;
>>>>>>>> + interrupt-parent = <&r_pio>;
>>>>>>>> + interrupts = <0 3 IRQ_TYPE_LEVEL_LOW>; /* PL3 */
>>>>>>>> + interrupt-names = "host-wake";
>>>>>>>> + };
>>>
>>> [...]
>>>
>>>>>> - This device node has no binding. The "host-wake" interrupt is
>>> part of
>>>>>> Broadcom SDIO Wi-Fi binding, rather than a generic one.
>>>> I think the general mmc and interrupts bindings apply. And the mmc
>>> binding
>>>> clearly states that for sub-nodes a compatible string is optional.
>>>>
>>>> However I just realized that the 'interrupt-names' property is not
>>> part
>>>> of the general interrupts binding, so I guess at least this
>property
>>> should
>>>> be removed.
>>>
>>> Indeed. If the device just used the SDIO interrupt this is not
>needed.
>>> The Broadcom device can use either SDIO interrupt or a so-called
>>> out-of-band host-wake interrupt, which is what the above represents.
>>
>> RTL8....S is also capable of use OOB interrupt.
>
>Ok. Is it also in-place in this TERES-I laptop? Anyway, if RTL8...S

In fact it's a regexp here, mean Realtek SDIO WLAN NICs.

>does
>not have a binding specification there is not much to do about it. In
>my
>opinion it does not make sense to add it to the generic mmc/sdio
>binding
>as this interrupt does not involve the mmc/sdio hardware hence the term
>
>OOB. There is generic wifi binding net/wireless/ieee80211.txt in which

It seems ok. Maybe it can be used for all interfaces, not
SDIO, although I don't think there's any other interfaces
that can use OOB IRQ except SPI and SDIO, maybe UART? :-)

>this could be added. Obviously it would just be a binding and no
>guarantee that the actual device driver supports it so the RTL driver
>would need modification for that.

Yes. Currently OOB interrupt is not used at all.

>
>Regards,
>Arend