Re: [PATCH v16 1/2] pwm: add microchip soft ip corePWM driver

[Conor Dooley]

On Tue, Apr 18, 2023 at 03:08:37PM +0200, Uwe Kleine-König wrote:
> On Tue, Apr 18, 2023 at 12:27:33PM +0100, Conor Dooley wrote:
> > On Tue, Apr 11, 2023 at 06:25:54PM +0200, Uwe Kleine-König wrote:


> I don't understand what you wanna say here. If tmp = 256 my suggestion
> is to pick prescale = 0 and period_steps = 254. Then
> 
> 	 (prescale + 1) * (period_steps + 1) ≤ tmp
> 
> and period_steps is big enough to ensure a a finegrained choice for the
> duty_cycle.
> 
> > That's then gonna give us one of the broken configurations from the
> > limitations.
> > 
> > tmp = 257
> > 
> > *prescale = 257 // (254 + 1) - 1
> >           ≈ 0
> > 
> > *prescale = 257 // (prescale + 1) - 1
> >           = 257 / (0 + 1) - 1
> > 	  = 256
> > 	  = 0 (registers are 8-bit)
> 
> I think you mean s/prescale/period_steps/ in the second part, but that's
> not what I meant. I meant to suggest:

I did, yeah!

> 	*prescale = tmp / (MCHPCOREPWM_PERIOD_STEPS_MAX + 1) - 1
> 	period_steps = MCHPCOREPWM_PERIOD_STEPS_MAX = 254
> 
> > I'm quite obviously missing something that you may think is obvious
> > here, but is not immediately clear to me.
> 
> That would be an explanation, yes. :-)

Right, it makes a lot more sense now. Definitely was not clear to me
that that was what you were suggesting.
I'm not sure that disallowing tmp < 255 is something I want to do
though, as this is mainly used as a "soft" IP core in the FPGA fabric,
the clock provided to it may not be particularly high.
Probably not the end of the world though, once added to the limitations.

The implemented period is also going to be quite a ways off with this
method (compared to the method I have been using until now) - although
it is of course far simpler.
You're the PWM expert and are suggesting it, so maybe I should just shut
up and go do it...

Attachment: signature.asc
Description: PGP signature