diff mbox

[v4,4/4] ARM: dts: Add exynos5250-spring device tree

Message ID 1406822910-6255-5-git-send-email-afaerber@suse.de (mailing list archive)
State New, archived
Headers show

Commit Message

Andreas Färber July 31, 2014, 4:08 p.m. UTC
Adds initial support for the HP Chromebook 11.

Cc: Vincent Palatin <vpalatin@chromium.org>
Cc: Doug Anderson <dianders@chromium.org>
Cc: Stephan van Schaik <stephan@synkhronix.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 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

Comments

Vincent Palatin July 31, 2014, 5 p.m. UTC | #1
Always a bit late to the game.
One small comment inline.

Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

On Thu, Jul 31, 2014 at 9:08 AM, Andreas Färber <afaerber@suse.de> wrote:
> Adds initial support for the HP Chromebook 11.
>
> Cc: Vincent Palatin <vpalatin@chromium.org>
> Cc: Doug Anderson <dianders@chromium.org>
> Cc: Stephan van Schaik <stephan@synkhronix.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  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
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 80a781f76e88..dec4c292f63d 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -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 \
> diff --git a/arch/arm/boot/dts/exynos5250-spring.dts b/arch/arm/boot/dts/exynos5250-spring.dts
> new file mode 100644
> index 000000000000..1fdfa04182fc
> --- /dev/null
> +++ b/arch/arm/boot/dts/exynos5250-spring.dts
> @@ -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;
> +               };
> +       };
> +
> +       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.



> +
> +       usb@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@14400000 {
> +               status = "okay";
> +               samsung,invert-vclk;
> +       };
> +
> +       dp-controller@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@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@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@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@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@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
>
Andreas Färber July 31, 2014, 5:14 p.m. UTC | #2
Hi,

Am 31.07.2014 19:00, schrieb Vincent Palatin:
> Always a bit late to the game.
> One small comment inline.
> 
> Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

Thanks,

> 
> On Thu, Jul 31, 2014 at 9:08 AM, Andreas Färber <afaerber@suse.de> wrote:
>> +       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).

Yes, that was a suggestion you made on v1.

> 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

>> +
>> +       usb@12110000 {
>> +               samsung,vbus-gpio = <&gpx1 1 GPIO_ACTIVE_HIGH>;
>> +       };
>> +
>> +       usb-hub {
>> +               compatible = "smsc,usb3503a";
>> +               reset-gpios = <&hsic_reset>;
>> +       };
[...]
>> +&usbdrd_phy {
>> +       vbus-supply = <&usb3_vbus_reg>;
>> +};
>> +
>> +#include "cros-ec-keyboard.dtsi"
>> --
>> 1.9.3
Andreas Färber July 31, 2014, 5:38 p.m. UTC | #3
Am 31.07.2014 19:14, schrieb Andreas Färber:
> Hi,
> 
> Am 31.07.2014 19:00, schrieb Vincent Palatin:
>> Always a bit late to the game.
>> One small comment inline.
>>
>> Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
> 
> Thanks,
> 
>>
>> On Thu, Jul 31, 2014 at 9:08 AM, Andreas Färber <afaerber@suse.de> wrote:
>>> +       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).
> 
> Yes, that was a suggestion you made on v1.
> 
>> 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

> 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
> 
>>> +
>>> +       usb@12110000 {
>>> +               samsung,vbus-gpio = <&gpx1 1 GPIO_ACTIVE_HIGH>;
>>> +       };
>>> +
>>> +       usb-hub {
>>> +               compatible = "smsc,usb3503a";
>>> +               reset-gpios = <&hsic_reset>;
>>> +       };
> [...]
>>> +&usbdrd_phy {
>>> +       vbus-supply = <&usb3_vbus_reg>;
>>> +};
>>> +
>>> +#include "cros-ec-keyboard.dtsi"
>>> --
>>> 1.9.3
>
Vincent Palatin July 31, 2014, 6:51 p.m. UTC | #4
On Thu, Jul 31, 2014 at 10:14 AM, Andreas Färber <afaerber@suse.de> wrote:
> Hi,
>
> Am 31.07.2014 19:00, schrieb Vincent Palatin:
>> Always a bit late to the game.
>> One small comment inline.
>>
>> Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
>
> Thanks,
>
>>
>> On Thu, Jul 31, 2014 at 9:08 AM, Andreas Färber <afaerber@suse.de> wrote:
>>> +       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).
>
> Yes, that was a suggestion you made on v1.
>
>> 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.


>>> +
>>> +       usb@12110000 {
>>> +               samsung,vbus-gpio = <&gpx1 1 GPIO_ACTIVE_HIGH>;
>>> +       };
>>> +
>>> +       usb-hub {
>>> +               compatible = "smsc,usb3503a";
>>> +               reset-gpios = <&hsic_reset>;
>>> +       };
> [...]
>>> +&usbdrd_phy {
>>> +       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
Tomasz Figa July 31, 2014, 7:05 p.m. UTC | #5
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.

On 31.07.2014 18:08, Andreas Färber wrote:
> Adds initial support for the HP Chromebook 11.

[snip]

> +	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?

> +			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;
> +		};
> +	};
> +
> +	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@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.

> +		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?

> +	};
> +
> +	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.

> +
> +	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.

> +
> +	fimd@14400000 {
> +		status = "okay";
> +		samsung,invert-vclk;
> +	};

Ditto.

> +
> +	dp-controller@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.

> +};
> +
> +&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....

> +
> +&i2c_0 {
> +	status = "okay";
> +	samsung,i2c-sda-delay = <100>;
> +	samsung,i2c-max-bus-freq = <378000>;

[snip]

> +/*
> + * 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.

> +
> +&i2c_2 {
> +	status = "okay";
> +	samsung,i2c-sda-delay = <100>;
> +	samsung,i2c-max-bus-freq = <66000>;
> +
> +	hdmiddc@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.

> +};
> +
> +&i2c_3 {
> +	status = "okay";
> +	samsung,i2c-sda-delay = <100>;
> +	samsung,i2c-max-bus-freq = <66000>;
> +};

[snip]

> +&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>;
> +};

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
Andreas Färber July 31, 2014, 7:20 p.m. UTC | #6
Hi Tomasz,

Am 31.07.2014 21:05, schrieb Tomasz Figa:
> 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.
> 
> On 31.07.2014 18:08, Andreas Färber wrote:
>> Adds initial support for the HP Chromebook 11.
> 
> [snip]
> 
>> +	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.

> 
>> +			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;
>> +		};
>> +	};
>> +
>> +	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@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.

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, ... ;)

>> +		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?

>> +	};
>> +
>> +	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.
> 
>> +
>> +	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?

> 
>> +
>> +	fimd@14400000 {
>> +		status = "okay";
>> +		samsung,invert-vclk;
>> +	};
> 
> Ditto.

fimd?

> 
>> +
>> +	dp-controller@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.

dp_controller? display_port_controller?

> 
>> +};
>> +
>> +&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.

>> +
>> +&i2c_0 {
>> +	status = "okay";
>> +	samsung,i2c-sda-delay = <100>;
>> +	samsung,i2c-max-bus-freq = <378000>;
> 
> [snip]
> 
>> +/*
>> + * 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.

>> +
>> +&i2c_2 {
>> +	status = "okay";
>> +	samsung,i2c-sda-delay = <100>;
>> +	samsung,i2c-max-bus-freq = <66000>;
>> +
>> +	hdmiddc@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.

>> +};
>> +
>> +&i2c_3 {
>> +	status = "okay";
>> +	samsung,i2c-sda-delay = <100>;
>> +	samsung,i2c-max-bus-freq = <66000>;
>> +};
> 
> [snip]
> 
>> +&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>;
>> +};
> 
> 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
Tomasz Figa July 31, 2014, 7:40 p.m. UTC | #7
Andreas,

On 31.07.2014 21:20, Andreas Färber wrote:
> Am 31.07.2014 21:05, schrieb Tomasz Figa:
>> On 31.07.2014 18:08, Andreas Färber wrote:
>>> Adds initial support for the HP Chromebook 11.
>>
>> [snip]
>>
>>> +	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.

Doug, Vincent?

> 
>>
>>> +			gpio-key,wakeup;
>>> +		};

[snip]

>>> +
>>> +	usb@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.
> 
> 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@1211000 the right label would be "ehci".

> 
>>> +		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?

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.

> 
>>> +	};
>>> +
>>> +	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.
>>
>>> +
>>> +	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?

Sounds good.

> 
>>
>>> +
>>> +	fimd@14400000 {
>>> +		status = "okay";
>>> +		samsung,invert-vclk;
>>> +	};
>>
>> Ditto.
> 
> fimd?

OK.

> 
>>
>>> +
>>> +	dp-controller@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.
> 
> dp_controller? display_port_controller?

dp?

> 
>>
>>> +};
>>> +
>>> +&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.
> 

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.

>>> +
>>> +&i2c_0 {
>>> +	status = "okay";
>>> +	samsung,i2c-sda-delay = <100>;
>>> +	samsung,i2c-max-bus-freq = <378000>;
>>
>> [snip]
>>
>>> +/*
>>> + * 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.
> 

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.

>>> +
>>> +&i2c_2 {
>>> +	status = "okay";
>>> +	samsung,i2c-sda-delay = <100>;
>>> +	samsung,i2c-max-bus-freq = <66000>;
>>> +
>>> +	hdmiddc@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.
> 

This means that somebody probably forgot to update -cros-common/-snow
dts(i). Not the first and I guess probably not the last time...

>>> +};
>>> +
>>> +&i2c_3 {
>>> +	status = "okay";
>>> +	samsung,i2c-sda-delay = <100>;
>>> +	samsung,i2c-max-bus-freq = <66000>;
>>> +};
>>
>> [snip]
>>
>>> +&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>;
>>> +};
>>
>> 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. :)

Doug, Vincent, someone else?

Best regards,
Tomasz
Andreas Färber July 31, 2014, 8:36 p.m. UTC | #8
Am 31.07.2014 21:05, schrieb Tomasz Figa:
>> +	};
>> +
>> +	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
Tomasz Figa July 31, 2014, 9:09 p.m. UTC | #9
On 31.07.2014 22:36, Andreas Färber wrote:
> Am 31.07.2014 21:05, schrieb Tomasz Figa:
>>> +	};
>>> +
>>> +	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.

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
Andreas Färber July 31, 2014, 11:17 p.m. UTC | #10
Am 31.07.2014 21:40, schrieb Tomasz Figa:
> On 31.07.2014 21:20, Andreas Färber wrote:
>> Am 31.07.2014 21:05, schrieb Tomasz Figa:
>>> On 31.07.2014 18:08, Andreas Färber wrote:
>>>> +	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?
> 
> 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!

[...]
>>>> +&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.
>>
> 
> 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.

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.

>>> [snip]
>>>
>>>> +/*
>>>> + * 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.
>>
> 
> 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
Tomasz Figa July 31, 2014, 11:26 p.m. UTC | #11
On 01.08.2014 01:17, Andreas Färber wrote:
> Am 31.07.2014 21:40, schrieb Tomasz Figa:
>> On 31.07.2014 21:20, Andreas Färber wrote:
>>> Am 31.07.2014 21:05, schrieb Tomasz Figa:
>>>> On 31.07.2014 18:08, Andreas Färber wrote:
[snip]
>>>>> +&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.
>>>
>>
>> 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.
> 
> 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

>>>> [snip]
>>>>
>>>>> +/*
>>>>> + * 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.
>>>
>>
>> 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.

> 
> 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
Andreas Färber July 31, 2014, 11:31 p.m. UTC | #12
Am 01.08.2014 01:26, schrieb Tomasz Figa:
> On 01.08.2014 01:17, Andreas Färber wrote:
>> Am 31.07.2014 21:40, schrieb Tomasz Figa:
>>> On 31.07.2014 21:20, Andreas Färber wrote:
>>>> Am 31.07.2014 21:05, schrieb Tomasz Figa:
>>>>> On 31.07.2014 18:08, Andreas Färber wrote:
> [snip]
>>>>>> +&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.
>>>>
>>>
>>> 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.
>>
>> 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.

Exactly what I meant! :)

Andreas
Andreas Färber Aug. 1, 2014, 3:17 a.m. UTC | #13
Am 31.07.2014 21:05, schrieb Tomasz Figa:
>> +
>> +&i2c_2 {
>> +	status = "okay";
>> +	samsung,i2c-sda-delay = <100>;
>> +	samsung,i2c-max-bus-freq = <66000>;
>> +
>> +	hdmiddc@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:

http://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/tree/Documentation/devicetree/bindings/video/exynos_hdmiddc.txt?h=for-next

https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/video/exynos_hdmiddc.txt

Andreas
Doug Anderson Aug. 2, 2014, 5:15 a.m. UTC | #14
Tomasz,

On Thu, Jul 31, 2014 at 12:40 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Andreas,
>
> On 31.07.2014 21:20, Andreas Färber wrote:
>> Am 31.07.2014 21:05, schrieb Tomasz Figa:
>>> On 31.07.2014 18:08, Andreas Färber wrote:
>>>> Adds initial support for the HP Chromebook 11.
>>>
>>> [snip]
>>>
>>>> +   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.
>
> Doug, Vincent?

Not something I've looked at, but it's never been a problem...


>>>> +&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>;
>>>> +};
>>>
>>> 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. :)
>
> 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...
Andreas Färber Aug. 2, 2014, 7:49 a.m. UTC | #15
Am 02.08.2014 07:15, schrieb Doug Anderson:
> On Thu, Jul 31, 2014 at 12:40 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>> On 31.07.2014 21:20, Andreas Färber wrote:
>>> Am 31.07.2014 21:05, schrieb Tomasz Figa:
>>>> On 31.07.2014 18:08, Andreas Färber wrote:
>>>>> +&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>;
>>>>> +};
>>>>
>>>> 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. :)
>>
>> 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 did copy such a comment for the MMC node in v5, still present in v6.

Andreas

> 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...
Tomasz Figa Aug. 2, 2014, 12:40 p.m. UTC | #16
On 01.08.2014 05:17, Andreas Färber wrote:
> Am 31.07.2014 21:05, schrieb Tomasz Figa:
>>> +
>>> +&i2c_2 {
>>> +	status = "okay";
>>> +	samsung,i2c-sda-delay = <100>;
>>> +	samsung,i2c-max-bus-freq = <66000>;
>>> +
>>> +	hdmiddc@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:
> 
> http://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/tree/Documentation/devicetree/bindings/video/exynos_hdmiddc.txt?h=for-next
> 
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/video/exynos_hdmiddc.txt

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:

https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/tree/drivers/gpu/drm/exynos/exynos_hdmi.c?h=exynos-drm-next#n2415

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
diff mbox

Patch

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 80a781f76e88..dec4c292f63d 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -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 \
diff --git a/arch/arm/boot/dts/exynos5250-spring.dts b/arch/arm/boot/dts/exynos5250-spring.dts
new file mode 100644
index 000000000000..1fdfa04182fc
--- /dev/null
+++ b/arch/arm/boot/dts/exynos5250-spring.dts
@@ -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;
+		};
+	};
+
+	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@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@14400000 {
+		status = "okay";
+		samsung,invert-vclk;
+	};
+
+	dp-controller@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@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@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@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@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@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"