| Message ID | 20250428013501.6354-4-looong.bin@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | riscv: pwm: sophgo: add pwm support for SG2044 | expand |
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
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
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");