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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230418-octagon-epilepsy-4d8516f91145@wendy>
Date:   Tue, 18 Apr 2023 14:27:09 +0100
From:   Conor Dooley <conor.dooley@...rochip.com>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
CC:     Thierry Reding <thierry.reding@...il.com>,
        Daire McNamara <daire.mcnamara@...rochip.com>,
        <linux-kernel@...r.kernel.org>, <linux-pwm@...r.kernel.org>,
        <linux-riscv@...ts.infradead.org>
Subject: Re: [PATCH v16 1/2] pwm: add microchip soft ip corePWM driver

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...

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)