Hi Naoki, On Tue, Dec 10, 2024 at 3:30 AM FUKAUMI Naoki <naoki@xxxxxxxxx> wrote: > > Hi, > > Thanks for your review! > > On 12/10/24 01:32, Dragan Simic wrote: > > Hello Fukaumi, > > > > On 2024-12-09 13:51, FUKAUMI Naoki wrote: > >> fix pwm period to match with vendor kernel[1]. > > > > Instead of simply referring to the downstream vendor kernel, in this > > specific case the reasons for adjusting the fan PWM parameters should > > be explained by referring to the actual fan setup you're using, the > > observed fan RPM behavior, etc. > > original commit message is: > > | arm64: dts: rockchip: modify fan pwm period to 60us > | Reduce pwm frequency to 16.6 KHz for a larger adjustable range of > AO3416 mosfet. > > I have no knowledge about this kind of things. Is quoting this message > enough? I think it would be better to expand a bit to make sure the commit message explains the whole rationale without too much extra digging. Something like this: arm64: dts: rockchip: Use a longer PWM period for the fan on Radxa ROCK 5C The fan on Radxa ROCK 5C is driven via an AO3416 MOSFET, which has a total switch-on time of 0,6us and a total switch-off time of 6us [1], meaning that the current PWM period of just 10us is too short for fine-grained fan speed control. Increase the PWM period to 60us, so that the switch-on and switch-off time of the MOSFET fall within a more reasonable ~10% of the full period, thus making lower PWM duty cycles meaningful. [1] https://www.aosmd.com/pdfs/datasheet/AO3416.pdf Best regards, Alexey
