Skip to contentSkip to conversation

MIPS: Fixes and improvements for CI20 board (JZ4780)

PATCH Superseded H. Nikolaus Schaller 2 participants 13 messages Feb 16, 2020 – Feb 28, 2020

Subsystems (4)
Stable branch Device tree bindings MIPS Ingenic JZ47xx SoCs

v3 of 7 Superseded Latest is v7 →

lore
Share & tools
lore

H. Nikolaus Schaller

author
#0 lore raw

v3 cover MIPS: Fixes and improvements for CI20 board (JZ4780)

V3:
* dropped "drm: ingenic-drm: add MODULE_DEVICE_TABLE" because it will be resubmitted
with a HDMI driver solution
* added Cc: [email protected] where appropriate (suggested by Paul Cercueil <[email protected]>)
* squashed all other DTS and CONFIG changes into single patches (suggested by Paul Cercueil <[email protected]>)

PATCH V2 2020-02-14 17:10:33:
* dropped "net: davicom: dm9000: allow to pass MAC address through mac_addr module parameter" from this series because it goes through the netdev tree
(suggested by Andrew Lunn <[email protected]>)
* added a "fixes:" for "MIPS: DTS: CI20: fix PMU definitions for ACT8600"
and "MIPS: DTS: CI20: fix interrupt for pcf8563 RTC"
(suggested by Andreas Kemnade <[email protected]>)
* "i2c: jz4780: silence log flood on txabrt" dropped because it is
replaced by a new version in v5.6 by Wolfram Sang <[email protected]>

PATCH V1 2020-02-11 22:41:43:
This patch set provides several improvements for the CI20 board:

* suppress warnings from i2c if device is not responding
* make ingenic-drm found through DT
* allow davicom dm9000 ethernet controller to use MAC address provided by U-Boot
* fix #include in jz4780.dtsi
* configure for loadable kernel modules
* add DTS for IR sensor and SW1 button
* configure so that LEDs, IR sensor, SW1 button have drivers
* fix DTS for ACT8600 PMU and configure driver
* fix interrupt of nxp,pcf8563

There is another patch set in our queue to add HDMI support on top of this work.

Signed-off-by Paul Boddie <[email protected]>
Signed-off-by H. Nikolaus Schaller <[email protected]>

Alex Smith (1):
MIPS: DTS: CI20: add DT node for IR sensor

H. Nikolaus Schaller (5):
MIPS: DTS: jz4780: add #includes for irq.h and gpio.h
MIPS: DTS: CI20: fix PMU definitions for ACT8600
MIPS: DTS: CI20: fix interrupt for pcf8563 RTC
MIPS: DTS: CI20: multiple DTS improvements
MIPS: CI20: defconfig: multiple improvements

 arch/mips/boot/dts/ingenic/ci20.dts    | 71 ++++++++++++++++++++------
 arch/mips/boot/dts/ingenic/jz4780.dtsi |  2 +
 arch/mips/configs/ci20_defconfig       | 21 ++++++++
 3 files changed, 78 insertions(+), 16 deletions(-)
Signature · 1 line
2.23.0
author
#1 lore raw

PATCH v3 1/6 MIPS: DTS: jz4780: add #includes for irq.h and gpio.h

The constants from irq.h and gpio.h can and should be used in the jz4780.dtsi and derived DTS like ci20.dts.

Signed-off-by H. Nikolaus Schaller <[email protected]>
---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 ++
 1 file changed, 2 insertions(+)
arch/mips/boot/dts/ingenic/jz4780.dtsi +2 -0
index f928329b034b..112a24deff71 100644
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <dt-bindings/clock/jz4780-cgu.h>
 #include <dt-bindings/dma/jz4780-dma.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
 
 / {
 	#address-cells = <1>;
Signature · 1 line
2.23.0
maintainer
#2 lore raw

Re: [PATCH v3 1/6] MIPS: DTS: jz4780: add #includes for irq.h and gpio.h

Hi Nikolaus,

Quoted 13 lines · H. Nikolaus Schaller

The constants from irq.h and gpio.h can and should be used in the jz4780.dtsi and derived DTS like ci20.dts.

Signed-off-by H. Nikolaus Schaller <[email protected]>
---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 ++
 1 file changed, 2 insertions(+)
arch/mips/boot/dts/ingenic/jz4780.dtsi +0 -0
index f928329b034b..112a24deff71 100644

arch/mips/boot/dts/ingenic/jz4780.dtsi line 5

Quoted 5 lines
arch/mips/boot/dts/ingenic/jz4780.dtsi +2 -0
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <dt-bindings/clock/jz4780-cgu.h>
 #include <dt-bindings/dma/jz4780-dma.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>

I don't think you need an extra patch for that - just add each include in the patch that first use them.

-Paul

Quoted 6 lines
arch/mips/boot/dts/ingenic/jz4780.dtsi +0 -1
 / {
 	#address-cells = <1>;
--

2.23.0

author
#3 lore raw

PATCH v3 2/6 MIPS: DTS: CI20: add DT node for IR sensor

In reply to H. Nikolaus Schaller

From: Alex Smith <[email protected]>

The infrared sensor on the CI20 board is connected to a GPIO and can be operated by using the gpio-ir-recv driver. Add a DT node for the sensor to allow that driver to be used.

Signed-off-by Alex Smith <[email protected]>
Signed-off-by H. Nikolaus Schaller <[email protected]>
---
 arch/mips/boot/dts/ingenic/ci20.dts | 5 +++++
 1 file changed, 5 insertions(+)
arch/mips/boot/dts/ingenic/ci20.dts +5 -0
index 37b93166bf22..59c104289ece 100644
@@ -60,6 +60,11 @@
 		enable-active-high;
 	};
 
+	ir: ir-receiver {
+		compatible = "gpio-ir-receiver";
+		gpios = <&gpe 3 GPIO_ACTIVE_LOW>;
+	};
+
 	wlan0_power: fixedregulator@1 {
 		compatible = "regulator-fixed";
 		regulator-name = "wlan0_power";
Signature · 1 line
2.23.0
author
#4 lore raw

PATCH v3 3/6 MIPS: DTS: CI20: fix PMU definitions for ACT8600

In reply to H. Nikolaus Schaller

There is a ACT8600 on the CI20 board and the bindings of the ACT8865 driver have changed without updating the CI20 device tree. Therefore the PMU can not be probed successfully and is running in power-on reset state.

Fix DT to match the latest act8865-regulator bindings.

Fixes 73f2b940474d ("MIPS: CI20: DTS: Add I2C nodes")
Cc [email protected]
Signed-off-by H. Nikolaus Schaller <[email protected]>
---
 arch/mips/boot/dts/ingenic/ci20.dts | 48 ++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 15 deletions(-)
arch/mips/boot/dts/ingenic/ci20.dts +33 -15
index 59c104289ece..4f48bc16fb52 100644
@@ -153,6 +153,8 @@
 	pinctrl-0 = <&pins_uart4>;
 };
 
+#include <dt-bindings/regulator/active-semi,8865-regulator.h>
+
 &i2c0 {
 	status = "okay";
 

@@ -166,65 +168,81 @@
 		reg = <0x5a>;
 		status = "okay";
 
+/*
+Optional input supply properties:
+- for act8600:
+  - vp1-supply: The input supply for DCDC_REG1
+  - vp2-supply: The input supply for DCDC_REG2
+  - vp3-supply: The input supply for DCDC_REG3
+  - inl-supply: The input supply for LDO_REG5, LDO_REG6, LDO_REG7 and LDO_REG8
+  SUDCDC_REG4, LDO_REG9 and LDO_REG10 do not have separate supplies.
+*/
+
 		regulators {
 			vddcore: SUDCDC1 {
-				regulator-name = "VDDCORE";
+				regulator-name = "DCDC_REG1";
 				regulator-min-microvolt = <1100000>;
 				regulator-max-microvolt = <1100000>;
 				regulator-always-on;
 			};
 			vddmem: SUDCDC2 {
-				regulator-name = "VDDMEM";
+				regulator-name = "DCDC_REG2";
 				regulator-min-microvolt = <1500000>;
 				regulator-max-microvolt = <1500000>;
 				regulator-always-on;
 			};
 			vcc_33: SUDCDC3 {
-				regulator-name = "VCC33";
+				regulator-name = "DCDC_REG3";
 				regulator-min-microvolt = <3300000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-always-on;
 			};
 			vcc_50: SUDCDC4 {
-				regulator-name = "VCC50";
+				regulator-name = "SUDCDC_REG4";
 				regulator-min-microvolt = <5000000>;
 				regulator-max-microvolt = <5000000>;
 				regulator-always-on;
 			};
 			vcc_25: LDO_REG5 {
-				regulator-name = "VCC25";
+				regulator-name = "LDO_REG5";
 				regulator-min-microvolt = <2500000>;
 				regulator-max-microvolt = <2500000>;
 				regulator-always-on;
 			};
 			wifi_io: LDO_REG6 {
-				regulator-name = "WIFIIO";
+				regulator-name = "LDO_REG6";
 				regulator-min-microvolt = <2500000>;
 				regulator-max-microvolt = <2500000>;
 				regulator-always-on;
 			};
 			vcc_28: LDO_REG7 {
-				regulator-name = "VCC28";
+				regulator-name = "LDO_REG7";
 				regulator-min-microvolt = <2800000>;
 				regulator-max-microvolt = <2800000>;
 				regulator-always-on;
 			};
 			vcc_15: LDO_REG8 {
-				regulator-name = "VCC15";
+				regulator-name = "LDO_REG8";
 				regulator-min-microvolt = <1500000>;
 				regulator-max-microvolt = <1500000>;
 				regulator-always-on;
 			};
-			vcc_18: LDO_REG9 {
-				regulator-name = "VCC18";
-				regulator-min-microvolt = <1800000>;
-				regulator-max-microvolt = <1800000>;
+			vrtc_18: LDO_REG9 {
+				regulator-name = "LDO_REG9";
+				/* Despite the datasheet stating 3.3V for REG9 and
+				   driver expecting that, REG9 outputs 1.8V.
+				   Likely the CI20 uses a chip variant.
+				   Since it is a simple on/off LDO the exact values
+				   do not matter.
+				*/
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
 				regulator-always-on;
 			};
 			vcc_11: LDO_REG10 {
-				regulator-name = "VCC11";
-				regulator-min-microvolt = <1100000>;
-				regulator-max-microvolt = <1100000>;
+				regulator-name = "LDO_REG10";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1200000>;
 				regulator-always-on;
 			};
 		};
Signature · 1 line
2.23.0
maintainer
#5 lore raw

Re: [PATCH v3 3/6] MIPS: DTS: CI20: fix PMU definitions for ACT8600

Hi Nikolaus,

Quoted 20 lines · H. Nikolaus Schaller

There is a ACT8600 on the CI20 board and the bindings of the ACT8865 driver have changed without updating the CI20 device tree. Therefore the PMU can not be probed successfully and is running in power-on reset state.

Fix DT to match the latest act8865-regulator bindings.

Fixes 73f2b940474d ("MIPS: CI20: DTS: Add I2C nodes")
Cc [email protected]
Signed-off-by H. Nikolaus Schaller <[email protected]>
---
 arch/mips/boot/dts/ingenic/ci20.dts | 48 
++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 15 deletions(-)
arch/mips/boot/dts/ingenic/ci20.dts +0 -0
index 59c104289ece..4f48bc16fb52 100644

arch/mips/boot/dts/ingenic/ci20.dts line 156

Quoted 4 lines
arch/mips/boot/dts/ingenic/ci20.dts +1 -0
@@ -153,6 +153,8 @@
 	pinctrl-0 = <&pins_uart4>;
 };
 
+#include <dt-bindings/regulator/active-semi,8865-regulator.h>

Includes at the beginning of the file please. Keeps it tidy.

Quoted 105 lines
arch/mips/boot/dts/ingenic/ci20.dts +8 -0
+
 &i2c0 {
 	status = "okay";
 

@@ -166,65 +168,81 @@
 		reg = <0x5a>;
 		status = "okay";
 
+/*
+Optional input supply properties:
+- for act8600:
+  - vp1-supply: The input supply for DCDC_REG1
+  - vp2-supply: The input supply for DCDC_REG2
+  - vp3-supply: The input supply for DCDC_REG3
+  - inl-supply: The input supply for LDO_REG5, LDO_REG6, LDO_REG7

and LDO_REG8

arch/mips/boot/dts/ingenic/ci20.dts +24 -16
+  SUDCDC_REG4, LDO_REG9 and LDO_REG10 do not have separate supplies.
+*/
+
 		regulators {
 			vddcore: SUDCDC1 {
-				regulator-name = "VDDCORE";
+				regulator-name = "DCDC_REG1";
 				regulator-min-microvolt = <1100000>;
 				regulator-max-microvolt = <1100000>;
 				regulator-always-on;
 			};
 			vddmem: SUDCDC2 {
-				regulator-name = "VDDMEM";
+				regulator-name = "DCDC_REG2";
 				regulator-min-microvolt = <1500000>;
 				regulator-max-microvolt = <1500000>;
 				regulator-always-on;
 			};
 			vcc_33: SUDCDC3 {
-				regulator-name = "VCC33";
+				regulator-name = "DCDC_REG3";
 				regulator-min-microvolt = <3300000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-always-on;
 			};
 			vcc_50: SUDCDC4 {
-				regulator-name = "VCC50";
+				regulator-name = "SUDCDC_REG4";
 				regulator-min-microvolt = <5000000>;
 				regulator-max-microvolt = <5000000>;
 				regulator-always-on;
 			};
 			vcc_25: LDO_REG5 {
-				regulator-name = "VCC25";
+				regulator-name = "LDO_REG5";
 				regulator-min-microvolt = <2500000>;
 				regulator-max-microvolt = <2500000>;
 				regulator-always-on;
 			};
 			wifi_io: LDO_REG6 {
-				regulator-name = "WIFIIO";
+				regulator-name = "LDO_REG6";
 				regulator-min-microvolt = <2500000>;
 				regulator-max-microvolt = <2500000>;
 				regulator-always-on;
 			};
 			vcc_28: LDO_REG7 {
-				regulator-name = "VCC28";
+				regulator-name = "LDO_REG7";
 				regulator-min-microvolt = <2800000>;
 				regulator-max-microvolt = <2800000>;
 				regulator-always-on;
 			};
 			vcc_15: LDO_REG8 {
-				regulator-name = "VCC15";
+				regulator-name = "LDO_REG8";
 				regulator-min-microvolt = <1500000>;
 				regulator-max-microvolt = <1500000>;
 				regulator-always-on;
 			};
-			vcc_18: LDO_REG9 {
-				regulator-name = "VCC18";
-				regulator-min-microvolt = <1800000>;
-				regulator-max-microvolt = <1800000>;
+			vrtc_18: LDO_REG9 {
+				regulator-name = "LDO_REG9";
+				/* Despite the datasheet stating 3.3V for REG9 and
+				   driver expecting that, REG9 outputs 1.8V.
+				   Likely the CI20 uses a chip variant.
+				   Since it is a simple on/off LDO the exact values
+				   do not matter.
+				*/
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
 				regulator-always-on;
 			};
 			vcc_11: LDO_REG10 {
-				regulator-name = "VCC11";
-				regulator-min-microvolt = <1100000>;
-				regulator-max-microvolt = <1100000>;
+				regulator-name = "LDO_REG10";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1200000>;
 				regulator-always-on;
 			};
 		};
--

2.23.0

author
#6 lore raw

Re: [PATCH v3 3/6] MIPS: DTS: CI20: fix PMU definitions for ACT8600

Quoted 20 lines

Am 28.02.2020 um 15:34 schrieb Paul Cercueil <[email protected]>:

Hi Nikolaus,

Quoted 13 lines · H. Nikolaus Schaller

There is a ACT8600 on the CI20 board and the bindings of the ACT8865 driver have changed without updating the CI20 device tree. Therefore the PMU can not be probed successfully and is running in power-on reset state.
Fix DT to match the latest act8865-regulator bindings.

Fixes 73f2b940474d ("MIPS: CI20: DTS: Add I2C nodes")
Cc [email protected]
Signed-off-by H. Nikolaus Schaller <[email protected]>

---
arch/mips/boot/dts/ingenic/ci20.dts | 48 ++++++++++++++++++++---------

1 file changed, 33 insertions(+), 15 deletions(-)
arch/mips/boot/dts/ingenic/ci20.dts +0 -0
index 59c104289ece..4f48bc16fb52 100644
Quoted 8 lines

arch/mips/boot/dts/ingenic/ci20.dts line 155

Quoted 6 lines
arch/mips/boot/dts/ingenic/ci20.dts +1 -0
@@ -153,6 +153,8 @@
 	pinctrl-0 = <&pins_uart4>;
 };
+#include <dt-bindings/regulator/active-semi,8865-regulator.h>

Includes at the beginning of the file please. Keeps it tidy.

Ok.

Well, I am infected by omap boards where this is not uncommon:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/omap3-beagle-xm.dts?h=v5.6-rc3#n294

But it may be historic and not a good style.

BR and thanks,
Nikolaus

author
#7 lore raw

PATCH v3 4/6 MIPS: DTS: CI20: fix interrupt for pcf8563 RTC

In reply to H. Nikolaus Schaller

Interrupts should not be specified by interrupt line but by gpio parent and reference.

Fixes 73f2b940474d ("MIPS: CI20: DTS: Add I2C nodes")
Cc [email protected]
Signed-off-by H. Nikolaus Schaller <[email protected]>
---
 arch/mips/boot/dts/ingenic/ci20.dts | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/ci20.dts +3 -1
index 4f48bc16fb52..1ab55be707af 100644
@@ -284,7 +284,9 @@ Optional input supply properties:
 		rtc@51 {
 			compatible = "nxp,pcf8563";
 			reg = <0x51>;
-			interrupts = <110>;
+
+			interrupt-parent = <&gpf>;
+			interrupts = <30 IRQ_TYPE_LEVEL_LOW>;
 		};
 };
Signature · 1 line
2.23.0
author
#8 lore raw

PATCH v3 5/6 MIPS: DTS: CI20: multiple DTS improvements

In reply to H. Nikolaus Schaller

a) add DT node for SW1 as Enter button

The SW1 button can be used as a simple one-button keyboard and is connected to PD17.

Note: SW1 has a second meaning to change the boot sequence when pressed while powering on.

b) give eth0_power a defined voltage.

This is a 3.3V power switch (DVNET3.3V ).

Signed-off-by H. Nikolaus Schaller <[email protected]>
---
 arch/mips/boot/dts/ingenic/ci20.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
arch/mips/boot/dts/ingenic/ci20.dts +14 -0
index 1ab55be707af..4bacefa2cfce 100644
@@ -4,6 +4,7 @@
 #include "jz4780.dtsi"
 #include <dt-bindings/clock/ingenic,tcu.h>
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
 
 / {
 	compatible = "img,ci20", "ingenic,jz4780";

@@ -25,6 +26,17 @@
 		       0x30000000 0x30000000>;
 	};
 
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		sw1 {
+			label = "ci20:sw1";
+			linux,code = <KEY_ENTER>;
+			gpios = <&gpd 17 GPIO_ACTIVE_HIGH>;
+			wakeup-source;
+		};
+	};
+
 	leds {
 		compatible = "gpio-leds";
 

@@ -56,6 +68,8 @@
 	eth0_power: fixedregulator@0 {
 		compatible = "regulator-fixed";
 		regulator-name = "eth0_power";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
 		gpio = <&gpb 25 GPIO_ACTIVE_LOW>;
 		enable-active-high;
 	};
Signature · 1 line
2.23.0
maintainer
#9 lore raw

Re: [PATCH v3 5/6] MIPS: DTS: CI20: multiple DTS improvements

Hi Nikolaus,

Quoted 30 lines · H. Nikolaus Schaller

a) add DT node for SW1 as Enter button

The SW1 button can be used as a simple one-button keyboard and is connected to PD17.

Note: SW1 has a second meaning to change the boot sequence when pressed while powering on.

b) give eth0_power a defined voltage.

This is a 3.3V power switch (DVNET3.3V ).

Signed-off-by H. Nikolaus Schaller <[email protected]>
---
 arch/mips/boot/dts/ingenic/ci20.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
arch/mips/boot/dts/ingenic/ci20.dts +1 -0
index 1ab55be707af..4bacefa2cfce 100644
@@ -4,6 +4,7 @@
 #include "jz4780.dtsi"
 #include <dt-bindings/clock/ingenic,tcu.h>
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
 
 / {
 	compatible = "img,ci20", "ingenic,jz4780";

arch/mips/boot/dts/ingenic/ci20.dts line 34

Quoted 9 lines
arch/mips/boot/dts/ingenic/ci20.dts +6 -0
@@ -25,6 +26,17 @@
 		       0x30000000 0x30000000>;
 	};
 
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		sw1 {
+			label = "ci20:sw1";
+			linux,code = <KEY_ENTER>;

Why KEY_ENTER? It would make it impossible for applications to know that it's actually the switch that has been pressed an not the keyboard.

Quoted 20 lines
arch/mips/boot/dts/ingenic/ci20.dts +7 -1
+			gpios = <&gpd 17 GPIO_ACTIVE_HIGH>;
+			wakeup-source;
+		};
+	};
+
 	leds {
 		compatible = "gpio-leds";
 

@@ -56,6 +68,8 @@
 	eth0_power: fixedregulator@0 {
 		compatible = "regulator-fixed";
 		regulator-name = "eth0_power";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
 		gpio = <&gpb 25 GPIO_ACTIVE_LOW>;
 		enable-active-high;
 	};
--

2.23.0

author
#10 lore raw

Re: [PATCH v3 5/6] MIPS: DTS: CI20: multiple DTS improvements

Quoted 31 lines

Am 28.02.2020 um 15:36 schrieb Paul Cercueil <[email protected]>:

Hi Nikolaus,

Quoted 25 lines · H. Nikolaus Schaller

a) add DT node for SW1 as Enter button
The SW1 button can be used as a simple one-button keyboard and is connected to PD17.
Note: SW1 has a second meaning to change the boot sequence when pressed while powering on.
b) give eth0_power a defined voltage.
This is a 3.3V power switch (DVNET3.3V ).

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
arch/mips/boot/dts/ingenic/ci20.dts | 14 ++++++++++++++

1 file changed, 14 insertions(+)
arch/mips/boot/dts/ingenic/ci20.dts +1 -0
index 1ab55be707af..4bacefa2cfce 100644
@@ -4,6 +4,7 @@
 #include "jz4780.dtsi"
 #include <dt-bindings/clock/ingenic,tcu.h>
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
 / {
 	compatible = "img,ci20", "ingenic,jz4780";

@@ -25,6 +26,17 @@
 		       0x30000000 0x30000000>;
 	};
Quoted 8 lines

arch/mips/boot/dts/ingenic/ci20.dts line 34

Quoted 6 lines
arch/mips/boot/dts/ingenic/ci20.dts +6 -0
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		sw1 {
+			label = "ci20:sw1";
+			linux,code = <KEY_ENTER>;

Why KEY_ENTER? It would make it impossible for applications to know that it's actually the switch that has been pressed an not the keyboard.

Ah, ok. I didn't think about the use case that a physical keyboard is connected to one of the USB ports.

What else would you propose? I think your argument is for every existing KEY_CODE. Should we add a new one?

BR and thanks,
Nikolaus

maintainer
#11 lore raw

Re: [PATCH v3 5/6] MIPS: DTS: CI20: multiple DTS improvements

Quoted 40 lines · H. Nikolaus Schaller
Quoted 40 lines

Am 28.02.2020 um 15:36 schrieb Paul Cercueil <[email protected]>:

Hi Nikolaus,

Quoted 32 lines · H. Nikolaus Schaller

a) add DT node for SW1 as Enter button
The SW1 button can be used as a simple one-button keyboard and is connected to PD17.
Note: SW1 has a second meaning to change the boot sequence when pressed while powering on.
b) give eth0_power a defined voltage.
This is a 3.3V power switch (DVNET3.3V ).

Signed-off-by H. Nikolaus Schaller <[email protected]>

---

 arch/mips/boot/dts/ingenic/ci20.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/mips/boot/dts/ingenic/ci20.dts
b/arch/mips/boot/dts/ingenic/ci20.dts
index 1ab55be707af..4bacefa2cfce 100644
--- a/arch/mips/boot/dts/ingenic/ci20.dts
+++ b/arch/mips/boot/dts/ingenic/ci20.dts
@@ -4,6 +4,7 @@
#include "jz4780.dtsi"
#include <dt-bindings/clock/ingenic,tcu.h>
#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
/ {

(quoted diff) +0 -0
 	compatible = "img,ci20", "ingenic,jz4780";
 @@ -25,6 +26,17 @@
 		       0x30000000 0x30000000>;
 	};
 +	gpio-keys {
 +		compatible = "gpio-keys";
 +
 +		sw1 {
 +			label = "ci20:sw1";
 +			linux,code = <KEY_ENTER>;
Quoted 6 lines
Quoted 2 lines

Why KEY_ENTER? It would make it impossible for applications to know
that it's actually the switch that has been pressed an not the keyboard.

Ah, ok. I didn't think about the use case that a physical keyboard is connected to one of the USB ports.

What else would you propose? I think your argument is for every existing KEY_CODE. Should we add a new one?

There are plenty you can use. I'd suggest KEY_F13 as it's general-purpose.

-Paul

author
#12 lore raw

PATCH v3 6/6 MIPS: CI20: defconfig: multiple improvements

In reply to H. Nikolaus Schaller

a) configure for supporting modules

Not all drivers need to be compiled into the kernel. Support building and loading of kernel modules.

b) compile leds-gpio driver into the kernel and configure for LED triggers

DTS has been augmented to add some gpio-leds. We need the leds-gpio driver and enable the triggers.

c) configure CONFIG_REGULATOR_ACT8865 for PMU

The PMU on the CI20 board is an ACT8600 using the ACT8865 driver. Since it is not compiled, the PMU and the CI20 board is running in power-on reset state of the PMU.

d) compile gpio-ir driver

The CI20 board has a gpio based IR receiver.

e) configure for CONFIG_KEYBOARD_GPIO=m

The SW1 button is hooked up to send input events.

Signed-off-by H. Nikolaus Schaller <[email protected]>
---
 arch/mips/configs/ci20_defconfig | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
arch/mips/configs/ci20_defconfig +21 -0
index be41df2a81fb..0db0088bbc1c 100644
@@ -1,4 +1,5 @@
 # CONFIG_LOCALVERSION_AUTO is not set
+CONFIG_MODULES=y
 CONFIG_KERNEL_XZ=y
 CONFIG_SYSVIPC=y
 CONFIG_POSIX_MQUEUE=y

@@ -88,12 +89,14 @@ CONFIG_I2C_JZ4780=y
 CONFIG_SPI=y
 CONFIG_SPI_GPIO=y
 CONFIG_GPIO_SYSFS=y
+CONFIG_KEYBOARD_GPIO=m
 # CONFIG_HWMON is not set
 CONFIG_WATCHDOG=y
 CONFIG_JZ4740_WDT=y
 CONFIG_REGULATOR=y
 CONFIG_REGULATOR_DEBUG=y
 CONFIG_REGULATOR_FIXED_VOLTAGE=y
+CONFIG_REGULATOR_ACT8865=y
 # CONFIG_VGA_CONSOLE is not set
 # CONFIG_HID is not set
 # CONFIG_USB_SUPPORT is not set

@@ -166,3 +169,21 @@ CONFIG_STACKTRACE=y
 # CONFIG_FTRACE is not set
 CONFIG_CMDLINE_BOOL=y
 CONFIG_CMDLINE="earlycon console=ttyS4,115200 clk_ignore_unused"
+CONFIG_LEDS_CLASS=y
+CONFIG_LEDS_GPIO=y
+CONFIG_LEDS_TRIGGERS=y
+CONFIG_LEDS_TRIGGER_MTD=y
+CONFIG_LEDS_TRIGGER_TIMER=y
+CONFIG_LEDS_TRIGGER_ONESHOT=y
+CONFIG_LEDS_TRIGGER_ONESHOT=y
+CONFIG_LEDS_TRIGGER_HEARTBEAT=y
+CONFIG_LEDS_TRIGGER_BACKLIGHT=m
+CONFIG_LEDS_TRIGGER_CPU=y
+CONFIG_LEDS_TRIGGER_DEFAULT_ON=y
+CONFIG_LEDS_TRIGGER_TRANSIENT=y
+CONFIG_LEDS_TRIGGER_CAMERA=m
+CONFIG_LIRC=y
+CONFIG_MEDIA_SUPPORT=m
+CONFIG_RC_DEVICES=y
+CONFIG_IR_GPIO_CIR=m
+CONFIG_IR_GPIO_TX=m
Signature · 1 line
2.23.0