Re: [PATCH v1 2/2] Add PWM driver for LGM

[Tanwar, Rahul]

On 27/5/2020 5:15 pm, Andy Shevchenko wrote:
> On Wed, May 27, 2020 at 02:28:53PM +0800, Tanwar, Rahul wrote:
>> On 22/5/2020 4:56 pm, Uwe Kleine-KÃnig wrote:
>>> On Fri, May 22, 2020 at 03:41:59PM +0800, Rahul Tanwar wrote:
> ...
>
>>> 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.
> Actually I agree with Uwe.
> Here is layering violation, i.e. provider and consumer in the same pot. It's
> not good from design perspective.
>

Just to clarify, the PWM controller in our SoC serves just one purpose which
is to control the fan. Its actually named as PWM Fan Controller. There is no
other generic usage or any other consumer for this PWM driver. So separating
out this part seems redundant to me. Also, if we separate it out as a
dedicated driver, this will endup as a very small daemon which is going to be
very hard to justify while upstreaming..

Regards,
RahulÂ