Hello Biju, On Wed, Dec 04, 2024 at 06:24:19PM +0000, Biju Das wrote: > > > + * different settings. > > > + */ > > > + if (rzg2l_gpt->user_count[ch] > 1 && period_ticks < rzg2l_gpt->period_ticks[ch]) > > > + return -EBUSY; > > Do we need to allow this operation (period_ticks < rzg2l_gpt->period_ticks[ch]) ? > > For example, > First IO (IO_A) period/duty is in the order nsec (PWM frequency in MHz) and second channel period/duty in the > order of microsec(PWM frequency in kHz) > > Allowing period_ticks < rzg2l_gpt->period_ticks[ch] will lead to incorrect operations > for First IO (IO_A) as PWM frequency will be in kHz compared to MHz. Well, the policy is to pick the highest possible period not bigger than the requested period. So if B is asked to be set to 5ms and 5ns is the highest currently possible value, that's it. I agree that being off by a factor of 1000 isn't nice. But if you say this is too much, you have to draw a line somewhere. Where should that be? Everything you pick is arbitrary and I'm sure there is a use case for every choice where it's wrong. Additionally if you are too picky -- in the extreme case don't allow any modification >= 1ns -- the API becomes very hard to use. The only sensible way out is to allow consumers to query the result before actually applying a request. This is in the making and if you want to benefit from it, look at the waveform stuff that recently hit mainline. (deaba9cff8092cbb2bef4dc79a6ce296017904b1 is an example driver conversion, 6c5126c6406d1c31e91f5b925c621c1c785366be and 17e40c25158f2505cbcdeda96624afcbab4af368 are the relevant core changes.) > According to me, we should not allow updating periods for second enabled channel. Not entirely sure you mean the right thing here. If IO A operates at 5ns and IO B is requested to set .period = 5 ms, every operation that also changes A is out-of-bounds. So your options are only: Use the 5ns, or return an error. The latter is hard for consumers because it's unclear what to do then. Best regards Uwe
Attachment:
signature.asc
Description: PGP signature
