[PATCH v4 03/12] arm64: dts: rockchip: fix property for pwm-fan for Radxa ROCK 5C

[Alexey Charkov]

Hi Naoki,

On Tue, Dec 10, 2024 at 3:30 AM FUKAUMI Naoki <naoki at radxa.com> 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