Skip to contentSkip to conversation

riscv: pwm: sophgo: add pwm support for SG2044

PATCH Superseded Longbin Li 3 participants 9 messages Apr 28, 2025 – May 27, 2025

Subsystems (4)
Device tree bindings RISC-V PWM Sophgo devicetrees and drivers

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Longbin Li

author
#0 lore raw

v4 cover riscv: pwm: sophgo: add pwm support for SG2044

This patch adds PWM controller support for four independent PWM channel outputs.

---

Changes in v4:

- add tags for mail.

Changes in v3:
You can simply review or test the patches at the link [3].

- Rename macro definitions to unify naming.
- Modify code style.

Changes in v2:
You can simply review or test the patches at the link [2].

- Modify variable naming and code logic.
- update "MODULE_AUTHOR".

Changes in v1:
You can simply review or test the patches at the link [1].

Link https://lore.kernel.org/linux-riscv/[email protected]/ [1] open here
Link https://lore.kernel.org/linux-riscv/[email protected]/ [2] open here
Link https://lore.kernel.org/linux-riscv/[email protected]/ [3] open here

---

Longbin Li (3):
pwm: sophgo: reorganize the code structure
pwm: sophgo: add driver for SG2044
dt-bindings: pwm: sophgo: add pwm controller for SG2044

 .../bindings/pwm/sophgo,sg2042-pwm.yaml       |   4 +-
 drivers/pwm/pwm-sophgo-sg2042.c               | 151 ++++++++++++++----
 2 files changed, 125 insertions(+), 30 deletions(-)

--
2.49.0

author
#1 lore raw

PATCH v4 1/3 dt-bindings: pwm: sophgo: add pwm controller for SG2044

Add compatible string for PWM controller on SG2044.

Signed-off-by Longbin Li <[email protected]>
Tested-by Chen Wang <[email protected]>
Reviewed-by Chen Wang <[email protected]>
Acked-by Krzysztof Kozlowski <[email protected]>
---
 Documentation/devicetree/bindings/pwm/sophgo,sg2042-pwm.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/pwm/sophgo,sg2042-pwm.yaml +3 -1
index bbb6326d47d7..e0e91aa237ec 100644
@@ -17,7 +17,9 @@ allOf:
 
 properties:
   compatible:
-    const: sophgo,sg2042-pwm
+    enum:
+      - sophgo,sg2042-pwm
+      - sophgo,sg2044-pwm
 
   reg:
     maxItems: 1

--
2.49.0

author
#2 lore raw

PATCH v4 2/3 pwm: sophgo: reorganize the code structure

In reply to Longbin Li

As the driver logic can be used in both SG2042 and SG2044, it will be better to reorganize the code structure.

Signed-off-by Longbin Li <[email protected]>
Reviewed-by Chen Wang <[email protected]>
---
 drivers/pwm/pwm-sophgo-sg2042.c | 62 +++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 27 deletions(-)
drivers/pwm/pwm-sophgo-sg2042.c +35 -27
index ff4639d849ce..23a83843ba53 100644
@@ -26,18 +26,6 @@
 #include <linux/pwm.h>
 #include <linux/reset.h>
 
-/*
- * Offset RegisterName
- * 0x0000 HLPERIOD0
- * 0x0004 PERIOD0
- * 0x0008 HLPERIOD1
- * 0x000C PERIOD1
- * 0x0010 HLPERIOD2
- * 0x0014 PERIOD2
- * 0x0018 HLPERIOD3
- * 0x001C PERIOD3
- * Four groups and every group is composed of HLPERIOD & PERIOD
- */
 #define SG2042_PWM_HLPERIOD(chan) ((chan) * 8 + 0)
 #define SG2042_PWM_PERIOD(chan) ((chan) * 8 + 4)
 

@@ -53,6 +41,10 @@ struct sg2042_pwm_ddata {
 	unsigned long clk_rate_hz;
 };
 
+struct sg2042_chip_data {
+	const struct pwm_ops ops;
+};
+
 /*
  * period_ticks: PERIOD
  * hlperiod_ticks: HLPERIOD

@@ -66,21 +58,13 @@ static void pwm_sg2042_config(struct sg2042_pwm_ddata *ddata, unsigned int chan,
 	writel(hlperiod_ticks, base + SG2042_PWM_HLPERIOD(chan));
 }
 
-static int pwm_sg2042_apply(struct pwm_chip *chip, struct pwm_device *pwm,
-			    const struct pwm_state *state)
+static void pwm_set_dutycycle(struct pwm_chip *chip, struct pwm_device *pwm,
+			      const struct pwm_state *state)
 {
 	struct sg2042_pwm_ddata *ddata = pwmchip_get_drvdata(chip);
 	u32 hlperiod_ticks;
 	u32 period_ticks;
 
-	if (state->polarity == PWM_POLARITY_INVERSED)
-		return -EINVAL;
-
-	if (!state->enabled) {
-		pwm_sg2042_config(ddata, pwm->hwpwm, 0, 0);
-		return 0;
-	}
-
 	/*
 	 * Duration of High level (duty_cycle) = HLPERIOD x Period_of_input_clk
 	 * Duration of One Cycle (period) = PERIOD x Period_of_input_clk

@@ -92,6 +76,22 @@ static int pwm_sg2042_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 		pwm->hwpwm, period_ticks, hlperiod_ticks);
 
 	pwm_sg2042_config(ddata, pwm->hwpwm, period_ticks, hlperiod_ticks);
+}
+
+static int pwm_sg2042_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			    const struct pwm_state *state)
+{
+	struct sg2042_pwm_ddata *ddata = pwmchip_get_drvdata(chip);
+
+	if (state->polarity == PWM_POLARITY_INVERSED)
+		return -EINVAL;
+
+	if (!state->enabled) {
+		pwm_sg2042_config(ddata, pwm->hwpwm, 0, 0);
+		return 0;
+	}
+
+	pwm_set_dutycycle(chip, pwm, state);
 
 	return 0;
 }

@@ -123,13 +123,16 @@ static int pwm_sg2042_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 	return 0;
 }
 
-static const struct pwm_ops pwm_sg2042_ops = {
-	.apply = pwm_sg2042_apply,
-	.get_state = pwm_sg2042_get_state,
+static const struct sg2042_chip_data sg2042_chip_data = {
+	.ops = {
+		.apply = pwm_sg2042_apply,
+		.get_state = pwm_sg2042_get_state,
+	}
 };
 
 static const struct of_device_id sg2042_pwm_ids[] = {
-	{ .compatible = "sophgo,sg2042-pwm" },
+	{ .compatible = "sophgo,sg2042-pwm",
+	  .data = &sg2042_chip_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sg2042_pwm_ids);

@@ -137,12 +140,17 @@ MODULE_DEVICE_TABLE(of, sg2042_pwm_ids);
 static int pwm_sg2042_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	const struct sg2042_chip_data *chip_data;
 	struct sg2042_pwm_ddata *ddata;
 	struct reset_control *rst;
 	struct pwm_chip *chip;
 	struct clk *clk;
 	int ret;
 
+	chip_data = device_get_match_data(dev);
+	if (!chip_data)
+		return -ENODEV;
+
 	chip = devm_pwmchip_alloc(dev, SG2042_PWM_CHANNELNUM, sizeof(*ddata));
 	if (IS_ERR(chip))
 		return PTR_ERR(chip);

@@ -170,7 +178,7 @@ static int pwm_sg2042_probe(struct platform_device *pdev)
 	if (IS_ERR(rst))
 		return dev_err_probe(dev, PTR_ERR(rst), "Failed to get reset\n");
 
-	chip->ops = &pwm_sg2042_ops;
+	chip->ops = &chip_data->ops;
 	chip->atomic = true;
 
 	ret = devm_pwmchip_add(dev, chip);

--
2.49.0

maintainer
#3 lore raw

Re: [PATCH v4 2/3] pwm: sophgo: reorganize the code structure

Hello,

Quoted 19 lines · Longbin Li

As the driver logic can be used in both SG2042 and SG2044, it will be better to reorganize the code structure.

Signed-off-by Longbin Li <[email protected]>
Reviewed-by Chen Wang <[email protected]>
---
 drivers/pwm/pwm-sophgo-sg2042.c | 62 +++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 27 deletions(-)
drivers/pwm/pwm-sophgo-sg2042.c +0 -2
index ff4639d849ce..23a83843ba53 100644
@@ -26,18 +26,6 @@
 #include <linux/pwm.h>
 #include <linux/reset.h>
 
-/*
- * Offset RegisterName

drivers/pwm/pwm-sophgo-sg2042.c line 40

Quoted 10 lines
drivers/pwm/pwm-sophgo-sg2042.c +0 -10
- * 0x0000 HLPERIOD0
- * 0x0004 PERIOD0
- * 0x0008 HLPERIOD1
- * 0x000C PERIOD1
- * 0x0010 HLPERIOD2
- * 0x0014 PERIOD2
- * 0x0018 HLPERIOD3
- * 0x001C PERIOD3
- * Four groups and every group is composed of HLPERIOD & PERIOD
- */

This seems to be still correct? Why remove it then?

Quoted 15 lines
drivers/pwm/pwm-sophgo-sg2042.c +4 -0
 #define SG2042_PWM_HLPERIOD(chan) ((chan) * 8 + 0)
 #define SG2042_PWM_PERIOD(chan) ((chan) * 8 + 4)
 

@@ -53,6 +41,10 @@ struct sg2042_pwm_ddata {
 	unsigned long clk_rate_hz;
 };
 
+struct sg2042_chip_data {
+	const struct pwm_ops ops;
+};
+
 /*
  * period_ticks: PERIOD
  * hlperiod_ticks: HLPERIOD

drivers/pwm/pwm-sophgo-sg2042.c static void pwm_sg2042_config(struct sg2042_pwm_ddata *ddata, unsigned int chan, · line 61

Quoted 6 lines
drivers/pwm/pwm-sophgo-sg2042.c +1 -2
@@ -66,21 +58,13 @@ static void pwm_sg2042_config(struct sg2042_pwm_ddata *ddata, unsigned int chan,
 	writel(hlperiod_ticks, base + SG2042_PWM_HLPERIOD(chan));
 }
 
-static int pwm_sg2042_apply(struct pwm_chip *chip, struct pwm_device *pwm,
-			    const struct pwm_state *state)
+static void pwm_set_dutycycle(struct pwm_chip *chip, struct pwm_device *pwm,

This is not a global pwm API function, so please stick to the pwm_sg2042 prefix.

Quoted 26 lines
drivers/pwm/pwm-sophgo-sg2042.c +2 -11
+			      const struct pwm_state *state)
 {
 	struct sg2042_pwm_ddata *ddata = pwmchip_get_drvdata(chip);
 	u32 hlperiod_ticks;
 	u32 period_ticks;
 
-	if (state->polarity == PWM_POLARITY_INVERSED)
-		return -EINVAL;
-
-	if (!state->enabled) {
-		pwm_sg2042_config(ddata, pwm->hwpwm, 0, 0);
-		return 0;
-	}
-
 	/*
 	 * Duration of High level (duty_cycle) = HLPERIOD x Period_of_input_clk
 	 * Duration of One Cycle (period) = PERIOD x Period_of_input_clk
[...]

@@ -123,13 +123,16 @@ static int pwm_sg2042_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 	return 0;
 }
 
-static const struct pwm_ops pwm_sg2042_ops = {
-	.apply = pwm_sg2042_apply,
-	.get_state = pwm_sg2042_get_state,
+static const struct sg2042_chip_data sg2042_chip_data = {

drivers/pwm/pwm-sophgo-sg2042.c static int pwm_sg2042_get_state(struct pwm_chip *chip, struct pwm_device *pwm, · line 135

Quoted 10 lines
drivers/pwm/pwm-sophgo-sg2042.c +6 -1
+	.ops = {
+		.apply = pwm_sg2042_apply,
+		.get_state = pwm_sg2042_get_state,
+	}
 };
 
 static const struct of_device_id sg2042_pwm_ids[] = {
-	{ .compatible = "sophgo,sg2042-pwm" },
+	{ .compatible = "sophgo,sg2042-pwm",
+	  .data = &sg2042_chip_data },

I would have expected that checkpatch doesn't like that. At least I don't. Please make this

	{
		.compatible = ...;
		.data = ...;
	},
Quoted 3 lines
drivers/pwm/pwm-sophgo-sg2042.c +0 -0
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sg2042_pwm_ids);
author
#4 lore raw

Re: [PATCH v4 2/3] pwm: sophgo: reorganize the code structure

Quoted 20 lines · Uwe Kleine-König

Hello,

Quoted 17 lines · Longbin Li

As the driver logic can be used in both SG2042 and SG2044, it will be better to reorganize the code structure.

Signed-off-by Longbin Li <[email protected]>
Reviewed-by Chen Wang <[email protected]>
---
 drivers/pwm/pwm-sophgo-sg2042.c | 62 +++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 27 deletions(-)
drivers/pwm/pwm-sophgo-sg2042.c +0 -0
index ff4639d849ce..23a83843ba53 100644
@@ -26,18 +26,6 @@
 #include <linux/pwm.h>
 #include <linux/reset.h>
Quoted 15 lines

drivers/pwm/pwm-sophgo-sg2042.c line 40

Quoted 12 lines
drivers/pwm/pwm-sophgo-sg2042.c +0 -12
-/*
- * Offset RegisterName
- * 0x0000 HLPERIOD0
- * 0x0004 PERIOD0
- * 0x0008 HLPERIOD1
- * 0x000C PERIOD1
- * 0x0010 HLPERIOD2
- * 0x0014 PERIOD2
- * 0x0018 HLPERIOD3
- * 0x001C PERIOD3
- * Four groups and every group is composed of HLPERIOD & PERIOD
- */

This seems to be still correct? Why remove it then?

The following "#define" has already expressd it. For clarity, i will add it back.

Quoted 18 lines
Quoted 18 lines
drivers/pwm/pwm-sophgo-sg2042.c +4 -0
 #define SG2042_PWM_HLPERIOD(chan) ((chan) * 8 + 0)
 #define SG2042_PWM_PERIOD(chan) ((chan) * 8 + 4)
 

@@ -53,6 +41,10 @@ struct sg2042_pwm_ddata {
 	unsigned long clk_rate_hz;
 };
 
+struct sg2042_chip_data {
+	const struct pwm_ops ops;
+};
+
 /*
  * period_ticks: PERIOD
  * hlperiod_ticks: HLPERIOD

@@ -66,21 +58,13 @@ static void pwm_sg2042_config(struct sg2042_pwm_ddata *ddata, unsigned int chan,
 	writel(hlperiod_ticks, base + SG2042_PWM_HLPERIOD(chan));
 }
Quoted 7 lines

drivers/pwm/pwm-sophgo-sg2042.c static void pwm_sg2042_config(struct sg2042_pwm_ddata *ddata, unsigned int chan, · line 61

Quoted 3 lines
drivers/pwm/pwm-sophgo-sg2042.c +1 -2
-static int pwm_sg2042_apply(struct pwm_chip *chip, struct pwm_device *pwm,
-			    const struct pwm_state *state)
+static void pwm_set_dutycycle(struct pwm_chip *chip, struct pwm_device *pwm,

This is not a global pwm API function, so please stick to the pwm_sg2042 prefix.

I will modify that, thanks.

Quoted 40 lines
Quoted 26 lines
drivers/pwm/pwm-sophgo-sg2042.c +2 -11
+			      const struct pwm_state *state)
 {
 	struct sg2042_pwm_ddata *ddata = pwmchip_get_drvdata(chip);
 	u32 hlperiod_ticks;
 	u32 period_ticks;
 
-	if (state->polarity == PWM_POLARITY_INVERSED)
-		return -EINVAL;
-
-	if (!state->enabled) {
-		pwm_sg2042_config(ddata, pwm->hwpwm, 0, 0);
-		return 0;
-	}
-
 	/*
 	 * Duration of High level (duty_cycle) = HLPERIOD x Period_of_input_clk
 	 * Duration of One Cycle (period) = PERIOD x Period_of_input_clk
[...]

@@ -123,13 +123,16 @@ static int pwm_sg2042_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 	return 0;
 }
 
-static const struct pwm_ops pwm_sg2042_ops = {
-	.apply = pwm_sg2042_apply,
-	.get_state = pwm_sg2042_get_state,
+static const struct sg2042_chip_data sg2042_chip_data = {

drivers/pwm/pwm-sophgo-sg2042.c static int pwm_sg2042_get_state(struct pwm_chip *chip, struct pwm_device *pwm, · line 135

Quoted 10 lines
drivers/pwm/pwm-sophgo-sg2042.c +6 -1
+	.ops = {
+		.apply = pwm_sg2042_apply,
+		.get_state = pwm_sg2042_get_state,
+	}
 };
 
 static const struct of_device_id sg2042_pwm_ids[] = {
-	{ .compatible = "sophgo,sg2042-pwm" },
+	{ .compatible = "sophgo,sg2042-pwm",
+	  .data = &sg2042_chip_data },

I would have expected that checkpatch doesn't like that. At least I don't. Please make this

Quoted 5 lines
	{
		.compatible = ...;
		.data = ...;
	},

I will check that, thanks.

Quoted 3 lines
Quoted 3 lines
drivers/pwm/pwm-sophgo-sg2042.c +0 -0
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sg2042_pwm_ids);
author
#5 lore raw

PATCH v4 3/3 pwm: sophgo: add driver for SG2044

In reply to Longbin Li

Add PWM controller for SG2044 on base of SG2042.

Signed-off-by Longbin Li <[email protected]>
Reviewed-by Chen Wang <[email protected]>
---
 drivers/pwm/pwm-sophgo-sg2042.c | 89 ++++++++++++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 2 deletions(-)
drivers/pwm/pwm-sophgo-sg2042.c +87 -2
index 23a83843ba53..5bb92c910540 100644
@@ -13,6 +13,9 @@
  *   the running period.
  * - When PERIOD and HLPERIOD is set to 0, the PWM wave output will
  *   be stopped and the output is pulled to high.
+ * - SG2044 support polarity while SG2042 does not. When PWMSTART is
+ *   false, POLARITY being NORMAL will make output being low,
+ *   POLARITY being INVERSED will make output being high.
  * See the datasheet [1] for more details.
  * [1]:https://github.com/sophgo/sophgo-doc/tree/main/SG2042/TRM
  */

@@ -26,6 +29,10 @@
 #include <linux/pwm.h>
 #include <linux/reset.h>
 
+#define SG2044_PWM_POLARITY		0x40
+#define SG2044_PWM_PWMSTART		0x44
+#define SG2044_PWM_OE			0xd0
+
 #define SG2042_PWM_HLPERIOD(chan) ((chan) * 8 + 0)
 #define SG2042_PWM_PERIOD(chan) ((chan) * 8 + 4)
 

@@ -72,8 +79,8 @@ static void pwm_set_dutycycle(struct pwm_chip *chip, struct pwm_device *pwm,
 	period_ticks = min(mul_u64_u64_div_u64(ddata->clk_rate_hz, state->period, NSEC_PER_SEC), U32_MAX);
 	hlperiod_ticks = min(mul_u64_u64_div_u64(ddata->clk_rate_hz, state->duty_cycle, NSEC_PER_SEC), U32_MAX);
 
-	dev_dbg(pwmchip_parent(chip), "chan[%u]: PERIOD=%u, HLPERIOD=%u\n",
-		pwm->hwpwm, period_ticks, hlperiod_ticks);
+	dev_dbg(pwmchip_parent(chip), "chan[%u]: ENABLE=%u, PERIOD=%u, HLPERIOD=%u, POLARITY=%u\n",
+		pwm->hwpwm, state->enabled, period_ticks, hlperiod_ticks, state->polarity);
 
 	pwm_sg2042_config(ddata, pwm->hwpwm, period_ticks, hlperiod_ticks);
 }

@@ -123,6 +130,74 @@ static int pwm_sg2042_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 	return 0;
 }
 
+static void pwm_sg2044_set_start(struct sg2042_pwm_ddata *ddata, struct pwm_device *pwm,
+				 bool enabled)
+{
+	u32 pwm_value;
+
+	pwm_value = readl(ddata->base + SG2044_PWM_PWMSTART);
+
+	if (enabled)
+		pwm_value |= BIT(pwm->hwpwm);
+	else
+		pwm_value &= ~BIT(pwm->hwpwm);
+
+	writel(pwm_value, ddata->base + SG2044_PWM_PWMSTART);
+}
+
+static void pwm_sg2044_set_outputdir(struct sg2042_pwm_ddata *ddata, struct pwm_device *pwm,
+				     bool enabled)
+{
+	u32 pwm_value;
+
+	pwm_value = readl(ddata->base + SG2044_PWM_OE);
+
+	if (enabled)
+		pwm_value |= BIT(pwm->hwpwm);
+	else
+		pwm_value &= ~BIT(pwm->hwpwm);
+
+	writel(pwm_value, ddata->base + SG2044_PWM_OE);
+}
+
+static void pwm_sg2044_set_polarity(struct sg2042_pwm_ddata *ddata, struct pwm_device *pwm,
+				    const struct pwm_state *state)
+{
+	u32 pwm_value;
+
+	pwm_value = readl(ddata->base + SG2044_PWM_POLARITY);
+
+	if (state->polarity == PWM_POLARITY_NORMAL)
+		pwm_value &= ~BIT(pwm->hwpwm);
+	else
+		pwm_value |= BIT(pwm->hwpwm);
+
+	writel(pwm_value, ddata->base + SG2044_PWM_POLARITY);
+}
+
+static int pwm_sg2044_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			    const struct pwm_state *state)
+{
+	struct sg2042_pwm_ddata *ddata = pwmchip_get_drvdata(chip);
+
+	pwm_sg2044_set_polarity(ddata, pwm, state);
+
+	pwm_set_dutycycle(chip, pwm, state);
+
+	/*
+	 * re-enable PWMSTART to refresh the register period
+	 */
+	pwm_sg2044_set_start(ddata, pwm, false);
+
+	if (!state->enabled)
+		return 0;
+
+	pwm_sg2044_set_outputdir(ddata, pwm, true);
+	pwm_sg2044_set_start(ddata, pwm, true);
+
+	return 0;
+}
+
 static const struct sg2042_chip_data sg2042_chip_data = {
 	.ops = {
 		.apply = pwm_sg2042_apply,

@@ -130,9 +205,18 @@ static const struct sg2042_chip_data sg2042_chip_data = {
 	}
 };
 
+static const struct sg2042_chip_data sg2044_chip_data = {
+	.ops = {
+		.apply = pwm_sg2044_apply,
+		.get_state = pwm_sg2042_get_state,
+	}
+};
+
 static const struct of_device_id sg2042_pwm_ids[] = {
 	{ .compatible = "sophgo,sg2042-pwm",
 	  .data = &sg2042_chip_data },
+	{ .compatible = "sophgo,sg2044-pwm",
+	  .data = &sg2044_chip_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sg2042_pwm_ids);

@@ -198,5 +282,6 @@ static struct platform_driver pwm_sg2042_driver = {
 module_platform_driver(pwm_sg2042_driver);
 
 MODULE_AUTHOR("Chen Wang");
+MODULE_AUTHOR("Longbin Li <[email protected]>");
 MODULE_DESCRIPTION("Sophgo SG2042 PWM driver");
 MODULE_LICENSE("GPL");

--
2.49.0

maintainer
#6 lore raw

Re: [PATCH v4 3/3] pwm: sophgo: add driver for SG2044

Quoted 13 lines · Longbin Li

Add PWM controller for SG2044 on base of SG2042.

Signed-off-by Longbin Li <[email protected]>
Reviewed-by Chen Wang <[email protected]>
---
 drivers/pwm/pwm-sophgo-sg2042.c | 89 ++++++++++++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 2 deletions(-)
drivers/pwm/pwm-sophgo-sg2042.c +0 -0
index 23a83843ba53..5bb92c910540 100644

drivers/pwm/pwm-sophgo-sg2042.c line 18

Quoted 6 lines
drivers/pwm/pwm-sophgo-sg2042.c +3 -0
@@ -13,6 +13,9 @@
  *   the running period.
  * - When PERIOD and HLPERIOD is set to 0, the PWM wave output will
  *   be stopped and the output is pulled to high.
+ * - SG2044 support polarity while SG2042 does not. When PWMSTART is
+ *   false, POLARITY being NORMAL will make output being low,
+ *   POLARITY being INVERSED will make output being high.

Without detailed knowledge about the hardware this isn't understandable. What is PWMSTART? I think this can just read:

SG2044 supports both polarities, SG2042 only normal polarity.

The rest is an implementation detail.

drivers/pwm/pwm-sophgo-sg2042.c line 20

Quoted 2 lines
drivers/pwm/pwm-sophgo-sg2042.c +0 -0
  * See the datasheet [1] for more details.
  * [1]:https://github.com/sophgo/sophgo-doc/tree/main/SG2042/TRM

This only describes SG2042, right? Can you please add a link for the new variant?

Quoted 24 lines
drivers/pwm/pwm-sophgo-sg2042.c +6 -2
  */

@@ -26,6 +29,10 @@
 #include <linux/pwm.h>
 #include <linux/reset.h>
 
+#define SG2044_PWM_POLARITY		0x40
+#define SG2044_PWM_PWMSTART		0x44
+#define SG2044_PWM_OE			0xd0
+
 #define SG2042_PWM_HLPERIOD(chan) ((chan) * 8 + 0)
 #define SG2042_PWM_PERIOD(chan) ((chan) * 8 + 4)
 

@@ -72,8 +79,8 @@ static void pwm_set_dutycycle(struct pwm_chip *chip, struct pwm_device *pwm,
 	period_ticks = min(mul_u64_u64_div_u64(ddata->clk_rate_hz, state->period, NSEC_PER_SEC), U32_MAX);
 	hlperiod_ticks = min(mul_u64_u64_div_u64(ddata->clk_rate_hz, state->duty_cycle, NSEC_PER_SEC), U32_MAX);
 
-	dev_dbg(pwmchip_parent(chip), "chan[%u]: PERIOD=%u, HLPERIOD=%u\n",
-		pwm->hwpwm, period_ticks, hlperiod_ticks);
+	dev_dbg(pwmchip_parent(chip), "chan[%u]: ENABLE=%u, PERIOD=%u, HLPERIOD=%u, POLARITY=%u\n",
+		pwm->hwpwm, state->enabled, period_ticks, hlperiod_ticks, state->polarity);
 
 	pwm_sg2042_config(ddata, pwm->hwpwm, period_ticks, hlperiod_ticks);
 }

drivers/pwm/pwm-sophgo-sg2042.c static int pwm_sg2042_get_state(struct pwm_chip *chip, struct pwm_device *pwm, · line 138

Quoted 9 lines
drivers/pwm/pwm-sophgo-sg2042.c +6 -0
@@ -123,6 +130,74 @@ static int pwm_sg2042_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 	return 0;
 }
 
+static void pwm_sg2044_set_start(struct sg2042_pwm_ddata *ddata, struct pwm_device *pwm,
+				 bool enabled)
+{
+	u32 pwm_value;
+
+	pwm_value = readl(ddata->base + SG2044_PWM_PWMSTART);

Please use a variable name that matches the register this is used for.

Quoted 41 lines
drivers/pwm/pwm-sophgo-sg2042.c +41 -0
+	if (enabled)
+		pwm_value |= BIT(pwm->hwpwm);
+	else
+		pwm_value &= ~BIT(pwm->hwpwm);
+
+	writel(pwm_value, ddata->base + SG2044_PWM_PWMSTART);
+}
+
+static void pwm_sg2044_set_outputdir(struct sg2042_pwm_ddata *ddata, struct pwm_device *pwm,
+				     bool enabled)
+{
+	u32 pwm_value;
+
+	pwm_value = readl(ddata->base + SG2044_PWM_OE);
+
+	if (enabled)
+		pwm_value |= BIT(pwm->hwpwm);
+	else
+		pwm_value &= ~BIT(pwm->hwpwm);
+
+	writel(pwm_value, ddata->base + SG2044_PWM_OE);
+}
+
+static void pwm_sg2044_set_polarity(struct sg2042_pwm_ddata *ddata, struct pwm_device *pwm,
+				    const struct pwm_state *state)
+{
+	u32 pwm_value;
+
+	pwm_value = readl(ddata->base + SG2044_PWM_POLARITY);
+
+	if (state->polarity == PWM_POLARITY_NORMAL)
+		pwm_value &= ~BIT(pwm->hwpwm);
+	else
+		pwm_value |= BIT(pwm->hwpwm);
+
+	writel(pwm_value, ddata->base + SG2044_PWM_POLARITY);
+}
+
+static int pwm_sg2044_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			    const struct pwm_state *state)
+{

drivers/pwm/pwm-sophgo-sg2042.c static int pwm_sg2042_get_state(struct pwm_chip *chip, struct pwm_device *pwm, · line 190

Quoted 10 lines
drivers/pwm/pwm-sophgo-sg2042.c +10 -0
+	struct sg2042_pwm_ddata *ddata = pwmchip_get_drvdata(chip);
+
+	pwm_sg2044_set_polarity(ddata, pwm, state);
+
+	pwm_set_dutycycle(chip, pwm, state);
+
+	/*
+	 * re-enable PWMSTART to refresh the register period
+	 */
+	pwm_sg2044_set_start(ddata, pwm, false);

I don't understand the effect of the START register.

drivers/pwm/pwm-sophgo-sg2042.c static int pwm_sg2042_get_state(struct pwm_chip *chip, struct pwm_device *pwm, · line 203

Quoted 12 lines
drivers/pwm/pwm-sophgo-sg2042.c +9 -0
+	if (!state->enabled)
+		return 0;
+
+	pwm_sg2044_set_outputdir(ddata, pwm, true);
+	pwm_sg2044_set_start(ddata, pwm, true);
+
+	return 0;
+}
+
 static const struct sg2042_chip_data sg2042_chip_data = {
 	.ops = {
 		.apply = pwm_sg2042_apply,

Best regards
Uwe

author
#7 lore raw

Re: [PATCH v4 3/3] pwm: sophgo: add driver for SG2044

Quoted 24 lines · Uwe Kleine-König
Quoted 13 lines · Longbin Li

Add PWM controller for SG2044 on base of SG2042.

Signed-off-by Longbin Li <[email protected]>
Reviewed-by Chen Wang <[email protected]>
---
 drivers/pwm/pwm-sophgo-sg2042.c | 89 ++++++++++++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 2 deletions(-)
drivers/pwm/pwm-sophgo-sg2042.c +0 -0
index 23a83843ba53..5bb92c910540 100644

drivers/pwm/pwm-sophgo-sg2042.c line 18

Quoted 6 lines
drivers/pwm/pwm-sophgo-sg2042.c +3 -0
@@ -13,6 +13,9 @@
  *   the running period.
  * - When PERIOD and HLPERIOD is set to 0, the PWM wave output will
  *   be stopped and the output is pulled to high.
+ * - SG2044 support polarity while SG2042 does not. When PWMSTART is
+ *   false, POLARITY being NORMAL will make output being low,
+ *   POLARITY being INVERSED will make output being high.

Without detailed knowledge about the hardware this isn't understandable. What is PWMSTART? I think this can just read:

Quoted 4 lines

SG2044 supports both polarities, SG2042 only normal polarity.

The rest is an implementation detail.

I will modify that, thanks.

Quoted 6 lines

drivers/pwm/pwm-sophgo-sg2042.c line 20

Quoted 2 lines
drivers/pwm/pwm-sophgo-sg2042.c +0 -0
  * See the datasheet [1] for more details.
  * [1]:https://github.com/sophgo/sophgo-doc/tree/main/SG2042/TRM

This only describes SG2042, right? Can you please add a link for the new variant?

Sorry, there is no public document for SG2044.

Quoted 27 lines
Quoted 27 lines
drivers/pwm/pwm-sophgo-sg2042.c +6 -2
  */

@@ -26,6 +29,10 @@
 #include <linux/pwm.h>
 #include <linux/reset.h>
 
+#define SG2044_PWM_POLARITY		0x40
+#define SG2044_PWM_PWMSTART		0x44
+#define SG2044_PWM_OE			0xd0
+
 #define SG2042_PWM_HLPERIOD(chan) ((chan) * 8 + 0)
 #define SG2042_PWM_PERIOD(chan) ((chan) * 8 + 4)
 

@@ -72,8 +79,8 @@ static void pwm_set_dutycycle(struct pwm_chip *chip, struct pwm_device *pwm,
 	period_ticks = min(mul_u64_u64_div_u64(ddata->clk_rate_hz, state->period, NSEC_PER_SEC), U32_MAX);
 	hlperiod_ticks = min(mul_u64_u64_div_u64(ddata->clk_rate_hz, state->duty_cycle, NSEC_PER_SEC), U32_MAX);
 
-	dev_dbg(pwmchip_parent(chip), "chan[%u]: PERIOD=%u, HLPERIOD=%u\n",
-		pwm->hwpwm, period_ticks, hlperiod_ticks);
+	dev_dbg(pwmchip_parent(chip), "chan[%u]: ENABLE=%u, PERIOD=%u, HLPERIOD=%u, POLARITY=%u\n",
+		pwm->hwpwm, state->enabled, period_ticks, hlperiod_ticks, state->polarity);
 
 	pwm_sg2042_config(ddata, pwm->hwpwm, period_ticks, hlperiod_ticks);
 }

@@ -123,6 +130,74 @@ static int pwm_sg2042_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 	return 0;
 }
Quoted 9 lines

drivers/pwm/pwm-sophgo-sg2042.c static int pwm_sg2042_get_state(struct pwm_chip *chip, struct pwm_device *pwm, · line 138

Quoted 6 lines
drivers/pwm/pwm-sophgo-sg2042.c +6 -0
+static void pwm_sg2044_set_start(struct sg2042_pwm_ddata *ddata, struct pwm_device *pwm,
+				 bool enabled)
+{
+	u32 pwm_value;
+
+	pwm_value = readl(ddata->base + SG2044_PWM_PWMSTART);

Please use a variable name that matches the register this is used for.

I will fix that.

Quoted 45 lines
Quoted 45 lines
drivers/pwm/pwm-sophgo-sg2042.c +45 -0
+	if (enabled)
+		pwm_value |= BIT(pwm->hwpwm);
+	else
+		pwm_value &= ~BIT(pwm->hwpwm);
+
+	writel(pwm_value, ddata->base + SG2044_PWM_PWMSTART);
+}
+
+static void pwm_sg2044_set_outputdir(struct sg2042_pwm_ddata *ddata, struct pwm_device *pwm,
+				     bool enabled)
+{
+	u32 pwm_value;
+
+	pwm_value = readl(ddata->base + SG2044_PWM_OE);
+
+	if (enabled)
+		pwm_value |= BIT(pwm->hwpwm);
+	else
+		pwm_value &= ~BIT(pwm->hwpwm);
+
+	writel(pwm_value, ddata->base + SG2044_PWM_OE);
+}
+
+static void pwm_sg2044_set_polarity(struct sg2042_pwm_ddata *ddata, struct pwm_device *pwm,
+				    const struct pwm_state *state)
+{
+	u32 pwm_value;
+
+	pwm_value = readl(ddata->base + SG2044_PWM_POLARITY);
+
+	if (state->polarity == PWM_POLARITY_NORMAL)
+		pwm_value &= ~BIT(pwm->hwpwm);
+	else
+		pwm_value |= BIT(pwm->hwpwm);
+
+	writel(pwm_value, ddata->base + SG2044_PWM_POLARITY);
+}
+
+static int pwm_sg2044_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			    const struct pwm_state *state)
+{
+	struct sg2042_pwm_ddata *ddata = pwmchip_get_drvdata(chip);
+
+	pwm_sg2044_set_polarity(ddata, pwm, state);
+
Quoted 9 lines

drivers/pwm/pwm-sophgo-sg2042.c static int pwm_sg2042_get_state(struct pwm_chip *chip, struct pwm_device *pwm, · line 190

Quoted 6 lines
drivers/pwm/pwm-sophgo-sg2042.c +6 -0
+	pwm_set_dutycycle(chip, pwm, state);
+
+	/*
+	 * re-enable PWMSTART to refresh the register period
+	 */
+	pwm_sg2044_set_start(ddata, pwm, false);

I don't understand the effect of the START register.

PWMSTART[3:0] enable the output of the corresponding channel. Must re-enable it to refresh the register period when changed the duty cycle. I will find a better name, thanks.

Quoted 15 lines

drivers/pwm/pwm-sophgo-sg2042.c static int pwm_sg2042_get_state(struct pwm_chip *chip, struct pwm_device *pwm, · line 203

Quoted 12 lines
drivers/pwm/pwm-sophgo-sg2042.c +9 -0
+	if (!state->enabled)
+		return 0;
+
+	pwm_sg2044_set_outputdir(ddata, pwm, true);
+	pwm_sg2044_set_start(ddata, pwm, true);
+
+	return 0;
+}
+
 static const struct sg2042_chip_data sg2042_chip_data = {
 	.ops = {
 		.apply = pwm_sg2042_apply,

Best regards
Uwe

#8 lore raw
In reply to Longbin Li

Seems only tag changes. LGTM.

Thanks,

Chen

Quoted 40 lines · Longbin Li

This patch adds PWM controller support for four independent PWM channel outputs.

---

Changes in v4:

- add tags for mail.

Changes in v3:
You can simply review or test the patches at the link [3].

- Rename macro definitions to unify naming.
- Modify code style.

Changes in v2:
You can simply review or test the patches at the link [2].

- Modify variable naming and code logic.
- update "MODULE_AUTHOR".

Changes in v1:
You can simply review or test the patches at the link [1].

Link https://lore.kernel.org/linux-riscv/[email protected]/ [1] open here
Link https://lore.kernel.org/linux-riscv/[email protected]/ [2] open here
Link https://lore.kernel.org/linux-riscv/[email protected]/ [3] open here

---

Longbin Li (3):
pwm: sophgo: reorganize the code structure
pwm: sophgo: add driver for SG2044
dt-bindings: pwm: sophgo: add pwm controller for SG2044

  .../bindings/pwm/sophgo,sg2042-pwm.yaml       |   4 +-
  drivers/pwm/pwm-sophgo-sg2042.c               | 151 ++++++++++++++----
  2 files changed, 125 insertions(+), 30 deletions(-)

--
2.49.0