diff mbox series

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

Message ID 20191010192357.27884-3-andreas@kemnade.info (mailing list archive)
State New, archived
Headers show
Series dts: ARM: add Kobo Clara HD eBook reader | expand

Commit Message

Andreas Kemnade Oct. 10, 2019, 7:23 p.m. UTC
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

Comments

Marco Felsch Oct. 11, 2019, 6:56 a.m. UTC | #1
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
>
Andreas Kemnade Oct. 11, 2019, 7:41 a.m. UTC | #2
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
Rob Herring (Arm) Oct. 11, 2019, 2:29 p.m. UTC | #3
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
Andreas Kemnade Oct. 11, 2019, 3:05 p.m. UTC | #4
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
Marco Felsch Oct. 11, 2019, 3:22 p.m. UTC | #5
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
Andreas Kemnade Oct. 11, 2019, 4:19 p.m. UTC | #6
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
Marco Felsch Oct. 11, 2019, 4:56 p.m. UTC | #7
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
Andreas Kemnade Oct. 13, 2019, 3:56 p.m. UTC | #8
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
Shawn Guo Oct. 25, 2019, 9:14 a.m. UTC | #9
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
Andreas Kemnade Oct. 25, 2019, 10:09 a.m. UTC | #10
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
Shawn Guo Oct. 25, 2019, 1:06 p.m. UTC | #11
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
>
diff mbox series

Patch

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