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


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux