linux-kernel - Re: [PATCH v2 3/6] pwm: jz4740: Apply configuration atomically

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: <1563914800.1918.0@crapouillou.net>
Date:   Tue, 23 Jul 2019 16:46:40 -0400
From:   Paul Cercueil <paul@...pouillou.net>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>, od@...c.me,
        linux-pwm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/6] pwm: jz4740: Apply configuration atomically

Hi Uwe,



Le lun. 22 juil. 2019 à 15:34, Uwe =?iso-8859-1?q?Kleine-K=F6nig?= 
<u.kleine-koenig@...gutronix.de> a écrit :
> Hello Paul,
> 
> On Fri, Jun 07, 2019 at 05:44:07PM +0200, Paul Cercueil wrote:
>>  -static int jz4740_pwm_config(struct pwm_chip *chip, struct 
>> pwm_device *pwm,
>>  -			     int duty_ns, int period_ns)
>>  +static int jz4740_pwm_apply(struct pwm_chip *chip, struct 
>> pwm_device *pwm,
>>  +			    struct pwm_state *state)
>>   {
>>   	struct jz4740_pwm_chip *jz4740 = to_jz4740(pwm->chip);
>>   	unsigned long long tmp;
>>   	unsigned long period, duty;
>>   	unsigned int prescaler = 0;
>>   	uint16_t ctrl;
>>  -	bool is_enabled;
>> 
>>  -	tmp = (unsigned long long)clk_get_rate(jz4740->clk) * period_ns;
>>  +	tmp = (unsigned long long)clk_get_rate(jz4740->clk) * 
>> state->period;
>>   	do_div(tmp, 1000000000);
>>   	period = tmp;
>> 
>>  @@ -96,16 +95,14 @@ static int jz4740_pwm_config(struct pwm_chip 
>> *chip, struct pwm_device *pwm,
>>   	if (prescaler == 6)
>>   		return -EINVAL;
>> 
>>  -	tmp = (unsigned long long)period * duty_ns;
>>  -	do_div(tmp, period_ns);
>>  +	tmp = (unsigned long long)period * state->duty_cycle;
>>  +	do_div(tmp, state->period);
>>   	duty = period - tmp;
>> 
>>   	if (duty >= period)
>>   		duty = period - 1;
>> 
>>  -	is_enabled = jz4740_timer_is_enabled(pwm->hwpwm);
>>  -	if (is_enabled)
>>  -		jz4740_pwm_disable(chip, pwm);
>>  +	jz4740_pwm_disable(chip, pwm);
> 
> I assume this stops the PWM. Does this complete the currently running
> period? How does the PWM behave then? (Does it still drive the output?
> If so, on which level?)

Some PWM channels work in one mode "TCU1" and others work in "TCU2". The
mode in which channels work depends on the version of the SoC.

When stopped, the pins of TCU1 channels will be driven to the inactive
level (which depends on the polarity). It is unknown whether or not the
currently running period is completed. We set a bit to configure for
"abrupt shutdown", so I expect that it's not, but somebody would need
to hook up a logic analyzer to see what's the exact behaviour with
and without that bit.

TCU2 channels on the other hand will stop in the middle of a period,
leaving the pin hanging at whatever level it was before the stop.
That's the rationale behind the trick in commit 6580fd173070 ("pwm:
jz4740: Force TCU2 channels to return to their init level").

Regards,
-Paul


>> 
>>   	jz4740_timer_set_count(pwm->hwpwm, 0);
>>   	jz4740_timer_set_duty(pwm->hwpwm, duty);
> 
> Best regards
> Uwe
> 
> --
> Pengutronix e.K.                           | Uwe Kleine-König        
>     |
> Industrial Linux Solutions                 | 
> http://www.pengutronix.de/  |