linux-kernel - Re: [PATCH v7 2/2] pwm: Add clock based PWM output 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: <20220711070349.udyej2qxj2hqyowz@pengutronix.de>
Date:   Mon, 11 Jul 2022 09:03:49 +0200
From:   Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To:     Nikita Travkin <nikita@...n.ru>
Cc:     thierry.reding@...il.com, lee.jones@...aro.org, robh+dt@...nel.org,
        sboyd@...nel.org, krzk@...nel.org, linus.walleij@...aro.org,
        masneyb@...tation.org, sean.anderson@...o.com,
        linux-pwm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, ~postmarketos/upstreaming@...ts.sr.ht
Subject: Re: [PATCH v7 2/2] pwm: Add clock based PWM output driver

On Mon, Jul 11, 2022 at 10:55:09AM +0500, Nikita Travkin wrote:
> Uwe Kleine-König писал(а) 01.07.2022 12:50:
> > On Sun, Jun 12, 2022 at 06:22:03PM +0500, Nikita Travkin wrote:
> >> Some systems have clocks exposed to external devices. If the clock
> >> controller supports duty-cycle configuration, such clocks can be used as
> >> pwm outputs. In fact PWM and CLK subsystems are interfaced with in a
> >> similar way and an "opposite" driver already exists (clk-pwm). Add a
> >> driver that would enable pwm devices to be used via clk subsystem.
> >>
> >> Signed-off-by: Nikita Travkin <nikita@...n.ru>
> >> --
> >>
> >> Changes in v2:
> >>  - Address Uwe's review comments:
> >>    - Round set clk rate up
> >>    - Add a description with limitations of the driver
> >>    - Disable and unprepare clock before removing pwmchip
> >> Changes in v3:
> >>  - Use 64bit version of div round up
> >>  - Address Uwe's review comments:
> >>    - Reword the limitations to avoid incorrect claims
> >>    - Move the clk_enabled flag assignment
> >>    - Drop unnecessary statements
> >> Changes in v5:
> >>  - add missed returns
> >> Changes in v6:
> >>  - Unprepare the clock on error
> >>  - Drop redundant limitations points
> >> Changes in v7:
> >>  - Rename some variables to be in line with common naming
> >>
> >> --
> >> It seems like my mailserver wasn't able to send the last review
> >> response to Uwe's so I'll repeat here that afaict clk.h has all the
> >> methods stubbed out so compiling without HAVE_CLK is possible.
> >> Sorry for a long delay with sending this since v6.

FTR: The only problems I have with mail sending in this thread is to
"devicetree@...r.kernel.or", I added a 'g' for this mail to that address
:-) Otherwise if you diagnose to have problems with the pengutronix
server accepting your mail, I'd like to hear about that.

> >> +	pcchip = devm_kzalloc(&pdev->dev, sizeof(*pcchip), GFP_KERNEL);
> >> +	if (!pcchip)
> >> +		return -ENOMEM;
> >> +
> >> +	pcchip->clk = devm_clk_get(&pdev->dev, NULL);
> > 
> > You can use devm_clk_get_prepared() here and drop the clk_prepare()
> > below and the clk_unprepare in .remove().
> 
> Here I spent a bit of time trying to remember why I thought
> I've already looked at this, but after figuring out that this
> devm helper didn't even exist earlier (I only see it in clk-next)
> I remembered considering a totally different thing (being
> clk_disable_unprepare in the _remove, which doesn't play well)
> 
> Given that this seems to be absent from 5.19-rc6, I'm afraid adding
> it here will upset the 0day as well as possibly cause issues in case
> both are taken for the same merge window...

Pass --base with a sensible parameter to git-format-patch (or
git-send-email) to make the 0day bots happy.

> On the other hand it takes me quite a while to provide replies for
> this series (the trend I'm not happy with) so maybe 3-4 weeks
> will indeed pass for 5.20-rc1 to have it...

It's not me who merges PWM patches but Thierry. I don't know his plans
and if he would be willing to pick up a new driver for the next cycle.
You might still get lucky with a fast next iteration.

If you want ignore the devm_clk_get_prepared() suggestion, we can still
convert to that once both patches hit Linus Torvald's tree.

> I think I will try to send a new version with just the comment
> added shortly in case it's still not too late for the next merge
> window and you can feel free to nack it if you think it already is :)

I think the driver looks good otherwise, so I don't expect to have more
feedback in the next round.

Best regards
Uwe

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

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