lkml.org 
[lkml]   [2022]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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@trvn.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@vger.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/ |
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2022-07-11 09:04    [W:0.054 / U:3.164 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site