Re: [PATCH] arm64: dts: rockchip: pwm-fan overlay for NanoPC-T6

[Hugh Cole-Baker <sigmaris@gmail.com>]

On 01/11/2025 11:44, Heiko Stuebner wrote:
> Am Montag, 27. Oktober 2025, 22:15:05 Mitteleuropäische Normalzeit schrieb Dragan Simic:
>> Hello Alexey,
>>
>> On Monday, October 27, 2025 21:56 CET, Alexey Charkov <alchark@gmail.com> wrote:
>>> On Tue, Oct 28, 2025 at 12:02 AM Dragan Simic <dsimic@manjaro.org> wrote:
>>>> On Mon, Oct 27, 2025 at 7:08 PM Hugh Cole-Baker <sigmaris@gmail.com> wrote:
>>>>> On 27/10/2025 09:14, Alexey Charkov wrote:
>>>>>
>>>>>> Is there any downside to enabling this unconditionally in the board
>>>>>> .dts?
>>>>>
>>>>> Only that it goes against the principle that the DT should describe the
>>>>> hardware; the board .dts would describe a cooling device that doesn't
>>>>> actually exist on the base board.
>>>>
>>>> Having a separate DT overlay is perfectly fine if we want to
>>>> describe a board absolutely correctly: if the fan actually isn't
>>>> present, the operating system shouldn't be made to think it is
>>>> there, especially if there's no fan RPM feedback, which is the
>>>> case on almost all Rockchip boards that support a fan.
>>>>
>>>> Preventing the kernel from managing a non-existent fan might even
>>>> save some CPU cycles, ending up producing a bit less heat, which
>>>> can only help in passively cooled setups.
>>>
>>> Sounds like an overcomplication without real benefit. It's one thing
>>> with overlays for functionality that can be software-incompatible
>>> depending on whether an external attachment is connected or depending
>>> on the type of attachment connected. Here we are looking at a plain
>>> 2-pin fan which cannot be software incompatible to anything really
>>> (it's not like one could repurpose the fan connector for anything
>>> meaningful if a fan is not in use, and noone gets hurt if a PWM output
>>> is left running without load).
>>>
>>> The CPU cycles spent parsing a slightly larger DTB at boot are likely
>>> comparable to those spent activating a PWM output needlessly upon
>>> hitting the active cooling trip point, and both are negligible for any
>>> practical purpose.
>>
>> Hmm, right, I forgot for a moment that the PWM output is generated
>> by dedicated hardware, so not many CPU cycles would be wasted.
>>
>> BTW, with a fan PWM signal generated by a soft-GPIO output, much
>> more CPU cycles would've been saved by omitting the fan definition
>> if it isn't present, but that isn't the case here.
>>
>>>> However, the practice so far has been to describe the fans in the
>>>> main board dts files, if the board provides fan support, regardless
>>>> of the fan being present in a particular board setup or not.
>>>>
>>>>> I guess then in theory, an OS might allow the SoC to reach undesirably high
>>>>> temperatures if it's relying on the nonexistent fan to cool it down. But I
>>>>> don't think this would be an issue on Linux, at least, in practice.
>>>>
>>>> We're safe, a thermal runaway isn't going to happen when the fan is
>>>> defined in a board DT but actually isn't present.  Thermal CPU and
>>>> GPU throttling will prevent the overheating from happening.
>>>>
>>>>>> Overlays require more user configuration, and not all
>>>>>> bootloaders support them directly (e.g. systemd-boot users would
>>>>>> struggle). Compiling with overlays enabled also makes .dtb's a lot
>>>>>> larger due to added symbols information.
>>>>>
>>>>> Nowadays (on Debian at least) using overlays is pretty easy, I'm using the
>>>>> u-boot-menu package in Debian, I just copy the overlay(s) to /boot/dtbo/ and
>>>>> it detects them automatically and adds them to extlinux.conf for u-boot to
>>>>> apply.
>>>>>
>>>>> Couldn't systemd-boot users just use rk3588-nanopc-t6-(lts-)with-fan.dtb as
>>>>> their single DT to load, if it doesn't support applying overlays and they
>>>>> want to use the fan addon?
>>>
>>> Sure, but it's a manual configuration step, where otherwise the kernel
>>> would just default to the correct dtb for the board that the firmware
>>> told it about. The fan is not discoverable, so the firmware won't ever
>>> offer the "-with-fan" variant, meaning users would need to supply it
>>> manually in every instance.
>>
>> FWIW, the most user-friendly SBC family in the world, Raspberry
>> Pi, :) requires manual enabling of the fan on Raspberry Pi 4.
>> I haven't researched what's the background for that, perhaps the
>> need to keep the GPIO expansion header completely unoccupied by
>> default, because the fan attaches to the GPIO header, instead of
>> to some dedicated fan connector.
>>
>>>> Yes, that's an option.  However, that in general doesn't resolve
>>>> the issues arising from systemd-boot users wanting to apply more
>>>> than a single DT overlay.
>>>>
>>>>> FWIW, I haven't noticed any problems with having a larger .dtb (using mainline
>>>>> U-Boot to load it) and several other RK3588 boards are also compiled with
>>>>> symbols enabled already, and I haven't seen any issues reported with them.
>>>>
>>>> After thinking a bit about it, I'd support the extraction of fan
>>>> definitions into separate DT overlays.  As I wrote above already,
>>>> not managing the non-existent fan might actually help a bit with
>>>> passively cooled board setups, which is a good enough reason for
>>>> me to support separate DT overlays.
>>>
>>> Practical benefits sound far fetched here, while forcing users to
>>> manually configure something that would have otherwise just worked.
>>> Let's see what Heiko thinks.
> 
> Personally, I'm more on the less complication side.
> 
> I.e. if there is an actual fan-connector on the board we should describe
> it as such.
> 
> Overlays I see for things where you attach hats to generic pin headers
> to create specific functionality on top of a generic interface.
> 
> But if the board itself has an actual fan header, it should be described
> as such. Because that then _is_ the standard use of that.

The board does have a fan connector, just no fan by default. But anyway,
since it sounds like the preferred approach I'll send a v2 which puts the
fan into the base board .dts.

Dragan, you mentioned there's no need for more than 2 trip points - if
I remove the trip points between "SoC is warm, start fan at slow speed"
and "SoC is v. hot, run fan at full speed" is the OS/kernel expected to
interpolate between those 2 trip points (if you have a link to docs or
code about this it'd be interesting, I couldn't find anything in the
dt-bindings)?

Best regards, Hugh