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

[Uwe Kleine-König]

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 at gmail.com>
> Reviewed-by: Chen Wang <unicorn_wang at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-riscv/attachments/20250526/a3f9ce52/attachment.sig>