Michael has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44165 )
Change subject: superio/ite: configure EC for fans to full at thermal limit ......................................................................
superio/ite: configure EC for fans to full at thermal limit
This applies to the automatic fan control mode of the environment controller (EC). Previously the affected bit was always cleared while the default value is 1 according to datasheets. Add a variable that can be set per mainboard in devicetree.cb.
In the IT8783E datasheet that bit is marked as reserved.
Signed-off-by: Michael Büchler michael.buechler@posteo.net Change-Id: Ie74102ac0d54be33558c161c9c84594d121772b1 --- M src/mainboard/asus/f2a85-m/devicetree_f2a85-m.cb M src/mainboard/asus/f2a85-m/devicetree_f2a85-m_le.cb M src/mainboard/foxconn/g41s-k/devicetree.cb M src/mainboard/gigabyte/ga-945gcm-s2l/devicetree.cb M src/mainboard/gigabyte/ga-g41m-es2l/devicetree.cb M src/mainboard/libretrend/lt1000/devicetree.cb M src/mainboard/roda/rv11/variants/rw11/devicetree.cb M src/superio/ite/common/env_ctrl.c M src/superio/ite/common/env_ctrl.h M src/superio/ite/common/env_ctrl_chip.h 10 files changed, 16 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/44165/1
diff --git a/src/mainboard/asus/f2a85-m/devicetree_f2a85-m.cb b/src/mainboard/asus/f2a85-m/devicetree_f2a85-m.cb index 08a74e8..11c4186 100644 --- a/src/mainboard/asus/f2a85-m/devicetree_f2a85-m.cb +++ b/src/mainboard/asus/f2a85-m/devicetree_f2a85-m.cb @@ -49,6 +49,7 @@ register "FAN1.smart.tmp_start" = "20" register "FAN1.smart.tmp_full" = "70" register "FAN1.smart.tmp_delta" = "0" + register "FAN1.smart.full_lmt" = "0" register "FAN1.smart.smoothing" = "1" register "FAN1.smart.pwm_start" = "20" register "FAN1.smart.slope" = "32" diff --git a/src/mainboard/asus/f2a85-m/devicetree_f2a85-m_le.cb b/src/mainboard/asus/f2a85-m/devicetree_f2a85-m_le.cb index bc0dc42..a1b1917 100644 --- a/src/mainboard/asus/f2a85-m/devicetree_f2a85-m_le.cb +++ b/src/mainboard/asus/f2a85-m/devicetree_f2a85-m_le.cb @@ -49,6 +49,7 @@ register "FAN1.smart.tmp_start" = "20" register "FAN1.smart.tmp_full" = "70" register "FAN1.smart.tmp_delta" = "0" + register "FAN1.smart.full_lmt" = "0" register "FAN1.smart.smoothing" = "1" register "FAN1.smart.pwm_start" = "20" register "FAN1.smart.slope" = "32" diff --git a/src/mainboard/foxconn/g41s-k/devicetree.cb b/src/mainboard/foxconn/g41s-k/devicetree.cb index 9bde4b2..89a801c 100644 --- a/src/mainboard/foxconn/g41s-k/devicetree.cb +++ b/src/mainboard/foxconn/g41s-k/devicetree.cb @@ -65,6 +65,7 @@ register "FAN1.smart.tmp_start" = "30" register "FAN1.smart.tmp_full" = "65" register "FAN1.smart.tmp_delta" = "3" + register "FAN1.smart.full_lmt" = "0" register "FAN1.smart.smoothing" = "1" register "FAN1.smart.pwm_start" = "20" register "FAN1.smart.slope" = "10" @@ -74,6 +75,7 @@ register "FAN2.smart.tmp_start" = "30" register "FAN2.smart.tmp_full" = "65" register "FAN2.smart.tmp_delta" = "3" + register "FAN2.smart.full_lmt" = "0" register "FAN2.smart.smoothing" = "1" register "FAN2.smart.pwm_start" = "20" register "FAN2.smart.slope" = "10" diff --git a/src/mainboard/gigabyte/ga-945gcm-s2l/devicetree.cb b/src/mainboard/gigabyte/ga-945gcm-s2l/devicetree.cb index c0f198f..f80c02d 100644 --- a/src/mainboard/gigabyte/ga-945gcm-s2l/devicetree.cb +++ b/src/mainboard/gigabyte/ga-945gcm-s2l/devicetree.cb @@ -111,6 +111,7 @@ register "FAN1.smart.tmp_start" = "35" register "FAN1.smart.tmp_full" = "75" register "FAN1.smart.tmp_delta" = "3" + register "FAN1.smart.full_lmt" = "0" register "FAN1.smart.smoothing" = "1" register "FAN1.smart.pwm_start" = "0" register "FAN1.smart.slope" = "10" @@ -121,6 +122,7 @@ register "FAN2.smart.tmp_start" = "35" register "FAN2.smart.tmp_full" = "75" register "FAN2.smart.tmp_delta" = "3" + register "FAN2.smart.full_lmt" = "0" register "FAN2.smart.smoothing" = "1" register "FAN2.smart.pwm_start" = "0" register "FAN2.smart.slope" = "10" diff --git a/src/mainboard/gigabyte/ga-g41m-es2l/devicetree.cb b/src/mainboard/gigabyte/ga-g41m-es2l/devicetree.cb index 6328bc6..d61342c 100644 --- a/src/mainboard/gigabyte/ga-g41m-es2l/devicetree.cb +++ b/src/mainboard/gigabyte/ga-g41m-es2l/devicetree.cb @@ -81,6 +81,7 @@ register "FAN1.smart.tmp_start" = "30" register "FAN1.smart.tmp_full" = "65" register "FAN1.smart.tmp_delta" = "3" + register "FAN1.smart.full_lmt" = "0" register "FAN1.smart.smoothing" = "1" register "FAN1.smart.pwm_start" = "0" register "FAN1.smart.slope" = "10" @@ -91,6 +92,7 @@ register "FAN2.smart.tmp_start" = "30" register "FAN2.smart.tmp_full" = "65" register "FAN2.smart.tmp_delta" = "3" + register "FAN2.smart.full_lmt" = "0" register "FAN2.smart.smoothing" = "1" register "FAN2.smart.pwm_start" = "0" register "FAN2.smart.slope" = "10" diff --git a/src/mainboard/libretrend/lt1000/devicetree.cb b/src/mainboard/libretrend/lt1000/devicetree.cb index a58e169..7fa941a 100644 --- a/src/mainboard/libretrend/lt1000/devicetree.cb +++ b/src/mainboard/libretrend/lt1000/devicetree.cb @@ -220,6 +220,7 @@ register "FAN1.smart.tmp_start" = "60" register "FAN1.smart.tmp_full" = "85" register "FAN1.smart.tmp_delta" = " 2" + register "FAN1.smart.full_lmt" = " 0" register "FAN1.smart.pwm_start" = "20" register "FAN1.smart.slope" = "24" # FAN2 is system fan (4 pin connector populated) diff --git a/src/mainboard/roda/rv11/variants/rw11/devicetree.cb b/src/mainboard/roda/rv11/variants/rw11/devicetree.cb index 988c2a6..3e28ba8 100644 --- a/src/mainboard/roda/rv11/variants/rw11/devicetree.cb +++ b/src/mainboard/roda/rv11/variants/rw11/devicetree.cb @@ -105,6 +105,7 @@ register "FAN1.smart.tmp_off" = "60" register "FAN1.smart.tmp_start" = "64" register "FAN1.smart.tmp_delta" = " 2" + register "FAN1.smart.full_lmt" = " 0" register "FAN1.smart.pwm_start" = "30" register "FAN1.smart.slope" = "64" register "FAN2.mode" = "FAN_SMART_AUTOMATIC" @@ -112,6 +113,7 @@ register "FAN2.smart.tmp_off" = "60" register "FAN2.smart.tmp_start" = "64" register "FAN2.smart.tmp_delta" = " 2" + register "FAN2.smart.full_lmt" = " 0" register "FAN2.smart.pwm_start" = "30" register "FAN2.smart.slope" = "64" register "FAN3.mode" = "FAN_MODE_OFF" diff --git a/src/superio/ite/common/env_ctrl.c b/src/superio/ite/common/env_ctrl.c index f63bfd3..52981ec 100644 --- a/src/superio/ite/common/env_ctrl.c +++ b/src/superio/ite/common/env_ctrl.c @@ -147,6 +147,8 @@ conf->tmp_full ? conf->tmp_full : 127); pnp_write_hwm5_index(base, ITE_EC_FAN_CTL_DELTA_TEMP(fan), ITE_EC_FAN_CTL_DELTA_TEMP_INTRVL(conf->tmp_delta)); + pnp_write_hwm5_index(base, ITE_EC_FAN_CTL_DELTA_TEMP(fan), + ITE_EC_FAN_CTL_FULL_AT_THRML_LMT(conf->full_lmt)); }
pnp_write_hwm5_index(base, ITE_EC_FAN_CTL_PWM_CONTROL(fan), pwm_ctrl); diff --git a/src/superio/ite/common/env_ctrl.h b/src/superio/ite/common/env_ctrl.h index e8fb1f5..7bce809 100644 --- a/src/superio/ite/common/env_ctrl.h +++ b/src/superio/ite/common/env_ctrl.h @@ -185,6 +185,7 @@
/* Common for ITE_EC_FAN_CTL_DELTA_TEMP */ #define ITE_EC_FAN_CTL_DELTA_TEMP_INTRVL(c) ((c) & 0x1f) +#define ITE_EC_FAN_CTL_FULL_AT_THRML_LMT(x) (((x) & 0x1) << 6) #define ITE_EC_FAN_CTL_TARGET_ZONE(x) (0x66 + ((x)-1) * 8) #define ITE_EC_FAN_CTL_TARGET_ZONE_MASK 0x0f
diff --git a/src/superio/ite/common/env_ctrl_chip.h b/src/superio/ite/common/env_ctrl_chip.h index 6a027b9..5f824be 100644 --- a/src/superio/ite/common/env_ctrl_chip.h +++ b/src/superio/ite/common/env_ctrl_chip.h @@ -56,6 +56,8 @@ u8 tmp_full; /* 100% duty cycle above (°C) */ u8 tmp_delta; /* adapt fan speed when temperature changed by at least `tmp_delta`°C */ + u8 full_lmt; /* force fan to full PWM at thermal + limit */ u8 smoothing; /* enable smoothing */ u8 pwm_start; /* start at this duty cycle (%) */ u8 slope; /* increase duty cycle by `slope`%/°C */
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44165 )
Change subject: superio/ite: configure EC for fans to full at thermal limit ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/44165/1/src/mainboard/asus/f2a85-m/... File src/mainboard/asus/f2a85-m/devicetree_f2a85-m.cb:
https://review.coreboot.org/c/coreboot/+/44165/1/src/mainboard/asus/f2a85-m/... PS1, Line 52: register "FAN1.smart.full_lmt" = "0" Default for devicetree options is 0, so you can avoid having to add it everywhere 😊
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44165 )
Change subject: superio/ite: configure EC for fans to full at thermal limit ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44165/1/src/superio/ite/common/env_... File src/superio/ite/common/env_ctrl_chip.h:
https://review.coreboot.org/c/coreboot/+/44165/1/src/superio/ite/common/env_... PS1, Line 60: limit */ please, no space before tabs
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44165 )
Change subject: superio/ite: configure EC for fans to full at thermal limit ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44165/1/src/superio/ite/common/env_... File src/superio/ite/common/env_ctrl.c:
https://review.coreboot.org/c/coreboot/+/44165/1/src/superio/ite/common/env_... PS1, Line 151: ITE_EC_FAN_CTL_FULL_AT_THRML_LMT(conf->full_lmt)); this overwrites what the function call in the lines above set. you probably need to or the two settings that go in the same register together and write it in one command. there might have been some commit introducing a command to only write certain bits of a register; don't remember exactly though
https://review.coreboot.org/c/coreboot/+/44165/1/src/superio/ite/common/env_... File src/superio/ite/common/env_ctrl_chip.h:
https://review.coreboot.org/c/coreboot/+/44165/1/src/superio/ite/common/env_... PS1, Line 60: limit */
please, no space before tabs
that should fit all in one line; the characters per line limit was raised to 96 some time ago
Hello build bot (Jenkins), Angel Pons, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44165
to look at the new patch set (#2).
Change subject: superio/ite: configure EC for fans to full at thermal limit ......................................................................
superio/ite: configure EC for fans to full at thermal limit
This applies to the automatic fan control mode of the environment controller (EC). Previously the affected bit was always cleared while the default value is 1 according to datasheets. Add a variable that can be set per mainboard in devicetree.cb.
In the IT8783E datasheet that bit is marked as reserved.
Signed-off-by: Michael Büchler michael.buechler@posteo.net Change-Id: Ie74102ac0d54be33558c161c9c84594d121772b1 --- M src/mainboard/asus/f2a85-m/devicetree_f2a85-m.cb M src/mainboard/asus/f2a85-m/devicetree_f2a85-m_le.cb M src/mainboard/foxconn/g41s-k/devicetree.cb M src/mainboard/gigabyte/ga-945gcm-s2l/devicetree.cb M src/mainboard/gigabyte/ga-g41m-es2l/devicetree.cb M src/mainboard/libretrend/lt1000/devicetree.cb M src/mainboard/roda/rv11/variants/rw11/devicetree.cb M src/superio/ite/common/env_ctrl.c M src/superio/ite/common/env_ctrl.h M src/superio/ite/common/env_ctrl_chip.h 10 files changed, 27 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/44165/2
Michael has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44165 )
Change subject: superio/ite: configure EC for fans to full at thermal limit ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/44165/1/src/mainboard/asus/f2a85-m/... File src/mainboard/asus/f2a85-m/devicetree_f2a85-m.cb:
https://review.coreboot.org/c/coreboot/+/44165/1/src/mainboard/asus/f2a85-m/... PS1, Line 52: register "FAN1.smart.full_lmt" = "0"
Default for devicetree options is 0, so you can avoid having to add it everywhere 😊
Thanks! I was unsure about that. Should we keep it anyway?
https://review.coreboot.org/c/coreboot/+/44165/1/src/superio/ite/common/env_... File src/superio/ite/common/env_ctrl.c:
https://review.coreboot.org/c/coreboot/+/44165/1/src/superio/ite/common/env_... PS1, Line 151: ITE_EC_FAN_CTL_FULL_AT_THRML_LMT(conf->full_lmt));
this overwrites what the function call in the lines above set. […]
Ouch. The OR way sounds good. Thanks!
https://review.coreboot.org/c/coreboot/+/44165/1/src/superio/ite/common/env_... File src/superio/ite/common/env_ctrl_chip.h:
https://review.coreboot.org/c/coreboot/+/44165/1/src/superio/ite/common/env_... PS1, Line 60: limit */
that should fit all in one line; the characters per line limit was raised to 96 some time ago
Done. The tmp_delta line could also be squeezed into 96 characters but then for aligning the */ on other lines they would be preceded by mixed tabs/spaces.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44165 )
Change subject: superio/ite: configure EC for fans to full at thermal limit ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44165/1/src/superio/ite/common/env_... File src/superio/ite/common/env_ctrl.c:
https://review.coreboot.org/c/coreboot/+/44165/1/src/superio/ite/common/env_... PS1, Line 151: ITE_EC_FAN_CTL_FULL_AT_THRML_LMT(conf->full_lmt));
Ouch. The OR way sounds good. […]
That commit would be CB:42134 which is on permanent hold because people want me to ask on the mailing list about introducing new naming conventions (and which I'm not going to do because I kind of gave up on that patch series)
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44165 )
Change subject: superio/ite: configure EC for fans to full at thermal limit ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44165/1/src/mainboard/asus/f2a85-m/... File src/mainboard/asus/f2a85-m/devicetree_f2a85-m.cb:
https://review.coreboot.org/c/coreboot/+/44165/1/src/mainboard/asus/f2a85-m/... PS1, Line 52: register "FAN1.smart.full_lmt" = "0"
Thanks! I was unsure about that. […]
I'd drop it to make this change more localized and so that this change doesn't conflict as much with other unrelated changes
Hello build bot (Jenkins), Angel Pons, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44165
to look at the new patch set (#3).
Change subject: superio/ite: configure EC for fans to full at thermal limit ......................................................................
superio/ite: configure EC for fans to full at thermal limit
This applies to the automatic fan control mode of the environment controller (EC). Previously the affected bit was always cleared while the default value is 1 according to datasheets. Add a variable that can be set per mainboard in devicetree.cb.
In the IT8783E datasheet that bit is marked as reserved.
Signed-off-by: Michael Büchler michael.buechler@posteo.net Change-Id: Ie74102ac0d54be33558c161c9c84594d121772b1 --- M src/superio/ite/common/env_ctrl.c M src/superio/ite/common/env_ctrl.h M src/superio/ite/common/env_ctrl_chip.h 3 files changed, 16 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/44165/3
Michael Büchler has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44165 )
Change subject: superio/ite: configure EC for fans to full at thermal limit ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44165/1/src/mainboard/asus/f2a85-m/... File src/mainboard/asus/f2a85-m/devicetree_f2a85-m.cb:
https://review.coreboot.org/c/coreboot/+/44165/1/src/mainboard/asus/f2a85-m/... PS1, Line 52: register "FAN1.smart.full_lmt" = "0"
I'd drop it to make this change more localized and so that this change doesn't conflict as much with […]
Done
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44165 )
Change subject: superio/ite: configure EC for fans to full at thermal limit ......................................................................
Patch Set 3: Code-Review+2
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44165 )
Change subject: superio/ite: configure EC for fans to full at thermal limit ......................................................................
superio/ite: configure EC for fans to full at thermal limit
This applies to the automatic fan control mode of the environment controller (EC). Previously the affected bit was always cleared while the default value is 1 according to datasheets. Add a variable that can be set per mainboard in devicetree.cb.
In the IT8783E datasheet that bit is marked as reserved.
Signed-off-by: Michael Büchler michael.buechler@posteo.net Change-Id: Ie74102ac0d54be33558c161c9c84594d121772b1 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44165 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Felix Held felix-coreboot@felixheld.de --- M src/superio/ite/common/env_ctrl.c M src/superio/ite/common/env_ctrl.h M src/superio/ite/common/env_ctrl_chip.h 3 files changed, 16 insertions(+), 10 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved
diff --git a/src/superio/ite/common/env_ctrl.c b/src/superio/ite/common/env_ctrl.c index f63bfd3..fbf7446 100644 --- a/src/superio/ite/common/env_ctrl.c +++ b/src/superio/ite/common/env_ctrl.c @@ -113,6 +113,7 @@ u8 pwm_ctrl; u8 pwm_start = 0; u8 pwm_auto = 0; + u8 delta_temp;
if (mode == FAN_SMART_SOFTWARE) { pwm_ctrl = ITE_EC_FAN_CTL_PWM_MODE_SOFTWARE; @@ -145,8 +146,11 @@ /* Full speed above 127°C by default */ pnp_write_hwm5_index(base, ITE_EC_FAN_CTL_TEMP_LIMIT_FULL(fan), conf->tmp_full ? conf->tmp_full : 127); + + delta_temp = ITE_EC_FAN_CTL_DELTA_TEMP_INTRVL(conf->tmp_delta); + delta_temp |= ITE_EC_FAN_CTL_FULL_AT_THRML_LMT(conf->full_lmt); pnp_write_hwm5_index(base, ITE_EC_FAN_CTL_DELTA_TEMP(fan), - ITE_EC_FAN_CTL_DELTA_TEMP_INTRVL(conf->tmp_delta)); + delta_temp); }
pnp_write_hwm5_index(base, ITE_EC_FAN_CTL_PWM_CONTROL(fan), pwm_ctrl); diff --git a/src/superio/ite/common/env_ctrl.h b/src/superio/ite/common/env_ctrl.h index e8fb1f5..7bce809 100644 --- a/src/superio/ite/common/env_ctrl.h +++ b/src/superio/ite/common/env_ctrl.h @@ -185,6 +185,7 @@
/* Common for ITE_EC_FAN_CTL_DELTA_TEMP */ #define ITE_EC_FAN_CTL_DELTA_TEMP_INTRVL(c) ((c) & 0x1f) +#define ITE_EC_FAN_CTL_FULL_AT_THRML_LMT(x) (((x) & 0x1) << 6) #define ITE_EC_FAN_CTL_TARGET_ZONE(x) (0x66 + ((x)-1) * 8) #define ITE_EC_FAN_CTL_TARGET_ZONE_MASK 0x0f
diff --git a/src/superio/ite/common/env_ctrl_chip.h b/src/superio/ite/common/env_ctrl_chip.h index 6a027b9..09577a4 100644 --- a/src/superio/ite/common/env_ctrl_chip.h +++ b/src/superio/ite/common/env_ctrl_chip.h @@ -50,15 +50,16 @@ };
struct ite_ec_fan_smartconfig { - u8 tmpin; /* select TMPINx (1, 2 or 3) */ - u8 tmp_off; /* turn fan off below (°C) */ - u8 tmp_start; /* turn fan on above (°C) */ - u8 tmp_full; /* 100% duty cycle above (°C) */ - u8 tmp_delta; /* adapt fan speed when temperature - changed by at least `tmp_delta`°C */ - u8 smoothing; /* enable smoothing */ - u8 pwm_start; /* start at this duty cycle (%) */ - u8 slope; /* increase duty cycle by `slope`%/°C */ + u8 tmpin; /* select TMPINx (1, 2 or 3) */ + u8 tmp_off; /* turn fan off below (°C) */ + u8 tmp_start; /* turn fan on above (°C) */ + u8 tmp_full; /* 100% duty cycle above (°C) */ + u8 tmp_delta; /* adapt fan speed when temperature changed by + at least `tmp_delta`°C */ + u8 full_lmt; /* force fan to full PWM at thermal limit */ + u8 smoothing; /* enable smoothing */ + u8 pwm_start; /* start at this duty cycle (%) */ + u8 slope; /* increase duty cycle by `slope`%/°C */ };
struct ite_ec_fan_config {
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Felix Held (Code Review)
-
Michael (Code Review)
-
Michael Büchler (Code Review)