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
| ||
|
Date: Mon, 24 Apr 2017 21:08:09 +0800 From: Yang Ling <gnaygnil@...il.com> To: Marcin Nowakowski <marcin.nowakowski@...tec.com> Cc: Yang Ling <gnaygnil@...il.com>, thierry.reding@...il.com, keguang.zhang@...il.com, linux-kernel@...r.kernel.org, linux-pwm@...r.kernel.org, linux-mips@...ux-mips.org Subject: Re: [PATCH 1/2] pwm: loongson1: Add PWM driver for Loongson1 SoC Hi, Marcin, I am sorry for the late reply. On Thu, Feb 16, 2017 at 10:27:15AM +0100, Marcin Nowakowski wrote: > Hi Yang, > > On 15.02.2017 14:09, Yang Ling wrote: > > >>>+ tmp = (unsigned long long)clk_get_rate(pc->clk) * period_ns; > >>>+ do_div(tmp, 1000000000); > > NSEC_PER_SEC ? > Indeed, NSEC_PER_SEC should be used. > >>>+ period = tmp; > >>>+ > >>>+ tmp = (unsigned long long)period * duty_ns; > >>>+ do_div(tmp, period_ns); > >>>+ duty = period - tmp; > >>>+ > >>>+ if (duty >= period) > >>>+ duty = period - 1; > >>>+ > >>>+ if (duty >> 24 || period >> 24) > >>>+ return -EINVAL; > >>>+ > >>>+ chan->period_ns = period_ns; > >>>+ chan->duty_ns = duty_ns; > >>>+ > >>>+ writel(duty, pc->base + PWM_HRC(pwm->hwpwm)); > >>>+ writel(period, pc->base + PWM_LRC(pwm->hwpwm)); > >>>+ writel(0x00, pc->base + PWM_CNT(pwm->hwpwm)); > >>>+ > >> > >>PWM_HRC and PWM_LRC names suggest that you're using high/low state > >>counters here rather than duty/period - but with no documentation > >>I'm just guessing here. > > > >Indeed, the high/low state counters is used here. > >Change the name to duty_cnt/period_cnt. > > > > > > What I was referring to here is that if you have a high/low value counters > that you enter then these are not the same as duty/period, in simple terms: > high_cnt = duty_cnt > low_cnt = period_cnt - duty_cnt > > so please double check that this is what you want to be doing? As the names > used suggest that this code may be wrong. Or maybe what you're doing is > correct but the register access macros have misleading names? > The macro definition of the register here is misleading. I will fix these problems afterwards. Thanks for your friendly reminder. Yang
Powered by blists - more mailing lists