ARM: dts: exynos: Prepare Spring
Subsystems (5)
cover ARM: dts: exynos: Prepare Spring
From: Andreas Färber <[email protected]>
Hello,
Based on the preinstalled 3.8 based ChromeOS kernel and previous 3.15 based attempts by Stephan and me that broke for 3.16, I've prepared a device tree for the HP Chromebook 11 aka Google Spring.
The first three patches should be good to go and contain documentation fixes found while comparing exynos5250-snow.dts vs. /proc/device-tree.
The main patch was tested using a chained non-verified U-Boot (simplefb) and a rootfs on USB-attached SD card.
The display goes dark unfortunately (drm bridge series not yet tested), but I am able to log in via ssh over USB ethernet adapter okay.
Audio support is likely missing as my focus was getting USB booting. Not included is touchpad support, as "atmel,atmel_mxt_tp" is not documented to be available upstream. And no /dev/mmcblk0 or Wifi yet. Also when the screen stayed on, the embedded controller's keymap seems hardcoded to US English with system settings not taking effect; but surely we don't want per-keyboard device tree files to remedy that.
I'd appreciate if we can get a core .dts sorted out and consider incremental changes from there.
Regards,
Andreas
Andreas Färber (4):
Documentation: devicetree: Fix s2mps11 and s5m8767 typos
Documentation: devicetree: Fix s2mps11 example syntax
Documentation: devicetree: Fix tps65090 typos
ARM: dts: exynos5250: Add Spring device tree
Documentation/devicetree/bindings/mfd/s2mps11.txt | 4 +- .../bindings/regulator/s5m8767-regulator.txt | 2 +- .../devicetree/bindings/regulator/tps65090.txt | 4 +- arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/exynos5250-spring.dts | 556 +++++++++++++++++++++ 5 files changed, 562 insertions(+), 5 deletions(-) create mode 100644 arch/arm/boot/dts/exynos5250-spring.dts
Signature · 1 line
1.9.3
PATCH 1/4 Documentation: devicetree: Fix s2mps11 and s5m8767 typos
It's LDO2, not LD02.
--- Documentation/devicetree/bindings/mfd/s2mps11.txt | 2 +- Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/mfd/s2mps11.txt +1 -1
@@ -81,7 +81,7 @@ as per the datasheet of s2mps11. - valid values for n are: - S2MPS11: 1 to 38 - S2MPS14: 1 to 25 - - Example: LDO1, LD02, LDO28 + - Example: LDO1, LDO2, LDO28 - BUCKn - valid values for n are: - S2MPS11: 1 to 10
Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt +1 -1
@@ -86,7 +86,7 @@ as per the datasheet of s5m8767. - LDOn - valid values for n are 1 to 28 - - Example: LDO1, LD02, LDO28 + - Example: LDO1, LDO2, LDO28 - BUCKn - valid values for n are 1 to 9. - Example: BUCK1, BUCK2, BUCK9
Signature · 1 line
1.9.3
Re: [PATCH 1/4] Documentation: devicetree: Fix s2mps11 and s5m8767 typos
Quoted 26 lines · Andreas Färber
It's LDO2, not LD02.
Signed-off-by Andreas Färber <[email protected]>--- Documentation/devicetree/bindings/mfd/s2mps11.txt | 2 +- Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/mfd/s2mps11.txt +1
-1@@ -81,7 +81,7 @@ as per the datasheet of s2mps11. - valid values for n are: - S2MPS11: 1 to 38 - S2MPS14: 1 to 25 - - Example: LDO1, LD02, LDO28 + - Example: LDO1, LDO2, LDO28 - BUCKn - valid values for n are: - S2MPS11: 1 to 10Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt +0
-0
Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt as per the datasheet of s5m8767.
Quoted 9 lines
Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt +1
-2@@ -86,7 +86,7 @@ as per the datasheet of s5m8767. - LDOn - valid values for n are 1 to 28 - - Example: LDO1, LD02, LDO28 + - Example: LDO1, LDO2, LDO28 - BUCKn - valid values for n are 1 to 9. - Example: BUCK1, BUCK2, BUCK9 --
Very keen observation :)
Re: [PATCH 1/4] Documentation: devicetree: Fix s2mps11 and s5m8767 typos
Am 23.06.2014 05:21, schrieb Sachin Kamat:
Quoted 39 lines · Andreas Färber
Quoted 26 lines
It's LDO2, not LD02.
Signed-off-by Andreas Färber <[email protected]>--- Documentation/devicetree/bindings/mfd/s2mps11.txt | 2 +- Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/mfd/s2mps11.txt +1
-1@@ -81,7 +81,7 @@ as per the datasheet of s2mps11. - valid values for n are: - S2MPS11: 1 to 38 - S2MPS14: 1 to 25 - - Example: LDO1, LD02, LDO28 + - Example: LDO1, LDO2, LDO28 - BUCKn - valid values for n are: - S2MPS11: 1 to 10Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt +0
-0Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt as per the datasheet of s5m8767.
Quoted 9 lines
Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt +1
-2@@ -86,7 +86,7 @@ as per the datasheet of s5m8767. - LDOn - valid values for n are 1 to 28 - - Example: LDO1, LD02, LDO28 + - Example: LDO1, LDO2, LDO28 - BUCKn - valid values for n are 1 to 9. - Example: BUCK1, BUCK2, BUCK9 --Very keen observation :)
Quoted 1 line
Reviewed-by Sachin Kamat <[email protected]>
A font that distinguishes the zero with a dot or dash helps! :)
I was wondering which character to type, and found two undocumented s5m8767_pmic properties downstream (s5m-core,enable-low-jitter and s5m-core,device_type = <0x2>), which I then left out.
Cheers,
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [PATCH 1/4] Documentation: devicetree: Fix s2mps11 and s5m8767 typos
Andreas,
Quoted 3 lines · Andreas Färber
I was wondering which character to type, and found two undocumented
s5m8767_pmicproperties downstream (s5m-core,enable-low-jitter and s5m-core,device_type = <0x2>), which I then left out.
I don't know much about "s5m-core,device_type", but I doubt it's needed. You can see <http://crosreview.com/42202> for details. I haven't looked but I'd bet that we just get this from the compatible string now.
I did do a (very!) quick look and I see that low-jitter was originally implemented in the local 3.4 kernel at <http://crosreview.com/43624>.
...and the local 3.8 kernel at <http://crosreview.com/66037>.
NOTE: it's pretty important to make sure low-jitter is turned on for Chromebooks if you actually want full functionality. At least on exynos5250-snow (with the max77686 PMIC) you'd get occasional (and very strange and very hard to debug) TPM errors if you didn't have low-jitter. The TPM is part of the security model on Chromebooks and you might have a hard time accessing the encrypted parts of the disk without it.
-Doug
Lee Jones
AppliedRe: [PATCH 1/4] Documentation: devicetree: Fix s2mps11 and s5m8767 typos
In reply to Andreas FärberQuoted 7 lines · Andreas Färber
It's LDO2, not LD02.
Signed-off-by Andreas Färber <[email protected]>--- Documentation/devicetree/bindings/mfd/s2mps11.txt | 2 +- Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Applied, thanks.
Quoted 26 lines
Documentation/devicetree/bindings/mfd/s2mps11.txt +1
-1@@ -81,7 +81,7 @@ as per the datasheet of s2mps11. - valid values for n are: - S2MPS11: 1 to 38 - S2MPS14: 1 to 25 - - Example: LDO1, LD02, LDO28 + - Example: LDO1, LDO2, LDO28 - BUCKn - valid values for n are: - S2MPS11: 1 to 10Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt +1
-1@@ -86,7 +86,7 @@ as per the datasheet of s5m8767. - LDOn - valid values for n are 1 to 28 - - Example: LDO1, LD02, LDO28 + - Example: LDO1, LDO2, LDO28 - BUCKn - valid values for n are 1 to 9. - Example: BUCK1, BUCK2, BUCK9
Signature · 4 lines
Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
PATCH 2/4 Documentation: devicetree: Fix s2mps11 example syntax
In reply to Andreas FärberIt's <1>, not 1.
--- Documentation/devicetree/bindings/mfd/s2mps11.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/devicetree/bindings/mfd/s2mps11.txt +1 -1
@@ -96,7 +96,7 @@ Example: s2m_osc: clocks { compatible = "samsung,s2mps11-clk"; - #clock-cells = 1; + #clock-cells = <1>; clock-output-names = "xx", "yy", "zz"; };
Signature · 1 line
1.9.3
Re: [PATCH 2/4] Documentation: devicetree: Fix s2mps11 example syntax
Quoted 21 lines · Andreas Färber
It's <1>, not 1.
Signed-off-by Andreas Färber <[email protected]>--- Documentation/devicetree/bindings/mfd/s2mps11.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Documentation/devicetree/bindings/mfd/s2mps11.txt +1
-2@@ -96,7 +96,7 @@ Example: s2m_osc: clocks { compatible = "samsung,s2mps11-clk"; - #clock-cells = 1; + #clock-cells = <1>; clock-output-names = "xx", "yy", "zz"; }; --
Quoted 2 lines
1.9.3
Lee Jones
AppliedRe: [PATCH 2/4] Documentation: devicetree: Fix s2mps11 example syntax
In reply to Andreas FärberQuoted 6 lines · Andreas Färber
It's <1>, not 1.
Signed-off-by Andreas Färber <[email protected]>--- Documentation/devicetree/bindings/mfd/s2mps11.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied, thanks.
Quoted 13 lines
Documentation/devicetree/bindings/mfd/s2mps11.txt +1
-1@@ -96,7 +96,7 @@ Example: s2m_osc: clocks { compatible = "samsung,s2mps11-clk"; - #clock-cells = 1; + #clock-cells = <1>; clock-output-names = "xx", "yy", "zz"; };
Signature · 4 lines
Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
PATCH 3/4 Documentation: devicetree: Fix tps65090 typos
In reply to Andreas FärberIt's vsys-l{1,2}-supply, not vsys_l{1,2}-supply.
--- Documentation/devicetree/bindings/regulator/tps65090.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/regulator/tps65090.txt +2 -2
@@ -45,8 +45,8 @@ Example: infet5-supply = <&some_reg>; infet6-supply = <&some_reg>; infet7-supply = <&some_reg>; - vsys_l1-supply = <&some_reg>; - vsys_l2-supply = <&some_reg>; + vsys-l1-supply = <&some_reg>; + vsys-l2-supply = <&some_reg>; regulators { dcdc1 {
Signature · 1 line
1.9.3
Re: [PATCH 3/4] Documentation: devicetree: Fix tps65090 typos
Andreas,
Quoted 12 lines · Andreas Färber
It's vsys-l{1,2}-supply, not vsys_l{1,2}-supply.
Signed-off-by Andreas Färber <[email protected]>--- Documentation/devicetree/bindings/regulator/tps65090.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/regulator/tps65090.txt +0
-0
Documentation/devicetree/bindings/regulator/tps65090.txt Example: · line 49
Quoted 7 lines
Documentation/devicetree/bindings/regulator/tps65090.txt +2
-2@@ -45,8 +45,8 @@ Example: infet5-supply = <&some_reg>; infet6-supply = <&some_reg>; infet7-supply = <&some_reg>; - vsys_l1-supply = <&some_reg>; - vsys_l2-supply = <&some_reg>; + vsys-l1-supply = <&some_reg>; + vsys-l2-supply = <&some_reg>;
Your change matches the code and all existing device trees in the Linux kernel. I also see plenty of other bindings with dashes, so this seems reasonable.
Mark Rutland
Acked-byRe: [PATCH 3/4] Documentation: devicetree: Fix tps65090 typos
Quoted 10 lines · Doug Anderson
Andreas,
Quoted 7 lines · Andreas Färber
It's vsys-l{1,2}-supply, not vsys_l{1,2}-supply.
Signed-off-by Andreas Färber <[email protected]>--- Documentation/devicetree/bindings/regulator/tps65090.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Quoted 15 lines
Documentation/devicetree/bindings/regulator/tps65090.txt Example: · line 49
Quoted 12 lines
Documentation/devicetree/bindings/regulator/tps65090.txt +2
-2@@ -45,8 +45,8 @@ Example: infet5-supply = <&some_reg>; infet6-supply = <&some_reg>; infet7-supply = <&some_reg>; - vsys_l1-supply = <&some_reg>; - vsys_l2-supply = <&some_reg>; + vsys-l1-supply = <&some_reg>; + vsys-l2-supply = <&some_reg>;Your change matches the code and all existing device trees in the Linux kernel.
Could this fact please be mentioned in the commit message?
Given that:
Quoted 2 lines
I also see plenty of other bindings with dashes, so this seems reasonable.
Dashes rather than underscores are preferred/correct for property names and compatible strings. Given no-one can possibly be using the bad/incorrect form with underscores, fixing the documentation to use dashes makes sense.
Thanks,
Mark.
Re: [PATCH 3/4] Documentation: devicetree: Fix tps65090 typos
Am 25.06.2014 12:47, schrieb Mark Rutland:
Quoted 27 lines · Doug Anderson
Quoted 10 lines
Andreas,
Quoted 7 lines · Andreas Färber
It's vsys-l{1,2}-supply, not vsys_l{1,2}-supply.
Signed-off-by Andreas Färber <[email protected]>--- Documentation/devicetree/bindings/regulator/tps65090.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)Quoted 15 lines
Documentation/devicetree/bindings/regulator/tps65090.txt Example: · line 49
Quoted 12 lines
Documentation/devicetree/bindings/regulator/tps65090.txt +2
-2@@ -45,8 +45,8 @@ Example: infet5-supply = <&some_reg>; infet6-supply = <&some_reg>; infet7-supply = <&some_reg>; - vsys_l1-supply = <&some_reg>; - vsys_l2-supply = <&some_reg>; + vsys-l1-supply = <&some_reg>; + vsys-l2-supply = <&some_reg>;Your change matches the code and all existing device trees in the Linux kernel.
Quoted 1 line
Could this fact please be mentioned in the commit message?
Yes, I admit the commit message could've been clearer in stating that only the example is modified, not the actual specification. What about:
"Specification and existing device trees use vsys-l{1,2}-supply, not vsys_l{1,2}-supply. Fix the example to match the specification."
Maybe also "... typos in example" in the subject.
Let me know whether I should send a v2 or let maintainers fix it up.
Regards,
Andreas
Quoted 14 lines
Given that:
Acked-by Mark Rutland <[email protected]>Quoted 2 lines
I also see plenty of other bindings with dashes, so this seems reasonable.
Dashes rather than underscores are preferred/correct for property names and compatible strings. Given no-one can possibly be using the bad/incorrect form with underscores, fixing the documentation to use dashes makes sense.
Thanks,
Mark.
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [PATCH 3/4] Documentation: devicetree: Fix tps65090 typos
Quoted 33 lines · Andreas Färber
Am 25.06.2014 12:47, schrieb Mark Rutland:
Quoted 27 lines · Doug Anderson
Quoted 10 lines
Andreas,
Quoted 7 lines · Andreas Färber
It's vsys-l{1,2}-supply, not vsys_l{1,2}-supply.
Signed-off-by Andreas Färber <[email protected]>--- Documentation/devicetree/bindings/regulator/tps65090.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)Quoted 15 lines
Documentation/devicetree/bindings/regulator/tps65090.txt Example: · line 49
Quoted 12 lines
Documentation/devicetree/bindings/regulator/tps65090.txt +2
-2@@ -45,8 +45,8 @@ Example: infet5-supply = <&some_reg>; infet6-supply = <&some_reg>; infet7-supply = <&some_reg>; - vsys_l1-supply = <&some_reg>; - vsys_l2-supply = <&some_reg>; + vsys-l1-supply = <&some_reg>; + vsys-l2-supply = <&some_reg>;Your change matches the code and all existing device trees in the Linux kernel.
Quoted 1 line
Could this fact please be mentioned in the commit message?
Yes, I admit the commit message could've been clearer in stating that only the example is modified, not the actual specification. What about:
Quoted 6 lines
"Specification and existing device trees use vsys-l{1,2}-supply, not vsys_l{1,2}-supply. Fix the example to match the specification."
Maybe also "... typos in example" in the subject.
Let me know whether I should send a v2 or let maintainers fix it up.
Please send v2.
Rob
RFC 4/4 ARM: dts: exynos5250: Add Spring device tree
In reply to Andreas FärberAdds initial support for the HP Chromebook 11.
--- arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/exynos5250-spring.dts | 556 ++++++++++++++++++++++++++++++++ 2 files changed, 557 insertions(+) create mode 100644 arch/arm/boot/dts/exynos5250-spring.dts
arch/arm/boot/dts/Makefile +1 -0
@@ -74,6 +74,7 @@ dtb-$(CONFIG_ARCH_EXYNOS) += exynos4210-origen.dtb \ exynos5250-arndale.dtb \ exynos5250-smdk5250.dtb \ exynos5250-snow.dtb \ + exynos5250-spring.dtb \ exynos5260-xyref5260.dtb \ exynos5410-smdk5410.dtb \ exynos5420-arndale-octa.dtb \
arch/arm/boot/dts/exynos5250-spring.dts new file +556 -0
556 lines not on this page · raw message
Signature · 1 line
1.9.3
Re: [RFC 4/4] ARM: dts: exynos5250: Add Spring device tree
Andreas,
Thanks for posting! A first pass on this is below...
Quoted 34 lines · Andreas F?rber
Adds initial support for the HP Chromebook 11.
Cc Vincent Palatin <[email protected]>Cc Doug Anderson <[email protected]>Cc Stephan van Schaik <[email protected]>Signed-off-by Andreas F?rber <[email protected]>--- arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/exynos5250-spring.dts | 556 ++++++++++++++++++++++++++++++++ 2 files changed, 557 insertions(+) create mode 100644 arch/arm/boot/dts/exynos5250-spring.dtsarch/arm/boot/dts/Makefile +1
-0@@ -74,6 +74,7 @@ dtb-$(CONFIG_ARCH_EXYNOS) += exynos4210-origen.dtb \ exynos5250-arndale.dtb \ exynos5250-smdk5250.dtb \ exynos5250-snow.dtb \ + exynos5250-spring.dtb \ exynos5260-xyref5260.dtb \ exynos5410-smdk5410.dtb \ exynos5420-arndale-octa.dtb \arch/arm/boot/dts/exynos5250-spring.dts new file +4
-0@@ -0,0 +1,556 @@ +/* + * Google Spring board device tree source + * + * Copyright (c) 2013 Google, Inc
arch/arm/boot/dts/exynos5250-spring.dts line 14
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ * Copyright (c) 2014 SUSE LINUX Products GmbH + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +/dts-v1/; +#include "exynos5250.dtsi" +#include "exynos5250-cros-common.dtsi"
It is possible we may want to backpedal on the use of "exynos5250-cros-common.dtsi". I know that Olof (now CCed) said he wasn't a fan of how it turned out.
The original idea was that it should include the arbitrary set of things that are common between a chunk of Chrome OS boards. As more boards were introduced things would need to migrate from the "common" file to the board files.
At the moment the current conventional wisdom is that some duplication is better than the confusing movement of everything back and forth. See exynos5420-peach-pit and exynos5800-peach-pi in ToT linux-next.
arch/arm/boot/dts/exynos5250-spring.dts line 20
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+/ { + model = "Google Spring"; + compatible = "google,spring", "samsung,exynos5250", "samsung,exynos5"; + + pinctrl at 11400000 {
The new best way to do things is to put this down at the bottom. See exynos5420-peach-pit as an example:
&pinctrl_0 {
...
}
Note that I believe it was decided that top-level references like that should be sorted alphabetically.
If you wanted to apply that run to exynos5250-snow I don't think it would be a terrible idea.
Quoted 33 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +33
-0+ s5m8767_dvs: s5m8767-dvs { + samsung,pins = "gpd1-0", "gpd1-1", "gpd1-2"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>; + }; + + s5m8767_ds: s5m8767-ds { + samsung,pins = "gpx2-3", "gpx2-4", "gpx2-5"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>; + }; + + tps65090_irq: tps65090-irq { + samsung,pins = "gpx2-6"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + s5m8767_irq: s5m8767-irq { + samsung,pins = "gpx3-2"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + hdmi_hpd_irq: hdmi-hpd-irq { + samsung,pins = "gpx3-7"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>;
arch/arm/boot/dts/exynos5250-spring.dts line 63
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ }; + }; + + pinctrl at 13400000 { + hsic_reset: hsic-reset { + samsung,pins = "gpe1-0"; + samsung,pin-function = <1>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + };
I'm pretty sure that the HSIC reset needed some funky code to make it work and I'm not sure what the status of that is upstream
Quoted 37 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +37
-0+ }; + + vbat: vbat-fixed-regulator { + compatible = "regulator-fixed"; + regulator-name = "vbat-supply"; + regulator-boot-on; + }; + + usb at 12000000 { + status = "okay"; + }; + + usb3_vbus_reg: regulator-usb3 { + compatible = "regulator-fixed"; + regulator-name = "P5.0V_USB3CON"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + gpio = <&gpe1 0 1>; + pinctrl-names = "default"; + pinctrl-0 = <&hsic_reset>; + enable-active-high; + }; + + phy at 12100000 { + vbus-supply = <&usb3_vbus_reg>; + }; + + usb at 12110000 { + samsung,vbus-gpio = <&gpx1 1 0>; + status = "okay"; + }; + + usb at 12120000 { + status = "okay"; + }; + + mmc at 12220000 {
arch/arm/boot/dts/exynos5250-spring.dts line 110
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ /* MMC2 pins are used as GPIO for eDP bridge control. */ + status = "disabled"; + }; + + mmc at 12230000 { + status = "disabled"; + }; + + i2c at 12C60000 { + max77686 at 09 {
There is no max77686 on spring. It uses s5m8767 in the place of max77686.
...you've got "status = disabled", just remove it.
Quoted 15 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +15
-0+ #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + + rtc { + reg = <0x6>; + }; + }; + + s5m8767_pmic at 66 { + compatible = "samsung,s5m8767-pmic"; + reg = <0x66>; + interrupt-parent = <&gpx3>; + interrupts = <2 0>; + pinctrl-names = "default";
arch/arm/boot/dts/exynos5250-spring.dts line 135
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ pinctrl-0 = <&s5m8767_irq &s5m8767_dvs &s5m8767_ds>; + wakeup-source; + + s5m8767,pmic-buck-dvs-gpios = <&gpd1 0 1>, /* DVS1 */ + <&gpd1 1 1>, /* DVS2 */ + <&gpd1 2 1>; /* DVS3 */ + + s5m8767,pmic-buck-ds-gpios = <&gpx2 3 1>, /* SET1 */ + <&gpx2 4 1>, /* SET2 */ + <&gpx2 5 1>; /* SET3 */
The final "1" in each of the GPIO specifiers above should be GPIO_ACTIVE_LOW.
Quoted 26 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +26
-0+ + /* + * The following arrays of DVS voltages are not used, since we are + * not using GPIOs to control PMIC bucks, but they must be defined + * to please the driver. + */ + s5m8767,pmic-buck2-dvs-voltage = <1350000>, <1300000>, + <1250000>, <1200000>, + <1150000>, <1100000>, + <1000000>, <950000>; + + s5m8767,pmic-buck3-dvs-voltage = <1100000>, <1100000>, + <1100000>, <1100000>, + <1000000>, <1000000>, + <1000000>, <1000000>; + + s5m8767,pmic-buck4-dvs-voltage = <1200000>, <1200000>, + <1200000>, <1200000>, + <1200000>, <1200000>, + <1200000>, <1200000>; + + clocks { + compatible = "samsung,s5m8767-clk"; + #clock-cells = <1>; + clock-output-names = "en32khz_ap", + "en32khz_cp",
arch/arm/boot/dts/exynos5250-spring.dts line 171
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ "en32khz_bt"; + }; + + regulators { + s5m_ldo4_reg: LDO4 { + regulator-name = "P1.0V_LDO_OUT4"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <0>;
I think that "op_mode" here is questionable. Adding Javier to the thread who was looking at this for max77802 and possibly max77686.
Quoted 164 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +164
-0+ }; + + s5m_ldo5_reg: LDO5 { + regulator-name = "P1.0V_LDO_OUT5"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo6_reg: LDO6 { + regulator-name = "vdd_mydp"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo7_reg: LDO7 { + regulator-name = "P1.1V_LDO_OUT7"; + regulator-min-microvolt = <1100000>; + regulator-max-microvolt = <1100000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo8_reg: LDO8 { + regulator-name = "P1.0V_LDO_OUT8"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo10_reg: LDO10 { + regulator-name = "P1.8V_LDO_OUT10"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo11_reg: LDO11 { + regulator-name = "P1.8V_LDO_OUT11"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo12_reg: LDO12 { + regulator-name = "P3.0V_LDO_OUT12"; + regulator-min-microvolt = <3000000>; + regulator-max-microvolt = <3000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo13_reg: LDO13 { + regulator-name = "P1.8V_LDO_OUT13"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo14_reg: LDO14 { + regulator-name = "P1.8V_LDO_OUT14"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo15_reg: LDO15 { + regulator-name = "P1.0V_LDO_OUT15"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo16_reg: LDO16 { + regulator-name = "P1.8V_LDO_OUT16"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo17_reg: LDO17 { + regulator-name = "P2.8V_LDO_OUT17"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo25_reg: LDO25 { + regulator-name = "vdd_bridge"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + op_mode = <1>; + }; + + BUCK1 { + regulator-name = "vdd_mif"; + regulator-min-microvolt = <950000>; + regulator-max-microvolt = <1300000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK2 { + regulator-name = "vdd_arm"; + regulator-min-microvolt = <850000>; + regulator-max-microvolt = <1350000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK3 { + regulator-name = "vdd_int"; + regulator-min-microvolt = <900000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK4 { + regulator-name = "vdd_g3d"; + regulator-min-microvolt = <850000>; + regulator-max-microvolt = <1300000>; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK5 { + regulator-name = "P1.8V_BUCK_OUT5"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + regulator-boot-on; + op_mode = <1>; + }; + + BUCK6 { + regulator-name = "P1.2V_BUCK_OUT6"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + regulator-boot-on; + op_mode = <0>; + }; + + BUCK9 { + regulator-name = "vdd_ummc"; + regulator-min-microvolt = <950000>; + regulator-max-microvolt = <3000000>; + regulator-always-on;
arch/arm/boot/dts/exynos5250-spring.dts line 345
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ regulator-boot-on; + op_mode = <3>; + }; + }; + }; + }; + + i2c at 12C70000 { + trackpad { + status = "disabled";
Having this bogus entry here doesn't add anything. Remove it until the trackpad should be added. See http://crbug.com/371114 for a slightly stale bug about trackpad.
arch/arm/boot/dts/exynos5250-spring.dts line 350
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ }; + }; + + i2c at 12CA0000 { + embedded-controller {
Add "cros_ec" alias like snow.
arch/arm/boot/dts/exynos5250-spring.dts line 357
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+ compatible = "google,cros-ec-i2c"; + reg = <0x1e>; + interrupts = <6 0>; + interrupt-parent = <&gpx1>; + wakeup-source; + + keyboard-controller {
Don't include keyboard-controller here. Add:
#include "cros-ec-keyboard.dtsi"
...at the bottom. Note that I think that the spring EC has a special "charger" key that it uses to indicate when a charger was plugged in and unplugged. I'm not sure how that will end up getting supported upstream but you could just leave it out for now.
arch/arm/boot/dts/exynos5250-spring.dts line 360
Quoted 3 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +3
-0+ compatible = "google,cros-ec-keyb"; + keypad,num-rows = <8>; + keypad,num-columns = <13>;
Don't you need pinctrl here?
Quoted 80 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +80
-0+ google,needs-ghost-filter; + linux,keymap = < + 0x0001007d /* L_META */ + 0x0002003b /* F1 */ + 0x00030030 /* B */ + 0x00040044 /* F10 */ + 0x00060031 /* N */ + 0x0008000d /* = */ + 0x000a0064 /* R_ALT */ + + 0x01010001 /* ESC */ + 0x0102003e /* F4 */ + 0x01030022 /* G */ + 0x01040041 /* F7 */ + 0x01060023 /* H */ + 0x01080028 /* ' */ + 0x01090043 /* F9 */ + 0x010b000e /* BKSPACE */ + + 0x0200001d /* L_CTRL */ + 0x0201000f /* TAB */ + 0x0202003d /* F3 */ + 0x02030014 /* T */ + 0x02040040 /* F6 */ + 0x0205001b /* ] */ + 0x02060015 /* Y */ + 0x02070056 /* 102ND */ + 0x0208001a /* [ */ + 0x02090042 /* F8 */ + + 0x03010029 /* GRAVE */ + 0x0302003c /* F2 */ + 0x03030006 /* 5 */ + 0x0304003f /* F5 */ + 0x03060007 /* 6 */ + 0x0308000c /* - */ + 0x030b002b /* \ */ + + 0x04000061 /* R_CTRL */ + 0x0401001e /* A */ + 0x04020020 /* D */ + 0x04030021 /* F */ + 0x0404001f /* S */ + 0x04050025 /* K */ + 0x04060024 /* J */ + 0x04080027 /* ; */ + 0x04090026 /* L */ + 0x040a002b /* \ */ + 0x040b001c /* ENTER */ + + 0x0501002c /* Z */ + 0x0502002e /* C */ + 0x0503002f /* V */ + 0x0504002d /* X */ + 0x05050033 /* , */ + 0x05060032 /* M */ + 0x0507002a /* L_SHIFT */ + 0x05080035 /* / */ + 0x05090034 /* . */ + 0x050B0039 /* SPACE */ + + 0x06010002 /* 1 */ + 0x06020004 /* 3 */ + 0x06030005 /* 4 */ + 0x06040003 /* 2 */ + 0x06050009 /* 8 */ + 0x06060008 /* 7 */ + 0x0608000b /* 0 */ + 0x0609000a /* 9 */ + 0x060a0038 /* L_ALT */ + 0x060b006c /* DOWN */ + 0x060c006a /* RIGHT */ + + 0x07010010 /* Q */ + 0x07020012 /* E */ + 0x07030013 /* R */ + 0x07040011 /* W */ + 0x07050017 /* I */ + 0x07060016 /* U */ + 0x07070036 /* R_SHIFT */
arch/arm/boot/dts/exynos5250-spring.dts line 450
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ 0x07080019 /* P */ + 0x07090018 /* O */ + 0x070b0067 /* UP */ + 0x070c0069 /* LEFT */ + >; + }; + }; + + power-regulator { + compatible = "ti,tps65090";
I doubt tps65090 will "just work". Does it?
On spring the tps65090 is not directly on the same i2c bus as the EC. It's actually hidden behind the EC.
Locally in the ChromeOS tree there appears to be a forked copy of the 65090 regulator driver that's in use just for spring. See this from the ChromeOS 3.8 tree:
./drivers/regulator/tps65090-regulator.c./drivers/regulator/cros_ec-tps65090.c
The Spring version of the driver sends EC commands directly to access the tps65090.
It is possible (untested) that you could also talk to tps65090 over an i2c tunnel. On exynos5420-peach-pit we have a full fledged i2c tunnel, but you may be able to extend the tunnel to export an i2c tunnel for spring using something like
<https://chromium-review.googlesource.com/66116>
Quoted 64 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +64
-0+ reg = <0x48>; + + /* + * Config irq to disable internal pulls + * even though we run in polling mode. + */ + pinctrl-names = "default"; + pinctrl-0 = <&tps65090_irq>; + + vsys1-supply = <&vbat>; + vsys2-supply = <&vbat>; + vsys3-supply = <&vbat>; + infet1-supply = <&vbat>; + infet2-supply = <&vbat>; + infet3-supply = <&vbat>; + infet4-supply = <&vbat>; + infet5-supply = <&vbat>; + infet6-supply = <&vbat>; + infet7-supply = <&vbat>; + vsys-l1-supply = <&vbat>; + vsys-l2-supply = <&vbat>; + + regulators { + fet1 { + regulator-name = "vcd_led"; + regulator-min-microvolt = <12000000>; + regulator-max-microvolt = <12000000>; + }; + fet3 { + regulator-name = "wwan_r"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + fet5 { + regulator-name = "cam"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + fet6 { + regulator-name = "lcd_vdd"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + }; + fet7 { + regulator-name = "ts"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + }; + }; + + charger { + compatible = "ti,tps65090-charger"; + }; + }; + }; + + hdmi { + hdmi-en-supply = <&s5m_ldo8_reg>; + vdd-supply = <&s5m_ldo8_reg>; + vdd_osc-supply = <&s5m_ldo10_reg>; + vdd_pll-supply = <&s5m_ldo8_reg>; + };
arch/arm/boot/dts/exynos5250-spring.dts line 524
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ + fimd at 14400000 { + status = "okay"; + samsung,invert-vclk; + }; + + dp-controller at 145B0000 { + status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&dp_hpd>;
This is probably not right. It looks as if spring uses gpc3-0 for display port HPD (as a GPIO). The upstream has this in the exynos5250-pinctrl.dtsi as a different pin.
I think you'll need to define your own pinctrl here.
Quoted 22 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +22
-0+ samsung,color-space = <0>; + samsung,dynamic-range = <0>; + samsung,ycbcr-coeff = <0>; + samsung,color-depth = <1>; + samsung,link-rate = <0x0a>; + samsung,lane-count = <1>; + samsung,hpd-gpio = <&gpc3 0 0>; + + display-timings { + native-mode = <&timing1>; + + timing1: timing at 1 { + clock-frequency = <70589280>; + hactive = <1366>; + vactive = <768>; + hfront-porch = <40>; + hback-porch = <40>; + hsync-len = <32>; + vback-porch = <10>; + vfront-porch = <12>; + vsync-len = <6>; + };
arch/arm/boot/dts/exynos5250-spring.dts line 556
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ }; + }; + + fixed-rate-clocks { + xxti { + compatible = "samsung,clock-xxti"; + clock-frequency = <24000000>; + }; + }; +};
-Doug
Re: [RFC 4/4] ARM: dts: exynos5250: Add Spring device tree
Hi Doug,
Am 23.06.2014 21:47, schrieb Doug Anderson:
Quoted 1 line
Thanks for posting! A first pass on this is below...
Thanks a lot for your quick review! My first big .dts patch, and no datasheets for the hardware at hand as a user.
A first pass of replies to my defense. ;)
Quoted 1 line · Andreas F?rber
[...]
Quoted 44 lines
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +4
-0@@ -0,0 +1,556 @@ +/* + * Google Spring board device tree source + * + * Copyright (c) 2013 Google, Incarch/arm/boot/dts/exynos5250-spring.dts line 14
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ * Copyright (c) 2014 SUSE LINUX Products GmbH + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +/dts-v1/; +#include "exynos5250.dtsi" +#include "exynos5250-cros-common.dtsi"It is possible we may want to backpedal on the use of "
exynos5250-cros-common.dtsi". I know that Olof (now CCed) said he wasn't a fan of how it turned out.The original idea was that it should include the arbitrary set of things that are common between a chunk of Chrome OS boards. As more boards were introduced things would need to migrate from the "common" file to the board files.
At the moment the current conventional wisdom is that some duplication is better than the confusing movement of everything back and forth. See exynos5420-peach-pit and exynos5800-peach-pi in ToT linux-next.
arch/arm/boot/dts/exynos5250-spring.dts line 20
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+/ { + model = "Google Spring"; + compatible = "google,spring", "samsung,exynos5250", "samsung,exynos5"; + + pinctrl at 11400000 {The new best way to do things is to put this down at the bottom. See exynos5420-peach-pit as an example:
Quoted 6 lines
&pinctrl_0{
...
}Note that I believe it was decided that top-level references like that should be sorted alphabetically.
Thanks for the hint. (My chosen sort order here was by address.)
Quoted 2 lines
If you wanted to apply that run to exynos5250-snow I don't think it would be a terrible idea.
I can of course apply changes to Snow, but I cannot test them myself.
Quoted 38 lines
Quoted 38 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +38
-0+ s5m8767_dvs: s5m8767-dvs { + samsung,pins = "gpd1-0", "gpd1-1", "gpd1-2"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>; + }; + + s5m8767_ds: s5m8767-ds { + samsung,pins = "gpx2-3", "gpx2-4", "gpx2-5"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>; + }; + + tps65090_irq: tps65090-irq { + samsung,pins = "gpx2-6"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + s5m8767_irq: s5m8767-irq { + samsung,pins = "gpx3-2"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + hdmi_hpd_irq: hdmi-hpd-irq { + samsung,pins = "gpx3-7"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>; + }; + }; + + pinctrl at 13400000 { + hsic_reset: hsic-reset {
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 63
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ samsung,pins = "gpe1-0"; + samsung,pin-function = <1>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + };I'm pretty sure that the HSIC reset needed some funky code to make it work and I'm not sure what the status of that is upstream
Yeah, you mentioned something along those lines. However it's the equivalent of the usb3-vbus-en in -snow.dts. Rename or drop?
Quoted 50 lines
Quoted 37 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +37
-0+ }; + + vbat: vbat-fixed-regulator { + compatible = "regulator-fixed"; + regulator-name = "vbat-supply"; + regulator-boot-on; + }; + + usb at 12000000 { + status = "okay"; + }; + + usb3_vbus_reg: regulator-usb3 { + compatible = "regulator-fixed"; + regulator-name = "P5.0V_USB3CON"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + gpio = <&gpe1 0 1>; + pinctrl-names = "default"; + pinctrl-0 = <&hsic_reset>; + enable-active-high; + }; + + phy at 12100000 { + vbus-supply = <&usb3_vbus_reg>; + }; + + usb at 12110000 { + samsung,vbus-gpio = <&gpx1 1 0>; + status = "okay"; + }; + + usb at 12120000 { + status = "okay"; + }; + + mmc at 12220000 {arch/arm/boot/dts/exynos5250-spring.dts line 110
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ /* MMC2 pins are used as GPIO for eDP bridge control. */ + status = "disabled"; + }; + + mmc at 12230000 { + status = "disabled"; + }; + + i2c at 12C60000 { + max77686 at 09 {There is no max77686 on spring. It uses s5m8767 in the place of max77686.
Quoted 1 line
...you've got "status = disabled", just remove it.
That's because it's inherited from exynos5250-cros-common.dtsi.
The rtc that the system successfully uses is "s5m-rtc", which I guess is on the s5m8767 then? (Confusing that 3.8 Spring had this rtc node despite Snow's max77686 not having it.)
Quoted 60 lines
Quoted 15 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +15
-0+ #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + + rtc { + reg = <0x6>; + }; + }; + + s5m8767_pmic at 66 { + compatible = "samsung,s5m8767-pmic"; + reg = <0x66>; + interrupt-parent = <&gpx3>; + interrupts = <2 0>; + pinctrl-names = "default";arch/arm/boot/dts/exynos5250-spring.dts line 135
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ pinctrl-0 = <&s5m8767_irq &s5m8767_dvs &s5m8767_ds>; + wakeup-source; + + s5m8767,pmic-buck-dvs-gpios = <&gpd1 0 1>, /* DVS1 */ + <&gpd1 1 1>, /* DVS2 */ + <&gpd1 2 1>; /* DVS3 */ + + s5m8767,pmic-buck-ds-gpios = <&gpx2 3 1>, /* SET1 */ + <&gpx2 4 1>, /* SET2 */ + <&gpx2 5 1>; /* SET3 */The final "1" in each of the GPIO specifiers above should be
GPIO_ACTIVE_LOW.Quoted 31 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +31
-0+ + /* + * The following arrays of DVS voltages are not used, since we are + * not using GPIOs to control PMIC bucks, but they must be defined + * to please the driver. + */ + s5m8767,pmic-buck2-dvs-voltage = <1350000>, <1300000>, + <1250000>, <1200000>, + <1150000>, <1100000>, + <1000000>, <950000>; + + s5m8767,pmic-buck3-dvs-voltage = <1100000>, <1100000>, + <1100000>, <1100000>, + <1000000>, <1000000>, + <1000000>, <1000000>; + + s5m8767,pmic-buck4-dvs-voltage = <1200000>, <1200000>, + <1200000>, <1200000>, + <1200000>, <1200000>, + <1200000>, <1200000>; + + clocks { + compatible = "samsung,s5m8767-clk"; + #clock-cells = <1>; + clock-output-names = "en32khz_ap", + "en32khz_cp", + "en32khz_bt"; + }; + + regulators { + s5m_ldo4_reg: LDO4 {
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 171
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ regulator-name = "P1.0V_LDO_OUT4"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <0>;I think that "
op_mode" here is questionable. Adding Javier to the thread who was looking at this for max77802 and possibly max77686.
Confirming that this op_mode is present in the original 3.8 device tree.
(I used dtc to compile my /proc/device-tree tarball back to .dts, so it has hexadecimal <0x0> but that shouldn't matter to dtc AFAIU.)
Quoted 148 lines
Quoted 148 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +148
-0+ }; + + s5m_ldo5_reg: LDO5 { + regulator-name = "P1.0V_LDO_OUT5"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo6_reg: LDO6 { + regulator-name = "vdd_mydp"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo7_reg: LDO7 { + regulator-name = "P1.1V_LDO_OUT7"; + regulator-min-microvolt = <1100000>; + regulator-max-microvolt = <1100000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo8_reg: LDO8 { + regulator-name = "P1.0V_LDO_OUT8"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo10_reg: LDO10 { + regulator-name = "P1.8V_LDO_OUT10"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo11_reg: LDO11 { + regulator-name = "P1.8V_LDO_OUT11"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo12_reg: LDO12 { + regulator-name = "P3.0V_LDO_OUT12"; + regulator-min-microvolt = <3000000>; + regulator-max-microvolt = <3000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo13_reg: LDO13 { + regulator-name = "P1.8V_LDO_OUT13"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo14_reg: LDO14 { + regulator-name = "P1.8V_LDO_OUT14"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo15_reg: LDO15 { + regulator-name = "P1.0V_LDO_OUT15"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo16_reg: LDO16 { + regulator-name = "P1.8V_LDO_OUT16"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo17_reg: LDO17 { + regulator-name = "P2.8V_LDO_OUT17"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo25_reg: LDO25 { + regulator-name = "vdd_bridge"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + op_mode = <1>; + }; + + BUCK1 { + regulator-name = "vdd_mif"; + regulator-min-microvolt = <950000>; + regulator-max-microvolt = <1300000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK2 { + regulator-name = "vdd_arm"; + regulator-min-microvolt = <850000>; + regulator-max-microvolt = <1350000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK3 { + regulator-name = "vdd_int"; + regulator-min-microvolt = <900000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK4 { + regulator-name = "vdd_g3d"; + regulator-min-microvolt = <850000>; + regulator-max-microvolt = <1300000>; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK5 { + regulator-name = "P1.8V_BUCK_OUT5"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + regulator-boot-on; + op_mode = <1>;
Quoted 9 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+ }; + + BUCK6 { + regulator-name = "P1.2V_BUCK_OUT6"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + regulator-boot>
-on;
Quoted 14 lines
Quoted 14 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +14
-0+ op_mode = <0>; + }; + + BUCK9 { + regulator-name = "vdd_ummc"; + regulator-min-microvolt = <950000>; + regulator-max-microvolt = <3000000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + }; + }; + };
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 345
Quoted 4 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +4
-0+ + i2c at 12C70000 { + trackpad { + status = "disabled";Having this bogus entry here doesn't add anything. Remove it until the trackpad should be added. See http://crbug.com/371114 for a slightly stale bug about trackpad.
You misunderstand: This resolves an error about the cypress,cyapa inherited from -cros-common.dtsi. Spring uses a different device and two different I2C addresses.
Nodes named trackpad-bootloader and trackpad-alt are prepared here:
https://github.com/afaerber/linux/commits/spring-next
Quoted 17 lines
arch/arm/boot/dts/exynos5250-spring.dts line 350
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ }; + }; + + i2c at 12CA0000 { + embedded-controller {Add "
cros_ec" alias like snow.arch/arm/boot/dts/exynos5250-spring.dts line 357
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+ compatible = "google,cros-ec-i2c"; + reg = <0x1e>; + interrupts = <6 0>; + interrupt-parent = <&gpx1>; + wakeup-source; + + keyboard-controller {
Quoted 8 lines
Don't include keyboard-controller here. Add:
#include "
cros-ec-keyboard.dtsi"...at the bottom. Note that I think that the spring EC has a special "charger" key that it uses to indicate when a charger was plugged in and unplugged. I'm not sure how that will end up getting supported upstream but you could just leave it out for now.
Is there such a pending patch for snow? That's what I modeled after.
Where is cros-ec-keyboard.dtsi? Don't see it in
http://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/tree/arch/arm/boot/dts?h=for-next
or
http://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/tree/include/dt-bindings?h=for-next
Are you proposing I factor it out?
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts line 360
Quoted 3 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +3
-0+ compatible = "google,cros-ec-keyb"; + keypad,num-rows = <8>; + keypad,num-columns = <13>;Don't you need pinctrl here?
The keyboard is usable; what would pinctrl be needed for? There's none in the 3.8 device tree.
Quoted 84 lines
Quoted 84 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +84
-0+ google,needs-ghost-filter; + linux,keymap = < + 0x0001007d /* L_META */ + 0x0002003b /* F1 */ + 0x00030030 /* B */ + 0x00040044 /* F10 */ + 0x00060031 /* N */ + 0x0008000d /* = */ + 0x000a0064 /* R_ALT */ + + 0x01010001 /* ESC */ + 0x0102003e /* F4 */ + 0x01030022 /* G */ + 0x01040041 /* F7 */ + 0x01060023 /* H */ + 0x01080028 /* ' */ + 0x01090043 /* F9 */ + 0x010b000e /* BKSPACE */ + + 0x0200001d /* L_CTRL */ + 0x0201000f /* TAB */ + 0x0202003d /* F3 */ + 0x02030014 /* T */ + 0x02040040 /* F6 */ + 0x0205001b /* ] */ + 0x02060015 /* Y */ + 0x02070056 /* 102ND */ + 0x0208001a /* [ */ + 0x02090042 /* F8 */ + + 0x03010029 /* GRAVE */ + 0x0302003c /* F2 */ + 0x03030006 /* 5 */ + 0x0304003f /* F5 */ + 0x03060007 /* 6 */ + 0x0308000c /* - */ + 0x030b002b /* \ */ + + 0x04000061 /* R_CTRL */ + 0x0401001e /* A */ + 0x04020020 /* D */ + 0x04030021 /* F */ + 0x0404001f /* S */ + 0x04050025 /* K */ + 0x04060024 /* J */ + 0x04080027 /* ; */ + 0x04090026 /* L */ + 0x040a002b /* \ */ + 0x040b001c /* ENTER */ + + 0x0501002c /* Z */ + 0x0502002e /* C */ + 0x0503002f /* V */ + 0x0504002d /* X */ + 0x05050033 /* , */ + 0x05060032 /* M */ + 0x0507002a /* L_SHIFT */ + 0x05080035 /* / */ + 0x05090034 /* . */ + 0x050B0039 /* SPACE */ + + 0x06010002 /* 1 */ + 0x06020004 /* 3 */ + 0x06030005 /* 4 */ + 0x06040003 /* 2 */ + 0x06050009 /* 8 */ + 0x06060008 /* 7 */ + 0x0608000b /* 0 */ + 0x0609000a /* 9 */ + 0x060a0038 /* L_ALT */ + 0x060b006c /* DOWN */ + 0x060c006a /* RIGHT */ + + 0x07010010 /* Q */ + 0x07020012 /* E */ + 0x07030013 /* R */ + 0x07040011 /* W */ + 0x07050017 /* I */ + 0x07060016 /* U */ + 0x07070036 /* R_SHIFT */ + 0x07080019 /* P */ + 0x07090018 /* O */ + 0x070b0067 /* UP */ + 0x070c0069 /* LEFT */
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 450
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ >; + }; + }; + + power-regulator { + compatible = "ti,tps65090";I doubt tps65090 will "just work". Does it?
Good question. How to tell? :) Not familiar with clocks and regulators. I don't see the nodes referenced anywhere, so I could just try to drop (as I would, as per my cover letter, propose for anything non-essential that turns controversial).
If we drop tps65090, can we safely drop vbat-fixed-regulator as well?
Quoted 10 lines
On spring the tps65090 is not directly on the same i2c bus as the EC. It's actually hidden behind the EC.
Locally in the ChromeOS tree there appears to be a forked copy of the 65090 regulator driver that's in use just for spring. See this from the ChromeOS 3.8 tree:
./drivers/regulator/tps65090-regulator.c./drivers/regulator/cros_ec-tps65090.c
Quoted 8 lines
The Spring version of the driver sends EC commands directly to access the tps65090.
It is possible (untested) that you could also talk to tps65090 over an i2c tunnel. On exynos5420-peach-pit we have a full fledged i2c tunnel, but you may be able to extend the tunnel to export an i2c tunnel for spring using something like
<https://chromium-review.googlesource.com/66116>
The SBS battery thingy (not in this patch) should also be behind some tunnel. There's none defined in -cros-common.dtsi or in my patch, and still it shows up in dmesg to my surprise, complaining "Couldn't read remote-bus property".
Quoted 106 lines
Quoted 64 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +64
-0+ reg = <0x48>; + + /* + * Config irq to disable internal pulls + * even though we run in polling mode. + */ + pinctrl-names = "default"; + pinctrl-0 = <&tps65090_irq>; + + vsys1-supply = <&vbat>; + vsys2-supply = <&vbat>; + vsys3-supply = <&vbat>; + infet1-supply = <&vbat>; + infet2-supply = <&vbat>; + infet3-supply = <&vbat>; + infet4-supply = <&vbat>; + infet5-supply = <&vbat>; + infet6-supply = <&vbat>; + infet7-supply = <&vbat>; + vsys-l1-supply = <&vbat>; + vsys-l2-supply = <&vbat>; + + regulators { + fet1 { + regulator-name = "vcd_led"; + regulator-min-microvolt = <12000000>; + regulator-max-microvolt = <12000000>; + }; + fet3 { + regulator-name = "wwan_r"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + fet5 { + regulator-name = "cam"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + fet6 { + regulator-name = "lcd_vdd"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + }; + fet7 { + regulator-name = "ts"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + }; + }; + + charger { + compatible = "ti,tps65090-charger"; + }; + }; + }; + + hdmi { + hdmi-en-supply = <&s5m_ldo8_reg>; + vdd-supply = <&s5m_ldo8_reg>; + vdd_osc-supply = <&s5m_ldo10_reg>; + vdd_pll-supply = <&s5m_ldo8_reg>; + };arch/arm/boot/dts/exynos5250-spring.dts line 524
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ + fimd at 14400000 { + status = "okay"; + samsung,invert-vclk; + }; + + dp-controller at 145B0000 { + status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&dp_hpd>;This is probably not right. It looks as if spring uses gpc3-0 for display port HPD (as a GPIO). The upstream has this in the
exynos5250-pinctrl.dtsias a different pin.I think you'll need to define your own pinctrl here.
Quoted 24 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +24
-0+ samsung,color-space = <0>; + samsung,dynamic-range = <0>; + samsung,ycbcr-coeff = <0>; + samsung,color-depth = <1>; + samsung,link-rate = <0x0a>; + samsung,lane-count = <1>; + samsung,hpd-gpio = <&gpc3 0 0>; + + display-timings { + native-mode = <&timing1>; + + timing1: timing at 1 { + clock-frequency = <70589280>; + hactive = <1366>; + vactive = <768>; + hfront-porch = <40>; + hback-porch = <40>; + hsync-len = <32>; + vback-porch = <10>; + vfront-porch = <12>; + vsync-len = <6>; + }; + }; + };
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 556
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+ + fixed-rate-clocks { + xxti { + compatible = "samsung,clock-xxti"; + clock-frequency = <24000000>; + }; + }; +};
Will check on the other suggestions.
Thanks,
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend?rffer; HRB 16746 AG N?rnberg
Re: [RFC 4/4] ARM: dts: exynos5250: Add Spring device tree
Andreas,
Quoted 63 lines · Andreas F?rber
Hi Doug,
Am 23.06.2014 21:47, schrieb Doug Anderson:
Quoted 1 line
Thanks for posting! A first pass on this is below...
Thanks a lot for your quick review! My first big .dts patch, and no datasheets for the hardware at hand as a user.
A first pass of replies to my defense. ;)
Quoted 1 line · Andreas F?rber
[...]
Quoted 44 lines
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +4
-0@@ -0,0 +1,556 @@ +/* + * Google Spring board device tree source + * + * Copyright (c) 2013 Google, Incarch/arm/boot/dts/exynos5250-spring.dts line 14
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ * Copyright (c) 2014 SUSE LINUX Products GmbH + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +/dts-v1/; +#include "exynos5250.dtsi" +#include "exynos5250-cros-common.dtsi"It is possible we may want to backpedal on the use of "
exynos5250-cros-common.dtsi". I know that Olof (now CCed) said he wasn't a fan of how it turned out.The original idea was that it should include the arbitrary set of things that are common between a chunk of Chrome OS boards. As more boards were introduced things would need to migrate from the "common" file to the board files.
At the moment the current conventional wisdom is that some duplication is better than the confusing movement of everything back and forth. See exynos5420-peach-pit and exynos5800-peach-pi in ToT linux-next.
arch/arm/boot/dts/exynos5250-spring.dts line 20
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+/ { + model = "Google Spring"; + compatible = "google,spring", "samsung,exynos5250", "samsung,exynos5"; + + pinctrl at 11400000 {The new best way to do things is to put this down at the bottom. See exynos5420-peach-pit as an example:
Quoted 6 lines
&pinctrl_0{
...
}Note that I believe it was decided that top-level references like that should be sorted alphabetically.
Quoted 6 lines
Thanks for the hint. (My chosen sort order here was by address.)
Quoted 2 lines
If you wanted to apply that run to exynos5250-snow I don't think it would be a terrible idea.
I can of course apply changes to Snow, but I cannot test them myself.
If you want to send up a patch like that I'm happy to give it a once over and also to test it. ...but don't feel obligated
Quoted 47 lines
Quoted 38 lines
Quoted 38 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +38
-0+ s5m8767_dvs: s5m8767-dvs { + samsung,pins = "gpd1-0", "gpd1-1", "gpd1-2"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>; + }; + + s5m8767_ds: s5m8767-ds { + samsung,pins = "gpx2-3", "gpx2-4", "gpx2-5"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>; + }; + + tps65090_irq: tps65090-irq { + samsung,pins = "gpx2-6"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + s5m8767_irq: s5m8767-irq { + samsung,pins = "gpx3-2"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + hdmi_hpd_irq: hdmi-hpd-irq { + samsung,pins = "gpx3-7"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>; + }; + }; + + pinctrl at 13400000 { + hsic_reset: hsic-reset {Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 63
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ samsung,pins = "gpe1-0"; + samsung,pin-function = <1>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + };I'm pretty sure that the HSIC reset needed some funky code to make it work and I'm not sure what the status of that is upstream
Quoted 2 lines
Yeah, you mentioned something along those lines. However it's the equivalent of the usb3-vbus-en in
-snow.dts. Rename or drop?
On snow locally I see USB2 vbus is gpx1-1 and USB3 vbus is gpx2-7. I don't see that in spring.
This will take more time than I have right now to track down. I added Julius to the thread in case he has time to answer and can suggest what to do for upstream purposes. I may be able to look more tomorrow.
You can always send up the next version and include this and we'll look at it again.
Quoted 50 lines
Quoted 50 lines
Quoted 37 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +37
-0+ }; + + vbat: vbat-fixed-regulator { + compatible = "regulator-fixed"; + regulator-name = "vbat-supply"; + regulator-boot-on; + }; + + usb at 12000000 { + status = "okay"; + }; + + usb3_vbus_reg: regulator-usb3 { + compatible = "regulator-fixed"; + regulator-name = "P5.0V_USB3CON"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + gpio = <&gpe1 0 1>; + pinctrl-names = "default"; + pinctrl-0 = <&hsic_reset>; + enable-active-high; + }; + + phy at 12100000 { + vbus-supply = <&usb3_vbus_reg>; + }; + + usb at 12110000 { + samsung,vbus-gpio = <&gpx1 1 0>; + status = "okay"; + }; + + usb at 12120000 { + status = "okay"; + }; + + mmc at 12220000 {arch/arm/boot/dts/exynos5250-spring.dts line 110
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ /* MMC2 pins are used as GPIO for eDP bridge control. */ + status = "disabled"; + }; + + mmc at 12230000 { + status = "disabled"; + }; + + i2c at 12C60000 { + max77686 at 09 {There is no max77686 on spring. It uses s5m8767 in the place of max77686.
Quoted 3 lines
Quoted 1 line
...you've got "status = disabled", just remove it.
That's because it's inherited from
exynos5250-cros-common.dtsi.
Ah, right. So if we're keeping cros-common the proper thing to do is to move it out of cros-common in a patch before this one. ...but I think people might be happier if you simply don't include cros-common.
Quoted 3 lines
The rtc that the system successfully uses is "s5m-rtc", which I guess is on the s5m8767 then? (Confusing that 3.8 Spring had this rtc node despite Snow's max77686 not having it.)
I think it's not really supposed to. It was a bit of a hack to handle the fact that the RTC has two different i2c addresses on both the 77686 and this PMIC. The max77686 driver upstream handles it all in code and doesn't ask the device tree to list both addresses.
I think that _upstream_ snow doesn't have the node but _downstream_ spring might have the node (though having a child of a disabled node isn't particularly useful).
Quoted 69 lines
Quoted 60 lines
Quoted 15 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +15
-0+ #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + + rtc { + reg = <0x6>; + }; + }; + + s5m8767_pmic at 66 { + compatible = "samsung,s5m8767-pmic"; + reg = <0x66>; + interrupt-parent = <&gpx3>; + interrupts = <2 0>; + pinctrl-names = "default";arch/arm/boot/dts/exynos5250-spring.dts line 135
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ pinctrl-0 = <&s5m8767_irq &s5m8767_dvs &s5m8767_ds>; + wakeup-source; + + s5m8767,pmic-buck-dvs-gpios = <&gpd1 0 1>, /* DVS1 */ + <&gpd1 1 1>, /* DVS2 */ + <&gpd1 2 1>; /* DVS3 */ + + s5m8767,pmic-buck-ds-gpios = <&gpx2 3 1>, /* SET1 */ + <&gpx2 4 1>, /* SET2 */ + <&gpx2 5 1>; /* SET3 */The final "1" in each of the GPIO specifiers above should be
GPIO_ACTIVE_LOW.Quoted 31 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +31
-0+ + /* + * The following arrays of DVS voltages are not used, since we are + * not using GPIOs to control PMIC bucks, but they must be defined + * to please the driver. + */ + s5m8767,pmic-buck2-dvs-voltage = <1350000>, <1300000>, + <1250000>, <1200000>, + <1150000>, <1100000>, + <1000000>, <950000>; + + s5m8767,pmic-buck3-dvs-voltage = <1100000>, <1100000>, + <1100000>, <1100000>, + <1000000>, <1000000>, + <1000000>, <1000000>; + + s5m8767,pmic-buck4-dvs-voltage = <1200000>, <1200000>, + <1200000>, <1200000>, + <1200000>, <1200000>, + <1200000>, <1200000>; + + clocks { + compatible = "samsung,s5m8767-clk"; + #clock-cells = <1>; + clock-output-names = "en32khz_ap", + "en32khz_cp", + "en32khz_bt"; + }; + + regulators { + s5m_ldo4_reg: LDO4 {Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 171
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ regulator-name = "P1.0V_LDO_OUT4"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <0>;I think that "
op_mode" here is questionable. Adding Javier to the thread who was looking at this for max77802 and possibly max77686.
Quoted 1 line
Confirming that this
op_modeis present in the original 3.8 device tree.
Yes and it needs to be handled eventually. It makes suspend/resume work properly. ...but I think it needs to be thought out better.
...but given that other users of this PMIC have it I guess it makes sense to leave it in. Javier was going to try to think it through better for max77686 and max77802.
Quoted 184 lines
(I used dtc to compile my
/proc/device-treetarball back to .dts, so it has hexadecimal <0x0> but that shouldn't matter to dtc AFAIU.)Quoted 148 lines
Quoted 148 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +148
-0+ }; + + s5m_ldo5_reg: LDO5 { + regulator-name = "P1.0V_LDO_OUT5"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo6_reg: LDO6 { + regulator-name = "vdd_mydp"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo7_reg: LDO7 { + regulator-name = "P1.1V_LDO_OUT7"; + regulator-min-microvolt = <1100000>; + regulator-max-microvolt = <1100000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo8_reg: LDO8 { + regulator-name = "P1.0V_LDO_OUT8"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo10_reg: LDO10 { + regulator-name = "P1.8V_LDO_OUT10"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo11_reg: LDO11 { + regulator-name = "P1.8V_LDO_OUT11"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo12_reg: LDO12 { + regulator-name = "P3.0V_LDO_OUT12"; + regulator-min-microvolt = <3000000>; + regulator-max-microvolt = <3000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo13_reg: LDO13 { + regulator-name = "P1.8V_LDO_OUT13"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo14_reg: LDO14 { + regulator-name = "P1.8V_LDO_OUT14"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo15_reg: LDO15 { + regulator-name = "P1.0V_LDO_OUT15"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo16_reg: LDO16 { + regulator-name = "P1.8V_LDO_OUT16"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo17_reg: LDO17 { + regulator-name = "P2.8V_LDO_OUT17"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo25_reg: LDO25 { + regulator-name = "vdd_bridge"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + op_mode = <1>; + }; + + BUCK1 { + regulator-name = "vdd_mif"; + regulator-min-microvolt = <950000>; + regulator-max-microvolt = <1300000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK2 { + regulator-name = "vdd_arm"; + regulator-min-microvolt = <850000>; + regulator-max-microvolt = <1350000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK3 { + regulator-name = "vdd_int"; + regulator-min-microvolt = <900000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK4 { + regulator-name = "vdd_g3d"; + regulator-min-microvolt = <850000>; + regulator-max-microvolt = <1300000>; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK5 { + regulator-name = "P1.8V_BUCK_OUT5"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + regulator-boot-on; + op_mode = <1>;Quoted 9 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+ }; + + BUCK6 { + regulator-name = "P1.2V_BUCK_OUT6"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + regulator-boot>-on;
Quoted 14 lines
Quoted 14 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +14
-0+ op_mode = <0>; + }; + + BUCK9 { + regulator-name = "vdd_ummc"; + regulator-min-microvolt = <950000>; + regulator-max-microvolt = <3000000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + }; + }; + };Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 345
Quoted 4 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +4
-0+ + i2c at 12C70000 { + trackpad { + status = "disabled";Having this bogus entry here doesn't add anything. Remove it until the trackpad should be added. See http://crbug.com/371114 for a slightly stale bug about trackpad.
Quoted 3 lines
You misunderstand: This resolves an error about the cypress,cyapa inherited from
-cros-common.dtsi. Spring uses a different device and two different I2C addresses.
Ah, right. I forgot about cros5250-common.
Quoted 2 lines
Nodes named trackpad-bootloader and trackpad-alt are prepared here:
https://github.com/afaerber/linux/commits/spring-next
It might work, but you run into pinctrl problems. You need to put the pinctrl node _somewhere_ and if you put it with the trackpad entries themselves then you get conflicts. ...and it doesn't belong in the i2c bus (though that's what we have locally in our tree)
See <http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/30953>
for some background.
Quoted 19 lines
Quoted 19 lines
arch/arm/boot/dts/exynos5250-spring.dts line 350
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ }; + }; + + i2c at 12CA0000 { + embedded-controller {Add "
cros_ec" alias like snow.arch/arm/boot/dts/exynos5250-spring.dts line 357
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+ compatible = "google,cros-ec-i2c"; + reg = <0x1e>; + interrupts = <6 0>; + interrupt-parent = <&gpx1>; + wakeup-source; + + keyboard-controller {Don't include keyboard-controller here. Add:
Quoted 8 lines
Quoted 6 lines
#include "
cros-ec-keyboard.dtsi"...at the bottom. Note that I think that the spring EC has a special "charger" key that it uses to indicate when a charger was plugged in and unplugged. I'm not sure how that will end up getting supported upstream but you could just leave it out for now.
Is there such a pending patch for snow? That's what I modeled after.
Yeah, and it appears to be in linux-next. (1a395e3 ARM: dts: Use the cros-ec-keyboard fragment in exynos5250-snow). It actually ended up going through the tegra tree since it was in a series with a similar tegra change. I asked Kukjin to pick it up via merge but he didn't appear to.
Quoted 7 lines
Where is
cros-ec-keyboard.dtsi? Don't see it in
http://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/tree/arch/arm/boot/dts?h=for-next
or
http://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/tree/include/dt-bindings?h=for-nextAre you proposing I factor it out?
Quoted 8 lines
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts line 360
Quoted 3 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +3
-0+ compatible = "google,cros-ec-keyb"; + keypad,num-rows = <8>; + keypad,num-columns = <13>;Don't you need pinctrl here?
The keyboard is usable; what would pinctrl be needed for? There's none in the 3.8 device tree.
Generally it's good to have the pinctrl listed to configure the IRQ properly (get the pulls right). It might work without it but it's a little less ideal I think.
I see this, right?
pinctrl-names = "default";
pinctrl-0 = <&ec_irq>;...in the downstream "arch/arm/boot/dts/exynos5250-spring.dts"
Quoted 93 lines
Quoted 84 lines
Quoted 84 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +84
-0+ google,needs-ghost-filter; + linux,keymap = < + 0x0001007d /* L_META */ + 0x0002003b /* F1 */ + 0x00030030 /* B */ + 0x00040044 /* F10 */ + 0x00060031 /* N */ + 0x0008000d /* = */ + 0x000a0064 /* R_ALT */ + + 0x01010001 /* ESC */ + 0x0102003e /* F4 */ + 0x01030022 /* G */ + 0x01040041 /* F7 */ + 0x01060023 /* H */ + 0x01080028 /* ' */ + 0x01090043 /* F9 */ + 0x010b000e /* BKSPACE */ + + 0x0200001d /* L_CTRL */ + 0x0201000f /* TAB */ + 0x0202003d /* F3 */ + 0x02030014 /* T */ + 0x02040040 /* F6 */ + 0x0205001b /* ] */ + 0x02060015 /* Y */ + 0x02070056 /* 102ND */ + 0x0208001a /* [ */ + 0x02090042 /* F8 */ + + 0x03010029 /* GRAVE */ + 0x0302003c /* F2 */ + 0x03030006 /* 5 */ + 0x0304003f /* F5 */ + 0x03060007 /* 6 */ + 0x0308000c /* - */ + 0x030b002b /* \ */ + + 0x04000061 /* R_CTRL */ + 0x0401001e /* A */ + 0x04020020 /* D */ + 0x04030021 /* F */ + 0x0404001f /* S */ + 0x04050025 /* K */ + 0x04060024 /* J */ + 0x04080027 /* ; */ + 0x04090026 /* L */ + 0x040a002b /* \ */ + 0x040b001c /* ENTER */ + + 0x0501002c /* Z */ + 0x0502002e /* C */ + 0x0503002f /* V */ + 0x0504002d /* X */ + 0x05050033 /* , */ + 0x05060032 /* M */ + 0x0507002a /* L_SHIFT */ + 0x05080035 /* / */ + 0x05090034 /* . */ + 0x050B0039 /* SPACE */ + + 0x06010002 /* 1 */ + 0x06020004 /* 3 */ + 0x06030005 /* 4 */ + 0x06040003 /* 2 */ + 0x06050009 /* 8 */ + 0x06060008 /* 7 */ + 0x0608000b /* 0 */ + 0x0609000a /* 9 */ + 0x060a0038 /* L_ALT */ + 0x060b006c /* DOWN */ + 0x060c006a /* RIGHT */ + + 0x07010010 /* Q */ + 0x07020012 /* E */ + 0x07030013 /* R */ + 0x07040011 /* W */ + 0x07050017 /* I */ + 0x07060016 /* U */ + 0x07070036 /* R_SHIFT */ + 0x07080019 /* P */ + 0x07090018 /* O */ + 0x070b0067 /* UP */ + 0x070c0069 /* LEFT */Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 450
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ >; + }; + }; + + power-regulator { + compatible = "ti,tps65090";I doubt tps65090 will "just work". Does it?
Quoted 4 lines
Good question. How to tell? :) Not familiar with clocks and regulators. I don't see the nodes referenced anywhere, so I could just try to drop (as I would, as per my cover letter, propose for anything non-essential that turns controversial).
What does "dmesg | grep tps65090" say?
What does this show:
grep "" /sys/class/regulator/*/name
Quoted 1 line
If we drop tps65090, can we safely drop vbat-fixed-regulator as well?
Yes
Quoted 19 lines
Quoted 10 lines
On spring the tps65090 is not directly on the same i2c bus as the EC. It's actually hidden behind the EC.
Locally in the ChromeOS tree there appears to be a forked copy of the 65090 regulator driver that's in use just for spring. See this from the ChromeOS 3.8 tree:
./drivers/regulator/tps65090-regulator.c./drivers/regulator/cros_ec-tps65090.cQuoted 8 lines
The Spring version of the driver sends EC commands directly to access the tps65090.
It is possible (untested) that you could also talk to tps65090 over an i2c tunnel. On exynos5420-peach-pit we have a full fledged i2c tunnel, but you may be able to extend the tunnel to export an i2c tunnel for spring using something like
<https://chromium-review.googlesource.com/66116>
Quoted 4 lines
The SBS battery thingy (not in this patch) should also be behind some tunnel. There's none defined in
-cros-common.dtsior in my patch, and still it shows up in dmesg to my surprise, complaining "Couldn't read remote-bus property".
Might be due to the cros5250-common. ...but yes, the battery should be behind the tunnel too and the patches needed for Spring haven't been posted by anyone yet. I picked up a whole bunch of cros_ec cleanup patches and thought that the tunnel patches for spring could possible go up after those land.
Quoted 108 lines
Quoted 108 lines
Quoted 64 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +64
-0+ reg = <0x48>; + + /* + * Config irq to disable internal pulls + * even though we run in polling mode. + */ + pinctrl-names = "default"; + pinctrl-0 = <&tps65090_irq>; + + vsys1-supply = <&vbat>; + vsys2-supply = <&vbat>; + vsys3-supply = <&vbat>; + infet1-supply = <&vbat>; + infet2-supply = <&vbat>; + infet3-supply = <&vbat>; + infet4-supply = <&vbat>; + infet5-supply = <&vbat>; + infet6-supply = <&vbat>; + infet7-supply = <&vbat>; + vsys-l1-supply = <&vbat>; + vsys-l2-supply = <&vbat>; + + regulators { + fet1 { + regulator-name = "vcd_led"; + regulator-min-microvolt = <12000000>; + regulator-max-microvolt = <12000000>; + }; + fet3 { + regulator-name = "wwan_r"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + fet5 { + regulator-name = "cam"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + fet6 { + regulator-name = "lcd_vdd"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + }; + fet7 { + regulator-name = "ts"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + }; + }; + + charger { + compatible = "ti,tps65090-charger"; + }; + }; + }; + + hdmi { + hdmi-en-supply = <&s5m_ldo8_reg>; + vdd-supply = <&s5m_ldo8_reg>; + vdd_osc-supply = <&s5m_ldo10_reg>; + vdd_pll-supply = <&s5m_ldo8_reg>; + };arch/arm/boot/dts/exynos5250-spring.dts line 524
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ + fimd at 14400000 { + status = "okay"; + samsung,invert-vclk; + }; + + dp-controller at 145B0000 { + status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&dp_hpd>;This is probably not right. It looks as if spring uses gpc3-0 for display port HPD (as a GPIO). The upstream has this in the
exynos5250-pinctrl.dtsias a different pin.I think you'll need to define your own pinctrl here.
Quoted 26 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +26
-0+ samsung,color-space = <0>; + samsung,dynamic-range = <0>; + samsung,ycbcr-coeff = <0>; + samsung,color-depth = <1>; + samsung,link-rate = <0x0a>; + samsung,lane-count = <1>; + samsung,hpd-gpio = <&gpc3 0 0>; + + display-timings { + native-mode = <&timing1>; + + timing1: timing at 1 { + clock-frequency = <70589280>; + hactive = <1366>; + vactive = <768>; + hfront-porch = <40>; + hback-porch = <40>; + hsync-len = <32>; + vback-porch = <10>; + vfront-porch = <12>; + vsync-len = <6>; + }; + }; + }; + + fixed-rate-clocks {
Quoted 8 lines
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts line 556
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ xxti { + compatible = "samsung,clock-xxti"; + clock-frequency = <24000000>; + }; + }; +};Will check on the other suggestions.
-Doug
Re: [RFC 4/4] ARM: dts: exynos5250: Add Spring device tree
Hello Doug,
Quoted 50 lines · Doug Anderson
Andreas,
Quoted 47 lines · Andreas F?rber
Hi Doug,
Am 23.06.2014 21:47, schrieb Doug Anderson:
Quoted 1 line
Thanks for posting! A first pass on this is below...
Thanks a lot for your quick review! My first big .dts patch, and no datasheets for the hardware at hand as a user.
A first pass of replies to my defense. ;)
Quoted 1 line · Andreas F?rber
[...]
Quoted 35 lines
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +4
-0@@ -0,0 +1,556 @@ +/* + * Google Spring board device tree source + * + * Copyright (c) 2013 Google, Incarch/arm/boot/dts/exynos5250-spring.dts line 14
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ * Copyright (c) 2014 SUSE LINUX Products GmbH + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +/dts-v1/; +#include "exynos5250.dtsi" +#include "exynos5250-cros-common.dtsi"It is possible we may want to backpedal on the use of "
exynos5250-cros-common.dtsi". I know that Olof (now CCed) said he wasn't a fan of how it turned out.The original idea was that it should include the arbitrary set of things that are common between a chunk of Chrome OS boards. As more boards were introduced things would need to migrate from the "common" file to the board files.
At the moment the current conventional wisdom is that some duplication is better than the confusing movement of everything back and forth. See exynos5420-peach-pit and exynos5800-peach-pi in ToT linux-next.
Another option is to identify DTS fragments that are common across boards and create .dtsi files for these specific chunks instead of trying to group all set of common things on a single .dtsi file.
For example, a quite common design for OMAP2+ based boards is to use a SMSC LAN chip connected to OMAP's General-Purpose Memory Controller (GPMC). So the following files were created to reduce DTS duplication:
arch/arm/boot/dts/omap-gpmc-smsc911x.dtsiarch/arm/boot/dts/omap-gpmc-smsc9221.dtsi
Now that I think about it, is the same that what you did for arm/boot/dts/cros-ec-keyboard.dtsi.
Maybe splitting exynos5250-cros-common.dtsi in a set of .dtsi files will make it more flexible/reusable?
Quoted 152 lines
Quoted 16 lines
Quoted 9 lines
arch/arm/boot/dts/exynos5250-spring.dts line 20
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+/ { + model = "Google Spring"; + compatible = "google,spring", "samsung,exynos5250", "samsung,exynos5"; + + pinctrl at 11400000 {The new best way to do things is to put this down at the bottom. See exynos5420-peach-pit as an example:
Quoted 6 lines
&pinctrl_0{
...
}Note that I believe it was decided that top-level references like that should be sorted alphabetically.
Quoted 6 lines
Thanks for the hint. (My chosen sort order here was by address.)
Quoted 2 lines
If you wanted to apply that run to exynos5250-snow I don't think it would be a terrible idea.
I can of course apply changes to Snow, but I cannot test them myself.
If you want to send up a patch like that I'm happy to give it a once over and also to test it. ...but don't feel obligated
Quoted 47 lines
Quoted 38 lines
Quoted 38 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +38
-0+ s5m8767_dvs: s5m8767-dvs { + samsung,pins = "gpd1-0", "gpd1-1", "gpd1-2"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>; + }; + + s5m8767_ds: s5m8767-ds { + samsung,pins = "gpx2-3", "gpx2-4", "gpx2-5"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>; + }; + + tps65090_irq: tps65090-irq { + samsung,pins = "gpx2-6"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + s5m8767_irq: s5m8767-irq { + samsung,pins = "gpx3-2"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + hdmi_hpd_irq: hdmi-hpd-irq { + samsung,pins = "gpx3-7"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>; + }; + }; + + pinctrl at 13400000 { + hsic_reset: hsic-reset {Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 63
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ samsung,pins = "gpe1-0"; + samsung,pin-function = <1>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + };I'm pretty sure that the HSIC reset needed some funky code to make it work and I'm not sure what the status of that is upstream
Quoted 2 lines
Yeah, you mentioned something along those lines. However it's the equivalent of the usb3-vbus-en in
-snow.dts. Rename or drop?On snow locally I see USB2 vbus is gpx1-1 and USB3 vbus is gpx2-7. I don't see that in spring.
This will take more time than I have right now to track down. I added Julius to the thread in case he has time to answer and can suggest what to do for upstream purposes. I may be able to look more tomorrow.
You can always send up the next version and include this and we'll look at it again.
Quoted 50 lines
Quoted 50 lines
Quoted 37 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +37
-0+ }; + + vbat: vbat-fixed-regulator { + compatible = "regulator-fixed"; + regulator-name = "vbat-supply"; + regulator-boot-on; + }; + + usb at 12000000 { + status = "okay"; + }; + + usb3_vbus_reg: regulator-usb3 { + compatible = "regulator-fixed"; + regulator-name = "P5.0V_USB3CON"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + gpio = <&gpe1 0 1>; + pinctrl-names = "default"; + pinctrl-0 = <&hsic_reset>; + enable-active-high; + }; + + phy at 12100000 { + vbus-supply = <&usb3_vbus_reg>; + }; + + usb at 12110000 { + samsung,vbus-gpio = <&gpx1 1 0>; + status = "okay"; + }; + + usb at 12120000 { + status = "okay"; + }; + + mmc at 12220000 {arch/arm/boot/dts/exynos5250-spring.dts line 110
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ /* MMC2 pins are used as GPIO for eDP bridge control. */ + status = "disabled"; + }; + + mmc at 12230000 { + status = "disabled"; + }; + + i2c at 12C60000 { + max77686 at 09 {There is no max77686 on spring. It uses s5m8767 in the place of max77686.
Quoted 3 lines
Quoted 1 line
...you've got "status = disabled", just remove it.
That's because it's inherited from
exynos5250-cros-common.dtsi.Ah, right. So if we're keeping cros-common the proper thing to do is to move it out of cros-common in a patch before this one. ...but I think people might be happier if you simply don't include cros-common.
Quoted 3 lines
The rtc that the system successfully uses is "s5m-rtc", which I guess is on the s5m8767 then? (Confusing that 3.8 Spring had this rtc node despite Snow's max77686 not having it.)
Quoted 10 lines
I think it's not really supposed to. It was a bit of a hack to handle the fact that the RTC has two different i2c addresses on both the 77686 and this PMIC. The max77686 driver upstream handles it all in code and doesn't ask the device tree to list both addresses.
I think that _upstream_ snow doesn't have the node but _downstream_ spring might have the node (though having a child of a disabled node isn't particularly useful).
Personally I think that "status = [enabled | disabled]" only makes sense for IP blocks that are part of the SoC but may or may not be used by a board (e.g: i2c and spi buses, sdhci and usb host controllers, etc).
DTS should be a description of the hardware so I agree that having a disabled node for a device that is not present in the board is not right.
Quoted 69 lines
Quoted 69 lines
Quoted 68 lines
Quoted 15 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +15
-0+ #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + + rtc { + reg = <0x6>; + }; + }; + + s5m8767_pmic at 66 { + compatible = "samsung,s5m8767-pmic"; + reg = <0x66>; + interrupt-parent = <&gpx3>; + interrupts = <2 0>; + pinctrl-names = "default";arch/arm/boot/dts/exynos5250-spring.dts line 135
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ pinctrl-0 = <&s5m8767_irq &s5m8767_dvs &s5m8767_ds>; + wakeup-source; + + s5m8767,pmic-buck-dvs-gpios = <&gpd1 0 1>, /* DVS1 */ + <&gpd1 1 1>, /* DVS2 */ + <&gpd1 2 1>; /* DVS3 */ + + s5m8767,pmic-buck-ds-gpios = <&gpx2 3 1>, /* SET1 */ + <&gpx2 4 1>, /* SET2 */ + <&gpx2 5 1>; /* SET3 */The final "1" in each of the GPIO specifiers above should be
GPIO_ACTIVE_LOW.Quoted 26 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +26
-0+ + /* + * The following arrays of DVS voltages are not used, since we are + * not using GPIOs to control PMIC bucks, but they must be defined + * to please the driver. + */ + s5m8767,pmic-buck2-dvs-voltage = <1350000>, <1300000>, + <1250000>, <1200000>, + <1150000>, <1100000>, + <1000000>, <950000>; + + s5m8767,pmic-buck3-dvs-voltage = <1100000>, <1100000>, + <1100000>, <1100000>, + <1000000>, <1000000>, + <1000000>, <1000000>; + + s5m8767,pmic-buck4-dvs-voltage = <1200000>, <1200000>, + <1200000>, <1200000>, + <1200000>, <1200000>, + <1200000>, <1200000>; + + clocks { + compatible = "samsung,s5m8767-clk"; + #clock-cells = <1>; + clock-output-names = "en32khz_ap", + "en32khz_cp",arch/arm/boot/dts/exynos5250-spring.dts line 171
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ "en32khz_bt"; + }; + + regulators { + s5m_ldo4_reg: LDO4 { + regulator-name = "P1.0V_LDO_OUT4"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <0>;I think that "
op_mode" here is questionable. Adding Javier to the thread who was looking at this for max77802 and possibly max77686.
Quoted 10 lines
Quoted 1 line
Confirming that this
op_modeis present in the original 3.8 device tree.Yes and it needs to be handled eventually. It makes suspend/resume work properly. ...but I think it needs to be thought out better.
...but given that other users of this PMIC have it I guess it makes sense to leave it in. Javier was going to try to think it through better for max77686 and max77802.
The op_mode property is supported in mainline for this driver. I think that is not a great binding tbh since the property has a generic name while it's specific to the s5m8767 PMIC. But the driver uses it so makes sense to have it in the DTS.
I've on my TODO list after the Max 77802 PMIC driver is merged to propose a property for the generic regulator binding along the lines of what you proposed in [0].
If a generic property is added to the regulator binding, the exynos5250-spring.dts can later be changed to use this new property or keep using the s5m8767 specific one since the driver has to keep supporting it for backward compatibility.
Quoted 523 lines
Quoted 184 lines
(I used dtc to compile my
/proc/device-treetarball back to .dts, so it has hexadecimal <0x0> but that shouldn't matter to dtc AFAIU.)Quoted 148 lines
Quoted 148 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +148
-0+ }; + + s5m_ldo5_reg: LDO5 { + regulator-name = "P1.0V_LDO_OUT5"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo6_reg: LDO6 { + regulator-name = "vdd_mydp"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo7_reg: LDO7 { + regulator-name = "P1.1V_LDO_OUT7"; + regulator-min-microvolt = <1100000>; + regulator-max-microvolt = <1100000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo8_reg: LDO8 { + regulator-name = "P1.0V_LDO_OUT8"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo10_reg: LDO10 { + regulator-name = "P1.8V_LDO_OUT10"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo11_reg: LDO11 { + regulator-name = "P1.8V_LDO_OUT11"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo12_reg: LDO12 { + regulator-name = "P3.0V_LDO_OUT12"; + regulator-min-microvolt = <3000000>; + regulator-max-microvolt = <3000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo13_reg: LDO13 { + regulator-name = "P1.8V_LDO_OUT13"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo14_reg: LDO14 { + regulator-name = "P1.8V_LDO_OUT14"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo15_reg: LDO15 { + regulator-name = "P1.0V_LDO_OUT15"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo16_reg: LDO16 { + regulator-name = "P1.8V_LDO_OUT16"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo17_reg: LDO17 { + regulator-name = "P2.8V_LDO_OUT17"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo25_reg: LDO25 { + regulator-name = "vdd_bridge"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + op_mode = <1>; + }; + + BUCK1 { + regulator-name = "vdd_mif"; + regulator-min-microvolt = <950000>; + regulator-max-microvolt = <1300000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK2 { + regulator-name = "vdd_arm"; + regulator-min-microvolt = <850000>; + regulator-max-microvolt = <1350000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK3 { + regulator-name = "vdd_int"; + regulator-min-microvolt = <900000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK4 { + regulator-name = "vdd_g3d"; + regulator-min-microvolt = <850000>; + regulator-max-microvolt = <1300000>; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK5 { + regulator-name = "P1.8V_BUCK_OUT5"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + regulator-boot-on; + op_mode = <1>;Quoted 9 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+ }; + + BUCK6 { + regulator-name = "P1.2V_BUCK_OUT6"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + regulator-boot>-on;
Quoted 14 lines
Quoted 14 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +14
-0+ op_mode = <0>; + }; + + BUCK9 { + regulator-name = "vdd_ummc"; + regulator-min-microvolt = <950000>; + regulator-max-microvolt = <3000000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + }; + }; + };Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 345
Quoted 4 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +4
-0+ + i2c at 12C70000 { + trackpad { + status = "disabled";Having this bogus entry here doesn't add anything. Remove it until the trackpad should be added. See http://crbug.com/371114 for a slightly stale bug about trackpad.
Quoted 3 lines
You misunderstand: This resolves an error about the cypress,cyapa inherited from
-cros-common.dtsi. Spring uses a different device and two different I2C addresses.Ah, right. I forgot about cros5250-common.
Quoted 2 lines
Nodes named trackpad-bootloader and trackpad-alt are prepared here:
https://github.com/afaerber/linux/commits/spring-nextIt might work, but you run into pinctrl problems. You need to put the pinctrl node _somewhere_ and if you put it with the trackpad entries themselves then you get conflicts. ...and it doesn't belong in the i2c bus (though that's what we have locally in our tree)
See <http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/30953>
for some background.Quoted 19 lines
Quoted 19 lines
arch/arm/boot/dts/exynos5250-spring.dts line 350
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ }; + }; + + i2c at 12CA0000 { + embedded-controller {Add "
cros_ec" alias like snow.arch/arm/boot/dts/exynos5250-spring.dts line 357
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+ compatible = "google,cros-ec-i2c"; + reg = <0x1e>; + interrupts = <6 0>; + interrupt-parent = <&gpx1>; + wakeup-source; + + keyboard-controller {Don't include keyboard-controller here. Add:
Quoted 8 lines
Quoted 6 lines
#include "
cros-ec-keyboard.dtsi"...at the bottom. Note that I think that the spring EC has a special "charger" key that it uses to indicate when a charger was plugged in and unplugged. I'm not sure how that will end up getting supported upstream but you could just leave it out for now.
Is there such a pending patch for snow? That's what I modeled after.
Yeah, and it appears to be in linux-next. (1a395e3 ARM: dts: Use the cros-ec-keyboard fragment in exynos5250-snow). It actually ended up going through the tegra tree since it was in a series with a similar tegra change. I asked Kukjin to pick it up via merge but he didn't appear to.
Quoted 7 lines
Where is
cros-ec-keyboard.dtsi? Don't see it in
http://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/tree/arch/arm/boot/dts?h=for-next
or
http://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/tree/include/dt-bindings?h=for-nextAre you proposing I factor it out?
Quoted 8 lines
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts line 360
Quoted 3 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +3
-0+ compatible = "google,cros-ec-keyb"; + keypad,num-rows = <8>; + keypad,num-columns = <13>;Don't you need pinctrl here?
The keyboard is usable; what would pinctrl be needed for? There's none in the 3.8 device tree.
Generally it's good to have the pinctrl listed to configure the IRQ properly (get the pulls right). It might work without it but it's a little less ideal I think.
I see this, right?
pinctrl-names = "default"; pinctrl-0 = <&ec_irq>;...in the downstream "
arch/arm/boot/dts/exynos5250-spring.dts"Quoted 93 lines
Quoted 84 lines
Quoted 84 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +84
-0+ google,needs-ghost-filter; + linux,keymap = < + 0x0001007d /* L_META */ + 0x0002003b /* F1 */ + 0x00030030 /* B */ + 0x00040044 /* F10 */ + 0x00060031 /* N */ + 0x0008000d /* = */ + 0x000a0064 /* R_ALT */ + + 0x01010001 /* ESC */ + 0x0102003e /* F4 */ + 0x01030022 /* G */ + 0x01040041 /* F7 */ + 0x01060023 /* H */ + 0x01080028 /* ' */ + 0x01090043 /* F9 */ + 0x010b000e /* BKSPACE */ + + 0x0200001d /* L_CTRL */ + 0x0201000f /* TAB */ + 0x0202003d /* F3 */ + 0x02030014 /* T */ + 0x02040040 /* F6 */ + 0x0205001b /* ] */ + 0x02060015 /* Y */ + 0x02070056 /* 102ND */ + 0x0208001a /* [ */ + 0x02090042 /* F8 */ + + 0x03010029 /* GRAVE */ + 0x0302003c /* F2 */ + 0x03030006 /* 5 */ + 0x0304003f /* F5 */ + 0x03060007 /* 6 */ + 0x0308000c /* - */ + 0x030b002b /* \ */ + + 0x04000061 /* R_CTRL */ + 0x0401001e /* A */ + 0x04020020 /* D */ + 0x04030021 /* F */ + 0x0404001f /* S */ + 0x04050025 /* K */ + 0x04060024 /* J */ + 0x04080027 /* ; */ + 0x04090026 /* L */ + 0x040a002b /* \ */ + 0x040b001c /* ENTER */ + + 0x0501002c /* Z */ + 0x0502002e /* C */ + 0x0503002f /* V */ + 0x0504002d /* X */ + 0x05050033 /* , */ + 0x05060032 /* M */ + 0x0507002a /* L_SHIFT */ + 0x05080035 /* / */ + 0x05090034 /* . */ + 0x050B0039 /* SPACE */ + + 0x06010002 /* 1 */ + 0x06020004 /* 3 */ + 0x06030005 /* 4 */ + 0x06040003 /* 2 */ + 0x06050009 /* 8 */ + 0x06060008 /* 7 */ + 0x0608000b /* 0 */ + 0x0609000a /* 9 */ + 0x060a0038 /* L_ALT */ + 0x060b006c /* DOWN */ + 0x060c006a /* RIGHT */ + + 0x07010010 /* Q */ + 0x07020012 /* E */ + 0x07030013 /* R */ + 0x07040011 /* W */ + 0x07050017 /* I */ + 0x07060016 /* U */ + 0x07070036 /* R_SHIFT */ + 0x07080019 /* P */ + 0x07090018 /* O */ + 0x070b0067 /* UP */ + 0x070c0069 /* LEFT */Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 450
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ >; + }; + }; + + power-regulator { + compatible = "ti,tps65090";I doubt tps65090 will "just work". Does it?
Quoted 4 lines
Good question. How to tell? :) Not familiar with clocks and regulators. I don't see the nodes referenced anywhere, so I could just try to drop (as I would, as per my cover letter, propose for anything non-essential that turns controversial).
What does "dmesg | grep tps65090" say?
What does this show:
grep ""/sys/class/regulator/*/nameQuoted 1 line
If we drop tps65090, can we safely drop vbat-fixed-regulator as well?
Yes
Quoted 19 lines
Quoted 10 lines
On spring the tps65090 is not directly on the same i2c bus as the EC. It's actually hidden behind the EC.
Locally in the ChromeOS tree there appears to be a forked copy of the 65090 regulator driver that's in use just for spring. See this from the ChromeOS 3.8 tree:
./drivers/regulator/tps65090-regulator.c./drivers/regulator/cros_ec-tps65090.cQuoted 8 lines
The Spring version of the driver sends EC commands directly to access the tps65090.
It is possible (untested) that you could also talk to tps65090 over an i2c tunnel. On exynos5420-peach-pit we have a full fledged i2c tunnel, but you may be able to extend the tunnel to export an i2c tunnel for spring using something like
<https://chromium-review.googlesource.com/66116>Quoted 4 lines
The SBS battery thingy (not in this patch) should also be behind some tunnel. There's none defined in
-cros-common.dtsior in my patch, and still it shows up in dmesg to my surprise, complaining "Couldn't read remote-bus property".Might be due to the cros5250-common. ...but yes, the battery should be behind the tunnel too and the patches needed for Spring haven't been posted by anyone yet. I picked up a whole bunch of
cros_eccleanup patches and thought that the tunnel patches for spring could possible go up after those land.Quoted 108 lines
Quoted 108 lines
Quoted 64 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +64
-0+ reg = <0x48>; + + /* + * Config irq to disable internal pulls + * even though we run in polling mode. + */ + pinctrl-names = "default"; + pinctrl-0 = <&tps65090_irq>; + + vsys1-supply = <&vbat>; + vsys2-supply = <&vbat>; + vsys3-supply = <&vbat>; + infet1-supply = <&vbat>; + infet2-supply = <&vbat>; + infet3-supply = <&vbat>; + infet4-supply = <&vbat>; + infet5-supply = <&vbat>; + infet6-supply = <&vbat>; + infet7-supply = <&vbat>; + vsys-l1-supply = <&vbat>; + vsys-l2-supply = <&vbat>; + + regulators { + fet1 { + regulator-name = "vcd_led"; + regulator-min-microvolt = <12000000>; + regulator-max-microvolt = <12000000>; + }; + fet3 { + regulator-name = "wwan_r"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + fet5 { + regulator-name = "cam"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + fet6 { + regulator-name = "lcd_vdd"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + }; + fet7 { + regulator-name = "ts"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + }; + }; + + charger { + compatible = "ti,tps65090-charger"; + }; + }; + }; + + hdmi { + hdmi-en-supply = <&s5m_ldo8_reg>; + vdd-supply = <&s5m_ldo8_reg>; + vdd_osc-supply = <&s5m_ldo10_reg>; + vdd_pll-supply = <&s5m_ldo8_reg>; + };arch/arm/boot/dts/exynos5250-spring.dts line 524
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ + fimd at 14400000 { + status = "okay"; + samsung,invert-vclk; + }; + + dp-controller at 145B0000 { + status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&dp_hpd>;This is probably not right. It looks as if spring uses gpc3-0 for display port HPD (as a GPIO). The upstream has this in the
exynos5250-pinctrl.dtsias a different pin.I think you'll need to define your own pinctrl here.
Quoted 26 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +26
-0+ samsung,color-space = <0>; + samsung,dynamic-range = <0>; + samsung,ycbcr-coeff = <0>; + samsung,color-depth = <1>; + samsung,link-rate = <0x0a>; + samsung,lane-count = <1>; + samsung,hpd-gpio = <&gpc3 0 0>; + + display-timings { + native-mode = <&timing1>; + + timing1: timing at 1 { + clock-frequency = <70589280>; + hactive = <1366>; + vactive = <768>; + hfront-porch = <40>; + hback-porch = <40>; + hsync-len = <32>; + vback-porch = <10>; + vfront-porch = <12>; + vsync-len = <6>; + }; + }; + }; + + fixed-rate-clocks {Quoted 8 lines
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts line 556
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ xxti { + compatible = "samsung,clock-xxti"; + clock-frequency = <24000000>; + }; + }; +};Will check on the other suggestions.
Quoted 2 lines
-Doug
Best regards,
Javier
Re: [RFC 4/4] ARM: dts: exynos5250: Add Spring device tree
Javier,
Quoted 11 lines · Javier Martinez Canillas
Hello Doug,
On 06/24/2014 06:05 AM, Doug Anderson wrote:
Another option is to identify DTS fragments that are common across boards and create .dtsi files for these specific chunks instead of trying to group all set of common things on a single .dtsi file.For example, a quite common design for OMAP2+ based boards is to use a SMSC LAN chip connected to OMAP's General-Purpose Memory Controller (GPMC). So the following files were created to reduce DTS duplication:
Quoted 8 lines
arch/arm/boot/dts/omap-gpmc-smsc911x.dtsiarch/arm/boot/dts/omap-gpmc-smsc9221.dtsiNow that I think about it, is the same that what you did for
arm/boot/dts/cros-ec-keyboard.dtsi.Maybe splitting
exynos5250-cros-common.dtsiin a set of .dtsi files will make it more flexible/reusable?
Yes, I think the config fragments can be cleaner but I think we have to be judicious about using them. There are definitely tradeoffs involved. The keyboard was such an excessively large thing and totally duplicated, so moving it out made sense. Other bits are less obvious (to me) because there are so many interactions / combinations and you end up with a bit of spaghetti in terms of which labels are used by and provided by each fragment. I guess possibly you could codify that better...
A few thoughts looking at exynos5420-peach-pit:
* backlight: seems (?) too board specific
* samsung,exynos5420-oscclk: could totally be a fragment, but very small.
* power key: could be a fragment for all boards that happen to use gpx1-2 for this
* sound: could be a fragment for all devices using "google,snow-audio-max98090", possibly.
Quoted 6 lines
Personally I think that "status = [enabled | disabled]" only makes sense for IP blocks that are part of the SoC but may or may not be used by a board (e.g: i2c and spi buses, sdhci and usb host controllers, etc).
DTS should be a description of the hardware so I agree that having a disabled node for a device that is not present in the board is not right.
Right. We'll take a look again in v2 when cros-common isn't used. I think this could go in steps:
1. Don't use cros-common for spring
2. Don't use cros-common for snow (fold stuff in)
3. Introduce some fragments.
Re: [RFC 4/4] ARM: dts: exynos5250: Add Spring device tree
In reply to Doug AndersonQuoted 72 lines · Doug Anderson
Andreas,
Quoted 11 lines · Andreas Färber
Hi Doug,
Am 23.06.2014 21:47, schrieb Doug Anderson:
Quoted 1 line
Thanks for posting! A first pass on this is below...
Thanks a lot for your quick review! My first big .dts patch, and no datasheets for the hardware at hand as a user.
A first pass of replies to my defense. ;)
Quoted 1 line
On Sun, Jun 22, 2014 at 6:21 PM, Andreas Färber <[email protected]>
Quoted 2 lines · wrote:
[...]
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +0
-0
b/arch/arm/boot/dts/exynos5250-spring.dtsQuoted 4 lines
Quoted 4 lines
Quoted 4 lines
new file mode 100644
index 0000000..e857d44arch/arm/boot/dts/exynos5250-spring.dts new file +0
-0Quoted 8 lines
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 7
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0@@ -0,0 +1,556 @@ +/* + * Google Spring board device tree source + * + * Copyright (c) 2013 Google, Inc + * Copyright (c) 2014 SUSE LINUX Products GmbH + * + * This program is free software; you can redistribute it and/ormodify
Quoted 17 lines
Quoted 17 lines
arch/arm/boot/dts/exynos5250-spring.dts line 14
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+ * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +/dts-v1/; +#include "exynos5250.dtsi" +#include "exynos5250-cros-common.dtsi"It is possible we may want to backpedal on the use of "
exynos5250-cros-common.dtsi". I know that Olof (now CCed) said he wasn't a fan of how it turned out.The original idea was that it should include the arbitrary set of things that are common between a chunk of Chrome OS boards. As more boards were introduced things would need to migrate from the "common" file to the board files.
Quoted 8 lines
Quoted 8 lines
At the moment the current conventional wisdom is that some duplication is better than the confusing movement of everything back and forth. See exynos5420-peach-pit and exynos5800-peach-pi in ToT linux-next.
Quoted 3 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +3
-0+/ { + model = "Google Spring"; + compatible = "google,spring", "samsung,exynos5250","samsung,exynos5";
Quoted 15 lines
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts line 19
Quoted 2 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +2
-0+ + pinctrl@11400000 {The new best way to do things is to put this down at the bottom. See exynos5420-peach-pit as an example:
Quoted 6 lines
&pinctrl_0{
...
}Note that I believe it was decided that top-level references like that should be sorted alphabetically.
Thanks for the hint. (My chosen sort order here was by address.)
Quoted 59 lines
Quoted 4 lines
Quoted 2 lines
If you wanted to apply that run to exynos5250-snow I don't think it would be a terrible idea.
I can of course apply changes to Snow, but I cannot test them myself.
If you want to send up a patch like that I'm happy to give it a once over and also to test it. ...but don't feel obligated
Quoted 49 lines
Quoted 38 lines
Quoted 38 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +38
-0+ s5m8767_dvs: s5m8767-dvs { + samsung,pins = "gpd1-0", "gpd1-1", "gpd1-2"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>; + }; + + s5m8767_ds: s5m8767-ds { + samsung,pins = "gpx2-3", "gpx2-4", "gpx2-5"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>; + }; + + tps65090_irq: tps65090-irq { + samsung,pins = "gpx2-6"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + s5m8767_irq: s5m8767-irq { + samsung,pins = "gpx3-2"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + hdmi_hpd_irq: hdmi-hpd-irq { + samsung,pins = "gpx3-7"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>; + }; + }; + + pinctrl@13400000 { + hsic_reset: hsic-reset {Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 63
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ samsung,pins = "gpe1-0"; + samsung,pin-function = <1>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + };I'm pretty sure that the HSIC reset needed some funky code to make it work and I'm not sure what the status of that is upstream
Yeah, you mentioned something along those lines. However it's the equivalent of the usb3-vbus-en in
-snow.dts. Rename or drop?
It's clearly not equivalent, usb3-vbus-en just turns on/off the load switch for VBUS
while the HSIC chip (USB3503) is an external PHY (and hub) for the USB2 port, the reset of the PHY side of the HSIC needs to be done within the proper timing window compared to the SoC HSIC controller to get a working link.
As the current kernel has a binding for the SMSC USB3503 and a PHY driver (which is in driver/usb/misc probably because it is an external PHY), the hsic-reset should be connected there (as "reset-gpios").
On top of that the HSIC link is a bit finicky and it needs a few quirks if we want it to survive all kind of events :
eg s5p_hub_control() in
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/usb/host/ehci-s5p.c
Quoted 4 lines
On snow locally I see USB2 vbus is gpx1-1 and USB3 vbus is gpx2-7. I don't see that in spring.
On Spring, the USB3 root hub is just connected to the internal camera (which is powered by one of the TPS65090 LDOs)
On the external USB2 ports, the port enable going to the VBUS load switch is controlled directly the USB3503 (which has an internal mechanism, configured by default as an automatic control of overcurrent protection, it can be bypass by using the USB3503 I2C registers).
Quoted 779 lines
This will take more time than I have right now to track down. I added Julius to the thread in case he has time to answer and can suggest what to do for upstream purposes. I may be able to look more tomorrow.
You can always send up the next version and include this and we'll look at it again.
Quoted 30 lines
Quoted 30 lines
Quoted 30 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +30
-0+ }; + + vbat: vbat-fixed-regulator { + compatible = "regulator-fixed"; + regulator-name = "vbat-supply"; + regulator-boot-on; + }; + + usb@12000000 { + status = "okay"; + }; + + usb3_vbus_reg: regulator-usb3 { + compatible = "regulator-fixed"; + regulator-name = "P5.0V_USB3CON"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + gpio = <&gpe1 0 1>; + pinctrl-names = "default"; + pinctrl-0 = <&hsic_reset>; + enable-active-high; + }; + + phy@12100000 { + vbus-supply = <&usb3_vbus_reg>; + }; + + usb@12110000 { + samsung,vbus-gpio = <&gpx1 1 0>; + status = "okay";Quoted 8 lines
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+ }; + + usb@12120000 { + status = "okay"; + }; + + mmc@12220000 { + /* MMC2 pins are used as GPIO for eDP bridge control.*/
Quoted 11 lines
Quoted 11 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 9 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +9
-0+ status = "disabled"; + }; + + mmc@12230000 { + status = "disabled"; + }; + + i2c@12C60000 { + max77686@09 {There is no max77686 on spring. It uses s5m8767 in the place of
max77686.
Quoted 4 lines
Quoted 2 lines
...you've got "status = disabled", just remove it.
That's because it's inherited from
exynos5250-cros-common.dtsi.Ah, right. So if we're keeping cros-common the proper thing to do is to move it out of cros-common in a patch before this one. ...but I think people might be happier if you simply don't include cros-common.
Quoted 3 lines
The rtc that the system successfully uses is "s5m-rtc", which I guess is on the s5m8767 then? (Confusing that 3.8 Spring had this rtc node despite Snow's max77686 not having it.)
I think it's not really supposed to. It was a bit of a hack to handle the fact that the RTC has two different i2c addresses on both the 77686 and this PMIC. The max77686 driver upstream handles it all in code and doesn't ask the device tree to list both addresses.
I think that _upstream_ snow doesn't have the node but _downstream_ spring might have the node (though having a child of a disabled node isn't particularly useful).
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+ #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + + rtc { + reg = <0x6>; + }; + };Quoted 8 lines
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+ + s5m8767_pmic@66 { + compatible = "samsung,s5m8767-pmic"; + reg = <0x66>; + interrupt-parent = <&gpx3>; + interrupts = <2 0>; + pinctrl-names = "default"; + pinctrl-0 = <&s5m8767_irq &s5m8767_dvs
&s5m8767_ds>;Quoted 3 lines
Quoted 3 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 3 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +3
-0+ wakeup-source; + + s5m8767,pmic-buck-dvs-gpios = <&gpd1 0 1>, /*DVS1 */
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 129
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ <&gpd1 1 1>, /*DVS2 */
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 130
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ <&gpd1 2 1>; /*DVS3 */
Quoted 2 lines
Quoted 2 lines
arch/arm/boot/dts/exynos5250-spring.dts line 132
Quoted 2 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +2
-0+ + s5m8767,pmic-buck-ds-gpios = <&gpx2 3 1>, /*SET1 */
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 133
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ <&gpx2 4 1>, /*SET2 */
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 134
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ <&gpx2 5 1>; /*SET3 */
Quoted 2 lines
Quoted 2 lines
The final "1" in each of the GPIO specifiers above should be
GPIO_ACTIVE_LOW.Quoted 5 lines
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +3
-0+ + /* + * The following arrays of DVS voltages arenot used, since we are
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 138
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ * not using GPIOs to control PMIC bucks, butthey must be defined
Quoted 3 lines
Quoted 3 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 3 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +3
-0+ * to please the driver. + */ + s5m8767,pmic-buck2-dvs-voltage = <1350000>,<1300000>,
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 142
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ <1250000>,<1200000>,
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 143
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ <1150000>,<1100000>,
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 144
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ <1000000>,<950000>;
Quoted 2 lines
Quoted 2 lines
arch/arm/boot/dts/exynos5250-spring.dts line 146
Quoted 2 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +2
-0+ + s5m8767,pmic-buck3-dvs-voltage = <1100000>,<1100000>,
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 147
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ <1100000>,<1100000>,
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 148
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ <1000000>,<1000000>,
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 149
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ <1000000>,<1000000>;
Quoted 2 lines
Quoted 2 lines
arch/arm/boot/dts/exynos5250-spring.dts line 151
Quoted 2 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +2
-0+ + s5m8767,pmic-buck4-dvs-voltage = <1200000>,<1200000>,
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 152
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ <1200000>,<1200000>,
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 153
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ <1200000>,<1200000>,
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 154
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ <1200000>,<1200000>;
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +4
-0+ + clocks { + compatible = "samsung,s5m8767-clk"; + #clock-cells = <1>;Quoted 8 lines
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+ clock-output-names = "en32khz_ap", + "en32khz_cp", + "en32khz_bt"; + }; + + regulators { + s5m_ldo4_reg: LDO4 { + regulator-name ="P1.0V_LDO_OUT4";
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 167
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-min-microvolt =<1000000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 168
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1000000>;
Quoted 7 lines
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts line 170
Quoted 2 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +2
-0+ regulator-always-on; + op_mode = <0>;I think that "
op_mode" here is questionable. Adding Javier to the thread who was looking at this for max77802 and possibly max77686.Confirming that this
op_modeis present in the original 3.8 device tree.Yes and it needs to be handled eventually. It makes suspend/resume work properly. ...but I think it needs to be thought out better.
...but given that other users of this PMIC have it I guess it makes sense to leave it in. Javier was going to try to think it through better for max77686 and max77802.
Quoted 7 lines
(I used dtc to compile my
/proc/device-treetarball back to .dts, so it has hexadecimal <0x0> but that shouldn't matter to dtc AFAIU.)Quoted 4 lines
Quoted 4 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +4
-0+ }; + + s5m_ldo5_reg: LDO5 { + regulator-name ="P1.0V_LDO_OUT5";
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 175
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-min-microvolt =<1000000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 176
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1000000>;
Quoted 7 lines
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+ regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo6_reg: LDO6 { + regulator-name = "vdd_mydp"; + regulator-min-microvolt =<1000000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 184
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1000000>;
Quoted 6 lines
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo7_reg: LDO7 { + regulator-name ="P1.1V_LDO_OUT7";
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 191
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-min-microvolt =<1100000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 192
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1100000>;
Quoted 6 lines
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo8_reg: LDO8 { + regulator-name ="P1.0V_LDO_OUT8";
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 199
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-min-microvolt =<1000000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 200
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1000000>;
Quoted 6 lines
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo10_reg: LDO10 { + regulator-name ="P1.8V_LDO_OUT10";
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 207
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-min-microvolt =<1800000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 208
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1800000>;
Quoted 6 lines
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo11_reg: LDO11 { + regulator-name ="P1.8V_LDO_OUT11";
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 215
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-min-microvolt =<1800000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 216
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1800000>;
Quoted 6 lines
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo12_reg: LDO12 { + regulator-name ="P3.0V_LDO_OUT12";
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 223
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-min-microvolt =<3000000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 224
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<3000000>;
Quoted 6 lines
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo13_reg: LDO13 { + regulator-name ="P1.8V_LDO_OUT13";
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 231
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-min-microvolt =<1800000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 232
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1800000>;
Quoted 6 lines
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo14_reg: LDO14 { + regulator-name ="P1.8V_LDO_OUT14";
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 239
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-min-microvolt =<1800000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 240
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1800000>;
Quoted 6 lines
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo15_reg: LDO15 { + regulator-name ="P1.0V_LDO_OUT15";
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 247
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-min-microvolt =<1000000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 248
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1000000>;
Quoted 6 lines
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo16_reg: LDO16 { + regulator-name ="P1.8V_LDO_OUT16";
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 255
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-min-microvolt =<1800000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 256
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1800000>;
Quoted 6 lines
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo17_reg: LDO17 { + regulator-name ="P2.8V_LDO_OUT17";
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 263
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-min-microvolt =<2800000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 264
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<2800000>;
Quoted 7 lines
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+ regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo25_reg: LDO25 { + regulator-name = "vdd_bridge"; + regulator-min-microvolt =<1200000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 272
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1200000>;
Quoted 7 lines
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+ regulator-always-on; + op_mode = <1>; + }; + + BUCK1 { + regulator-name = "vdd_mif"; + regulator-min-microvolt =<950000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 280
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1300000>;
Quoted 8 lines
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+ regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK2 { + regulator-name = "vdd_arm"; + regulator-min-microvolt =<850000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 289
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1350000>;
Quoted 8 lines
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+ regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK3 { + regulator-name = "vdd_int"; + regulator-min-microvolt =<900000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 298
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1200000>;
Quoted 8 lines
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+ regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK4 { + regulator-name = "vdd_g3d"; + regulator-min-microvolt =<850000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 307
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1300000>;
Quoted 6 lines
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ regulator-boot-on; + op_mode = <3>; + }; + + BUCK5 { + regulator-name ="P1.8V_BUCK_OUT5";
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 314
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-min-microvolt =<1800000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 315
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1800000>;
Quoted 7 lines
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+ regulator-always-on; + regulator-boot-on; + op_mode = <1>; + }; + + BUCK6 { + regulator-name ="P1.2V_BUCK_OUT6";
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 323
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-min-microvolt =<1200000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 324
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<1200000>;
Quoted 10 lines
Quoted 3 lines
arch/arm/boot/dts/exynos5250-spring.dts line 326
Quoted 2 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +2
-0+ regulator-always-on; + regulator-boot>-on;
Quoted 6 lines
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ op_mode = <0>; + }; + + BUCK9 { + regulator-name = "vdd_ummc"; + regulator-min-microvolt =<950000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 333
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<3000000>;
Quoted 12 lines
Quoted 12 lines
Quoted 11 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +11
-0+ regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + }; + }; + }; + + i2c@12C70000 { + trackpad { + status = "disabled";Quoted 7 lines
Quoted 3 lines
Having this bogus entry here doesn't add anything. Remove it until the trackpad should be added. See http://crbug.com/371114 for a slightly stale bug about trackpad.
You misunderstand: This resolves an error about the cypress,cyapa inherited from
-cros-common.dtsi. Spring uses a different device and two different I2C addresses.Ah, right. I forgot about cros5250-common.
Quoted 2 lines
Nodes named trackpad-bootloader and trackpad-alt are prepared here:
https://github.com/afaerber/linux/commits/spring-nextIt might work, but you run into pinctrl problems. You need to put the pinctrl node _somewhere_ and if you put it with the trackpad entries themselves then you get conflicts. ...and it doesn't belong in the i2c bus (though that's what we have locally in our tree)
See <http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/30953>
for some background.Quoted 19 lines
Quoted 19 lines
arch/arm/boot/dts/exynos5250-spring.dts line 350
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ }; + }; + + i2c@12CA0000 { + embedded-controller {Add "
cros_ec" alias like snow.arch/arm/boot/dts/exynos5250-spring.dts line 357
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+ compatible = "google,cros-ec-i2c"; + reg = <0x1e>; + interrupts = <6 0>; + interrupt-parent = <&gpx1>; + wakeup-source; + + keyboard-controller {Don't include keyboard-controller here. Add:
Quoted 8 lines
Quoted 6 lines
#include "
cros-ec-keyboard.dtsi"...at the bottom. Note that I think that the spring EC has a special "charger" key that it uses to indicate when a charger was plugged in and unplugged. I'm not sure how that will end up getting supported upstream but you could just leave it out for now.
Is there such a pending patch for snow? That's what I modeled after.
Yeah, and it appears to be in linux-next. (1a395e3 ARM: dts: Use the cros-ec-keyboard fragment in exynos5250-snow). It actually ended up going through the tegra tree since it was in a series with a similar tegra change. I asked Kukjin to pick it up via merge but he didn't appear to.
Quoted 2 lines
Where is
cros-ec-keyboard.dtsi? Don't see it inQuoted 2 lines
or
Quoted 11 lines
Are you proposing I factor it out?
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts line 360
Quoted 3 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +3
-0+ compatible = "google,cros-ec-keyb"; + keypad,num-rows = <8>; + keypad,num-columns = <13>;Don't you need pinctrl here?
The keyboard is usable; what would pinctrl be needed for? There's none in the 3.8 device tree.
Generally it's good to have the pinctrl listed to configure the IRQ properly (get the pulls right). It might work without it but it's a little less ideal I think.
I see this, right?
pinctrl-names = "default"; pinctrl-0 = <&ec_irq>;...in the downstream "
arch/arm/boot/dts/exynos5250-spring.dts"Quoted 93 lines
Quoted 84 lines
Quoted 84 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +84
-0+ google,needs-ghost-filter; + linux,keymap = < + 0x0001007d /* L_META */ + 0x0002003b /* F1 */ + 0x00030030 /* B */ + 0x00040044 /* F10 */ + 0x00060031 /* N */ + 0x0008000d /* = */ + 0x000a0064 /* R_ALT */ + + 0x01010001 /* ESC */ + 0x0102003e /* F4 */ + 0x01030022 /* G */ + 0x01040041 /* F7 */ + 0x01060023 /* H */ + 0x01080028 /* ' */ + 0x01090043 /* F9 */ + 0x010b000e /* BKSPACE */ + + 0x0200001d /* L_CTRL */ + 0x0201000f /* TAB */ + 0x0202003d /* F3 */ + 0x02030014 /* T */ + 0x02040040 /* F6 */ + 0x0205001b /* ] */ + 0x02060015 /* Y */ + 0x02070056 /* 102ND */ + 0x0208001a /* [ */ + 0x02090042 /* F8 */ + + 0x03010029 /* GRAVE */ + 0x0302003c /* F2 */ + 0x03030006 /* 5 */ + 0x0304003f /* F5 */ + 0x03060007 /* 6 */ + 0x0308000c /* - */ + 0x030b002b /* \ */ + + 0x04000061 /* R_CTRL */ + 0x0401001e /* A */ + 0x04020020 /* D */ + 0x04030021 /* F */ + 0x0404001f /* S */ + 0x04050025 /* K */ + 0x04060024 /* J */ + 0x04080027 /* ; */ + 0x04090026 /* L */ + 0x040a002b /* \ */ + 0x040b001c /* ENTER */ + + 0x0501002c /* Z */ + 0x0502002e /* C */ + 0x0503002f /* V */ + 0x0504002d /* X */ + 0x05050033 /* , */ + 0x05060032 /* M */ + 0x0507002a /* L_SHIFT */ + 0x05080035 /* / */ + 0x05090034 /* . */ + 0x050B0039 /* SPACE */ + + 0x06010002 /* 1 */ + 0x06020004 /* 3 */ + 0x06030005 /* 4 */ + 0x06040003 /* 2 */ + 0x06050009 /* 8 */ + 0x06060008 /* 7 */ + 0x0608000b /* 0 */ + 0x0609000a /* 9 */ + 0x060a0038 /* L_ALT */ + 0x060b006c /* DOWN */ + 0x060c006a /* RIGHT */ + + 0x07010010 /* Q */ + 0x07020012 /* E */ + 0x07030013 /* R */ + 0x07040011 /* W */ + 0x07050017 /* I */ + 0x07060016 /* U */ + 0x07070036 /* R_SHIFT */ + 0x07080019 /* P */ + 0x07090018 /* O */ + 0x070b0067 /* UP */ + 0x070c0069 /* LEFT */Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 450
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ >; + }; + }; + + power-regulator { + compatible = "ti,tps65090";I doubt tps65090 will "just work". Does it?
Quoted 4 lines
Good question. How to tell? :) Not familiar with clocks and regulators. I don't see the nodes referenced anywhere, so I could just try to drop (as I would, as per my cover letter, propose for anything non-essential that turns controversial).
What does "dmesg | grep tps65090" say?
What does this show:
grep ""/sys/class/regulator/*/nameQuoted 1 line
If we drop tps65090, can we safely drop vbat-fixed-regulator as well?
Yes
Quoted 19 lines
Quoted 10 lines
On spring the tps65090 is not directly on the same i2c bus as the EC. It's actually hidden behind the EC.
Locally in the ChromeOS tree there appears to be a forked copy of the 65090 regulator driver that's in use just for spring. See this from the ChromeOS 3.8 tree:
./drivers/regulator/tps65090-regulator.c./drivers/regulator/cros_ec-tps65090.cQuoted 8 lines
The Spring version of the driver sends EC commands directly to access the tps65090.
It is possible (untested) that you could also talk to tps65090 over an i2c tunnel. On exynos5420-peach-pit we have a full fledged i2c tunnel, but you may be able to extend the tunnel to export an i2c tunnel for spring using something like
<https://chromium-review.googlesource.com/66116>Quoted 4 lines
The SBS battery thingy (not in this patch) should also be behind some tunnel. There's none defined in
-cros-common.dtsior in my patch, and still it shows up in dmesg to my surprise, complaining "Couldn't read remote-bus property".Might be due to the cros5250-common. ...but yes, the battery should be behind the tunnel too and the patches needed for Spring haven't been posted by anyone yet. I picked up a whole bunch of
cros_eccleanup patches and thought that the tunnel patches for spring could possible go up after those land.Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +18
-0+ reg = <0x48>; + + /* + * Config irq to disable internal pulls + * even though we run in polling mode. + */ + pinctrl-names = "default"; + pinctrl-0 = <&tps65090_irq>; + + vsys1-supply = <&vbat>; + vsys2-supply = <&vbat>; + vsys3-supply = <&vbat>; + infet1-supply = <&vbat>; + infet2-supply = <&vbat>; + infet3-supply = <&vbat>; + infet4-supply = <&vbat>; + infet5-supply = <&vbat>; + infet6-supply = <&vbat>;Quoted 8 lines
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+ infet7-supply = <&vbat>; + vsys-l1-supply = <&vbat>; + vsys-l2-supply = <&vbat>; + + regulators { + fet1 { + regulator-name = "vcd_led"; + regulator-min-microvolt =<12000000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 476
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<12000000>;
Quoted 4 lines
Quoted 4 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 4 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +4
-0+ }; + fet3 { + regulator-name = "wwan_r"; + regulator-min-microvolt =<3300000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 481
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<3300000>;
Quoted 5 lines
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ regulator-always-on; + }; + fet5 { + regulator-name = "cam"; + regulator-min-microvolt =<3300000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 487
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<3300000>;
Quoted 5 lines
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ regulator-always-on; + }; + fet6 { + regulator-name = "lcd_vdd"; + regulator-min-microvolt =<3300000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 493
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<3300000>;
Quoted 4 lines
Quoted 4 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 4 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +4
-0+ }; + fet7 { + regulator-name = "ts"; + regulator-min-microvolt =<5000000>;
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts line 498
Quoted 1 line
arch/arm/boot/dts/exynos5250-spring.dts new file +1
-0+ regulator-max-microvolt =<5000000>;
Quoted 59 lines
Quoted 59 lines
Quoted 15 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +15
-0+ }; + }; + + charger { + compatible = "ti,tps65090-charger"; + }; + }; + }; + + hdmi { + hdmi-en-supply = <&s5m_ldo8_reg>; + vdd-supply = <&s5m_ldo8_reg>; + vdd_osc-supply = <&s5m_ldo10_reg>; + vdd_pll-supply = <&s5m_ldo8_reg>; + };arch/arm/boot/dts/exynos5250-spring.dts line 524
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ + fimd@14400000 { + status = "okay"; + samsung,invert-vclk; + }; + + dp-controller@145B0000 { + status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&dp_hpd>;This is probably not right. It looks as if spring uses gpc3-0 for display port HPD (as a GPIO). The upstream has this in the
exynos5250-pinctrl.dtsias a different pin.I think you'll need to define your own pinctrl here.
Quoted 26 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +26
-0+ samsung,color-space = <0>; + samsung,dynamic-range = <0>; + samsung,ycbcr-coeff = <0>; + samsung,color-depth = <1>; + samsung,link-rate = <0x0a>; + samsung,lane-count = <1>; + samsung,hpd-gpio = <&gpc3 0 0>; + + display-timings { + native-mode = <&timing1>; + + timing1: timing@1 { + clock-frequency = <70589280>; + hactive = <1366>; + vactive = <768>; + hfront-porch = <40>; + hback-porch = <40>; + hsync-len = <32>; + vback-porch = <10>; + vfront-porch = <12>; + vsync-len = <6>; + }; + }; + }; + + fixed-rate-clocks {Quoted 8 lines
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts line 556
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ xxti { + compatible = "samsung,clock-xxti"; + clock-frequency = <24000000>; + }; + }; +};Will check on the other suggestions.
-Doug
Re: [RFC 4/4] ARM: dts: exynos5250: Add Spring device tree
In reply to Doug AndersonRe-sending ... the text-only encoding was not properly turned on on the previous one and irritated the mailing lists.
Quoted 69 lines · Doug Anderson
Andreas,
Quoted 65 lines · Andreas F?rber
Hi Doug,
Am 23.06.2014 21:47, schrieb Doug Anderson:
Quoted 1 line
Thanks for posting! A first pass on this is below...
Thanks a lot for your quick review! My first big .dts patch, and no datasheets for the hardware at hand as a user.
A first pass of replies to my defense. ;)
Quoted 1 line · Andreas F?rber
[...]
Quoted 44 lines
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +4
-0@@ -0,0 +1,556 @@ +/* + * Google Spring board device tree source + * + * Copyright (c) 2013 Google, Incarch/arm/boot/dts/exynos5250-spring.dts line 14
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ * Copyright (c) 2014 SUSE LINUX Products GmbH + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +/dts-v1/; +#include "exynos5250.dtsi" +#include "exynos5250-cros-common.dtsi"It is possible we may want to backpedal on the use of "
exynos5250-cros-common.dtsi". I know that Olof (now CCed) said he wasn't a fan of how it turned out.The original idea was that it should include the arbitrary set of things that are common between a chunk of Chrome OS boards. As more boards were introduced things would need to migrate from the "common" file to the board files.
At the moment the current conventional wisdom is that some duplication is better than the confusing movement of everything back and forth. See exynos5420-peach-pit and exynos5800-peach-pi in ToT linux-next.
arch/arm/boot/dts/exynos5250-spring.dts line 20
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+/ { + model = "Google Spring"; + compatible = "google,spring", "samsung,exynos5250", "samsung,exynos5"; + + pinctrl at 11400000 {The new best way to do things is to put this down at the bottom. See exynos5420-peach-pit as an example:
Quoted 6 lines
&pinctrl_0{
...
}Note that I believe it was decided that top-level references like that should be sorted alphabetically.
Thanks for the hint. (My chosen sort order here was by address.)
Quoted 58 lines
Quoted 4 lines
Quoted 2 lines
If you wanted to apply that run to exynos5250-snow I don't think it would be a terrible idea.
I can of course apply changes to Snow, but I cannot test them myself.
If you want to send up a patch like that I'm happy to give it a once over and also to test it. ...but don't feel obligated
Quoted 49 lines
Quoted 38 lines
Quoted 38 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +38
-0+ s5m8767_dvs: s5m8767-dvs { + samsung,pins = "gpd1-0", "gpd1-1", "gpd1-2"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>; + }; + + s5m8767_ds: s5m8767-ds { + samsung,pins = "gpx2-3", "gpx2-4", "gpx2-5"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>; + }; + + tps65090_irq: tps65090-irq { + samsung,pins = "gpx2-6"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + s5m8767_irq: s5m8767-irq { + samsung,pins = "gpx3-2"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + hdmi_hpd_irq: hdmi-hpd-irq { + samsung,pins = "gpx3-7"; + samsung,pin-function = <0>; + samsung,pin-pud = <1>; + samsung,pin-drv = <0>; + }; + }; + + pinctrl at 13400000 { + hsic_reset: hsic-reset {Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 63
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ samsung,pins = "gpe1-0"; + samsung,pin-function = <1>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + };I'm pretty sure that the HSIC reset needed some funky code to make it work and I'm not sure what the status of that is upstream
Yeah, you mentioned something along those lines. However it's the equivalent of the usb3-vbus-en in
-snow.dts. Rename or drop?
It's clearly not equivalent, usb3-vbus-en just turns on/off the load switch for VBUS
while the HSIC chip (USB3503) is an external PHY (and hub) for the USB2 port, the reset of the PHY side of the HSIC needs to be done within the proper timing window compared to the SoC HSIC controller to get a working link.
As the current kernel has a binding for the SMSC USB3503 and a PHY driver (which is in driver/usb/misc probably because it is an external PHY), the hsic-reset should be connected there (as "reset-gpios").
On top of that the HSIC link is a bit finicky and it needs a few quirks if we want it to survive all kind of events :
eg s5p_hub_control() in
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/usb/host/ehci-s5p.c
Quoted 3 lines
On snow locally I see USB2 vbus is gpx1-1 and USB3 vbus is gpx2-7. I don't see that in spring.
On Spring, the USB3 root hub is just connected to the internal camera (which is powered by one of the TPS65090 LDOs)
On the external USB2 ports, the port enable going to the VBUS load switch is controlled directly the USB3503 (which has an internal mechanism, configured by default as an automatic control of overcurrent protection, it can be bypass by using the USB3503 I2C registers).
Quoted 685 lines
This will take more time than I have right now to track down. I added Julius to the thread in case he has time to answer and can suggest what to do for upstream purposes. I may be able to look more tomorrow.
You can always send up the next version and include this and we'll look at it again.
Quoted 50 lines
Quoted 50 lines
Quoted 37 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +37
-0+ }; + + vbat: vbat-fixed-regulator { + compatible = "regulator-fixed"; + regulator-name = "vbat-supply"; + regulator-boot-on; + }; + + usb at 12000000 { + status = "okay"; + }; + + usb3_vbus_reg: regulator-usb3 { + compatible = "regulator-fixed"; + regulator-name = "P5.0V_USB3CON"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + gpio = <&gpe1 0 1>; + pinctrl-names = "default"; + pinctrl-0 = <&hsic_reset>; + enable-active-high; + }; + + phy at 12100000 { + vbus-supply = <&usb3_vbus_reg>; + }; + + usb at 12110000 { + samsung,vbus-gpio = <&gpx1 1 0>; + status = "okay"; + }; + + usb at 12120000 { + status = "okay"; + }; + + mmc at 12220000 {arch/arm/boot/dts/exynos5250-spring.dts line 110
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ /* MMC2 pins are used as GPIO for eDP bridge control. */ + status = "disabled"; + }; + + mmc at 12230000 { + status = "disabled"; + }; + + i2c at 12C60000 { + max77686 at 09 {There is no max77686 on spring. It uses s5m8767 in the place of max77686.
Quoted 3 lines
Quoted 1 line
...you've got "status = disabled", just remove it.
That's because it's inherited from
exynos5250-cros-common.dtsi.Ah, right. So if we're keeping cros-common the proper thing to do is to move it out of cros-common in a patch before this one. ...but I think people might be happier if you simply don't include cros-common.
Quoted 3 lines
The rtc that the system successfully uses is "s5m-rtc", which I guess is on the s5m8767 then? (Confusing that 3.8 Spring had this rtc node despite Snow's max77686 not having it.)
I think it's not really supposed to. It was a bit of a hack to handle the fact that the RTC has two different i2c addresses on both the 77686 and this PMIC. The max77686 driver upstream handles it all in code and doesn't ask the device tree to list both addresses.
I think that _upstream_ snow doesn't have the node but _downstream_ spring might have the node (though having a child of a disabled node isn't particularly useful).
Quoted 69 lines
Quoted 60 lines
Quoted 15 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +15
-0+ #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + + rtc { + reg = <0x6>; + }; + }; + + s5m8767_pmic at 66 { + compatible = "samsung,s5m8767-pmic"; + reg = <0x66>; + interrupt-parent = <&gpx3>; + interrupts = <2 0>; + pinctrl-names = "default";arch/arm/boot/dts/exynos5250-spring.dts line 135
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ pinctrl-0 = <&s5m8767_irq &s5m8767_dvs &s5m8767_ds>; + wakeup-source; + + s5m8767,pmic-buck-dvs-gpios = <&gpd1 0 1>, /* DVS1 */ + <&gpd1 1 1>, /* DVS2 */ + <&gpd1 2 1>; /* DVS3 */ + + s5m8767,pmic-buck-ds-gpios = <&gpx2 3 1>, /* SET1 */ + <&gpx2 4 1>, /* SET2 */ + <&gpx2 5 1>; /* SET3 */The final "1" in each of the GPIO specifiers above should be
GPIO_ACTIVE_LOW.Quoted 31 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +31
-0+ + /* + * The following arrays of DVS voltages are not used, since we are + * not using GPIOs to control PMIC bucks, but they must be defined + * to please the driver. + */ + s5m8767,pmic-buck2-dvs-voltage = <1350000>, <1300000>, + <1250000>, <1200000>, + <1150000>, <1100000>, + <1000000>, <950000>; + + s5m8767,pmic-buck3-dvs-voltage = <1100000>, <1100000>, + <1100000>, <1100000>, + <1000000>, <1000000>, + <1000000>, <1000000>; + + s5m8767,pmic-buck4-dvs-voltage = <1200000>, <1200000>, + <1200000>, <1200000>, + <1200000>, <1200000>, + <1200000>, <1200000>; + + clocks { + compatible = "samsung,s5m8767-clk"; + #clock-cells = <1>; + clock-output-names = "en32khz_ap", + "en32khz_cp", + "en32khz_bt"; + }; + + regulators { + s5m_ldo4_reg: LDO4 {Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 171
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ regulator-name = "P1.0V_LDO_OUT4"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <0>;I think that "
op_mode" here is questionable. Adding Javier to the thread who was looking at this for max77802 and possibly max77686.Quoted 1 line
Confirming that this
op_modeis present in the original 3.8 device tree.Yes and it needs to be handled eventually. It makes suspend/resume work properly. ...but I think it needs to be thought out better.
...but given that other users of this PMIC have it I guess it makes sense to leave it in. Javier was going to try to think it through better for max77686 and max77802.
Quoted 184 lines
(I used dtc to compile my
/proc/device-treetarball back to .dts, so it has hexadecimal <0x0> but that shouldn't matter to dtc AFAIU.)Quoted 148 lines
Quoted 148 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +148
-0+ }; + + s5m_ldo5_reg: LDO5 { + regulator-name = "P1.0V_LDO_OUT5"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo6_reg: LDO6 { + regulator-name = "vdd_mydp"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo7_reg: LDO7 { + regulator-name = "P1.1V_LDO_OUT7"; + regulator-min-microvolt = <1100000>; + regulator-max-microvolt = <1100000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo8_reg: LDO8 { + regulator-name = "P1.0V_LDO_OUT8"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo10_reg: LDO10 { + regulator-name = "P1.8V_LDO_OUT10"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo11_reg: LDO11 { + regulator-name = "P1.8V_LDO_OUT11"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo12_reg: LDO12 { + regulator-name = "P3.0V_LDO_OUT12"; + regulator-min-microvolt = <3000000>; + regulator-max-microvolt = <3000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo13_reg: LDO13 { + regulator-name = "P1.8V_LDO_OUT13"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo14_reg: LDO14 { + regulator-name = "P1.8V_LDO_OUT14"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo15_reg: LDO15 { + regulator-name = "P1.0V_LDO_OUT15"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo16_reg: LDO16 { + regulator-name = "P1.8V_LDO_OUT16"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + op_mode = <3>; + }; + + s5m_ldo17_reg: LDO17 { + regulator-name = "P2.8V_LDO_OUT17"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + regulator-always-on; + op_mode = <0>; + }; + + s5m_ldo25_reg: LDO25 { + regulator-name = "vdd_bridge"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + op_mode = <1>; + }; + + BUCK1 { + regulator-name = "vdd_mif"; + regulator-min-microvolt = <950000>; + regulator-max-microvolt = <1300000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK2 { + regulator-name = "vdd_arm"; + regulator-min-microvolt = <850000>; + regulator-max-microvolt = <1350000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK3 { + regulator-name = "vdd_int"; + regulator-min-microvolt = <900000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK4 { + regulator-name = "vdd_g3d"; + regulator-min-microvolt = <850000>; + regulator-max-microvolt = <1300000>; + regulator-boot-on; + op_mode = <3>; + }; + + BUCK5 { + regulator-name = "P1.8V_BUCK_OUT5"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + regulator-boot-on; + op_mode = <1>;Quoted 9 lines
arch/arm/boot/dts/exynos5250-spring.dts
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+ }; + + BUCK6 { + regulator-name = "P1.2V_BUCK_OUT6"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + regulator-boot>-on;
Quoted 14 lines
Quoted 14 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +14
-0+ op_mode = <0>; + }; + + BUCK9 { + regulator-name = "vdd_ummc"; + regulator-min-microvolt = <950000>; + regulator-max-microvolt = <3000000>; + regulator-always-on; + regulator-boot-on; + op_mode = <3>; + }; + }; + }; + };Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 345
Quoted 4 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +4
-0+ + i2c at 12C70000 { + trackpad { + status = "disabled";Having this bogus entry here doesn't add anything. Remove it until the trackpad should be added. See http://crbug.com/371114 for a slightly stale bug about trackpad.
Quoted 3 lines
You misunderstand: This resolves an error about the cypress,cyapa inherited from
-cros-common.dtsi. Spring uses a different device and two different I2C addresses.Ah, right. I forgot about cros5250-common.
Quoted 2 lines
Nodes named trackpad-bootloader and trackpad-alt are prepared here:
https://github.com/afaerber/linux/commits/spring-nextIt might work, but you run into pinctrl problems. You need to put the pinctrl node _somewhere_ and if you put it with the trackpad entries themselves then you get conflicts. ...and it doesn't belong in the i2c bus (though that's what we have locally in our tree)
See <http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/30953>
for some background.Quoted 19 lines
Quoted 19 lines
arch/arm/boot/dts/exynos5250-spring.dts line 350
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ }; + }; + + i2c at 12CA0000 { + embedded-controller {Add "
cros_ec" alias like snow.arch/arm/boot/dts/exynos5250-spring.dts line 357
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+ compatible = "google,cros-ec-i2c"; + reg = <0x1e>; + interrupts = <6 0>; + interrupt-parent = <&gpx1>; + wakeup-source; + + keyboard-controller {Don't include keyboard-controller here. Add:
Quoted 8 lines
Quoted 6 lines
#include "
cros-ec-keyboard.dtsi"...at the bottom. Note that I think that the spring EC has a special "charger" key that it uses to indicate when a charger was plugged in and unplugged. I'm not sure how that will end up getting supported upstream but you could just leave it out for now.
Is there such a pending patch for snow? That's what I modeled after.
Yeah, and it appears to be in linux-next. (1a395e3 ARM: dts: Use the cros-ec-keyboard fragment in exynos5250-snow). It actually ended up going through the tegra tree since it was in a series with a similar tegra change. I asked Kukjin to pick it up via merge but he didn't appear to.
Quoted 7 lines
Where is
cros-ec-keyboard.dtsi? Don't see it in
http://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/tree/arch/arm/boot/dts?h=for-next
or
http://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/tree/include/dt-bindings?h=for-nextAre you proposing I factor it out?
Quoted 8 lines
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts line 360
Quoted 3 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +3
-0+ compatible = "google,cros-ec-keyb"; + keypad,num-rows = <8>; + keypad,num-columns = <13>;Don't you need pinctrl here?
The keyboard is usable; what would pinctrl be needed for? There's none in the 3.8 device tree.
Generally it's good to have the pinctrl listed to configure the IRQ properly (get the pulls right). It might work without it but it's a little less ideal I think.
I see this, right?
pinctrl-names = "default"; pinctrl-0 = <&ec_irq>;...in the downstream "
arch/arm/boot/dts/exynos5250-spring.dts"Quoted 93 lines
Quoted 84 lines
Quoted 84 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +84
-0+ google,needs-ghost-filter; + linux,keymap = < + 0x0001007d /* L_META */ + 0x0002003b /* F1 */ + 0x00030030 /* B */ + 0x00040044 /* F10 */ + 0x00060031 /* N */ + 0x0008000d /* = */ + 0x000a0064 /* R_ALT */ + + 0x01010001 /* ESC */ + 0x0102003e /* F4 */ + 0x01030022 /* G */ + 0x01040041 /* F7 */ + 0x01060023 /* H */ + 0x01080028 /* ' */ + 0x01090043 /* F9 */ + 0x010b000e /* BKSPACE */ + + 0x0200001d /* L_CTRL */ + 0x0201000f /* TAB */ + 0x0202003d /* F3 */ + 0x02030014 /* T */ + 0x02040040 /* F6 */ + 0x0205001b /* ] */ + 0x02060015 /* Y */ + 0x02070056 /* 102ND */ + 0x0208001a /* [ */ + 0x02090042 /* F8 */ + + 0x03010029 /* GRAVE */ + 0x0302003c /* F2 */ + 0x03030006 /* 5 */ + 0x0304003f /* F5 */ + 0x03060007 /* 6 */ + 0x0308000c /* - */ + 0x030b002b /* \ */ + + 0x04000061 /* R_CTRL */ + 0x0401001e /* A */ + 0x04020020 /* D */ + 0x04030021 /* F */ + 0x0404001f /* S */ + 0x04050025 /* K */ + 0x04060024 /* J */ + 0x04080027 /* ; */ + 0x04090026 /* L */ + 0x040a002b /* \ */ + 0x040b001c /* ENTER */ + + 0x0501002c /* Z */ + 0x0502002e /* C */ + 0x0503002f /* V */ + 0x0504002d /* X */ + 0x05050033 /* , */ + 0x05060032 /* M */ + 0x0507002a /* L_SHIFT */ + 0x05080035 /* / */ + 0x05090034 /* . */ + 0x050B0039 /* SPACE */ + + 0x06010002 /* 1 */ + 0x06020004 /* 3 */ + 0x06030005 /* 4 */ + 0x06040003 /* 2 */ + 0x06050009 /* 8 */ + 0x06060008 /* 7 */ + 0x0608000b /* 0 */ + 0x0609000a /* 9 */ + 0x060a0038 /* L_ALT */ + 0x060b006c /* DOWN */ + 0x060c006a /* RIGHT */ + + 0x07010010 /* Q */ + 0x07020012 /* E */ + 0x07030013 /* R */ + 0x07040011 /* W */ + 0x07050017 /* I */ + 0x07060016 /* U */ + 0x07070036 /* R_SHIFT */ + 0x07080019 /* P */ + 0x07090018 /* O */ + 0x070b0067 /* UP */ + 0x070c0069 /* LEFT */Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 450
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ >; + }; + }; + + power-regulator { + compatible = "ti,tps65090";I doubt tps65090 will "just work". Does it?
Quoted 4 lines
Good question. How to tell? :) Not familiar with clocks and regulators. I don't see the nodes referenced anywhere, so I could just try to drop (as I would, as per my cover letter, propose for anything non-essential that turns controversial).
What does "dmesg | grep tps65090" say?
What does this show:
grep ""/sys/class/regulator/*/nameQuoted 1 line
If we drop tps65090, can we safely drop vbat-fixed-regulator as well?
Yes
Quoted 19 lines
Quoted 10 lines
On spring the tps65090 is not directly on the same i2c bus as the EC. It's actually hidden behind the EC.
Locally in the ChromeOS tree there appears to be a forked copy of the 65090 regulator driver that's in use just for spring. See this from the ChromeOS 3.8 tree:
./drivers/regulator/tps65090-regulator.c./drivers/regulator/cros_ec-tps65090.cQuoted 8 lines
The Spring version of the driver sends EC commands directly to access the tps65090.
It is possible (untested) that you could also talk to tps65090 over an i2c tunnel. On exynos5420-peach-pit we have a full fledged i2c tunnel, but you may be able to extend the tunnel to export an i2c tunnel for spring using something like
<https://chromium-review.googlesource.com/66116>Quoted 4 lines
The SBS battery thingy (not in this patch) should also be behind some tunnel. There's none defined in
-cros-common.dtsior in my patch, and still it shows up in dmesg to my surprise, complaining "Couldn't read remote-bus property".Might be due to the cros5250-common. ...but yes, the battery should be behind the tunnel too and the patches needed for Spring haven't been posted by anyone yet. I picked up a whole bunch of
cros_eccleanup patches and thought that the tunnel patches for spring could possible go up after those land.Quoted 108 lines
Quoted 108 lines
Quoted 64 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +64
-0+ reg = <0x48>; + + /* + * Config irq to disable internal pulls + * even though we run in polling mode. + */ + pinctrl-names = "default"; + pinctrl-0 = <&tps65090_irq>; + + vsys1-supply = <&vbat>; + vsys2-supply = <&vbat>; + vsys3-supply = <&vbat>; + infet1-supply = <&vbat>; + infet2-supply = <&vbat>; + infet3-supply = <&vbat>; + infet4-supply = <&vbat>; + infet5-supply = <&vbat>; + infet6-supply = <&vbat>; + infet7-supply = <&vbat>; + vsys-l1-supply = <&vbat>; + vsys-l2-supply = <&vbat>; + + regulators { + fet1 { + regulator-name = "vcd_led"; + regulator-min-microvolt = <12000000>; + regulator-max-microvolt = <12000000>; + }; + fet3 { + regulator-name = "wwan_r"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + fet5 { + regulator-name = "cam"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + fet6 { + regulator-name = "lcd_vdd"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + }; + fet7 { + regulator-name = "ts"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + }; + }; + + charger { + compatible = "ti,tps65090-charger"; + }; + }; + }; + + hdmi { + hdmi-en-supply = <&s5m_ldo8_reg>; + vdd-supply = <&s5m_ldo8_reg>; + vdd_osc-supply = <&s5m_ldo10_reg>; + vdd_pll-supply = <&s5m_ldo8_reg>; + };arch/arm/boot/dts/exynos5250-spring.dts line 524
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ + fimd at 14400000 { + status = "okay"; + samsung,invert-vclk; + }; + + dp-controller at 145B0000 { + status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&dp_hpd>;This is probably not right. It looks as if spring uses gpc3-0 for display port HPD (as a GPIO). The upstream has this in the
exynos5250-pinctrl.dtsias a different pin.I think you'll need to define your own pinctrl here.
Quoted 26 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +26
-0+ samsung,color-space = <0>; + samsung,dynamic-range = <0>; + samsung,ycbcr-coeff = <0>; + samsung,color-depth = <1>; + samsung,link-rate = <0x0a>; + samsung,lane-count = <1>; + samsung,hpd-gpio = <&gpc3 0 0>; + + display-timings { + native-mode = <&timing1>; + + timing1: timing at 1 { + clock-frequency = <70589280>; + hactive = <1366>; + vactive = <768>; + hfront-porch = <40>; + hback-porch = <40>; + hsync-len = <32>; + vback-porch = <10>; + vfront-porch = <12>; + vsync-len = <6>; + }; + }; + }; + + fixed-rate-clocks {Quoted 8 lines
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts line 556
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ xxti { + compatible = "samsung,clock-xxti"; + clock-frequency = <24000000>; + }; + }; +};Will check on the other suggestions.
-Doug
Andreas,
Quoted 16 lines · Andreas Färber
From: Andreas Färber <[email protected]>
Hello,
Based on the preinstalled 3.8 based ChromeOS kernel and previous 3.15 based attempts by Stephan and me that broke for 3.16, I've prepared a device tree for the HP Chromebook 11 aka Google Spring.
The first three patches should be good to go and contain documentation fixes found while comparing
exynos5250-snow.dtsvs./proc/device-tree.The main patch was tested using a chained non-verified U-Boot (simplefb) and a rootfs on USB-attached SD card.
The display goes dark unfortunately (drm bridge series not yet tested), but I am able to log in via ssh over USB ethernet adapter okay.
Quoted 6 lines
Audio support is likely missing as my focus was getting USB booting. Not included is touchpad support, as "atmel,atmel_mxt_tp" is not documented to be available upstream. And no
/dev/mmcblk0or Wifi yet. Also when the screen stayed on, the embedded controller's keymap seems hardcoded to US English with system settings not taking effect; but surely we don't want per-keyboard device tree files to remedy that.
+Benson may be able to answer this. I believe generally non-US keyboard layouts are handled at a higher level.
Quoted 6 lines · Doug Anderson
Quoted 3 lines
Also when the screen stayed on, the embedded controller's keymap seems hardcoded to US English with system settings not taking effect; but surely we don't want per-keyboard device tree files to remedy that.
+Benson may be able to answer this. I believe generally non-US keyboard layouts are handled at a higher level.
There's no such thing as a notion of US versus non-US keyboard layouts at the embedded controller level or even in the kernel. Indeed, this should all be handled in user space.
The chromeos firmware and kernel should return the correct key codes for every key pressed on keyboards with the ANSI layout (US based), or ISO (UK and most other countries).
The only differences are :
* the ISO keyboard has an extra key, which is immediately to the right of the Left Shift key. This must return KEY_102ND key code from the input layer.
* the ISO keyboard has a different location for the | \ key, which accomodates the upside L shaped Enter key on the right side of the keyboard. The keycode for this key is KEY_BACKSLASH.
Basically, all of this should be verified using evtest to test that the ec and kernel have the keys right.
If you are having other problems with keyboard layout being stuck to US QWERTY, please check your user space.
Signature · 3 lines
Benson Leung Software Engineer, Chrom* OS [email protected]
A message that is not in the archive (sent off-list or not yet received): CANLzEksRmsk1y2ZEjRVE8F8DjSUeW3AebaaDWL71Z4Gc9NvZeg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Am 23.06.2014 22:11, schrieb Benson Leung:
Quoted 24 lines · Doug Anderson
Quoted 6 lines
Quoted 3 lines
Also when the screen stayed on, the embedded controller's keymap seems hardcoded to US English with system settings not taking effect; but surely we don't want per-keyboard device tree files to remedy that.
+Benson may be able to answer this. I believe generally non-US keyboard layouts are handled at a higher level.
There's no such thing as a notion of US versus non-US keyboard layouts at the embedded controller level or even in the kernel. Indeed, this should all be handled in user space.
The chromeos firmware and kernel should return the correct key codes for every key pressed on keyboards with the ANSI layout (US based), or ISO (UK and most other countries).
The only differences are :
* the ISO keyboard has an extra key, which is immediately to the right of the Left Shift key. This must returnKEY_102NDkey code from the input layer.
* the ISO keyboard has a different location for the | \ key, which accomodates the upside L shaped Enter key on the right side of the keyboard. The keycode for this key isKEY_BACKSLASH.
Quoted 2 lines
Basically, all of this should be verified using evtest to test that the ec and kernel have the keys right.
Hm, we may be talking about two different things here? I have been doing a minimum system bring-up for 3.16, with openSUSE userspace. My YaST-selected system keymap (German with deadkeys) is not taking effect on German Spring at the *framebuffer console* (tty1) - evdev is not involved at that level AIUI.
Backspace and L-shaped enter keys work okay. The keymap here should be identical to that in the original German device and seemed to match that in the exynos5250-snow.dts file.
I just checked (w/ dp-controller, hdmi, fimd commented out in my patch):
* An external USB keyboard does not work correctly either.
* In X11 (xdm), both internal and USB keyboard work as expected.
Similar situation in ChromeOS IIRC, with keymap correct at graphical login but not on the right-arrow console - although I don't know the ChromeOS userland too well to judge if it was configured correctly.
Quoted 2 lines
If you are having other problems with keyboard layout being stuck to US QWERTY, please check your user space.
On my Raspberry Pi for instance, the equivalent openSUSE Factory works just fine with German keymap for USB keyboard. Might any related kernel config options be missing in exynos_defconfig? Anything in particular I should check in user space?
Cheers,
Andreas
Signature · 5 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- To unsubscribe, e-mail: [email protected] To contact the owner, e-mail: [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
Am 23.06.2014 23:24, schrieb Andreas Färber:
Quoted 34 lines
Am 23.06.2014 22:11, schrieb Benson Leung:
Quoted 24 lines · Doug Anderson
Quoted 6 lines
Quoted 3 lines
Also when the screen stayed on, the embedded controller's keymap seems hardcoded to US English with system settings not taking effect; but surely we don't want per-keyboard device tree files to remedy that.
+Benson may be able to answer this. I believe generally non-US keyboard layouts are handled at a higher level.
There's no such thing as a notion of US versus non-US keyboard layouts at the embedded controller level or even in the kernel. Indeed, this should all be handled in user space.
The chromeos firmware and kernel should return the correct key codes for every key pressed on keyboards with the ANSI layout (US based), or ISO (UK and most other countries).
The only differences are :
* the ISO keyboard has an extra key, which is immediately to the right of the Left Shift key. This must returnKEY_102NDkey code from the input layer.
* the ISO keyboard has a different location for the | \ key, which accomodates the upside L shaped Enter key on the right side of the keyboard. The keycode for this key isKEY_BACKSLASH.Quoted 2 lines
Basically, all of this should be verified using evtest to test that the ec and kernel have the keys right.
Hm, we may be talking about two different things here? I have been doing a minimum system bring-up for 3.16, with openSUSE userspace. My YaST-selected system keymap (German with deadkeys) is not taking effect on German Spring at the *framebuffer console* (tty1) - evdev is not involved at that level AIUI.
Quoted 7 lines
Backspace and L-shaped enter keys work okay. The keymap here should be identical to that in the original German device and seemed to match that in the
exynos5250-snow.dtsfile.I just checked (w/ dp-controller, hdmi, fimd commented out in my patch):
* An external USB keyboard does not work correctly either.
* In X11 (xdm), both internal and USB keyboard work as expected.
Another observation: Ctrl+o does not work on the Chromebook keyboard, whereas o and Shift+o or Ctrl+x do work as expected. On an external USB keyboard it works just fine. Same at the console and in X11.
Testcases are nano (saving) and gEdit (opening).
I tried the new cros-ec-keyboard.dtsi, no change; and given that the key itself works okay, I assume it's not the dt linux,keymap.
Anyone any suggestion how to debug?
Thanks,
Andreas
Quoted 14 lines
Similar situation in ChromeOS IIRC, with keymap correct at graphical login but not on the right-arrow console - although I don't know the ChromeOS userland too well to judge if it was configured correctly.
Quoted 2 lines
If you are having other problems with keyboard layout being stuck to US QWERTY, please check your user space.
On my Raspberry Pi for instance, the equivalent openSUSE Factory works just fine with German keymap for USB keyboard. Might any related kernel config options be missing in
exynos_defconfig? Anything in particular I should check in user space?Cheers,
Andreas
Signature · 5 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- To unsubscribe, e-mail: [email protected] To contact the owner, e-mail: [email protected]
Quoted 47 lines · Andreas Färber
Am 23.06.2014 23:24, schrieb Andreas Färber:
Quoted 34 lines
Am 23.06.2014 22:11, schrieb Benson Leung:
Quoted 24 lines · Doug Anderson
Quoted 6 lines
Quoted 3 lines
Also when the screen stayed on, the embedded controller's keymap seems hardcoded to US English with system settings not taking effect; but surely we don't want per-keyboard device tree files to remedy that.
+Benson may be able to answer this. I believe generally non-US keyboard layouts are handled at a higher level.
There's no such thing as a notion of US versus non-US keyboard layouts at the embedded controller level or even in the kernel. Indeed, this should all be handled in user space.
The chromeos firmware and kernel should return the correct key codes for every key pressed on keyboards with the ANSI layout (US based), or ISO (UK and most other countries).
The only differences are :
* the ISO keyboard has an extra key, which is immediately to the right of the Left Shift key. This must returnKEY_102NDkey code from the input layer.
* the ISO keyboard has a different location for the | \ key, which accomodates the upside L shaped Enter key on the right side of the keyboard. The keycode for this key isKEY_BACKSLASH.Quoted 2 lines
Basically, all of this should be verified using evtest to test that the ec and kernel have the keys right.
Hm, we may be talking about two different things here? I have been doing a minimum system bring-up for 3.16, with openSUSE userspace. My YaST-selected system keymap (German with deadkeys) is not taking effect on German Spring at the *framebuffer console* (tty1) - evdev is not involved at that level AIUI.
Quoted 7 lines
Backspace and L-shaped enter keys work okay. The keymap here should be identical to that in the original German device and seemed to match that in the
exynos5250-snow.dtsfile.I just checked (w/ dp-controller, hdmi, fimd commented out in my patch):
* An external USB keyboard does not work correctly either.
* In X11 (xdm), both internal and USB keyboard work as expected.Another observation: Ctrl+o does not work on the Chromebook keyboard, whereas o and Shift+o or Ctrl+x do work as expected. On an external USB keyboard it works just fine. Same at the console and in X11.
Quoted 6 lines
Testcases are nano (saving) and gEdit (opening).
I tried the new
cros-ec-keyboard.dtsi, no change; and given that the key itself works okay, I assume it's not the dt linux,keymap.Anyone any suggestion how to debug?
I guess Ctrl+U is not working either.
That's the combination of 2 factors :
a. a magic feature on Spring
b. a pre-existing bug in the cros_ec_keyb keyboard driver
a. On Spring, the AC adapter events are exported as a virtual key in the keyboard matrix
(mapped as KEY_BATTERY in the ChromeOS kernel)
b. the de-ghosting algorithm in the driver is slightly wrong.
In your case, the driver is seeing a (non existing) ghost when Ctrl + O + KEY_BATTERY are pressed, and discarding the key press. As KEY_BATTERY is a virtual key, this ghost obviously cannot exist. To verify that hypothesis, you can try to disconnect your charger (or connect it), and see if the key combination is now working.
We might need to upstream a patch similar to :
https://chromium-review.googlesource.com/#/c/178539/
to fix the de-ghosting and exclude KEY_BATTERY.
Quoted 18 lines
Quoted 7 lines
Similar situation in ChromeOS IIRC, with keymap correct at graphical login but not on the right-arrow console - although I don't know the ChromeOS userland too well to judge if it was configured correctly.
Quoted 2 lines
If you are having other problems with keyboard layout being stuck to US QWERTY, please check your user space.
Quoted 7 lines
On my Raspberry Pi for instance, the equivalent openSUSE Factory works just fine with German keymap for USB keyboard. Might any related kernel config options be missing in
exynos_defconfig? Anything in particular I should check in user space?Cheers,
Andreas--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Am 23.06.2014 03:21, schrieb Andreas Färber:
Quoted 2 lines
The display goes dark unfortunately (drm bridge series not yet tested), but I am able to log in via ssh over USB ethernet adapter okay.
Tracked this down to the dp-controller@145B0000 node: The pinctrl seems irrelevant, only commenting out the whole node helps keep the display. That is, hdmi and fimd nodes don't interfere.
Adding a pwm backlight node like on snow did not help. If it's supposed to work from GNOME's display brightness slider, it doesn't seem to work. The 3.8 tree had no backlight node at all.
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Andreas,
Quoted 7 lines · Andreas Färber
Am 23.06.2014 03:21, schrieb Andreas Färber:
Quoted 2 lines
The display goes dark unfortunately (drm bridge series not yet tested), but I am able to log in via ssh over USB ethernet adapter okay.
Tracked this down to the dp-controller@145B0000 node: The pinctrl seems irrelevant, only commenting out the whole node helps keep the display. That is, hdmi and fimd nodes don't interfere.
Adding Rahul from Samsung. I don't know that he's ever tested things on exynos5250-spring but he might be able to point you in the right direction.
Quoted 3 lines
Adding a pwm backlight node like on snow did not help. If it's supposed to work from GNOME's display brightness slider, it doesn't seem to work. The 3.8 tree had no backlight node at all.
I do know that we don't want the exynos pwm backlight for spring. On spring the backlight is controlled completely by the bridge chip which has a PWM on it.
-Doug
Quoted 9 lines · Andreas Färber
Am 23.06.2014 03:21, schrieb Andreas Färber:
Quoted 2 lines
The display goes dark unfortunately (drm bridge series not yet tested), but I am able to log in via ssh over USB ethernet adapter okay.
Tracked this down to the dp-controller@145B0000 node: The pinctrl seems irrelevant, only commenting out the whole node helps keep the display. That is, hdmi and fimd nodes don't interfere.
Adding a pwm backlight node like on snow did not help.
On this board, the backlight is driven by the Parade PS8622 DP-to-LVDS bridge (rather than a SoC PWM as on Snow).
You need to ensure that the PS8622 GPIOs (Reset/Sleep) are correctly driven oh, it seems that the PS8622 DRM bridge driver is still in-flight :
https://www.mail-archive.com/[email protected]/msg32243.html
Quoted 9 lines
If it's supposed
to work from GNOME's display brightness slider, it doesn't seem to work. The 3.8 tree had no backlight node at all.Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Vincent and Doug,
Am 25.06.2014 01:44, schrieb Vincent Palatin:
Quoted 9 lines · Andreas Färber
Quoted 8 lines
Am 23.06.2014 03:21, schrieb Andreas Färber:
Quoted 2 lines
The display goes dark unfortunately (drm bridge series not yet tested), but I am able to log in via ssh over USB ethernet adapter okay.
Tracked this down to the dp-controller@145B0000 node: The pinctrl seems irrelevant, only commenting out the whole node helps keep the display. That is, hdmi and fimd nodes don't interfere.
Quoted 7 lines
Quoted 1 line
Adding a pwm backlight node like on snow did not help.
On this board, the backlight is driven by the Parade PS8622 DP-to-LVDS bridge (rather than a SoC PWM as on Snow).
You need to ensure that the PS8622 GPIOs (Reset/Sleep) are correctly driven oh, it seems that the PS8622 DRM bridge driver is still in-flight :
https://www.mail-archive.com/[email protected]/msg32243.html
Yeah, that's the one I meant with "drm bridge series" above. :)
Thanks for confirming.
Cheers,
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg