zephyr: pwm-leds: convert pwm_led to the Zephyr PWM API

Convert the shimmed pwm_led driver to use the Zephyr PWM APIs.
This decouples the driver from the named-pwms nodes and the legacy ECOS
PWM APIs.

BRANCH=none
BUG=b:217741090
TEST=build and run on volteer

Signed-off-by: Fabio Baltieri <fabiobaltieri@google.com>
Change-Id: Ie179860cc8f72b8769a78a11f70307826c9744a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3442684
Reviewed-by: Keith Short <keithshort@chromium.org>

6 files changed, 120 insertions, 18 deletions

diff --git a/common/led_pwm.c b/common/led_pwm.c
index 4384530eb2..b4f42bb37d 100644
--- a/common/led_pwm.c
+++ b/common/led_pwm.c
@@ -93,6 +93,7 @@ static void set_led_color(int color)
 
 static void set_pwm_led_enable(enum pwm_led_id id, int enable)
 {
+#ifndef CONFIG_ZEPHYR
 	const struct pwm_led *led = &pwm_leds[id];
 
 	if ((id >= CONFIG_LED_PWM_COUNT) || (id < 0))
@@ -104,6 +105,7 @@ static void set_pwm_led_enable(enum pwm_led_id id, int enable)
 		led->enable(led->ch1, enable);
 	if (led->ch2 != PWM_LED_NO_CHANNEL)
 		led->enable(led->ch2, enable);
+#endif
 }
 
 static void init_leds_off(void)
diff --git a/include/led_pwm.h b/include/led_pwm.h
index a7a648be8d..d0295b8061 100644
--- a/include/led_pwm.h
+++ b/include/led_pwm.h
@@ -10,8 +10,28 @@
 
 #ifdef CONFIG_ZEPHYR
 #include "pwm/pwm.h"
+#include "drivers/pwm.h"
 #endif
 
+#ifdef CONFIG_ZEPHYR
+#define PWM_LED_NO_CHANNEL NULL
+
+/* This could really be pwm_dt_spec. */
+struct pwm_led_dt_channel {
+	const struct device *dev;
+	uint32_t channel;
+	pwm_flags_t flags;
+	uint32_t period_us;
+};
+
+struct pwm_led {
+	const struct pwm_led_dt_channel *ch0;
+	const struct pwm_led_dt_channel *ch1;
+	const struct pwm_led_dt_channel *ch2;
+
+	void (*set_duty)(const struct pwm_led_dt_channel *ch, int percent);
+};
+#else
 #define PWM_LED_NO_CHANNEL ((enum pwm_channel)(-1))
 
 struct pwm_led {
@@ -22,6 +42,7 @@ struct pwm_led {
 	void (*enable)(enum pwm_channel ch, int enabled);
 	void (*set_duty)(enum pwm_channel ch, int percent);
 };
+#endif
 
 struct pwm_led_color_map {
 	uint8_t ch0;
diff --git a/zephyr/dts/bindings/led/cros-ec,pwm-leds.yaml b/zephyr/dts/bindings/led/cros-ec,pwm-leds.yaml
index a8e1c05717..815b65742a 100644
--- a/zephyr/dts/bindings/led/cros-ec,pwm-leds.yaml
+++ b/zephyr/dts/bindings/led/cros-ec,pwm-leds.yaml
@@ -14,14 +14,17 @@ properties:
     required: true
     description: |
       List of pwm-leds nodes. These are expected to have a pwms property
-      pointing to the relevant PWMs, in red, green, blue and optionally sidesel
-      order.
+      pointing to the relevant PWMs, up to three PWM devices. The order is
+      matched against the array order of the color-map- properties.
 
       For example
         pwmleds {
                 compatible = "pwm-leds";
                 pwm_led0: pwm_led_0 {
-                        pwms = <&led3_red &led2_green &led1_blue>;
+                        pwms = <&pwm1 0 PWM_POLARITY_INVERTED
+                                &pwm0 0 PWM_POLARITY_INVERTED
+                                &pwm2 0 PWM_POLARITY_INVERTED>;
+
                 };
         };
         cros-pwmleds {
@@ -33,7 +36,27 @@ properties:
     type: phandle
     required: false
     description: |
-      PWM node phandle for the SIDESEL line, if present.
+      A pwm-leds node for the SIDESEL line, if present.
+
+      For example
+        pwmleds {
+                compatible = "pwm-leds";
+                pwm_sidesel: pwm_sidesel {
+                        pwms = <&pwm7 0 PWM_POLARITY_INVERTED>;
+
+                };
+        };
+        cros-pwmleds {
+                compatible = "cros-ec,pwm-leds";
+                sidesel = <&pwm_sidesel>;
+        };
+
+  frequency:
+    type: int
+    required: true
+    description: |
+      PWM frequency in Hz for LEDs. SIDESEL, if present, uses half the
+      frequency.
 
   color-map-red:
     type: array
diff --git a/zephyr/projects/brya/brya/pwm_leds.dts b/zephyr/projects/brya/brya/pwm_leds.dts
index 7ade811df7..61ab604d36 100644
--- a/zephyr/projects/brya/brya/pwm_leds.dts
+++ b/zephyr/projects/brya/brya/pwm_leds.dts
@@ -7,10 +7,12 @@
 	pwmleds {
 		compatible = "pwm-leds";
 		pwm_led0: pwm_led_0 {
-			pwms = <&led1 &led2>;
+			pwms = <&pwm2 0 PWM_POLARITY_INVERTED
+				&pwm0 0 PWM_POLARITY_INVERTED>;
 		};
 		pwm_led1: pwm_led_1 {
-			pwms = <&led3 &led4>;
+			pwms = <&pwm1 0 PWM_POLARITY_INVERTED
+				&pwm7 0 PWM_POLARITY_INVERTED>;
 		};
 	};
 
@@ -18,6 +20,8 @@
 		compatible = "cros-ec,pwm-leds";
 
 		leds = <&pwm_led0 &pwm_led1>;
+		frequency = <4800>;
+
 			      /*<amber   white>*/
 		color-map-red    = <0    0>;
 		color-map-green  = <0    0>;
diff --git a/zephyr/projects/volteer/volteer/pwm_leds.dts b/zephyr/projects/volteer/volteer/pwm_leds.dts
index d37c93871a..293cfce9a9 100644
--- a/zephyr/projects/volteer/volteer/pwm_leds.dts
+++ b/zephyr/projects/volteer/volteer/pwm_leds.dts
@@ -7,7 +7,12 @@
 	pwmleds {
 		compatible = "pwm-leds";
 		pwm_led0: pwm_led_0 {
-			pwms = <&led3_red &led2_green &led1_blue>;
+			pwms = <&pwm1 0 PWM_POLARITY_INVERTED
+				&pwm0 0 PWM_POLARITY_INVERTED
+				&pwm2 0 PWM_POLARITY_INVERTED>;
+		};
+		pwm_sidesel: pwm_sidesel {
+			pwms = <&pwm7 0 PWM_POLARITY_INVERTED>;
 		};
 	};
 
@@ -15,7 +20,9 @@
 		compatible = "cros-ec,pwm-leds";
 
 		leds = <&pwm_led0>;
-		sidesel = <&led3_sidesel>;
+		sidesel = <&pwm_sidesel>;
+
+		frequency = <4800>;
 
 		color-map-red    = <100   0   0>;
 		color-map-green  = <  0 100   0>;
diff --git a/zephyr/shim/src/pwm_led.c b/zephyr/shim/src/pwm_led.c
index 915e0b1d38..e13db9b1f0 100644
--- a/zephyr/shim/src/pwm_led.c
+++ b/zephyr/shim/src/pwm_led.c
@@ -17,12 +17,18 @@
 #include "led_common.h"
 #include "led_pwm.h"
 #include "pwm.h"
+#include "util.h"
+
+LOG_MODULE_REGISTER(pwm_led, LOG_LEVEL_ERR);
 
 BUILD_ASSERT(DT_NUM_INST_STATUS_OKAY(cros_ec_pwm_leds) <= 1,
 	     "Multiple CrOS EC PWM LED instances defined");
 BUILD_ASSERT(DT_INST_PROP_LEN(0, leds) <= 2,
 	     "Unsupported number of LEDs defined");
 
+#define PWM_LED_PERIOD_US (USEC_PER_SEC/DT_INST_PROP(0, frequency))
+#define PWM_SIDESEL_PERIOD_US (PWM_LED_PERIOD_US * 2)
+
 #define PWM_LED_NAME(node_id) DT_STRING_UPPER_TOKEN(node_id, ec_led_name)
 #define PWM_LED_NAME_WITH_COMMA(node_id) PWM_LED_NAME(node_id),
 
@@ -34,14 +40,53 @@ const int supported_led_ids_count = ARRAY_SIZE(supported_led_ids);
 BUILD_ASSERT(ARRAY_SIZE(supported_led_ids) == DT_INST_PROP_LEN(0, leds),
 	     "Mismatch count of LED device phandles and LED name map entries.");
 
-#define PWM_CHANNEL_BY_IDX(node_id, prop, idx, led_ch)          \
-	PWM_CHANNEL(DT_PWMS_CTLR_BY_IDX(                        \
-		DT_PHANDLE_BY_IDX(node_id, prop, idx), led_ch))
+static void pwm_led_set_duty(const struct pwm_led_dt_channel *ch, int percent)
+{
+	uint32_t pulse_us;
+	int rv;
+
+	pulse_us = DIV_ROUND_NEAREST(ch->period_us * percent, 100);
+
+	LOG_DBG("LED PWM %s set percent (%d), pulse %d", ch->dev->name, percent,
+		pulse_us);
+
+	rv = pwm_pin_set_usec(ch->dev, ch->channel, ch->period_us, pulse_us,
+			      ch->flags);
+	if (rv)
+		LOG_ERR("pwm_pin_set_usec() failed %s (%d)", ch->dev->name, rv);
+}
+
+#define PWM_CHANNEL_DT_BY_IDX_INIT(node_id, led_ch, _period_us)             \
+	{                                                                   \
+		.dev = DEVICE_DT_GET(DT_PWMS_CTLR_BY_IDX(node_id, led_ch)), \
+		.channel = DT_PWMS_CHANNEL_BY_IDX(node_id, led_ch),         \
+		.flags = DT_PWMS_FLAGS_BY_IDX(node_id, led_ch),             \
+		.period_us = _period_us, \
+	}
+
+#define PWM_CHANNEL_DT_BY_IDX(node_id, prop, idx, led_ch)                      \
+	static const struct pwm_led_dt_channel _pwm_led_dt_##idx##_ch_##led_ch = \
+		PWM_CHANNEL_DT_BY_IDX_INIT(                                    \
+			DT_PHANDLE_BY_IDX(node_id, prop, idx), led_ch,         \
+			PWM_LED_PERIOD_US);
+
+#define PWM_CHANNEL_DT_BY_IDX_COND(node_id, prop, idx, led_ch)        \
+	IF_ENABLED(DT_PROP_HAS_IDX(                                   \
+		DT_PHANDLE_BY_IDX(node_id, prop, idx), pwms, led_ch), \
+		(PWM_CHANNEL_DT_BY_IDX(node_id, prop, idx, led_ch))   \
+		)
+
+#define PWM_LED_DT_INIT(node_id, prop, idx) \
+	PWM_CHANNEL_DT_BY_IDX_COND(node_id, prop, idx, 0) \
+	PWM_CHANNEL_DT_BY_IDX_COND(node_id, prop, idx, 1) \
+	PWM_CHANNEL_DT_BY_IDX_COND(node_id, prop, idx, 2)
+
+DT_INST_FOREACH_PROP_ELEM(0, leds, PWM_LED_DT_INIT)
 
 #define PWM_CHANNEL_BY_IDX_COND(node_id, prop, idx, led_ch)           \
 	COND_CODE_1(DT_PROP_HAS_IDX(                                  \
 		DT_PHANDLE_BY_IDX(node_id, prop, idx), pwms, led_ch), \
-		(PWM_CHANNEL_BY_IDX(node_id, prop, idx, led_ch)),     \
+		(&_pwm_led_dt_##idx##_ch_##led_ch), \
 		(PWM_LED_NO_CHANNEL))
 
 #define PWM_LED_INIT(node_id, prop, idx) \
@@ -49,8 +94,7 @@ BUILD_ASSERT(ARRAY_SIZE(supported_led_ids) == DT_INST_PROP_LEN(0, leds),
 		.ch0 = PWM_CHANNEL_BY_IDX_COND(node_id, prop, idx, 0), \
 		.ch1 = PWM_CHANNEL_BY_IDX_COND(node_id, prop, idx, 1), \
 		.ch2 = PWM_CHANNEL_BY_IDX_COND(node_id, prop, idx, 2), \
-		.enable = &pwm_enable, \
-		.set_duty = &pwm_set_duty, \
+		.set_duty = &pwm_led_set_duty, \
 	},
 
 struct pwm_led pwm_leds[] = {
@@ -129,7 +173,9 @@ int led_set_brightness(enum ec_led_id led_id, const uint8_t *brightness)
 
 #if DT_INST_NODE_HAS_PROP(0, sidesel)
 
-#define PWM_LED_SIDESEL PWM_CHANNEL(DT_INST_PROP(0, sidesel))
+static const struct pwm_led_dt_channel _pwm_led_dt_sidesel =
+	PWM_CHANNEL_DT_BY_IDX_INIT(DT_INST_PROP(0, sidesel), 0,
+				   PWM_SIDESEL_PERIOD_US);
 
 /* Illuminates the LED on the side of the active charging port. If not charging,
  * illuminates both LEDs.
@@ -152,15 +198,14 @@ static void led_set_charge_port_tick(void)
 	}
 
 	if (led_auto_control_is_enabled(EC_LED_ID_POWER_LED))
-		pwm_set_duty(PWM_LED_SIDESEL, side_select_duty);
+		pwm_led_set_duty(&_pwm_led_dt_sidesel, side_select_duty);
 }
 DECLARE_HOOK(HOOK_TICK, led_set_charge_port_tick, HOOK_PRIO_DEFAULT);
 
 static void board_led_init(void)
 {
 	/* Illuminate motherboard and daughter board LEDs equally to start. */
-	pwm_enable(PWM_LED_SIDESEL, 1);
-	pwm_set_duty(PWM_LED_SIDESEL, 50);
+	pwm_led_set_duty(&_pwm_led_dt_sidesel, 50);
 }
 DECLARE_HOOK(HOOK_INIT, board_led_init, HOOK_PRIO_DEFAULT);