mbox series

[v2,00/10] Initial support for Samsung Galaxy S and Galaxy S 4G

Message ID 1530119544-30023-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 27, 2018, 5:12 p.m. UTC
This patch series adds support for Samsung Galaxy S and Galaxy S 4G.
Both are commercial phone based on Aries family.

Changes from v1:
  - Removed duplicated and unneeded headers
  - Corrected node names
  - Formatting fixes
  - Removed unneeded pinctrl and sorted entries
  - Set correct interrupt type for max8998 pmic
  - Add missing regulators
  - Added missing commit msg
  - Added stdout-path
  - Added information why we hardcode bootargs
  - Added new patch which adds information, that there are samsung boards,
    not using exynos bases soc.
  - Split patch updating s5pv210_defconfig into three parts.
    First is result of make savedefconfig, second adds drivers required
    by both devices and last adds some options, which are usefull in booting
    typical Linux distribution.

Jonathan Bakker (2):
  ARM: dts: s5pv210: Add initial DTS for SGH-T959P phone
  dt-bindings: samsung: Document bindings for SGH-T959P board

Paweł Chmiel (8):
  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
  dt-bindings: samsung: Add S5PV210 as possible soc in Samsung boards
  dt-bindings: samsung: Document bindings for Samsung aries boards
  ARM: s5pv210_defconfig: Run make savedefconfig
  ARM: s5pv210_defconfig: Enable drivers for Samsung Aries based phones
  ARM: s5pv210_defconfig: Enable options needed to boot typical Linux
    distro

 .../bindings/arm/samsung/samsung-boards.txt        |   5 +-
 arch/arm/boot/dts/Makefile                         |   2 +
 arch/arm/boot/dts/s5pv210-aries.dtsi               | 413 +++++++++++++++++++++
 arch/arm/boot/dts/s5pv210-fascinate4g.dts          |  45 +++
 arch/arm/boot/dts/s5pv210-galaxys.dts              |  77 ++++
 arch/arm/boot/dts/s5pv210-pinctrl.dtsi             |   2 +
 arch/arm/configs/s5pv210_defconfig                 |  49 ++-
 7 files changed, 589 insertions(+), 4 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 28, 2018, 7:41 a.m. UTC | #1
On 27 June 2018 at 19:12, 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>
> ---
>
> Changes from v1:
>   - Removed duplicated and unneeded headers
>   - Corrected node names
>   - Added missing spaces
>   - Removed unneeded pinctrl and sorted entries
>   - Set correct interrupt type for max8998 pmic
>   - Add missing regulators
> ---
> ---
>  arch/arm/boot/dts/s5pv210-aries.dtsi | 423 +++++++++++++++++++++++++++++++++++
>  1 file changed, 423 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..61b6cf76265f
> --- /dev/null
> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
> @@ -0,0 +1,423 @@
> +// 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 "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-gpio-0 {
> +               compatible = "i2c-gpio";
> +               sda-gpios = <&gpj4 0 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +               scl-gpios = <&gpj4 3 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +               i2c-gpio,delay-us = <2>;
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               pmic@66 {
> +                       compatible = "maxim,max8998";
> +                       reg = <0x66>;
> +                       interrupt-parent = <&gph0>;
> +                       interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> +
> +                       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_reg: LDO6 {
> +                                       regulator-name = "LDO6";
> +                                       regulator-min-microvolt = <1600000>;
> +                                       regulator-max-microvolt = <3600000>;
> +                               };
> +
> +                               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;
> +                               };
> +
> +                               ldo10_reg: LDO10 {
> +                                       regulator-name = "VPLL_1.2V";
> +                                       regulator-min-microvolt = <1200000>;
> +                                       regulator-max-microvolt = <1200000>;
> +                                       regulator-always-on;
> +
> +                                       regulator-state-mem {
> +                                               regulator-on-in-suspend;
> +                                       };
> +                               };
> +
> +                               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;
> +                                       };
> +                               };
> +
> +                               ap32khz_reg: EN32KHz-AP {
> +                                       regulator-name = "32KHz AP";
> +                                       regulator-always-on;
> +                               };
> +
> +                               cp32khz_reg: EN32KHz-CP {
> +                                       regulator-name = "32KHz CP";
> +                               };
> +
> +                               vichg_reg: ENVICHG {
> +                                       regulator-name = "VICHG";
> +                                       regulator-always-on;
> +                               };
> +
> +                               safe1_sreg: ESAFEOUT1 {
> +                                       regulator-name = "SAFEOUT1";
> +                               };
> +
> +                               safe2_sreg: ESAFEOUT2 {
> +                                       regulator-name = "SAFEOUT2";
> +                               };
> +                       };
> +               };
> +       };
> +
> +       i2c_fuel: 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)>;
> +               i2c-gpio,delay-us = <2>;
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               fuelgauge@36 {
> +                       compatible = "maxim,max17040";
> +                       interrupt-parent = <&vic0>;
> +                       interrupts = <7>;
> +                       reg = <0x36>;
> +               };
> +       };
> +};
> +
> +&xusbxti {
> +       clock-frequency = <24000000>;
> +};
> +
> +&pinctrl0 {

Thanks for changes. You missed one part - ordering the labels here
alphabetically, so:

&fimd {}
&hsotg {}
...
&xusbxti {}

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 28, 2018, 7:48 a.m. UTC | #2
On 28 June 2018 at 09:41, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On 27 June 2018 at 19:12, 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>
>> ---
>>
>> Changes from v1:
>>   - Removed duplicated and unneeded headers
>>   - Corrected node names
>>   - Added missing spaces
>>   - Removed unneeded pinctrl and sorted entries
>>   - Set correct interrupt type for max8998 pmic
>>   - Add missing regulators
>> ---
>> ---
>>  arch/arm/boot/dts/s5pv210-aries.dtsi | 423 +++++++++++++++++++++++++++++++++++
>>  1 file changed, 423 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..61b6cf76265f
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>> @@ -0,0 +1,423 @@
>> +// 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 "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-gpio-0 {
>> +               compatible = "i2c-gpio";
>> +               sda-gpios = <&gpj4 0 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> +               scl-gpios = <&gpj4 3 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> +               i2c-gpio,delay-us = <2>;
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +
>> +               pmic@66 {
>> +                       compatible = "maxim,max8998";
>> +                       reg = <0x66>;
>> +                       interrupt-parent = <&gph0>;
>> +                       interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
>> +
>> +                       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_reg: LDO6 {
>> +                                       regulator-name = "LDO6";
>> +                                       regulator-min-microvolt = <1600000>;
>> +                                       regulator-max-microvolt = <3600000>;
>> +                               };
>> +
>> +                               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;
>> +                               };
>> +
>> +                               ldo10_reg: LDO10 {
>> +                                       regulator-name = "VPLL_1.2V";
>> +                                       regulator-min-microvolt = <1200000>;
>> +                                       regulator-max-microvolt = <1200000>;
>> +                                       regulator-always-on;
>> +
>> +                                       regulator-state-mem {
>> +                                               regulator-on-in-suspend;
>> +                                       };
>> +                               };
>> +
>> +                               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;
>> +                                       };
>> +                               };
>> +
>> +                               ap32khz_reg: EN32KHz-AP {
>> +                                       regulator-name = "32KHz AP";
>> +                                       regulator-always-on;
>> +                               };
>> +
>> +                               cp32khz_reg: EN32KHz-CP {
>> +                                       regulator-name = "32KHz CP";
>> +                               };
>> +
>> +                               vichg_reg: ENVICHG {
>> +                                       regulator-name = "VICHG";
>> +                                       regulator-always-on;
>> +                               };
>> +
>> +                               safe1_sreg: ESAFEOUT1 {
>> +                                       regulator-name = "SAFEOUT1";
>> +                               };
>> +
>> +                               safe2_sreg: ESAFEOUT2 {
>> +                                       regulator-name = "SAFEOUT2";
>> +                               };
>> +                       };
>> +               };
>> +       };
>> +
>> +       i2c_fuel: 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)>;
>> +               i2c-gpio,delay-us = <2>;
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +
>> +               fuelgauge@36 {
>> +                       compatible = "maxim,max17040";
>> +                       interrupt-parent = <&vic0>;
>> +                       interrupts = <7>;
>> +                       reg = <0x36>;
>> +               };
>> +       };
>> +};
>> +
>> +&xusbxti {
>> +       clock-frequency = <24000000>;
>> +};
>> +
>> +&pinctrl0 {
>
> Thanks for changes. You missed one part - ordering the labels here
> alphabetically, so:
>
> &fimd {}
> &hsotg {}
> ...
> &xusbxti {}

Ah, now I see that you changed the order of nodes inside pinctrl. No
need. These were good - ordered by pin names (so gpb, gpg, gph, gpj).
I was referring only to the top-level overrides by label. The point is
when people add new overrides, they tend to add them to the end... and
this brings conflicts with multiple edits at the same time. When node
overrides are ordered and new entries are being added, the chance of
conflicts is reduced.

BR,
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 28, 2018, 3:12 p.m. UTC | #3
On Thursday, June 28, 2018 9:48:48 AM CEST Krzysztof Kozlowski wrote:
> On 28 June 2018 at 09:41, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > On 27 June 2018 at 19:12, 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>
> >> ---
> >>
> >> Changes from v1:
> >>   - Removed duplicated and unneeded headers
> >>   - Corrected node names
> >>   - Added missing spaces
> >>   - Removed unneeded pinctrl and sorted entries
> >>   - Set correct interrupt type for max8998 pmic
> >>   - Add missing regulators
> >> ---
> >> ---
> >>  arch/arm/boot/dts/s5pv210-aries.dtsi | 423 +++++++++++++++++++++++++++++++++++
> >>  1 file changed, 423 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..61b6cf76265f
> >> --- /dev/null
> >> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
> >> @@ -0,0 +1,423 @@
> >> +// 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 "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-gpio-0 {
> >> +               compatible = "i2c-gpio";
> >> +               sda-gpios = <&gpj4 0 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> >> +               scl-gpios = <&gpj4 3 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> >> +               i2c-gpio,delay-us = <2>;
> >> +               #address-cells = <1>;
> >> +               #size-cells = <0>;
> >> +
> >> +               pmic@66 {
> >> +                       compatible = "maxim,max8998";
> >> +                       reg = <0x66>;
> >> +                       interrupt-parent = <&gph0>;
> >> +                       interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> >> +
> >> +                       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_reg: LDO6 {
> >> +                                       regulator-name = "LDO6";
> >> +                                       regulator-min-microvolt = <1600000>;
> >> +                                       regulator-max-microvolt = <3600000>;
> >> +                               };
> >> +
> >> +                               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;
> >> +                               };
> >> +
> >> +                               ldo10_reg: LDO10 {
> >> +                                       regulator-name = "VPLL_1.2V";
> >> +                                       regulator-min-microvolt = <1200000>;
> >> +                                       regulator-max-microvolt = <1200000>;
> >> +                                       regulator-always-on;
> >> +
> >> +                                       regulator-state-mem {
> >> +                                               regulator-on-in-suspend;
> >> +                                       };
> >> +                               };
> >> +
> >> +                               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;
> >> +                                       };
> >> +                               };
> >> +
> >> +                               ap32khz_reg: EN32KHz-AP {
> >> +                                       regulator-name = "32KHz AP";
> >> +                                       regulator-always-on;
> >> +                               };
> >> +
> >> +                               cp32khz_reg: EN32KHz-CP {
> >> +                                       regulator-name = "32KHz CP";
> >> +                               };
> >> +
> >> +                               vichg_reg: ENVICHG {
> >> +                                       regulator-name = "VICHG";
> >> +                                       regulator-always-on;
> >> +                               };
> >> +
> >> +                               safe1_sreg: ESAFEOUT1 {
> >> +                                       regulator-name = "SAFEOUT1";
> >> +                               };
> >> +
> >> +                               safe2_sreg: ESAFEOUT2 {
> >> +                                       regulator-name = "SAFEOUT2";
> >> +                               };
> >> +                       };
> >> +               };
> >> +       };
> >> +
> >> +       i2c_fuel: 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)>;
> >> +               i2c-gpio,delay-us = <2>;
> >> +               #address-cells = <1>;
> >> +               #size-cells = <0>;
> >> +
> >> +               fuelgauge@36 {
> >> +                       compatible = "maxim,max17040";
> >> +                       interrupt-parent = <&vic0>;
> >> +                       interrupts = <7>;
> >> +                       reg = <0x36>;
> >> +               };
> >> +       };
> >> +};
> >> +
> >> +&xusbxti {
> >> +       clock-frequency = <24000000>;
> >> +};
> >> +
> >> +&pinctrl0 {
> >
> > Thanks for changes. You missed one part - ordering the labels here
> > alphabetically, so:
> >
> > &fimd {}
> > &hsotg {}
> > ...
> > &xusbxti {}
> 
> Ah, now I see that you changed the order of nodes inside pinctrl. No
> need. These were good - ordered by pin names (so gpb, gpg, gph, gpj).
> I was referring only to the top-level overrides by label. The point is
> when people add new overrides, they tend to add them to the end... and
> this brings conflicts with multiple edits at the same time. When node
> overrides are ordered and new entries are being added, the chance of
> conflicts is reduced.
> 
Ok now it's clear for me, will fix this  (finally) in v3.

Thanks
> BR,
> 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