[PATCH 0/7] Nokia N9/N950 misc. DT patches

[PATCH 0/7] Nokia N9/N950 misc. DT patches

[Sebastian Reichel]

Hi,

Here are a couple of N950/N9 DT patches. I only tested them
on the Nokia N950, but Filip tested his patches on N9 and
the regulator configuration in Nokia's kernel is the same
for N950 and N9.

The patches are based on 4.5-rc7.

-- Sebastian

Filip Matijević (2):
  ARM: dts: N9/N950: Add support for 1GHz CPU clock
  ARM: dts: N9/N950: Add support for accelerometer

Sebastian Reichel (5):
  ARM: dts: n9/n950: regulator configuration
  ARM: dts: OMAP3-N950: Add Keypad Matrix
  ARM: dts: OMAP3-N950: Add Vibrator
  ARM: dts: Enable N950 keyboard sleep leds by default
  ARM: dts: OMAP3-N950: Add Keypad Slide Switch

 arch/arm/boot/dts/omap3-n9.dts       |  14 ++++
 arch/arm/boot/dts/omap3-n950-n9.dtsi | 151 +++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/omap3-n950.dts     |  99 +++++++++++++++++++++++
 3 files changed, 264 insertions(+)

-- 
2.7.0

[PATCH 1/7] ARM: dts: n9/n950: regulator configuration

[Sebastian Reichel]

Add regulator configuration as found in the
board files of Nokia's kernel.

Signed-off-By: Sebastian Reichel <sre@kernel.org>
---
 arch/arm/boot/dts/omap3-n950-n9.dtsi | 72 ++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi
index a2c2b8d8dd2c..3c7f1d2deb2f 100644
--- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
+++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
@@ -72,6 +72,30 @@
 	ti,pulldowns	= <0x008106>; /* BIT(1) | BIT(2) | BIT(8) | BIT(15) */
 };
 
+&vdac {
+	regulator-name = "vdac";
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <1800000>;
+};
+
+&vpll1 {
+	regulator-name = "vpll1";
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <1800000>;
+};
+
+&vpll2 {
+	regulator-name = "vpll2";
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <1800000>;
+};
+
+&vaux1 {
+	regulator-name = "vaux1";
+	regulator-min-microvolt = <2800000>;
+	regulator-max-microvolt = <2800000>;
+};
+
 /* CSI-2 receiver */
 &vaux2 {
 	regulator-name = "vaux2";
@@ -86,6 +110,54 @@
 	regulator-max-microvolt = <2800000>;
 };
 
+&vaux4 {
+	regulator-name = "vaux4";
+	regulator-min-microvolt = <2800000>;
+	regulator-max-microvolt = <2800000>;
+};
+
+&vmmc1 {
+	regulator-name = "vmmc1";
+	regulator-min-microvolt = <1850000>;
+	regulator-max-microvolt = <3150000>;
+};
+
+&vmmc2 {
+	regulator-name = "vmmc2";
+	regulator-min-microvolt = <3000000>;
+	regulator-max-microvolt = <3000000>;
+};
+
+&vintana1 {
+	regulator-name = "vintana1";
+	regulator-min-microvolt = <1500000>;
+	regulator-max-microvolt = <1500000>;
+};
+
+&vintana2 {
+	regulator-name = "vintana2";
+	regulator-min-microvolt = <2750000>;
+	regulator-max-microvolt = <2750000>;
+};
+
+&vintdig {
+	regulator-name = "vintdig";
+	regulator-min-microvolt = <1500000>;
+	regulator-max-microvolt = <1500000>;
+};
+
+&vsim {
+	regulator-name = "vsim";
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <1800000>;
+};
+
+&vio {
+	regulator-name = "vio";
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <1800000>;
+};
+
 &i2c2 {
 	clock-frequency = <400000>;
 };
-- 
2.7.0

[PATCH 2/7] ARM: dts: OMAP3-N950: Add Keypad Matrix

[Sebastian Reichel]

Add keypad matrix information based on data from
Nokia N950 Kernel.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 arch/arm/boot/dts/omap3-n950.dts | 56 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-n950.dts b/arch/arm/boot/dts/omap3-n950.dts
index 0885b34d5d7d..80bed4eb6b08 100644
--- a/arch/arm/boot/dts/omap3-n950.dts
+++ b/arch/arm/boot/dts/omap3-n950.dts
@@ -11,6 +11,7 @@
 /dts-v1/;
 
 #include "omap3-n950-n9.dtsi"
+#include <dt-bindings/input/input.h>
 
 / {
 	model = "Nokia N950";
@@ -53,3 +54,58 @@
 		};
 	};
 };
+
+&twl_keypad {
+	linux,keymap = < MATRIX_KEY(0x00, 0x00, KEY_BACKSLASH)
+			 MATRIX_KEY(0x01, 0x00, KEY_LEFTSHIFT)
+			 MATRIX_KEY(0x02, 0x00, KEY_COMPOSE)
+			 MATRIX_KEY(0x03, 0x00, KEY_LEFTMETA)
+			 MATRIX_KEY(0x04, 0x00, KEY_RIGHTCTRL)
+			 MATRIX_KEY(0x05, 0x00, KEY_BACKSPACE)
+			 MATRIX_KEY(0x06, 0x00, KEY_VOLUMEDOWN)
+			 MATRIX_KEY(0x07, 0x00, KEY_VOLUMEUP)
+
+			 MATRIX_KEY(0x03, 0x01, KEY_Z)
+			 MATRIX_KEY(0x04, 0x01, KEY_A)
+			 MATRIX_KEY(0x05, 0x01, KEY_Q)
+			 MATRIX_KEY(0x06, 0x01, KEY_W)
+			 MATRIX_KEY(0x07, 0x01, KEY_E)
+
+			 MATRIX_KEY(0x03, 0x02, KEY_X)
+			 MATRIX_KEY(0x04, 0x02, KEY_S)
+			 MATRIX_KEY(0x05, 0x02, KEY_D)
+			 MATRIX_KEY(0x06, 0x02, KEY_C)
+			 MATRIX_KEY(0x07, 0x02, KEY_V)
+
+			 MATRIX_KEY(0x03, 0x03, KEY_O)
+			 MATRIX_KEY(0x04, 0x03, KEY_I)
+			 MATRIX_KEY(0x05, 0x03, KEY_U)
+			 MATRIX_KEY(0x06, 0x03, KEY_L)
+			 MATRIX_KEY(0x07, 0x03, KEY_APOSTROPHE)
+
+			 MATRIX_KEY(0x03, 0x04, KEY_Y)
+			 MATRIX_KEY(0x04, 0x04, KEY_K)
+			 MATRIX_KEY(0x05, 0x04, KEY_J)
+			 MATRIX_KEY(0x06, 0x04, KEY_H)
+			 MATRIX_KEY(0x07, 0x04, KEY_G)
+
+			 MATRIX_KEY(0x03, 0x05, KEY_B)
+			 MATRIX_KEY(0x04, 0x05, KEY_COMMA)
+			 MATRIX_KEY(0x05, 0x05, KEY_M)
+			 MATRIX_KEY(0x06, 0x05, KEY_N)
+			 MATRIX_KEY(0x07, 0x05, KEY_DOT)
+
+			 MATRIX_KEY(0x00, 0x06, KEY_SPACE)
+			 MATRIX_KEY(0x03, 0x06, KEY_T)
+			 MATRIX_KEY(0x04, 0x06, KEY_UP)
+			 MATRIX_KEY(0x05, 0x06, KEY_LEFT)
+			 MATRIX_KEY(0x06, 0x06, KEY_RIGHT)
+			 MATRIX_KEY(0x07, 0x06, KEY_DOWN)
+
+			 MATRIX_KEY(0x03, 0x07, KEY_P)
+			 MATRIX_KEY(0x04, 0x07, KEY_ENTER)
+			 MATRIX_KEY(0x05, 0x07, KEY_SLASH)
+			 MATRIX_KEY(0x06, 0x07, KEY_F)
+			 MATRIX_KEY(0x07, 0x07, KEY_R)
+			 >;
+};
-- 
2.7.0

[PATCH 3/7] ARM: dts: OMAP3-N950: Add Vibrator

[Sebastian Reichel]

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 arch/arm/boot/dts/omap3-n950.dts | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-n950.dts b/arch/arm/boot/dts/omap3-n950.dts
index 80bed4eb6b08..3dbf9d66bcc1 100644
--- a/arch/arm/boot/dts/omap3-n950.dts
+++ b/arch/arm/boot/dts/omap3-n950.dts
@@ -55,6 +55,13 @@
 	};
 };
 
+&twl {
+	twl_audio: audio {
+		compatible = "ti,twl4030-audio";
+		ti,enable-vibra = <1>;
+	};
+};
+
 &twl_keypad {
 	linux,keymap = < MATRIX_KEY(0x00, 0x00, KEY_BACKSLASH)
 			 MATRIX_KEY(0x01, 0x00, KEY_LEFTSHIFT)
-- 
2.7.0

[PATCH 4/7] ARM: dts: Enable N950 keyboard sleep leds by default

[Sebastian Reichel]

Like the Nokia N900, the N950 has leds to show
the state of sys_clkreq and sys_off_mode pins.

A detailed description for the LEDs and
OMAP's sleep states can be found in Tony's
commit for the Nokia N900:

c1be2032f66df9e1238bd5bc4ca666de88a62abc

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 arch/arm/boot/dts/omap3-n950-n9.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi
index 3c7f1d2deb2f..c42e8fc846b9 100644
--- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
+++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
@@ -31,9 +31,27 @@
 		startup-delay-us = <150>;
 		enable-active-high;
 	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		heartbeat {
+			label = "debug::sleep";
+			gpios = <&gpio3 28 GPIO_ACTIVE_HIGH>;  /* gpio92 */
+			linux,default-trigger = "default-on";
+			pinctrl-names = "default";
+			pinctrl-0 = <&debug_leds>;
+		};
+	};
 };
 
 &omap3_pmx_core {
+	debug_leds: pinmux_debug_led_pins {
+		pinctrl-single,pins = <
+			OMAP3_CORE1_IOPAD(0x2108, PIN_OUTPUT | MUX_MODE4)       /* dss_data22.gpio_92 */
+		>;
+	};
+
 	mmc2_pins: pinmux_mmc2_pins {
 		pinctrl-single,pins = <
 			OMAP3_CORE1_IOPAD(0x2158, PIN_INPUT_PULLUP | MUX_MODE0) /* sdmmc2_clk */
-- 
2.7.0

[PATCH 5/7] ARM: dts: OMAP3-N950: Add Keypad Slide Switch

[Sebastian Reichel]

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 arch/arm/boot/dts/omap3-n950.dts | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-n950.dts b/arch/arm/boot/dts/omap3-n950.dts
index 3dbf9d66bcc1..daa17d20ccf5 100644
--- a/arch/arm/boot/dts/omap3-n950.dts
+++ b/arch/arm/boot/dts/omap3-n950.dts
@@ -16,6 +16,28 @@
 / {
 	model = "Nokia N950";
 	compatible = "nokia,omap3-n950", "ti,omap36xx", "ti,omap3";
+
+	keys {
+		compatible = "gpio-keys";
+
+		keypad_slide {
+			label = "Keypad Slide";
+			gpios = <&gpio4 13 GPIO_ACTIVE_LOW>; /* 109 */
+			linux,input-type = <EV_SW>;
+			linux,code = <SW_KEYPAD_SLIDE>;
+			wakeup-source;
+			pinctrl-names = "default";
+			pinctrl-0 = <&keypad_slide_pins>;
+		};
+	};
+};
+
+&omap3_pmx_core {
+	keypad_slide_pins: pinmux_debug_led_pins {
+		pinctrl-single,pins = <
+			OMAP3_CORE1_IOPAD(0x212a, PIN_INPUT | MUX_MODE4)       /* cam_d10.gpio_109 */
+		>;
+	};
 };
 
 &i2c2 {
-- 
2.7.0

[PATCH 6/7] ARM: dts: N9/N950: Add support for 1GHz CPU clock

[Sebastian Reichel]

From: Filip Matijević <filip.matijevic.pz@gmail.com>

Signed-off-by: Filip Matijević <filip.matijevic.pz@gmail.com>
Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 arch/arm/boot/dts/omap3-n950-n9.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi
index c42e8fc846b9..38d2d756acdd 100644
--- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
+++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
@@ -14,6 +14,13 @@
 	cpus {
 		cpu@0 {
 			cpu0-supply = <&vcc>;
+			operating-points = <
+				/* kHz    uV */
+				300000  1012500
+				600000  1200000
+				800000  1325000
+				1000000	1375000
+			>;
 		};
 	};
 
-- 
2.7.0

[PATCH 7/7] ARM: dts: N9/N950: Add support for accelerometer

[Sebastian Reichel]

From: Filip Matijević <filip.matijevic.pz@gmail.com>

Signed-off-by: Filip Matijević <filip.matijevic.pz@gmail.com>
Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 arch/arm/boot/dts/omap3-n9.dts       | 14 ++++++++++
 arch/arm/boot/dts/omap3-n950-n9.dtsi | 54 ++++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/omap3-n950.dts     | 14 ++++++++++
 3 files changed, 82 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-n9.dts b/arch/arm/boot/dts/omap3-n9.dts
index f2e213931e09..32414707cc69 100644
--- a/arch/arm/boot/dts/omap3-n9.dts
+++ b/arch/arm/boot/dts/omap3-n9.dts
@@ -53,3 +53,17 @@
 		};
 	};
 };
+
+&lis302 {
+	st,axis-x = <1>;    /* LIS3_DEV_X */
+	st,axis-y = <(-2)>; /* LIS3_INV_DEV_Y */
+	st,axis-z = <(-3)>; /* LIS3_INV_DEV_Z */
+
+	st,min-limit-x = <(-46)>;
+	st,min-limit-y = <3>;
+	st,min-limit-z = <3>;
+
+	st,max-limit-x = <(-3)>;
+	st,max-limit-y = <46>;
+	st,max-limit-z = <46>;
+};
diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi
index 38d2d756acdd..22a7e8f7357c 100644
--- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
+++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
@@ -53,6 +53,13 @@
 };
 
 &omap3_pmx_core {
+	accelerator_pins: pinmux_accelerator_pins {
+		pinctrl-single,pins = <
+			OMAP3_CORE1_IOPAD(0x21da, PIN_INPUT | MUX_MODE4)	/* mcspi2_somi.gpio_180 -> LIS302 INT1 */
+			OMAP3_CORE1_IOPAD(0x21dc, PIN_INPUT | MUX_MODE4)	/* mcspi2_cs0.gpio_181 -> LIS302 INT2 */
+		>;
+	};
+
 	debug_leds: pinmux_debug_led_pins {
 		pinctrl-single,pins = <
 			OMAP3_CORE1_IOPAD(0x2108, PIN_OUTPUT | MUX_MODE4)       /* dss_data22.gpio_92 */
@@ -189,6 +196,53 @@
 
 &i2c3 {
 	clock-frequency = <400000>;
+
+	lis302: lis302@1d {
+		compatible = "st,lis3lv02d";
+		reg = <0x1d>;
+
+		Vdd-supply = <&vaux1>;
+		Vdd_IO-supply = <&vio>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&accelerator_pins>;
+
+                interrupts-extended = <&gpio6 20 IRQ_TYPE_EDGE_FALLING>, <&gpio6 21 IRQ_TYPE_EDGE_FALLING>; /* 180, 181 */
+
+		/* click flags */
+		st,click-single-x;
+		st,click-single-y;
+		st,click-single-z;
+
+		/* Limits are 0.5g * value */
+		st,click-threshold-x = <8>;
+		st,click-threshold-y = <8>;
+		st,click-threshold-z = <10>;
+
+		/* Click must be longer than time limit */
+		st,click-time-limit = <9>;
+
+		/* Kind of debounce filter */
+		st,click-latency = <50>;
+
+		st,wakeup-x-hi;
+		st,wakeup-y-hi;
+		st,wakeup-threshold = <(800/18)>; /* millig-value / 18 to get HW values */
+
+		st,wakeup2-z-hi;
+		st,wakeup2-threshold = <(1000/18)>; /* millig-value / 18 to get HW values */
+
+		st,highpass-cutoff-hz = <2>;
+
+		/* Interrupt line 1 for thresholds */
+		st,irq1-ff-wu-1;
+		st,irq1-ff-wu-2;
+		/* Interrupt line 2 for click detection */
+		st,irq2-click;
+
+		st,wu-duration-1 = <8>;
+		st,wu-duration-2 = <8>;
+	};
 };
 
 &mmc1 {
diff --git a/arch/arm/boot/dts/omap3-n950.dts b/arch/arm/boot/dts/omap3-n950.dts
index daa17d20ccf5..ff9544cae5af 100644
--- a/arch/arm/boot/dts/omap3-n950.dts
+++ b/arch/arm/boot/dts/omap3-n950.dts
@@ -138,3 +138,17 @@
 			 MATRIX_KEY(0x07, 0x07, KEY_R)
 			 >;
 };
+
+&lis302 {
+	st,axis-x = <(-2)>; /* LIS3_INV_DEV_Y */
+	st,axis-y = <(-1)>; /* LIS3_INV_DEV_X */
+	st,axis-z = <(-3)>; /* LIS3_INV_DEV_Z */
+
+	st,min-limit-x = <(-32)>;
+	st,min-limit-y = <3>;
+	st,min-limit-z = <3>;
+
+	st,max-limit-x = <(-3)>;
+	st,max-limit-y = <32>;
+	st,max-limit-z = <32>;
+};
-- 
2.7.0

Re: [PATCH 4/7] ARM: dts: Enable N950 keyboard sleep leds by default

[Pavel Machek]

Hi!

For 1-3 in the series, Acked-by: Pavel Machek <pavel@ucw.cz>

> Like the Nokia N900, the N950 has leds to show
> the state of sys_clkreq and sys_off_mode pins.
> 
> A detailed description for the LEDs and
> OMAP's sleep states can be found in Tony's
> commit for the Nokia N900:
> 
> c1be2032f66df9e1238bd5bc4ca666de88a62abc

I must say I've seen it on N900, and yes, it is useful, but no, I
don't think this is right.

This is not a LED. This is a interface that changes meaning of two
other LEDs. I guess it should go to debugfs somewhere.

Best regards,
								Pavel
								
> Signed-off-by: Sebastian Reichel <sre@kernel.org>

> +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
> @@ -31,9 +31,27 @@
>  		startup-delay-us = <150>;
>  		enable-active-high;
>  	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		heartbeat {
> +			label = "debug::sleep";
> +			gpios = <&gpio3 28 GPIO_ACTIVE_HIGH>;  /* gpio92 */
> +			linux,default-trigger = "default-on";
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&debug_leds>;
> +		};
> +	};
>  };
>  
>  &omap3_pmx_core {
> +	debug_leds: pinmux_debug_led_pins {
> +		pinctrl-single,pins = <
> +			OMAP3_CORE1_IOPAD(0x2108, PIN_OUTPUT | MUX_MODE4)       /* dss_data22.gpio_92 */
> +		>;
> +	};
> +
>  	mmc2_pins: pinmux_mmc2_pins {
>  		pinctrl-single,pins = <
>  			OMAP3_CORE1_IOPAD(0x2158, PIN_INPUT_PULLUP | MUX_MODE0) /* sdmmc2_clk */

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 7/7] ARM: dts: N9/N950: Add support for accelerometer

[Pavel Machek]

On Sun 2016-03-13 01:06:20, Sebastian Reichel wrote:
> From: Filip Matijević <filip.matijevic.pz@gmail.com>
> 
> Signed-off-by: Filip Matijević <filip.matijevic.pz@gmail.com>
> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> ---
>  arch/arm/boot/dts/omap3-n9.dts       | 14 ++++++++++
>  arch/arm/boot/dts/omap3-n950-n9.dtsi | 54 ++++++++++++++++++++++++++++++++++++
>  arch/arm/boot/dts/omap3-n950.dts     | 14 ++++++++++
>  3 files changed, 82 insertions(+)

5-7: Acked-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 4/7] ARM: dts: Enable N950 keyboard sleep leds by default

[Sebastian Reichel]

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

Hi,

On Tue, Mar 29, 2016 at 12:51:28PM +0200, Pavel Machek wrote:
> For 1-3 in the series, Acked-by: Pavel Machek <pavel@ucw.cz>
> 
> > Like the Nokia N900, the N950 has leds to show
> > the state of sys_clkreq and sys_off_mode pins.
> > 
> > A detailed description for the LEDs and
> > OMAP's sleep states can be found in Tony's
> > commit for the Nokia N900:
> > 
> > c1be2032f66df9e1238bd5bc4ca666de88a62abc
> 
> I must say I've seen it on N900, and yes, it is useful, but no, I
> don't think this is right.
>
> This is not a LED. This is a interface that changes meaning of two
> other LEDs. I guess it should go to debugfs somewhere.

I don't think we should diverge N900 and N950 userspace
APIs in this regard.

Actually the correct way would be a custom trigger
for the leds IMHO. I don't know if the led framework
supports per led custom triggers, though.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 4/7] ARM: dts: Enable N950 keyboard sleep leds by default

[Pavel Machek]

On Tue 2016-03-29 16:52:09, Sebastian Reichel wrote:
> Hi,
> 
> On Tue, Mar 29, 2016 at 12:51:28PM +0200, Pavel Machek wrote:
> > For 1-3 in the series, Acked-by: Pavel Machek <pavel@ucw.cz>
> > 
> > > Like the Nokia N900, the N950 has leds to show
> > > the state of sys_clkreq and sys_off_mode pins.
> > > 
> > > A detailed description for the LEDs and
> > > OMAP's sleep states can be found in Tony's
> > > commit for the Nokia N900:
> > > 
> > > c1be2032f66df9e1238bd5bc4ca666de88a62abc
> > 
> > I must say I've seen it on N900, and yes, it is useful, but no, I
> > don't think this is right.
> >
> > This is not a LED. This is a interface that changes meaning of two
> > other LEDs. I guess it should go to debugfs somewhere.
> 
> I don't think we should diverge N900 and N950 userspace
> APIs in this regard.

That's a good argument.

But maybe we should move both to debugfs somewhere before we get too
much userspace that relies on it...

> Actually the correct way would be a custom trigger
> for the leds IMHO. I don't know if the led framework
> supports per led custom triggers, though.

Well, not even custom trigger would make this easy: AFAIK you need to
enable this on two LEDs or not at all, so you'd have one trigger _that
would need to be shared over two LEDs_.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 4/7] ARM: dts: Enable N950 keyboard sleep leds by default

[Tony Lindgren]

* Sebastian Reichel <sre@kernel.org> [160329 07:53]:
> Hi,
> 
> On Tue, Mar 29, 2016 at 12:51:28PM +0200, Pavel Machek wrote:
> > For 1-3 in the series, Acked-by: Pavel Machek <pavel@ucw.cz>
> > 
> > > Like the Nokia N900, the N950 has leds to show
> > > the state of sys_clkreq and sys_off_mode pins.
> > > 
> > > A detailed description for the LEDs and
> > > OMAP's sleep states can be found in Tony's
> > > commit for the Nokia N900:
> > > 
> > > c1be2032f66df9e1238bd5bc4ca666de88a62abc
> > 
> > I must say I've seen it on N900, and yes, it is useful, but no, I
> > don't think this is right.
> >
> > This is not a LED. This is a interface that changes meaning of two
> > other LEDs. I guess it should go to debugfs somewhere.

Eh that sounds like a GPIO LED to me :) And it already has a
/sys/class/leds/debug interface.

The two LEDs this GPIO controls are hardwired to sys_clkreq and
sys_off_mode pins that are the control signals between the SoC
and PMIC.

In theory you could steal the sys_clkreq and sys_off_mode pins
from the PMIC at the cost of breaking PM. But I doubt anybody
wants to do that considering it's a battery operated device.

> I don't think we should diverge N900 and N950 userspace
> APIs in this regard.
> 
> Actually the correct way would be a custom trigger
> for the leds IMHO. I don't know if the led framework
> supports per led custom triggers, though.

Sure, there are something like 10 triggers already. And
you can already change them using:

echo none > /sys/class/leds/debug::sleep/trigger

That still does not change the fact that the LEDs trigger
based on the state of sys_clkreq and sys_off_mode.

Maybe I'm not following what you guys are trying to achieve
here though :) If so, please let me know.

Regards,

Tony

Re: [PATCH 4/7] ARM: dts: Enable N950 keyboard sleep leds by default

[Sebastian Reichel]

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

Hi,

On Wed, Mar 30, 2016 at 12:35:21PM -0700, Tony Lindgren wrote:
> > > I must say I've seen it on N900, and yes, it is useful, but no, I
> > > don't think this is right.
> > >
> > > This is not a LED. This is a interface that changes meaning of two
> > > other LEDs. I guess it should go to debugfs somewhere.
> 
> Eh that sounds like a GPIO LED to me :) And it already has a
> /sys/class/leds/debug interface.
> 
> The two LEDs this GPIO controls are hardwired to sys_clkreq and
> sys_off_mode pins that are the control signals between the SoC
> and PMIC.
> 
> In theory you could steal the sys_clkreq and sys_off_mode pins
> from the PMIC at the cost of breaking PM. But I doubt anybody
> wants to do that considering it's a battery operated device.
> 
> > I don't think we should diverge N900 and N950 userspace
> > APIs in this regard.
> > 
> > Actually the correct way would be a custom trigger
> > for the leds IMHO. I don't know if the led framework
> > supports per led custom triggers, though.
> 
> Sure, there are something like 10 triggers already. And
> you can already change them using:
> 
> echo none > /sys/class/leds/debug::sleep/trigger
> 
> That still does not change the fact that the LEDs trigger
> based on the state of sys_clkreq and sys_off_mode.
> 
> Maybe I'm not following what you guys are trying to achieve
> here though :) If so, please let me know.

If I understand the situation correctly there are two different
control methods for the same LED. I was wondering if the trigger
from lp5523:kb0 could enable the gpio. For example like this:

echo cpu-sleep-state > /sys/class/leds/lp5523:kb0/trigger

Thinking about it again supporting this requires quite some effort
with N9xx being the only affected platform. Apart from that (as
Pavel mentioned) two LEDs are affected by the GPIO, so even the
trigger is not a clean option.

So I'm fine with the current solution. IMHO moving it to debugfs
is not recommendable while keeping it default enabled (which makes
sense IMHO). At the end the correct solution is fixing the power
management. If the cpu sleeps the impact of this should be marginal.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 4/7] ARM: dts: Enable N950 keyboard sleep leds by default

[Pavel Machek]

Hi!

> > On Tue, Mar 29, 2016 at 12:51:28PM +0200, Pavel Machek wrote:
> > > For 1-3 in the series, Acked-by: Pavel Machek <pavel@ucw.cz>
> > > 
> > > > Like the Nokia N900, the N950 has leds to show
> > > > the state of sys_clkreq and sys_off_mode pins.
> > > > 
> > > > A detailed description for the LEDs and
> > > > OMAP's sleep states can be found in Tony's
> > > > commit for the Nokia N900:
> > > > 
> > > > c1be2032f66df9e1238bd5bc4ca666de88a62abc
> > > 
> > > I must say I've seen it on N900, and yes, it is useful, but no, I
> > > don't think this is right.
> > >
> > > This is not a LED. This is a interface that changes meaning of two
> > > other LEDs. I guess it should go to debugfs somewhere.
> 
> Eh that sounds like a GPIO LED to me :) And it already has a
> /sys/class/leds/debug interface.
> 
> The two LEDs this GPIO controls are hardwired to sys_clkreq and
> sys_off_mode pins that are the control signals between the SoC
> and PMIC.

No, not on N900. On N900, these LEDs are normally used for keyboard
backlight.

> In theory you could steal the sys_clkreq and sys_off_mode pins
> from the PMIC at the cost of breaking PM. But I doubt anybody
> wants to do that considering it's a battery operated device.

No. It seems that on N900, you can select whether sys_clkreq and
sys_off_mode is binary-or'ed with normal control of two keyboard
backlight LEDs.

> > I don't think we should diverge N900 and N950 userspace
> > APIs in this regard.
> > 
> > Actually the correct way would be a custom trigger
> > for the leds IMHO. I don't know if the led framework
> > supports per led custom triggers, though.
> 
> Sure, there are something like 10 triggers already. And
> you can already change them using:
> 
> echo none > /sys/class/leds/debug::sleep/trigger
> 
> That still does not change the fact that the LEDs trigger
> based on the state of sys_clkreq and sys_off_mode.
> 
> Maybe I'm not following what you guys are trying to achieve
> here though :) If so, please let me know.

...and this is what this GPIO does. So it is not exactly a LED. You
can turn it on, but than, _two_ LEDs will start blinking. You can't
control them with the brightess control. "Heartbeat" trigger is going
to be very confusing on debug::sleep.

Best regards,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 4/7] ARM: dts: Enable N950 keyboard sleep leds by default

[Tony Lindgren]

* Pavel Machek <pavel@ucw.cz> [160401 05:46]:
> Hi!
> 
> > > On Tue, Mar 29, 2016 at 12:51:28PM +0200, Pavel Machek wrote:
> > > > For 1-3 in the series, Acked-by: Pavel Machek <pavel@ucw.cz>
> > > > 
> > > > > Like the Nokia N900, the N950 has leds to show
> > > > > the state of sys_clkreq and sys_off_mode pins.
> > > > > 
> > > > > A detailed description for the LEDs and
> > > > > OMAP's sleep states can be found in Tony's
> > > > > commit for the Nokia N900:
> > > > > 
> > > > > c1be2032f66df9e1238bd5bc4ca666de88a62abc
> > > > 
> > > > I must say I've seen it on N900, and yes, it is useful, but no, I
> > > > don't think this is right.
> > > >
> > > > This is not a LED. This is a interface that changes meaning of two
> > > > other LEDs. I guess it should go to debugfs somewhere.
> > 
> > Eh that sounds like a GPIO LED to me :) And it already has a
> > /sys/class/leds/debug interface.
> > 
> > The two LEDs this GPIO controls are hardwired to sys_clkreq and
> > sys_off_mode pins that are the control signals between the SoC
> > and PMIC.
> 
> No, not on N900. On N900, these LEDs are normally used for keyboard
> backlight.

Oh I see. I've totally forgotten that as I always keep the debug
option enabled :) The extra battery consumption by those is quite
minimal and warns you if you have something hogging the CPU.

> > In theory you could steal the sys_clkreq and sys_off_mode pins
> > from the PMIC at the cost of breaking PM. But I doubt anybody
> > wants to do that considering it's a battery operated device.
> 
> No. It seems that on N900, you can select whether sys_clkreq and
> sys_off_mode is binary-or'ed with normal control of two keyboard
> backlight LEDs.

That might be doable by reconfiguring the I2C LED driver.

For the GPIO, the "default-on" just keeps the I2C LED driver
powered.

> > > I don't think we should diverge N900 and N950 userspace
> > > APIs in this regard.
> > > 
> > > Actually the correct way would be a custom trigger
> > > for the leds IMHO. I don't know if the led framework
> > > supports per led custom triggers, though.
> > 
> > Sure, there are something like 10 triggers already. And
> > you can already change them using:
> > 
> > echo none > /sys/class/leds/debug::sleep/trigger
> > 
> > That still does not change the fact that the LEDs trigger
> > based on the state of sys_clkreq and sys_off_mode.
> > 
> > Maybe I'm not following what you guys are trying to achieve
> > here though :) If so, please let me know.
> 
> ...and this is what this GPIO does. So it is not exactly a LED. You
> can turn it on, but than, _two_ LEDs will start blinking. You can't
> control them with the brightess control. "Heartbeat" trigger is going
> to be very confusing on debug::sleep.

And it occured to me that adding any other policy than
"default-on" to the GPIO LED will only work when the device
is active.

Sounds like the thing to do is to just configure the I2C LED
controller in the dts file if we don't already have that. And
assuming it has a Linux driver.

Regards,

Tony

Re: [PATCH 4/7] ARM: dts: Enable N950 keyboard sleep leds by default

[Pavel Machek]

Hi!

> > > The two LEDs this GPIO controls are hardwired to sys_clkreq and
> > > sys_off_mode pins that are the control signals between the SoC
> > > and PMIC.
> > 
> > No, not on N900. On N900, these LEDs are normally used for keyboard
> > backlight.
> 
> Oh I see. I've totally forgotten that as I always keep the debug
> option enabled :) The extra battery consumption by those is quite
> minimal and warns you if you have something hogging the CPU.

Yes, I like it.

> > ...and this is what this GPIO does. So it is not exactly a LED. You
> > can turn it on, but than, _two_ LEDs will start blinking. You can't
> > control them with the brightess control. "Heartbeat" trigger is going
> > to be very confusing on debug::sleep.
> 
> And it occured to me that adding any other policy than
> "default-on" to the GPIO LED will only work when the device
> is active.
> 
> Sounds like the thing to do is to just configure the I2C LED
> controller in the dts file if we don't already have that. And
> assuming it has a Linux driver.

I don't see what you mean here. If you want to always keep the debug
leds on... that may be a bit confusing for the users (and Pali wants
to keep kernel usable for mere mortals it seems).

Anyway, current solution is not too horrible (its wrong but it does
not hurt that much), so...

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 4/7] ARM: dts: Enable N950 keyboard sleep leds by default

[Tony Lindgren]

* Pavel Machek <pavel@ucw.cz> [160401 23:48]:
> > Sounds like the thing to do is to just configure the I2C LED
> > controller in the dts file if we don't already have that. And
> > assuming it has a Linux driver.
> 
> I don't see what you mean here. If you want to always keep the debug
> leds on... that may be a bit confusing for the users (and Pali wants
> to keep kernel usable for mere mortals it seems).

I think to make the LEDs ignore the hardware idle lines requires
reconfiguring the I2C LED controller.

> Anyway, current solution is not too horrible (its wrong but it does
> not hurt that much), so...

Yeah further changes can be done later.

Applying the whole series to omap-for-v4.7/dt with
your ack on patches 5 - 7.

Regards,

Tony