linux-kernel - Re: [PATCH v8 11/26] pwm: jz4740: Use regmap and clocks from TCU 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: <20181213093049.rxdvf6tip7iqdj3c@pengutronix.de>
Date:   Thu, 13 Dec 2018 10:30:49 +0100
From:   Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
To:     Paul Cercueil <paul@...pouillou.net>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ralf Baechle <ralf@...ux-mips.org>,
        Paul Burton <paul.burton@...s.com>,
        James Hogan <jhogan@...nel.org>,
        Jonathan Corbet <corbet@....net>,
        Mathieu Malaterre <malat@...ian.org>,
        Ezequiel Garcia <ezequiel@...labora.co.uk>,
        PrasannaKumar Muralidharan <prasannatsmkumar@...il.com>,
        linux-pwm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-watchdog@...r.kernel.org,
        linux-mips@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-clk@...r.kernel.org, od@...c.me
Subject: Re: [PATCH v8 11/26] pwm: jz4740: Use regmap and clocks from TCU
 driver

Hello,

On Wed, Dec 12, 2018 at 11:09:06PM +0100, Paul Cercueil wrote:
> [...]
>  static int jz4740_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>  {
> -	uint32_t ctrl = jz4740_timer_get_ctrl(pwm->pwm);
> +	struct jz4740_pwm_chip *jz = to_jz4740(chip);
>  
> -	ctrl |= JZ_TIMER_CTRL_PWM_ENABLE;
> -	jz4740_timer_set_ctrl(pwm->hwpwm, ctrl);
> -	jz4740_timer_enable(pwm->hwpwm);
> +	/* Enable PWM output */
> +	regmap_update_bits(jz->map, TCU_REG_TCSRc(pwm->hwpwm),
> +				TCU_TCSR_PWM_EN, TCU_TCSR_PWM_EN);

Usually follow-up lines are indented to the matching parenthesis.

> [...]
>  static int jz4740_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>  			     int duty_ns, int period_ns)
>  {
>  	struct jz4740_pwm_chip *jz4740 = to_jz4740(pwm->chip);
> +	struct clk *clk = jz4740->clks[pwm->hwpwm];
> +	unsigned long rate, new_rate, period, duty;
>  	unsigned long long tmp;
> -	unsigned long period, duty;
> -	unsigned int prescaler = 0;
> -	uint16_t ctrl;
> +	unsigned int tcsr;
>  	bool is_enabled;
>  
> -	tmp = (unsigned long long)clk_get_rate(jz4740->clk) * period_ns;
> -	do_div(tmp, 1000000000);
> -	period = tmp;
> +	rate = clk_get_rate(clk);
> +
> +	for (;;) {
> +		tmp = (unsigned long long) rate * period_ns;
> +		do_div(tmp, 1000000000);
>  
> -	while (period > 0xffff && prescaler < 6) {
> -		period >>= 2;
> -		++prescaler;
> +		if (tmp <= 0xffff)
> +			break;
> +
> +		new_rate = clk_round_rate(clk, rate / 2);
> +
> +		if (new_rate < rate)
> +			rate = new_rate;
> +		else
> +			return -EINVAL;
>  	}
>  
> -	if (prescaler == 6)
> -		return -EINVAL;
> +	clk_set_rate(clk, rate);

Maybe this could better live in a separate patch. If you split still
further to have the conversion to regmap in a single patch, then the
conversion to the clk_* functions and then improve the algorithm for the
clk settings each of the patches is easier to review than this one patch
that does all three things at once.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |