Hi,
On 6/9/20 1:32 PM, Andy Shevchenko wrote:
On Sun, Jun 07, 2020 at 08:18:35PM +0200, Hans de Goede wrote:
Replace the enable, disable and config pwm_ops with an apply op, to support the new atomic PWM API.
...
-static int crc_pwm_calc_clk_div(int period_ns) +static int crc_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
const struct pwm_state *state)
- int clk_div;
- clk_div = PWM_BASE_CLK_MHZ * period_ns / (256 * NSEC_PER_MHZ);
- /* clk_div 1 - 128, maps to register values 0-127 */
- if (clk_div > 0)
clk_div--;- return clk_div;
-}
...
clk_div = PWM_BASE_CLK_MHZ * state->period /(256 * NSEC_PER_MHZ);/* clk_div 1 - 128, maps to register values 0-127 */if (clk_div > 0)clk_div--;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.
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 ?
Regards,
Hans
*) 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.