Hi Uwe,
Thanks for review.
On 22/5/2020 4:56 pm, Uwe Kleine-König wrote:
> Hello,
>
> On Fri, May 22, 2020 at 03:41:59PM +0800, Rahul Tanwar wrote:
>> Add PWM controller driver for Intel's Lightning Mountain(LGM) SoC.
>>
>> Signed-off-by: Rahul Tanwar <rahul.tanwar@xxxxxxxxxxxxxxx>
>> ---
>> drivers/pwm/Kconfig | 9 ++
>> drivers/pwm/Makefile | 1 +
>> drivers/pwm/pwm-intel-lgm.c | 356 ++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 366 insertions(+)
>> create mode 100644 drivers/pwm/pwm-intel-lgm.c
>>
>> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
>> index eebbc917ac97..a582214f50b2 100644
>> --- a/drivers/pwm/Kconfig
>> +++ b/drivers/pwm/Kconfig
>> @@ -232,6 +232,15 @@ config PWM_IMX_TPM
>> To compile this driver as a module, choose M here: the module
>> will be called pwm-imx-tpm.
>>
>> +config PWM_INTEL_LGM
>> + tristate "Intel LGM PWM support"
>> + depends on X86 || COMPILE_TEST
>> + help
>> + Generic PWM framework driver for LGM SoC.
[...]
>> +};
>> +
>> +static void tach_work(struct work_struct *work)
>> +{
>> + struct intel_pwm_chip *pc = container_of(work, struct intel_pwm_chip,
>> + work.work);
>> + struct regmap *regmap = pc->regmap;
>> + u32 fan_tach, fan_dc, val;
>> + s32 diff;
>> + static u32 fanspeed_err_cnt, time_window, delta_dc;
>> +
>> + /*
>> + * Fan speed is tracked by reading the active duty cycle of PWM output
>> + * from the active duty cycle register. Some variance in the duty cycle
>> + * register value is expected. So we set a time window of 30 seconds and
>> + * if we detect inaccurate fan speed 6 times within 30 seconds then we
>> + * mark it as fan speed problem and fix it by readjusting the duty cycle.
>> + */
> I'm a unhappy to have this in the PWM driver. The PWM driver is supposed
> to be generic and I think this belongs into a dedicated driver.
Well noted about all other review concerns. I will rework the driver in v2.
However, i am not very sure about the above point - of having a separate
dedicated driver for tach_work because its logic is tightly coupled with
this driver.
Regards,
Rahul
