diff mbox

ARM: dts: rockchip: add the MiQi board's fan definition

Message ID 1486803415-14680-1-git-send-email-w@1wt.eu (mailing list archive)
State New, archived
Headers show

Commit Message

Willy Tarreau Feb. 11, 2017, 8:56 a.m. UTC
The MiQi board is sold with an enclosure in which a fan is connected
to the second LED output, and configured by default in "heartbeat"
mode so that it rotates slowly and increases when the CPU load
increases, ensuring appropriate cooling by default. This LED output
is called "Fan" in the original kernel and connected to GPIO18
(gpiochip 0, pin 18). Here we called it "miqi:green:fan" to stay
consistent with the kernel's naming conventions.

It's worth noting that without this patch the fan doesn't work at
all, risking to make the board overheat.

Fixes: 162718c (v4.7)
Cc: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 arch/arm/boot/dts/rk3288-miqi.dts | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Heiko Stübner Feb. 11, 2017, 4:33 p.m. UTC | #1
Hi Willy,

Am Samstag, 11. Februar 2017, 09:56:55 CET schrieb Willy Tarreau:
> The MiQi board is sold with an enclosure in which a fan is connected
> to the second LED output, and configured by default in "heartbeat"
> mode so that it rotates slowly and increases when the CPU load
> increases, ensuring appropriate cooling by default. This LED output
> is called "Fan" in the original kernel and connected to GPIO18
> (gpiochip 0, pin 18). Here we called it "miqi:green:fan" to stay
> consistent with the kernel's naming conventions.

I tend to disagree with this approach. A fan is not a led and the devicetree 
is about describing the hardware, not how a specific kernel likes to use things 
:-) .

The kernel already has a gpio-fan driver (drivers/hwmon) or you could 
resurrect the gpio-pwm patch [0] from Olliver Schinagl and use the pwm-fan on 
top of that for more intermediate steps.


> It's worth noting that without this patch the fan doesn't work at
> all, risking to make the board overheat.

At least cpufreq is already hooked to the thermal controller on the rk3288, so 
even without additional cooling it should select lower cpu frequencies keeping 
the heat in line and prevent overheating the board.

Both solutions shown above also hook into the thermal framework and act as 
cooling device according real temperature readings from the chip and not 
trying to determine some sort of cooling from load in a somewhat hacky way. 


Heiko

[0] https://patchwork.kernel.org/patch/7492201/


> 
> Fixes: 162718c (v4.7)
> Cc: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Willy Tarreau <w@1wt.eu>
> ---
>  arch/arm/boot/dts/rk3288-miqi.dts | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/rk3288-miqi.dts
> b/arch/arm/boot/dts/rk3288-miqi.dts index 2f4c635..ef702db 100644
> --- a/arch/arm/boot/dts/rk3288-miqi.dts
> +++ b/arch/arm/boot/dts/rk3288-miqi.dts
> @@ -67,6 +67,12 @@
>  	leds {
>  		compatible = "gpio-leds";
> 
> +		fan {
> +			gpios = <&gpio0 18 GPIO_ACTIVE_HIGH>;
> +			label = "miqi:green:fan";
> +			linux,default-trigger = "heartbeat";
> +		};
> +
>  		work {
>  			gpios = <&gpio7 2 GPIO_ACTIVE_HIGH>;
>  			label = "miqi:green:user";
Willy Tarreau Feb. 11, 2017, 5:45 p.m. UTC | #2
Hi Heiko,

On Sat, Feb 11, 2017 at 05:33:16PM +0100, Heiko Stuebner wrote:
> Hi Willy,
> 
> Am Samstag, 11. Februar 2017, 09:56:55 CET schrieb Willy Tarreau:
> > The MiQi board is sold with an enclosure in which a fan is connected
> > to the second LED output, and configured by default in "heartbeat"
> > mode so that it rotates slowly and increases when the CPU load
> > increases, ensuring appropriate cooling by default. This LED output
> > is called "Fan" in the original kernel and connected to GPIO18
> > (gpiochip 0, pin 18). Here we called it "miqi:green:fan" to stay
> > consistent with the kernel's naming conventions.
> 
> I tend to disagree with this approach. A fan is not a led and the devicetree 
> is about describing the hardware, not how a specific kernel likes to use things 
> :-) .

Sure but I was trying to stay as close as possible to the intended
purpose of the connector on the board as it is sold :-)

> The kernel already has a gpio-fan driver (drivers/hwmon) or you could 
> resurrect the gpio-pwm patch [0] from Olliver Schinagl and use the pwm-fan on 
> top of that for more intermediate steps.

Ah it's great to know there has already been something like this because
I thought about developing one for the same reason.

> > It's worth noting that without this patch the fan doesn't work at
> > all, risking to make the board overheat.
> 
> At least cpufreq is already hooked to the thermal controller on the rk3288, so 
> even without additional cooling it should select lower cpu frequencies keeping 
> the heat in line and prevent overheating the board.

Well that's one way to see it, as for me throttling the CPU is the last
resort before seeing it die ; I find it sad to waste all the performance
of a 3288 that way, otherwise it's easier to use something like a dirt
a much slower and cheaper cortex A5. But I agree with the point regarding
the gpio-pwm. I think that mqmaker initially designed the GPIO to be used
as a led to benefit from the heartbeat trigger which more or less replaces
what a more efficient thermal control could achieve.

Thanks!
Willy
Heiko Stübner Feb. 12, 2017, 1:41 a.m. UTC | #3
Hi Willy,

Am Samstag, 11. Februar 2017, 18:45:54 CET schrieb Willy Tarreau:
> On Sat, Feb 11, 2017 at 05:33:16PM +0100, Heiko Stuebner wrote:
> > Am Samstag, 11. Februar 2017, 09:56:55 CET schrieb Willy Tarreau:
> > > The MiQi board is sold with an enclosure in which a fan is connected
> > > to the second LED output, and configured by default in "heartbeat"
> > > mode so that it rotates slowly and increases when the CPU load
> > > increases, ensuring appropriate cooling by default. This LED output
> > > is called "Fan" in the original kernel and connected to GPIO18
> > > (gpiochip 0, pin 18). Here we called it "miqi:green:fan" to stay
> > > consistent with the kernel's naming conventions.
> > 
> > I tend to disagree with this approach. A fan is not a led and the
> > devicetree is about describing the hardware, not how a specific kernel
> > likes to use things> 
> > :-) .
> 
> Sure but I was trying to stay as close as possible to the intended
> purpose of the connector on the board as it is sold :-)
>
> > The kernel already has a gpio-fan driver (drivers/hwmon) or you could
> > resurrect the gpio-pwm patch [0] from Olliver Schinagl and use the pwm-fan
> > on top of that for more intermediate steps.
> 
> Ah it's great to know there has already been something like this because
> I thought about developing one for the same reason.
> 
> > > It's worth noting that without this patch the fan doesn't work at
> > > all, risking to make the board overheat.
> > 
> > At least cpufreq is already hooked to the thermal controller on the
> > rk3288, so even without additional cooling it should select lower cpu
> > frequencies keeping the heat in line and prevent overheating the board.
> 
> Well that's one way to see it, as for me throttling the CPU is the last
> resort before seeing it die ;

That's what I was trying to say :-) ... it won't overheat because cpufreq will 
throttle it as a last resort.


> I find it sad to waste all the performance
> of a 3288 that way, otherwise it's easier to use something like a dirt
> a much slower and cheaper cortex A5. But I agree with the point regarding
> the gpio-pwm.

Even with the gpio-fan you should see results if you simply add the fan as 
active trip point somewhere before the cpufreq trip points. So let the fan get 
activated at 50/60° or so.


Heiko

> I think that mqmaker initially designed the GPIO to be used
> as a led to benefit from the heartbeat trigger which more or less replaces
> what a more efficient thermal control could achieve.
Willy Tarreau Feb. 12, 2017, 7:17 a.m. UTC | #4
On Sun, Feb 12, 2017 at 02:41:35AM +0100, Heiko Stuebner wrote:
> > I find it sad to waste all the performance
> > of a 3288 that way, otherwise it's easier to use something like a dirt
> > a much slower and cheaper cortex A5. But I agree with the point regarding
> > the gpio-pwm.
> 
> Even with the gpio-fan you should see results if you simply add the fan as 
> active trip point somewhere before the cpufreq trip points. So let the fan get 
> activated at 50/60° or so.

Better not do it as-is, the small fan is extremely noisy. That was the
benefit of the heartbeat mode, it was a poor man's pwm. I'll see what I
can do with the gpio-pwm. BTW, on all my MiQi, I have raised the critical
points to 90 degrees, one of my workplace build farm made of 4 MiQi, I
have connected a large slow fan which start to run above 85 degrees and
it's enough to prevent the critical point from ever being reached. The
dissipate heat is very low and the heatsink small so with minimal air
flow, the temperature drops very quickly.

Thanks,
Willy
diff mbox

Patch

diff --git a/arch/arm/boot/dts/rk3288-miqi.dts b/arch/arm/boot/dts/rk3288-miqi.dts
index 2f4c635..ef702db 100644
--- a/arch/arm/boot/dts/rk3288-miqi.dts
+++ b/arch/arm/boot/dts/rk3288-miqi.dts
@@ -67,6 +67,12 @@ 
 	leds {
 		compatible = "gpio-leds";
 
+		fan {
+			gpios = <&gpio0 18 GPIO_ACTIVE_HIGH>;
+			label = "miqi:green:fan";
+			linux,default-trigger = "heartbeat";
+		};
+
 		work {
 			gpios = <&gpio7 2 GPIO_ACTIVE_HIGH>;
 			label = "miqi:green:user";