mbox series

[0/7] Initial support for Samsung Galaxy S and Galaxy S 4G

Message ID 1529608199-5583-1-git-send-email-pawel.mikolaj.chmiel@gmail.com
Headers show
Series Initial support for Samsung Galaxy S and Galaxy S 4G | expand

Message

Paweł Chmiel June 21, 2018, 7:09 p.m. UTC
Jonathan Bakker (2):
  ARM: dts: s5pv210: Add initial DTS config for SGH-T959P phone
  dt-bindings: samsung: Document binding for SGH-T959P board

Paweł Chmiel (5):
  ARM: dts: s5pv210: Add missing interrupt-controller property to gph2
  ARM: dts: s5pv210: Add initial DTS for Samsung Aries based phones.
  ARM: dts: s5pv210: Add initial DTS for Samsung Galaxy S phone.
  ARM: s5pv210_defconfig: Enable drivers for Samsung Aries based phones
  dt-bindings: samsung: Document bindings for Samsung aries boards

 .../bindings/arm/samsung/samsung-boards.txt        |   3 +
 arch/arm/boot/dts/Makefile                         |   2 +
 arch/arm/boot/dts/s5pv210-aries.dtsi               | 397 +++++++++++++++++++++
 arch/arm/boot/dts/s5pv210-fascinate4g.dts          |  40 +++
 arch/arm/boot/dts/s5pv210-galaxys.dts              |  72 ++++
 arch/arm/boot/dts/s5pv210-pinctrl.dtsi             |   2 +
 arch/arm/configs/s5pv210_defconfig                 |  49 ++-
 7 files changed, 562 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm/boot/dts/s5pv210-aries.dtsi
 create mode 100644 arch/arm/boot/dts/s5pv210-fascinate4g.dts
 create mode 100644 arch/arm/boot/dts/s5pv210-galaxys.dts

Comments

Krzysztof Kozlowski June 22, 2018, 7:41 a.m. UTC | #1
On 21 June 2018 at 21:09, Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com> wrote:
> This DTS file have initial support Samsung Aries based phones.
> Initial version have support for:
> - sdcard
> - internal memory (present only on non 4g variant)
> - max8998 pmic and rtc
> - max17040 fuel gauge
> - gpio keys
> - fimd (no panel driver yet)
> - usb (peripherial mode)
> - wifi
>
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>

Hi Pawel,

Nice job in mainlining!

Below you'll find some comments for improvement.

> ---
>  arch/arm/boot/dts/s5pv210-aries.dtsi | 397 +++++++++++++++++++++++++++++++++++
>  1 file changed, 397 insertions(+)
>  create mode 100644 arch/arm/boot/dts/s5pv210-aries.dtsi
>
> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
> new file mode 100644
> index 000000000000..6e8ac3615765
> --- /dev/null
> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
> @@ -0,0 +1,397 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Samsung's S5PV210 based Galaxy Aries board device tree source
> + */
> +
> +/dts-v1/;
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/input/input.h>

Is the input header used here?

> +#include <dt-bindings/interrupt-controller/irq.h>

Duplicated inclusion.

> +#include "s5pv210.dtsi"
> +
> +/ {
> +       compatible = "samsung,aries", "samsung,s5pv210";
> +
> +       aliases {
> +               i2c6 = &i2c_pmic;
> +               i2c9 = &i2c_fuel;
> +       };
> +
> +       memory@30000000 {
> +               device_type = "memory";
> +               reg = <0x30000000 0x05000000
> +                       0x40000000 0x10000000
> +                       0x50000000 0x08000000>;
> +       };
> +
> +       wifi_pwrseq: wifi-pwrseq {
> +               compatible = "mmc-pwrseq-simple";
> +               reset-gpios = <&gpg1 2 GPIO_ACTIVE_LOW>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&wlan_gpio_rst>;
> +               post-power-on-delay-ms = <500>;
> +               power-off-delay-us = <500>;
> +       };
> +
> +       i2c_pmic: i2c-pmic {

s/i2c-pmic/ to /i2c-gpio-0/
to reflect generic class of this node. Change only the node name. The
label can stay as is.

> +               compatible = "i2c-gpio";
> +               sda-gpios = <&gpj4 0 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> +               scl-gpios = <&gpj4 3 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;

Spaces around pipe |.

> +               i2c-gpio,delay-us = <2>;
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               pmic@66 {
> +                       compatible = "maxim,max8998";
> +                       reg = <0x66>;
> +                       interrupt-parent = <&gph0>;
> +                       interrupts = <7 0>;

If you really wanted 0 then IRQ_TYPE_NONE... but this should be a
proper interrupt type.

> +
> +                       max8998,pmic-buck1-default-dvs-idx = <1>;
> +                       max8998,pmic-buck1-dvs-gpios = <&gph0 3 GPIO_ACTIVE_HIGH>,
> +                                                       <&gph0 4 GPIO_ACTIVE_HIGH>;
> +                       max8998,pmic-buck1-dvs-voltage = <1275000>, <1200000>,
> +                                                       <1050000>, <950000>;
> +
> +                       max8998,pmic-buck2-default-dvs-idx = <0>;
> +                       max8998,pmic-buck2-dvs-gpio = <&gph0 5 GPIO_ACTIVE_HIGH>;
> +                       max8998,pmic-buck2-dvs-voltage = <1100000>, <1000000>;
> +
> +                       regulators {
> +                               ldo2_reg: LDO2 {
> +                                       regulator-name = "VALIVE_1.2V";
> +                                       regulator-min-microvolt = <1200000>;
> +                                       regulator-max-microvolt = <1200000>;
> +                                       regulator-always-on;
> +
> +                                       regulator-state-mem {
> +                                               regulator-on-in-suspend;
> +                                       };
> +                               };
> +
> +                               ldo3_reg: LDO3 {
> +                                       regulator-name = "VUSB_1.1V";
> +                                       regulator-min-microvolt = <1100000>;
> +                                       regulator-max-microvolt = <1100000>;
> +
> +                                       regulator-state-mem {
> +                                               regulator-off-in-suspend;
> +                                       };
> +                               };
> +
> +                               ldo4_reg: LDO4 {
> +                                       regulator-name = "VADC_3.3V";
> +                                       regulator-min-microvolt = <3300000>;
> +                                       regulator-max-microvolt = <3300000>;
> +                                       regulator-always-on;
> +
> +                                       regulator-state-mem {
> +                                               regulator-off-in-suspend;
> +                                       };
> +                               };
> +
> +                               ldo5_reg: LDO5 {
> +                                       regulator-name = "VTF_2.8V";
> +                                       regulator-min-microvolt = <2800000>;
> +                                       regulator-max-microvolt = <2800000>;
> +
> +                                       regulator-state-mem {
> +                                               regulator-off-in-suspend;
> +                                       };
> +                               };

LDO6?

All regulators should be defined in general. Most of the drivers would
anyway register all of them and use driver-specific defaults for the
ones which are missing in DT. I see that max8998 behaves differently
and silently ignores missing regulators leaving them at bootloader
settings. That's also not good because their initial settings might
not be correct for current load and kernel cannot turn them off if
needed. Also we want in general to have full description of HW in DT.

The same applies to LDO10, clocks and ESAFEOUT2 later.


> +
> +                               ldo7_reg: LDO7 {
> +                                       regulator-name = "VLCD_1.8V";
> +                                       regulator-min-microvolt = <1800000>;
> +                                       regulator-max-microvolt = <1800000>;
> +                                       /* Till we get panel driver */
> +                                       regulator-always-on;
> +
> +                                       regulator-state-mem {
> +                                               regulator-off-in-suspend;
> +                                       };
> +                               };
> +
> +                               ldo8_reg: LDO8 {
> +                                       regulator-name = "VUSB_3.3V";
> +                                       regulator-min-microvolt = <3300000>;
> +                                       regulator-max-microvolt = <3300000>;
> +
> +                                       regulator-state-mem {
> +                                               regulator-off-in-suspend;
> +                                       };
> +                               };
> +
> +                               ldo9_reg: LDO9 {
> +                                       regulator-name = "VCC_2.8V_PDA";
> +                                       regulator-min-microvolt = <2800000>;
> +                                       regulator-max-microvolt = <2800000>;
> +                                       regulator-always-on;
> +                               };
> +
> +                               ldo11_reg: LDO11 {
> +                                       regulator-name = "CAM_AF_3.0V";
> +                                       regulator-min-microvolt = <3000000>;
> +                                       regulator-max-microvolt = <3000000>;
> +
> +                                       regulator-state-mem {
> +                                               regulator-off-in-suspend;
> +                                       };
> +                               };
> +
> +                               ldo12_reg: LDO12 {
> +                                       regulator-name = "CAM_SENSOR_CORE_1.2V";
> +                                       regulator-min-microvolt = <1200000>;
> +                                       regulator-max-microvolt = <1200000>;
> +
> +                                       regulator-state-mem {
> +                                               regulator-off-in-suspend;
> +                                       };
> +                               };
> +
> +                               ldo13_reg: LDO13 {
> +                                       regulator-name = "VGA_VDDIO_2.8V";
> +                                       regulator-min-microvolt = <2800000>;
> +                                       regulator-max-microvolt = <2800000>;
> +
> +                                       regulator-state-mem {
> +                                               regulator-off-in-suspend;
> +                                       };
> +                               };
> +
> +                               ldo14_reg: LDO14 {
> +                                       regulator-name = "VGA_DVDD_1.8V";
> +                                       regulator-min-microvolt = <1800000>;
> +                                       regulator-max-microvolt = <1800000>;
> +
> +                                       regulator-state-mem {
> +                                               regulator-off-in-suspend;
> +                                       };
> +                               };
> +
> +                               ldo15_reg: LDO15 {
> +                                       regulator-name = "CAM_ISP_HOST_2.8V";
> +                                       regulator-min-microvolt = <2800000>;
> +                                       regulator-max-microvolt = <2800000>;
> +
> +                                       regulator-state-mem {
> +                                               regulator-off-in-suspend;
> +                                       };
> +                               };
> +
> +                               ldo16_reg: LDO16 {
> +                                       regulator-name = "VGA_AVDD_2.8V";
> +                                       regulator-min-microvolt = <2800000>;
> +                                       regulator-max-microvolt = <2800000>;
> +
> +                                       regulator-state-mem {
> +                                               regulator-off-in-suspend;
> +                                       };
> +                               };
> +
> +                               ldo17_reg: LDO17 {
> +                                       regulator-name = "VCC_3.0V_LCD";
> +                                       regulator-min-microvolt = <3000000>;
> +                                       regulator-max-microvolt = <3000000>;
> +                                       /* Till we get panel driver */
> +                                       regulator-always-on;
> +
> +                                       regulator-state-mem {
> +                                               regulator-off-in-suspend;
> +                                       };
> +                               };
> +
> +                               buck1_reg: BUCK1 {
> +                                       regulator-name = "vddarm";
> +                                       regulator-min-microvolt = <750000>;
> +                                       regulator-max-microvolt = <1500000>;
> +
> +                                       regulator-state-mem {
> +                                               regulator-off-in-suspend;
> +                                               regulator-suspend-microvolt = <1250000>;
> +                                       };
> +                               };
> +
> +                               buck2_reg: BUCK2 {
> +                                       regulator-name = "vddint";
> +                                       regulator-min-microvolt = <750000>;
> +                                       regulator-max-microvolt = <1500000>;
> +
> +                                       regulator-state-mem {
> +                                               regulator-off-in-suspend;
> +                                               regulator-suspend-microvolt = <1100000>;
> +                                       };
> +                               };
> +
> +                               buck3_reg: BUCK3 {
> +                                       regulator-name = "VCC_1.8V";
> +                                       regulator-min-microvolt = <1800000>;
> +                                       regulator-max-microvolt = <1800000>;
> +                                       regulator-always-on;
> +                               };
> +
> +                               buck4_reg: BUCK4 {
> +                                       regulator-name = "CAM_ISP_CORE_1.2V";
> +                                       regulator-min-microvolt = <1200000>;
> +                                       regulator-max-microvolt = <1200000>;
> +
> +                                       regulator-state-mem {
> +                                               regulator-off-in-suspend;
> +                                       };
> +                               };
> +
> +                               safe1_sreg: ESAFEOUT1 {
> +                                       regulator-name = "SAFEOUT1";
> +                               };
> +                       };
> +               };
> +       };
> +
> +       i2c_fuel: i2c-fuel {

s/i2c-fuel/ to /i2c-gpio-1/


> +               compatible = "i2c-gpio";
> +               sda-gpios = <&mp05 1 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> +               scl-gpios = <&mp05 0 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;

Spaces around | please.

> +               i2c-gpio,delay-us = <2>;
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               fuel@36 {

Name: fuelgauge

> +                       compatible = "maxim,max17040";
> +                       interrupt-parent = <&vic0>;
> +                       interrupts = <7>;
> +                       reg = <0x36>;
> +               };
> +       };
> +};
> +
> +&xusbxti {
> +       clock-frequency = <24000000>;
> +};
> +
> +&pinctrl0 {

Please order all labels alphabetically. It reduces conflicts later
with multiple changes.

> +       wlan_bt_en: wlan-bt-en {
> +               samsung,pins = "gpb-5";
> +               samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
> +               samsung,pin-pud = <S3C64XX_PIN_PULL_NONE>;
> +               samsung,pin-val = <1>;
> +       };
> +
> +       wlan_gpio_rst: wlan-gpio-rst {
> +               samsung,pins = "gpg1-2";
> +               samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
> +               samsung,pin-pud = <S3C64XX_PIN_PULL_NONE>;
> +       };
> +
> +       wifi_host_wake: wifi-host-wake {
> +               samsung,pins = "gph2-4";
> +               samsung,pin-function = <EXYNOS_PIN_FUNC_INPUT>;
> +               samsung,pin-pud = <S3C64XX_PIN_PULL_DOWN>;
> +               samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
> +       };
> +
> +       tf_detect: tf-detect {
> +               samsung,pins = "gph3-4";
> +               samsung,pin-function = <EXYNOS_PIN_FUNC_INPUT>;
> +               samsung,pin-pud = <S3C64XX_PIN_PULL_DOWN>;
> +               samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
> +       };
> +
> +       wifi_wake: wifi-wake {
> +               samsung,pins = "gph3-5";
> +               samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
> +               samsung,pin-pud = <S3C64XX_PIN_PULL_NONE>;
> +       };
> +
> +       massmemory_en: massmemory-en {
> +               samsung,pins = "gpj2-7";
> +               samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
> +               samsung,pin-pud = <S3C64XX_PIN_PULL_NONE>;
> +               samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
> +       };
> +};
> +
> +&uart0 {
> +       status = "okay";
> +};
> +
> +&uart1 {
> +       status = "okay";
> +};
> +
> +&uart2 {
> +       status = "okay";
> +};
> +
> +&sdhci1 {
> +       #address-cells = <1>;
> +       #size-cells = <0>;
> +
> +       bus-width = <4>;
> +       max-frequency = <38400000>;
> +       pinctrl-0 = <&sd1_clk &sd1_cmd &sd1_bus4 &wifi_wake &wifi_host_wake &wlan_bt_en>;
> +       pinctrl-names = "default";
> +       cap-sd-highspeed;
> +       cap-mmc-highspeed;
> +
> +       mmc-pwrseq = <&wifi_pwrseq>;
> +       non-removable;
> +       status = "okay";
> +
> +       brcmf: bcrmf@1 {

Name of node should describe generic class of the device so maybe
"wlan" or "wlanbt"? Label is okay.

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski June 22, 2018, 7:49 a.m. UTC | #2
On 21 June 2018 at 21:09, Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com> wrote:
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>

Please add commit message. This can be something as simple as "Add
Samsung Galaxy S DTS which is a commercial phone based on Aries
family." or something more (e.g. describe what is working).

> ---
>  arch/arm/boot/dts/Makefile            |  1 +
>  arch/arm/boot/dts/s5pv210-galaxys.dts | 72 +++++++++++++++++++++++++++++++++++
>  2 files changed, 73 insertions(+)
>  create mode 100644 arch/arm/boot/dts/s5pv210-galaxys.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 7e2424957809..522ebdca1d3d 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -846,6 +846,7 @@ dtb-$(CONFIG_ARCH_S3C64XX) += \
>         s3c6410-smdk6410.dtb
>  dtb-$(CONFIG_ARCH_S5PV210) += \
>         s5pv210-aquila.dtb \
> +       s5pv210-galaxys.dtb \
>         s5pv210-goni.dtb \
>         s5pv210-smdkc110.dtb \
>         s5pv210-smdkv210.dtb \
> diff --git a/arch/arm/boot/dts/s5pv210-galaxys.dts b/arch/arm/boot/dts/s5pv210-galaxys.dts
> new file mode 100644
> index 000000000000..d435032541a9
> --- /dev/null
> +++ b/arch/arm/boot/dts/s5pv210-galaxys.dts
> @@ -0,0 +1,72 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/dts-v1/;
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/interrupt-controller/irq.h>

Duplicated inclusion.

> +#include "s5pv210-aries.dtsi"
> +
> +/ {
> +       model = "Samsung Galaxy S1 (GT-I9000) based on S5PV210";
> +       compatible = "samsung,galaxys", "samsung,aries", "samsung,s5pv210";
> +
> +       chosen {
> +               bootargs = "console=ttySAC2,115200n8 root=/dev/mmcblk2p1 rw rootwait ignore_loglevel earlyprintk";

stdout-path = "serial2:115200n8";

Rest of bootargs should not be here (they are not HW dependent) unless
you cannot configure them through bootloader?

> +       };
> +
> +       nand_pwrseq: nand-pwrseq {
> +               compatible = "mmc-pwrseq-simple";
> +               reset-gpios = <&gpj2 7 GPIO_ACTIVE_LOW>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&massmemory_en>;

massmemory_en is used only here, so please move it from aries.dtsi.
The same applies to all other possible properties if they are not used
in other DTSes including aries.

> +       };
> +
> +       gpio-keys {
> +               compatible = "gpio-keys";
> +
> +               power {
> +                       label = "power";
> +                       gpios = <&gph2 6 GPIO_ACTIVE_LOW>;
> +                       linux,code = <KEY_POWER>;
> +                       wakeup-source;
> +               };
> +
> +               vol-down {
> +                       label = "volume_down";
> +                       gpios = <&gph3 1 GPIO_ACTIVE_LOW>;
> +                       linux,code = <KEY_VOLUMEDOWN>;
> +               };
> +
> +               vol-up {
> +                       label = "volume_up";
> +                       gpios = <&gph3 2 GPIO_ACTIVE_LOW>;
> +                       linux,code = <KEY_VOLUMEUP>;
> +               };
> +
> +               home {
> +                       label = "home";
> +                       gpios = <&gph3 5 GPIO_ACTIVE_LOW>;
> +                       linux,code = <KEY_HOME>;
> +                       wakeup-source;
> +               };
> +       };
> +};
> +
> +&pinctrl0 {
> +       massmemory_en: massmemory-en {

Oh wait, I see massmemory_en here... so there is no need of such in aries.dtsi.

Best regards,
Krzysztof

> +               samsung,pins = "gpj2-7";
> +               samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
> +               samsung,pin-pud = <S3C64XX_PIN_PULL_NONE>;
> +               samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
> +       };
> +};
> +
> +&sdhci0 {
> +       bus-width = <4>;
> +       non-removable;
> +       mmc-pwrseq = <&nand_pwrseq>;
> +       pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_bus4>;
> +       pinctrl-names = "default";
> +       status = "okay";
> +};
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski June 22, 2018, 7:54 a.m. UTC | #3
On 21 June 2018 at 21:09, Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com> wrote:
> Enable config options required for Samsung Aries based phones..

Two dots.

> While we're here, also enable other useful options like:

Please split it. One set of changes needed for Aries and second set of
other useful changes with short explanation why they are needed. This
explanation might be for example:
1. because typical Linux distro or systemd requires them,
2. they are useful for debugging.

> - SYSVIPC
> - CGROUPS
> - DEVTMPFS
> - networking support
> - ext4 and autofs
>
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> ---
>  arch/arm/configs/s5pv210_defconfig | 49 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/configs/s5pv210_defconfig b/arch/arm/configs/s5pv210_defconfig
> index 09b5a7386414..a077597369f1 100644
> --- a/arch/arm/configs/s5pv210_defconfig
> +++ b/arch/arm/configs/s5pv210_defconfig
> @@ -1,5 +1,7 @@
> +CONFIG_SYSVIPC=y
>  CONFIG_NO_HZ=y
>  CONFIG_HIGH_RES_TIMERS=y
> +CONFIG_CGROUPS=y
>  CONFIG_SYSFS_DEPRECATED=y
>  CONFIG_SYSFS_DEPRECATED_V2=y
>  CONFIG_BLK_DEV_INITRD=y
> @@ -13,28 +15,68 @@ CONFIG_SOLARIS_X86_PARTITION=y
>  CONFIG_ARCH_S5PV210=y
>  CONFIG_VMSPLIT_2G=y
>  CONFIG_PREEMPT=y
> -CONFIG_AEABI=y

I believe this comes from savedefconfig. In such case please make it a
separate commit, before all others, because it should not bring any
functional differences.

Best regards,
Krzysztof

> +CONFIG_ARM_APPENDED_DTB=y
>  CONFIG_CMDLINE="root=/dev/ram0 rw ramdisk=8192 initrd=0x20800000,8M console=ttySAC1,115200 init=/linuxrc"
>  CONFIG_VFP=y
>  CONFIG_NEON=y
> +CONFIG_NET=y
> +CONFIG_PACKET=y
> +CONFIG_UNIX=y
> +CONFIG_INET=y
> +CONFIG_IP_PNP=y
> +CONFIG_IP_PNP_DHCP=y
> +CONFIG_IP_PNP_BOOTP=y
> +CONFIG_IP_PNP_RARP=y
> +CONFIG_CFG80211=m
> +CONFIG_MAC80211=m
>  CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
> +CONFIG_DEVTMPFS=y
> +CONFIG_DEVTMPFS_MOUNT=y
>  CONFIG_BLK_DEV_LOOP=y
>  CONFIG_BLK_DEV_RAM=y
>  CONFIG_BLK_DEV_RAM_SIZE=8192
>  CONFIG_SCSI=y
>  CONFIG_BLK_DEV_SD=y
>  CONFIG_CHR_DEV_SG=y
> +CONFIG_NETDEVICES=y
> +CONFIG_BRCMFMAC=m
>  CONFIG_INPUT_EVDEV=y
> -# CONFIG_INPUT_KEYBOARD is not set
> +CONFIG_KEYBOARD_GPIO=y
>  # CONFIG_INPUT_MOUSE is not set
>  CONFIG_INPUT_TOUCHSCREEN=y
>  CONFIG_SERIAL_8250=y
>  CONFIG_SERIAL_SAMSUNG=y
>  CONFIG_SERIAL_SAMSUNG_CONSOLE=y
>  CONFIG_HW_RANDOM=y
> +CONFIG_I2C_GPIO=y
> +CONFIG_POWER_SUPPLY=y
> +CONFIG_BATTERY_MAX17040=y
>  # CONFIG_HWMON is not set
> -# CONFIG_USB_SUPPORT is not set
> +CONFIG_MFD_MAX8998=y
> +CONFIG_REGULATOR=y
> +CONFIG_REGULATOR_MAX8998=y
> +CONFIG_DRM=y
> +CONFIG_DRM_EXYNOS=y
> +CONFIG_DRM_EXYNOS_FIMD=y
> +CONFIG_DRM_EXYNOS_DPI=y
> +CONFIG_USB=y
> +CONFIG_USB_OTG=y
> +CONFIG_USB_EHCI_HCD=y
> +CONFIG_USB_EHCI_EXYNOS=y
> +CONFIG_USB_OHCI_HCD=y
> +CONFIG_USB_OHCI_EXYNOS=y
> +CONFIG_USB_DWC2=y
> +CONFIG_MMC=y
> +CONFIG_MMC_SDHCI=y
> +CONFIG_MMC_SDHCI_S3C=y
> +CONFIG_MMC_SDHCI_S3C_DMA=y
> +CONFIG_RTC_CLASS=y
> +CONFIG_RTC_DRV_MAX8998=m
> +CONFIG_PHY_SAMSUNG_USB2=m
> +CONFIG_PHY_S5PV210_USB2=y
>  CONFIG_EXT2_FS=y
> +CONFIG_EXT4_FS=y
> +CONFIG_AUTOFS4_FS=y
>  CONFIG_MSDOS_FS=y
>  CONFIG_VFAT_FS=y
>  CONFIG_TMPFS=y
> @@ -44,6 +86,7 @@ CONFIG_ROMFS_FS=y
>  CONFIG_NLS_CODEPAGE_437=y
>  CONFIG_NLS_ASCII=y
>  CONFIG_NLS_ISO8859_1=y
> +CONFIG_NLS_UTF8=y
>  CONFIG_DEBUG_INFO=y
>  CONFIG_MAGIC_SYSRQ=y
>  CONFIG_DEBUG_KERNEL=y
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski June 22, 2018, 7:58 a.m. UTC | #4
On 21 June 2018 at 21:09, Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com> wrote:
> From: Jonathan Bakker <xc-racer2@live.ca>

Commit msg needed.

> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> ---
>  arch/arm/boot/dts/Makefile                |  1 +
>  arch/arm/boot/dts/s5pv210-fascinate4g.dts | 40 +++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
>  create mode 100644 arch/arm/boot/dts/s5pv210-fascinate4g.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 522ebdca1d3d..d00e995875bb 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -846,6 +846,7 @@ dtb-$(CONFIG_ARCH_S3C64XX) += \
>         s3c6410-smdk6410.dtb
>  dtb-$(CONFIG_ARCH_S5PV210) += \
>         s5pv210-aquila.dtb \
> +       s5pv210-fascinate4g.dtb \
>         s5pv210-galaxys.dtb \
>         s5pv210-goni.dtb \
>         s5pv210-smdkc110.dtb \
> diff --git a/arch/arm/boot/dts/s5pv210-fascinate4g.dts b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
> new file mode 100644
> index 000000000000..37967f57c7b7
> --- /dev/null
> +++ b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/dts-v1/;
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/interrupt-controller/irq.h>

Duplicated header.

> +#include "s5pv210-aries.dtsi"
> +
> +/ {
> +       model = "Samsung Galaxy S Fascinate 4G (SGH-T959P) based on S5PV210";
> +       compatible = "samsung,fascinate4g", "samsung,aries", "samsung,s5pv210";
> +
> +       chosen {
> +               bootargs = "console=ttySAC2,115200n8 root=/dev/mmcblk1p1 rw rootwait ignore_loglevel earlyprintk";

Same comment as for Galaxy S.

Best regards,
Krzysztof

> +       };
> +
> +       gpio-keys {
> +               compatible = "gpio-keys";
> +
> +               power {
> +                       label = "power";
> +                       gpios = <&gph2 6 GPIO_ACTIVE_LOW>;
> +                       linux,code = <KEY_POWER>;
> +                       wakeup-source;
> +               };
> +
> +               vol-down {
> +                       label = "volume_down";
> +                       gpios = <&gph3 2 GPIO_ACTIVE_LOW>;
> +                       linux,code = <KEY_VOLUMEDOWN>;
> +               };
> +
> +               vol-up {
> +                       label = "volume_up";
> +                       gpios = <&gph3 1 GPIO_ACTIVE_LOW>;
> +                       linux,code = <KEY_VOLUMEUP>;
> +               };
> +       };
> +};
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paweł Chmiel June 22, 2018, 8:42 a.m. UTC | #5
On Friday, June 22, 2018 9:49:42 AM CEST Krzysztof Kozlowski wrote:
> On 21 June 2018 at 21:09, Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com> wrote:
> > Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> 
> Please add commit message. This can be something as simple as "Add
> Samsung Galaxy S DTS which is a commercial phone based on Aries
> family." or something more (e.g. describe what is working).
> 
> > ---
> >  arch/arm/boot/dts/Makefile            |  1 +
> >  arch/arm/boot/dts/s5pv210-galaxys.dts | 72 +++++++++++++++++++++++++++++++++++
> >  2 files changed, 73 insertions(+)
> >  create mode 100644 arch/arm/boot/dts/s5pv210-galaxys.dts
> >
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index 7e2424957809..522ebdca1d3d 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -846,6 +846,7 @@ dtb-$(CONFIG_ARCH_S3C64XX) += \
> >         s3c6410-smdk6410.dtb
> >  dtb-$(CONFIG_ARCH_S5PV210) += \
> >         s5pv210-aquila.dtb \
> > +       s5pv210-galaxys.dtb \
> >         s5pv210-goni.dtb \
> >         s5pv210-smdkc110.dtb \
> >         s5pv210-smdkv210.dtb \
> > diff --git a/arch/arm/boot/dts/s5pv210-galaxys.dts b/arch/arm/boot/dts/s5pv210-galaxys.dts
> > new file mode 100644
> > index 000000000000..d435032541a9
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/s5pv210-galaxys.dts
> > @@ -0,0 +1,72 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/dts-v1/;
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/interrupt-controller/irq.h>
> > +#include <dt-bindings/input/input.h>
> > +#include <dt-bindings/interrupt-controller/irq.h>
> 
> Duplicated inclusion.
> 
> > +#include "s5pv210-aries.dtsi"
> > +
> > +/ {
> > +       model = "Samsung Galaxy S1 (GT-I9000) based on S5PV210";
> > +       compatible = "samsung,galaxys", "samsung,aries", "samsung,s5pv210";
> > +
> > +       chosen {
> > +               bootargs = "console=ttySAC2,115200n8 root=/dev/mmcblk2p1 rw rootwait ignore_loglevel earlyprintk";
> 
> stdout-path = "serial2:115200n8";
> 
> Rest of bootargs should not be here (they are not HW dependent) unless
> you cannot configure them through bootloader?
> 
Stock (proprietary) bootloader is little problematic for me:
- to access it, You need to build special cable.
- i wasn't able to boot any kernel newer than 2.6.35/3.0 on it, without following hack/patch
https://github.com/tom3q/linux/commit/af96ebcba03b607ab93bd5778301890feb038479.patch

I would like to leave those bootargs for now, so anyone can easly test this kernel (just with that one patch),
without breaking booting existing software, so they could easly go back to stock software by just flashing old kernel.
Later it'll be removed because there is initial port of mainline u-boot started for both devices
- currently it can be flashed to device, instead of kernel and boot kernels from onenand/sdcard.
In this way migration from old kernel to new one will be much easier to users (and won't require special tools/cables/etc).


> > +       };
> > +
> > +       nand_pwrseq: nand-pwrseq {
> > +               compatible = "mmc-pwrseq-simple";
> > +               reset-gpios = <&gpj2 7 GPIO_ACTIVE_LOW>;
> > +               pinctrl-names = "default";
> > +               pinctrl-0 = <&massmemory_en>;
> 
> massmemory_en is used only here, so please move it from aries.dtsi.
> The same applies to all other possible properties if they are not used
> in other DTSes including aries.
> 
> > +       };
> > +
> > +       gpio-keys {
> > +               compatible = "gpio-keys";
> > +
> > +               power {
> > +                       label = "power";
> > +                       gpios = <&gph2 6 GPIO_ACTIVE_LOW>;
> > +                       linux,code = <KEY_POWER>;
> > +                       wakeup-source;
> > +               };
> > +
> > +               vol-down {
> > +                       label = "volume_down";
> > +                       gpios = <&gph3 1 GPIO_ACTIVE_LOW>;
> > +                       linux,code = <KEY_VOLUMEDOWN>;
> > +               };
> > +
> > +               vol-up {
> > +                       label = "volume_up";
> > +                       gpios = <&gph3 2 GPIO_ACTIVE_LOW>;
> > +                       linux,code = <KEY_VOLUMEUP>;
> > +               };
> > +
> > +               home {
> > +                       label = "home";
> > +                       gpios = <&gph3 5 GPIO_ACTIVE_LOW>;
> > +                       linux,code = <KEY_HOME>;
> > +                       wakeup-source;
> > +               };
> > +       };
> > +};
> > +
> > +&pinctrl0 {
> > +       massmemory_en: massmemory-en {
> 
> Oh wait, I see massmemory_en here... so there is no need of such in aries.dtsi.
> 
> Best regards,
> Krzysztof
> 
> > +               samsung,pins = "gpj2-7";
> > +               samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
> > +               samsung,pin-pud = <S3C64XX_PIN_PULL_NONE>;
> > +               samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
> > +       };
> > +};
> > +
> > +&sdhci0 {
> > +       bus-width = <4>;
> > +       non-removable;
> > +       mmc-pwrseq = <&nand_pwrseq>;
> > +       pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_bus4>;
> > +       pinctrl-names = "default";
> > +       status = "okay";
> > +};
> > --
> > 2.7.4
> >
> 


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski June 22, 2018, 8:51 a.m. UTC | #6
On 22 June 2018 at 10:42, Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com> wrote:
> On Friday, June 22, 2018 9:49:42 AM CEST Krzysztof Kozlowski wrote:
>> On 21 June 2018 at 21:09, Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com> wrote:
>> > Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
>>
>> Please add commit message. This can be something as simple as "Add
>> Samsung Galaxy S DTS which is a commercial phone based on Aries
>> family." or something more (e.g. describe what is working).
>>
>> > ---
>> >  arch/arm/boot/dts/Makefile            |  1 +
>> >  arch/arm/boot/dts/s5pv210-galaxys.dts | 72 +++++++++++++++++++++++++++++++++++
>> >  2 files changed, 73 insertions(+)
>> >  create mode 100644 arch/arm/boot/dts/s5pv210-galaxys.dts
>> >
>> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> > index 7e2424957809..522ebdca1d3d 100644
>> > --- a/arch/arm/boot/dts/Makefile
>> > +++ b/arch/arm/boot/dts/Makefile
>> > @@ -846,6 +846,7 @@ dtb-$(CONFIG_ARCH_S3C64XX) += \
>> >         s3c6410-smdk6410.dtb
>> >  dtb-$(CONFIG_ARCH_S5PV210) += \
>> >         s5pv210-aquila.dtb \
>> > +       s5pv210-galaxys.dtb \
>> >         s5pv210-goni.dtb \
>> >         s5pv210-smdkc110.dtb \
>> >         s5pv210-smdkv210.dtb \
>> > diff --git a/arch/arm/boot/dts/s5pv210-galaxys.dts b/arch/arm/boot/dts/s5pv210-galaxys.dts
>> > new file mode 100644
>> > index 000000000000..d435032541a9
>> > --- /dev/null
>> > +++ b/arch/arm/boot/dts/s5pv210-galaxys.dts
>> > @@ -0,0 +1,72 @@
>> > +// SPDX-License-Identifier: GPL-2.0
>> > +
>> > +/dts-v1/;
>> > +#include <dt-bindings/gpio/gpio.h>
>> > +#include <dt-bindings/interrupt-controller/irq.h>
>> > +#include <dt-bindings/input/input.h>
>> > +#include <dt-bindings/interrupt-controller/irq.h>
>>
>> Duplicated inclusion.
>>
>> > +#include "s5pv210-aries.dtsi"
>> > +
>> > +/ {
>> > +       model = "Samsung Galaxy S1 (GT-I9000) based on S5PV210";
>> > +       compatible = "samsung,galaxys", "samsung,aries", "samsung,s5pv210";
>> > +
>> > +       chosen {
>> > +               bootargs = "console=ttySAC2,115200n8 root=/dev/mmcblk2p1 rw rootwait ignore_loglevel earlyprintk";
>>
>> stdout-path = "serial2:115200n8";
>>
>> Rest of bootargs should not be here (they are not HW dependent) unless
>> you cannot configure them through bootloader?
>>
> Stock (proprietary) bootloader is little problematic for me:
> - to access it, You need to build special cable.
> - i wasn't able to boot any kernel newer than 2.6.35/3.0 on it, without following hack/patch
> https://github.com/tom3q/linux/commit/af96ebcba03b607ab93bd5778301890feb038479.patch
>
> I would like to leave those bootargs for now, so anyone can easly test this kernel (just with that one patch),
> without breaking booting existing software, so they could easly go back to stock software by just flashing old kernel.
> Later it'll be removed because there is initial port of mainline u-boot started for both devices
> - currently it can be flashed to device, instead of kernel and boot kernels from onenand/sdcard.
> In this way migration from old kernel to new one will be much easier to users (and won't require special tools/cables/etc).

OK, then move the console to stdout-path and leave the rest with a
comment explaining why they are useful.

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paweł Chmiel June 22, 2018, 10:15 a.m. UTC | #7
On Friday, June 22, 2018 9:41:02 AM CEST Krzysztof Kozlowski wrote:
> On 21 June 2018 at 21:09, Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com> wrote:
> > This DTS file have initial support Samsung Aries based phones.
> > Initial version have support for:
> > - sdcard
> > - internal memory (present only on non 4g variant)
> > - max8998 pmic and rtc
> > - max17040 fuel gauge
> > - gpio keys
> > - fimd (no panel driver yet)
> > - usb (peripherial mode)
> > - wifi
> >
> > Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> 
> Hi Pawel,
> 
> Nice job in mainlining!
> 
> Below you'll find some comments for improvement.
Thanks for all comments. I'll prepare v2 version with all issues fixed.

> 
> > ---
> >  arch/arm/boot/dts/s5pv210-aries.dtsi | 397 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 397 insertions(+)
> >  create mode 100644 arch/arm/boot/dts/s5pv210-aries.dtsi
> >
> > diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
> > new file mode 100644
> > index 000000000000..6e8ac3615765
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
> > @@ -0,0 +1,397 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Samsung's S5PV210 based Galaxy Aries board device tree source
> > + */
> > +
> > +/dts-v1/;
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/interrupt-controller/irq.h>
> > +#include <dt-bindings/input/input.h>
> 
> Is the input header used here?
> 
> > +#include <dt-bindings/interrupt-controller/irq.h>
> 
> Duplicated inclusion.
> 
> > +#include "s5pv210.dtsi"
> > +
> > +/ {
> > +       compatible = "samsung,aries", "samsung,s5pv210";
> > +
> > +       aliases {
> > +               i2c6 = &i2c_pmic;
> > +               i2c9 = &i2c_fuel;
> > +       };
> > +
> > +       memory@30000000 {
> > +               device_type = "memory";
> > +               reg = <0x30000000 0x05000000
> > +                       0x40000000 0x10000000
> > +                       0x50000000 0x08000000>;
> > +       };
> > +
> > +       wifi_pwrseq: wifi-pwrseq {
> > +               compatible = "mmc-pwrseq-simple";
> > +               reset-gpios = <&gpg1 2 GPIO_ACTIVE_LOW>;
> > +               pinctrl-names = "default";
> > +               pinctrl-0 = <&wlan_gpio_rst>;
> > +               post-power-on-delay-ms = <500>;
> > +               power-off-delay-us = <500>;
> > +       };
> > +
> > +       i2c_pmic: i2c-pmic {
> 
> s/i2c-pmic/ to /i2c-gpio-0/
> to reflect generic class of this node. Change only the node name. The
> label can stay as is.
> 
> > +               compatible = "i2c-gpio";
> > +               sda-gpios = <&gpj4 0 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> > +               scl-gpios = <&gpj4 3 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> 
> Spaces around pipe |.
> 
> > +               i2c-gpio,delay-us = <2>;
> > +               #address-cells = <1>;
> > +               #size-cells = <0>;
> > +
> > +               pmic@66 {
> > +                       compatible = "maxim,max8998";
> > +                       reg = <0x66>;
> > +                       interrupt-parent = <&gph0>;
> > +                       interrupts = <7 0>;
> 
> If you really wanted 0 then IRQ_TYPE_NONE... but this should be a
> proper interrupt type.
> 
> > +
> > +                       max8998,pmic-buck1-default-dvs-idx = <1>;
> > +                       max8998,pmic-buck1-dvs-gpios = <&gph0 3 GPIO_ACTIVE_HIGH>,
> > +                                                       <&gph0 4 GPIO_ACTIVE_HIGH>;
> > +                       max8998,pmic-buck1-dvs-voltage = <1275000>, <1200000>,
> > +                                                       <1050000>, <950000>;
> > +
> > +                       max8998,pmic-buck2-default-dvs-idx = <0>;
> > +                       max8998,pmic-buck2-dvs-gpio = <&gph0 5 GPIO_ACTIVE_HIGH>;
> > +                       max8998,pmic-buck2-dvs-voltage = <1100000>, <1000000>;
> > +
> > +                       regulators {
> > +                               ldo2_reg: LDO2 {
> > +                                       regulator-name = "VALIVE_1.2V";
> > +                                       regulator-min-microvolt = <1200000>;
> > +                                       regulator-max-microvolt = <1200000>;
> > +                                       regulator-always-on;
> > +
> > +                                       regulator-state-mem {
> > +                                               regulator-on-in-suspend;
> > +                                       };
> > +                               };
> > +
> > +                               ldo3_reg: LDO3 {
> > +                                       regulator-name = "VUSB_1.1V";
> > +                                       regulator-min-microvolt = <1100000>;
> > +                                       regulator-max-microvolt = <1100000>;
> > +
> > +                                       regulator-state-mem {
> > +                                               regulator-off-in-suspend;
> > +                                       };
> > +                               };
> > +
> > +                               ldo4_reg: LDO4 {
> > +                                       regulator-name = "VADC_3.3V";
> > +                                       regulator-min-microvolt = <3300000>;
> > +                                       regulator-max-microvolt = <3300000>;
> > +                                       regulator-always-on;
> > +
> > +                                       regulator-state-mem {
> > +                                               regulator-off-in-suspend;
> > +                                       };
> > +                               };
> > +
> > +                               ldo5_reg: LDO5 {
> > +                                       regulator-name = "VTF_2.8V";
> > +                                       regulator-min-microvolt = <2800000>;
> > +                                       regulator-max-microvolt = <2800000>;
> > +
> > +                                       regulator-state-mem {
> > +                                               regulator-off-in-suspend;
> > +                                       };
> > +                               };
> 
> LDO6?
> 
> All regulators should be defined in general. Most of the drivers would
> anyway register all of them and use driver-specific defaults for the
> ones which are missing in DT. I see that max8998 behaves differently
> and silently ignores missing regulators leaving them at bootloader
> settings. That's also not good because their initial settings might
> not be correct for current load and kernel cannot turn them off if
> needed. Also we want in general to have full description of HW in DT.
> 
> The same applies to LDO10, clocks and ESAFEOUT2 later.
> 
> 
> > +
> > +                               ldo7_reg: LDO7 {
> > +                                       regulator-name = "VLCD_1.8V";
> > +                                       regulator-min-microvolt = <1800000>;
> > +                                       regulator-max-microvolt = <1800000>;
> > +                                       /* Till we get panel driver */
> > +                                       regulator-always-on;
> > +
> > +                                       regulator-state-mem {
> > +                                               regulator-off-in-suspend;
> > +                                       };
> > +                               };
> > +
> > +                               ldo8_reg: LDO8 {
> > +                                       regulator-name = "VUSB_3.3V";
> > +                                       regulator-min-microvolt = <3300000>;
> > +                                       regulator-max-microvolt = <3300000>;
> > +
> > +                                       regulator-state-mem {
> > +                                               regulator-off-in-suspend;
> > +                                       };
> > +                               };
> > +
> > +                               ldo9_reg: LDO9 {
> > +                                       regulator-name = "VCC_2.8V_PDA";
> > +                                       regulator-min-microvolt = <2800000>;
> > +                                       regulator-max-microvolt = <2800000>;
> > +                                       regulator-always-on;
> > +                               };
> > +
> > +                               ldo11_reg: LDO11 {
> > +                                       regulator-name = "CAM_AF_3.0V";
> > +                                       regulator-min-microvolt = <3000000>;
> > +                                       regulator-max-microvolt = <3000000>;
> > +
> > +                                       regulator-state-mem {
> > +                                               regulator-off-in-suspend;
> > +                                       };
> > +                               };
> > +
> > +                               ldo12_reg: LDO12 {
> > +                                       regulator-name = "CAM_SENSOR_CORE_1.2V";
> > +                                       regulator-min-microvolt = <1200000>;
> > +                                       regulator-max-microvolt = <1200000>;
> > +
> > +                                       regulator-state-mem {
> > +                                               regulator-off-in-suspend;
> > +                                       };
> > +                               };
> > +
> > +                               ldo13_reg: LDO13 {
> > +                                       regulator-name = "VGA_VDDIO_2.8V";
> > +                                       regulator-min-microvolt = <2800000>;
> > +                                       regulator-max-microvolt = <2800000>;
> > +
> > +                                       regulator-state-mem {
> > +                                               regulator-off-in-suspend;
> > +                                       };
> > +                               };
> > +
> > +                               ldo14_reg: LDO14 {
> > +                                       regulator-name = "VGA_DVDD_1.8V";
> > +                                       regulator-min-microvolt = <1800000>;
> > +                                       regulator-max-microvolt = <1800000>;
> > +
> > +                                       regulator-state-mem {
> > +                                               regulator-off-in-suspend;
> > +                                       };
> > +                               };
> > +
> > +                               ldo15_reg: LDO15 {
> > +                                       regulator-name = "CAM_ISP_HOST_2.8V";
> > +                                       regulator-min-microvolt = <2800000>;
> > +                                       regulator-max-microvolt = <2800000>;
> > +
> > +                                       regulator-state-mem {
> > +                                               regulator-off-in-suspend;
> > +                                       };
> > +                               };
> > +
> > +                               ldo16_reg: LDO16 {
> > +                                       regulator-name = "VGA_AVDD_2.8V";
> > +                                       regulator-min-microvolt = <2800000>;
> > +                                       regulator-max-microvolt = <2800000>;
> > +
> > +                                       regulator-state-mem {
> > +                                               regulator-off-in-suspend;
> > +                                       };
> > +                               };
> > +
> > +                               ldo17_reg: LDO17 {
> > +                                       regulator-name = "VCC_3.0V_LCD";
> > +                                       regulator-min-microvolt = <3000000>;
> > +                                       regulator-max-microvolt = <3000000>;
> > +                                       /* Till we get panel driver */
> > +                                       regulator-always-on;
> > +
> > +                                       regulator-state-mem {
> > +                                               regulator-off-in-suspend;
> > +                                       };
> > +                               };
> > +
> > +                               buck1_reg: BUCK1 {
> > +                                       regulator-name = "vddarm";
> > +                                       regulator-min-microvolt = <750000>;
> > +                                       regulator-max-microvolt = <1500000>;
> > +
> > +                                       regulator-state-mem {
> > +                                               regulator-off-in-suspend;
> > +                                               regulator-suspend-microvolt = <1250000>;
> > +                                       };
> > +                               };
> > +
> > +                               buck2_reg: BUCK2 {
> > +                                       regulator-name = "vddint";
> > +                                       regulator-min-microvolt = <750000>;
> > +                                       regulator-max-microvolt = <1500000>;
> > +
> > +                                       regulator-state-mem {
> > +                                               regulator-off-in-suspend;
> > +                                               regulator-suspend-microvolt = <1100000>;
> > +                                       };
> > +                               };
> > +
> > +                               buck3_reg: BUCK3 {
> > +                                       regulator-name = "VCC_1.8V";
> > +                                       regulator-min-microvolt = <1800000>;
> > +                                       regulator-max-microvolt = <1800000>;
> > +                                       regulator-always-on;
> > +                               };
> > +
> > +                               buck4_reg: BUCK4 {
> > +                                       regulator-name = "CAM_ISP_CORE_1.2V";
> > +                                       regulator-min-microvolt = <1200000>;
> > +                                       regulator-max-microvolt = <1200000>;
> > +
> > +                                       regulator-state-mem {
> > +                                               regulator-off-in-suspend;
> > +                                       };
> > +                               };
> > +
> > +                               safe1_sreg: ESAFEOUT1 {
> > +                                       regulator-name = "SAFEOUT1";
> > +                               };
> > +                       };
> > +               };
> > +       };
> > +
> > +       i2c_fuel: i2c-fuel {
> 
> s/i2c-fuel/ to /i2c-gpio-1/
> 
> 
> > +               compatible = "i2c-gpio";
> > +               sda-gpios = <&mp05 1 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> > +               scl-gpios = <&mp05 0 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> 
> Spaces around | please.
> 
> > +               i2c-gpio,delay-us = <2>;
> > +               #address-cells = <1>;
> > +               #size-cells = <0>;
> > +
> > +               fuel@36 {
> 
> Name: fuelgauge
> 
> > +                       compatible = "maxim,max17040";
> > +                       interrupt-parent = <&vic0>;
> > +                       interrupts = <7>;
> > +                       reg = <0x36>;
> > +               };
> > +       };
> > +};
> > +
> > +&xusbxti {
> > +       clock-frequency = <24000000>;
> > +};
> > +
> > +&pinctrl0 {
> 
> Please order all labels alphabetically. It reduces conflicts later
> with multiple changes.
> 
> > +       wlan_bt_en: wlan-bt-en {
> > +               samsung,pins = "gpb-5";
> > +               samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
> > +               samsung,pin-pud = <S3C64XX_PIN_PULL_NONE>;
> > +               samsung,pin-val = <1>;
> > +       };
> > +
> > +       wlan_gpio_rst: wlan-gpio-rst {
> > +               samsung,pins = "gpg1-2";
> > +               samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
> > +               samsung,pin-pud = <S3C64XX_PIN_PULL_NONE>;
> > +       };
> > +
> > +       wifi_host_wake: wifi-host-wake {
> > +               samsung,pins = "gph2-4";
> > +               samsung,pin-function = <EXYNOS_PIN_FUNC_INPUT>;
> > +               samsung,pin-pud = <S3C64XX_PIN_PULL_DOWN>;
> > +               samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
> > +       };
> > +
> > +       tf_detect: tf-detect {
> > +               samsung,pins = "gph3-4";
> > +               samsung,pin-function = <EXYNOS_PIN_FUNC_INPUT>;
> > +               samsung,pin-pud = <S3C64XX_PIN_PULL_DOWN>;
> > +               samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
> > +       };
> > +
> > +       wifi_wake: wifi-wake {
> > +               samsung,pins = "gph3-5";
> > +               samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
> > +               samsung,pin-pud = <S3C64XX_PIN_PULL_NONE>;
> > +       };
> > +
> > +       massmemory_en: massmemory-en {
> > +               samsung,pins = "gpj2-7";
> > +               samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
> > +               samsung,pin-pud = <S3C64XX_PIN_PULL_NONE>;
> > +               samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
> > +       };
> > +};
> > +
> > +&uart0 {
> > +       status = "okay";
> > +};
> > +
> > +&uart1 {
> > +       status = "okay";
> > +};
> > +
> > +&uart2 {
> > +       status = "okay";
> > +};
> > +
> > +&sdhci1 {
> > +       #address-cells = <1>;
> > +       #size-cells = <0>;
> > +
> > +       bus-width = <4>;
> > +       max-frequency = <38400000>;
> > +       pinctrl-0 = <&sd1_clk &sd1_cmd &sd1_bus4 &wifi_wake &wifi_host_wake &wlan_bt_en>;
> > +       pinctrl-names = "default";
> > +       cap-sd-highspeed;
> > +       cap-mmc-highspeed;
> > +
> > +       mmc-pwrseq = <&wifi_pwrseq>;
> > +       non-removable;
> > +       status = "okay";
> > +
> > +       brcmf: bcrmf@1 {
> 
> Name of node should describe generic class of the device so maybe
> "wlan" or "wlanbt"? Label is okay.
> 
> Best regards,
> Krzysztof
> 


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html