linux-kernel - Re: [PATCH v6 11/11] pwm: Add Raspberry Pi Firmware based PWM bus

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] [day] [month] [year] [list]

Message-ID: <99a7d6e80646ff7ac6c1fc615146fc4ad1428552.camel@suse.de>
Date:   Thu, 14 Jan 2021 19:44:46 +0100
From:   Nicolas Saenz Julienne <nsaenzjulienne@...e.de>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
Cc:     linux-kernel@...r.kernel.org,
        Thierry Reding <thierry.reding@...il.com>,
        Lee Jones <lee.jones@...aro.org>, f.fainelli@...il.com,
        linux-pwm@...r.kernel.org, bcm-kernel-feedback-list@...adcom.com,
        linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
        wahrenst@....net, linux-input@...r.kernel.org,
        dmitry.torokhov@...il.com, gregkh@...uxfoundation.org,
        devel@...verdev.osuosl.org, p.zabel@...gutronix.de,
        linux-gpio@...r.kernel.org, linus.walleij@...aro.org,
        linux-clk@...r.kernel.org, sboyd@...nel.org,
        linux-rpi-kernel@...ts.infradead.org, bgolaszewski@...libre.com,
        andy.shevchenko@...il.com
Subject: Re: [PATCH v6 11/11] pwm: Add Raspberry Pi Firmware based PWM bus

Hi Uwe, thanks for the review.

On Tue, 2021-01-12 at 10:18 +0100, Uwe Kleine-König wrote:

[...]

> > +                duty_cycle = DIV_ROUND_CLOSEST_ULL(state->duty_cycle * RPI_PWM_MAX_DUTY,
> > +					           RPI_PWM_PERIOD_NS);
> 
> ... and round down here.
> 
> Just to be sure: writing RPI_PWM_MAX_DUTY (i.e. 255) yields 100% duty
> cycle, right?

Yes, at 255 the signal is flat.

> > +        else
> > +                duty_cycle = RPI_PWM_MAX_DUTY;
> > +
> > +	if (duty_cycle == rpipwm->duty_cycle)
> > +		return 0;
> > +
> > +	ret = raspberrypi_pwm_set_property(rpipwm->firmware, RPI_PWM_CUR_DUTY_REG,
> > +					   duty_cycle);
> > +	if (ret) {
> > +		dev_err(chip->dev, "Failed to set duty cycle: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	/*
> > +	 * This sets the default duty cycle after resetting the board, we
> > +	 * updated it every time to mimic Raspberry Pi's downstream's driver
> > +	 * behaviour.
> > +	 */
> > +	ret = raspberrypi_pwm_set_property(rpipwm->firmware, RPI_PWM_DEF_DUTY_REG,
> > +					   duty_cycle);
> > +	if (ret) {
> > +		dev_err(chip->dev, "Failed to set default duty cycle: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +        rpipwm->duty_cycle = duty_cycle;
> 
> Please use tabs for indention. (The general hint is to use checkpatch
> which (I hope) tells you about problems like this.)

Sorry for that.

I took note of the rest of comments and will update the code.

Regards,
Nicolas



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