ARM: dts: exynos: Prepare Spring
Subsystems (3)
v4 cover ARM: dts: exynos: Prepare Spring
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.
v4 fixes a pinctrl bug.
Not yet enabled are trackpad, Wifi and sound.
My rebasing branch for testing is here:
https://github.com/afaerber/linux/commits/spring-next
Regards,
Andreas
v1 -> v2:
* Dropped 3 documentation patches from series:
2 picked up,
https://patchwork.kernel.org/patch/4397271/
https://patchwork.kernel.org/patch/4397291/
1 already sent separately as v2, pending.
https://patchwork.kernel.org/patch/4424881/
* Minimized and beautified Spring device tree and to some degree Snow as well. See individual patches.
v2 -> v3:
* Rebased: Two -cros-common cleanup patches were already applied
* Lots of small cleanups for both Snow and Spring
* Enabled I2S in Spring for symmetry
* Added lid-switch and gpio-keys pinctrls to Spring
v3 -> v4:
* Fixed pin-function 1 -> 0 for dp-hpd pinctrl, style cleanup
Andreas F?rber (4):
ARM: dts: Fix MMC pinctrl for exynos5250-snow
ARM: dts: Fold exynos5250-cros-common into exynos5250-snow
ARM: dts: Clean up exynos5250-snow
ARM: dts: Add exynos5250-spring device tree
arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/exynos5250-cros-common.dtsi | 164 -------- arch/arm/boot/dts/exynos5250-snow.dts | 338 ++++++++++------ arch/arm/boot/dts/exynos5250-spring.dts | 539 ++++++++++++++++++++++++++ 4 files changed, 770 insertions(+), 272 deletions(-) delete mode 100644 arch/arm/boot/dts/exynos5250-cros-common.dtsi create mode 100644 arch/arm/boot/dts/exynos5250-spring.dts
Signature · 1 line
1.9.3
PATCH v4 1/4 ARM: dts: Fix MMC pinctrl for exynos5250-snow
The pinctrl properties should be on the device directly and not on the slot sub-node.
---
v3 -> v4: Unchanged
v3: New (Doug Anderson)
Redundant with Jaehoon Chung's general slot at 0 deprecation, in case that hits the tree earlier.
arch/arm/boot/dts/exynos5250-snow.dts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
arch/arm/boot/dts/exynos5250-snow.dts +2 -4
@@ -240,10 +240,8 @@ */ mmc at 12230000 { status = "okay"; - slot at 0 { - pinctrl-names = "default"; - pinctrl-0 = <&sd3_clk &sd3_cmd &sd3_bus4>; - }; + pinctrl-names = "default"; + pinctrl-0 = <&sd3_clk &sd3_cmd &sd3_bus4>; }; i2c at 12CD0000 {
Signature · 1 line
1.9.3
Re: [PATCH v4 1/4] ARM: dts: Fix MMC pinctrl for exynos5250-snow
Quoted 23 lines · Andreas F?rber
The pinctrl properties should be on the device directly and not on the slot sub-node.
Reported-by Doug Anderson<[email protected]>Cc Jaehoon Chung<[email protected]>Signed-off-by Andreas F?rber<[email protected]>---
v3 -> v4: Unchangedv3: New (Doug Anderson)
Redundant with Jaehoon Chung's general slot at 0 deprecation, in case that hits the tree earlier.arch/arm/boot/dts/exynos5250-snow.dts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)arch/arm/boot/dts/exynos5250-snow.dts +0
-0@@ -240,10 +240,8 @@ */ mmc at 12230000 {
arch/arm/boot/dts/exynos5250-snow.dts line 247
Quoted 10 lines
arch/arm/boot/dts/exynos5250-snow.dts +2
-4status = "okay"; - slot at 0 { - pinctrl-names = "default"; - pinctrl-0 =<&sd3_clk&sd3_cmd&sd3_bus4>; - }; + pinctrl-names = "default"; + pinctrl-0 =<&sd3_clk&sd3_cmd&sd3_bus4>; }; i2c at 12CD0000 {
Once I get Doug's reviewed tag on exynos5250-snow dt changes, I will take this series :) Though Doug suggested or reported...
Thanks,
Kukjin
Re: [PATCH v4 1/4] ARM: dts: Fix MMC pinctrl for exynos5250-snow
In reply to Andreas FärberQuoted 9 lines · Andreas F?rber
The pinctrl properties should be on the device directly and not on the slot sub-node.
Reported-by Doug Anderson <[email protected]>Cc Jaehoon Chung <[email protected]>Signed-off-by Andreas F?rber <[email protected]>---
v3 -> v4: Unchanged
Quoted 7 lines
v3: New (Doug Anderson)
Redundant with Jaehoon Chung's general slot at 0 deprecation, in case that hits the tree earlier.arch/arm/boot/dts/exynos5250-snow.dts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
Best regards,
Tomasz
PATCH v4 2/4 ARM: dts: Fold exynos5250-cros-common into exynos5250-snow
In reply to Andreas FärberThe remaining common ChromeOS pieces are fairly minor.
---
v3 -> v4: Unchanged
v2 -> v3:
* Renamed subject to match Kukjin's style
* Rebased onto MMC pinctrl bug fix (Doug Anderson)
v2: New (Doug Anderson)
arch/arm/boot/dts/exynos5250-cros-common.dtsi | 164 -------------------------- arch/arm/boot/dts/exynos5250-snow.dts | 164 +++++++++++++++++++++++--- 2 files changed, 145 insertions(+), 183 deletions(-) delete mode 100644 arch/arm/boot/dts/exynos5250-cros-common.dtsi
arch/arm/boot/dts/exynos5250-cros-common.dtsi deleted +0 -164
@@ -1,164 +0,0 @@ -/* - * Common device tree include for all Exynos 5250 boards based off of Daisy. - * - * Copyright (c) 2012 Google, Inc - * - * 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. -*/ - -/ { - aliases { - }; - - memory { - reg = <0x40000000 0x80000000>; - }; - - chosen { - }; - - pinctrl at 11400000 { - /* - * Disabled pullups since external part has its own pullups and - * double-pulling gets us out of spec in some cases. - */ - i2c2_bus: i2c2-bus { - samsung,pin-pud = <0>; - }; - }; - - i2c at 12C60000 { - status = "okay"; - samsung,i2c-sda-delay = <100>; - samsung,i2c-max-bus-freq = <378000>; - }; - - i2c at 12C70000 { - status = "okay"; - samsung,i2c-sda-delay = <100>; - samsung,i2c-max-bus-freq = <378000>; - }; - - i2c at 12C80000 { - status = "okay"; - samsung,i2c-sda-delay = <100>; - samsung,i2c-max-bus-freq = <66000>; - - hdmiddc at 50 { - compatible = "samsung,exynos4210-hdmiddc"; - reg = <0x50>; - }; - }; - - i2c at 12C90000 { - status = "okay"; - samsung,i2c-sda-delay = <100>; - samsung,i2c-max-bus-freq = <66000>; - }; - - i2c at 12CA0000 { - status = "okay"; - samsung,i2c-sda-delay = <100>; - samsung,i2c-max-bus-freq = <66000>; - }; - - i2c at 12CB0000 { - status = "okay"; - samsung,i2c-sda-delay = <100>; - samsung,i2c-max-bus-freq = <66000>; - }; - - i2c at 12CD0000 { - status = "okay"; - samsung,i2c-sda-delay = <100>; - samsung,i2c-max-bus-freq = <66000>; - }; - - i2c at 12CE0000 { - status = "okay"; - samsung,i2c-sda-delay = <100>; - samsung,i2c-max-bus-freq = <378000>; - - hdmiphy: hdmiphy at 38 { - compatible = "samsung,exynos4212-hdmiphy"; - reg = <0x38>; - }; - }; - - mmc at 12200000 { - num-slots = <1>; - supports-highspeed; - broken-cd; - card-detect-delay = <200>; - samsung,dw-mshc-ciu-div = <3>; - samsung,dw-mshc-sdr-timing = <2 3>; - samsung,dw-mshc-ddr-timing = <1 2>; - pinctrl-names = "default"; - pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_cd &sd0_bus4 &sd0_bus8>; - - slot at 0 { - reg = <0>; - bus-width = <8>; - }; - }; - - mmc at 12220000 { - num-slots = <1>; - supports-highspeed; - card-detect-delay = <200>; - samsung,dw-mshc-ciu-div = <3>; - samsung,dw-mshc-sdr-timing = <2 3>; - samsung,dw-mshc-ddr-timing = <1 2>; - pinctrl-names = "default"; - pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>; - - slot at 0 { - reg = <0>; - bus-width = <4>; - wp-gpios = <&gpc2 1 0>; - }; - }; - - mmc at 12230000 { - num-slots = <1>; - supports-highspeed; - broken-cd; - card-detect-delay = <200>; - samsung,dw-mshc-ciu-div = <3>; - samsung,dw-mshc-sdr-timing = <2 3>; - samsung,dw-mshc-ddr-timing = <1 2>; - /* See board-specific dts files for pin setup */ - - slot at 0 { - reg = <0>; - bus-width = <4>; - }; - }; - - spi_1: spi at 12d30000 { - status = "okay"; - samsung,spi-src-clk = <0>; - num-cs = <1>; - }; - - hdmi { - hpd-gpio = <&gpx3 7 0>; - pinctrl-names = "default"; - pinctrl-0 = <&hdmi_hpd_irq>; - phy = <&hdmiphy>; - ddc = <&i2c_2>; - }; - - gpio-keys { - compatible = "gpio-keys"; - - power { - label = "Power"; - gpios = <&gpx1 3 1>; - linux,code = <116>; /* KEY_POWER */ - gpio-key,wakeup; - }; - }; -};
arch/arm/boot/dts/exynos5250-snow.dts +145 -19
216 lines not on this page · raw message
Signature · 1 line
1.9.3
Re: [PATCH v4 2/4] ARM: dts: Fold exynos5250-cros-common into exynos5250-snow
Please see my comments inline.
Quoted 2 lines · Andreas F?rber
The remaining common ChromeOS pieces are fairly minor.
For people that might stumble upon this patch in future, it would be nice to specify the reason for this change.
Otherwise feel free to add:
Best regards,
Tomasz
PATCH v4 3/4 ARM: dts: Clean up exynos5250-snow
In reply to Andreas FärberUse the new style of referencing inherited nodes and use symbolic names.
---
v3 -> v4: Unchanged
v3: New (Doug Anderson)
arch/arm/boot/dts/exynos5250-snow.dts | 182 +++++++++++++++++----------------- 1 file changed, 90 insertions(+), 92 deletions(-)
arch/arm/boot/dts/exynos5250-snow.dts +90 -92
@@ -9,6 +9,8 @@ */ /dts-v1/; +#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/input/input.h> #include "exynos5250.dtsi" / {
@@ -30,85 +32,19 @@ status = "okay"; }; - pinctrl at 11400000 { - ec_irq: ec-irq { - samsung,pins = "gpx1-6"; - samsung,pin-function = <0>; - samsung,pin-pud = <0>; - samsung,pin-drv = <0>; - }; - - sd3_clk: sd3-clk { - samsung,pin-drv = <0>; - }; - - sd3_cmd: sd3-cmd { - samsung,pin-pud = <3>; - samsung,pin-drv = <0>; - }; - - sd3_bus4: sd3-bus-width4 { - samsung,pin-drv = <0>; - }; - - max98095_en: max98095-en { - samsung,pins = "gpx1-7"; - samsung,pin-function = <0>; - samsung,pin-pud = <3>; - samsung,pin-drv = <0>; - }; - - tps65090_irq: tps65090-irq { - samsung,pins = "gpx2-6"; - samsung,pin-function = <0>; - samsung,pin-pud = <0>; - samsung,pin-drv = <0>; - }; - - usb3_vbus_en: usb3-vbus-en { - samsung,pins = "gpx2-7"; - samsung,pin-function = <1>; - 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 { - arb_their_claim: arb-their-claim { - samsung,pins = "gpe0-4"; - samsung,pin-function = <0>; - samsung,pin-pud = <3>; - samsung,pin-drv = <0>; - }; - - arb_our_claim: arb-our-claim { - samsung,pins = "gpf0-3"; - samsung,pin-function = <1>; - samsung,pin-pud = <0>; - samsung,pin-drv = <0>; - }; - }; - gpio-keys { compatible = "gpio-keys"; power { label = "Power"; - gpios = <&gpx1 3 1>; - linux,code = <116>; /* KEY_POWER */ + gpios = <&gpx1 3 GPIO_ACTIVE_LOW>; + linux,code = <KEY_POWER>; gpio-key,wakeup; }; lid-switch { label = "Lid"; - gpios = <&gpx3 5 1>; + gpios = <&gpx3 5 GPIO_ACTIVE_LOW>; linux,input-type = <5>; /* EV_SW */ linux,code = <0>; /* SW_LID */ debounce-interval = <1>;
@@ -129,8 +65,8 @@ i2c-parent = <&{/i2c@12CA0000}>; - our-claim-gpio = <&gpf0 3 1>; - their-claim-gpios = <&gpe0 4 1>; + our-claim-gpio = <&gpf0 3 GPIO_ACTIVE_LOW>; + their-claim-gpios = <&gpe0 4 GPIO_ACTIVE_LOW>; slew-delay-us = <10>; wait-retry-us = <3000>; wait-free-us = <50000>;
@@ -239,19 +175,6 @@ }; }; - i2c at 12CD0000 { - max98095: codec at 11 { - compatible = "maxim,max98095"; - reg = <0x11>; - pinctrl-0 = <&max98095_en>; - pinctrl-names = "default"; - }; - }; - - i2s0: i2s at 03830000 { - status = "okay"; - }; - sound { compatible = "google,snow-audio-max98095";
@@ -265,18 +188,14 @@ regulator-name = "P5.0V_USB3CON"; regulator-min-microvolt = <5000000>; regulator-max-microvolt = <5000000>; - gpio = <&gpx2 7 0>; + gpio = <&gpx2 7 GPIO_ACTIVE_HIGH>; pinctrl-names = "default"; pinctrl-0 = <&usb3_vbus_en>; enable-active-high; }; - phy at 12100000 { - vbus-supply = <&usb3_vbus_reg>; - }; - usb at 12110000 { - samsung,vbus-gpio = <&gpx1 1 0>; + samsung,vbus-gpio = <&gpx1 1 GPIO_ACTIVE_HIGH>; }; fixed-rate-clocks {
@@ -287,7 +206,7 @@ }; hdmi { - hpd-gpio = <&gpx3 7 0>; + hpd-gpio = <&gpx3 7 GPIO_ACTIVE_HIGH>; pinctrl-names = "default"; pinctrl-0 = <&hdmi_hpd_irq>; phy = <&hdmiphy>;
@@ -322,7 +241,7 @@ samsung,color-depth = <1>; samsung,link-rate = <0x0a>; samsung,lane-count = <2>; - samsung,hpd-gpio = <&gpx0 7 0>; + samsung,hpd-gpio = <&gpx0 7 GPIO_ACTIVE_HIGH>; display-timings { native-mode = <&timing1>;
@@ -547,6 +466,13 @@ status = "okay"; samsung,i2c-sda-delay = <100>; samsung,i2c-max-bus-freq = <66000>; + + max98095: codec at 11 { + compatible = "maxim,max98095"; + reg = <0x11>; + pinctrl-0 = <&max98095_en>; + pinctrl-names = "default"; + }; }; &i2c_8 {
@@ -560,6 +486,10 @@ }; }; +&i2s0 { + status = "okay"; +}; + &mmc_0 { status = "okay"; num-slots = <1>;
@@ -625,6 +555,70 @@ samsung,pin-pud = <0>; samsung,pin-drv = <0>; }; + + ec_irq: ec-irq { + samsung,pins = "gpx1-6"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + max98095_en: max98095-en { + samsung,pins = "gpx1-7"; + samsung,pin-function = <0>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + + tps65090_irq: tps65090-irq { + samsung,pins = "gpx2-6"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + usb3_vbus_en: usb3-vbus-en { + samsung,pins = "gpx2-7"; + samsung,pin-function = <1>; + 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_1 { + arb_their_claim: arb-their-claim { + samsung,pins = "gpe0-4"; + samsung,pin-function = <0>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + + arb_our_claim: arb-our-claim { + samsung,pins = "gpf0-3"; + samsung,pin-function = <1>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; +}; + +&sd3_clk { + samsung,pin-drv = <0>; +}; + +&sd3_cmd { + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; +}; + +&sd3_bus4 { + samsung,pin-drv = <0>; }; &spi_1 {
@@ -633,4 +627,8 @@ num-cs = <1>; }; +&usbdrd_phy { + vbus-supply = <&usb3_vbus_reg>; +}; + #include "cros-ec-keyboard.dtsi"
Signature · 1 line
1.9.3
Re: [PATCH v4 3/4] ARM: dts: Clean up exynos5250-snow
Andreas,
Please see my comments inline.
Quoted 4 lines · Andreas F?rber
Use the new style of referencing inherited nodes and use symbolic names.
Suggested-by Doug Anderson <[email protected]>Signed-off-by Andreas F?rber <[email protected]>
[snip]
arch/arm/boot/dts/exynos5250-snow.dts line 199
Quoted 5 lines
arch/arm/boot/dts/exynos5250-snow.dts +1
-2- usb at 12110000 { - samsung,vbus-gpio = <&gpx1 1 0>; + samsung,vbus-gpio = <&gpx1 1 GPIO_ACTIVE_HIGH>; };
This is a higher level node being overridden, so it should be specified using references too.
arch/arm/boot/dts/exynos5250-snow.dts line 208
Quoted 6 lines
arch/arm/boot/dts/exynos5250-snow.dts +0
-0fixed-rate-clocks {@@ -287,7 +206,7 @@ }; hdmi {
Ditto.
Best regards,
Tomasz
Re: [PATCH v4 3/4] ARM: dts: Clean up exynos5250-snow
Am 31.07.2014 21:19, schrieb Tomasz Figa:
Quoted 22 lines
Andreas,
Please see my comments inline.
Quoted 4 lines · Andreas F?rber
Use the new style of referencing inherited nodes and use symbolic names.
Suggested-by Doug Anderson <[email protected]>Signed-off-by Andreas F?rber <[email protected]>[snip]
arch/arm/boot/dts/exynos5250-snow.dts line 199
Quoted 5 lines
arch/arm/boot/dts/exynos5250-snow.dts +1
-2- usb at 12110000 { - samsung,vbus-gpio = <&gpx1 1 0>; + samsung,vbus-gpio = <&gpx1 1 GPIO_ACTIVE_HIGH>; };This is a higher level node being overridden, so it should be specified using references too.
Quoted 1 line
Quoted 7 lines
arch/arm/boot/dts/exynos5250-snow.dts line 208
Quoted 5 lines
arch/arm/boot/dts/exynos5250-snow.dts +0
-0fixed-rate-clocks {@@ -287,7 +206,7 @@ }; hdmi {Ditto.
Same problem as with spring: We need the labels first!
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
PATCH v4 4/4 ARM: dts: Add exynos5250-spring device tree
In reply to Andreas FärberAdds initial support for the HP Chromebook 11.
---
v3 -> v4:
* Fixed samsung,pin-function 1 -> 0 for dp-hpd-gpio
* Replaced dp-hpd-gpio with existing dp_hpd, overriding it
v2 -> v3:
* Use GPIO_ACTIVE_{LOW,HIGH} (Doug Anderson)
* Use symbolic KEY_POWER instead of comment
* Moved hsic_reset to new USB3503 node's reset-gpios (Vincent Palatin)
* Use dp_hpd_gpio for dp-controller (Doug Anderson, Ajay Kumar)
* Override sd1_{clk,cmd,cd,bus4} pinctrl similar to Snow (Doug Anderson)
* Added ec_irq pinctrl for cros_ec (Doug Anderson)
* Reordered nodes to minimize diff against Snow (Doug Anderson)
* Dropped obsolete mmc_2 override (Doug Anderson)
* Added lid-switch node (Doug Anderson)
* Added gpio-keys pinctrl (Doug Anderson)
* Added bootargs to avoid empty /chosen node and to document console setting
* Renamed s5m8767_pmic node to avoid underscore
* Use new style for overriding inherited pinctrl nodes, too
* Enable i2s0 node
v1 -> v2:
* Use label-based overriding/extension of nodes. (Doug Anderson)
* Dropped tps65090 for now, until we know where to place it.
* Dropped non-Spring nodes from -cros-common.dtsi rather than disabling them.
* Enabled a missing MMC node for access to internal storage.
* Dropped display-timings from dp-controller node. (Ajay Kumar)
arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/exynos5250-spring.dts | 539 ++++++++++++++++++++++++++++++++ 2 files changed, 540 insertions(+) create mode 100644 arch/arm/boot/dts/exynos5250-spring.dts
arch/arm/boot/dts/Makefile +1 -0
@@ -76,6 +76,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 +539 -0
539 lines not on this page · raw message
Signature · 1 line
1.9.3
Re: [PATCH v4 4/4] ARM: dts: Add exynos5250-spring device tree
Always a bit late to the game.
One small comment inline.
Quoted 105 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]>---
v3 -> v4:
* Fixed samsung,pin-function 1 -> 0 for dp-hpd-gpio
* Replaced dp-hpd-gpio with existingdp_hpd, overriding itv2 -> v3:
* Use GPIO_ACTIVE_{LOW,HIGH} (Doug Anderson)
* Use symbolicKEY_POWERinstead of comment
* Movedhsic_resetto new USB3503 node's reset-gpios (Vincent Palatin)
* Usedp_hpd_gpiofor dp-controller (Doug Anderson, Ajay Kumar)
* Override sd1_{clk,cmd,cd,bus4} pinctrl similar to Snow (Doug Anderson)
* Addedec_irqpinctrl forcros_ec(Doug Anderson)
* Reordered nodes to minimize diff against Snow (Doug Anderson)
* Dropped obsoletemmc_2override (Doug Anderson)
* Added lid-switch node (Doug Anderson)
* Added gpio-keys pinctrl (Doug Anderson)
* Added bootargs to avoid empty /chosen node and to document console setting
* Renameds5m8767_pmicnode to avoid underscore
* Use new style for overriding inherited pinctrl nodes, too
* Enable i2s0 nodev1 -> v2:
* Use label-based overriding/extension of nodes. (Doug Anderson)
* Dropped tps65090 for now, until we know where to place it.
* Dropped non-Spring nodes from-cros-common.dtsirather than disabling them.
* Enabled a missing MMC node for access to internal storage.
* Dropped display-timings from dp-controller node. (Ajay Kumar)arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/exynos5250-spring.dts | 539 ++++++++++++++++++++++++++++++++ 2 files changed, 540 insertions(+) create mode 100644 arch/arm/boot/dts/exynos5250-spring.dtsarch/arm/boot/dts/Makefile +1
-0@@ -76,6 +76,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 +48
-0@@ -0,0 +1,539 @@ +/* + * 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/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +/dts-v1/; +#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/input/input.h> +#include "exynos5250.dtsi" + +/ { + model = "Google Spring"; + compatible = "google,spring", "samsung,exynos5250", "samsung,exynos5"; + + memory { + reg = <0x40000000 0x80000000>; + }; + + chosen { + bootargs = "console=tty1"; + }; + + gpio-keys { + compatible = "gpio-keys"; + pinctrl-names = "default"; + pinctrl-0 = <&power_key_irq>, <&lid_irq>; + + power { + label = "Power"; + gpios = <&gpx1 3 GPIO_ACTIVE_LOW>; + linux,code = <KEY_POWER>; + gpio-key,wakeup; + }; + + lid-switch { + label = "Lid"; + gpios = <&gpx3 5 GPIO_ACTIVE_LOW>; + linux,input-type = <5>; /* EV_SW */ + linux,code = <0>; /* SW_LID */ + debounce-interval = <1>; + gpio-key,wakeup; + };
arch/arm/boot/dts/exynos5250-spring.dts line 58
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ }; + + usb3_vbus_reg: regulator-usb3 { + compatible = "regulator-fixed"; + regulator-name = "P5.0V_USB3CON"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + gpio = <&gpe1 0 GPIO_ACTIVE_LOW>; + enable-active-high; + };
GPE1_0 GPIO is the HSIC hub (SMSC 3503) reset# line (already defined below afaik).
On this design there is no external USB3 port, so no VBUS reg/load switch for USB3.
Quoted 484 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +481
-1+ + usb at 12110000 { + samsung,vbus-gpio = <&gpx1 1 GPIO_ACTIVE_HIGH>; + }; + + usb-hub { + compatible = "smsc,usb3503a"; + reset-gpios = <&hsic_reset>; + }; + + fixed-rate-clocks { + xxti { + compatible = "samsung,clock-xxti"; + clock-frequency = <24000000>; + }; + }; + + hdmi { + hpd-gpio = <&gpx3 7 GPIO_ACTIVE_HIGH>; + pinctrl-names = "default"; + pinctrl-0 = <&hdmi_hpd_irq>; + phy = <&hdmiphy>; + ddc = <&i2c_2>; + hdmi-en-supply = <&s5m_ldo8_reg>; + vdd-supply = <&s5m_ldo8_reg>; + vdd_osc-supply = <&s5m_ldo10_reg>; + vdd_pll-supply = <&s5m_ldo8_reg>; + }; + + fimd at 14400000 { + status = "okay"; + samsung,invert-vclk; + }; + + dp-controller at 145B0000 { + status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&dp_hpd>; + 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 GPIO_ACTIVE_HIGH>; + }; +}; + +&dp_hpd { + samsung,pins = "gpc3-0"; + samsung,pin-function = <0>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; +}; + +&i2c_0 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <378000>; + + s5m8767-pmic at 66 { + compatible = "samsung,s5m8767-pmic"; + reg = <0x66>; + interrupt-parent = <&gpx3>; + interrupts = <2 0>; + pinctrl-names = "default"; + pinctrl-0 = <&s5m8767_irq &s5m8767_dvs &s5m8767_ds>; + wakeup-source; + + s5m8767,pmic-buck-dvs-gpios = <&gpd1 0 GPIO_ACTIVE_LOW>, /* DVS1 */ + <&gpd1 1 GPIO_ACTIVE_LOW>, /* DVS2 */ + <&gpd1 2 GPIO_ACTIVE_LOW>; /* DVS3 */ + + s5m8767,pmic-buck-ds-gpios = <&gpx2 3 GPIO_ACTIVE_LOW>, /* SET1 */ + <&gpx2 4 GPIO_ACTIVE_LOW>, /* SET2 */ + <&gpx2 5 GPIO_ACTIVE_LOW>; /* SET3 */ + + /* + * 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 { + regulator-name = "P1.0V_LDO_OUT4"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-always-on; + op_mode = <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; + regulator-boot-on; + op_mode = <3>; + }; + }; + }; +}; + +&i2c_1 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <378000>; +}; + +/* + * Disabled pullups since external part has its own pullups and + * double-pulling gets us out of spec in some cases. + */ +&i2c2_bus { + samsung,pin-pud = <0>; +}; + +&i2c_2 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <66000>; + + hdmiddc at 50 { + compatible = "samsung,exynos4210-hdmiddc"; + reg = <0x50>; + }; +}; + +&i2c_3 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <66000>; +}; + +&i2c_4 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <66000>; + + cros_ec: embedded-controller { + compatible = "google,cros-ec-i2c"; + reg = <0x1e>; + interrupts = <6 0>; + interrupt-parent = <&gpx1>; + wakeup-source; + pinctrl-names = "default"; + pinctrl-0 = <&ec_irq>; + }; +}; + +&i2c_5 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <66000>; +}; + +&i2c_7 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <66000>; +}; + +&i2c_8 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <378000>; + + hdmiphy: hdmiphy at 38 { + compatible = "samsung,exynos4212-hdmiphy"; + reg = <0x38>; + }; +}; + +&i2s0 { + status = "okay"; +}; + +&mmc_0 { + status = "okay"; + num-slots = <1>; + supports-highspeed; + broken-cd; + card-detect-delay = <200>; + samsung,dw-mshc-ciu-div = <3>; + samsung,dw-mshc-sdr-timing = <2 3>; + samsung,dw-mshc-ddr-timing = <1 2>; + pinctrl-names = "default"; + pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_cd &sd0_bus4 &sd0_bus8>; + + slot at 0 { + reg = <0>; + bus-width = <8>; + }; +}; + +&mmc_1 { + status = "okay"; + num-slots = <1>; + supports-highspeed; + broken-cd; + card-detect-delay = <200>; + samsung,dw-mshc-ciu-div = <3>; + samsung,dw-mshc-sdr-timing = <2 3>; + samsung,dw-mshc-ddr-timing = <1 2>; + pinctrl-names = "default"; + pinctrl-0 = <&sd1_clk &sd1_cmd &sd1_cd &sd1_bus4>; + + slot at 0 { + reg = <0>; + bus-width = <4>; + }; +}; + +&pinctrl_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>; + }; + + power_key_irq: power-key-irq { + samsung,pins = "gpx1-3"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + ec_irq: ec-irq { + samsung,pins = "gpx1-6"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + 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>; + }; + + s5m8767_irq: s5m8767-irq { + samsung,pins = "gpx3-2"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + lid_irq: lid-irq { + samsung,pins = "gpx3-5"; + 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_1 { + hsic_reset: hsic-reset { + samsung,pins = "gpe1-0"; + samsung,pin-function = <1>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; +}; + +&sd1_clk { + samsung,pin-drv = <0>; +}; + +&sd1_cmd { + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; +}; + +&sd1_cd { + samsung,pin-drv = <0>; +}; + +&sd1_bus4 { + samsung,pin-drv = <0>; +}; + +&spi_1 { + status = "okay"; + samsung,spi-src-clk = <0>; + num-cs = <1>; +}; + +&usbdrd_phy { + vbus-supply = <&usb3_vbus_reg>; +}; + +#include "cros-ec-keyboard.dtsi" --1.9.3
Re: [PATCH v4 4/4] ARM: dts: Add exynos5250-spring device tree
Hi,
Am 31.07.2014 19:00, schrieb Vincent Palatin:
Quoted 4 lines
Always a bit late to the game.
One small comment inline.Reviewed-by Vincent Palatin <[email protected]>
Thanks,
Quoted 11 lines · Andreas F?rber
arch/arm/boot/dts/exynos5250-spring.dts line 58
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+ usb3_vbus_reg: regulator-usb3 { + compatible = "regulator-fixed"; + regulator-name = "P5.0V_USB3CON"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + gpio = <&gpe1 0 GPIO_ACTIVE_LOW>; + enable-active-high; + };
Quoted 2 lines
GPE1_0GPIO is the HSIC hub (SMSC 3503) reset# line (already defined below afaik).
Yes, that was a suggestion you made on v1.
Quoted 2 lines
On this design there is no external USB3 port, so no VBUS reg/load switch for USB3.
Could you be a little clearer? Are you suggesting to drop the gpio property? I just re-tested that without the regulator node plus the vbus-supply below I don't get any USB2 (so maybe rename the regulator?).
Regards,
Andreas
Quoted 9 lines
Quoted 9 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +9
-0+ + usb at 12110000 { + samsung,vbus-gpio = <&gpx1 1 GPIO_ACTIVE_HIGH>; + }; + + usb-hub { + compatible = "smsc,usb3503a"; + reset-gpios = <&hsic_reset>; + };
[...]
Quoted 7 lines
Quoted 7 lines
+&usbdrd_phy {
arch/arm/boot/dts/exynos5250-spring.dts new file +4
-1+ vbus-supply = <&usb3_vbus_reg>; +}; + +#include "cros-ec-keyboard.dtsi" --1.9.3
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 v4 4/4] ARM: dts: Add exynos5250-spring device tree
Am 31.07.2014 19:14, schrieb Andreas F?rber:
Quoted 24 lines
Hi,
Am 31.07.2014 19:00, schrieb Vincent Palatin:
Quoted 4 lines
Always a bit late to the game.
One small comment inline.Reviewed-by Vincent Palatin <[email protected]>Thanks,
Quoted 11 lines · Andreas F?rber
arch/arm/boot/dts/exynos5250-spring.dts line 58
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+ usb3_vbus_reg: regulator-usb3 { + compatible = "regulator-fixed"; + regulator-name = "P5.0V_USB3CON"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + gpio = <&gpe1 0 GPIO_ACTIVE_LOW>; + enable-active-high; + };Quoted 2 lines
GPE1_0GPIO is the HSIC hub (SMSC 3503) reset# line (already defined below afaik).
Quoted 7 lines
Yes, that was a suggestion you made on v1.
Quoted 2 lines
On this design there is no external USB3 port, so no VBUS reg/load switch for USB3.
Could you be a little clearer? Are you suggesting to drop the gpio property?
Nah, doesn't work. Do we need a pinctrl on the usb-hub instead?
Andreas
Quoted 24 lines
I just re-tested that without the regulator node plus the vbus-supply below I don't get any USB2 (so maybe rename the regulator?).
Regards,
AndreasQuoted 9 lines
Quoted 9 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +9
-0+ + usb at 12110000 { + samsung,vbus-gpio = <&gpx1 1 GPIO_ACTIVE_HIGH>; + }; + + usb-hub { + compatible = "smsc,usb3503a"; + reset-gpios = <&hsic_reset>; + };[...]
Quoted 7 lines
Quoted 7 lines
+&usbdrd_phy {
arch/arm/boot/dts/exynos5250-spring.dts new file +4
-1+ vbus-supply = <&usb3_vbus_reg>; +}; + +#include "cros-ec-keyboard.dtsi" --1.9.3
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 v4 4/4] ARM: dts: Add exynos5250-spring device tree
In reply to Andreas FärberQuoted 24 lines · Andreas F?rber
Hi,
Am 31.07.2014 19:00, schrieb Vincent Palatin:
Quoted 4 lines
Always a bit late to the game.
One small comment inline.Reviewed-by Vincent Palatin <[email protected]>Thanks,
Quoted 11 lines · Andreas F?rber
arch/arm/boot/dts/exynos5250-spring.dts line 58
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+ usb3_vbus_reg: regulator-usb3 { + compatible = "regulator-fixed"; + regulator-name = "P5.0V_USB3CON"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + gpio = <&gpe1 0 GPIO_ACTIVE_LOW>; + enable-active-high; + };Quoted 2 lines
GPE1_0GPIO is the HSIC hub (SMSC 3503) reset# line (already defined below afaik).
Quoted 8 lines
Yes, that was a suggestion you made on v1.
Quoted 2 lines
On this design there is no external USB3 port, so no VBUS reg/load switch for USB3.
Could you be a little clearer? Are you suggesting to drop the gpio property? I just re-tested that without the regulator node plus the vbus-supply below I don't get any USB2 (so maybe rename the regulator?).
The 3503 PHY driver is not fully correct, so we probably need to keep this to get the right init timings when the bootloader has initiliazed it before.
but yes renaming the regulator to mention that's actually the hsic hub reset would make it clearer.
Quoted 21 lines
Quoted 9 lines
Quoted 9 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +9
-0+ + usb at 12110000 { + samsung,vbus-gpio = <&gpx1 1 GPIO_ACTIVE_HIGH>; + }; + + usb-hub { + compatible = "smsc,usb3503a"; + reset-gpios = <&hsic_reset>; + };[...]
Quoted 7 lines
Quoted 7 lines
+&usbdrd_phy {
arch/arm/boot/dts/exynos5250-spring.dts new file +4
-1+ vbus-supply = <&usb3_vbus_reg>; +}; + +#include "cros-ec-keyboard.dtsi" --1.9.3
--
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 v4 4/4] ARM: dts: Add exynos5250-spring device tree
In reply to Andreas FärberHi Andreas,
Sorry for joining the party a bit late, but there were patches with less people involved so I preferred to review them first.
You can find my comments inline.
Quoted 1 line · Andreas F?rber
Adds initial support for the HP Chromebook 11.
[snip]
arch/arm/boot/dts/exynos5250-spring.dts line 37
Quoted 9 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +9
-0+ gpio-keys { + compatible = "gpio-keys"; + pinctrl-names = "default"; + pinctrl-0 = <&power_key_irq>, <&lid_irq>; + + power { + label = "Power"; + gpios = <&gpx1 3 GPIO_ACTIVE_LOW>; + linux,code = <KEY_POWER>;
I assume the key is debounced in hardware, so there is no need for debounce-interval here. Is this correct?
Quoted 13 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +13
-0+ gpio-key,wakeup; + }; + + lid-switch { + label = "Lid"; + gpios = <&gpx3 5 GPIO_ACTIVE_LOW>; + linux,input-type = <5>; /* EV_SW */ + linux,code = <0>; /* SW_LID */ + debounce-interval = <1>; + gpio-key,wakeup; + }; + }; +
arch/arm/boot/dts/exynos5250-spring.dts line 60
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ usb3_vbus_reg: regulator-usb3 { + compatible = "regulator-fixed"; + regulator-name = "P5.0V_USB3CON"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + gpio = <&gpe1 0 GPIO_ACTIVE_LOW>; + enable-active-high; + }; + + usb at 12110000 {
Since this is a brand new dts file, it should use the reference based syntax, which would be something like
&usbhost {
...
};below the / { ... }; block.
arch/arm/boot/dts/exynos5250-spring.dts line 66
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ samsung,vbus-gpio = <&gpx1 1 GPIO_ACTIVE_HIGH>; + }; + + usb-hub { + compatible = "smsc,usb3503a"; + reset-gpios = <&hsic_reset>;
Hmm, why a -gpios property points to a pinctrl node? Shouldn't there be a phandle to GPIO bank + GPIO specifier instead?
arch/arm/boot/dts/exynos5250-spring.dts line 74
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>; + }; + };
This is also referencing a node from higher level, so it should be done using a reference.
arch/arm/boot/dts/exynos5250-spring.dts line 86
Quoted 12 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +12
-0+ + hdmi { + hpd-gpio = <&gpx3 7 GPIO_ACTIVE_HIGH>; + pinctrl-names = "default"; + pinctrl-0 = <&hdmi_hpd_irq>; + phy = <&hdmiphy>; + ddc = <&i2c_2>; + hdmi-en-supply = <&s5m_ldo8_reg>; + vdd-supply = <&s5m_ldo8_reg>; + vdd_osc-supply = <&s5m_ldo10_reg>; + vdd_pll-supply = <&s5m_ldo8_reg>; + };
Ditto.
arch/arm/boot/dts/exynos5250-spring.dts line 91
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ + fimd at 14400000 { + status = "okay"; + samsung,invert-vclk; + };
Ditto.
arch/arm/boot/dts/exynos5250-spring.dts line 104
Quoted 13 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +13
-0+ + dp-controller at 145B0000 { + status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&dp_hpd>; + 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 GPIO_ACTIVE_HIGH>; + };
Ditto.
arch/arm/boot/dts/exynos5250-spring.dts line 112
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+}; + +&dp_hpd { + samsung,pins = "gpc3-0"; + samsung,pin-function = <0>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; +};
Hmm, what node is this referencing? I believe this should rather reference the pin controller and add a new board-specific pinconf/pinmux group instead....
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ +&i2c_0 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <378000>;
[snip]
arch/arm/boot/dts/exynos5250-spring.dts line 354
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+/* + * Disabled pullups since external part has its own pullups and + * double-pulling gets us out of spec in some cases. + */ +&i2c2_bus { + samsung,pin-pud = <0>; +};
OK, here overriding a generic pinconf group is justified and nicely explained by a comment.
arch/arm/boot/dts/exynos5250-spring.dts line 364
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ +&i2c_2 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <66000>; + + hdmiddc at 50 { + compatible = "samsung,exynos4210-hdmiddc"; + reg = <0x50>; + };
I don't think this matches current Exynos HDMI bindings, which I believe have been changed to just take a phandle to i2c bus instead.
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+}; + +&i2c_3 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <66000>; +};
[snip]
Quoted 6 lines
+&sd1_clk {
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ samsung,pin-drv = <0>; +}; + +&sd1_cmd { + samsung,pin-pud = <3>;
arch/arm/boot/dts/exynos5250-spring.dts line 527
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ samsung,pin-drv = <0>; +}; + +&sd1_cd { + samsung,pin-drv = <0>; +}; + +&sd1_bus4 { + samsung,pin-drv = <0>; +};
Here generic settings are being overridden, so it might be a good idea to explain why, like with i2c pull-up above.
Best regards,
Tomasz
Re: [PATCH v4 4/4] ARM: dts: Add exynos5250-spring device tree
Hi Tomasz,
Am 31.07.2014 21:05, schrieb Tomasz Figa:
Quoted 12 lines
Hi Andreas,
Sorry for joining the party a bit late, but there were patches with less people involved so I preferred to review them first.
You can find my comments inline.
Quoted 1 line · Andreas F?rber
Adds initial support for the HP Chromebook 11.
[snip]
Quoted 12 lines
arch/arm/boot/dts/exynos5250-spring.dts line 37
Quoted 9 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +9
-0+ gpio-keys { + compatible = "gpio-keys"; + pinctrl-names = "default"; + pinctrl-0 = <&power_key_irq>, <&lid_irq>; + + power { + label = "Power"; + gpios = <&gpx1 3 GPIO_ACTIVE_LOW>; + linux,code = <KEY_POWER>;I assume the key is debounced in hardware, so there is no need for debounce-interval here. Is this correct?
You're asking the wrong person... This is copied from -cros-common/-snow. Downstream 3.8 does not have a debounce-interval property.
Quoted 28 lines
Quoted 14 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +13
-0+ gpio-key,wakeup; + }; + + lid-switch { + label = "Lid"; + gpios = <&gpx3 5 GPIO_ACTIVE_LOW>; + linux,input-type = <5>; /* EV_SW */ + linux,code = <0>; /* SW_LID */ + debounce-interval = <1>; + gpio-key,wakeup; + }; + }; +arch/arm/boot/dts/exynos5250-spring.dts line 60
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ usb3_vbus_reg: regulator-usb3 { + compatible = "regulator-fixed"; + regulator-name = "P5.0V_USB3CON"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + gpio = <&gpe1 0 GPIO_ACTIVE_LOW>; + enable-active-high; + }; + + usb at 12110000 {Since this is a brand new dts file, it should use the reference based syntax, which would be something like
Quoted 5 lines
&usbhost { ... };below the / { ... }; block.
You will find that I already did that for all nodes that have a label. Since there are lots of usb nodes, please suggest specific label names.
I originally tried to stay out of existing code, then I was asked to fix -cros-common, clean up -snow too, now the SoC, ... ;)
Quoted 9 lines
arch/arm/boot/dts/exynos5250-spring.dts line 66
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ samsung,vbus-gpio = <&gpx1 1 GPIO_ACTIVE_HIGH>; + }; + + usb-hub { + compatible = "smsc,usb3503a"; + reset-gpios = <&hsic_reset>;Hmm, why a -gpios property points to a pinctrl node? Shouldn't there be a phandle to GPIO bank + GPIO specifier instead?
Dunno, can change it. Can I just copy the gpio property from the regulator above?
Quoted 12 lines
arch/arm/boot/dts/exynos5250-spring.dts line 74
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>; + }; + };This is also referencing a node from higher level, so it should be done using a reference.
Quoted 14 lines
arch/arm/boot/dts/exynos5250-spring.dts line 86
Quoted 12 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +12
-0+ + hdmi { + hpd-gpio = <&gpx3 7 GPIO_ACTIVE_HIGH>; + pinctrl-names = "default"; + pinctrl-0 = <&hdmi_hpd_irq>; + phy = <&hdmiphy>; + ddc = <&i2c_2>; + hdmi-en-supply = <&s5m_ldo8_reg>; + vdd-supply = <&s5m_ldo8_reg>; + vdd_osc-supply = <&s5m_ldo10_reg>; + vdd_pll-supply = <&s5m_ldo8_reg>; + };Ditto.
hdmi?
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 91
Quoted 6 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ + fimd at 14400000 { + status = "okay"; + samsung,invert-vclk; + };Ditto.
fimd?
Quoted 16 lines
Quoted 4 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +3
-0+ + dp-controller at 145B0000 { + status = "okay";arch/arm/boot/dts/exynos5250-spring.dts line 104
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ pinctrl-names = "default"; + pinctrl-0 = <&dp_hpd>; + 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 GPIO_ACTIVE_HIGH>; + };Ditto.
dp_controller? display_port_controller?
Quoted 13 lines
arch/arm/boot/dts/exynos5250-spring.dts line 112
Quoted 9 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+}; + +&dp_hpd { + samsung,pins = "gpc3-0"; + samsung,pin-function = <0>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; +};Hmm, what node is this referencing? I believe this should rather reference the pin controller and add a new board-specific pinconf/pinmux group instead....
It's a -pinctrl node. See v3->v4 change log and discussion on v3.
Quoted 8 lines
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ +&i2c_0 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <378000>;[snip]
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts line 354
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+/* + * Disabled pullups since external part has its own pullups and + * double-pulling gets us out of spec in some cases. + */ +&i2c2_bus { + samsung,pin-pud = <0>; +};OK, here overriding a generic pinconf group is justified and nicely explained by a comment.
You seem to assume that I actually understand these things. ;) Just copied from -cros-common/-snow.
Quoted 5 lines
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ +&i2c_2 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <66000>;
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 364
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ + hdmiddc at 50 { + compatible = "samsung,exynos4210-hdmiddc"; + reg = <0x50>; + };I don't think this matches current Exynos HDMI bindings, which I believe have been changed to just take a phandle to i2c bus instead.
Copied from -cros-common/-snow.
Quoted 21 lines
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+}; + +&i2c_3 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <66000>; +};[snip]
Quoted 11 lines
+&sd1_clk {
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ samsung,pin-drv = <0>; +}; + +&sd1_cmd { + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; +}; + +&sd1_cd { + samsung,pin-drv = <0>;
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 527
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+}; + +&sd1_bus4 { + samsung,pin-drv = <0>; +};Here generic settings are being overridden, so it might be a good idea to explain why, like with i2c pull-up above.
Snow does not have an explanation either, so please suggest what comment you'd like to see. Consider me just a user with no specs. :)
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 v4 4/4] ARM: dts: Add exynos5250-spring device tree
Andreas,
Quoted 19 lines · Andreas F?rber
Am 31.07.2014 21:05, schrieb Tomasz Figa:
Quoted 5 lines · Andreas F?rber
Quoted 1 line
Adds initial support for the HP Chromebook 11.
[snip]
Quoted 12 lines
arch/arm/boot/dts/exynos5250-spring.dts line 37
Quoted 9 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +9
-0+ gpio-keys { + compatible = "gpio-keys"; + pinctrl-names = "default"; + pinctrl-0 = <&power_key_irq>, <&lid_irq>; + + power { + label = "Power"; + gpios = <&gpx1 3 GPIO_ACTIVE_LOW>; + linux,code = <KEY_POWER>;I assume the key is debounced in hardware, so there is no need for debounce-interval here. Is this correct?
Quoted 3 lines
You're asking the wrong person... This is copied from -cros-common/-snow. Downstream 3.8 does not have a debounce-interval property.
Doug, Vincent?
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
(quoted diff) +2
-0+ gpio-key,wakeup; + };
[snip]
Quoted 10 lines
Quoted 10 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ + usb at 12110000 {Since this is a brand new dts file, it should use the reference based syntax, which would be something like
&usbhost { ... };
Quoted 7 lines
Quoted 1 line
below the / { ... }; block.
You will find that I already did that for all nodes that have a label. Since there are lots of usb nodes, please suggest specific label names.
I originally tried to stay out of existing code, then I was asked to fix -cros-common, clean up -snow too, now the SoC, ... ;)
Well, adding labels should be pretty trivial and IMHO could be squashed into this patch. For usb at 1211000 the right label would be "ehci".
Quoted 11 lines
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts line 66
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +6
-0+ samsung,vbus-gpio = <&gpx1 1 GPIO_ACTIVE_HIGH>; + }; + + usb-hub { + compatible = "smsc,usb3503a"; + reset-gpios = <&hsic_reset>;Hmm, why a -gpios property points to a pinctrl node? Shouldn't there be a phandle to GPIO bank + GPIO specifier instead?
Quoted 2 lines
Dunno, can change it. Can I just copy the gpio property from the regulator above?
Reading what Vincent posted earlier I would consider this as the right thing to do and it might even let you remove the fake regulator node.
Quoted 28 lines
Quoted 13 lines
arch/arm/boot/dts/exynos5250-spring.dts line 74
Quoted 9 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+ }; + + fixed-rate-clocks { + xxti { + compatible = "samsung,clock-xxti"; + clock-frequency = <24000000>; + }; + };This is also referencing a node from higher level, so it should be done using a reference.
Quoted 14 lines
arch/arm/boot/dts/exynos5250-spring.dts line 86
Quoted 12 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +12
-0+ + hdmi { + hpd-gpio = <&gpx3 7 GPIO_ACTIVE_HIGH>; + pinctrl-names = "default"; + pinctrl-0 = <&hdmi_hpd_irq>; + phy = <&hdmiphy>; + ddc = <&i2c_2>; + hdmi-en-supply = <&s5m_ldo8_reg>; + vdd-supply = <&s5m_ldo8_reg>; + vdd_osc-supply = <&s5m_ldo10_reg>; + vdd_pll-supply = <&s5m_ldo8_reg>; + };Ditto.
Quoted 1 line
hdmi?
Sounds good.
Quoted 11 lines
Quoted 9 lines
arch/arm/boot/dts/exynos5250-spring.dts line 91
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ + fimd at 14400000 { + status = "okay"; + samsung,invert-vclk; + };Ditto.
fimd?
OK.
Quoted 18 lines
Quoted 17 lines
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +3
-0+ + dp-controller at 145B0000 { + status = "okay";arch/arm/boot/dts/exynos5250-spring.dts line 104
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ pinctrl-names = "default"; + pinctrl-0 = <&dp_hpd>; + 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 GPIO_ACTIVE_HIGH>; + };Ditto.
Quoted 1 line
dp_controller?display_port_controller?
dp?
Quoted 15 lines
Quoted 14 lines
arch/arm/boot/dts/exynos5250-spring.dts line 112
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +8
-0+}; + +&dp_hpd { + samsung,pins = "gpc3-0"; + samsung,pin-function = <0>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; +};Hmm, what node is this referencing? I believe this should rather reference the pin controller and add a new board-specific pinconf/pinmux group instead....
Quoted 2 lines
It's a -pinctrl node. See
v3->v4change log and discussion on v3.
Well, this is clearly a board specific node anyway, because it does not refer to a special function, but simply an input/interrupt GPIO. If it somehow has landed in generic pinctrl dtsi then it should be removed from there and this patch should simply introduce its own instance of dp_hpd node, so you did the right thing in v3.
Quoted 19 lines
Quoted 8 lines
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ +&i2c_0 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <378000>;[snip]
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts line 354
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+/* + * Disabled pullups since external part has its own pullups and + * double-pulling gets us out of spec in some cases. + */ +&i2c2_bus { + samsung,pin-pud = <0>; +};OK, here overriding a generic pinconf group is justified and nicely explained by a comment.
Quoted 3 lines
You seem to assume that I actually understand these things. ;) Just copied from -cros-common/-snow.
It is good if those things are being done with some level of understanding. The DT mechanics are quite well documented in Documentation/devicetree/bindings, while for HW-specific bits I believe Chromium guys could give you a hand.
Quoted 14 lines
Quoted 5 lines
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ +&i2c_2 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <66000>;Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 364
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ + hdmiddc at 50 { + compatible = "samsung,exynos4210-hdmiddc"; + reg = <0x50>; + };I don't think this matches current Exynos HDMI bindings, which I believe have been changed to just take a phandle to i2c bus instead.
Quoted 2 lines
Copied from -cros-common/-snow.
This means that somebody probably forgot to update -cros-common/-snow dts(i). Not the first and I guess probably not the last time...
Quoted 30 lines
Quoted 21 lines
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+}; + +&i2c_3 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <66000>; +};[snip]
Quoted 11 lines
+&sd1_clk {
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ samsung,pin-drv = <0>; +}; + +&sd1_cmd { + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; +}; + +&sd1_cd { + samsung,pin-drv = <0>;Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 527
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+}; + +&sd1_bus4 { + samsung,pin-drv = <0>; +};Here generic settings are being overridden, so it might be a good idea to explain why, like with i2c pull-up above.
Quoted 2 lines
Snow does not have an explanation either, so please suggest what comment you'd like to see. Consider me just a user with no specs. :)
Doug, Vincent, someone else?
Best regards,
Tomasz
Re: [PATCH v4 4/4] ARM: dts: Add exynos5250-spring device tree
Am 31.07.2014 21:40, schrieb Tomasz Figa:
Quoted 10 lines · Andreas F?rber
Quoted 9 lines
Am 31.07.2014 21:05, schrieb Tomasz Figa:
Quoted 7 lines · Andreas F?rber
arch/arm/boot/dts/exynos5250-spring.dts line 66
Quoted 3 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +3
-0+ usb-hub { + compatible = "smsc,usb3503a"; + reset-gpios = <&hsic_reset>;Hmm, why a -gpios property points to a pinctrl node? Shouldn't there be a phandle to GPIO bank + GPIO specifier instead?
Quoted 5 lines
Quoted 2 lines
Dunno, can change it. Can I just copy the gpio property from the regulator above?
Reading what Vincent posted earlier I would consider this as the right thing to do and it might even let you remove the fake regulator node.
Indeed it does, thanks for spotting this!
[...]
Quoted 11 lines
Quoted 11 lines
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts line 112
Quoted 6 lines
+&dp_hpd {
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ samsung,pins = "gpc3-0"; + samsung,pin-function = <0>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; +};Hmm, what node is this referencing? I believe this should rather reference the pin controller and add a new board-specific pinconf/pinmux group instead....
Quoted 8 lines
Quoted 2 lines
It's a -pinctrl node. See
v3->v4change log and discussion on v3.Well, this is clearly a board specific node anyway, because it does not refer to a special function, but simply an input/interrupt GPIO. If it somehow has landed in generic pinctrl dtsi then it should be removed from there and this patch should simply introduce its own instance of
dp_hpdnode, so you did the right thing in v3.
Well, my point was that the 3.8 tree contains only one dp-hpd node, not two as we would get by adding a new node here.
Apart from Spring, it's used in Snow and SMDK5250, so moving it there seems feasible and the cleanest solution to me.
Quoted 13 lines
Quoted 13 lines
Quoted 12 lines
[snip]
arch/arm/boot/dts/exynos5250-spring.dts line 354
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+/* + * Disabled pullups since external part has its own pullups and + * double-pulling gets us out of spec in some cases. + */ +&i2c2_bus { + samsung,pin-pud = <0>; +};OK, here overriding a generic pinconf group is justified and nicely explained by a comment.
Quoted 8 lines
Quoted 3 lines
You seem to assume that I actually understand these things. ;) Just copied from -cros-common/-snow.
It is good if those things are being done with some level of understanding. The DT mechanics are quite well documented in
Documentation/devicetree/bindings, while for HW-specific bits I believe Chromium guys could give you a hand.
I did read and even fix documentation for those bindings that I added myself in Spring, just not for those that were already in common code, like this one here.
A tps65090 patch has been ignored since being asked to extend the commit message, v3 was recently sent. Help getting that in appreciated.
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 v4 4/4] ARM: dts: Add exynos5250-spring device tree
Quoted 4 lines · Andreas F?rber
Am 31.07.2014 21:40, schrieb Tomasz Figa:
Quoted 3 lines · Andreas F?rber
Quoted 2 lines
Am 31.07.2014 21:05, schrieb Tomasz Figa:
Quoted 1 line · Andreas F?rber
[snip]
Quoted 20 lines
Quoted 11 lines
Quoted 11 lines
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts line 112
Quoted 6 lines
+&dp_hpd {
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ samsung,pins = "gpc3-0"; + samsung,pin-function = <0>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; +};Hmm, what node is this referencing? I believe this should rather reference the pin controller and add a new board-specific pinconf/pinmux group instead....
Quoted 8 lines
Quoted 2 lines
It's a -pinctrl node. See
v3->v4change log and discussion on v3.Well, this is clearly a board specific node anyway, because it does not refer to a special function, but simply an input/interrupt GPIO. If it somehow has landed in generic pinctrl dtsi then it should be removed from there and this patch should simply introduce its own instance of
dp_hpdnode, so you did the right thing in v3.
Quoted 6 lines
Well, my point was that the 3.8 tree contains only one dp-hpd node, not two as we would get by adding a new node here.
Apart from Spring, it's used in Snow and SMDK5250, so moving it there seems feasible and the cleanest solution to me.
What I mean is that in exynos5250-pinctrl.dtsi only generic SoC pin groups should be defined and those more or less correspond to groups with samsung,pin-function set to something other than 0 (input) or 1 (output). Now here hpd_gpio is just a normal GPIO input used as interrupt source to detect when a cable is plugged or unplugged. This is by no means generic to the SoC, because any GPIO with interrupt capability can be used for this purpose. This means that the whole pin{conf,mux} group should be defined on board level.
Best regards,
Tomasz
Quoted 17 lines
Quoted 17 lines
Quoted 16 lines
Quoted 12 lines
[snip]
arch/arm/boot/dts/exynos5250-spring.dts line 354
Quoted 7 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +7
-0+/* + * Disabled pullups since external part has its own pullups and + * double-pulling gets us out of spec in some cases. + */ +&i2c2_bus { + samsung,pin-pud = <0>; +};OK, here overriding a generic pinconf group is justified and nicely explained by a comment.
You seem to assume that I actually understand these things. ;) Just copied from -cros-common/-snow.
Quoted 8 lines
Quoted 4 lines
It is good if those things are being done with some level of understanding. The DT mechanics are quite well documented in
Documentation/devicetree/bindings, while for HW-specific bits I believe Chromium guys could give you a hand.I did read and even fix documentation for those bindings that I added myself in Spring, just not for those that were already in common code, like this one here.
My intention was that if there is something not clear, rather than blindly copy-pasting it, it might be worth to ask people that might know. Although any issues should generally be found at review stage, so I don't really have any objections, assuming that Chromium people assure that this patch adds valid data indeed.
Quoted 3 lines
A tps65090 patch has been ignored since being asked to extend the commit message, v3 was recently sent. Help getting that in appreciated.
Will take a look if time allows. Thanks for your work on this.
Best regards,
Tomasz
Re: [PATCH v4 4/4] ARM: dts: Add exynos5250-spring device tree
Am 01.08.2014 01:26, schrieb Tomasz Figa:
Quoted 33 lines · Andreas F?rber
Quoted 4 lines
Am 31.07.2014 21:40, schrieb Tomasz Figa:
Quoted 3 lines · Andreas F?rber
Quoted 2 lines
Am 31.07.2014 21:05, schrieb Tomasz Figa:
Quoted 1 line · Andreas F?rber
[snip]
Quoted 20 lines
Quoted 11 lines
Quoted 11 lines
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts line 112
Quoted 6 lines
+&dp_hpd {
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ samsung,pins = "gpc3-0"; + samsung,pin-function = <0>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; +};Hmm, what node is this referencing? I believe this should rather reference the pin controller and add a new board-specific pinconf/pinmux group instead....
Quoted 8 lines
Quoted 2 lines
It's a -pinctrl node. See
v3->v4change log and discussion on v3.Well, this is clearly a board specific node anyway, because it does not refer to a special function, but simply an input/interrupt GPIO. If it somehow has landed in generic pinctrl dtsi then it should be removed from there and this patch should simply introduce its own instance of
dp_hpdnode, so you did the right thing in v3.Quoted 6 lines
Well, my point was that the 3.8 tree contains only one dp-hpd node, not two as we would get by adding a new node here.
Apart from Spring, it's used in Snow and SMDK5250, so moving it there seems feasible and the cleanest solution to me.
Quoted 8 lines
What I mean is that in
exynos5250-pinctrl.dtsionly generic SoC pin groups should be defined and those more or less correspond to groups with samsung,pin-function set to something other than 0 (input) or 1 (output). Now herehpd_gpiois just a normal GPIO input used as interrupt source to detect when a cable is plugged or unplugged. This is by no means generic to the SoC, because any GPIO with interrupt capability can be used for this purpose. This means that the whole pin{conf,mux} group should be defined on board level.
Exactly what I meant! :)
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 v4 4/4] ARM: dts: Add exynos5250-spring device tree
In reply to Tomasz FigaTomasz,
Quoted 19 lines · Tomasz Figa
Andreas,
Quoted 16 lines · Andreas F?rber
Am 31.07.2014 21:05, schrieb Tomasz Figa:
Quoted 15 lines · Andreas F?rber
Quoted 1 line
Adds initial support for the HP Chromebook 11.
[snip]
Quoted 9 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +9
-0+ gpio-keys { + compatible = "gpio-keys"; + pinctrl-names = "default"; + pinctrl-0 = <&power_key_irq>, <&lid_irq>; + + power { + label = "Power"; + gpios = <&gpx1 3 GPIO_ACTIVE_LOW>; + linux,code = <KEY_POWER>;
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
I assume the key is debounced in hardware, so there is no need for debounce-interval here. Is this correct?
You're asking the wrong person... This is copied from -cros-common/-snow. Downstream 3.8 does not have a debounce-interval property.
Doug, Vincent?
Not something I've looked at, but it's never been a problem...
Quoted 20 lines
Quoted 20 lines
Quoted 19 lines
Quoted 6 lines
+&sd1_clk {
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ samsung,pin-drv = <0>; +}; + +&sd1_cmd { + samsung,pin-pud = <3>;arch/arm/boot/dts/exynos5250-spring.dts line 527
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ samsung,pin-drv = <0>; +}; + +&sd1_cd { + samsung,pin-drv = <0>; +}; + +&sd1_bus4 { + samsung,pin-drv = <0>; +};Here generic settings are being overridden, so it might be a good idea to explain why, like with i2c pull-up above.
Quoted 4 lines
Quoted 2 lines
Snow does not have an explanation either, so please suggest what comment you'd like to see. Consider me just a user with no specs. :)
Doug, Vincent, someone else?
The comment is just in a different place--it's in the dw_mmc node. Probably belongs here, though:
/*
* Wifi is a SiP, so can keep drive strengths low
* to reduce EMI.
*/
I guess the cmd line isn't documented. There is no external pull on the command line and on most boards there is one. Our hardware guys thought we didn't need it and apparently we don't...
Re: [PATCH v4 4/4] ARM: dts: Add exynos5250-spring device tree
Am 02.08.2014 07:15, schrieb Doug Anderson:
Quoted 29 lines · Tomasz Figa
Quoted 23 lines · Andreas F?rber
Quoted 22 lines
Am 31.07.2014 21:05, schrieb Tomasz Figa:
Quoted 20 lines · Andreas F?rber
Quoted 6 lines
+&sd1_clk {
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ samsung,pin-drv = <0>; +}; + +&sd1_cmd { + samsung,pin-pud = <3>;arch/arm/boot/dts/exynos5250-spring.dts line 527
Quoted 10 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +10
-0+ samsung,pin-drv = <0>; +}; + +&sd1_cd { + samsung,pin-drv = <0>; +}; + +&sd1_bus4 { + samsung,pin-drv = <0>; +};Here generic settings are being overridden, so it might be a good idea to explain why, like with i2c pull-up above.
Quoted 4 lines
Quoted 2 lines
Snow does not have an explanation either, so please suggest what comment you'd like to see. Consider me just a user with no specs. :)
Doug, Vincent, someone else?
Quoted 8 lines
The comment is just in a different place--it's in the
dw_mmcnode. Probably belongs here, though:/*
* Wifi is a SiP, so can keep drive strengths low
* to reduce EMI.
*/
I did copy such a comment for the MMC node in v5, still present in v6.
Andreas
Quoted 3 lines
I guess the cmd line isn't documented. There is no external pull on the command line and on most boards there is one. Our hardware guys thought we didn't need it and apparently we don't...
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 v4 4/4] ARM: dts: Add exynos5250-spring device tree
In reply to Tomasz FigaAm 31.07.2014 21:05, schrieb Tomasz Figa:
Quoted 11 lines
arch/arm/boot/dts/exynos5250-spring.dts line 74
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>; + }; + };This is also referencing a node from higher level, so it should be done using a reference.
Actually this one isn't! Neither fixed-rate-clocks nor xxti exists in the .dtsi.
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 v4 4/4] ARM: dts: Add exynos5250-spring device tree
Quoted 13 lines · Andreas F?rber
Am 31.07.2014 21:05, schrieb Tomasz Figa:
Quoted 11 lines
arch/arm/boot/dts/exynos5250-spring.dts line 74
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>; + }; + };This is also referencing a node from higher level, so it should be done using a reference.
Quoted 2 lines
Actually this one isn't! Neither fixed-rate-clocks nor xxti exists in the .dtsi.
You've got me here. :)
In new dts I actually suggest to specify those on SoC level, since they are generic SoC inputs and just override their frequencies on board level, but apparently Exynos5250 doesn't follow this fashion. So this is fine as is, sorry for noise.
Best regards,
Tomasz
Re: [PATCH v4 4/4] ARM: dts: Add exynos5250-spring device tree
In reply to Tomasz FigaAm 31.07.2014 21:05, schrieb Tomasz Figa:
Quoted 5 lines
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ +&i2c_2 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <66000>;
Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 364
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ + hdmiddc at 50 { + compatible = "samsung,exynos4210-hdmiddc"; + reg = <0x50>; + };I don't think this matches current Exynos HDMI bindings, which I believe have been changed to just take a phandle to i2c bus instead.
Looks correct to me:
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 v4 4/4] ARM: dts: Add exynos5250-spring device tree
Quoted 15 lines · Andreas F?rber
Am 31.07.2014 21:05, schrieb Tomasz Figa:
Quoted 5 lines
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ +&i2c_2 { + status = "okay"; + samsung,i2c-sda-delay = <100>; + samsung,i2c-max-bus-freq = <66000>;Quoted 8 lines
arch/arm/boot/dts/exynos5250-spring.dts line 364
Quoted 5 lines
arch/arm/boot/dts/exynos5250-spring.dts new file +5
-0+ + hdmiddc at 50 { + compatible = "samsung,exynos4210-hdmiddc"; + reg = <0x50>; + };I don't think this matches current Exynos HDMI bindings, which I believe have been changed to just take a phandle to i2c bus instead.
Quoted 5 lines
Looks correct to me:
Looks correct according to the documentation, but unfortunately this is not the first time (and most likely not the last time) when our Exynos DRM people carelessly change the binding without taking care of updating relevant documentation correctly...
Please see following code:
To get the DDC I2C adapter it first tries to look by a hardcoded compatible string for backwards compatibility, but when there is no such node, it uses the "ddc" property which is a phandle to the I2C adapter node on which the DDC channel will be present.
Long story short, the binding of fake DDC device is obsolete and should be marked so in documentation, while new device trees should only use the new way with phandle.
However what this patch adds is for now technically correct and should work even if support for old bindings is dropped in future, so I won't consider this an issue.
Best regards,
Tomasz
v4 cover ARM: dts: exynos: Prepare Spring
In reply to Andreas FärberAm 31.07.2014 18:08, schrieb Andreas F?rber:
Quoted 7 lines
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.
v4 fixes a pinctrl bug.
Once again, resulting Snow vs. Spring diff attached for convenience.
Andreas
Signature · 9 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend?rffer; HRB 16746 AG N?rnberg -------------- next part -------------- A non-text attachment was scrubbed... Name: spring-snow.v4.diff Type: text/x-patch Size: 17117 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140731/5522a095/attachment.bin>