Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35475 )
Change subject: drivers/i2c/lm96000: Add more settings for fan control ......................................................................
drivers/i2c/lm96000: Add more settings for fan control
Allow to set a lower temperature limit, as the currently hard-coded 25C may be to low for a given temperature sensor. Also enable smoo- thing, currently hard-coded to the maximum interval of 35s, and set the hysteresis value.
Change-Id: I5fde1cf909e8fbbaf8a345790b00c58a73c19ef8 Signed-off-by: Nico Huber nico.huber@secunet.com --- M src/drivers/i2c/lm96000/chip.h M src/drivers/i2c/lm96000/lm96000.c 2 files changed, 15 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/35475/1
diff --git a/src/drivers/i2c/lm96000/chip.h b/src/drivers/i2c/lm96000/chip.h index ab782db..cbf8601 100644 --- a/src/drivers/i2c/lm96000/chip.h +++ b/src/drivers/i2c/lm96000/chip.h @@ -93,6 +93,7 @@ };
struct lm96000_temp_zone { + u8 low_temp; /* temperature for min. duty cycle (in °C) */ u8 target_temp; /* temperature for 100% duty cycle (in °C) */ u8 panic_temp; /* temperature for 100% duty cycle on all fans */
@@ -100,9 +101,12 @@ with. (Datasheet clearly states the opposite, that this is tied to each PWM output so YMMV.) */ enum { - LM96000_LOW_TEMP_OFF = 0, /* turn fan off below low temp. */ - LM96000_LOW_TEMP_MIN = 1, /* keep PWM at mininum duty cycle */ + /* turn fan off below `low_temp - hysteresis` */ + LM96000_LOW_TEMP_OFF = 0, + /* keep PWM at mininum duty cycle */ + LM96000_LOW_TEMP_MIN = 1, } min_off; + u8 hysteresis; };
/* Implements only those parts currently used by coreboot mainboards. */ diff --git a/src/drivers/i2c/lm96000/lm96000.c b/src/drivers/i2c/lm96000/lm96000.c index 4a3c2eb..7fbb31b 100644 --- a/src/drivers/i2c/lm96000/lm96000.c +++ b/src/drivers/i2c/lm96000/lm96000.c @@ -156,9 +156,9 @@ { 2, 3, 3, 4, 5, 7, 8, 10, 13, 16, 20, 27, 32, 40, 53, 80 }; unsigned int i;
- /* find longest range that starts from 25°C */ + /* find longest range that starts from `low_temp` */ for (i = ARRAY_SIZE(temp_range) - 1; i > 0; --i) { - if (temp_range[i] + 25 <= config->target_temp) + if (config->low_temp + temp_range[i] <= config->target_temp) break; }
@@ -170,9 +170,16 @@ : 0); lm96000_write(dev, LM96000_ZONE_TEMP_PANIC(zone), config->panic_temp ? config->panic_temp : 100); + lm96000_update(dev, LM96000_ZONE_SMOOTH(zone), + LM96000_ZONE_SMOOTH_MASK(zone), + LM96000_ZONE_SMOOTH_EN(zone) | 0); /* 0: 35s */ lm96000_update(dev, LM96000_FAN_MIN_OFF, LM96000_FAN_MIN(zone), config->min_off ? LM96000_FAN_MIN(zone) : 0); + lm96000_update(dev, LM96000_ZONE_HYSTERESIS(zone), + LM96000_ZONE_HYST_MASK(zone), + config->hysteresis << LM96000_ZONE_HYST_SHIFT(zone) + & LM96000_ZONE_HYST_MASK(zone)); }
static void lm96000_init(struct device *const dev)
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35475 )
Change subject: drivers/i2c/lm96000: Add more settings for fan control ......................................................................
Patch Set 1: Code-Review+2
No devicetree to change? Boards using this have not landed?
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35475 )
Change subject: drivers/i2c/lm96000: Add more settings for fan control ......................................................................
drivers/i2c/lm96000: Add more settings for fan control
Allow to set a lower temperature limit, as the currently hard-coded 25C may be to low for a given temperature sensor. Also enable smoo- thing, currently hard-coded to the maximum interval of 35s, and set the hysteresis value.
Change-Id: I5fde1cf909e8fbbaf8a345790b00c58a73c19ef8 Signed-off-by: Nico Huber nico.huber@secunet.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35475 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/drivers/i2c/lm96000/chip.h M src/drivers/i2c/lm96000/lm96000.c 2 files changed, 15 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved
diff --git a/src/drivers/i2c/lm96000/chip.h b/src/drivers/i2c/lm96000/chip.h index ab782db..cbf8601 100644 --- a/src/drivers/i2c/lm96000/chip.h +++ b/src/drivers/i2c/lm96000/chip.h @@ -93,6 +93,7 @@ };
struct lm96000_temp_zone { + u8 low_temp; /* temperature for min. duty cycle (in °C) */ u8 target_temp; /* temperature for 100% duty cycle (in °C) */ u8 panic_temp; /* temperature for 100% duty cycle on all fans */
@@ -100,9 +101,12 @@ with. (Datasheet clearly states the opposite, that this is tied to each PWM output so YMMV.) */ enum { - LM96000_LOW_TEMP_OFF = 0, /* turn fan off below low temp. */ - LM96000_LOW_TEMP_MIN = 1, /* keep PWM at mininum duty cycle */ + /* turn fan off below `low_temp - hysteresis` */ + LM96000_LOW_TEMP_OFF = 0, + /* keep PWM at mininum duty cycle */ + LM96000_LOW_TEMP_MIN = 1, } min_off; + u8 hysteresis; };
/* Implements only those parts currently used by coreboot mainboards. */ diff --git a/src/drivers/i2c/lm96000/lm96000.c b/src/drivers/i2c/lm96000/lm96000.c index 4a3c2eb..7fbb31b 100644 --- a/src/drivers/i2c/lm96000/lm96000.c +++ b/src/drivers/i2c/lm96000/lm96000.c @@ -156,9 +156,9 @@ { 2, 3, 3, 4, 5, 7, 8, 10, 13, 16, 20, 27, 32, 40, 53, 80 }; unsigned int i;
- /* find longest range that starts from 25°C */ + /* find longest range that starts from `low_temp` */ for (i = ARRAY_SIZE(temp_range) - 1; i > 0; --i) { - if (temp_range[i] + 25 <= config->target_temp) + if (config->low_temp + temp_range[i] <= config->target_temp) break; }
@@ -170,9 +170,16 @@ : 0); lm96000_write(dev, LM96000_ZONE_TEMP_PANIC(zone), config->panic_temp ? config->panic_temp : 100); + lm96000_update(dev, LM96000_ZONE_SMOOTH(zone), + LM96000_ZONE_SMOOTH_MASK(zone), + LM96000_ZONE_SMOOTH_EN(zone) | 0); /* 0: 35s */ lm96000_update(dev, LM96000_FAN_MIN_OFF, LM96000_FAN_MIN(zone), config->min_off ? LM96000_FAN_MIN(zone) : 0); + lm96000_update(dev, LM96000_ZONE_HYSTERESIS(zone), + LM96000_ZONE_HYST_MASK(zone), + config->hysteresis << LM96000_ZONE_HYST_SHIFT(zone) + & LM96000_ZONE_HYST_MASK(zone)); }
static void lm96000_init(struct device *const dev)
-
Kyösti Mälkki (Code Review) -
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)