[PATCH v3 0/3] dts: ARM: add Kobo Clara HD eBook reader

[PATCH v3 0/3] dts: ARM: add Kobo Clara HD eBook reader

[Andreas Kemnade]

This adds a device tree for the Kobo Clara HD eBook reader.
Name on mainboard is: 37NB-E60K00+4A4
Serials start with: E60K02 (a number also seen in
vendor kernel sources)
These boards are also found in the Tolino Shine 3 reader
but equipped with a i.MX6SL processor. Support for that
device is planned to be added in a later patch series.
To prepare for that the device tree is split up into
a board file containing SoC-independent stuff and a
file containing the SoC-dependent stuff.

Work is based on code from the vendor kernel at
https://github.com/kobolabs/Kobo-Reader/blob/master/hw/imx6sll-clara/kernel.tar.bz2
but things need to be heavily reworked due to
incompatible bindings and to prepare for Tolino Shine 3

Changes in v2:
- reordered patches
- various cleanups as suggested by Marco Felsch

Changes in v3:
- correct memory size
- better name for led
- comments about missing i2c devices

Andreas Kemnade (3):
  dt-bindings: arm: fsl: add compatible string for Kobo Clara HD
  ARM: dts: add Netronix E60K02 board common file
  ARM: dts: imx: add devicetree for Kobo Clara HD

 .../devicetree/bindings/arm/fsl.yaml          |   1 +
 arch/arm/boot/dts/Makefile                    |   3 +-
 arch/arm/boot/dts/e60k02.dtsi                 | 337 ++++++++++++++++++
 arch/arm/boot/dts/imx6sll-kobo-clarahd.dts    | 279 +++++++++++++++
 4 files changed, 619 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/e60k02.dtsi
 create mode 100644 arch/arm/boot/dts/imx6sll-kobo-clarahd.dts

-- 
2.20.1

[PATCH v3 1/3] dt-bindings: arm: fsl: add compatible string for Kobo Clara HD

[Andreas Kemnade]

This adds a compatible string for the Kobo Clara HD eBook reader.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
Changes in v2: reordered, was 2/3
 Documentation/devicetree/bindings/arm/fsl.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index 7294ac36f4c0b..afa3bfeca0c08 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -148,6 +148,7 @@ properties:
         items:
           - enum:
               - fsl,imx6sll-evk
+              - kobo,clarahd
           - const: fsl,imx6sll
 
       - description: i.MX6SX based Boards
-- 
2.20.1

Re: [PATCH v3 1/3] dt-bindings: arm: fsl: add compatible string for Kobo Clara HD

[Rob Herring]

On Thu, 10 Oct 2019 21:23:55 +0200, Andreas Kemnade wrote:
> This adds a compatible string for the Kobo Clara HD eBook reader.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
> Changes in v2: reordered, was 2/3
>  Documentation/devicetree/bindings/arm/fsl.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH v3 2/3] ARM: dts: add Netronix E60K02 board common file

[Andreas Kemnade]

The Netronix board E60K02 can be found some several Ebook-Readers,
at least the Kobo Clara HD and the Tolino Shine 3. The board
is equipped with different SoCs requiring different pinmuxes.

For now the following peripherals are included:
- LED
- Power Key
- Cover (gpio via hall sensor)
- RC5T619 PMIC (the kernel misses support for rtc and charger
  subdevices).
- Backlight via lm3630a
- Wifi sdio chip detection (mmc-powerseq and stuff)

It is based on vendor kernel but heavily reworked due to many
changed bindings.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
Changes in v3:
- better led name
- correct memory size
- comments about missing devices

Changes in v2:
- reordered, was 1/3
- moved pinmuxes to their actual users, not the parents
  of them
- removed some already-disabled stuff
- minor cleanups

backligt dependencies:
module autoloading:
https://patchwork.kernel.org/patch/11139987/ 
enable-gpios property (accepted and acked):
https://patchwork.kernel.org/patch/11143795/

 arch/arm/boot/dts/e60k02.dtsi | 337 ++++++++++++++++++++++++++++++++++
 1 file changed, 337 insertions(+)
 create mode 100644 arch/arm/boot/dts/e60k02.dtsi

diff --git a/arch/arm/boot/dts/e60k02.dtsi b/arch/arm/boot/dts/e60k02.dtsi
new file mode 100644
index 0000000000000..84c0447b9a1bd
--- /dev/null
+++ b/arch/arm/boot/dts/e60k02.dtsi
@@ -0,0 +1,337 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 Andreas Kemnade
+ * based on works
+ * Copyright 2016 Freescale Semiconductor, Inc.
+ * and
+ * Copyright (C) 2014 Ricoh Electronic Devices Co., Ltd
+ *
+ * Netronix E60K02 board common.
+ * This board is equipped with different SoCs and
+ * found in ebook-readers like the Kobo Clara HD (with i.MX6SLL) and
+ * the Tolino Shine 3 (with i.MX6SL)
+ */
+#include <dt-bindings/input/input.h>
+
+/ {
+
+	chosen {
+		stdout-path = &uart1;
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_gpio_keys>;
+		power {
+			label = "Power";
+			gpios = <&gpio5 8 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_POWER>;
+			gpio-key,wakeup;
+		};
+		cover {
+			label = "Cover";
+			gpios = <&gpio5 12 GPIO_ACTIVE_LOW>;
+			linux,code = <SW_LID>;
+			linux,input-type = <EV_SW>;
+			gpio-key,wakeup;
+		};
+	};
+
+	leds {
+		compatible = "gpio-leds";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_led>;
+
+		on {
+			label = "e60k02:white:on";
+			gpios = <&gpio5 7 GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "timer";
+		};
+	};
+
+	memory {
+		reg = <0x80000000 0x20000000>;
+	};
+
+	reg_wifi: regulator-wifi {
+		compatible = "regulator-fixed";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_wifi_power>;
+		regulator-name = "SD3_SPWR";
+		regulator-min-microvolt = <3000000>;
+		regulator-max-microvolt = <3000000>;
+
+		gpio = <&gpio4 29 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+	};
+
+	wifi_pwrseq: wifi_pwrseq {
+		compatible = "mmc-pwrseq-simple";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_wifi_reset>;
+		post-power-on-delay-ms = <20>;
+		reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
+	};
+
+};
+
+
+&i2c1 {
+	clock-frequency = <100000>;
+	pinctrl-names = "default","sleep";
+	pinctrl-0 = <&pinctrl_i2c1>;
+	pinctrl-1 = <&pinctrl_i2c1_sleep>;
+	status = "okay";
+
+	lm3630a: backlight@36 {
+		reg = <0x36>;
+
+		compatible = "ti,lm3630a";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_lm3630a_bl_gpio>;
+		enable-gpios = <&gpio2 10 GPIO_ACTIVE_HIGH>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		led@0 {
+			reg = <0>;
+			led-sources = <0>;
+			label = "backlight_warm";
+			default-brightness = <0>;
+			max-brightness = <255>;
+		};
+
+		led@1 {
+			reg = <1>;
+			led-sources = <1>;
+			label = "backlight_cold";
+			default-brightness = <0>;
+			max-brightness = <255>;
+		};
+
+	};
+};
+
+&i2c2 {
+	clock-frequency = <100000>;
+	pinctrl-names = "default","sleep";
+	pinctrl-0 = <&pinctrl_i2c2>;
+	pinctrl-1 = <&pinctrl_i2c2_sleep>;
+	status = "okay";
+
+	/* TODO: CYTTSP5 touch controller at 0x24 */
+
+	/* TODO: TPS65185 PMIC for E Ink at 0x68 */
+
+};
+
+&i2c3 {
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c3>;
+	status = "okay";
+
+	ricoh619: pmic@32 {
+		compatible = "ricoh,rc5t619";
+		reg = <0x32>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_ricoh_gpio>;
+		system-power-controller;
+
+		regulators {
+			dcdc1_reg: DCDC1 {
+				regulator-name = "DCDC1";
+				regulator-min-microvolt = <300000>;
+				regulator-max-microvolt = <1875000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-max-microvolt = <900000>;
+					regulator-suspend-min-microvolt = <900000>;
+				};
+			};
+
+			/* Core3_3V3 */
+			dcdc2_reg: DCDC2 {
+				regulator-name = "DCDC2";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-max-microvolt = <3300000>;
+					regulator-suspend-min-microvolt = <3300000>;
+				};
+			};
+
+			dcdc3_reg: DCDC3 {
+				regulator-name = "DCDC3";
+				regulator-min-microvolt = <300000>;
+				regulator-max-microvolt = <1875000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-max-microvolt = <1140000>;
+					regulator-suspend-min-microvolt = <1140000>;
+				};
+			};
+
+			/* Core4_1V2 */
+			dcdc4_reg: DCDC4 {
+				regulator-name = "DCDC4";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-max-microvolt = <1140000>;
+					regulator-suspend-min-microvolt = <1140000>;
+				};
+			};
+
+			/* Core4_1V8 */
+			dcdc5_reg: DCDC5 {
+				regulator-name = "DCDC5";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-max-microvolt = <1700000>;
+					regulator-suspend-min-microvolt = <1700000>;
+				};
+			};
+
+			/* IR_3V3 */
+			ldo1_reg: LDO1  {
+				regulator-name = "LDO1";
+				regulator-boot-on;
+			};
+
+			/* Core1_3V3 */
+			ldo2_reg: LDO2  {
+				regulator-name = "LDO2";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-max-microvolt = <3000000>;
+					regulator-suspend-min-microvolt = <3000000>;
+				};
+			};
+
+			/* Core5_1V2 */
+			ldo3_reg: LDO3  {
+				regulator-name = "LDO3";
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			ldo4_reg: LDO4 {
+				regulator-name = "LDO4";
+				regulator-boot-on;
+			};
+
+			/* SPD_3V3 */
+			ldo5_reg: LDO5 {
+				regulator-name = "LDO5";
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			/* DDR_0V6 */
+			ldo6_reg: LDO6 {
+				regulator-name = "LDO6";
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			/* VDD_PWM */
+			ldo7_reg: LDO7 {
+				regulator-name = "LDO7";
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			/* ldo_1v8 */
+			ldo8_reg: LDO8 {
+				regulator-name = "LDO8";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			ldo9_reg: LDO9 {
+				regulator-name = "LDO9";
+				regulator-boot-on;
+			};
+
+			ldo10_reg: LDO10 {
+				regulator-name = "LDO10";
+				regulator-boot-on;
+			};
+
+			ldortc1_reg: LDORTC1  {
+				regulator-name = "LDORTC1";
+				regulator-boot-on;
+			};
+
+			ldortc2_reg: LDORTC2 {
+				regulator-name = "LDORTC2";
+				regulator-boot-on;
+			};
+		};
+
+	};
+
+};
+
+&snvs_rtc {
+	/* we are using the rtc in the pmic, not disabled imx6sll.dtsi */
+	status = "disabled";
+};
+
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart1>;
+	status = "okay";
+};
+
+&usdhc2 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz","sleep";
+	pinctrl-0 = <&pinctrl_usdhc2>;
+	pinctrl-1 = <&pinctrl_usdhc2_100mhz>;
+	pinctrl-2 = <&pinctrl_usdhc2_200mhz>;
+	pinctrl-3 = <&pinctrl_usdhc2_sleep>;
+	non-removable;
+	status = "okay";
+};
+
+&usdhc3 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz","sleep";
+	pinctrl-0 = <&pinctrl_usdhc3>;
+	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
+	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
+	pinctrl-3 = <&pinctrl_usdhc3_sleep>;
+	vmmc-supply = <&reg_wifi>;
+	mmc-pwrseq = <&wifi_pwrseq>;
+	cap-power-off-card;
+	non-removable;
+	status = "okay";
+};
+
+&usbotg1 {
+	pinctrl-names = "default";
+	disable-over-current;
+	srp-disable;
+	hnp-disable;
+	adp-disable;
+	status = "okay";
+};
-- 
2.20.1

Re: [PATCH v3 2/3] ARM: dts: add Netronix E60K02 board common file

[Marco Felsch]

Hi Andreas,

On 19-10-10 21:23, Andreas Kemnade wrote:
> The Netronix board E60K02 can be found some several Ebook-Readers,
> at least the Kobo Clara HD and the Tolino Shine 3. The board
> is equipped with different SoCs requiring different pinmuxes.
> 
> For now the following peripherals are included:
> - LED
> - Power Key
> - Cover (gpio via hall sensor)
> - RC5T619 PMIC (the kernel misses support for rtc and charger
>   subdevices).
> - Backlight via lm3630a
> - Wifi sdio chip detection (mmc-powerseq and stuff)
> 
> It is based on vendor kernel but heavily reworked due to many
> changed bindings.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
> Changes in v3:
> - better led name
> - correct memory size
> - comments about missing devices
> 
> Changes in v2:
> - reordered, was 1/3
> - moved pinmuxes to their actual users, not the parents
>   of them
> - removed some already-disabled stuff
> - minor cleanups

You won't change the muxing, so a this dtsi can be self contained?

Regards,
  Marco

> backligt dependencies:
> module autoloading:
> https://patchwork.kernel.org/patch/11139987/ 
> enable-gpios property (accepted and acked):
> https://patchwork.kernel.org/patch/11143795/
> 
>  arch/arm/boot/dts/e60k02.dtsi | 337 ++++++++++++++++++++++++++++++++++
>  1 file changed, 337 insertions(+)
>  create mode 100644 arch/arm/boot/dts/e60k02.dtsi
> 
> diff --git a/arch/arm/boot/dts/e60k02.dtsi b/arch/arm/boot/dts/e60k02.dtsi
> new file mode 100644
> index 0000000000000..84c0447b9a1bd
> --- /dev/null
> +++ b/arch/arm/boot/dts/e60k02.dtsi
> @@ -0,0 +1,337 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2019 Andreas Kemnade
> + * based on works
> + * Copyright 2016 Freescale Semiconductor, Inc.
> + * and
> + * Copyright (C) 2014 Ricoh Electronic Devices Co., Ltd
> + *
> + * Netronix E60K02 board common.
> + * This board is equipped with different SoCs and
> + * found in ebook-readers like the Kobo Clara HD (with i.MX6SLL) and
> + * the Tolino Shine 3 (with i.MX6SL)
> + */
> +#include <dt-bindings/input/input.h>
> +
> +/ {
> +
> +	chosen {
> +		stdout-path = &uart1;
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_gpio_keys>;
> +		power {
> +			label = "Power";
> +			gpios = <&gpio5 8 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_POWER>;
> +			gpio-key,wakeup;
> +		};
> +		cover {
> +			label = "Cover";
> +			gpios = <&gpio5 12 GPIO_ACTIVE_LOW>;
> +			linux,code = <SW_LID>;
> +			linux,input-type = <EV_SW>;
> +			gpio-key,wakeup;
> +		};
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_led>;
> +
> +		on {
> +			label = "e60k02:white:on";
> +			gpios = <&gpio5 7 GPIO_ACTIVE_LOW>;
> +			linux,default-trigger = "timer";
> +		};
> +	};
> +
> +	memory {
> +		reg = <0x80000000 0x20000000>;
> +	};
> +
> +	reg_wifi: regulator-wifi {
> +		compatible = "regulator-fixed";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_wifi_power>;
> +		regulator-name = "SD3_SPWR";
> +		regulator-min-microvolt = <3000000>;
> +		regulator-max-microvolt = <3000000>;
> +
> +		gpio = <&gpio4 29 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +
> +	};
> +
> +	wifi_pwrseq: wifi_pwrseq {
> +		compatible = "mmc-pwrseq-simple";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_wifi_reset>;
> +		post-power-on-delay-ms = <20>;
> +		reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
> +	};
> +
> +};
> +
> +
> +&i2c1 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default","sleep";
> +	pinctrl-0 = <&pinctrl_i2c1>;
> +	pinctrl-1 = <&pinctrl_i2c1_sleep>;
> +	status = "okay";
> +
> +	lm3630a: backlight@36 {
> +		reg = <0x36>;
> +
> +		compatible = "ti,lm3630a";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_lm3630a_bl_gpio>;
> +		enable-gpios = <&gpio2 10 GPIO_ACTIVE_HIGH>;
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		led@0 {
> +			reg = <0>;
> +			led-sources = <0>;
> +			label = "backlight_warm";
> +			default-brightness = <0>;
> +			max-brightness = <255>;
> +		};
> +
> +		led@1 {
> +			reg = <1>;
> +			led-sources = <1>;
> +			label = "backlight_cold";
> +			default-brightness = <0>;
> +			max-brightness = <255>;
> +		};
> +
> +	};
> +};
> +
> +&i2c2 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default","sleep";
> +	pinctrl-0 = <&pinctrl_i2c2>;
> +	pinctrl-1 = <&pinctrl_i2c2_sleep>;
> +	status = "okay";
> +
> +	/* TODO: CYTTSP5 touch controller at 0x24 */
> +
> +	/* TODO: TPS65185 PMIC for E Ink at 0x68 */
> +
> +};
> +
> +&i2c3 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c3>;
> +	status = "okay";
> +
> +	ricoh619: pmic@32 {
> +		compatible = "ricoh,rc5t619";
> +		reg = <0x32>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_ricoh_gpio>;
> +		system-power-controller;
> +
> +		regulators {
> +			dcdc1_reg: DCDC1 {
> +				regulator-name = "DCDC1";
> +				regulator-min-microvolt = <300000>;
> +				regulator-max-microvolt = <1875000>;
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-max-microvolt = <900000>;
> +					regulator-suspend-min-microvolt = <900000>;
> +				};
> +			};
> +
> +			/* Core3_3V3 */
> +			dcdc2_reg: DCDC2 {
> +				regulator-name = "DCDC2";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-max-microvolt = <3300000>;
> +					regulator-suspend-min-microvolt = <3300000>;
> +				};
> +			};
> +
> +			dcdc3_reg: DCDC3 {
> +				regulator-name = "DCDC3";
> +				regulator-min-microvolt = <300000>;
> +				regulator-max-microvolt = <1875000>;
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-max-microvolt = <1140000>;
> +					regulator-suspend-min-microvolt = <1140000>;
> +				};
> +			};
> +
> +			/* Core4_1V2 */
> +			dcdc4_reg: DCDC4 {
> +				regulator-name = "DCDC4";
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-max-microvolt = <1140000>;
> +					regulator-suspend-min-microvolt = <1140000>;
> +				};
> +			};
> +
> +			/* Core4_1V8 */
> +			dcdc5_reg: DCDC5 {
> +				regulator-name = "DCDC5";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-max-microvolt = <1700000>;
> +					regulator-suspend-min-microvolt = <1700000>;
> +				};
> +			};
> +
> +			/* IR_3V3 */
> +			ldo1_reg: LDO1  {
> +				regulator-name = "LDO1";
> +				regulator-boot-on;
> +			};
> +
> +			/* Core1_3V3 */
> +			ldo2_reg: LDO2  {
> +				regulator-name = "LDO2";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-max-microvolt = <3000000>;
> +					regulator-suspend-min-microvolt = <3000000>;
> +				};
> +			};
> +
> +			/* Core5_1V2 */
> +			ldo3_reg: LDO3  {
> +				regulator-name = "LDO3";
> +				regulator-always-on;
> +				regulator-boot-on;
> +			};
> +
> +			ldo4_reg: LDO4 {
> +				regulator-name = "LDO4";
> +				regulator-boot-on;
> +			};
> +
> +			/* SPD_3V3 */
> +			ldo5_reg: LDO5 {
> +				regulator-name = "LDO5";
> +				regulator-always-on;
> +				regulator-boot-on;
> +			};
> +
> +			/* DDR_0V6 */
> +			ldo6_reg: LDO6 {
> +				regulator-name = "LDO6";
> +				regulator-always-on;
> +				regulator-boot-on;
> +			};
> +
> +			/* VDD_PWM */
> +			ldo7_reg: LDO7 {
> +				regulator-name = "LDO7";
> +				regulator-always-on;
> +				regulator-boot-on;
> +			};
> +
> +			/* ldo_1v8 */
> +			ldo8_reg: LDO8 {
> +				regulator-name = "LDO8";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-always-on;
> +				regulator-boot-on;
> +			};
> +
> +			ldo9_reg: LDO9 {
> +				regulator-name = "LDO9";
> +				regulator-boot-on;
> +			};
> +
> +			ldo10_reg: LDO10 {
> +				regulator-name = "LDO10";
> +				regulator-boot-on;
> +			};
> +
> +			ldortc1_reg: LDORTC1  {
> +				regulator-name = "LDORTC1";
> +				regulator-boot-on;
> +			};
> +
> +			ldortc2_reg: LDORTC2 {
> +				regulator-name = "LDORTC2";
> +				regulator-boot-on;
> +			};
> +		};
> +
> +	};
> +
> +};
> +
> +&snvs_rtc {
> +	/* we are using the rtc in the pmic, not disabled imx6sll.dtsi */
> +	status = "disabled";
> +};
> +
> +&uart1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart1>;
> +	status = "okay";
> +};
> +
> +&usdhc2 {
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz","sleep";
> +	pinctrl-0 = <&pinctrl_usdhc2>;
> +	pinctrl-1 = <&pinctrl_usdhc2_100mhz>;
> +	pinctrl-2 = <&pinctrl_usdhc2_200mhz>;
> +	pinctrl-3 = <&pinctrl_usdhc2_sleep>;
> +	non-removable;
> +	status = "okay";
> +};
> +
> +&usdhc3 {
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz","sleep";
> +	pinctrl-0 = <&pinctrl_usdhc3>;
> +	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
> +	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
> +	pinctrl-3 = <&pinctrl_usdhc3_sleep>;
> +	vmmc-supply = <&reg_wifi>;
> +	mmc-pwrseq = <&wifi_pwrseq>;
> +	cap-power-off-card;
> +	non-removable;
> +	status = "okay";
> +};
> +
> +&usbotg1 {
> +	pinctrl-names = "default";
> +	disable-over-current;
> +	srp-disable;
> +	hnp-disable;
> +	adp-disable;
> +	status = "okay";
> +};
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v3 2/3] ARM: dts: add Netronix E60K02 board common file

[Andreas Kemnade]

On Fri, 11 Oct 2019 08:56:09 +0200
Marco Felsch <m.felsch@pengutronix.de> wrote:

> Hi Andreas,
> 
> On 19-10-10 21:23, Andreas Kemnade wrote:
> > The Netronix board E60K02 can be found some several Ebook-Readers,
> > at least the Kobo Clara HD and the Tolino Shine 3. The board
> > is equipped with different SoCs requiring different pinmuxes.
> > 
> > For now the following peripherals are included:
> > - LED
> > - Power Key
> > - Cover (gpio via hall sensor)
> > - RC5T619 PMIC (the kernel misses support for rtc and charger
> >   subdevices).
> > - Backlight via lm3630a
> > - Wifi sdio chip detection (mmc-powerseq and stuff)
> > 
> > It is based on vendor kernel but heavily reworked due to many
> > changed bindings.
> > 
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> > Changes in v3:
> > - better led name
> > - correct memory size
> > - comments about missing devices
> > 
> > Changes in v2:
> > - reordered, was 1/3
> > - moved pinmuxes to their actual users, not the parents
> >   of them
> > - removed some already-disabled stuff
> > - minor cleanups  
> 
> You won't change the muxing, so a this dtsi can be self contained?
> 
So you want me to put a big 
#if defined(MX6SLL) 
[...]
             pinctrl_i2c1: i2c1grp {
                        fsl,pins = <
                                MX6SLL_PAD_I2C1_SCL__I2C1_SCL    0x4001f8b1
                                MX6SLL_PAD_I2C1_SDA__I2C1_SDA    0x4001f8b1
                        >;
                };

#elif (MX6SL)
[...]
               pinctrl_i2c1: i2c1grp {
                        fsl,pins = <
                                MX6SL_PAD_I2C1_SCL__I2C1_SCL     0x4001f8b1
                                MX6SL_PAD_I2C1_SDA__I2C1_SDA     0x4001f8b1
                        >;
                };

#endif
in the dtsi?

Regards,
Andreas

Re: [PATCH v3 2/3] ARM: dts: add Netronix E60K02 board common file

[Rob Herring]

On Fri, Oct 11, 2019 at 09:41:48AM +0200, Andreas Kemnade wrote:
> On Fri, 11 Oct 2019 08:56:09 +0200
> Marco Felsch <m.felsch@pengutronix.de> wrote:
> 
> > Hi Andreas,
> > 
> > On 19-10-10 21:23, Andreas Kemnade wrote:
> > > The Netronix board E60K02 can be found some several Ebook-Readers,
> > > at least the Kobo Clara HD and the Tolino Shine 3. The board
> > > is equipped with different SoCs requiring different pinmuxes.
> > > 
> > > For now the following peripherals are included:
> > > - LED
> > > - Power Key
> > > - Cover (gpio via hall sensor)
> > > - RC5T619 PMIC (the kernel misses support for rtc and charger
> > >   subdevices).
> > > - Backlight via lm3630a
> > > - Wifi sdio chip detection (mmc-powerseq and stuff)
> > > 
> > > It is based on vendor kernel but heavily reworked due to many
> > > changed bindings.
> > > 
> > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > > ---
> > > Changes in v3:
> > > - better led name
> > > - correct memory size
> > > - comments about missing devices
> > > 
> > > Changes in v2:
> > > - reordered, was 1/3
> > > - moved pinmuxes to their actual users, not the parents
> > >   of them
> > > - removed some already-disabled stuff
> > > - minor cleanups  
> > 
> > You won't change the muxing, so a this dtsi can be self contained?
> > 
> So you want me to put a big 
> #if defined(MX6SLL) 

Not sure what the comment meant, but no, don't do this. C defines in dts 
files are for symbolic names for numbers and assembling bitfields and 
that's it.

> [...]
>              pinctrl_i2c1: i2c1grp {
>                         fsl,pins = <
>                                 MX6SLL_PAD_I2C1_SCL__I2C1_SCL    0x4001f8b1
>                                 MX6SLL_PAD_I2C1_SDA__I2C1_SDA    0x4001f8b1
>                         >;
>                 };
> 
> #elif (MX6SL)
> [...]
>                pinctrl_i2c1: i2c1grp {
>                         fsl,pins = <
>                                 MX6SL_PAD_I2C1_SCL__I2C1_SCL     0x4001f8b1
>                                 MX6SL_PAD_I2C1_SDA__I2C1_SDA     0x4001f8b1
>                         >;
>                 };
> 
> #endif
> in the dtsi?
> 
> Regards,
> Andreas

Re: [PATCH v3 2/3] ARM: dts: add Netronix E60K02 board common file

[Andreas Kemnade]

[-- Attachment #1: Type: text/plain, Size: 1968 bytes --]

On Fri, 11 Oct 2019 09:29:27 -0500
Rob Herring <robh@kernel.org> wrote:

> On Fri, Oct 11, 2019 at 09:41:48AM +0200, Andreas Kemnade wrote:
> > On Fri, 11 Oct 2019 08:56:09 +0200
> > Marco Felsch <m.felsch@pengutronix.de> wrote:
> >   
> > > Hi Andreas,
> > > 
> > > On 19-10-10 21:23, Andreas Kemnade wrote:  
> > > > The Netronix board E60K02 can be found some several Ebook-Readers,
> > > > at least the Kobo Clara HD and the Tolino Shine 3. The board
> > > > is equipped with different SoCs requiring different pinmuxes.
> > > > 
> > > > For now the following peripherals are included:
> > > > - LED
> > > > - Power Key
> > > > - Cover (gpio via hall sensor)
> > > > - RC5T619 PMIC (the kernel misses support for rtc and charger
> > > >   subdevices).
> > > > - Backlight via lm3630a
> > > > - Wifi sdio chip detection (mmc-powerseq and stuff)
> > > > 
> > > > It is based on vendor kernel but heavily reworked due to many
> > > > changed bindings.
> > > > 
> > > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > > > ---
> > > > Changes in v3:
> > > > - better led name
> > > > - correct memory size
> > > > - comments about missing devices
> > > > 
> > > > Changes in v2:
> > > > - reordered, was 1/3
> > > > - moved pinmuxes to their actual users, not the parents
> > > >   of them
> > > > - removed some already-disabled stuff
> > > > - minor cleanups    
> > > 
> > > You won't change the muxing, so a this dtsi can be self contained?
> > >   
> > So you want me to put a big 
> > #if defined(MX6SLL)   
> 
> Not sure what the comment meant, but no, don't do this. C defines in dts 
> files are for symbolic names for numbers and assembling bitfields and 
> that's it.

yes, that is also my opinion. For now, there is only one user
of this .dtsi, but I have another one in preparation. That is the
reason for splitting things between .dts and .dtsi to avoid such ugly
ifdefs

Regards,
Andreas

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 2/3] ARM: dts: add Netronix E60K02 board common file

[Marco Felsch]

On 19-10-11 17:05, Andreas Kemnade wrote:
> On Fri, 11 Oct 2019 09:29:27 -0500
> Rob Herring <robh@kernel.org> wrote:
> 
> > On Fri, Oct 11, 2019 at 09:41:48AM +0200, Andreas Kemnade wrote:
> > > On Fri, 11 Oct 2019 08:56:09 +0200
> > > Marco Felsch <m.felsch@pengutronix.de> wrote:
> > >   
> > > > Hi Andreas,
> > > > 
> > > > On 19-10-10 21:23, Andreas Kemnade wrote:  
> > > > > The Netronix board E60K02 can be found some several Ebook-Readers,
> > > > > at least the Kobo Clara HD and the Tolino Shine 3. The board
> > > > > is equipped with different SoCs requiring different pinmuxes.
> > > > > 
> > > > > For now the following peripherals are included:
> > > > > - LED
> > > > > - Power Key
> > > > > - Cover (gpio via hall sensor)
> > > > > - RC5T619 PMIC (the kernel misses support for rtc and charger
> > > > >   subdevices).
> > > > > - Backlight via lm3630a
> > > > > - Wifi sdio chip detection (mmc-powerseq and stuff)
> > > > > 
> > > > > It is based on vendor kernel but heavily reworked due to many
> > > > > changed bindings.
> > > > > 
> > > > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > > > > ---
> > > > > Changes in v3:
> > > > > - better led name
> > > > > - correct memory size
> > > > > - comments about missing devices
> > > > > 
> > > > > Changes in v2:
> > > > > - reordered, was 1/3
> > > > > - moved pinmuxes to their actual users, not the parents
> > > > >   of them
> > > > > - removed some already-disabled stuff
> > > > > - minor cleanups    
> > > > 
> > > > You won't change the muxing, so a this dtsi can be self contained?
> > > >   
> > > So you want me to put a big 
> > > #if defined(MX6SLL)   
> > 
> > Not sure what the comment meant, but no, don't do this. C defines in dts 
> > files are for symbolic names for numbers and assembling bitfields and 
> > that's it.
> 
> yes, that is also my opinion. For now, there is only one user
> of this .dtsi, but I have another one in preparation. That is the
> reason for splitting things between .dts and .dtsi to avoid such ugly
> ifdefs

Then IMHO the pnictrl-* entries shouldn't appear in the dsti.

Regards,
  Marco

> Regards,
> Andreas



> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v3 2/3] ARM: dts: add Netronix E60K02 board common file

[Andreas Kemnade]

[-- Attachment #1: Type: text/plain, Size: 3146 bytes --]

On Fri, 11 Oct 2019 17:22:14 +0200
Marco Felsch <m.felsch@pengutronix.de> wrote:

> On 19-10-11 17:05, Andreas Kemnade wrote:
> > On Fri, 11 Oct 2019 09:29:27 -0500
> > Rob Herring <robh@kernel.org> wrote:
> >   
> > > On Fri, Oct 11, 2019 at 09:41:48AM +0200, Andreas Kemnade wrote:  
> > > > On Fri, 11 Oct 2019 08:56:09 +0200
> > > > Marco Felsch <m.felsch@pengutronix.de> wrote:
> > > >     
> > > > > Hi Andreas,
> > > > > 
> > > > > On 19-10-10 21:23, Andreas Kemnade wrote:    
> > > > > > The Netronix board E60K02 can be found some several Ebook-Readers,
> > > > > > at least the Kobo Clara HD and the Tolino Shine 3. The board
> > > > > > is equipped with different SoCs requiring different pinmuxes.
> > > > > > 
> > > > > > For now the following peripherals are included:
> > > > > > - LED
> > > > > > - Power Key
> > > > > > - Cover (gpio via hall sensor)
> > > > > > - RC5T619 PMIC (the kernel misses support for rtc and charger
> > > > > >   subdevices).
> > > > > > - Backlight via lm3630a
> > > > > > - Wifi sdio chip detection (mmc-powerseq and stuff)
> > > > > > 
> > > > > > It is based on vendor kernel but heavily reworked due to many
> > > > > > changed bindings.
> > > > > > 
> > > > > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > > > > > ---
> > > > > > Changes in v3:
> > > > > > - better led name
> > > > > > - correct memory size
> > > > > > - comments about missing devices
> > > > > > 
> > > > > > Changes in v2:
> > > > > > - reordered, was 1/3
> > > > > > - moved pinmuxes to their actual users, not the parents
> > > > > >   of them
> > > > > > - removed some already-disabled stuff
> > > > > > - minor cleanups      
> > > > > 
> > > > > You won't change the muxing, so a this dtsi can be self contained?
> > > > >     
> > > > So you want me to put a big 
> > > > #if defined(MX6SLL)     
> > > 
> > > Not sure what the comment meant, but no, don't do this. C defines in dts 
> > > files are for symbolic names for numbers and assembling bitfields and 
> > > that's it.  
> > 
> > yes, that is also my opinion. For now, there is only one user
> > of this .dtsi, but I have another one in preparation. That is the
> > reason for splitting things between .dts and .dtsi to avoid such ugly
> > ifdefs  
> 
> Then IMHO the pnictrl-* entries shouldn't appear in the dsti.
> 
hmm, maybe now I understand your idea:
You do not want only to have

  pinctrl_lm3630a_bl_gpio: lm3630a_bl_gpio_grp {
                        fsl,pins = <
                                MX6SLL_PAD_EPDC_PWR_CTRL3__GPIO2_IO10   0x10059 /* HWEN */
                        >;
                };
in dts, but also  do not have these in .dtsi:

                pinctrl-names = "default";
                pinctrl-0 = <&pinctrl_lm3630a_bl_gpio>;

and instead have in dts:
&lm3630a {
 	pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_lm3630a_bl_gpio>;
	
};


just to make sure I get it right before doing the restructuring work. That way of structuring things did not come to my mind, but then the .dtsi is self-contained.

Regards,
Andreas

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 2/3] ARM: dts: add Netronix E60K02 board common file

[Marco Felsch]

On 19-10-11 18:19, Andreas Kemnade wrote:
> On Fri, 11 Oct 2019 17:22:14 +0200
> Marco Felsch <m.felsch@pengutronix.de> wrote:
> 
> > On 19-10-11 17:05, Andreas Kemnade wrote:
> > > On Fri, 11 Oct 2019 09:29:27 -0500
> > > Rob Herring <robh@kernel.org> wrote:
> > >   
> > > > On Fri, Oct 11, 2019 at 09:41:48AM +0200, Andreas Kemnade wrote:  
> > > > > On Fri, 11 Oct 2019 08:56:09 +0200
> > > > > Marco Felsch <m.felsch@pengutronix.de> wrote:
> > > > >     
> > > > > > Hi Andreas,
> > > > > > 
> > > > > > On 19-10-10 21:23, Andreas Kemnade wrote:    
> > > > > > > The Netronix board E60K02 can be found some several Ebook-Readers,
> > > > > > > at least the Kobo Clara HD and the Tolino Shine 3. The board
> > > > > > > is equipped with different SoCs requiring different pinmuxes.
> > > > > > > 
> > > > > > > For now the following peripherals are included:
> > > > > > > - LED
> > > > > > > - Power Key
> > > > > > > - Cover (gpio via hall sensor)
> > > > > > > - RC5T619 PMIC (the kernel misses support for rtc and charger
> > > > > > >   subdevices).
> > > > > > > - Backlight via lm3630a
> > > > > > > - Wifi sdio chip detection (mmc-powerseq and stuff)
> > > > > > > 
> > > > > > > It is based on vendor kernel but heavily reworked due to many
> > > > > > > changed bindings.
> > > > > > > 
> > > > > > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > > > > > > ---
> > > > > > > Changes in v3:
> > > > > > > - better led name
> > > > > > > - correct memory size
> > > > > > > - comments about missing devices
> > > > > > > 
> > > > > > > Changes in v2:
> > > > > > > - reordered, was 1/3
> > > > > > > - moved pinmuxes to their actual users, not the parents
> > > > > > >   of them
> > > > > > > - removed some already-disabled stuff
> > > > > > > - minor cleanups      
> > > > > > 
> > > > > > You won't change the muxing, so a this dtsi can be self contained?
> > > > > >     
> > > > > So you want me to put a big 
> > > > > #if defined(MX6SLL)     
> > > > 
> > > > Not sure what the comment meant, but no, don't do this. C defines in dts 
> > > > files are for symbolic names for numbers and assembling bitfields and 
> > > > that's it.  
> > > 
> > > yes, that is also my opinion. For now, there is only one user
> > > of this .dtsi, but I have another one in preparation. That is the
> > > reason for splitting things between .dts and .dtsi to avoid such ugly
> > > ifdefs  
> > 
> > Then IMHO the pnictrl-* entries shouldn't appear in the dsti.
> > 
> hmm, maybe now I understand your idea:
> You do not want only to have
> 
>   pinctrl_lm3630a_bl_gpio: lm3630a_bl_gpio_grp {
>                         fsl,pins = <
>                                 MX6SLL_PAD_EPDC_PWR_CTRL3__GPIO2_IO10   0x10059 /* HWEN */
>                         >;
>                 };
> in dts, but also  do not have these in .dtsi:
> 
>                 pinctrl-names = "default";
>                 pinctrl-0 = <&pinctrl_lm3630a_bl_gpio>;
> 
> and instead have in dts:
> &lm3630a {
>  	pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_lm3630a_bl_gpio>;
> 	
> };
> 
> 
> just to make sure I get it right before doing the restructuring work. That way of structuring things did not come to my mind, but then the .dtsi is self-contained.

That is what I mean but wait for Shawn's comments. It's just my opinion
that .dtsi and .dts files should be self-contained.

Regards,
  Marco

> Regards,
> Andreas



-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v3 2/3] ARM: dts: add Netronix E60K02 board common file

[Andreas Kemnade]

[-- Attachment #1: Type: text/plain, Size: 4019 bytes --]

On Fri, 11 Oct 2019 18:56:33 +0200
Marco Felsch <m.felsch@pengutronix.de> wrote:

> On 19-10-11 18:19, Andreas Kemnade wrote:
> > On Fri, 11 Oct 2019 17:22:14 +0200
> > Marco Felsch <m.felsch@pengutronix.de> wrote:
> >   
> > > On 19-10-11 17:05, Andreas Kemnade wrote:  
> > > > On Fri, 11 Oct 2019 09:29:27 -0500
> > > > Rob Herring <robh@kernel.org> wrote:
> > > >     
> > > > > On Fri, Oct 11, 2019 at 09:41:48AM +0200, Andreas Kemnade wrote:    
> > > > > > On Fri, 11 Oct 2019 08:56:09 +0200
> > > > > > Marco Felsch <m.felsch@pengutronix.de> wrote:
> > > > > >       
> > > > > > > Hi Andreas,
> > > > > > > 
> > > > > > > On 19-10-10 21:23, Andreas Kemnade wrote:      
> > > > > > > > The Netronix board E60K02 can be found some several Ebook-Readers,
> > > > > > > > at least the Kobo Clara HD and the Tolino Shine 3. The board
> > > > > > > > is equipped with different SoCs requiring different pinmuxes.
> > > > > > > > 
> > > > > > > > For now the following peripherals are included:
> > > > > > > > - LED
> > > > > > > > - Power Key
> > > > > > > > - Cover (gpio via hall sensor)
> > > > > > > > - RC5T619 PMIC (the kernel misses support for rtc and charger
> > > > > > > >   subdevices).
> > > > > > > > - Backlight via lm3630a
> > > > > > > > - Wifi sdio chip detection (mmc-powerseq and stuff)
> > > > > > > > 
> > > > > > > > It is based on vendor kernel but heavily reworked due to many
> > > > > > > > changed bindings.
> > > > > > > > 
> > > > > > > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > > > > > > > ---
> > > > > > > > Changes in v3:
> > > > > > > > - better led name
> > > > > > > > - correct memory size
> > > > > > > > - comments about missing devices
> > > > > > > > 
> > > > > > > > Changes in v2:
> > > > > > > > - reordered, was 1/3
> > > > > > > > - moved pinmuxes to their actual users, not the parents
> > > > > > > >   of them
> > > > > > > > - removed some already-disabled stuff
> > > > > > > > - minor cleanups        
> > > > > > > 
> > > > > > > You won't change the muxing, so a this dtsi can be self contained?
> > > > > > >       
> > > > > > So you want me to put a big 
> > > > > > #if defined(MX6SLL)       
> > > > > 
> > > > > Not sure what the comment meant, but no, don't do this. C defines in dts 
> > > > > files are for symbolic names for numbers and assembling bitfields and 
> > > > > that's it.    
> > > > 
> > > > yes, that is also my opinion. For now, there is only one user
> > > > of this .dtsi, but I have another one in preparation. That is the
> > > > reason for splitting things between .dts and .dtsi to avoid such ugly
> > > > ifdefs    
> > > 
> > > Then IMHO the pnictrl-* entries shouldn't appear in the dsti.
> > >   
> > hmm, maybe now I understand your idea:
> > You do not want only to have
> > 
> >   pinctrl_lm3630a_bl_gpio: lm3630a_bl_gpio_grp {
> >                         fsl,pins = <
> >                                 MX6SLL_PAD_EPDC_PWR_CTRL3__GPIO2_IO10   0x10059 /* HWEN */  
> >                         >;  
> >                 };
> > in dts, but also  do not have these in .dtsi:
> > 
> >                 pinctrl-names = "default";
> >                 pinctrl-0 = <&pinctrl_lm3630a_bl_gpio>;
> > 
> > and instead have in dts:
> > &lm3630a {
> >  	pinctrl-names = "default";
> >         pinctrl-0 = <&pinctrl_lm3630a_bl_gpio>;
> > 	
> > };
> > 
> > 
> > just to make sure I get it right before doing the restructuring work. That way of structuring things did not come to my mind, but then the .dtsi is self-contained.  
> 
> That is what I mean but wait for Shawn's comments. It's just my opinion
> that .dtsi and .dts files should be self-contained.

for files like the imx6sll.dtsi, I would clearly agree, here it might
hide errors like missing pinmuxes in the dts, so it is not so clear.
But if there is is consensus about .dtsi being self-contained I will not
refuse to restructurize my work.

Regards,
Andreas

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 2/3] ARM: dts: add Netronix E60K02 board common file

[Shawn Guo]

On Sun, Oct 13, 2019 at 05:56:44PM +0200, Andreas Kemnade wrote:
> On Fri, 11 Oct 2019 18:56:33 +0200
> Marco Felsch <m.felsch@pengutronix.de> wrote:
> 
> > On 19-10-11 18:19, Andreas Kemnade wrote:
> > > On Fri, 11 Oct 2019 17:22:14 +0200
> > > Marco Felsch <m.felsch@pengutronix.de> wrote:
> > >   
> > > > On 19-10-11 17:05, Andreas Kemnade wrote:  
> > > > > On Fri, 11 Oct 2019 09:29:27 -0500
> > > > > Rob Herring <robh@kernel.org> wrote:
> > > > >     
> > > > > > On Fri, Oct 11, 2019 at 09:41:48AM +0200, Andreas Kemnade wrote:    
> > > > > > > On Fri, 11 Oct 2019 08:56:09 +0200
> > > > > > > Marco Felsch <m.felsch@pengutronix.de> wrote:
> > > > > > >       
> > > > > > > > Hi Andreas,
> > > > > > > > 
> > > > > > > > On 19-10-10 21:23, Andreas Kemnade wrote:      
> > > > > > > > > The Netronix board E60K02 can be found some several Ebook-Readers,
> > > > > > > > > at least the Kobo Clara HD and the Tolino Shine 3. The board
> > > > > > > > > is equipped with different SoCs requiring different pinmuxes.
> > > > > > > > > 
> > > > > > > > > For now the following peripherals are included:
> > > > > > > > > - LED
> > > > > > > > > - Power Key
> > > > > > > > > - Cover (gpio via hall sensor)
> > > > > > > > > - RC5T619 PMIC (the kernel misses support for rtc and charger
> > > > > > > > >   subdevices).
> > > > > > > > > - Backlight via lm3630a
> > > > > > > > > - Wifi sdio chip detection (mmc-powerseq and stuff)
> > > > > > > > > 
> > > > > > > > > It is based on vendor kernel but heavily reworked due to many
> > > > > > > > > changed bindings.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > > > > > > > > ---
> > > > > > > > > Changes in v3:
> > > > > > > > > - better led name
> > > > > > > > > - correct memory size
> > > > > > > > > - comments about missing devices
> > > > > > > > > 
> > > > > > > > > Changes in v2:
> > > > > > > > > - reordered, was 1/3
> > > > > > > > > - moved pinmuxes to their actual users, not the parents
> > > > > > > > >   of them
> > > > > > > > > - removed some already-disabled stuff
> > > > > > > > > - minor cleanups        
> > > > > > > > 
> > > > > > > > You won't change the muxing, so a this dtsi can be self contained?
> > > > > > > >       
> > > > > > > So you want me to put a big 
> > > > > > > #if defined(MX6SLL)       
> > > > > > 
> > > > > > Not sure what the comment meant, but no, don't do this. C defines in dts 
> > > > > > files are for symbolic names for numbers and assembling bitfields and 
> > > > > > that's it.    
> > > > > 
> > > > > yes, that is also my opinion. For now, there is only one user
> > > > > of this .dtsi, but I have another one in preparation. That is the
> > > > > reason for splitting things between .dts and .dtsi to avoid such ugly
> > > > > ifdefs    
> > > > 
> > > > Then IMHO the pnictrl-* entries shouldn't appear in the dsti.
> > > >   
> > > hmm, maybe now I understand your idea:
> > > You do not want only to have
> > > 
> > >   pinctrl_lm3630a_bl_gpio: lm3630a_bl_gpio_grp {
> > >                         fsl,pins = <
> > >                                 MX6SLL_PAD_EPDC_PWR_CTRL3__GPIO2_IO10   0x10059 /* HWEN */  
> > >                         >;  
> > >                 };
> > > in dts, but also  do not have these in .dtsi:
> > > 
> > >                 pinctrl-names = "default";
> > >                 pinctrl-0 = <&pinctrl_lm3630a_bl_gpio>;
> > > 
> > > and instead have in dts:
> > > &lm3630a {
> > >  	pinctrl-names = "default";
> > >         pinctrl-0 = <&pinctrl_lm3630a_bl_gpio>;
> > > 	
> > > };
> > > 
> > > 
> > > just to make sure I get it right before doing the restructuring work. That way of structuring things did not come to my mind, but then the .dtsi is self-contained.  
> > 
> > That is what I mean but wait for Shawn's comments. It's just my opinion
> > that .dtsi and .dts files should be self-contained.
> 
> for files like the imx6sll.dtsi, I would clearly agree, here it might
> hide errors like missing pinmuxes in the dts, so it is not so clear.
> But if there is is consensus about .dtsi being self-contained I will not
> refuse to restructurize my work.

Yes, I would appreciate the effort of keep .dtsi being self-contained.

Shawn

Re: [PATCH v3 2/3] ARM: dts: add Netronix E60K02 board common file

[Andreas Kemnade]

Hi Shawn,

On Fri, 25 Oct 2019 17:14:04 +0800
Shawn Guo <shawnguo@kernel.org> wrote:

> On Sun, Oct 13, 2019 at 05:56:44PM +0200, Andreas Kemnade wrote:
> > On Fri, 11 Oct 2019 18:56:33 +0200
> > Marco Felsch <m.felsch@pengutronix.de> wrote:
> >   
> > > On 19-10-11 18:19, Andreas Kemnade wrote:  
> > > > On Fri, 11 Oct 2019 17:22:14 +0200
> > > > Marco Felsch <m.felsch@pengutronix.de> wrote:
> > > >     
> > > > > On 19-10-11 17:05, Andreas Kemnade wrote:    
> > > > > > On Fri, 11 Oct 2019 09:29:27 -0500
> > > > > > Rob Herring <robh@kernel.org> wrote:
> > > > > >       
> > > > > > > On Fri, Oct 11, 2019 at 09:41:48AM +0200, Andreas Kemnade wrote:      
> > > > > > > > On Fri, 11 Oct 2019 08:56:09 +0200
> > > > > > > > Marco Felsch <m.felsch@pengutronix.de> wrote:
> > > > > > > >         
> > > > > > > > > Hi Andreas,
> > > > > > > > > 
> > > > > > > > > On 19-10-10 21:23, Andreas Kemnade wrote:        
> > > > > > > > > > The Netronix board E60K02 can be found some several Ebook-Readers,
> > > > > > > > > > at least the Kobo Clara HD and the Tolino Shine 3. The board
> > > > > > > > > > is equipped with different SoCs requiring different pinmuxes.
> > > > > > > > > > 
> > > > > > > > > > For now the following peripherals are included:
> > > > > > > > > > - LED
> > > > > > > > > > - Power Key
> > > > > > > > > > - Cover (gpio via hall sensor)
> > > > > > > > > > - RC5T619 PMIC (the kernel misses support for rtc and charger
> > > > > > > > > >   subdevices).
> > > > > > > > > > - Backlight via lm3630a
> > > > > > > > > > - Wifi sdio chip detection (mmc-powerseq and stuff)
> > > > > > > > > > 
> > > > > > > > > > It is based on vendor kernel but heavily reworked due to many
> > > > > > > > > > changed bindings.
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > > > > > > > > > ---
> > > > > > > > > > Changes in v3:
> > > > > > > > > > - better led name
> > > > > > > > > > - correct memory size
> > > > > > > > > > - comments about missing devices
> > > > > > > > > > 
> > > > > > > > > > Changes in v2:
> > > > > > > > > > - reordered, was 1/3
> > > > > > > > > > - moved pinmuxes to their actual users, not the parents
> > > > > > > > > >   of them
> > > > > > > > > > - removed some already-disabled stuff
> > > > > > > > > > - minor cleanups          
> > > > > > > > > 
> > > > > > > > > You won't change the muxing, so a this dtsi can be self contained?
> > > > > > > > >         
> > > > > > > > So you want me to put a big 
> > > > > > > > #if defined(MX6SLL)         
> > > > > > > 
> > > > > > > Not sure what the comment meant, but no, don't do this. C defines in dts 
> > > > > > > files are for symbolic names for numbers and assembling bitfields and 
> > > > > > > that's it.      
> > > > > > 
> > > > > > yes, that is also my opinion. For now, there is only one user
> > > > > > of this .dtsi, but I have another one in preparation. That is the
> > > > > > reason for splitting things between .dts and .dtsi to avoid such ugly
> > > > > > ifdefs      
> > > > > 
> > > > > Then IMHO the pnictrl-* entries shouldn't appear in the dsti.
> > > > >     
> > > > hmm, maybe now I understand your idea:
> > > > You do not want only to have
> > > > 
> > > >   pinctrl_lm3630a_bl_gpio: lm3630a_bl_gpio_grp {
> > > >                         fsl,pins = <
> > > >                                 MX6SLL_PAD_EPDC_PWR_CTRL3__GPIO2_IO10   0x10059 /* HWEN */    
> > > >                         >;    
> > > >                 };
> > > > in dts, but also  do not have these in .dtsi:
> > > > 
> > > >                 pinctrl-names = "default";
> > > >                 pinctrl-0 = <&pinctrl_lm3630a_bl_gpio>;
> > > > 
> > > > and instead have in dts:
> > > > &lm3630a {
> > > >  	pinctrl-names = "default";
> > > >         pinctrl-0 = <&pinctrl_lm3630a_bl_gpio>;
> > > > 	
> > > > };
> > > > 
> > > > 
> > > > just to make sure I get it right before doing the restructuring work. That way of structuring things did not come to my mind, but then the .dtsi is self-contained.    
> > > 
> > > That is what I mean but wait for Shawn's comments. It's just my opinion
> > > that .dtsi and .dts files should be self-contained.  
> > 
> > for files like the imx6sll.dtsi, I would clearly agree, here it might
> > hide errors like missing pinmuxes in the dts, so it is not so clear.
> > But if there is is consensus about .dtsi being self-contained I will not
> > refuse to restructurize my work.  
> 
> Yes, I would appreciate the effort of keep .dtsi being self-contained.

ok, then I will restructurize as proposed and create a v4 this weekend.

Regards,
Andreas

Re: [PATCH v3 2/3] ARM: dts: add Netronix E60K02 board common file

[Shawn Guo]

On Thu, Oct 10, 2019 at 09:23:56PM +0200, Andreas Kemnade wrote:
> The Netronix board E60K02 can be found some several Ebook-Readers,
> at least the Kobo Clara HD and the Tolino Shine 3. The board
> is equipped with different SoCs requiring different pinmuxes.
> 
> For now the following peripherals are included:
> - LED
> - Power Key
> - Cover (gpio via hall sensor)
> - RC5T619 PMIC (the kernel misses support for rtc and charger
>   subdevices).
> - Backlight via lm3630a
> - Wifi sdio chip detection (mmc-powerseq and stuff)
> 
> It is based on vendor kernel but heavily reworked due to many
> changed bindings.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
> Changes in v3:
> - better led name
> - correct memory size
> - comments about missing devices
> 
> Changes in v2:
> - reordered, was 1/3
> - moved pinmuxes to their actual users, not the parents
>   of them
> - removed some already-disabled stuff
> - minor cleanups
> 
> backligt dependencies:
> module autoloading:
> https://patchwork.kernel.org/patch/11139987/ 
> enable-gpios property (accepted and acked):
> https://patchwork.kernel.org/patch/11143795/
> 
>  arch/arm/boot/dts/e60k02.dtsi | 337 ++++++++++++++++++++++++++++++++++
>  1 file changed, 337 insertions(+)
>  create mode 100644 arch/arm/boot/dts/e60k02.dtsi
> 
> diff --git a/arch/arm/boot/dts/e60k02.dtsi b/arch/arm/boot/dts/e60k02.dtsi
> new file mode 100644
> index 0000000000000..84c0447b9a1bd
> --- /dev/null
> +++ b/arch/arm/boot/dts/e60k02.dtsi
> @@ -0,0 +1,337 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2019 Andreas Kemnade
> + * based on works
> + * Copyright 2016 Freescale Semiconductor, Inc.
> + * and
> + * Copyright (C) 2014 Ricoh Electronic Devices Co., Ltd
> + *
> + * Netronix E60K02 board common.
> + * This board is equipped with different SoCs and
> + * found in ebook-readers like the Kobo Clara HD (with i.MX6SLL) and
> + * the Tolino Shine 3 (with i.MX6SL)
> + */
> +#include <dt-bindings/input/input.h>
> +
> +/ {
> +
> +	chosen {
> +		stdout-path = &uart1;
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_gpio_keys>;

Please have a newline between property list and child node.

> +		power {
> +			label = "Power";
> +			gpios = <&gpio5 8 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_POWER>;
> +			gpio-key,wakeup;

Check out Documentation/devicetree/bindings/power/wakeup-source.txt

> +		};
> +		cover {
> +			label = "Cover";
> +			gpios = <&gpio5 12 GPIO_ACTIVE_LOW>;
> +			linux,code = <SW_LID>;
> +			linux,input-type = <EV_SW>;
> +			gpio-key,wakeup;
> +		};
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_led>;
> +
> +		on {
> +			label = "e60k02:white:on";
> +			gpios = <&gpio5 7 GPIO_ACTIVE_LOW>;
> +			linux,default-trigger = "timer";
> +		};
> +	};
> +
> +	memory {
> +		reg = <0x80000000 0x20000000>;
> +	};
> +
> +	reg_wifi: regulator-wifi {
> +		compatible = "regulator-fixed";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_wifi_power>;
> +		regulator-name = "SD3_SPWR";
> +		regulator-min-microvolt = <3000000>;
> +		regulator-max-microvolt = <3000000>;
> +

Drop this newline.

> +		gpio = <&gpio4 29 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +

Ditto

> +	};
> +
> +	wifi_pwrseq: wifi_pwrseq {
> +		compatible = "mmc-pwrseq-simple";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_wifi_reset>;
> +		post-power-on-delay-ms = <20>;
> +		reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
> +	};
> +

Ditto

> +};
> +
> +
> +&i2c1 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default","sleep";
> +	pinctrl-0 = <&pinctrl_i2c1>;
> +	pinctrl-1 = <&pinctrl_i2c1_sleep>;
> +	status = "okay";
> +
> +	lm3630a: backlight@36 {
> +		reg = <0x36>;
> +

Ditto

> +		compatible = "ti,lm3630a";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_lm3630a_bl_gpio>;
> +		enable-gpios = <&gpio2 10 GPIO_ACTIVE_HIGH>;
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		led@0 {
> +			reg = <0>;
> +			led-sources = <0>;
> +			label = "backlight_warm";
> +			default-brightness = <0>;
> +			max-brightness = <255>;
> +		};
> +
> +		led@1 {
> +			reg = <1>;
> +			led-sources = <1>;
> +			label = "backlight_cold";
> +			default-brightness = <0>;
> +			max-brightness = <255>;
> +		};
> +

Ditto

> +	};
> +};
> +
> +&i2c2 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default","sleep";
> +	pinctrl-0 = <&pinctrl_i2c2>;
> +	pinctrl-1 = <&pinctrl_i2c2_sleep>;
> +	status = "okay";
> +
> +	/* TODO: CYTTSP5 touch controller at 0x24 */
> +
> +	/* TODO: TPS65185 PMIC for E Ink at 0x68 */
> +
> +};
> +
> +&i2c3 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c3>;
> +	status = "okay";
> +
> +	ricoh619: pmic@32 {
> +		compatible = "ricoh,rc5t619";
> +		reg = <0x32>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_ricoh_gpio>;
> +		system-power-controller;
> +
> +		regulators {
> +			dcdc1_reg: DCDC1 {
> +				regulator-name = "DCDC1";
> +				regulator-min-microvolt = <300000>;
> +				regulator-max-microvolt = <1875000>;
> +				regulator-always-on;
> +				regulator-boot-on;

Have a newline between property list and child node.

> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-max-microvolt = <900000>;
> +					regulator-suspend-min-microvolt = <900000>;
> +				};
> +			};
> +
> +			/* Core3_3V3 */
> +			dcdc2_reg: DCDC2 {
> +				regulator-name = "DCDC2";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-max-microvolt = <3300000>;
> +					regulator-suspend-min-microvolt = <3300000>;
> +				};
> +			};
> +
> +			dcdc3_reg: DCDC3 {
> +				regulator-name = "DCDC3";
> +				regulator-min-microvolt = <300000>;
> +				regulator-max-microvolt = <1875000>;
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-max-microvolt = <1140000>;
> +					regulator-suspend-min-microvolt = <1140000>;
> +				};
> +			};
> +
> +			/* Core4_1V2 */
> +			dcdc4_reg: DCDC4 {
> +				regulator-name = "DCDC4";
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-max-microvolt = <1140000>;
> +					regulator-suspend-min-microvolt = <1140000>;
> +				};
> +			};
> +
> +			/* Core4_1V8 */
> +			dcdc5_reg: DCDC5 {
> +				regulator-name = "DCDC5";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-max-microvolt = <1700000>;
> +					regulator-suspend-min-microvolt = <1700000>;
> +				};
> +			};
> +
> +			/* IR_3V3 */
> +			ldo1_reg: LDO1  {
> +				regulator-name = "LDO1";
> +				regulator-boot-on;
> +			};
> +
> +			/* Core1_3V3 */
> +			ldo2_reg: LDO2  {
> +				regulator-name = "LDO2";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-max-microvolt = <3000000>;
> +					regulator-suspend-min-microvolt = <3000000>;
> +				};
> +			};
> +
> +			/* Core5_1V2 */
> +			ldo3_reg: LDO3  {
> +				regulator-name = "LDO3";
> +				regulator-always-on;
> +				regulator-boot-on;
> +			};
> +
> +			ldo4_reg: LDO4 {
> +				regulator-name = "LDO4";
> +				regulator-boot-on;
> +			};
> +
> +			/* SPD_3V3 */
> +			ldo5_reg: LDO5 {
> +				regulator-name = "LDO5";
> +				regulator-always-on;
> +				regulator-boot-on;
> +			};
> +
> +			/* DDR_0V6 */
> +			ldo6_reg: LDO6 {
> +				regulator-name = "LDO6";
> +				regulator-always-on;
> +				regulator-boot-on;
> +			};
> +
> +			/* VDD_PWM */
> +			ldo7_reg: LDO7 {
> +				regulator-name = "LDO7";
> +				regulator-always-on;
> +				regulator-boot-on;
> +			};
> +
> +			/* ldo_1v8 */
> +			ldo8_reg: LDO8 {
> +				regulator-name = "LDO8";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-always-on;
> +				regulator-boot-on;
> +			};
> +
> +			ldo9_reg: LDO9 {
> +				regulator-name = "LDO9";
> +				regulator-boot-on;
> +			};
> +
> +			ldo10_reg: LDO10 {
> +				regulator-name = "LDO10";
> +				regulator-boot-on;
> +			};
> +
> +			ldortc1_reg: LDORTC1  {
> +				regulator-name = "LDORTC1";
> +				regulator-boot-on;
> +			};
> +
> +			ldortc2_reg: LDORTC2 {
> +				regulator-name = "LDORTC2";
> +				regulator-boot-on;
> +			};
> +		};
> +

Drop the newline.

> +	};
> +

Ditto

Shawn

> +};
> +
> +&snvs_rtc {
> +	/* we are using the rtc in the pmic, not disabled imx6sll.dtsi */
> +	status = "disabled";
> +};
> +
> +&uart1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart1>;
> +	status = "okay";
> +};
> +
> +&usdhc2 {
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz","sleep";
> +	pinctrl-0 = <&pinctrl_usdhc2>;
> +	pinctrl-1 = <&pinctrl_usdhc2_100mhz>;
> +	pinctrl-2 = <&pinctrl_usdhc2_200mhz>;
> +	pinctrl-3 = <&pinctrl_usdhc2_sleep>;
> +	non-removable;
> +	status = "okay";
> +};
> +
> +&usdhc3 {
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz","sleep";
> +	pinctrl-0 = <&pinctrl_usdhc3>;
> +	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
> +	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
> +	pinctrl-3 = <&pinctrl_usdhc3_sleep>;
> +	vmmc-supply = <&reg_wifi>;
> +	mmc-pwrseq = <&wifi_pwrseq>;
> +	cap-power-off-card;
> +	non-removable;
> +	status = "okay";
> +};
> +
> +&usbotg1 {
> +	pinctrl-names = "default";
> +	disable-over-current;
> +	srp-disable;
> +	hnp-disable;
> +	adp-disable;
> +	status = "okay";
> +};
> -- 
> 2.20.1
> 

[PATCH v3 3/3] ARM: dts: imx: add devicetree for Kobo Clara HD

[Andreas Kemnade]

This adds a devicetree for the Kobo Clara HD Ebook reader. It
is on based on boards called "e60k02". It is equipped with an
imx6sll SoC.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 arch/arm/boot/dts/Makefile                 |   3 +-
 arch/arm/boot/dts/imx6sll-kobo-clarahd.dts | 279 +++++++++++++++++++++
 2 files changed, 281 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/imx6sll-kobo-clarahd.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 9159fa2cea90c..a8a235c74c37f 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -551,7 +551,8 @@ dtb-$(CONFIG_SOC_IMX6SL) += \
 	imx6sl-evk.dtb \
 	imx6sl-warp.dtb
 dtb-$(CONFIG_SOC_IMX6SLL) += \
-	imx6sll-evk.dtb
+	imx6sll-evk.dtb \
+	imx6sll-kobo-clarahd.dtb
 dtb-$(CONFIG_SOC_IMX6SX) += \
 	imx6sx-nitrogen6sx.dtb \
 	imx6sx-sabreauto.dtb \
diff --git a/arch/arm/boot/dts/imx6sll-kobo-clarahd.dts b/arch/arm/boot/dts/imx6sll-kobo-clarahd.dts
new file mode 100644
index 0000000000000..c2df2a567585f
--- /dev/null
+++ b/arch/arm/boot/dts/imx6sll-kobo-clarahd.dts
@@ -0,0 +1,279 @@
+// SPDX-License-Identifier: (GPL-2.0)
+/*
+ * Device tree for the Kobo Clara HD ebook reader
+ *
+ * Name on mainboard is: 37NB-E60K00+4A4
+ * Serials start with: E60K02 (a number also seen in
+ * vendor kernel sources)
+ *
+ * This mainboard seems to be equipped with different SoCs.
+ * In the Kobo Clara HD ebook reader it is an i.MX6SLL
+ *
+ * Copyright 2019 Andreas Kemnade
+ * based on works
+ * Copyright 2016 Freescale Semiconductor, Inc.
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/gpio/gpio.h>
+#include "imx6sll.dtsi"
+#include "e60k02.dtsi"
+
+/ {
+	model = "Kobo Clara HD";
+	compatible = "kobo,clarahd", "fsl,imx6sll";
+};
+
+&clks {
+	assigned-clocks = <&clks IMX6SLL_CLK_PLL4_AUDIO_DIV>;
+	assigned-clock-rates = <393216000>;
+};
+
+&cpu0 {
+	arm-supply = <&dcdc3_reg>;
+	soc-supply = <&dcdc1_reg>;
+};
+
+&iomuxc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_hog>;
+
+	imx6sll-lpddr3-arm2 {
+		pinctrl_hog: hoggrp {
+			fsl,pins = <
+				MX6SLL_PAD_LCD_DATA00__GPIO2_IO20	0x79
+				MX6SLL_PAD_LCD_DATA01__GPIO2_IO21	0x79
+				MX6SLL_PAD_LCD_DATA02__GPIO2_IO22	0x79
+				MX6SLL_PAD_LCD_DATA03__GPIO2_IO23	0x79
+				MX6SLL_PAD_LCD_DATA04__GPIO2_IO24	0x79
+				MX6SLL_PAD_LCD_DATA05__GPIO2_IO25	0x79
+				MX6SLL_PAD_LCD_DATA06__GPIO2_IO26	0x79
+				MX6SLL_PAD_LCD_DATA07__GPIO2_IO27	0x79
+				MX6SLL_PAD_LCD_DATA08__GPIO2_IO28	0x79
+				MX6SLL_PAD_LCD_DATA09__GPIO2_IO29	0x79
+				MX6SLL_PAD_LCD_DATA10__GPIO2_IO30	0x79
+				MX6SLL_PAD_LCD_DATA11__GPIO2_IO31	0x79
+				MX6SLL_PAD_LCD_DATA12__GPIO3_IO00	0x79
+				MX6SLL_PAD_LCD_DATA13__GPIO3_IO01	0x79
+				MX6SLL_PAD_LCD_DATA14__GPIO3_IO02	0x79
+				MX6SLL_PAD_LCD_DATA15__GPIO3_IO03	0x79
+				MX6SLL_PAD_LCD_DATA16__GPIO3_IO04	0x79
+				MX6SLL_PAD_LCD_DATA17__GPIO3_IO05	0x79
+				MX6SLL_PAD_LCD_DATA18__GPIO3_IO06	0x79
+				MX6SLL_PAD_LCD_DATA19__GPIO3_IO07	0x79
+				MX6SLL_PAD_LCD_DATA20__GPIO3_IO08	0x79
+				MX6SLL_PAD_LCD_DATA21__GPIO3_IO09	0x79
+				MX6SLL_PAD_LCD_DATA22__GPIO3_IO10	0x79
+				MX6SLL_PAD_LCD_DATA23__GPIO3_IO11	0x79
+				MX6SLL_PAD_LCD_CLK__GPIO2_IO15		0x79
+				MX6SLL_PAD_LCD_ENABLE__GPIO2_IO16	0x79
+				MX6SLL_PAD_LCD_HSYNC__GPIO2_IO17	0x79
+				MX6SLL_PAD_LCD_VSYNC__GPIO2_IO18	0x79
+				MX6SLL_PAD_LCD_RESET__GPIO2_IO19	0x79
+				MX6SLL_PAD_KEY_COL3__GPIO3_IO30		0x79
+				MX6SLL_PAD_KEY_ROW7__GPIO4_IO07		0x79
+				MX6SLL_PAD_ECSPI2_MOSI__GPIO4_IO13	0x79
+				MX6SLL_PAD_KEY_COL5__GPIO4_IO02		0x79
+				MX6SLL_PAD_KEY_ROW6__GPIO4_IO05		0x79
+			>;
+		};
+
+		pinctrl_wifi_reset: wifi_reset_grp {
+			fsl,pins = <
+				MX6SLL_PAD_SD2_DATA7__GPIO5_IO00	0x10059		/* WIFI_RST */
+			>;
+		};
+
+		pinctrl_wifi_power: wifi_power_grp {
+			fsl,pins = <
+				MX6SLL_PAD_SD2_DATA6__GPIO4_IO29	0x10059		/* WIFI_3V3_ON */
+			>;
+		};
+
+		pinctrl_audmux3: audmux3grp {
+			fsl,pins = <
+				MX6SLL_PAD_AUD_TXC__AUD3_TXC		0x4130b0
+				MX6SLL_PAD_AUD_TXFS__AUD3_TXFS		0x4130b0
+				MX6SLL_PAD_AUD_TXD__AUD3_TXD		0x4110b0
+				MX6SLL_PAD_AUD_RXD__AUD3_RXD		0x4130b0
+				MX6SLL_PAD_AUD_MCLK__AUDIO_CLK_OUT	0x4130b0
+			>;
+		};
+
+		pinctrl_led: ledgrp {
+			fsl,pins = <
+				MX6SLL_PAD_SD1_DATA6__GPIO5_IO07 0x17059
+			>;
+		};
+
+		pinctrl_uart1: uart1grp {
+			fsl,pins = <
+				MX6SLL_PAD_UART1_TXD__UART1_DCE_TX 0x1b0b1
+				MX6SLL_PAD_UART1_RXD__UART1_DCE_RX 0x1b0b1
+			>;
+		};
+
+		pinctrl_usdhc2: usdhc2grp {
+			fsl,pins = <
+				MX6SLL_PAD_SD2_CMD__SD2_CMD		0x17059
+				MX6SLL_PAD_SD2_CLK__SD2_CLK		0x13059
+				MX6SLL_PAD_SD2_DATA0__SD2_DATA0		0x17059
+				MX6SLL_PAD_SD2_DATA1__SD2_DATA1		0x17059
+				MX6SLL_PAD_SD2_DATA2__SD2_DATA2		0x17059
+				MX6SLL_PAD_SD2_DATA3__SD2_DATA3		0x17059
+			>;
+		};
+
+		pinctrl_usdhc2_100mhz: usdhc2grp_100mhz {
+			fsl,pins = <
+				MX6SLL_PAD_SD2_CMD__SD2_CMD		0x170b9
+				MX6SLL_PAD_SD2_CLK__SD2_CLK		0x130b9
+				MX6SLL_PAD_SD2_DATA0__SD2_DATA0		0x170b9
+				MX6SLL_PAD_SD2_DATA1__SD2_DATA1		0x170b9
+				MX6SLL_PAD_SD2_DATA2__SD2_DATA2		0x170b9
+				MX6SLL_PAD_SD2_DATA3__SD2_DATA3		0x170b9
+			>;
+		};
+
+		pinctrl_usdhc2_200mhz: usdhc2grp_200mhz {
+			fsl,pins = <
+				MX6SLL_PAD_SD2_CMD__SD2_CMD		0x170f9
+				MX6SLL_PAD_SD2_CLK__SD2_CLK		0x130f9
+				MX6SLL_PAD_SD2_DATA0__SD2_DATA0		0x170f9
+				MX6SLL_PAD_SD2_DATA1__SD2_DATA1		0x170f9
+				MX6SLL_PAD_SD2_DATA2__SD2_DATA2		0x170f9
+				MX6SLL_PAD_SD2_DATA3__SD2_DATA3		0x170f9
+			>;
+		};
+
+		pinctrl_usdhc2_sleep: usdhc2grp_sleep {
+			fsl,pins = <
+				MX6SLL_PAD_SD2_CMD__GPIO5_IO04		0x100f9
+				MX6SLL_PAD_SD2_CLK__GPIO5_IO05		0x100f9
+				MX6SLL_PAD_SD2_DATA0__GPIO5_IO01	0x100f9
+				MX6SLL_PAD_SD2_DATA1__GPIO4_IO30	0x100f9
+				MX6SLL_PAD_SD2_DATA2__GPIO5_IO03	0x100f9
+				MX6SLL_PAD_SD2_DATA3__GPIO4_IO28	0x100f9
+			>;
+		};
+
+		pinctrl_usdhc3: usdhc3grp {
+			fsl,pins = <
+				MX6SLL_PAD_SD3_CMD__SD3_CMD	0x11059
+				MX6SLL_PAD_SD3_CLK__SD3_CLK	0x11059
+				MX6SLL_PAD_SD3_DATA0__SD3_DATA0	0x11059
+				MX6SLL_PAD_SD3_DATA1__SD3_DATA1	0x11059
+				MX6SLL_PAD_SD3_DATA2__SD3_DATA2	0x11059
+				MX6SLL_PAD_SD3_DATA3__SD3_DATA3	0x11059
+			>;
+		};
+
+		pinctrl_usdhc3_100mhz: usdhc3grp_100mhz {
+			fsl,pins = <
+				MX6SLL_PAD_SD3_CMD__SD3_CMD	0x170b9
+				MX6SLL_PAD_SD3_CLK__SD3_CLK	0x170b9
+				MX6SLL_PAD_SD3_DATA0__SD3_DATA0	0x170b9
+				MX6SLL_PAD_SD3_DATA1__SD3_DATA1	0x170b9
+				MX6SLL_PAD_SD3_DATA2__SD3_DATA2	0x170b9
+				MX6SLL_PAD_SD3_DATA3__SD3_DATA3	0x170b9
+			>;
+		};
+
+		pinctrl_usdhc3_200mhz: usdhc3grp_200mhz {
+			fsl,pins = <
+				MX6SLL_PAD_SD3_CMD__SD3_CMD	0x170f9
+				MX6SLL_PAD_SD3_CLK__SD3_CLK	0x170f9
+				MX6SLL_PAD_SD3_DATA0__SD3_DATA0	0x170f9
+				MX6SLL_PAD_SD3_DATA1__SD3_DATA1	0x170f9
+				MX6SLL_PAD_SD3_DATA2__SD3_DATA2	0x170f9
+				MX6SLL_PAD_SD3_DATA3__SD3_DATA3	0x170f9
+			>;
+		};
+
+		pinctrl_usdhc3_sleep: usdhc3grp_sleep {
+			fsl,pins = <
+				MX6SLL_PAD_SD3_CMD__GPIO5_IO21	0x100c1
+				MX6SLL_PAD_SD3_CLK__GPIO5_IO18	0x100c1
+				MX6SLL_PAD_SD3_DATA0__GPIO5_IO19	0x100c1
+				MX6SLL_PAD_SD3_DATA1__GPIO5_IO20	0x100c1
+				MX6SLL_PAD_SD3_DATA2__GPIO5_IO16	0x100c1
+				MX6SLL_PAD_SD3_DATA3__GPIO5_IO17	0x100c1
+			>;
+		};
+
+		pinctrl_usbotg1: usbotg1grp {
+			fsl,pins = <
+				MX6SLL_PAD_EPDC_PWR_COM__USB_OTG1_ID 0x17059
+			>;
+		};
+
+		pinctrl_i2c1: i2c1grp {
+			fsl,pins = <
+				MX6SLL_PAD_I2C1_SCL__I2C1_SCL	 0x4001f8b1
+				MX6SLL_PAD_I2C1_SDA__I2C1_SDA	 0x4001f8b1
+			>;
+		};
+
+		pinctrl_i2c1_sleep: i2c1grp_sleep {
+			fsl,pins = <
+				MX6SLL_PAD_I2C1_SCL__I2C1_SCL	 0x400108b1
+				MX6SLL_PAD_I2C1_SDA__I2C1_SDA	 0x400108b1
+			>;
+		};
+
+		pinctrl_i2c2: i2c2grp {
+			fsl,pins = <
+				MX6SLL_PAD_I2C2_SCL__I2C2_SCL	 0x4001f8b1
+				MX6SLL_PAD_I2C2_SDA__I2C2_SDA	 0x4001f8b1
+			>;
+		};
+
+		pinctrl_i2c2_sleep: i2c2grp_sleep {
+			fsl,pins = <
+				MX6SLL_PAD_I2C2_SCL__I2C2_SCL	 0x400108b1
+				MX6SLL_PAD_I2C2_SDA__I2C2_SDA	 0x400108b1
+			>;
+		};
+
+		pinctrl_i2c3: i2c3grp {
+			fsl,pins = <
+				MX6SLL_PAD_REF_CLK_24M__I2C3_SCL  0x4001f8b1
+				MX6SLL_PAD_REF_CLK_32K__I2C3_SDA  0x4001f8b1
+			>;
+		};
+
+		pinctrl_ecspi1: ecspi1grp {
+			fsl,pins = <
+				MX6SLL_PAD_ECSPI1_MISO__ECSPI1_MISO	 0x100b1
+				MX6SLL_PAD_ECSPI1_MOSI__ECSPI1_MOSI	 0x100b1
+				MX6SLL_PAD_ECSPI1_SCLK__ECSPI1_SCLK	 0x100b1
+				MX6SLL_PAD_ECSPI1_SS0__GPIO4_IO11	 0x100b1
+			>;
+		};
+
+		pinctrl_lm3630a_bl_gpio: lm3630a_bl_gpio_grp {
+			fsl,pins = <
+				MX6SLL_PAD_EPDC_PWR_CTRL3__GPIO2_IO10	0x10059 /* HWEN */
+			>;
+		};
+
+		pinctrl_ricoh_gpio: ricoh_gpio_grp {
+			fsl,pins = <
+				MX6SLL_PAD_SD1_CLK__GPIO5_IO15	0x1b8b1 /* ricoh619 chg */
+				MX6SLL_PAD_SD1_DATA0__GPIO5_IO11 0x1b8b1 /* ricoh619 irq */
+				MX6SLL_PAD_KEY_COL2__GPIO3_IO28	0x1b8b1 /* ricoh619 bat_low_int */
+			>;
+		};
+
+		pinctrl_gpio_keys: gpio_keys_grp {
+			fsl,pins = <
+				MX6SLL_PAD_SD1_DATA1__GPIO5_IO08	0x17059	/* PWR_SW */
+				MX6SLL_PAD_SD1_DATA4__GPIO5_IO12	0x17059	/* HALL_EN */
+			>;
+		};
+
+	};
+};
+
-- 
2.20.1

Re: [PATCH v3 3/3] ARM: dts: imx: add devicetree for Kobo Clara HD

[Shawn Guo]

On Thu, Oct 10, 2019 at 09:23:57PM +0200, Andreas Kemnade wrote:
> This adds a devicetree for the Kobo Clara HD Ebook reader. It
> is on based on boards called "e60k02". It is equipped with an
> imx6sll SoC.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
>  arch/arm/boot/dts/Makefile                 |   3 +-
>  arch/arm/boot/dts/imx6sll-kobo-clarahd.dts | 279 +++++++++++++++++++++
>  2 files changed, 281 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/imx6sll-kobo-clarahd.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 9159fa2cea90c..a8a235c74c37f 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -551,7 +551,8 @@ dtb-$(CONFIG_SOC_IMX6SL) += \
>  	imx6sl-evk.dtb \
>  	imx6sl-warp.dtb
>  dtb-$(CONFIG_SOC_IMX6SLL) += \
> -	imx6sll-evk.dtb
> +	imx6sll-evk.dtb \
> +	imx6sll-kobo-clarahd.dtb
>  dtb-$(CONFIG_SOC_IMX6SX) += \
>  	imx6sx-nitrogen6sx.dtb \
>  	imx6sx-sabreauto.dtb \
> diff --git a/arch/arm/boot/dts/imx6sll-kobo-clarahd.dts b/arch/arm/boot/dts/imx6sll-kobo-clarahd.dts
> new file mode 100644
> index 0000000000000..c2df2a567585f
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6sll-kobo-clarahd.dts
> @@ -0,0 +1,279 @@
> +// SPDX-License-Identifier: (GPL-2.0)
> +/*
> + * Device tree for the Kobo Clara HD ebook reader
> + *
> + * Name on mainboard is: 37NB-E60K00+4A4
> + * Serials start with: E60K02 (a number also seen in
> + * vendor kernel sources)
> + *
> + * This mainboard seems to be equipped with different SoCs.
> + * In the Kobo Clara HD ebook reader it is an i.MX6SLL
> + *
> + * Copyright 2019 Andreas Kemnade
> + * based on works
> + * Copyright 2016 Freescale Semiconductor, Inc.
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include "imx6sll.dtsi"
> +#include "e60k02.dtsi"
> +
> +/ {
> +	model = "Kobo Clara HD";
> +	compatible = "kobo,clarahd", "fsl,imx6sll";
> +};
> +
> +&clks {
> +	assigned-clocks = <&clks IMX6SLL_CLK_PLL4_AUDIO_DIV>;
> +	assigned-clock-rates = <393216000>;
> +};
> +
> +&cpu0 {
> +	arm-supply = <&dcdc3_reg>;
> +	soc-supply = <&dcdc1_reg>;
> +};
> +
> +&iomuxc {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_hog>;
> +
> +	imx6sll-lpddr3-arm2 {

This container node is not needed.

> +		pinctrl_hog: hoggrp {
> +			fsl,pins = <
> +				MX6SLL_PAD_LCD_DATA00__GPIO2_IO20	0x79
> +				MX6SLL_PAD_LCD_DATA01__GPIO2_IO21	0x79
> +				MX6SLL_PAD_LCD_DATA02__GPIO2_IO22	0x79
> +				MX6SLL_PAD_LCD_DATA03__GPIO2_IO23	0x79
> +				MX6SLL_PAD_LCD_DATA04__GPIO2_IO24	0x79
> +				MX6SLL_PAD_LCD_DATA05__GPIO2_IO25	0x79
> +				MX6SLL_PAD_LCD_DATA06__GPIO2_IO26	0x79
> +				MX6SLL_PAD_LCD_DATA07__GPIO2_IO27	0x79
> +				MX6SLL_PAD_LCD_DATA08__GPIO2_IO28	0x79
> +				MX6SLL_PAD_LCD_DATA09__GPIO2_IO29	0x79
> +				MX6SLL_PAD_LCD_DATA10__GPIO2_IO30	0x79
> +				MX6SLL_PAD_LCD_DATA11__GPIO2_IO31	0x79
> +				MX6SLL_PAD_LCD_DATA12__GPIO3_IO00	0x79
> +				MX6SLL_PAD_LCD_DATA13__GPIO3_IO01	0x79
> +				MX6SLL_PAD_LCD_DATA14__GPIO3_IO02	0x79
> +				MX6SLL_PAD_LCD_DATA15__GPIO3_IO03	0x79
> +				MX6SLL_PAD_LCD_DATA16__GPIO3_IO04	0x79
> +				MX6SLL_PAD_LCD_DATA17__GPIO3_IO05	0x79
> +				MX6SLL_PAD_LCD_DATA18__GPIO3_IO06	0x79
> +				MX6SLL_PAD_LCD_DATA19__GPIO3_IO07	0x79
> +				MX6SLL_PAD_LCD_DATA20__GPIO3_IO08	0x79
> +				MX6SLL_PAD_LCD_DATA21__GPIO3_IO09	0x79
> +				MX6SLL_PAD_LCD_DATA22__GPIO3_IO10	0x79
> +				MX6SLL_PAD_LCD_DATA23__GPIO3_IO11	0x79
> +				MX6SLL_PAD_LCD_CLK__GPIO2_IO15		0x79
> +				MX6SLL_PAD_LCD_ENABLE__GPIO2_IO16	0x79
> +				MX6SLL_PAD_LCD_HSYNC__GPIO2_IO17	0x79
> +				MX6SLL_PAD_LCD_VSYNC__GPIO2_IO18	0x79
> +				MX6SLL_PAD_LCD_RESET__GPIO2_IO19	0x79
> +				MX6SLL_PAD_KEY_COL3__GPIO3_IO30		0x79
> +				MX6SLL_PAD_KEY_ROW7__GPIO4_IO07		0x79
> +				MX6SLL_PAD_ECSPI2_MOSI__GPIO4_IO13	0x79
> +				MX6SLL_PAD_KEY_COL5__GPIO4_IO02		0x79
> +				MX6SLL_PAD_KEY_ROW6__GPIO4_IO05		0x79
> +			>;
> +		};
> +
> +		pinctrl_wifi_reset: wifi_reset_grp {
> +			fsl,pins = <
> +				MX6SLL_PAD_SD2_DATA7__GPIO5_IO00	0x10059		/* WIFI_RST */
> +			>;
> +		};
> +
> +		pinctrl_wifi_power: wifi_power_grp {

I guess you can have one pinctrl node to include both reset and power
pins?  Also, to be consistent with other pinctrl nodes on naming, the
node name should probably be wifigrp.

> +			fsl,pins = <
> +				MX6SLL_PAD_SD2_DATA6__GPIO4_IO29	0x10059		/* WIFI_3V3_ON */
> +			>;
> +		};
> +
> +		pinctrl_audmux3: audmux3grp {

Please keep pinctrl nodes sort alphabetically.

> +			fsl,pins = <
> +				MX6SLL_PAD_AUD_TXC__AUD3_TXC		0x4130b0
> +				MX6SLL_PAD_AUD_TXFS__AUD3_TXFS		0x4130b0
> +				MX6SLL_PAD_AUD_TXD__AUD3_TXD		0x4110b0
> +				MX6SLL_PAD_AUD_RXD__AUD3_RXD		0x4130b0
> +				MX6SLL_PAD_AUD_MCLK__AUDIO_CLK_OUT	0x4130b0
> +			>;
> +		};
> +
> +		pinctrl_led: ledgrp {
> +			fsl,pins = <
> +				MX6SLL_PAD_SD1_DATA6__GPIO5_IO07 0x17059
> +			>;
> +		};
> +
> +		pinctrl_uart1: uart1grp {
> +			fsl,pins = <
> +				MX6SLL_PAD_UART1_TXD__UART1_DCE_TX 0x1b0b1
> +				MX6SLL_PAD_UART1_RXD__UART1_DCE_RX 0x1b0b1
> +			>;
> +		};
> +
> +		pinctrl_usdhc2: usdhc2grp {
> +			fsl,pins = <
> +				MX6SLL_PAD_SD2_CMD__SD2_CMD		0x17059
> +				MX6SLL_PAD_SD2_CLK__SD2_CLK		0x13059
> +				MX6SLL_PAD_SD2_DATA0__SD2_DATA0		0x17059
> +				MX6SLL_PAD_SD2_DATA1__SD2_DATA1		0x17059
> +				MX6SLL_PAD_SD2_DATA2__SD2_DATA2		0x17059
> +				MX6SLL_PAD_SD2_DATA3__SD2_DATA3		0x17059
> +			>;
> +		};
> +
> +		pinctrl_usdhc2_100mhz: usdhc2grp_100mhz {

We generally use hyphen than underscore in node name.

Shawn

> +			fsl,pins = <
> +				MX6SLL_PAD_SD2_CMD__SD2_CMD		0x170b9
> +				MX6SLL_PAD_SD2_CLK__SD2_CLK		0x130b9
> +				MX6SLL_PAD_SD2_DATA0__SD2_DATA0		0x170b9
> +				MX6SLL_PAD_SD2_DATA1__SD2_DATA1		0x170b9
> +				MX6SLL_PAD_SD2_DATA2__SD2_DATA2		0x170b9
> +				MX6SLL_PAD_SD2_DATA3__SD2_DATA3		0x170b9
> +			>;
> +		};
> +
> +		pinctrl_usdhc2_200mhz: usdhc2grp_200mhz {
> +			fsl,pins = <
> +				MX6SLL_PAD_SD2_CMD__SD2_CMD		0x170f9
> +				MX6SLL_PAD_SD2_CLK__SD2_CLK		0x130f9
> +				MX6SLL_PAD_SD2_DATA0__SD2_DATA0		0x170f9
> +				MX6SLL_PAD_SD2_DATA1__SD2_DATA1		0x170f9
> +				MX6SLL_PAD_SD2_DATA2__SD2_DATA2		0x170f9
> +				MX6SLL_PAD_SD2_DATA3__SD2_DATA3		0x170f9
> +			>;
> +		};
> +
> +		pinctrl_usdhc2_sleep: usdhc2grp_sleep {
> +			fsl,pins = <
> +				MX6SLL_PAD_SD2_CMD__GPIO5_IO04		0x100f9
> +				MX6SLL_PAD_SD2_CLK__GPIO5_IO05		0x100f9
> +				MX6SLL_PAD_SD2_DATA0__GPIO5_IO01	0x100f9
> +				MX6SLL_PAD_SD2_DATA1__GPIO4_IO30	0x100f9
> +				MX6SLL_PAD_SD2_DATA2__GPIO5_IO03	0x100f9
> +				MX6SLL_PAD_SD2_DATA3__GPIO4_IO28	0x100f9
> +			>;
> +		};
> +
> +		pinctrl_usdhc3: usdhc3grp {
> +			fsl,pins = <
> +				MX6SLL_PAD_SD3_CMD__SD3_CMD	0x11059
> +				MX6SLL_PAD_SD3_CLK__SD3_CLK	0x11059
> +				MX6SLL_PAD_SD3_DATA0__SD3_DATA0	0x11059
> +				MX6SLL_PAD_SD3_DATA1__SD3_DATA1	0x11059
> +				MX6SLL_PAD_SD3_DATA2__SD3_DATA2	0x11059
> +				MX6SLL_PAD_SD3_DATA3__SD3_DATA3	0x11059
> +			>;
> +		};
> +
> +		pinctrl_usdhc3_100mhz: usdhc3grp_100mhz {
> +			fsl,pins = <
> +				MX6SLL_PAD_SD3_CMD__SD3_CMD	0x170b9
> +				MX6SLL_PAD_SD3_CLK__SD3_CLK	0x170b9
> +				MX6SLL_PAD_SD3_DATA0__SD3_DATA0	0x170b9
> +				MX6SLL_PAD_SD3_DATA1__SD3_DATA1	0x170b9
> +				MX6SLL_PAD_SD3_DATA2__SD3_DATA2	0x170b9
> +				MX6SLL_PAD_SD3_DATA3__SD3_DATA3	0x170b9
> +			>;
> +		};
> +
> +		pinctrl_usdhc3_200mhz: usdhc3grp_200mhz {
> +			fsl,pins = <
> +				MX6SLL_PAD_SD3_CMD__SD3_CMD	0x170f9
> +				MX6SLL_PAD_SD3_CLK__SD3_CLK	0x170f9
> +				MX6SLL_PAD_SD3_DATA0__SD3_DATA0	0x170f9
> +				MX6SLL_PAD_SD3_DATA1__SD3_DATA1	0x170f9
> +				MX6SLL_PAD_SD3_DATA2__SD3_DATA2	0x170f9
> +				MX6SLL_PAD_SD3_DATA3__SD3_DATA3	0x170f9
> +			>;
> +		};
> +
> +		pinctrl_usdhc3_sleep: usdhc3grp_sleep {
> +			fsl,pins = <
> +				MX6SLL_PAD_SD3_CMD__GPIO5_IO21	0x100c1
> +				MX6SLL_PAD_SD3_CLK__GPIO5_IO18	0x100c1
> +				MX6SLL_PAD_SD3_DATA0__GPIO5_IO19	0x100c1
> +				MX6SLL_PAD_SD3_DATA1__GPIO5_IO20	0x100c1
> +				MX6SLL_PAD_SD3_DATA2__GPIO5_IO16	0x100c1
> +				MX6SLL_PAD_SD3_DATA3__GPIO5_IO17	0x100c1
> +			>;
> +		};
> +
> +		pinctrl_usbotg1: usbotg1grp {
> +			fsl,pins = <
> +				MX6SLL_PAD_EPDC_PWR_COM__USB_OTG1_ID 0x17059
> +			>;
> +		};
> +
> +		pinctrl_i2c1: i2c1grp {
> +			fsl,pins = <
> +				MX6SLL_PAD_I2C1_SCL__I2C1_SCL	 0x4001f8b1
> +				MX6SLL_PAD_I2C1_SDA__I2C1_SDA	 0x4001f8b1
> +			>;
> +		};
> +
> +		pinctrl_i2c1_sleep: i2c1grp_sleep {
> +			fsl,pins = <
> +				MX6SLL_PAD_I2C1_SCL__I2C1_SCL	 0x400108b1
> +				MX6SLL_PAD_I2C1_SDA__I2C1_SDA	 0x400108b1
> +			>;
> +		};
> +
> +		pinctrl_i2c2: i2c2grp {
> +			fsl,pins = <
> +				MX6SLL_PAD_I2C2_SCL__I2C2_SCL	 0x4001f8b1
> +				MX6SLL_PAD_I2C2_SDA__I2C2_SDA	 0x4001f8b1
> +			>;
> +		};
> +
> +		pinctrl_i2c2_sleep: i2c2grp_sleep {
> +			fsl,pins = <
> +				MX6SLL_PAD_I2C2_SCL__I2C2_SCL	 0x400108b1
> +				MX6SLL_PAD_I2C2_SDA__I2C2_SDA	 0x400108b1
> +			>;
> +		};
> +
> +		pinctrl_i2c3: i2c3grp {
> +			fsl,pins = <
> +				MX6SLL_PAD_REF_CLK_24M__I2C3_SCL  0x4001f8b1
> +				MX6SLL_PAD_REF_CLK_32K__I2C3_SDA  0x4001f8b1
> +			>;
> +		};
> +
> +		pinctrl_ecspi1: ecspi1grp {
> +			fsl,pins = <
> +				MX6SLL_PAD_ECSPI1_MISO__ECSPI1_MISO	 0x100b1
> +				MX6SLL_PAD_ECSPI1_MOSI__ECSPI1_MOSI	 0x100b1
> +				MX6SLL_PAD_ECSPI1_SCLK__ECSPI1_SCLK	 0x100b1
> +				MX6SLL_PAD_ECSPI1_SS0__GPIO4_IO11	 0x100b1
> +			>;
> +		};
> +
> +		pinctrl_lm3630a_bl_gpio: lm3630a_bl_gpio_grp {
> +			fsl,pins = <
> +				MX6SLL_PAD_EPDC_PWR_CTRL3__GPIO2_IO10	0x10059 /* HWEN */
> +			>;
> +		};
> +
> +		pinctrl_ricoh_gpio: ricoh_gpio_grp {
> +			fsl,pins = <
> +				MX6SLL_PAD_SD1_CLK__GPIO5_IO15	0x1b8b1 /* ricoh619 chg */
> +				MX6SLL_PAD_SD1_DATA0__GPIO5_IO11 0x1b8b1 /* ricoh619 irq */
> +				MX6SLL_PAD_KEY_COL2__GPIO3_IO28	0x1b8b1 /* ricoh619 bat_low_int */
> +			>;
> +		};
> +
> +		pinctrl_gpio_keys: gpio_keys_grp {
> +			fsl,pins = <
> +				MX6SLL_PAD_SD1_DATA1__GPIO5_IO08	0x17059	/* PWR_SW */
> +				MX6SLL_PAD_SD1_DATA4__GPIO5_IO12	0x17059	/* HALL_EN */
> +			>;
> +		};
> +
> +	};
> +};
> +
> -- 
> 2.20.1
> 

Re: [PATCH v3 3/3] ARM: dts: imx: add devicetree for Kobo Clara HD

[Andreas Kemnade]

Hi,

On Fri, 25 Oct 2019 21:46:24 +0800
Shawn Guo <shawnguo@kernel.org> wrote:

[...]
> > +
> > +		pinctrl_wifi_reset: wifi_reset_grp {
> > +			fsl,pins = <
> > +				MX6SLL_PAD_SD2_DATA7__GPIO5_IO00	0x10059		/* WIFI_RST */
> > +			>;
> > +		};
> > +
> > +		pinctrl_wifi_power: wifi_power_grp {  
> 
> I guess you can have one pinctrl node to include both reset and power
> pins?  Also, to be consistent with other pinctrl nodes on naming, the
> node name should probably be wifigrp.
> 
well, the problems they are used in different nodes, so I cannot do
that:

       reg_wifi: regulator-wifi {
                compatible = "regulator-fixed";
                pinctrl-names = "default";
                pinctrl-0 = <&pinctrl_wifi_power>;
                regulator-name = "SD3_SPWR";
                regulator-min-microvolt = <3000000>;
                regulator-max-microvolt = <3000000>;
                gpio = <&gpio4 29 GPIO_ACTIVE_HIGH>;
                enable-active-high;
        };

        wifi_pwrseq: wifi_pwrseq {
                compatible = "mmc-pwrseq-simple";
                pinctrl-names = "default";
                pinctrl-0 = <&pinctrl_wifi_reset>;
                post-power-on-delay-ms = <20>;
                reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
        };

So having them combined breaks the mux where you use it rule.
I got in earlier mails:

> > +	wifi_pwrseq: wifi_pwrseq {
> > +		compatible = "mmc-pwrseq-simple";
> > +		post-power-on-delay-ms = <20>;
> > +		reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;  

> Can you add a pinctrl-entry here please? The general rule is to mux
> things where you use it
[...]
> > +			compatible = "regulator-fixed";
> > +			regulator-name = "SD3_SPWR";
> > +			regulator-min-microvolt = <3000000>;
> > +			regulator-max-microvolt = <3000000>;
> > +
> > +			gpio = <&gpio4 29 GPIO_ACTIVE_HIGH>;  

> Please add a pinctrl here to mux this gpio.

Regards,
Andreas

Re: [PATCH v3 3/3] ARM: dts: imx: add devicetree for Kobo Clara HD

[Shawn Guo]

On Fri, Oct 25, 2019 at 08:07:43PM +0200, Andreas Kemnade wrote:
> Hi,
> 
> On Fri, 25 Oct 2019 21:46:24 +0800
> Shawn Guo <shawnguo@kernel.org> wrote:
> 
> [...]
> > > +
> > > +		pinctrl_wifi_reset: wifi_reset_grp {
> > > +			fsl,pins = <
> > > +				MX6SLL_PAD_SD2_DATA7__GPIO5_IO00	0x10059		/* WIFI_RST */
> > > +			>;
> > > +		};
> > > +
> > > +		pinctrl_wifi_power: wifi_power_grp {  
> > 
> > I guess you can have one pinctrl node to include both reset and power
> > pins?  Also, to be consistent with other pinctrl nodes on naming, the
> > node name should probably be wifigrp.
> > 
> well, the problems they are used in different nodes, so I cannot do
> that:
> 
>        reg_wifi: regulator-wifi {
>                 compatible = "regulator-fixed";
>                 pinctrl-names = "default";
>                 pinctrl-0 = <&pinctrl_wifi_power>;
>                 regulator-name = "SD3_SPWR";
>                 regulator-min-microvolt = <3000000>;
>                 regulator-max-microvolt = <3000000>;
>                 gpio = <&gpio4 29 GPIO_ACTIVE_HIGH>;
>                 enable-active-high;
>         };
> 
>         wifi_pwrseq: wifi_pwrseq {
>                 compatible = "mmc-pwrseq-simple";
>                 pinctrl-names = "default";
>                 pinctrl-0 = <&pinctrl_wifi_reset>;
>                 post-power-on-delay-ms = <20>;
>                 reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
>         };

Ah, yes, it makes more sense.  I missed that.

Shawn

> 
> So having them combined breaks the mux where you use it rule.
> I got in earlier mails:
> 
> > > +	wifi_pwrseq: wifi_pwrseq {
> > > +		compatible = "mmc-pwrseq-simple";
> > > +		post-power-on-delay-ms = <20>;
> > > +		reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;  
> 
> > Can you add a pinctrl-entry here please? The general rule is to mux
> > things where you use it
> [...]
> > > +			compatible = "regulator-fixed";
> > > +			regulator-name = "SD3_SPWR";
> > > +			regulator-min-microvolt = <3000000>;
> > > +			regulator-max-microvolt = <3000000>;
> > > +
> > > +			gpio = <&gpio4 29 GPIO_ACTIVE_HIGH>;  
> 
> > Please add a pinctrl here to mux this gpio.
> 
> Regards,
> Andreas