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
