[PATCH 0/5] Add GameForce Ace

[PATCH 0/5] Add GameForce Ace

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Add support for the GameForce Ace. The GameForce Ace is an RK3588s
based gaming device with a 1080p display, touchscreen, hall effect
joysticks and triggers, 128GB of eMMC, 8GB or 12GB of RAM, WiFi 5,
and support for a 2242 NVME.

Chris Morgan (5):
  dt-bindings: power: supply: add dual-cell for cw2015
  power: supply: cw2015: Add support for dual-cell configurations
  arm64: dts: rockchip: Pull up sdio pins on RK3588
  dt-bindings: arm: rockchip: Add GameForce Ace
  arm64: dts: rockchip: Add GameForce Ace

 .../devicetree/bindings/arm/rockchip.yaml     |    5 +
 .../bindings/power/supply/cw2015_battery.yaml |    6 +
 arch/arm64/boot/dts/rockchip/Makefile         |    1 +
 .../dts/rockchip/rk3588-base-pinctrl.dtsi     |   10 +-
 .../dts/rockchip/rk3588s-gameforce-ace.dts    | 1315 +++++++++++++++++
 drivers/power/supply/cw2015_battery.c         |    7 +
 6 files changed, 1339 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3588s-gameforce-ace.dts

-- 
2.34.1

[PATCH 1/5] dt-bindings: power: supply: add dual-cell for cw2015

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

According to the datasheet for the cw2015 the device supports dual-cell
configurations. Add a new device tree property for this condition so
that the voltage values reported to userspace are correct.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 .../devicetree/bindings/power/supply/cw2015_battery.yaml    | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/cw2015_battery.yaml b/Documentation/devicetree/bindings/power/supply/cw2015_battery.yaml
index dc697b6147b2..0e7866d42cca 100644
--- a/Documentation/devicetree/bindings/power/supply/cw2015_battery.yaml
+++ b/Documentation/devicetree/bindings/power/supply/cw2015_battery.yaml
@@ -34,6 +34,12 @@ properties:
     minItems: 64
     maxItems: 64
 
+  cellwise,dual-cell:
+    description: |
+      This property specifies if the battery is used in a dual-cell series
+      configuration so that the correct voltage is presented to userspace.
+    type: boolean
+
   cellwise,monitor-interval-ms:
     description:
       Specifies the interval in milliseconds gauge values are polled at
-- 
2.34.1

[PATCH 2/5] power: supply: cw2015: Add support for dual-cell configurations

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

The Cellwise cw2015 datasheet reports that it can handle two cells
in a series configuration. Allow a device tree parameter to note
this condition so that the driver reports the correct voltage values
to userspace.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 drivers/power/supply/cw2015_battery.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/power/supply/cw2015_battery.c b/drivers/power/supply/cw2015_battery.c
index f63c3c410451..b23a6d4fa4fa 100644
--- a/drivers/power/supply/cw2015_battery.c
+++ b/drivers/power/supply/cw2015_battery.c
@@ -77,6 +77,8 @@ struct cw_battery {
 	u32 poll_interval_ms;
 	u8 alert_level;
 
+	bool dual_cell;
+
 	unsigned int read_errors;
 	unsigned int charge_stuck_cnt;
 };
@@ -325,6 +327,9 @@ static int cw_get_voltage(struct cw_battery *cw_bat)
 	 */
 	voltage_mv = avg * 312 / 1024;
 
+	if (cw_bat->dual_cell)
+		voltage_mv *= 2;
+
 	dev_dbg(cw_bat->dev, "Read voltage: %d mV, raw=0x%04x\n",
 		voltage_mv, reg_val);
 	return voltage_mv;
@@ -587,6 +592,8 @@ static int cw2015_parse_properties(struct cw_battery *cw_bat)
 			return ret;
 	}
 
+	cw_bat->dual_cell = device_property_read_bool(dev, "cellwise,dual-cell");
+
 	ret = device_property_read_u32(dev, "cellwise,monitor-interval-ms",
 				       &cw_bat->poll_interval_ms);
 	if (ret) {
-- 
2.34.1

[PATCH 3/5] arm64: dts: rockchip: Pull up sdio pins on RK3588

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

When using an Ampak derived bcm43456 on an RK3588s based GameForce Ace
the WiFi failed to work properly until I set the SDIO pins from
pull-none to pull-up. This matches the vendor kernel located at [1].
I tested this then on an RK3588s based Indiedroid Nova and did not
observe any adverse effects.

[1] https://github.com/rockchip-linux/kernel/commit/b96485b7af46a99c14f3c4818eb18c7836eb809c

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi
index 30db12c4fc82..d1368418502a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi
@@ -2449,15 +2449,15 @@ sdiom1_pins: sdiom1-pins {
 				/* sdio_clk_m1 */
 				<3 RK_PA5 2 &pcfg_pull_none>,
 				/* sdio_cmd_m1 */
-				<3 RK_PA4 2 &pcfg_pull_none>,
+				<3 RK_PA4 2 &pcfg_pull_up>,
 				/* sdio_d0_m1 */
-				<3 RK_PA0 2 &pcfg_pull_none>,
+				<3 RK_PA0 2 &pcfg_pull_up>,
 				/* sdio_d1_m1 */
-				<3 RK_PA1 2 &pcfg_pull_none>,
+				<3 RK_PA1 2 &pcfg_pull_up>,
 				/* sdio_d2_m1 */
-				<3 RK_PA2 2 &pcfg_pull_none>,
+				<3 RK_PA2 2 &pcfg_pull_up>,
 				/* sdio_d3_m1 */
-				<3 RK_PA3 2 &pcfg_pull_none>;
+				<3 RK_PA3 2 &pcfg_pull_up>;
 		};
 	};
 
-- 
2.34.1

[PATCH 4/5] dt-bindings: arm: rockchip: Add GameForce Ace

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Add devicetree binding for the GameForce Ace.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
index 1ef09fbfdfaf..a5a2621404ae 100644
--- a/Documentation/devicetree/bindings/arm/rockchip.yaml
+++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
@@ -255,6 +255,11 @@ properties:
           - const: friendlyarm,cm3588
           - const: rockchip,rk3588
 
+      - description: GameForce Ace
+        items:
+          - const: gameforce,ace
+          - const: rockchip,rk3588s
+
       - description: GameForce Chi
         items:
           - const: gameforce,chi
-- 
2.34.1

[PATCH 5/5] arm64: dts: rockchip: Add GameForce Ace

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

The GameForce Ace is a portable gaming device based on the Rockchip
RK3588s SoC.

The device contains the following hardware that is tested/working:
 - 128GB eMMC
 - SDMMC card slot
 - Ampak SDIO WiFi 5/BT
 - NVME 2242 socket
 - 8 or 12GB of RAM
 - Goodix based touchscreen
 - Stereo speakers, internal microphone, and TRRS headphone jack.
 - Dual GPIO vibrators (implemented as gpio-pwm because they are
   quite strong)
 - Multiple face buttons, dual analog joysticks, and dual analog
   triggers
 - PWM fan with tach pin.

The device also contains the following hardware that is partially or
currently not working:
 - 1920x1080 DSI display
 - HDMI port
 - USB-C port with DP alt-mode
 - TI bq25703 charger controller

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 arch/arm64/boot/dts/rockchip/Makefile         |    1 +
 .../dts/rockchip/rk3588s-gameforce-ace.dts    | 1315 +++++++++++++++++
 2 files changed, 1316 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3588s-gameforce-ace.dts

diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index fda1b980eb4b..62c3e53d24b4 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -139,6 +139,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-tiger-haikou.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-toybrick-x0.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-turing-rk1.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-coolpi-4b.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-gameforce-ace.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-indiedroid-nova.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-khadas-edge2.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-nanopi-r6s.dtb
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-gameforce-ace.dts b/arch/arm64/boot/dts/rockchip/rk3588s-gameforce-ace.dts
new file mode 100644
index 000000000000..01483444cee6
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-gameforce-ace.dts
@@ -0,0 +1,1315 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/linux-event-codes.h>
+#include <dt-bindings/leds/common.h>
+#include <dt-bindings/pinctrl/rockchip.h>
+#include <dt-bindings/pwm/pwm.h>
+#include <dt-bindings/thermal/thermal.h>
+#include <dt-bindings/usb/pd.h>
+#include "rk3588s.dtsi"
+
+/ {
+	model = "Gameforce Ace";
+	chassis-type = "handset";
+	compatible = "gameforce,ace", "rockchip,rk3588s";
+
+	aliases {
+		mmc0 = &sdhci;
+		mmc1 = &sdmmc;
+		mmc2 = &sdio;
+	};
+
+	chosen {
+		stdout-path = "serial2:1500000n8";
+	};
+
+	adc_keys: adc-keys {
+		compatible = "adc-keys";
+		io-channels = <&saradc 1>;
+		io-channel-names = "buttons";
+		keyup-threshold-microvolt = <1800000>;
+		poll-interval = <60>;
+
+		button-vol-up {
+			label = "VOLUMEUP";
+			linux,code = <KEY_VOLUMEUP>;
+			press-threshold-microvolt = <17000>;
+		};
+
+		button-vol-down {
+			label = "VOLUMEDOWN";
+			linux,code = <KEY_VOLUMEDOWN>;
+			press-threshold-microvolt = <417000>;
+		};
+	};
+
+	/* Joystick range values based on hardware observation. */
+	adc_joystick: adc-joystick {
+		compatible = "adc-joystick";
+		#address-cells = <1>;
+		io-channels = <&saradc 2>, <&saradc 3>,
+			      <&saradc 4>, <&saradc 5>;
+		poll-interval = <60>;
+		#size-cells = <0>;
+
+		axis@0 {
+			reg = <0>;
+			abs-flat = <40>;
+			abs-fuzz = <30>;
+			abs-range = <0 4095>;
+			linux,code = <ABS_RX>;
+		};
+
+		axis@1 {
+			reg = <1>;
+			abs-flat = <40>;
+			abs-fuzz = <30>;
+			abs-range = <0 4095>;
+			linux,code = <ABS_RY>;
+		};
+
+		axis@2 {
+			reg = <2>;
+			abs-flat = <40>;
+			abs-fuzz = <30>;
+			abs-range = <0 4095>;
+			linux,code = <ABS_Y>;
+		};
+
+		axis@3 {
+			reg = <3>;
+			abs-flat = <40>;
+			abs-fuzz = <30>;
+			abs-range = <0 4095>;
+			linux,code = <ABS_X>;
+		};
+	};
+
+	/* Trigger range values based on hardware observation. */
+	adc_triggers: adc-trigger {
+		compatible = "adc-joystick";
+		#address-cells = <1>;
+		io-channels = <&ti_adc 6>,
+			      <&ti_adc 7>;
+		poll-interval = <60>;
+		#size-cells = <0>;
+
+		axis@0 {
+			reg = <0>;
+			abs-flat = <15>;
+			abs-fuzz = <15>;
+			abs-range = <890 1530>;
+			linux,code = <ABS_HAT2X>;
+		};
+
+		axis@1 {
+			reg = <1>;
+			abs-flat = <15>;
+			abs-fuzz = <15>;
+			abs-range = <1010 1550>;
+			linux,code = <ABS_HAT2Y>;
+		};
+	};
+
+	amp_headphone: headphone-amplifier {
+		compatible = "simple-audio-amplifier";
+		enable-gpios = <&gpio0 RK_PD4 GPIO_ACTIVE_HIGH>;
+		pinctrl-0 = <&headphone_amplifier_en>;
+		pinctrl-names = "default";
+		sound-name-prefix = "Headphones Amplifier";
+	};
+
+	amp_speaker: speaker-amplifier {
+		compatible = "simple-audio-amplifier";
+		enable-gpios = <&gpio4 RK_PA5 GPIO_ACTIVE_HIGH>;
+		pinctrl-0 = <&speaker_amplifier_en>;
+		pinctrl-names = "default";
+		sound-name-prefix = "Speaker Amplifier";
+		VCC-supply = <&vcc5v0_spk>;
+	};
+
+	analog-sound {
+		compatible = "simple-audio-card";
+		pinctrl-0 = <&hp_detect>;
+		pinctrl-names = "default";
+		simple-audio-card,aux-devs = <&amp_headphone>, <&amp_speaker>;
+		simple-audio-card,bitclock-master = <&masterdai>;
+		simple-audio-card,format = "i2s";
+		simple-audio-card,frame-master = <&masterdai>;
+		simple-audio-card,hp-det-gpio = <&gpio3 RK_PA6 GPIO_ACTIVE_LOW>;
+		simple-audio-card,mclk-fs = <256>;
+		simple-audio-card,name = "rockchip,es8388-codec";
+		simple-audio-card,pin-switches = "Headphones", "Speaker";
+		simple-audio-card,routing =
+			"Speaker Amplifier INL", "LOUT2",
+			"Speaker Amplifier INR", "ROUT2",
+			"Speaker", "Speaker Amplifier OUTL",
+			"Speaker", "Speaker Amplifier OUTR",
+			"Headphones Amplifier INL", "LOUT1",
+			"Headphones Amplifier INR", "ROUT1",
+			"Headphones", "Headphones Amplifier OUTL",
+			"Headphones", "Headphones Amplifier OUTR",
+			"LINPUT1", "Microphone Jack",
+			"RINPUT1", "Microphone Jack",
+			"LINPUT2", "Onboard Microphone",
+			"RINPUT2", "Onboard Microphone";
+		simple-audio-card,widgets =
+			"Microphone", "Microphone Jack",
+			"Microphone", "Onboard Microphone",
+			"Headphone", "Headphones",
+			"Speaker", "Speaker";
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s0_8ch>;
+		};
+
+		masterdai: simple-audio-card,codec {
+			sound-dai = <&es8388>;
+			system-clock-frequency = <12288000>;
+		};
+	};
+
+	battery: battery {
+		compatible = "simple-battery";
+		charge-full-design-microamp-hours = <3700000>;
+		constant-charge-current-max-microamp = <2500000>;
+		constant-charge-voltage-max-microvolt = <8750000>;
+		voltage-min-design-microvolt = <7400000>;
+	};
+
+	gpio_keys: gpio-keys {
+		compatible = "gpio-keys";
+		pinctrl-0 = <&btn_pins_ctrl>;
+		pinctrl-names = "default";
+
+		button-a {
+			gpios = <&gpio1 RK_PC6 GPIO_ACTIVE_LOW>;
+			label = "EAST";
+			linux,code = <BTN_EAST>;
+		};
+
+		button-b {
+			gpios = <&gpio1 RK_PA5 GPIO_ACTIVE_LOW>;
+			label = "SOUTH";
+			linux,code = <BTN_SOUTH>;
+		};
+
+		button-down {
+			gpios = <&gpio1 RK_PB1 GPIO_ACTIVE_LOW>;
+			label = "DPAD-DOWN";
+			linux,code = <BTN_DPAD_DOWN>;
+		};
+
+		button-home {
+			gpios = <&gpio1 RK_PA0 GPIO_ACTIVE_LOW>;
+			label = "FUNCTION";
+			linux,code = <BTN_MODE>;
+		};
+
+		button-l1 {
+			gpios = <&gpio1 RK_PB5 GPIO_ACTIVE_LOW>;
+			label = "L1";
+			linux,code = <BTN_TL>;
+		};
+
+		button-left {
+			gpios = <&gpio1 RK_PD7 GPIO_ACTIVE_LOW>;
+			label = "DPAD-LEFT";
+			linux,code = <BTN_DPAD_LEFT>;
+		};
+
+		button-menu {
+			gpios = <&gpio1 RK_PB0 GPIO_ACTIVE_LOW>;
+			label = "HOME";
+			linux,code = <KEY_HOME>;
+		};
+
+		button-r1 {
+			gpios = <&gpio1 RK_PB2 GPIO_ACTIVE_LOW>;
+			label = "R1";
+			linux,code = <BTN_TR>;
+		};
+
+		button-right {
+			gpios = <&gpio1 RK_PB7 GPIO_ACTIVE_LOW>;
+			label = "DPAD-RIGHT";
+			linux,code = <BTN_DPAD_RIGHT>;
+		};
+
+		button-select {
+			gpios = <&gpio1 RK_PA3 GPIO_ACTIVE_LOW>;
+			label = "SELECT";
+			linux,code = <BTN_SELECT>;
+		};
+
+		button-start {
+			gpios = <&gpio1 RK_PB6 GPIO_ACTIVE_LOW>;
+			label = "START";
+			linux,code = <BTN_START>;
+		};
+
+		button-thumbl {
+			gpios = <&gpio1 RK_PA4 GPIO_ACTIVE_LOW>;
+			label = "THUMBL";
+			linux,code = <BTN_THUMBL>;
+		};
+
+		button-thumbr {
+			gpios = <&gpio1 RK_PD6 GPIO_ACTIVE_LOW>;
+			label = "THUMBR";
+			linux,code = <BTN_THUMBR>;
+		};
+
+		button-up {
+			gpios = <&gpio1 RK_PA2 GPIO_ACTIVE_LOW>;
+			label = "DPAD-UP";
+			linux,code = <BTN_DPAD_UP>;
+		};
+
+		button-x {
+			gpios = <&gpio1 RK_PB4 GPIO_ACTIVE_LOW>;
+			label = "NORTH";
+			linux,code = <BTN_NORTH>;
+		};
+
+		button-y {
+			gpios = <&gpio1 RK_PB3 GPIO_ACTIVE_LOW>;
+			label = "WEST";
+			linux,code = <BTN_WEST>;
+		};
+	};
+
+	gpio_leds: gpio-leds {
+		compatible = "gpio-leds";
+		pinctrl-names = "default";
+		pinctrl-0 = <&led_pins>;
+
+		green_led: led-0 {
+			color = <LED_COLOR_ID_GREEN>;
+			gpios = <&gpio3 RK_PC1 GPIO_ACTIVE_HIGH>;
+			function = LED_FUNCTION_STATUS;
+		};
+
+		red_led: led-1 {
+			color = <LED_COLOR_ID_RED>;
+			gpios = <&gpio3 RK_PC2 GPIO_ACTIVE_HIGH>;
+			function = LED_FUNCTION_CHARGING;
+		};
+	};
+
+	pwm_fan: pwm-fan {
+		compatible = "pwm-fan";
+		#cooling-cells = <2>;
+		cooling-levels = <0 120 150 180 210 240 255>;
+		fan-supply = <&vcc5v0_sys>;
+		interrupt-parent = <&gpio4>;
+		interrupts = <RK_PB2 IRQ_TYPE_EDGE_RISING>;
+		pulses-per-revolution = <4>;
+		pwms = <&pwm12 0 50000 PWM_POLARITY_INVERTED>;
+	};
+
+	pwm_gpio33: pwm-33 {
+		compatible = "pwm-gpio";
+		gpios = <&gpio1 RK_PA1 GPIO_ACTIVE_HIGH>;
+		pinctrl-0 = <&vib_right_h>;
+		pinctrl-names = "default";
+		#pwm-cells = <3>;
+	};
+
+	pwm_gpio132: pwm-132 {
+		compatible = "pwm-gpio";
+		gpios = <&gpio4 RK_PA4 GPIO_ACTIVE_HIGH>;
+		pinctrl-0 = <&vib_left_h>;
+		pinctrl-names = "default";
+		#pwm-cells = <3>;
+	};
+
+	sdio_pwrseq: sdio-pwrseq {
+		compatible = "mmc-pwrseq-simple";
+		clock-names = "ext_clock";
+		clocks = <&rtc_hym8563>;
+		pinctrl-0 = <&wifi_enable_h>;
+		pinctrl-names = "default";
+		post-power-on-delay-ms = <200>;
+		power-off-delay-us = <5000000>;
+		reset-gpios = <&gpio3 RK_PB4 GPIO_ACTIVE_LOW>;
+	};
+
+	vcc_1v1_nldo_s3: vcc-1v1-nldo-s3-regulator {
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-max-microvolt = <1100000>;
+		regulator-min-microvolt = <1100000>;
+		regulator-name = "vcc_1v1_nldo_s3";
+		vin-supply = <&vcc5v0_sys>;
+	};
+
+	vcc3v3_lcd0_n: vcc3v3-lcd0-n-regulator {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio4 RK_PA7 GPIO_ACTIVE_HIGH>;
+		pinctrl-0 = <&vcc_lcd_h>;
+		pinctrl-names = "default";
+		regulator-max-microvolt = <3300000>;
+		regulator-min-microvolt = <3300000>;
+		regulator-name = "vcc3v3_lcd0_n";
+		vin-supply = <&vcc_3v3_s3>;
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
+	};
+
+	vcc_3v3_sd_s0: vcc-3v3-sd-s0-regulator {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio4 RK_PB4 GPIO_ACTIVE_HIGH>;
+		pinctrl-0 = <&sd_s0_pwr>;
+		pinctrl-names = "default";
+		regulator-max-microvolt = <3000000>;
+		regulator-min-microvolt = <3000000>;
+		regulator-name = "vcc_3v3_sd_s0";
+		vin-supply = <&vcc_3v3_s3>;
+	};
+
+	vcc5v0_spk: vcc5v0-spk-regulator {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio4 RK_PA2 GPIO_ACTIVE_HIGH>;
+		pinctrl-0 = <&vcc5v0_spk_pwr>;
+		pinctrl-names = "default";
+		regulator-max-microvolt = <5000000>;
+		regulator-min-microvolt = <5000000>;
+		regulator-name = "vcc5v0_spk";
+		vin-supply = <&vcc5v0_sys>;
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
+	};
+
+	vcc5v0_sys: vcc5v0-sys-regulator {
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-max-microvolt = <5000000>;
+		regulator-min-microvolt = <5000000>;
+		regulator-name = "vcc5v0_sys";
+	};
+
+	vibrator_l: vibrator-l {
+		compatible = "pwm-vibrator";
+		pwm-names = "enable";
+		pwms = <&pwm_gpio132 0 20000000 0>;
+	};
+
+	vibrator_r: vibrator-r {
+		compatible = "pwm-vibrator";
+		pwm-names = "enable";
+		pwms = <&pwm_gpio33 0 20000000 0>;
+	};
+};
+
+&combphy2_psu {
+	status = "okay";
+};
+
+&cpu_l0 {
+	cpu-supply = <&vdd_cpu_lit_s0>;
+};
+
+&cpu_l1 {
+	cpu-supply = <&vdd_cpu_lit_s0>;
+};
+
+&cpu_l2 {
+	cpu-supply = <&vdd_cpu_lit_s0>;
+};
+
+&cpu_l3 {
+	cpu-supply = <&vdd_cpu_lit_s0>;
+};
+
+&cpu_b0 {
+	cpu-supply = <&vdd_cpu_big0_s0>;
+};
+
+&cpu_b1 {
+	cpu-supply = <&vdd_cpu_big0_s0>;
+};
+
+&cpu_b2 {
+	cpu-supply = <&vdd_cpu_big1_s0>;
+};
+
+&cpu_b3 {
+	cpu-supply = <&vdd_cpu_big1_s0>;
+};
+
+&gpu {
+	mali-supply = <&vdd_gpu_s0>;
+	status = "okay";
+};
+
+&i2c0 {
+	pinctrl-0 = <&i2c0m2_xfer>;
+	pinctrl-names = "default";
+	status = "okay";
+
+	vdd_cpu_big0_s0: regulator@42 {
+		compatible = "rockchip,rk8602";
+		reg = <0x42>;
+		regulator-max-microvolt = <1050000>;
+		regulator-min-microvolt = <550000>;
+		regulator-name = "vdd_cpu_big0_s0";
+		regulator-ramp-delay = <2300>;
+		fcs,suspend-voltage-selector = <1>;
+		vin-supply = <&vcc5v0_sys>;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
+	};
+
+	vdd_cpu_big1_s0: regulator@43 {
+		compatible = "rockchip,rk8603", "rockchip,rk8602";
+		reg = <0x43>;
+		regulator-max-microvolt = <1050000>;
+		regulator-min-microvolt = <550000>;
+		regulator-name = "vdd_cpu_big1_s0";
+		regulator-ramp-delay = <2300>;
+		fcs,suspend-voltage-selector = <1>;
+		vin-supply = <&vcc5v0_sys>;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
+	};
+};
+
+&i2c2 {
+	status = "okay";
+
+	vdd_npu_s0: regulator@42 {
+		compatible = "rockchip,rk8602";
+		reg = <0x42>;
+		regulator-max-microvolt = <950000>;
+		regulator-min-microvolt = <550000>;
+		regulator-name = "vdd_npu_s0";
+		regulator-ramp-delay = <2300>;
+		fcs,suspend-voltage-selector = <1>;
+		vin-supply = <&vcc5v0_sys>;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
+	};
+};
+
+&i2c3 {
+	status = "okay";
+
+	touchscreen@14 {
+		compatible = "goodix,gt911";
+		reg = <0x14>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <RK_PA6 IRQ_TYPE_LEVEL_LOW>;
+		irq-gpios = <&gpio1 RK_PA6 GPIO_ACTIVE_HIGH>;
+		pinctrl-0 = <&touch_int>, <&touch_rst>;
+		pinctrl-names = "default";
+		reset-gpios = <&gpio1 RK_PA7 GPIO_ACTIVE_HIGH>;
+		touchscreen-inverted-x;
+		touchscreen-size-x = <1080>;
+		touchscreen-size-y = <1920>;
+		touchscreen-swapped-x-y;
+	};
+};
+
+&i2c4 {
+	pinctrl-0 = <&i2c4m2_xfer>;
+	status = "okay";
+
+	ti_adc: adc@48 {
+		compatible = "ti,ads1015";
+		reg = <0x48>;
+		#address-cells = <1>;
+		#io-channel-cells = <1>;
+		#size-cells = <0>;
+
+		channel@4 {
+			reg = <4>;
+		};
+
+		channel@5 {
+			reg = <5>;
+		};
+
+		channel@6 {
+			reg = <6>;
+		};
+
+		channel@7 {
+			reg = <7>;
+		};
+	};
+
+	imu@68 {
+		compatible = "invensense,mpu6880";
+		reg = <0x68>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <RK_PD3 IRQ_TYPE_EDGE_RISING>;
+	};
+};
+
+&i2c6 {
+	pinctrl-0 = <&i2c6m3_xfer>;
+	status = "okay";
+
+	/*
+	 * vbus-supply provided by ti,bq27503 which does not yet have
+	 * a driver. Leaving unpopulated until a driver can be written.
+	 */
+	fusb302: typec-portc@22 {
+		compatible = "fcs,fusb302";
+		reg = <0x22>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <RK_PC7 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-0 = <&usbc0_int>;
+		pinctrl-names = "default";
+
+		connector {
+			compatible = "usb-c-connector";
+			data-role = "dual";
+			label = "USB-C";
+			op-sink-microwatt = <1000000>;
+			power-role = "dual";
+			self-powered;
+			sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
+				     PDO_FIXED(9000, 3000, PDO_FIXED_USB_COMM)
+				     PDO_FIXED(12000, 3000, PDO_FIXED_USB_COMM)>;
+			source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
+			try-power-role = "sink";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					usbc0_orien_sw: endpoint {
+						remote-endpoint = <&usbdp_phy0_orientation_switch>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					usbc0_role_sw: endpoint {
+						remote-endpoint = <&dwc3_0_role_switch>;
+					};
+				};
+
+				port@2 {
+					reg = <2>;
+					dp_altmode_mux: endpoint {
+						remote-endpoint = <&usbdp_phy0_dp_altmode_mux>;
+					};
+				};
+			};
+		};
+	};
+
+	rtc_hym8563: rtc@51 {
+		compatible = "haoyu,hym8563";
+		reg = <0x51>;
+		#clock-cells = <0>;
+		clock-output-names = "hym8563";
+		interrupt-parent = <&gpio0>;
+		interrupts = <RK_PB0 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-0 = <&hym8563_int>, <&clk32k_in>;
+		pinctrl-names = "default";
+		wakeup-source;
+	};
+
+	/* Battery profile from BSP device tree. */
+	cw2015@62 {
+		compatible = "cellwise,cw2015";
+		reg = <0x62>;
+
+		cellwise,battery-profile = /bits/ 8
+			<0x18 0x0A 0x76 0x6A 0x6A 0x6A 0x68 0x66
+			 0x62 0x5E 0x5A 0x58 0x5F 0x59 0x46 0x3D
+			 0x35 0x2D 0x28 0x21 0x29 0x38 0x44 0x50
+			 0x1A 0x85 0x07 0xAE 0x14 0x28 0x48 0x56
+			 0x66 0x66 0x66 0x6A 0x3E 0x1A 0x6C 0x3D
+			 0x09 0x38 0x1A 0x49 0x7B 0x96 0xA2 0x15
+			 0x3B 0x77 0x9A 0xB1 0x80 0x87 0xB0 0xCB
+			 0x2F 0x00 0x64 0xA5 0xB5 0x1C 0xF0 0x49>;
+		cellwise,dual-cell;
+		cellwise,monitor-interval-ms = <5000>;
+		monitored-battery = <&battery>;
+		power-supplies = <&fusb302>;
+		status = "okay";
+	};
+
+	/*
+	 * At 0x6b there is a Texas Instruments bq25703 for which no
+	 * driver currently exists. The physical USB-C port is wired
+	 * into the fusb302, and the power lines are run through the
+	 * bq25703 which is used to either charge the battery when the
+	 * port is used as a sink or power the port when it is used as
+	 * a source.
+	 */
+};
+
+&i2c7 {
+	status = "okay";
+
+	es8388: audio-codec@11 {
+		compatible = "everest,es8388";
+		reg = <0x11>;
+		assigned-clock-rates = <12288000>;
+		assigned-clocks = <&cru I2S0_8CH_MCLKOUT>;
+		AVDD-supply = <&vcc_3v3_s3>;
+		clocks = <&cru I2S0_8CH_MCLKOUT>;
+		DVDD-supply = <&vcc_1v8_s3>;
+		HPVDD-supply = <&vcc_3v3_s3>;
+		PVDD-supply = <&vcc_1v8_s3>;
+		#sound-dai-cells = <0>;
+	};
+};
+
+&i2s0_8ch {
+	pinctrl-0 = <&i2s0_lrck
+		     &i2s0_mclk
+		     &i2s0_sclk
+		     &i2s0_sdi0
+		     &i2s0_sdo0>;
+	status = "okay";
+};
+
+&package_thermal {
+	polling-delay = <1000>;
+
+	trips {
+		package_fan0: package-fan0 {
+			temperature = <55000>;
+			hysteresis = <2000>;
+			type = "active";
+		};
+
+		package_fan1: package-fan1 {
+			temperature = <65000>;
+			hysteresis = <2000>;
+			type = "active";
+		};
+	};
+
+	cooling-maps {
+		map1 {
+			trip = <&package_fan0>;
+			cooling-device = <&pwm_fan THERMAL_NO_LIMIT 1>;
+		};
+
+		map2 {
+			trip = <&package_fan1>;
+			cooling-device = <&pwm_fan 2 THERMAL_NO_LIMIT>;
+		};
+	};
+};
+
+/*
+ * Attempts to use an M.2 SATA in this slot worked intermittently
+ * with the correct nodes enabled in device-tree, but eventually
+ * resulted in a destroyed board. Advise caution.
+ */
+&pcie2x1l1 {
+	pinctrl-0 = <&pcie_rst>;
+	pinctrl-names = "default";
+	reset-gpios = <&gpio3 RK_PD1 GPIO_ACTIVE_HIGH>;
+	status = "okay";
+};
+
+&pinctrl {
+	audio-amplifier {
+		hp_detect: headphone-detect {
+			rockchip,pins =
+				<3 RK_PA6 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+
+		headphone_amplifier_en: headphone-amplifier-en {
+			rockchip,pins =
+				<0 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+
+		speaker_amplifier_en: speaker-amplifier-en {
+			rockchip,pins =
+				<4 RK_PA5 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	bt {
+		bt_enable_h: bt-enable-h {
+			rockchip,pins =
+				<3 RK_PB7 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+
+		bt_host_wake_l: bt-host-wake-l {
+			rockchip,pins =
+				<3 RK_PB0 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+
+		bt_wake_l: bt-wake-l {
+			rockchip,pins =
+				<3 RK_PB1 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+
+	charger {
+		charger_int_h: charger-int-h {
+			rockchip,pins =
+				<0 RK_PD5 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+
+	hym8563 {
+		hym8563_int: hym8563-int {
+			rockchip,pins =
+				<0 RK_PB0 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+
+	gpio-btns {
+		btn_pins_ctrl: btn-pins-ctrl {
+			rockchip,pins =
+				<1 RK_PA0 RK_FUNC_GPIO &pcfg_pull_up>,
+				<1 RK_PA2 RK_FUNC_GPIO &pcfg_pull_up>,
+				<1 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>,
+				<1 RK_PA4 RK_FUNC_GPIO &pcfg_pull_up>,
+				<1 RK_PA5 RK_FUNC_GPIO &pcfg_pull_up>,
+				<1 RK_PB0 RK_FUNC_GPIO &pcfg_pull_up>,
+				<1 RK_PB1 RK_FUNC_GPIO &pcfg_pull_up>,
+				<1 RK_PB2 RK_FUNC_GPIO &pcfg_pull_up>,
+				<1 RK_PB3 RK_FUNC_GPIO &pcfg_pull_up>,
+				<1 RK_PB4 RK_FUNC_GPIO &pcfg_pull_up>,
+				<1 RK_PB5 RK_FUNC_GPIO &pcfg_pull_up>,
+				<1 RK_PB6 RK_FUNC_GPIO &pcfg_pull_up>,
+				<1 RK_PB7 RK_FUNC_GPIO &pcfg_pull_up>,
+				<1 RK_PC6 RK_FUNC_GPIO &pcfg_pull_up>,
+				<1 RK_PD6 RK_FUNC_GPIO &pcfg_pull_up>,
+				<1 RK_PD7 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+
+	gpio-leds {
+		led_pins: led-pins {
+			rockchip,pins =
+				<3 RK_PC1 RK_FUNC_GPIO &pcfg_pull_up>,
+				<3 RK_PC2 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+
+	pcie-pins {
+		pcie_rst: pcie-rst {
+			rockchip,pins =
+				<3 RK_PD1 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+
+	sd-pwr {
+		sd_s0_pwr: sd-s0-pwr {
+			rockchip,pins =
+				<4 RK_PB4 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+
+	spk-pwr {
+		vcc5v0_spk_pwr: vcc5v0-spk-pwr {
+			rockchip,pins =
+				<4 RK_PA2 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+
+	touch {
+		touch_int: touch-int {
+			rockchip,pins =
+				<1 RK_PA6 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+
+		touch_rst: touch-rst {
+			rockchip,pins =
+				<1 RK_PA7 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+
+	usb-typec {
+		usbc0_int: usbc0-int {
+			rockchip,pins =
+				<0 RK_PC7 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+
+	vcc3v3-lcd {
+		vcc_lcd_h: vcc-lcd-h {
+			rockchip,pins =
+				<4 RK_PA7 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+
+	vibrator {
+		vib_left_h: vib-left-h {
+			rockchip,pins =
+				<4 RK_PA4 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+
+		vib_right_h: vib-right-h {
+			rockchip,pins =
+				<1 RK_PA1 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+	};
+
+	wifi {
+		wifi_enable_h: wifi-enable-h {
+			rockchip,pins =
+				<3 RK_PB4 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+
+		wifi_host_wake_irq: wifi-host-wake-irq {
+			rockchip,pins =
+				<0 RK_PA0 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+	};
+};
+
+&pwm12 {
+	pinctrl-0 = <&pwm12m1_pins>;
+	status = "okay";
+};
+
+&saradc {
+	vref-supply = <&vcc_1v8_s0>;
+	status = "okay";
+};
+
+&sdhci {
+	bus-width = <8>;
+	mmc-hs400-1_8v;
+	mmc-hs400-enhanced-strobe;
+	no-sd;
+	no-sdio;
+	non-removable;
+	status = "okay";
+};
+
+&sdio {
+	#address-cells = <1>;
+	bus-width = <4>;
+	cap-sd-highspeed;
+	cap-sdio-irq;
+	disable-wp;
+	keep-power-in-suspend;
+	max-frequency = <150000000>;
+	mmc-pwrseq = <&sdio_pwrseq>;
+	no-mmc;
+	no-sd;
+	sd-uhs-sdr104;
+	#size-cells = <0>;
+	status = "okay";
+
+	brcmf: wifi@1 {
+		compatible = "brcm,bcm43456-fmac", "brcm,bcm4329-fmac";
+		reg = <1>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <RK_PA0 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "host-wake";
+		pinctrl-0 = <&wifi_host_wake_irq>;
+		pinctrl-names = "default";
+	};
+};
+
+&sdmmc {
+	bus-width = <4>;
+	cap-mmc-highspeed;
+	cap-sd-highspeed;
+	disable-wp;
+	max-frequency = <150000000>;
+	no-sdio;
+	no-mmc;
+	sd-uhs-sdr104;
+	vmmc-supply = <&vcc_3v3_sd_s0>;
+	vqmmc-supply = <&vccio_sd_s0>;
+	status = "okay";
+};
+
+&spi2 {
+	#address-cells = <1>;
+	assigned-clocks = <&cru CLK_SPI2>;
+	assigned-clock-rates = <200000000>;
+	num-cs = <1>;
+	pinctrl-0 = <&spi2m2_pins>, <&spi2m2_cs0>;
+	pinctrl-names = "default";
+	#size-cells = <0>;
+	status = "okay";
+
+	pmic@0 {
+		compatible = "rockchip,rk806";
+		reg = <0x0>;
+		#gpio-cells = <2>;
+		gpio-controller;
+		interrupt-parent = <&gpio0>;
+		interrupts = <RK_PA7 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-0 = <&pmic_pins>, <&rk806_dvs1_null>,
+			    <&rk806_dvs2_null>, <&rk806_dvs3_null>;
+		pinctrl-names = "default";
+		spi-max-frequency = <1000000>;
+
+		vcc1-supply = <&vcc5v0_sys>;
+		vcc2-supply = <&vcc5v0_sys>;
+		vcc3-supply = <&vcc5v0_sys>;
+		vcc4-supply = <&vcc5v0_sys>;
+		vcc5-supply = <&vcc5v0_sys>;
+		vcc6-supply = <&vcc5v0_sys>;
+		vcc7-supply = <&vcc5v0_sys>;
+		vcc8-supply = <&vcc5v0_sys>;
+		vcc9-supply = <&vcc5v0_sys>;
+		vcc10-supply = <&vcc5v0_sys>;
+		vcc11-supply = <&vcc_2v0_pldo_s3>;
+		vcc12-supply = <&vcc5v0_sys>;
+		vcc13-supply = <&vcc_1v1_nldo_s3>;
+		vcc14-supply = <&vcc_1v1_nldo_s3>;
+		vcca-supply = <&vcc5v0_sys>;
+
+		rk806_dvs1_null: dvs1-null-pins {
+			pins = "gpio_pwrctrl1";
+			function = "pin_fun0";
+		};
+
+		rk806_dvs2_null: dvs2-null-pins {
+			pins = "gpio_pwrctrl2";
+			function = "pin_fun0";
+		};
+
+		rk806_dvs3_null: dvs3-null-pins {
+			pins = "gpio_pwrctrl3";
+			function = "pin_fun0";
+		};
+
+		regulators {
+			vdd_gpu_s0: dcdc-reg1 {
+				regulator-boot-on;
+				regulator-enable-ramp-delay = <400>;
+				regulator-max-microvolt = <950000>;
+				regulator-min-microvolt = <550000>;
+				regulator-name = "vdd_gpu_s0";
+				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_cpu_lit_s0: dcdc-reg2 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-max-microvolt = <950000>;
+				regulator-min-microvolt = <550000>;
+				regulator-ramp-delay = <12500>;
+				regulator-name = "vdd_cpu_lit_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_logic_s0: dcdc-reg3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-max-microvolt = <750000>;
+				regulator-min-microvolt = <675000>;
+				regulator-name = "vdd_logic_s0";
+				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <750000>;
+				};
+			};
+
+			vdd_vdenc_s0: dcdc-reg4 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-max-microvolt = <950000>;
+				regulator-min-microvolt = <550000>;
+				regulator-name = "vdd_vdenc_s0";
+				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_ddr_s0: dcdc-reg5 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <675000>;
+				regulator-max-microvolt = <900000>;
+				regulator-ramp-delay = <12500>;
+				regulator-name = "vdd_ddr_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+					regulator-suspend-microvolt = <850000>;
+				};
+			};
+
+			vdd2_ddr_s3: dcdc-reg6 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-name = "vdd2_ddr_s3";
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
+			};
+
+			vcc_2v0_pldo_s3: dcdc-reg7 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-max-microvolt = <2000000>;
+				regulator-min-microvolt = <2000000>;
+				regulator-name = "vdd_2v0_pldo_s3";
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <2000000>;
+				};
+			};
+
+			vcc_3v3_s3: dcdc-reg8 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-max-microvolt = <3300000>;
+				regulator-min-microvolt = <3300000>;
+				regulator-name = "vcc_3v3_s3";
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <3300000>;
+				};
+			};
+
+			vddq_ddr_s0: dcdc-reg9 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-name = "vddq_ddr_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_1v8_s3: dcdc-reg10 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-max-microvolt = <1800000>;
+				regulator-min-microvolt = <1800000>;
+				regulator-name = "vcc_1v8_s3";
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			avcc_1v8_s0: pldo-reg1 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-max-microvolt = <1800000>;
+				regulator-min-microvolt = <1800000>;
+				regulator-name = "avcc_1v8_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_1v8_s0: pldo-reg2 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-max-microvolt = <1800000>;
+				regulator-min-microvolt = <1800000>;
+				regulator-name = "vcc_1v8_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			avdd_1v2_s0: pldo-reg3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-max-microvolt = <1200000>;
+				regulator-min-microvolt = <1200000>;
+				regulator-name = "avdd_1v2_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_3v3_s0: pldo-reg4 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-max-microvolt = <3300000>;
+				regulator-min-microvolt = <3300000>;
+				regulator-name = "vcc_3v3_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vccio_sd_s0: pldo-reg5 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-max-microvolt = <3300000>;
+				regulator-min-microvolt = <1800000>;
+				regulator-name = "vccio_sd_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_1v8_s3_pldo6: pldo-reg6 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-max-microvolt = <1800000>;
+				regulator-min-microvolt = <1800000>;
+				regulator-name = "vcc_1v8_s3_pldo6";
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			vdd_0v75_s3: nldo-reg1 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-max-microvolt = <750000>;
+				regulator-min-microvolt = <750000>;
+				regulator-name = "vdd_0v75_s3";
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <750000>;
+				};
+			};
+
+			vdd_ddr_pll_s0: nldo-reg2 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-max-microvolt = <850000>;
+				regulator-min-microvolt = <850000>;
+				regulator-name = "vdd_ddr_pll_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+					regulator-suspend-microvolt = <850000>;
+				};
+			};
+
+			avdd_0v75_s0: nldo-reg3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-max-microvolt = <837500>;
+				regulator-min-microvolt = <837500>;
+				regulator-name = "avdd_0v75_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_0v85_s0: nldo-reg4 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <850000>;
+				regulator-max-microvolt = <850000>;
+				regulator-name = "vdd_0v85_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_0v75_s0: nldo-reg5 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <750000>;
+				regulator-max-microvolt = <750000>;
+				regulator-name = "vdd_0v75_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+		};
+	};
+};
+
+&tsadc {
+	status = "okay";
+};
+
+&u2phy0 {
+	status = "okay";
+};
+
+&u2phy0_otg {
+	status = "okay";
+};
+
+&uart2 {
+	pinctrl-0 = <&uart2m0_xfer>;
+	status = "okay";
+};
+
+&uart9 {
+	pinctrl-0 = <&uart9m2_xfer>, <&uart9m2_ctsn>, <&uart9m2_rtsn>;
+	uart-has-rtscts;
+	status = "okay";
+
+	bluetooth {
+		compatible = "brcm,bcm4345c5";
+		clocks = <&rtc_hym8563>;
+		clock-names = "lpo";
+		device-wakeup-gpios = <&gpio3 RK_PB1 GPIO_ACTIVE_HIGH>;
+		interrupt-parent = <&gpio3>;
+		interrupts = <RK_PB0 IRQ_TYPE_EDGE_FALLING>;
+		pinctrl-0 = <&bt_enable_h>, <&bt_host_wake_l>, <&bt_wake_l>;
+		pinctrl-names = "default";
+		shutdown-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_HIGH>;
+	};
+};
+
+&usb_host0_xhci {
+	dr_mode = "otg";
+	usb-role-switch;
+	status = "okay";
+
+	port {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		dwc3_0_role_switch: endpoint@0 {
+			reg = <0>;
+			remote-endpoint = <&usbc0_role_sw>;
+		};
+	};
+};
+
+&usbdp_phy0 {
+	mode-switch;
+	orientation-switch;
+	sbu1-dc-gpios = <&gpio4 RK_PA0 GPIO_ACTIVE_HIGH>;
+	sbu2-dc-gpios = <&gpio4 RK_PA1 GPIO_ACTIVE_HIGH>;
+	rockchip,dp-lane-mux = <2 3>;
+	status = "okay";
+
+	port {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		usbdp_phy0_orientation_switch: endpoint@0 {
+			reg = <0>;
+			remote-endpoint = <&usbc0_orien_sw>;
+		};
+
+		usbdp_phy0_dp_altmode_mux: endpoint@1 {
+			reg = <1>;
+			remote-endpoint = <&dp_altmode_mux>;
+		};
+	};
+};
-- 
2.34.1

Re: [PATCH 2/5] power: supply: cw2015: Add support for dual-cell configurations

[Sebastian Reichel]

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

Hi,

On Fri, Jul 26, 2024 at 02:49:45PM GMT, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> The Cellwise cw2015 datasheet reports that it can handle two cells
> in a series configuration. Allow a device tree parameter to note
> this condition so that the driver reports the correct voltage values
> to userspace.

I found this:

http://www.cellwise-semi.com/Public/assests/menu/20230302/6400076806706.pdf

Which says:

  CW2015 can be used in 2 or more batteries connected in series, or
  several cells connected in parallel.

So dual-cell seems like a bad property name. Instead the number of
serial cells should be provided. This property is then not really
specific to the Cellwise fuel gauge and instead a property of the
battery pack (i.e. simple-battery.yaml).

> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  drivers/power/supply/cw2015_battery.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/power/supply/cw2015_battery.c b/drivers/power/supply/cw2015_battery.c
> index f63c3c410451..b23a6d4fa4fa 100644
> --- a/drivers/power/supply/cw2015_battery.c
> +++ b/drivers/power/supply/cw2015_battery.c
> @@ -77,6 +77,8 @@ struct cw_battery {
>  	u32 poll_interval_ms;
>  	u8 alert_level;
>  
> +	bool dual_cell;
> +
>  	unsigned int read_errors;
>  	unsigned int charge_stuck_cnt;
>  };
> @@ -325,6 +327,9 @@ static int cw_get_voltage(struct cw_battery *cw_bat)
>  	 */
>  	voltage_mv = avg * 312 / 1024;
>  
> +	if (cw_bat->dual_cell)
> +		voltage_mv *= 2;

Unfortunately there are no details in the document, but this looks
very fishy. Does it only measure the first cell and hope that the
other cells have the same voltage?

This (unmerged) series also applies to your problem to some degree:

https://lore.kernel.org/all/20240416121818.543896-3-mike.looijmans@topic.nl/

-- Sebastian

>  	dev_dbg(cw_bat->dev, "Read voltage: %d mV, raw=0x%04x\n",
>  		voltage_mv, reg_val);
>  	return voltage_mv;
> @@ -587,6 +592,8 @@ static int cw2015_parse_properties(struct cw_battery *cw_bat)
>  			return ret;
>  	}
>  
> +	cw_bat->dual_cell = device_property_read_bool(dev, "cellwise,dual-cell");
> +
>  	ret = device_property_read_u32(dev, "cellwise,monitor-interval-ms",
>  				       &cw_bat->poll_interval_ms);
>  	if (ret) {
> -- 
> 2.34.1
> 
> 

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

Re: [PATCH 3/5] arm64: dts: rockchip: Pull up sdio pins on RK3588

[Heiko Stübner]

Am Freitag, 26. Juli 2024, 21:49:46 CEST schrieb Chris Morgan:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> When using an Ampak derived bcm43456 on an RK3588s based GameForce Ace
> the WiFi failed to work properly until I set the SDIO pins from
> pull-none to pull-up. This matches the vendor kernel located at [1].
> I tested this then on an RK3588s based Indiedroid Nova and did not
> observe any adverse effects.
> 
> [1] https://github.com/rockchip-linux/kernel/commit/b96485b7af46a99c14f3c4818eb18c7836eb809c

As you're essentially duplicating the change by Alex Zhao it might be
nice to keep their authorship?

So, "From" + first Signed-off from Alex, then you add a
[adapted to pinctrl filename change]
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>

below that.

> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi
> index 30db12c4fc82..d1368418502a 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi
> @@ -2449,15 +2449,15 @@ sdiom1_pins: sdiom1-pins {
>  				/* sdio_clk_m1 */
>  				<3 RK_PA5 2 &pcfg_pull_none>,
>  				/* sdio_cmd_m1 */
> -				<3 RK_PA4 2 &pcfg_pull_none>,
> +				<3 RK_PA4 2 &pcfg_pull_up>,
>  				/* sdio_d0_m1 */
> -				<3 RK_PA0 2 &pcfg_pull_none>,
> +				<3 RK_PA0 2 &pcfg_pull_up>,
>  				/* sdio_d1_m1 */
> -				<3 RK_PA1 2 &pcfg_pull_none>,
> +				<3 RK_PA1 2 &pcfg_pull_up>,
>  				/* sdio_d2_m1 */
> -				<3 RK_PA2 2 &pcfg_pull_none>,
> +				<3 RK_PA2 2 &pcfg_pull_up>,
>  				/* sdio_d3_m1 */
> -				<3 RK_PA3 2 &pcfg_pull_none>;
> +				<3 RK_PA3 2 &pcfg_pull_up>;
>  		};
>  	};
>  
> 

Re: [PATCH 4/5] dt-bindings: arm: rockchip: Add GameForce Ace

[Krzysztof Kozlowski]

On 26/07/2024 21:49, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add devicetree binding for the GameForce Ace.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
>  1 file changed, 5 insertions(+)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 0/5] Add GameForce Ace

[Rob Herring (Arm)]

On Fri, 26 Jul 2024 14:49:43 -0500, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add support for the GameForce Ace. The GameForce Ace is an RK3588s
> based gaming device with a 1080p display, touchscreen, hall effect
> joysticks and triggers, 128GB of eMMC, 8GB or 12GB of RAM, WiFi 5,
> and support for a 2242 NVME.
> 
> Chris Morgan (5):
>   dt-bindings: power: supply: add dual-cell for cw2015
>   power: supply: cw2015: Add support for dual-cell configurations
>   arm64: dts: rockchip: Pull up sdio pins on RK3588
>   dt-bindings: arm: rockchip: Add GameForce Ace
>   arm64: dts: rockchip: Add GameForce Ace
> 
>  .../devicetree/bindings/arm/rockchip.yaml     |    5 +
>  .../bindings/power/supply/cw2015_battery.yaml |    6 +
>  arch/arm64/boot/dts/rockchip/Makefile         |    1 +
>  .../dts/rockchip/rk3588-base-pinctrl.dtsi     |   10 +-
>  .../dts/rockchip/rk3588s-gameforce-ace.dts    | 1315 +++++++++++++++++
>  drivers/power/supply/cw2015_battery.c         |    7 +
>  6 files changed, 1339 insertions(+), 5 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3588s-gameforce-ace.dts
> 
> --
> 2.34.1
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y rockchip/rk3588s-gameforce-ace.dtb' for 20240726194948.109326-1-macroalpha82@gmail.com:

arch/arm64/boot/dts/rockchip/rk3588s-gameforce-ace.dtb: typec-portc@22: 'vbus-supply' is a required property
	from schema $id: http://devicetree.org/schemas/usb/fcs,fusb302.yaml#

Re: [PATCH 1/5] dt-bindings: power: supply: add dual-cell for cw2015

[Rob Herring]

On Fri, Jul 26, 2024 at 02:49:44PM -0500, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> According to the datasheet for the cw2015 the device supports dual-cell
> configurations. Add a new device tree property for this condition so
> that the voltage values reported to userspace are correct.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  .../devicetree/bindings/power/supply/cw2015_battery.yaml    | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/cw2015_battery.yaml b/Documentation/devicetree/bindings/power/supply/cw2015_battery.yaml
> index dc697b6147b2..0e7866d42cca 100644
> --- a/Documentation/devicetree/bindings/power/supply/cw2015_battery.yaml
> +++ b/Documentation/devicetree/bindings/power/supply/cw2015_battery.yaml
> @@ -34,6 +34,12 @@ properties:
>      minItems: 64
>      maxItems: 64
>  
> +  cellwise,dual-cell:
> +    description: |

Don't need '|' if no formatting.

Acked-by: Rob Herring (Arm) <robh@kernel.org>

> +      This property specifies if the battery is used in a dual-cell series
> +      configuration so that the correct voltage is presented to userspace.
> +    type: boolean
> +
>    cellwise,monitor-interval-ms:
>      description:
>        Specifies the interval in milliseconds gauge values are polled at
> -- 
> 2.34.1
> 

Re: [PATCH 3/5] arm64: dts: rockchip: Pull up sdio pins on RK3588

[Chris Morgan]

On Fri, Jul 26, 2024 at 11:42:28PM +0200, Heiko Stübner wrote:
> Am Freitag, 26. Juli 2024, 21:49:46 CEST schrieb Chris Morgan:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > When using an Ampak derived bcm43456 on an RK3588s based GameForce Ace
> > the WiFi failed to work properly until I set the SDIO pins from
> > pull-none to pull-up. This matches the vendor kernel located at [1].
> > I tested this then on an RK3588s based Indiedroid Nova and did not
> > observe any adverse effects.
> > 
> > [1] https://github.com/rockchip-linux/kernel/commit/b96485b7af46a99c14f3c4818eb18c7836eb809c
> 
> As you're essentially duplicating the change by Alex Zhao it might be
> nice to keep their authorship?
> 
> So, "From" + first Signed-off from Alex, then you add a
> [adapted to pinctrl filename change]
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> 
> below that.

Thank you, that was an oversight but I'll get it done in v2.

> 
> > 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi
> > index 30db12c4fc82..d1368418502a 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi
> > @@ -2449,15 +2449,15 @@ sdiom1_pins: sdiom1-pins {
> >  				/* sdio_clk_m1 */
> >  				<3 RK_PA5 2 &pcfg_pull_none>,
> >  				/* sdio_cmd_m1 */
> > -				<3 RK_PA4 2 &pcfg_pull_none>,
> > +				<3 RK_PA4 2 &pcfg_pull_up>,
> >  				/* sdio_d0_m1 */
> > -				<3 RK_PA0 2 &pcfg_pull_none>,
> > +				<3 RK_PA0 2 &pcfg_pull_up>,
> >  				/* sdio_d1_m1 */
> > -				<3 RK_PA1 2 &pcfg_pull_none>,
> > +				<3 RK_PA1 2 &pcfg_pull_up>,
> >  				/* sdio_d2_m1 */
> > -				<3 RK_PA2 2 &pcfg_pull_none>,
> > +				<3 RK_PA2 2 &pcfg_pull_up>,
> >  				/* sdio_d3_m1 */
> > -				<3 RK_PA3 2 &pcfg_pull_none>;
> > +				<3 RK_PA3 2 &pcfg_pull_up>;
> >  		};
> >  	};
> >  
> > 
> 
> 
> 
> 

Re: [PATCH 0/5] Add GameForce Ace

[Chris Morgan]

On Mon, Jul 29, 2024 at 12:52:50PM -0600, Rob Herring (Arm) wrote:
> 
> On Fri, 26 Jul 2024 14:49:43 -0500, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Add support for the GameForce Ace. The GameForce Ace is an RK3588s
> > based gaming device with a 1080p display, touchscreen, hall effect
> > joysticks and triggers, 128GB of eMMC, 8GB or 12GB of RAM, WiFi 5,
> > and support for a 2242 NVME.
> > 
> > Chris Morgan (5):
> >   dt-bindings: power: supply: add dual-cell for cw2015
> >   power: supply: cw2015: Add support for dual-cell configurations
> >   arm64: dts: rockchip: Pull up sdio pins on RK3588
> >   dt-bindings: arm: rockchip: Add GameForce Ace
> >   arm64: dts: rockchip: Add GameForce Ace
> > 
> >  .../devicetree/bindings/arm/rockchip.yaml     |    5 +
> >  .../bindings/power/supply/cw2015_battery.yaml |    6 +
> >  arch/arm64/boot/dts/rockchip/Makefile         |    1 +
> >  .../dts/rockchip/rk3588-base-pinctrl.dtsi     |   10 +-
> >  .../dts/rockchip/rk3588s-gameforce-ace.dts    | 1315 +++++++++++++++++
> >  drivers/power/supply/cw2015_battery.c         |    7 +
> >  6 files changed, 1339 insertions(+), 5 deletions(-)
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3588s-gameforce-ace.dts
> > 
> > --
> > 2.34.1
> > 
> > 
> > 
> 
> 
> My bot found new DTB warnings on the .dts files added or changed in this
> series.
> 
> Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
> are fixed by another series. Ultimately, it is up to the platform
> maintainer whether these warnings are acceptable or not. No need to reply
> unless the platform maintainer has comments.
> 
> If you already ran DT checks and didn't see these error(s), then
> make sure dt-schema is up to date:
> 
>   pip3 install dtschema --upgrade
> 
> 
> New warnings running 'make CHECK_DTBS=y rockchip/rk3588s-gameforce-ace.dtb' for 20240726194948.109326-1-macroalpha82@gmail.com:
> 
> arch/arm64/boot/dts/rockchip/rk3588s-gameforce-ace.dtb: typec-portc@22: 'vbus-supply' is a required property
> 	from schema $id: http://devicetree.org/schemas/usb/fcs,fusb302.yaml#
> 

For the moment this is expected, because the regulator is driven by a
TI BQ25703 chip that I'm still working on getting supported. Is it
preferable to either keep it empty (and cause this error) or fill it
with a dummy regulator for now?

Thank you.

> 
> 
> 
> 

Re: [PATCH 2/5] power: supply: cw2015: Add support for dual-cell configurations

[Chris Morgan]

On Fri, Jul 26, 2024 at 11:06:21PM +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, Jul 26, 2024 at 02:49:45PM GMT, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > The Cellwise cw2015 datasheet reports that it can handle two cells
> > in a series configuration. Allow a device tree parameter to note
> > this condition so that the driver reports the correct voltage values
> > to userspace.
> 
> I found this:
> 
> http://www.cellwise-semi.com/Public/assests/menu/20230302/6400076806706.pdf
> 
> Which says:
> 
>   CW2015 can be used in 2 or more batteries connected in series, or
>   several cells connected in parallel.
> 
> So dual-cell seems like a bad property name. Instead the number of
> serial cells should be provided. This property is then not really
> specific to the Cellwise fuel gauge and instead a property of the
> battery pack (i.e. simple-battery.yaml).

It's conflicting information (which further confuses me). I see in that
datasheet it says 2 or more, whereas the datasheet found here says only
2 cells:

https://www.lestat.st/_media/informatique/projets/python-cw2015/cw2015-power-management-datasheet.pdf

But I agree in principle that we should be setting this as a property
of a simple-battery rather than a manufacturer specific parameter.

> 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> >  drivers/power/supply/cw2015_battery.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/power/supply/cw2015_battery.c b/drivers/power/supply/cw2015_battery.c
> > index f63c3c410451..b23a6d4fa4fa 100644
> > --- a/drivers/power/supply/cw2015_battery.c
> > +++ b/drivers/power/supply/cw2015_battery.c
> > @@ -77,6 +77,8 @@ struct cw_battery {
> >  	u32 poll_interval_ms;
> >  	u8 alert_level;
> >  
> > +	bool dual_cell;
> > +
> >  	unsigned int read_errors;
> >  	unsigned int charge_stuck_cnt;
> >  };
> > @@ -325,6 +327,9 @@ static int cw_get_voltage(struct cw_battery *cw_bat)
> >  	 */
> >  	voltage_mv = avg * 312 / 1024;
> >  
> > +	if (cw_bat->dual_cell)
> > +		voltage_mv *= 2;
> 
> Unfortunately there are no details in the document, but this looks
> very fishy. Does it only measure the first cell and hope that the
> other cells have the same voltage?
> 
> This (unmerged) series also applies to your problem to some degree:
> 
> https://lore.kernel.org/all/20240416121818.543896-3-mike.looijmans@topic.nl/

I think based on the application diagram it is in fact measuring the
cell voltage. That said, this ultimately boils down to a cosmetic thing
as not having this property simply means userspace sees the battery
voltage as 3.8v instead of 7.6v as is written on the side.

I think for simplification sake I will remove this from the series, add
a note to the device tree, and wait for the other battery series to get
pulled. When it gets pulled I'll request a device tree property so we
can add POWER_SUPPLY_PROP_NUMBER_OF_SERIAL_CELLS to the simple-battery
and then parse this value. Or if that series ends up getting abandoned
I can just add a quick and dirty new property like this.

Thank you,
Chris

> 
> -- Sebastian
> 
> >  	dev_dbg(cw_bat->dev, "Read voltage: %d mV, raw=0x%04x\n",
> >  		voltage_mv, reg_val);
> >  	return voltage_mv;
> > @@ -587,6 +592,8 @@ static int cw2015_parse_properties(struct cw_battery *cw_bat)
> >  			return ret;
> >  	}
> >  
> > +	cw_bat->dual_cell = device_property_read_bool(dev, "cellwise,dual-cell");
> > +
> >  	ret = device_property_read_u32(dev, "cellwise,monitor-interval-ms",
> >  				       &cw_bat->poll_interval_ms);
> >  	if (ret) {
> > -- 
> > 2.34.1
> > 
> > 

Re: [PATCH 2/5] power: supply: cw2015: Add support for dual-cell configurations

[Sebastian Reichel]

Hi,

On Wed, Jul 31, 2024 at 11:02:11AM GMT, Chris Morgan wrote:
> On Fri, Jul 26, 2024 at 11:06:21PM +0200, Sebastian Reichel wrote:
> > Hi,
> > 
> > On Fri, Jul 26, 2024 at 02:49:45PM GMT, Chris Morgan wrote:
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > > 
> > > The Cellwise cw2015 datasheet reports that it can handle two cells
> > > in a series configuration. Allow a device tree parameter to note
> > > this condition so that the driver reports the correct voltage values
> > > to userspace.
> > 
> > I found this:
> > 
> > http://www.cellwise-semi.com/Public/assests/menu/20230302/6400076806706.pdf
> > 
> > Which says:
> > 
> >   CW2015 can be used in 2 or more batteries connected in series, or
> >   several cells connected in parallel.
> > 
> > So dual-cell seems like a bad property name. Instead the number of
> > serial cells should be provided. This property is then not really
> > specific to the Cellwise fuel gauge and instead a property of the
> > battery pack (i.e. simple-battery.yaml).
> 
> It's conflicting information (which further confuses me). I see in that
> datasheet it says 2 or more, whereas the datasheet found here says only
> 2 cells:
> 
> https://www.lestat.st/_media/informatique/projets/python-cw2015/cw2015-power-management-datasheet.pdf
> 
> But I agree in principle that we should be setting this as a property
> of a simple-battery rather than a manufacturer specific parameter.
> 
> > 
> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > ---
> > >  drivers/power/supply/cw2015_battery.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/drivers/power/supply/cw2015_battery.c b/drivers/power/supply/cw2015_battery.c
> > > index f63c3c410451..b23a6d4fa4fa 100644
> > > --- a/drivers/power/supply/cw2015_battery.c
> > > +++ b/drivers/power/supply/cw2015_battery.c
> > > @@ -77,6 +77,8 @@ struct cw_battery {
> > >  	u32 poll_interval_ms;
> > >  	u8 alert_level;
> > >  
> > > +	bool dual_cell;
> > > +
> > >  	unsigned int read_errors;
> > >  	unsigned int charge_stuck_cnt;
> > >  };
> > > @@ -325,6 +327,9 @@ static int cw_get_voltage(struct cw_battery *cw_bat)
> > >  	 */
> > >  	voltage_mv = avg * 312 / 1024;
> > >  
> > > +	if (cw_bat->dual_cell)
> > > +		voltage_mv *= 2;
> > 
> > Unfortunately there are no details in the document, but this looks
> > very fishy. Does it only measure the first cell and hope that the
> > other cells have the same voltage?
> > 
> > This (unmerged) series also applies to your problem to some degree:
> > 
> > https://lore.kernel.org/all/20240416121818.543896-3-mike.looijmans@topic.nl/
> 
> I think based on the application diagram it is in fact measuring the
> cell voltage.
>
> That said, this ultimately boils down to a cosmetic thing
> as not having this property simply means userspace sees the battery
> voltage as 3.8v instead of 7.6v as is written on the side.

With the cells being connected in serial, the voltage of both cells
can be different. There is not "the cell voltage". Instead there is
a voltage for cell 1 and a voltage for cell 2. In a perfect battery
they are the same, but in reality they are not. In the extreme case
one of the cells may be broken while the other is still fine. It
sounds as if this is just measuring the voltage from the first
cell and assumes the second cell has the same voltage.

Ideally the voltage on these platforms is not exposed via the normal
VOLTAGE property and instead uses a new property for telling
userspace the voltage for a single cell. The kernel simply does not
know the voltage of the whole battery pack.

FWIW this is the worst battery measurement system I've seen so far
if my understanding of the hardware design is correct.

-- Sebastian

> I think for simplification sake I will remove this from the series, add
> a note to the device tree, and wait for the other battery series to get
> pulled. When it gets pulled I'll request a device tree property so we
> can add POWER_SUPPLY_PROP_NUMBER_OF_SERIAL_CELLS to the simple-battery
> and then parse this value. Or if that series ends up getting abandoned
> I can just add a quick and dirty new property like this.


> 
> Thank you,
> Chris
> 
> > 
> > -- Sebastian
> > 
> > >  	dev_dbg(cw_bat->dev, "Read voltage: %d mV, raw=0x%04x\n",
> > >  		voltage_mv, reg_val);
> > >  	return voltage_mv;
> > > @@ -587,6 +592,8 @@ static int cw2015_parse_properties(struct cw_battery *cw_bat)
> > >  			return ret;
> > >  	}
> > >  
> > > +	cw_bat->dual_cell = device_property_read_bool(dev, "cellwise,dual-cell");
> > > +
> > >  	ret = device_property_read_u32(dev, "cellwise,monitor-interval-ms",
> > >  				       &cw_bat->poll_interval_ms);
> > >  	if (ret) {
> > > -- 
> > > 2.34.1
> > > 
> > > 
> 
> 

Re: [PATCH 2/5] power: supply: cw2015: Add support for dual-cell configurations

[Chris Morgan]

On Wed, Jul 31, 2024 at 07:02:28PM +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Wed, Jul 31, 2024 at 11:02:11AM GMT, Chris Morgan wrote:
> > On Fri, Jul 26, 2024 at 11:06:21PM +0200, Sebastian Reichel wrote:
> > > Hi,
> > > 
> > > On Fri, Jul 26, 2024 at 02:49:45PM GMT, Chris Morgan wrote:
> > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > 
> > > > The Cellwise cw2015 datasheet reports that it can handle two cells
> > > > in a series configuration. Allow a device tree parameter to note
> > > > this condition so that the driver reports the correct voltage values
> > > > to userspace.
> > > 
> > > I found this:
> > > 
> > > http://www.cellwise-semi.com/Public/assests/menu/20230302/6400076806706.pdf
> > > 
> > > Which says:
> > > 
> > >   CW2015 can be used in 2 or more batteries connected in series, or
> > >   several cells connected in parallel.
> > > 
> > > So dual-cell seems like a bad property name. Instead the number of
> > > serial cells should be provided. This property is then not really
> > > specific to the Cellwise fuel gauge and instead a property of the
> > > battery pack (i.e. simple-battery.yaml).
> > 
> > It's conflicting information (which further confuses me). I see in that
> > datasheet it says 2 or more, whereas the datasheet found here says only
> > 2 cells:
> > 
> > https://www.lestat.st/_media/informatique/projets/python-cw2015/cw2015-power-management-datasheet.pdf
> > 
> > But I agree in principle that we should be setting this as a property
> > of a simple-battery rather than a manufacturer specific parameter.
> > 
> > > 
> > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > ---
> > > >  drivers/power/supply/cw2015_battery.c | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > > 
> > > > diff --git a/drivers/power/supply/cw2015_battery.c b/drivers/power/supply/cw2015_battery.c
> > > > index f63c3c410451..b23a6d4fa4fa 100644
> > > > --- a/drivers/power/supply/cw2015_battery.c
> > > > +++ b/drivers/power/supply/cw2015_battery.c
> > > > @@ -77,6 +77,8 @@ struct cw_battery {
> > > >  	u32 poll_interval_ms;
> > > >  	u8 alert_level;
> > > >  
> > > > +	bool dual_cell;
> > > > +
> > > >  	unsigned int read_errors;
> > > >  	unsigned int charge_stuck_cnt;
> > > >  };
> > > > @@ -325,6 +327,9 @@ static int cw_get_voltage(struct cw_battery *cw_bat)
> > > >  	 */
> > > >  	voltage_mv = avg * 312 / 1024;
> > > >  
> > > > +	if (cw_bat->dual_cell)
> > > > +		voltage_mv *= 2;
> > > 
> > > Unfortunately there are no details in the document, but this looks
> > > very fishy. Does it only measure the first cell and hope that the
> > > other cells have the same voltage?
> > > 
> > > This (unmerged) series also applies to your problem to some degree:
> > > 
> > > https://lore.kernel.org/all/20240416121818.543896-3-mike.looijmans@topic.nl/
> > 
> > I think based on the application diagram it is in fact measuring the
> > cell voltage.
> >
> > That said, this ultimately boils down to a cosmetic thing
> > as not having this property simply means userspace sees the battery
> > voltage as 3.8v instead of 7.6v as is written on the side.
> 
> With the cells being connected in serial, the voltage of both cells
> can be different. There is not "the cell voltage". Instead there is
> a voltage for cell 1 and a voltage for cell 2. In a perfect battery
> they are the same, but in reality they are not. In the extreme case
> one of the cells may be broken while the other is still fine. It
> sounds as if this is just measuring the voltage from the first
> cell and assumes the second cell has the same voltage.
> 
> Ideally the voltage on these platforms is not exposed via the normal
> VOLTAGE property and instead uses a new property for telling
> userspace the voltage for a single cell. The kernel simply does not
> know the voltage of the whole battery pack.
> 
> FWIW this is the worst battery measurement system I've seen so far
> if my understanding of the hardware design is correct.
> 
> -- Sebastian
> 

Don't get me started, this whole stack is a nightmare. The battery
won't charge at all without a bq25703 driver (so this current DTS
basically has the device running while plugged in); and for some
reason the fcs,fc302 chip wants to go interrupt crazy if you're not
using a PD charger. Part of the reason of getting this upstreamed
is so I can have a common base for others to help me get this last
bit of nonsense sorted out. Displaying the correct voltage on the
battery however isn't critical, as the device will still report the
correct SoC and will even charge and function just fine when a driver
is in place for the bq25703. It's just the voltage reported in sysfs
will be wrong.

Thank you,
Chris

> > I think for simplification sake I will remove this from the series, add
> > a note to the device tree, and wait for the other battery series to get
> > pulled. When it gets pulled I'll request a device tree property so we
> > can add POWER_SUPPLY_PROP_NUMBER_OF_SERIAL_CELLS to the simple-battery
> > and then parse this value. Or if that series ends up getting abandoned
> > I can just add a quick and dirty new property like this.
> 
> 
> > 
> > Thank you,
> > Chris
> > 
> > > 
> > > -- Sebastian
> > > 
> > > >  	dev_dbg(cw_bat->dev, "Read voltage: %d mV, raw=0x%04x\n",
> > > >  		voltage_mv, reg_val);
> > > >  	return voltage_mv;
> > > > @@ -587,6 +592,8 @@ static int cw2015_parse_properties(struct cw_battery *cw_bat)
> > > >  			return ret;
> > > >  	}
> > > >  
> > > > +	cw_bat->dual_cell = device_property_read_bool(dev, "cellwise,dual-cell");
> > > > +
> > > >  	ret = device_property_read_u32(dev, "cellwise,monitor-interval-ms",
> > > >  				       &cw_bat->poll_interval_ms);
> > > >  	if (ret) {
> > > > -- 
> > > > 2.34.1
> > > > 
> > > > 
> > 
> > 

Re: [PATCH 2/5] power: supply: cw2015: Add support for dual-cell configurations

[Dragan Simic]

Hello all,

On 2024-07-31 19:02, Sebastian Reichel wrote:
> On Wed, Jul 31, 2024 at 11:02:11AM GMT, Chris Morgan wrote:
>> On Fri, Jul 26, 2024 at 11:06:21PM +0200, Sebastian Reichel wrote:
>> > On Fri, Jul 26, 2024 at 02:49:45PM GMT, Chris Morgan wrote:
>> > > From: Chris Morgan <macromorgan@hotmail.com>
>> > >
>> > > The Cellwise cw2015 datasheet reports that it can handle two cells
>> > > in a series configuration. Allow a device tree parameter to note
>> > > this condition so that the driver reports the correct voltage values
>> > > to userspace.
>> >
>> > I found this:
>> >
>> > http://www.cellwise-semi.com/Public/assests/menu/20230302/6400076806706.pdf
>> >
>> > Which says:
>> >
>> >   CW2015 can be used in 2 or more batteries connected in series, or
>> >   several cells connected in parallel.
>> >
>> > So dual-cell seems like a bad property name. Instead the number of
>> > serial cells should be provided. This property is then not really
>> > specific to the Cellwise fuel gauge and instead a property of the
>> > battery pack (i.e. simple-battery.yaml).
>> 
>> It's conflicting information (which further confuses me). I see in 
>> that
>> datasheet it says 2 or more, whereas the datasheet found here says 
>> only
>> 2 cells:
>> 
>> https://www.lestat.st/_media/informatique/projets/python-cw2015/cw2015-power-management-datasheet.pdf
>> 
>> But I agree in principle that we should be setting this as a property
>> of a simple-battery rather than a manufacturer specific parameter.
>> 
>> >
>> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>> > > ---
>> > >  drivers/power/supply/cw2015_battery.c | 7 +++++++
>> > >  1 file changed, 7 insertions(+)
>> > >
>> > > diff --git a/drivers/power/supply/cw2015_battery.c b/drivers/power/supply/cw2015_battery.c
>> > > index f63c3c410451..b23a6d4fa4fa 100644
>> > > --- a/drivers/power/supply/cw2015_battery.c
>> > > +++ b/drivers/power/supply/cw2015_battery.c
>> > > @@ -77,6 +77,8 @@ struct cw_battery {
>> > >  	u32 poll_interval_ms;
>> > >  	u8 alert_level;
>> > >
>> > > +	bool dual_cell;
>> > > +
>> > >  	unsigned int read_errors;
>> > >  	unsigned int charge_stuck_cnt;
>> > >  };
>> > > @@ -325,6 +327,9 @@ static int cw_get_voltage(struct cw_battery *cw_bat)
>> > >  	 */
>> > >  	voltage_mv = avg * 312 / 1024;
>> > >
>> > > +	if (cw_bat->dual_cell)
>> > > +		voltage_mv *= 2;
>> >
>> > Unfortunately there are no details in the document, but this looks
>> > very fishy. Does it only measure the first cell and hope that the
>> > other cells have the same voltage?
>> >
>> > This (unmerged) series also applies to your problem to some degree:
>> >
>> > https://lore.kernel.org/all/20240416121818.543896-3-mike.looijmans@topic.nl/
>> 
>> I think based on the application diagram it is in fact measuring the
>> cell voltage.
>> 
>> That said, this ultimately boils down to a cosmetic thing
>> as not having this property simply means userspace sees the battery
>> voltage as 3.8v instead of 7.6v as is written on the side.
> 
> With the cells being connected in serial, the voltage of both cells
> can be different. There is not "the cell voltage". Instead there is
> a voltage for cell 1 and a voltage for cell 2. In a perfect battery
> they are the same, but in reality they are not. In the extreme case
> one of the cells may be broken while the other is still fine. It
> sounds as if this is just measuring the voltage from the first
> cell and assumes the second cell has the same voltage.
> 
> Ideally the voltage on these platforms is not exposed via the normal
> VOLTAGE property and instead uses a new property for telling
> userspace the voltage for a single cell. The kernel simply does not
> know the voltage of the whole battery pack.
> 
> FWIW this is the worst battery measurement system I've seen so far
> if my understanding of the hardware design is correct.

Please note that two facts should be considered here:

  - The GenBook schematic [1] clearly shows that the individual battery
    cells aren't exposed at its internal battery connector and, as a
    result, aren't available for individual cell voltage monitoring

  - The GenBook uses a CW2013 as it fuel gauge, [1] instead of CW2015
    as mentioned here a few times, but I haven't went through the CW2013
    datasheet(s) yet to see what are the actual differences between it
    and the CW2015

[1] https://wiki.cool-pi.com/notebook/coolpi-genbook-v20.pdf

>> I think for simplification sake I will remove this from the series, 
>> add
>> a note to the device tree, and wait for the other battery series to 
>> get
>> pulled. When it gets pulled I'll request a device tree property so we
>> can add POWER_SUPPLY_PROP_NUMBER_OF_SERIAL_CELLS to the simple-battery
>> and then parse this value. Or if that series ends up getting abandoned
>> I can just add a quick and dirty new property like this.
>> >
>> > >  	dev_dbg(cw_bat->dev, "Read voltage: %d mV, raw=0x%04x\n",
>> > >  		voltage_mv, reg_val);
>> > >  	return voltage_mv;
>> > > @@ -587,6 +592,8 @@ static int cw2015_parse_properties(struct cw_battery *cw_bat)
>> > >  			return ret;
>> > >  	}
>> > >
>> > > +	cw_bat->dual_cell = device_property_read_bool(dev, "cellwise,dual-cell");
>> > > +
>> > >  	ret = device_property_read_u32(dev, "cellwise,monitor-interval-ms",
>> > >  				       &cw_bat->poll_interval_ms);
>> > >  	if (ret) {
>> > > --
>> > > 2.34.1

Re: [PATCH 2/5] power: supply: cw2015: Add support for dual-cell configurations

[Sebastian Reichel]

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

Hi,

On Fri, Aug 02, 2024 at 11:39:24PM GMT, Dragan Simic wrote:
> On 2024-07-31 19:02, Sebastian Reichel wrote:
> > On Wed, Jul 31, 2024 at 11:02:11AM GMT, Chris Morgan wrote:
> > > On Fri, Jul 26, 2024 at 11:06:21PM +0200, Sebastian Reichel wrote:
> > > > On Fri, Jul 26, 2024 at 02:49:45PM GMT, Chris Morgan wrote:
> > > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > >
> > > > > The Cellwise cw2015 datasheet reports that it can handle two cells
> > > > > in a series configuration. Allow a device tree parameter to note
> > > > > this condition so that the driver reports the correct voltage values
> > > > > to userspace.
> > > >
> > > > I found this:
> > > >
> > > > http://www.cellwise-semi.com/Public/assests/menu/20230302/6400076806706.pdf
> > > >
> > > > Which says:
> > > >
> > > >   CW2015 can be used in 2 or more batteries connected in series, or
> > > >   several cells connected in parallel.
> > > >
> > > > So dual-cell seems like a bad property name. Instead the number of
> > > > serial cells should be provided. This property is then not really
> > > > specific to the Cellwise fuel gauge and instead a property of the
> > > > battery pack (i.e. simple-battery.yaml).
> > > 
> > > It's conflicting information (which further confuses me). I see in
> > > that
> > > datasheet it says 2 or more, whereas the datasheet found here says
> > > only
> > > 2 cells:
> > > 
> > > https://www.lestat.st/_media/informatique/projets/python-cw2015/cw2015-power-management-datasheet.pdf
> > > 
> > > But I agree in principle that we should be setting this as a property
> > > of a simple-battery rather than a manufacturer specific parameter.
> > > 
> > > >
> > > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > > ---
> > > > >  drivers/power/supply/cw2015_battery.c | 7 +++++++
> > > > >  1 file changed, 7 insertions(+)
> > > > >
> > > > > diff --git a/drivers/power/supply/cw2015_battery.c b/drivers/power/supply/cw2015_battery.c
> > > > > index f63c3c410451..b23a6d4fa4fa 100644
> > > > > --- a/drivers/power/supply/cw2015_battery.c
> > > > > +++ b/drivers/power/supply/cw2015_battery.c
> > > > > @@ -77,6 +77,8 @@ struct cw_battery {
> > > > >  	u32 poll_interval_ms;
> > > > >  	u8 alert_level;
> > > > >
> > > > > +	bool dual_cell;
> > > > > +
> > > > >  	unsigned int read_errors;
> > > > >  	unsigned int charge_stuck_cnt;
> > > > >  };
> > > > > @@ -325,6 +327,9 @@ static int cw_get_voltage(struct cw_battery *cw_bat)
> > > > >  	 */
> > > > >  	voltage_mv = avg * 312 / 1024;
> > > > >
> > > > > +	if (cw_bat->dual_cell)
> > > > > +		voltage_mv *= 2;
> > > >
> > > > Unfortunately there are no details in the document, but this looks
> > > > very fishy. Does it only measure the first cell and hope that the
> > > > other cells have the same voltage?
> > > >
> > > > This (unmerged) series also applies to your problem to some degree:
> > > >
> > > > https://lore.kernel.org/all/20240416121818.543896-3-mike.looijmans@topic.nl/
> > > 
> > > I think based on the application diagram it is in fact measuring the
> > > cell voltage.
> > > 
> > > That said, this ultimately boils down to a cosmetic thing
> > > as not having this property simply means userspace sees the battery
> > > voltage as 3.8v instead of 7.6v as is written on the side.
> > 
> > With the cells being connected in serial, the voltage of both cells
> > can be different. There is not "the cell voltage". Instead there is
> > a voltage for cell 1 and a voltage for cell 2. In a perfect battery
> > they are the same, but in reality they are not. In the extreme case
> > one of the cells may be broken while the other is still fine. It
> > sounds as if this is just measuring the voltage from the first
> > cell and assumes the second cell has the same voltage.
> > 
> > Ideally the voltage on these platforms is not exposed via the normal
> > VOLTAGE property and instead uses a new property for telling
> > userspace the voltage for a single cell. The kernel simply does not
> > know the voltage of the whole battery pack.
> > 
> > FWIW this is the worst battery measurement system I've seen so far
> > if my understanding of the hardware design is correct.
> 
> Please note that two facts should be considered here:
> 
>  - The GenBook schematic [1] clearly shows that the individual battery
>    cells aren't exposed at its internal battery connector and, as a
>    result, aren't available for individual cell voltage monitoring
> 
>  - The GenBook uses a CW2013 as it fuel gauge, [1] instead of CW2015
>    as mentioned here a few times, but I haven't went through the CW2013
>    datasheet(s) yet to see what are the actual differences between it
>    and the CW2015
> 
> [1] https://wiki.cool-pi.com/notebook/coolpi-genbook-v20.pdf

Ah, thanks for pointing to the schematics. So the fuel gauge can
only measure the voltage of the whole battery, but VCELL register
provides the voltage for 1 cell? What is the origin of the dual-cell
property? Was this something you came up with yourself when noticing
the wrong voltage?

Based on the above information my guess would be that CW2013 uses a
different voltage resolution than CW2015 for the VCELL register. The
datasheet for CW2015 says 14bit ADC with 305uV resolution. Maybe
the CW2013 simply uses a different ADC. Do you have the datasheet
for the chip?

-- Sebastian

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

Re: [PATCH 2/5] power: supply: cw2015: Add support for dual-cell configurations

[Dragan Simic]

Hello Sebastian,

(I'm adding a couple of Cool Pi contacts to the Cc list.)

On 2024-08-04 18:29, Sebastian Reichel wrote:
> On Fri, Aug 02, 2024 at 11:39:24PM GMT, Dragan Simic wrote:
>> On 2024-07-31 19:02, Sebastian Reichel wrote:
>> > On Wed, Jul 31, 2024 at 11:02:11AM GMT, Chris Morgan wrote:
>> > > On Fri, Jul 26, 2024 at 11:06:21PM +0200, Sebastian Reichel wrote:
>> > > > On Fri, Jul 26, 2024 at 02:49:45PM GMT, Chris Morgan wrote:
>> > > > > From: Chris Morgan <macromorgan@hotmail.com>
>> > > > >
>> > > > > The Cellwise cw2015 datasheet reports that it can handle two cells
>> > > > > in a series configuration. Allow a device tree parameter to note
>> > > > > this condition so that the driver reports the correct voltage values
>> > > > > to userspace.
>> > > >
>> > > > I found this:
>> > > >
>> > > > http://www.cellwise-semi.com/Public/assests/menu/20230302/6400076806706.pdf
>> > > >
>> > > > Which says:
>> > > >
>> > > >   CW2015 can be used in 2 or more batteries connected in series, or
>> > > >   several cells connected in parallel.
>> > > >
>> > > > So dual-cell seems like a bad property name. Instead the number of
>> > > > serial cells should be provided. This property is then not really
>> > > > specific to the Cellwise fuel gauge and instead a property of the
>> > > > battery pack (i.e. simple-battery.yaml).
>> > >
>> > > It's conflicting information (which further confuses me). I see in
>> > > that
>> > > datasheet it says 2 or more, whereas the datasheet found here says
>> > > only
>> > > 2 cells:
>> > >
>> > > https://www.lestat.st/_media/informatique/projets/python-cw2015/cw2015-power-management-datasheet.pdf
>> > >
>> > > But I agree in principle that we should be setting this as a property
>> > > of a simple-battery rather than a manufacturer specific parameter.
>> > >
>> > > >
>> > > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>> > > > > ---
>> > > > >  drivers/power/supply/cw2015_battery.c | 7 +++++++
>> > > > >  1 file changed, 7 insertions(+)
>> > > > >
>> > > > > diff --git a/drivers/power/supply/cw2015_battery.c b/drivers/power/supply/cw2015_battery.c
>> > > > > index f63c3c410451..b23a6d4fa4fa 100644
>> > > > > --- a/drivers/power/supply/cw2015_battery.c
>> > > > > +++ b/drivers/power/supply/cw2015_battery.c
>> > > > > @@ -77,6 +77,8 @@ struct cw_battery {
>> > > > >  	u32 poll_interval_ms;
>> > > > >  	u8 alert_level;
>> > > > >
>> > > > > +	bool dual_cell;
>> > > > > +
>> > > > >  	unsigned int read_errors;
>> > > > >  	unsigned int charge_stuck_cnt;
>> > > > >  };
>> > > > > @@ -325,6 +327,9 @@ static int cw_get_voltage(struct cw_battery *cw_bat)
>> > > > >  	 */
>> > > > >  	voltage_mv = avg * 312 / 1024;
>> > > > >
>> > > > > +	if (cw_bat->dual_cell)
>> > > > > +		voltage_mv *= 2;
>> > > >
>> > > > Unfortunately there are no details in the document, but this looks
>> > > > very fishy. Does it only measure the first cell and hope that the
>> > > > other cells have the same voltage?
>> > > >
>> > > > This (unmerged) series also applies to your problem to some degree:
>> > > >
>> > > > https://lore.kernel.org/all/20240416121818.543896-3-mike.looijmans@topic.nl/
>> > >
>> > > I think based on the application diagram it is in fact measuring the
>> > > cell voltage.
>> > >
>> > > That said, this ultimately boils down to a cosmetic thing
>> > > as not having this property simply means userspace sees the battery
>> > > voltage as 3.8v instead of 7.6v as is written on the side.
>> >
>> > With the cells being connected in serial, the voltage of both cells
>> > can be different. There is not "the cell voltage". Instead there is
>> > a voltage for cell 1 and a voltage for cell 2. In a perfect battery
>> > they are the same, but in reality they are not. In the extreme case
>> > one of the cells may be broken while the other is still fine. It
>> > sounds as if this is just measuring the voltage from the first
>> > cell and assumes the second cell has the same voltage.
>> >
>> > Ideally the voltage on these platforms is not exposed via the normal
>> > VOLTAGE property and instead uses a new property for telling
>> > userspace the voltage for a single cell. The kernel simply does not
>> > know the voltage of the whole battery pack.
>> >
>> > FWIW this is the worst battery measurement system I've seen so far
>> > if my understanding of the hardware design is correct.
>> 
>> Please note that two facts should be considered here:
>> 
>>  - The GenBook schematic [1] clearly shows that the individual battery
>>    cells aren't exposed at its internal battery connector and, as a
>>    result, aren't available for individual cell voltage monitoring
>> 
>>  - The GenBook uses a CW2013 as it fuel gauge, [1] instead of CW2015
>>    as mentioned here a few times, but I haven't went through the 
>> CW2013
>>    datasheet(s) yet to see what are the actual differences between it
>>    and the CW2015
>> 
>> [1] https://wiki.cool-pi.com/notebook/coolpi-genbook-v20.pdf
> 
> Ah, thanks for pointing to the schematics. So the fuel gauge can
> only measure the voltage of the whole battery, but VCELL register
> provides the voltage for 1 cell? What is the origin of the dual-cell
> property? Was this something you came up with yourself when noticing
> the wrong voltage?

I'm not sure where does the idea for the dual-cell property come from,
but please note that it wasn't me who invented it, so to speak. :)
I don't even have the required hardware to test and develop these 
patches
on, i.e. I don't have a CM5-based GenBook.

> Based on the above information my guess would be that CW2013 uses a
> different voltage resolution than CW2015 for the VCELL register. The
> datasheet for CW2015 says 14bit ADC with 305uV resolution. Maybe
> the CW2013 simply uses a different ADC. Do you have the datasheet
> for the chip?

I've managed to find a few (seemingly a bit different) CW2013 
datasheets,
but as usual with CellWise, some of them may contain a bit confusing or
incomplete information.  I'd suggest that you wait for a couple of days
until I sift through the obtained datasheets, to save you from doing the
same. :)  Of course, I'll then send over the datasheet that seems 
correct
and the most complete to me.

I'll see to add support for CW2013 to the cw2015 driver, for which I
already have a couple of pending cleanup patches.