Hello,
On Mon, May 03, 2021 at 04:42:59AM +0000, Billy Tsai wrote:
> On 2021/4/27, 4:44 AM,Uwe Kleine-Königwrote:
> >> +/* PWM fixed value */
> >> +#define PWM_FIXED_PERIOD 0xff
> >> +
> >> +struct aspeed_pwm_data {
> >> + struct pwm_chip chip;
> >> + struct clk *clk;
> >> + struct regmap *regmap;
> >> + struct reset_control *reset;
> >> +};
> >> +
> >> +static void aspeed_set_pwm_channel_enable(struct regmap *regmap, u8 pwm_channel,
> >> + bool enable)
> >> +{
> >> + regmap_update_bits(regmap, PWM_ASPEED_CTRL_CH(pwm_channel),
> >> + (PWM_CLK_ENABLE | PWM_PIN_ENABLE),
> >> + enable ? (PWM_CLK_ENABLE | PWM_PIN_ENABLE) : 0);
>
> > What is the semantic difference between CLK_ENABLE and PIN_ENABLE? Does
> > the pin stay at it's inactive level if PIN_ENABLE is unset? Does the
> > output just freeze at it's current level if CLK_ENABLE is unset?
>
> Yes.
> When PIN_ENABLE is unset the pwm controller will always output low to the extern.
> When CLK_ENABLE is unset the pwm controller will freeze at it's current level.
These two are relevant to mention at the top of the driver.
> > The intended goal is to provide the biggest possible period not bigger
> > than the requested value.
>
> So, did you mean that if the request period is 100ns and our divide can reach 100.1ns or 95ns
> the user prefer 95ns to 100.1ns?
Yes. It's unclear if the user really prefers 95ns, but to get a
consistent behaviour among the different drivers, that's what I ask you
to implement.
Currently there is no way that allows a consumer to specify which
setting they prefer, I have an idea for a fix though. For that it is
also important that .apply() doesn't yield 100.1 ns.
> >> + dev_dbg(dev, "freq: %d, duty_pt: %d", freq, duty_pt);
> >> + if (state->enabled) {
> >> + aspeed_set_pwm_freq(priv, pwm, freq);
> >> + aspeed_set_pwm_duty(priv, pwm, duty_pt);
> >> + aspeed_set_pwm_polarity(priv, pwm, state->polarity);
>
> > How does the hardware behave in between these calls? If for example the
> > polarity is changed, does this affect the output immediately? Does this
> > start a new period?
>
> The pwm output will inverse immediately. The period will not change.
Please mention that at the top of the driver.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
