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

Uwe Kleine-König ukleinek at kernel.org
Fri May 30 00:50:25 PDT 2025


Hello,

On Wed, May 28, 2025 at 06:11:38PM +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>
> Tested-by: Chen Wang <unicorn_wang at outlook.com>

Nitpick: Make your S-o-b line the last line. This way you document that
it was you who added the tags for Chen Wang.

> [...]
> +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_sg2042_set_dutycycle(chip, pwm, state);
> +
> +	/*
> +	 * re-enable PWMSTART to refresh the register period
> +	 */
> +	 pwm_sg2044_set_outputen(ddata, pwm, false);

I'm astonished that checkpatch doesn't spot the wrong indention here.

> +
> +	if (!state->enabled)
> +		return 0;
> +
> +	pwm_sg2044_set_outputdir(ddata, pwm, true);
> +	pwm_sg2044_set_outputen(ddata, pwm, true);
> +
> +	return 0;
> +}
> +
>  static const struct sg2042_chip_data sg2042_chip_data = {
>  	.ops = {
>  		.apply = pwm_sg2042_apply,
> @@ -142,11 +215,22 @@ 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,
> +	}

Missing , after }.

If you're ok, I'll pick up this version and fixup the two code changes
and the order of the tags in the commit log.

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/20250530/3d49cbb0/attachment.sig>


More information about the linux-riscv mailing list