Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxx> 於 2024年12月27日 週五 下午4:20寫道: > > Hello Nylon, > > On Tue, Dec 24, 2024 at 05:38:58PM +0800, Nylon Chen wrote: > > According to the circuit diagram of User LEDs - RGB described in the > > manual hifive-unleashed-a00.pdf[0] and hifive-unmatched-schematics-v3.pdf[1]. > > > > The behavior of PWM is acitve-high. > > > > According to the descriptionof PWM for pwmcmp in SiFive FU740-C000 Manual[2]. > > > > The pwm algorithm is (PW) pulse active time = (D) duty * (T) period. > > The `frac` variable is pulse "inactive" time so we need to invert it. > > I'm trying to understand that. You're saying that the PWMCMP register > holds the inactive time. Looking at the logic diagram (Figure 29) of > "SiFive FU740-C000 Manual v1p6" that is because pwms is feed into the > comparator after going through that XNOR where the lower input is always > 0 (as pwmcmpXcenter is always 0) and so effectively counts backwards, > right? > In that case the sentence "The output of each comparator is high > whenever the value of pwms is greater than or equal to the corresponding > pwmcmpX." from the description of the Compare Registers is wrong. > Hi Uwe, Please give us some time to clarify these questions, thank you. > With that assumption there are a few issues with the second patch: > > - The Limitations paragraph still says "The hardware cannot generate a > 100% duty cycle." > - If pwm_sifive_apply() is called with state->duty_cycle = 0 the PWMCMP > register becomes (1U << PWM_SIFIVE_CMPWIDTH) - 1 which results in a > wave form that is active for 1 clock tick each period. That's bogus. > If duty_cycle = 0 is requested, either make sure the output is > inactive the whole time, or return an error. > - With the above error in the official documentation, I'd like to have > a code comment that explains the mismatch such that a future reader > of the code has a chance to understand the situation without in > detail review of the manual and the driver. > > Orthogonal to your patches, I wonder about > > frac = DIV64_U64_ROUND_CLOSEST(num, state->period); > > . Round-closest is usually wrong in an .apply() callback. I didn't do > the detailed math, but I think you need to round up here. I will conduct relevant experiments to clarify this issue. Thanks again. > > Best regards > Uwe