Re: [PATCH v2 10/15] pwm: crc: Implement apply() method to support the new atomic PWM API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jun 09, 2020 at 03:44:18PM +0200, Hans de Goede wrote:
> On 6/9/20 1:32 PM, Andy Shevchenko wrote:
> > On Sun, Jun 07, 2020 at 08:18:35PM +0200, Hans de Goede wrote:

...

> > And again... :-(
> 
> Well yes I cannot help it that the original code, as submitted by Intel,
> was of very questionable quality, so instead of just converting it to the
> atomic PWM API I had to do a ton of bugfixes first...   I tried to do
> this all in small bits rather then in a single big rewrite the buggy
> <beep> commit to make life easier for reviewers.

Yes, I know about that old code quality, sorry, we were not at Intel that time
(or were just right-less newbies).

> I can introduce the crc_pwm_calc_clk_div helper earlier as you suggested
> in an earlier mail. I guess I could also keep the helper here, and then
> fold it into the function in a later commit (*).
> 
> Would that work for you ?

Definitely.

> *) Because having a helper for 3 lines of code when it is used only
> once is not helpful IMHO, it only makes it harder to figure out what
> the code is exactly doing when readin the code.

At least it will reduce churn to just

1) introduce foo();
2) do many changes with foo() being used;
3) drop foo() *if* it's not needed / makes little sense.

-- 
With Best Regards,
Andy Shevchenko





[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux