[v4] riscv: pwm: sophgo: add pwm support for SG2044

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

Posted by Longbin Li 9 months, 2 weeks ago

Add PWM controller for SG2044 on base of SG2042.

Signed-off-by: Longbin Li <looong.bin@gmail.com>
Reviewed-by: Chen Wang <unicorn_wang@outlook.com>
---
 drivers/pwm/pwm-sophgo-sg2042.c | 89 ++++++++++++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-sophgo-sg2042.c b/drivers/pwm/pwm-sophgo-sg2042.c
index 23a83843ba53..5bb92c910540 100644
--- a/drivers/pwm/pwm-sophgo-sg2042.c
+++ b/drivers/pwm/pwm-sophgo-sg2042.c
@@ -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 <looong.bin@gmail.com>");
 MODULE_DESCRIPTION("Sophgo SG2042 PWM driver");
 MODULE_LICENSE("GPL");
--
2.49.0

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

Posted by Uwe Kleine-König 8 months, 2 weeks ago

On Mon, Apr 28, 2025 at 09:34:50AM +0800, Longbin Li wrote:
> Add PWM controller for SG2044 on base of SG2042.
> 
> Signed-off-by: Longbin Li <looong.bin@gmail.com>
> Reviewed-by: Chen Wang <unicorn_wang@outlook.com>
> ---
>  drivers/pwm/pwm-sophgo-sg2042.c | 89 ++++++++++++++++++++++++++++++++-
>  1 file changed, 87 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-sophgo-sg2042.c b/drivers/pwm/pwm-sophgo-sg2042.c
> index 23a83843ba53..5bb92c910540 100644
> --- a/drivers/pwm/pwm-sophgo-sg2042.c
> +++ b/drivers/pwm/pwm-sophgo-sg2042.c
> @@ -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.

>   * 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?

>   */
> @@ -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);

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

> +	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);

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

> +	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

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

Posted by Longbin Li 8 months, 2 weeks ago

On Mon, May 26, 2025 at 05:06:25PM +0200, Uwe Kleine-König wrote:
> On Mon, Apr 28, 2025 at 09:34:50AM +0800, Longbin Li wrote:
> > Add PWM controller for SG2044 on base of SG2042.
> > 
> > Signed-off-by: Longbin Li <looong.bin@gmail.com>
> > Reviewed-by: Chen Wang <unicorn_wang@outlook.com>
> > ---
> >  drivers/pwm/pwm-sophgo-sg2042.c | 89 ++++++++++++++++++++++++++++++++-
> >  1 file changed, 87 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-sophgo-sg2042.c b/drivers/pwm/pwm-sophgo-sg2042.c
> > index 23a83843ba53..5bb92c910540 100644
> > --- a/drivers/pwm/pwm-sophgo-sg2042.c
> > +++ b/drivers/pwm/pwm-sophgo-sg2042.c
> > @@ -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.
>

I will modify that, thanks.
 
> >   * 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.

> >   */
> > @@ -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);
> 
> Please use a variable name that matches the register this is used for.
> 

I will fix that.

> > +	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);
> 
> 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.

> > +	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

[PATCH v4 1/3] dt-bindings: pwm: sophgo: add pwm controller for SG2044
[PATCH v4 2/3] pwm: sophgo: reorganize the code structure
[PATCH v4 3/3] pwm: sophgo: add driver for SG2044