* [PATCH 0/3] dts: ARM: add Kobo Clara HD eBook reader
@ 2019-09-27 6:14 Andreas Kemnade
2019-09-27 6:14 ` [PATCH 1/3] ARM: dts: add Netronix E60K02 board common file Andreas Kemnade
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Andreas Kemnade @ 2019-09-27 6:14 UTC (permalink / raw)
To: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam,
linux-imx, manivannan.sadhasivam, andrew.smirnov, marex, angus,
devicetree, linux-kernel, linux-arm-kernel, j.neuschaefer,
Discussions about the Letux Kernel
Cc: 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
Andreas Kemnade (3):
ARM: dts: add Netronix E60K02 board common file
dt-bindings: arm: fsl: add compatible string for Kobo Clara HD
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 | 339 ++++++++++++++++++
arch/arm/boot/dts/imx6sll-kobo-clarahd.dts | 275 ++++++++++++++
4 files changed, 617 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] ARM: dts: add Netronix E60K02 board common file
2019-09-27 6:14 [PATCH 0/3] dts: ARM: add Kobo Clara HD eBook reader Andreas Kemnade
@ 2019-09-27 6:14 ` Andreas Kemnade
2019-09-27 9:47 ` Marco Felsch
2019-09-27 6:14 ` [PATCH 2/3] dt-bindings: arm: fsl: add compatible string for Kobo Clara HD Andreas Kemnade
2019-09-27 6:14 ` [PATCH 3/3] ARM: dts: imx: add devicetree " Andreas Kemnade
2 siblings, 1 reply; 9+ messages in thread
From: Andreas Kemnade @ 2019-09-27 6:14 UTC (permalink / raw)
To: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam,
linux-imx, manivannan.sadhasivam, andrew.smirnov, marex, angus,
devicetree, linux-kernel, linux-arm-kernel, j.neuschaefer,
Discussions about the Letux Kernel
Cc: 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.
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>
---
backligt dependencies:
module autoloading:
https://patchwork.kernel.org/patch/11139987/
enable-gpios property:
https://patchwork.kernel.org/patch/11143795/
arch/arm/boot/dts/e60k02.dtsi | 339 ++++++++++++++++++++++++++++++++++
1 file changed, 339 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 000000000000..c4fa8e314e2e
--- /dev/null
+++ b/arch/arm/boot/dts/e60k02.dtsi
@@ -0,0 +1,339 @@
+// 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)
+ */
+
+/ {
+
+ memory {
+ reg = <0x80000000 0x80000000>;
+ };
+
+ chosen {
+ stdout-path = &uart1;
+ };
+
+ wifi_pwrseq: wifi_pwrseq {
+ compatible = "mmc-pwrseq-simple";
+ post-power-on-delay-ms = <20>;
+ reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
+ };
+
+ regulators {
+ compatible = "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ reg_sd3_vmmc: wifi_regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "SD3_SPWR";
+ regulator-min-microvolt = <3000000>;
+ regulator-max-microvolt = <3000000>;
+
+ gpio = <&gpio4 29 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+
+ };
+ };
+
+ leds {
+ compatible = "gpio-leds";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_led>;
+
+ GLED {
+ gpios = <&gpio5 7 GPIO_ACTIVE_LOW>;
+ linux,default-trigger = "timer";
+ };
+ };
+
+ 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 = <0x05>; /* EV_SW */
+ gpio-key,wakeup;
+ };
+ };
+
+};
+
+
+
+&audmux {
+ pinctrl-names = "default";
+ status = "disabled";
+};
+
+&snvs_rtc {
+ status = "disabled";
+};
+
+&i2c1 {
+ clock-frequency = <100000>;
+ pinctrl-names = "default","sleep";
+ pinctrl-0 = <&pinctrl_i2c1 &pinctrl_lm3630a_bl_gpio>;
+ pinctrl-1 = <&pinctrl_i2c1_sleep>;
+ status = "okay";
+
+ lm3630a: lm3630a-i2c@36 {
+ reg = <0x36>;
+ status = "ok";
+
+ compatible = "ti,lm3630a";
+ enable-gpios = <&gpio2 10 0>;
+
+ #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>;
+ };
+
+ };
+};
+
+&i2c3 {
+ clock-frequency = <100000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i2c3 &pinctrl_ricoh_gpio>;
+ status = "okay";
+
+ ricoh619: ricoh619-i2c@32 {
+ compatible = "ricoh,rc5t619";
+ reg = <0x32>;
+
+ system-power-controller;
+ gpios = <&gpio5 11 GPIO_ACTIVE_LOW>;
+ gpio_chg = <&gpio5 15 GPIO_ACTIVE_LOW>;
+ gpio_bat_low = <&gpio3 28 GPIO_ACTIVE_LOW>;
+ };
+
+};
+
+&ricoh619
+{
+ compatible = "ricoh,rc5t619";
+
+ regulators {
+ ricoh619_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-microvolt = <900000>;
+ };
+ };
+
+ /* Core3_3V3 */
+ ricoh619_dcdc2_reg: DCDC2 {
+ regulator-name = "DCDC2";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ regulator-suspend-microvolt = <3300000>;
+ };
+ };
+
+ ricoh619_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-microvolt = <1140000>;
+ };
+ };
+
+ /* Core4_1V2 */
+ ricoh619_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-microvolt = <1140000>;
+ };
+ };
+
+ /* Core4_1V8 */
+ ricoh619_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-microvolt = <1700000>;
+ };
+ };
+
+ /* IR_3V3 */
+ ricoh619_ldo1_reg: LDO1 {
+ regulator-name = "LDO1";
+ //regulator-always-on;
+ regulator-boot-on;
+ };
+
+ /* Core1_3V3 */
+ ricoh619_ldo2_reg: LDO2 {
+ regulator-name = "LDO2";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ regulator-suspend-microvolt = <3000000>;
+ };
+ };
+
+ /* Core5_1V2 */
+ ricoh619_ldo3_reg: LDO3 {
+ regulator-name = "LDO3";
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ ricoh619_ldo4_reg: LDO4 {
+ regulator-name = "LDO4";
+ regulator-boot-on;
+ };
+
+ /* SPD_3V3 */
+ ricoh619_ldo5_reg: LDO5 {
+ regulator-name = "LDO5";
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ /* DDR_0V6 */
+ ricoh619_ldo6_reg: LDO6 {
+ regulator-name = "LDO6";
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ /* VDD_PWM */
+ ricoh619_ldo7_reg: LDO7 {
+ regulator-name = "LDO7";
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ /* ldo_1v8 */
+ ricoh619_ldo8_reg: LDO8 {
+ regulator-name = "LDO8";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ ricoh619_ldo9_reg: LDO9 {
+ regulator-name = "LDO9";
+ regulator-boot-on;
+ };
+
+ ricoh619_ldo10_reg: LDO10 {
+ regulator-name = "LDO10";
+ regulator-boot-on;
+ };
+
+ ricoh619_ldortc1_reg: LDORTC1 {
+ regulator-name = "LDORTC1";
+ regulator-boot-on;
+ };
+
+ ricoh619_ldortc2_reg: LDORTC2 {
+ regulator-name = "LDORTC2";
+ regulator-boot-on;
+ };
+ };
+};
+
+&uart1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart1>;
+ status = "okay";
+};
+
+&usdhc1 {
+ status = "disabled";
+};
+
+&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_usdhc3_pwr>;
+ pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
+ pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
+ pinctrl-3 = <&pinctrl_usdhc3_sleep>, <&pinctrl_usdhc3_pwr>;
+ vmmc-supply = <®_sd3_vmmc>;
+ 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";
+};
+
+
+&ssi2 {
+ status = "disabled";
+};
+
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] dt-bindings: arm: fsl: add compatible string for Kobo Clara HD
2019-09-27 6:14 [PATCH 0/3] dts: ARM: add Kobo Clara HD eBook reader Andreas Kemnade
2019-09-27 6:14 ` [PATCH 1/3] ARM: dts: add Netronix E60K02 board common file Andreas Kemnade
@ 2019-09-27 6:14 ` Andreas Kemnade
2019-09-27 9:19 ` Marco Felsch
2019-09-27 6:14 ` [PATCH 3/3] ARM: dts: imx: add devicetree " Andreas Kemnade
2 siblings, 1 reply; 9+ messages in thread
From: Andreas Kemnade @ 2019-09-27 6:14 UTC (permalink / raw)
To: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam,
linux-imx, manivannan.sadhasivam, andrew.smirnov, marex, angus,
devicetree, linux-kernel, linux-arm-kernel, j.neuschaefer,
Discussions about the Letux Kernel
Cc: Andreas Kemnade
This adds a compatible string fro the Kobo Clara HD eBook reader.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
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 7294ac36f4c0..afa3bfeca0c0 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] ARM: dts: imx: add devicetree for Kobo Clara HD
2019-09-27 6:14 [PATCH 0/3] dts: ARM: add Kobo Clara HD eBook reader Andreas Kemnade
2019-09-27 6:14 ` [PATCH 1/3] ARM: dts: add Netronix E60K02 board common file Andreas Kemnade
2019-09-27 6:14 ` [PATCH 2/3] dt-bindings: arm: fsl: add compatible string for Kobo Clara HD Andreas Kemnade
@ 2019-09-27 6:14 ` Andreas Kemnade
2 siblings, 0 replies; 9+ messages in thread
From: Andreas Kemnade @ 2019-09-27 6:14 UTC (permalink / raw)
To: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam,
linux-imx, manivannan.sadhasivam, andrew.smirnov, marex, angus,
devicetree, linux-kernel, linux-arm-kernel, j.neuschaefer,
Discussions about the Letux Kernel
Cc: 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 | 275 +++++++++++++++++++++
2 files changed, 277 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 9159fa2cea90..a8a235c74c37 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 000000000000..f5ca44171d7c
--- /dev/null
+++ b/arch/arm/boot/dts/imx6sll-kobo-clarahd.dts
@@ -0,0 +1,275 @@
+// 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 = <&ricoh619_dcdc3_reg>;
+ soc-supply = <&ricoh619_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_usdhc3_pwr: usdhc3_pwr_grp {
+ fsl,pins = <
+ MX6SLL_PAD_SD2_DATA7__GPIO5_IO00 0x10059 /* WIFI_RST */
+ MX6SLL_PAD_SD2_DATA5__GPIO4_IO31 0x10059 /* WIFI_INT */
+ 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] dt-bindings: arm: fsl: add compatible string for Kobo Clara HD
2019-09-27 6:14 ` [PATCH 2/3] dt-bindings: arm: fsl: add compatible string for Kobo Clara HD Andreas Kemnade
@ 2019-09-27 9:19 ` Marco Felsch
0 siblings, 0 replies; 9+ messages in thread
From: Marco Felsch @ 2019-09-27 9:19 UTC (permalink / raw)
To: Andreas Kemnade
Cc: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam,
linux-imx, manivannan.sadhasivam, andrew.smirnov, marex, angus,
devicetree, linux-kernel, linux-arm-kernel, j.neuschaefer,
Discussions about the Letux Kernel
Hi Andreas,
On 19-09-27 08:14, Andreas Kemnade wrote:
> This adds a compatible string fro the Kobo Clara HD eBook reader.
>
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
Just a nitpick, this should be patch number 1.
Regards,
Marco
> 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 7294ac36f4c0..afa3bfeca0c0 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
>
>
>
--
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 |
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] ARM: dts: add Netronix E60K02 board common file
2019-09-27 6:14 ` [PATCH 1/3] ARM: dts: add Netronix E60K02 board common file Andreas Kemnade
@ 2019-09-27 9:47 ` Marco Felsch
2019-09-27 19:08 ` Andreas Kemnade
0 siblings, 1 reply; 9+ messages in thread
From: Marco Felsch @ 2019-09-27 9:47 UTC (permalink / raw)
To: Andreas Kemnade
Cc: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam,
linux-imx, manivannan.sadhasivam, andrew.smirnov, marex, angus,
devicetree, linux-kernel, linux-arm-kernel, j.neuschaefer,
Discussions about the Letux Kernel
Hi Andreas,
thanks for the patch.
On 19-09-27 08:14, 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.
>
> 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>
> ---
> backligt dependencies:
> module autoloading:
> https://patchwork.kernel.org/patch/11139987/
> enable-gpios property:
> https://patchwork.kernel.org/patch/11143795/
>
> arch/arm/boot/dts/e60k02.dtsi | 339 ++++++++++++++++++++++++++++++++++
> 1 file changed, 339 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 000000000000..c4fa8e314e2e
> --- /dev/null
> +++ b/arch/arm/boot/dts/e60k02.dtsi
> @@ -0,0 +1,339 @@
> +// 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)
> + */
> +
> +/ {
> +
> + memory {
> + reg = <0x80000000 0x80000000>;
> + };
> +
> + chosen {
> + stdout-path = &uart1;
> + };
> +
> + 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.
> + };
Please reorder the whole dt alphabetical.
> +
> + regulators {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <0>;
Drop the regultors { } container especially if we only have a single
regulator.
> +
> + reg_sd3_vmmc: wifi_regulator {
Either use:
reg_sd3_vmmc: regulator-sd3-vmmc
or
reg_wifi: regulator-wifi
> + 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.
> + enable-active-high;
> +
> + };
> + };
> +
> + leds {
> + compatible = "gpio-leds";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_led>;
Please move all muxing you made here into this file or add phandles so
the dts file need to add only the muxing stuff. This applies to all
pinctrl you made here.
> +
> + GLED {
> + gpios = <&gpio5 7 GPIO_ACTIVE_LOW>;
> + linux,default-trigger = "timer";
> + };
> + };
> +
> + 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>;
Add missing header: dt-bindings/input/input.h to use this.
> + gpio-key,wakeup;
> + };
> + cover {
> + label = "Cover";
> + gpios = <&gpio5 12 GPIO_ACTIVE_LOW>;
> + linux,code = <SW_LID>;
> + linux,input-type = <0x05>; /* EV_SW */
In the header above EV_SW is also specified so please use it here.
> + gpio-key,wakeup;
> + };
> + };
> +
> +};
> +
> +
> +
Whitespaces
> +&audmux {
> + pinctrl-names = "default";
> + status = "disabled";
Why you mentioned a pinctrl-names here without the mux? Do we need the
status line here? The common case is that such devices are off by
default/the base dt.
> +};
> +
> +&snvs_rtc {
> + status = "disabled";
Same applies here.
> +};
> +
> +&i2c1 {
> + clock-frequency = <100000>;
> + pinctrl-names = "default","sleep";
> + pinctrl-0 = <&pinctrl_i2c1 &pinctrl_lm3630a_bl_gpio>;
The &pinctrl_lm3630a_bl_gpio should be moved into the lm3630a node.
> + pinctrl-1 = <&pinctrl_i2c1_sleep>;
> + status = "okay";
> +
> + lm3630a: lm3630a-i2c@36 {
please name it backlight@36
> + reg = <0x36>;
> + status = "ok";
status lines are always be the last and if it is okay you can drop it
because the default is okay.
> +
> + compatible = "ti,lm3630a";
> + enable-gpios = <&gpio2 10 0>;
Please use 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>;
> + };
> +
> + };
> +};
> +
> +&i2c3 {
> + clock-frequency = <100000>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_i2c3 &pinctrl_ricoh_gpio>;
Device mux goes into the device.
> + status = "okay";
> +
> + ricoh619: ricoh619-i2c@32 {
Please name it pmic@32
> + compatible = "ricoh,rc5t619";
> + reg = <0x32>;
> +
> + system-power-controller;
> + gpios = <&gpio5 11 GPIO_ACTIVE_LOW>;
> + gpio_chg = <&gpio5 15 GPIO_ACTIVE_LOW>;
> + gpio_bat_low = <&gpio3 28 GPIO_ACTIVE_LOW>;
> + };
> +
> +};
> +
> +&ricoh619
> +{
Nope. The whole bunch should be moved to the place above where add the
device.
> + compatible = "ricoh,rc5t619";
> +
> + regulators {
> + ricoh619_dcdc1_reg: DCDC1 {
please drop the ricoh619_ prefix.
> + 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-microvolt = <900000>;
regulator-suspend-microvolt is deprecated. Please use
regulator-suspend-min-microvolt and regulator-suspend-max-microvolt.
> + };
> + };
> +
> + /* Core3_3V3 */
> + ricoh619_dcdc2_reg: DCDC2 {
> + regulator-name = "DCDC2";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + regulator-suspend-microvolt = <3300000>;
> + };
> + };
> +
> + ricoh619_dcdc3_reg: DCDC3 {
> + regulator-name = "DCDC3";
> + regulator-min-microvolt = <300000>;
> + regulator-max-microvolt = <1875000>;
> + regulator-always-on;//
Remove //
> + regulator-boot-on;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + regulator-suspend-microvolt = <1140000>;
> + };
> + };
> +
> + /* Core4_1V2 */
> + ricoh619_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-microvolt = <1140000>;
> + };
> + };
> +
> + /* Core4_1V8 */
> + ricoh619_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-microvolt = <1700000>;
> + };
> + };
> +
> + /* IR_3V3 */
> + ricoh619_ldo1_reg: LDO1 {
> + regulator-name = "LDO1";
> + //regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + /* Core1_3V3 */
> + ricoh619_ldo2_reg: LDO2 {
> + regulator-name = "LDO2";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + regulator-suspend-microvolt = <3000000>;
> + };
> + };
> +
> + /* Core5_1V2 */
> + ricoh619_ldo3_reg: LDO3 {
> + regulator-name = "LDO3";
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + ricoh619_ldo4_reg: LDO4 {
> + regulator-name = "LDO4";
> + regulator-boot-on;
> + };
> +
> + /* SPD_3V3 */
> + ricoh619_ldo5_reg: LDO5 {
> + regulator-name = "LDO5";
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + /* DDR_0V6 */
> + ricoh619_ldo6_reg: LDO6 {
> + regulator-name = "LDO6";
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + /* VDD_PWM */
> + ricoh619_ldo7_reg: LDO7 {
> + regulator-name = "LDO7";
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + /* ldo_1v8 */
> + ricoh619_ldo8_reg: LDO8 {
> + regulator-name = "LDO8";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + ricoh619_ldo9_reg: LDO9 {
> + regulator-name = "LDO9";
> + regulator-boot-on;
> + };
> +
> + ricoh619_ldo10_reg: LDO10 {
> + regulator-name = "LDO10";
> + regulator-boot-on;
> + };
> +
> + ricoh619_ldortc1_reg: LDORTC1 {
> + regulator-name = "LDORTC1";
> + regulator-boot-on;
> + };
> +
> + ricoh619_ldortc2_reg: LDORTC2 {
> + regulator-name = "LDORTC2";
> + regulator-boot-on;
> + };
> + };
> +};
> +
> +&uart1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_uart1>;
> + status = "okay";
> +};
> +
> +&usdhc1 {
> + status = "disabled";
> +};
Can be dropped
> +
> +&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_usdhc3_pwr>;
> + pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
> + pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
> + pinctrl-3 = <&pinctrl_usdhc3_sleep>, <&pinctrl_usdhc3_pwr>;
> + vmmc-supply = <®_sd3_vmmc>;
> + 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";
> +};
> +
> +
> +&ssi2 {
> + status = "disabled";
> +};
Can be dropped.
Thanks for your patch.
Regards,
Marco
> +
> --
> 2.20.1
>
>
>
--
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 |
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] ARM: dts: add Netronix E60K02 board common file
2019-09-27 9:47 ` Marco Felsch
@ 2019-09-27 19:08 ` Andreas Kemnade
2019-09-30 8:27 ` Marco Felsch
0 siblings, 1 reply; 9+ messages in thread
From: Andreas Kemnade @ 2019-09-27 19:08 UTC (permalink / raw)
To: Marco Felsch
Cc: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam,
linux-imx, manivannan.sadhasivam, andrew.smirnov, marex, angus,
devicetree, linux-kernel, linux-arm-kernel, j.neuschaefer,
Discussions about the Letux Kernel
Hi Marco,
On Fri, 27 Sep 2019 11:47:21 +0200
Marco Felsch <m.felsch@pengutronix.de> wrote:
> Hi Andreas,
>
> thanks for the patch.
>
thanks for the quick review. Most of your comments are clear.
[...]
> > + 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.
>
yes, there are many places in my patch where they are added to some
parent devices.
I will fix that.
[...]
> > + leds {
> > + compatible = "gpio-leds";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_led>;
>
> Please move all muxing you made here into this file or add phandles so
> the dts file need to add only the muxing stuff. This applies to all
> pinctrl you made here.
>
so you disagree with this pattern:
in .dtsi
some_device {
pinctrl-0 = <&pinctrl_some_device>;
};
and in .dts (one I sent with this patch series and the tolino/mx6sl one
is not ready-cooked yet, will be part of a later series)
&iomuxc {
pinctrl_some_device: some_devicegrp {
fsl,pins = <...>;
};
};
?
> > +
> > + GLED {
> > + gpios = <&gpio5 7 GPIO_ACTIVE_LOW>;
> > + linux,default-trigger = "timer";
> > + };
> > + };
> > +
> > + 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>;
>
> Add missing header: dt-bindings/input/input.h to use this.
>
I am doing this in the .dts but it is probably better to do it here
because it is used here.
> > + gpio-key,wakeup;
> > + };
> > + cover {
> > + label = "Cover";
> > + gpios = <&gpio5 12 GPIO_ACTIVE_LOW>;
> > + linux,code = <SW_LID>;
> > + linux,input-type = <0x05>; /* EV_SW */
>
> In the header above EV_SW is also specified so please use it here.
>
This pattern is in many files. I took one as an example. It seems that
50% of devicetree files have this pattern, the other 50% do have the
pattern you proposed (which I like more). So probably EV_SW was not
available in former times?
> > + gpio-key,wakeup;
> > + };
> > + };
> > +
> > +};
> > +
> > +
> > +
>
> Whitespaces
>
> > +&audmux {
> > + pinctrl-names = "default";
> > + status = "disabled";
>
> Why you mentioned a pinctrl-names here without the mux? Do we need the
> status line here? The common case is that such devices are off by
> default/the base dt.
>
yes, that things can be removed.
> > +};
> > +
> > +&snvs_rtc {
> > + status = "disabled";
>
> Same applies here.
>
No, seems to be an exception, it does not have a status = "disabled" in
imx6sll.dtsi.
> > +};
> > +
> > +&i2c1 {
> > + clock-frequency = <100000>;
> > + pinctrl-names = "default","sleep";
> > + pinctrl-0 = <&pinctrl_i2c1 &pinctrl_lm3630a_bl_gpio>;
>
> The &pinctrl_lm3630a_bl_gpio should be moved into the lm3630a node.
>
> > + pinctrl-1 = <&pinctrl_i2c1_sleep>;
> > + status = "okay";
> > +
> > + lm3630a: lm3630a-i2c@36 {
>
> please name it backlight@36
>
> > + reg = <0x36>;
> > + status = "ok";
>
> status lines are always be the last and if it is okay you can drop it
> because the default is okay.
>
well, I added it because the driver was not loaded but later I found out
that the real reason is that module aliases were broken and forgot to
remove that "ok".
Regards,
Andreas
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] ARM: dts: add Netronix E60K02 board common file
2019-09-27 19:08 ` Andreas Kemnade
@ 2019-09-30 8:27 ` Marco Felsch
2019-09-30 11:02 ` Andreas Kemnade
0 siblings, 1 reply; 9+ messages in thread
From: Marco Felsch @ 2019-09-30 8:27 UTC (permalink / raw)
To: Andreas Kemnade
Cc: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam,
linux-imx, manivannan.sadhasivam, andrew.smirnov, marex, angus,
devicetree, linux-kernel, linux-arm-kernel, j.neuschaefer,
Discussions about the Letux Kernel
Hi Andreas,
On 19-09-27 21:08, Andreas Kemnade wrote:
> Hi Marco,
>
> On Fri, 27 Sep 2019 11:47:21 +0200
> Marco Felsch <m.felsch@pengutronix.de> wrote:
>
> > Hi Andreas,
> >
> > thanks for the patch.
> >
> thanks for the quick review. Most of your comments are clear.
>
> [...]
> > > + 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.
> >
> yes, there are many places in my patch where they are added to some
> parent devices.
> I will fix that.
> [...]
> > > + leds {
> > > + compatible = "gpio-leds";
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&pinctrl_led>;
> >
> > Please move all muxing you made here into this file or add phandles so
> > the dts file need to add only the muxing stuff. This applies to all
> > pinctrl you made here.
> >
> so you disagree with this pattern:
> in .dtsi
> some_device {
> pinctrl-0 = <&pinctrl_some_device>;
> };
>
> and in .dts (one I sent with this patch series and the tolino/mx6sl one
> is not ready-cooked yet, will be part of a later series)
> &iomuxc {
> pinctrl_some_device: some_devicegrp {
> fsl,pins = <...>;
> };
> };
>
> ?
Yes, because IMHO a dtsi is self contained as well as a dts. If it is
common for all boards you can move the muxing into the dtsi else it
should be done within the dts.
> > > +
> > > + GLED {
> > > + gpios = <&gpio5 7 GPIO_ACTIVE_LOW>;
> > > + linux,default-trigger = "timer";
> > > + };
> > > + };
> > > +
> > > + 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>;
> >
> > Add missing header: dt-bindings/input/input.h to use this.
> >
> I am doing this in the .dts but it is probably better to do it here
> because it is used here.
>
> > > + gpio-key,wakeup;
> > > + };
> > > + cover {
> > > + label = "Cover";
> > > + gpios = <&gpio5 12 GPIO_ACTIVE_LOW>;
> > > + linux,code = <SW_LID>;
> > > + linux,input-type = <0x05>; /* EV_SW */
> >
> > In the header above EV_SW is also specified so please use it here.
> >
> This pattern is in many files. I took one as an example. It seems that
> 50% of devicetree files have this pattern, the other 50% do have the
> pattern you proposed (which I like more). So probably EV_SW was not
> available in former times?
I don't know, checking the git history should bring the answer ;)
Anyway, if it is available we should use it.
> > > + gpio-key,wakeup;
> > > + };
> > > + };
> > > +
> > > +};
> > > +
> > > +
> > > +
> >
> > Whitespaces
> >
> > > +&audmux {
> > > + pinctrl-names = "default";
> > > + status = "disabled";
> >
> > Why you mentioned a pinctrl-names here without the mux? Do we need the
> > status line here? The common case is that such devices are off by
> > default/the base dt.
> >
> yes, that things can be removed.
> > > +};
> > > +
> > > +&snvs_rtc {
> > > + status = "disabled";
> >
> > Same applies here.
> >
>
> No, seems to be an exception, it does not have a status = "disabled" in
> imx6sll.dtsi.
Did you mean 6sll or 6ull?
Okay, is this baseboard only used with a 6ull?
Regards,
Marco
> > > +};
> > > +
> > > +&i2c1 {
> > > + clock-frequency = <100000>;
> > > + pinctrl-names = "default","sleep";
> > > + pinctrl-0 = <&pinctrl_i2c1 &pinctrl_lm3630a_bl_gpio>;
> >
> > The &pinctrl_lm3630a_bl_gpio should be moved into the lm3630a node.
> >
> > > + pinctrl-1 = <&pinctrl_i2c1_sleep>;
> > > + status = "okay";
> > > +
> > > + lm3630a: lm3630a-i2c@36 {
> >
> > please name it backlight@36
> >
> > > + reg = <0x36>;
> > > + status = "ok";
> >
> > status lines are always be the last and if it is okay you can drop it
> > because the default is okay.
> >
> well, I added it because the driver was not loaded but later I found out
> that the real reason is that module aliases were broken and forgot to
> remove that "ok".
>
> 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 |
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] ARM: dts: add Netronix E60K02 board common file
2019-09-30 8:27 ` Marco Felsch
@ 2019-09-30 11:02 ` Andreas Kemnade
0 siblings, 0 replies; 9+ messages in thread
From: Andreas Kemnade @ 2019-09-30 11:02 UTC (permalink / raw)
To: Marco Felsch
Cc: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam,
linux-imx, manivannan.sadhasivam, andrew.smirnov, marex, angus,
devicetree, linux-kernel, linux-arm-kernel, j.neuschaefer,
Discussions about the Letux Kernel
On Mon, 30 Sep 2019 10:27:15 +0200
Marco Felsch <m.felsch@pengutronix.de> wrote:
[..]
> > so you disagree with this pattern:
> > in .dtsi
> > some_device {
> > pinctrl-0 = <&pinctrl_some_device>;
> > };
> >
> > and in .dts (one I sent with this patch series and the tolino/mx6sl one
> > is not ready-cooked yet, will be part of a later series)
> > &iomuxc {
> > pinctrl_some_device: some_devicegrp {
> > fsl,pins = <...>;
> > };
> > };
> >
> > ?
>
> Yes, because IMHO a dtsi is self contained as well as a dts. If it is
> common for all boards you can move the muxing into the dtsi else it
> should be done within the dts.
>
well, since imx6sll-pinfunc.h is different than imx6sl-pinfunc.h,
we agree that this belongs to the dts.
> > > > +&snvs_rtc {
> > > > + status = "disabled";
> > >
> > > Same applies here.
> > >
> >
> > No, seems to be an exception, it does not have a status = "disabled" in
> > imx6sll.dtsi.
>
> Did you mean 6sll or 6ull?
>
> Okay, is this baseboard only used with a 6ull?
>
MCIMX6V7DVN10AB and MCIMX6L8DVN10AB
So it is 6sll and 6sl (6sl support will be added in a follow-up patch
series).
I will send a v2 this evening, so we can all look at better-sorted
things.
Regards,
Andreas
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-09-30 11:02 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-27 6:14 [PATCH 0/3] dts: ARM: add Kobo Clara HD eBook reader Andreas Kemnade
2019-09-27 6:14 ` [PATCH 1/3] ARM: dts: add Netronix E60K02 board common file Andreas Kemnade
2019-09-27 9:47 ` Marco Felsch
2019-09-27 19:08 ` Andreas Kemnade
2019-09-30 8:27 ` Marco Felsch
2019-09-30 11:02 ` Andreas Kemnade
2019-09-27 6:14 ` [PATCH 2/3] dt-bindings: arm: fsl: add compatible string for Kobo Clara HD Andreas Kemnade
2019-09-27 9:19 ` Marco Felsch
2019-09-27 6:14 ` [PATCH 3/3] ARM: dts: imx: add devicetree " Andreas Kemnade
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).