Re: [PATCH 2/5] ARM: dts: imx50: Add Kobo Aura DTS

From: Shawn Guo
Date: Thu Mar 21 2019 - 21:32:39 EST

On Tue, Mar 19, 2019 at 04:24:17PM +0100, Jonathan Neuschäfer wrote:
> The Kobo Aura is an e-book reader released in 2013.
>
> With the devicetree in its current state, the kernel will boot and run
> for about ten seconds. To solve this, the embedded controller needs to
> be told that the system should stay powered on. This will be done in a
> later patchset.
>
> - The IOMUXC mode bits for the SD interfaces were taken from the
> vendor's U-Boot fork.
> - The bus width of the eMMC is 4 bits in the vendor kernel, but I
> achieved better performance with 8 bits.
> - The SDIO clock frequency for the WiFi chip is 25MHz in the vendor
> kernel, but the WiFi chip (BCM43362) supports 50MHz, which works
> reliably on this board and gives slightly better performance.
> - The I2C pins' IOMUXC settings come from the vendor's U-Boot fork.
>
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@xxxxxxx>
> ---
> arch/arm/boot/dts/Makefile | 3 +-
> arch/arm/boot/dts/imx50-kobo-aura.dts | 245 ++++++++++++++++++++++++++
> 2 files changed, 247 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm/boot/dts/imx50-kobo-aura.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index f4f5aeaf3298..0c85156c552b 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -363,7 +363,8 @@ dtb-$(CONFIG_SOC_IMX35) += \
> imx35-eukrea-mbimxsd35-baseboard.dtb \
> imx35-pdk.dtb
> dtb-$(CONFIG_SOC_IMX50) += \
> - imx50-evk.dtb
> + imx50-evk.dtb \
> + imx50-kobo-aura.dtb
> dtb-$(CONFIG_SOC_IMX51) += \
> imx51-apf51.dtb \
> imx51-apf51dev.dtb \
> diff --git a/arch/arm/boot/dts/imx50-kobo-aura.dts b/arch/arm/boot/dts/imx50-kobo-aura.dts
> new file mode 100644
> index 000000000000..e778a677c752
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx50-kobo-aura.dts
> @@ -0,0 +1,245 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2019 Jonathan Neuschäfer
> +//
> +// The Kobo Aura e-book reader, model N514. The mainboard is marked as E606F0B.
> +
> +/dts-v1/;
> +#include "imx50.dtsi"
> +#include <dt-bindings/input/input.h>
> +
> +/ {
> + model = "Kobo Aura (N514)";
> + compatible = "kobo,aura", "fsl,imx50";

Board compatible should also be documented.

> +
> + chosen {
> + stdout-path = "serial1:115200n8";
> + };
> +
> + memory@70000000 {
> + device_type = "memory";
> + reg = <0x70000000 0x10000000>;
> + };
> +
> + gpio-leds {
> + compatible = "gpio-leds";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_leds>;
> +
> + on {
> + label = "kobo_aura:orange:on";
> + gpios = <&gpio6 24 GPIO_ACTIVE_LOW>;
> + panic-indicator;
> + };
> + };
> +
> + gpio-keys {
> + compatible = "gpio-keys";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_gpiokeys>;
> +
> + power {
> + label = "Power Button";
> + gpios = <&gpio4 10 GPIO_ACTIVE_LOW>;
> + linux,code = <KEY_POWER>;
> + };
> +
> + hallsensor {
> + label = "Hallsensor";
> + gpios = <&gpio5 15 GPIO_ACTIVE_LOW>;
> + linux,code = <0>;

Use define KEY_RESERVED?

> + linux,input-type = <5>;

Use define EV_SW?

> + };
> +
> + frontlight {
> + label = "Frontlight";
> + gpios = <&gpio4 1 GPIO_ACTIVE_LOW>;
> + linux,code = <KEY_DISPLAYTOGGLE>;
> + };
> + };
> +
> + sd2_pwrseq: pwrseq {
> + compatible = "mmc-pwrseq-simple";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_sd2_reset>;
> +

Please do not have random newlines.

> + reset-gpios = <&gpio4 17 GPIO_ACTIVE_LOW>;
> + };
> +
> + sd2_vmmc: gpio-regulator {
> + compatible = "regulator-gpio";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_sd2_vmmc>;
> +
> + regulator-name = "vmmc";
> + states = <3300000 0>;
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + enable-gpio = <&gpio4 12 GPIO_ACTIVE_LOW>;
> + startup-delay-us = <100000>;
> + };
> +};
> +
> +&iomuxc {
> + pinctrl_uart2: uart2 {
> + fsl,pins = <
> + MX50_PAD_UART2_TXD__UART2_TXD_MUX 0x1e4
> + MX50_PAD_UART2_RXD__UART2_RXD_MUX 0x1e4
> + >;
> + };
> +
> + pinctrl_i2c1: i2c1 {

Please sort these pinctrl nodes alphabetically.

> + fsl,pins = <
> + MX50_PAD_I2C1_SCL__I2C1_SCL 0x400001fd
> + MX50_PAD_I2C1_SDA__I2C1_SDA 0x400001fd
> + >;
> + };
> +
> + pinctrl_i2c2: i2c2 {
> + fsl,pins = <
> + MX50_PAD_I2C2_SCL__I2C2_SCL 0x400001fd
> + MX50_PAD_I2C2_SDA__I2C2_SDA 0x400001fd
> + >;
> + };
> +
> + pinctrl_i2c3: i2c3 {
> + fsl,pins = <
> + MX50_PAD_I2C3_SCL__I2C3_SCL 0x400001fd
> + MX50_PAD_I2C3_SDA__I2C3_SDA 0x400001fd
> + >;
> + };
> +
> + pinctrl_leds: leds {
> + fsl,pins = <
> + MX50_PAD_PWM1__GPIO6_24 0x0
> + >;
> + };
> +
> + pinctrl_gpiokeys: gpiokeys {
> + fsl,pins = <
> + MX50_PAD_CSPI_MISO__GPIO4_10 0x0
> + MX50_PAD_SD2_D7__GPIO5_15 0x0
> + MX50_PAD_KEY_ROW0__GPIO4_1 0x0
> + >;
> + };
> +
> + pinctrl_sd1: sd1 {
> + fsl,pins = <
> + MX50_PAD_SD1_CMD__ESDHC1_CMD 0x1e4
> + MX50_PAD_SD1_CLK__ESDHC1_CLK 0xd4
> + MX50_PAD_SD1_D0__ESDHC1_DAT0 0x1d4
> + MX50_PAD_SD1_D1__ESDHC1_DAT1 0x1d4
> + MX50_PAD_SD1_D2__ESDHC1_DAT2 0x1d4
> + MX50_PAD_SD1_D3__ESDHC1_DAT3 0x1d4
> +
> + MX50_PAD_SD2_CD__GPIO5_17 0x0
> + >;
> + };
> +
> + pinctrl_sd2: sd2 {
> + fsl,pins = <
> + MX50_PAD_SD2_CMD__ESDHC2_CMD 0x1e4
> + MX50_PAD_SD2_CLK__ESDHC2_CLK 0xd4
> + MX50_PAD_SD2_D0__ESDHC2_DAT0 0x1d4
> + MX50_PAD_SD2_D1__ESDHC2_DAT1 0x1d4
> + MX50_PAD_SD2_D2__ESDHC2_DAT2 0x1d4
> + MX50_PAD_SD2_D3__ESDHC2_DAT3 0x1d4
> + >;
> + };
> +
> + pinctrl_sd2_reset: sd2-reset {
> + fsl,pins = <
> + MX50_PAD_ECSPI2_MOSI__GPIO4_17 0x0
> + >;
> + };
> +
> + pinctrl_sd2_vmmc: sd2-vmmc {
> + fsl,pins = <
> + MX50_PAD_ECSPI1_SCLK__GPIO4_12 0x0
> + >;
> + };
> +
> + pinctrl_sd3: sd3 {
> + fsl,pins = <
> + MX50_PAD_SD3_CMD__ESDHC3_CMD 0x1e4
> + MX50_PAD_SD3_CLK__ESDHC3_CLK 0xd4
> + MX50_PAD_SD3_D0__ESDHC3_DAT0 0x1d4
> + MX50_PAD_SD3_D1__ESDHC3_DAT1 0x1d4
> + MX50_PAD_SD3_D2__ESDHC3_DAT2 0x1d4
> + MX50_PAD_SD3_D3__ESDHC3_DAT3 0x1d4
> + MX50_PAD_SD3_D4__ESDHC3_DAT4 0x1d4
> + MX50_PAD_SD3_D5__ESDHC3_DAT5 0x1d4
> + MX50_PAD_SD3_D6__ESDHC3_DAT6 0x1d4
> + MX50_PAD_SD3_D7__ESDHC3_DAT7 0x1d4
> + >;
> + };
> +};
> +
> +&uart2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_uart2>;
> + status = "okay";
> +};
> +
> +&i2c1 {

Please sort these labeled nodes alphabetically.

> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_i2c1>;
> + status = "okay";
> +
> + /* TODO: ektf2132 touch controller at 0x15 */
> +};
> +
> +&i2c2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_i2c2>;
> + status = "okay";
> +
> + /* TODO: TPS65185 PMIC for E Ink at 0x68 */
> +};
> +
> +&i2c3 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_i2c3>;
> + status = "okay";
> +
> + /* TODO: embedded controller at 0x43 */
> +};
> +
> +&esdhc1 {
> + status = "okay";

We usually end property list with 'status'.

Shawn

> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_sd1>;
> +
> + max-frequency = <50000000>;
> + bus-width = <4>;
> + cd-gpios = <&gpio5 17 GPIO_ACTIVE_LOW>;
> + disable-wp;
> +
> + /* External µSD card */
> +};
> +
> +&esdhc2 {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_sd2>;
> +
> + bus-width = <4>;
> + max-frequency = <50000000>;
> + disable-wp;
> + mmc-pwrseq = <&sd2_pwrseq>;
> + vmmc-supply = <&sd2_vmmc>;
> +
> + /* CyberTan WC121 SDIO WiFi (BCM43362) */
> +};
> +
> +&esdhc3 {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_sd3>;
> +
> + bus-width = <8>;
> + non-removable;
> + max-frequency = <50000000>;
> + disable-wp;
> +
> + /* Internal eMMC */
> +};
> --
> 2.20.1
>