diff mbox

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

Message ID 20180315162510.11669-6-harald@ccbib.org (mailing list archive)
State New, archived
Headers show

Commit Message

Harald Geyer March 15, 2018, 4:25 p.m. UTC
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 <harald@ccbib.org>
---
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

Comments

afzal mohammed March 16, 2018, 6:37 a.m. UTC | #1
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 <harald@ccbib.org>

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 <afzal.mohd.ma@gmail.com>

afzal
afzal mohammed March 16, 2018, 5:50 p.m. UTC | #2
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
Maxime Ripard March 18, 2018, 8:22 p.m. UTC | #3
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
Harald Geyer March 19, 2018, 3:27 p.m. UTC | #4
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
Maxime Ripard March 20, 2018, 2:13 p.m. UTC | #5
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
Andre Przywara March 20, 2018, 5:09 p.m. UTC | #6
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.
Maxime Ripard March 22, 2018, 5:59 p.m. UTC | #7
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
Icenowy Zheng June 22, 2018, 4:27 p.m. UTC | #8
在 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 <harald@ccbib.org>
> ---
> 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 <harald@ccbib.org>
> + * based on sun50i-a64-olinuxino.dts by Jagan Teki <jteki@openedev.c
> 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";
> +};
Harald Geyer June 24, 2018, 4:34 p.m. UTC | #9
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
Arend van Spriel June 25, 2018, 7:43 a.m. UTC | #10
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
Icenowy Zheng June 25, 2018, 7:47 a.m. UTC | #11
于 2018年6月25日 GMT+08:00 下午3:43:51, Arend van Spriel <arend.vanspriel@broadcom.com> 写到:
>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
Arend van Spriel June 25, 2018, 8:13 a.m. UTC | #12
On 6/25/2018 9:47 AM, Icenowy Zheng wrote:
>
>
> 于 2018年6月25日 GMT+08:00 下午3:43:51, Arend van Spriel <arend.vanspriel@broadcom.com> 写到:
>> 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
Icenowy Zheng June 25, 2018, 10:42 a.m. UTC | #13
于 2018年6月25日 GMT+08:00 下午4:13:01, Arend van Spriel <arend.vanspriel@broadcom.com> 写到:
>On 6/25/2018 9:47 AM, Icenowy Zheng wrote:
>>
>>
>> 于 2018年6月25日 GMT+08:00 下午3:43:51, Arend van Spriel
><arend.vanspriel@broadcom.com> 写到:
>>> 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
diff mbox

Patch

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 <harald@ccbib.org>
+ * based on sun50i-a64-olinuxino.dts by Jagan Teki <jteki@openedev.com>
+ *
+ * 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";
+};