HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38127 )
Change subject: mb/ibase/mb899: Reformat code ......................................................................
mb/ibase/mb899: Reformat code
Change-Id: Ic8c131b8dc6308ed813820f67477d157ccea5b05 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/ibase/mb899/early_init.c M src/mainboard/ibase/mb899/mainboard.c M src/mainboard/ibase/mb899/superio_hwm.c 3 files changed, 50 insertions(+), 37 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/38127/1
diff --git a/src/mainboard/ibase/mb899/early_init.c b/src/mainboard/ibase/mb899/early_init.c index fd96f66..997cf04 100644 --- a/src/mainboard/ibase/mb899/early_init.c +++ b/src/mainboard/ibase/mb899/early_init.c @@ -14,12 +14,12 @@ */
#include <bootblock_common.h> -#include <stdint.h> -#include <device/pnp_ops.h> #include <cpu/x86/lapic.h> #include <device/pnp_def.h> +#include <device/pnp_ops.h> #include <northbridge/intel/i945/i945.h> #include <southbridge/intel/i82801gx/i82801gx.h> +#include <stdint.h> #include <superio/winbond/common/winbond.h> #include <superio/winbond/w83627ehg/w83627ehg.h>
@@ -65,7 +65,7 @@ pnp_set_enable(dev, 0); pnp_set_iobase(dev, PNP_IDX_IO0, 0x60); pnp_set_iobase(dev, PNP_IDX_IO1, 0x64); - //pnp_write_config(dev, 0xf0, 0x82); + // pnp_write_config(dev, 0xf0, 0x82); pnp_set_enable(dev, 1);
dev = PNP_DEV(0x4e, W83627EHG_GPIO2); diff --git a/src/mainboard/ibase/mb899/mainboard.c b/src/mainboard/ibase/mb899/mainboard.c index d4d05c3..ada5fa3 100644 --- a/src/mainboard/ibase/mb899/mainboard.c +++ b/src/mainboard/ibase/mb899/mainboard.c @@ -15,6 +15,7 @@
#include <device/device.h> #include <drivers/intel/gma/int15.h> + #include "superio_hwm.h"
// mainboard_enable is executed as first thing after @@ -22,7 +23,9 @@
static void mainboard_enable(struct device *dev) { - install_intel_vga_int15_handler(GMA_INT15_ACTIVE_LFP_INT_LVDS, GMA_INT15_PANEL_FIT_DEFAULT, GMA_INT15_BOOT_DISPLAY_DEFAULT, 3); + install_intel_vga_int15_handler(GMA_INT15_ACTIVE_LFP_INT_LVDS, + GMA_INT15_PANEL_FIT_DEFAULT, + GMA_INT15_BOOT_DISPLAY_DEFAULT, 3); hwm_setup(); }
diff --git a/src/mainboard/ibase/mb899/superio_hwm.c b/src/mainboard/ibase/mb899/superio_hwm.c index 175c9a0..3bf3314 100644 --- a/src/mainboard/ibase/mb899/superio_hwm.c +++ b/src/mainboard/ibase/mb899/superio_hwm.c @@ -14,22 +14,22 @@ * GNU General Public License for more details. */
-#include <types.h> #include <console/console.h> #include <device/device.h> #include <pc80/mc146818rtc.h> #include <superio/hwm5_conf.h> #include <superio/nuvoton/common/hwm.h> +#include <types.h>
#include "superio_hwm.h"
/* Hardware Monitor */
-#define FAN_CRUISE_CONTROL_DISABLED 0 -#define FAN_CRUISE_CONTROL_SPEED 1 -#define FAN_CRUISE_CONTROL_THERMAL 2 +#define FAN_CRUISE_CONTROL_DISABLED 0 +#define FAN_CRUISE_CONTROL_SPEED 1 +#define FAN_CRUISE_CONTROL_THERMAL 2
-#define FAN_SPEED_5625 0 +#define FAN_SPEED_5625 0 //#define FAN_TEMPERATURE_30DEGC 0
#define HWM_BASE 0x290 @@ -42,11 +42,11 @@ // FANIN Target Speed Register // FANIN = 337500 / RPM struct fan_speed fan_speeds[] = { - { 0x3c, 5625 }, { 0x41, 5192 }, { 0x47, 4753 }, { 0x4e, 4326 }, - { 0x56, 3924 }, { 0x5f, 3552 }, { 0x69, 3214 }, { 0x74, 2909 }, - { 0x80, 2636 }, { 0x8d, 2393 }, { 0x9b, 2177 }, { 0xaa, 1985 }, - { 0xba, 1814 }, { 0xcb, 1662 }, { 0xdd, 1527 }, { 0xf0, 1406 } -}; + {0x3c, 5625}, {0x41, 5192}, {0x47, 4753}, {0x4e, 4326}, + {0x56, 3924}, {0x5f, 3552}, {0x69, 3214}, {0x74, 2909}, + {0x80, 2636}, {0x8d, 2393}, {0x9b, 2177}, {0xaa, 1985}, + {0xba, 1814}, {0xcb, 1662}, {0xdd, 1527}, {0xf0, 1406} + };
struct temperature { u8 deg_celsius; @@ -54,11 +54,11 @@ };
struct temperature temperatures[] = { - { 30, 86 }, { 33, 91 }, { 36, 96 }, { 39, 102 }, - { 42, 107 }, { 45, 113 }, { 48, 118 }, { 51, 123 }, - { 54, 129 }, { 57, 134 }, { 60, 140 }, { 63, 145 }, - { 66, 150 }, { 69, 156 }, { 72, 161 }, { 75, 167 } -}; + {30, 86}, {33, 91}, {36, 96}, {39, 102}, + {42, 107}, {45, 113}, {48, 118}, {51, 123}, + {54, 129}, {57, 134}, {60, 140}, {63, 145}, + {66, 150}, {69, 156}, {72, 161}, {75, 167} + };
void hwm_setup(void) { @@ -70,15 +70,15 @@ get_option(&cpufan_control, "cpufan_cruise_control"); cpufan_speed = FAN_SPEED_5625; get_option(&cpufan_speed, "cpufan_speed"); - //cpufan_temperature = FAN_TEMPERATURE_30DEGC; - //get_option(&cpufan_temperature, "cpufan_temperature"); + // cpufan_temperature = FAN_TEMPERATURE_30DEGC; + // get_option(&cpufan_temperature, "cpufan_temperature");
sysfan_control = FAN_CRUISE_CONTROL_DISABLED; get_option(&sysfan_control, "sysfan_cruise_control"); sysfan_speed = FAN_SPEED_5625; get_option(&sysfan_speed, "sysfan_speed"); - //sysfan_temperature = FAN_TEMPERATURE_30DEGC; - //get_option(&sysfan_temperature, "sysfan_temperature"); + // sysfan_temperature = FAN_TEMPERATURE_30DEGC; + // get_option(&sysfan_temperature, "sysfan_temperature");
// pnp_write_hwm5_index(HWM_BASE, 0x31, 0x20); // AVCC high limit // pnp_write_hwm5_index(HWM_BASE, 0x34, 0x06); // VIN2 low limit @@ -99,12 +99,20 @@ // 01 FANOUT is Thermal Cruise Mode // 10 FANOUT is Fan Speed Cruise Mode switch (cpufan_control) { - case FAN_CRUISE_CONTROL_SPEED: fan_config |= (2 << 4); break; - case FAN_CRUISE_CONTROL_THERMAL: fan_config |= (1 << 4); break; + case FAN_CRUISE_CONTROL_SPEED: + fan_config |= (2 << 4); + break; + case FAN_CRUISE_CONTROL_THERMAL: + fan_config |= (1 << 4); + break; } switch (sysfan_control) { - case FAN_CRUISE_CONTROL_SPEED: fan_config |= (2 << 2); break; - case FAN_CRUISE_CONTROL_THERMAL: fan_config |= (1 << 2); break; + case FAN_CRUISE_CONTROL_SPEED: + fan_config |= (2 << 2); + break; + case FAN_CRUISE_CONTROL_THERMAL: + fan_config |= (1 << 2); + break; } // This register must be written first pnp_write_hwm5_index(HWM_BASE, 0x04, fan_config); @@ -112,13 +120,14 @@ switch (cpufan_control) { case FAN_CRUISE_CONTROL_SPEED: /* CPUFANIN target speed */ printk(BIOS_DEBUG, "Fan Cruise Control setting CPU fan to %d RPM\n", - fan_speeds[cpufan_speed].fan_speed); + fan_speeds[cpufan_speed].fan_speed); pnp_write_hwm5_index(HWM_BASE, 0x06, fan_speeds[cpufan_speed].fan_in); break; case FAN_CRUISE_CONTROL_THERMAL: /* CPUFANIN target temperature */ - printk(BIOS_DEBUG, "Fan Cruise Control setting CPU fan to activation at %d deg C/%d deg F\n", - temperatures[cpufan_temperature].deg_celsius, - temperatures[cpufan_temperature].deg_fahrenheit); + printk(BIOS_DEBUG, + "Fan Cruise Control setting CPU fan to activation at %d deg C/%d deg F\n", + temperatures[cpufan_temperature].deg_celsius, + temperatures[cpufan_temperature].deg_fahrenheit); pnp_write_hwm5_index(HWM_BASE, 0x06, temperatures[cpufan_temperature].deg_celsius); break; @@ -127,15 +136,16 @@ switch (sysfan_control) { case FAN_CRUISE_CONTROL_SPEED: /* SYSFANIN target speed */ printk(BIOS_DEBUG, "Fan Cruise Control setting system fan to %d RPM\n", - fan_speeds[sysfan_speed].fan_speed); + fan_speeds[sysfan_speed].fan_speed); pnp_write_hwm5_index(HWM_BASE, 0x05, fan_speeds[sysfan_speed].fan_in); break; case FAN_CRUISE_CONTROL_THERMAL: /* SYSFANIN target temperature */ - printk(BIOS_DEBUG, "Fan Cruise Control setting system fan to activation at %d deg C/%d deg F\n", - temperatures[sysfan_temperature].deg_celsius, - temperatures[sysfan_temperature].deg_fahrenheit); - pnp_write_hwm5_index(HWM_BASE, - 0x05, temperatures[sysfan_temperature].deg_celsius); + printk(BIOS_DEBUG, + "Fan Cruise Control setting system fan to activation at %d deg C/%d deg F\n", + temperatures[sysfan_temperature].deg_celsius, + temperatures[sysfan_temperature].deg_fahrenheit); + pnp_write_hwm5_index(HWM_BASE, 0x05, + temperatures[sysfan_temperature].deg_celsius); break; }
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38127
to look at the new patch set (#2).
Change subject: mb/ibase/mb899: Reformat code ......................................................................
mb/ibase/mb899: Reformat code
Change-Id: Ic8c131b8dc6308ed813820f67477d157ccea5b05 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/ibase/mb899/early_init.c M src/mainboard/ibase/mb899/mainboard.c M src/mainboard/ibase/mb899/superio_hwm.c 3 files changed, 48 insertions(+), 35 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/38127/2
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38127 )
Change subject: mb/ibase/mb899: Reformat code ......................................................................
Patch Set 2: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38127 )
Change subject: mb/ibase/mb899: Reformat code ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/c/coreboot/+/38127/2/src/mainboard/ibase/mb899/s... File src/mainboard/ibase/mb899/superio_hwm.c:
https://review.coreboot.org/c/coreboot/+/38127/2/src/mainboard/ibase/mb899/s... PS2, Line 28: #define FAN_CRUISE_CONTROL_DISABLED 0 Why? Those values were nicely aligned!
https://review.coreboot.org/c/coreboot/+/38127/2/src/mainboard/ibase/mb899/s... PS2, Line 57: {30, 86}, {33, 91}, {36, 96}, {39, 102}, I would add some extra spaces so that the numbers are nicely aligned
https://review.coreboot.org/c/coreboot/+/38127/2/src/mainboard/ibase/mb899/s... PS2, Line 73: // cpufan_temperature = FAN_TEMPERATURE_30DEGC; Isn't this just dead code?
https://review.coreboot.org/c/coreboot/+/38127/2/src/mainboard/ibase/mb899/s... PS2, Line 102: case FAN_CRUISE_CONTROL_SPEED: I don't see why this is an improvement
Hello Frans Hendriks, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38127
to look at the new patch set (#3).
Change subject: mb/ibase/mb899: Reformat code ......................................................................
mb/ibase/mb899: Reformat code
Change-Id: Ic8c131b8dc6308ed813820f67477d157ccea5b05 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/ibase/mb899/early_init.c M src/mainboard/ibase/mb899/mainboard.c M src/mainboard/ibase/mb899/superio_hwm.c 3 files changed, 47 insertions(+), 34 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/38127/3
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38127 )
Change subject: mb/ibase/mb899: Reformat code ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38127/3/src/mainboard/ibase/mb899/s... File src/mainboard/ibase/mb899/superio_hwm.c:
https://review.coreboot.org/c/coreboot/+/38127/3/src/mainboard/ibase/mb899/s... PS3, Line 33: //#define FAN_TEMPERATURE_30DEGC 0 Why not remove this line? Or replace TAB by SPACE
https://review.coreboot.org/c/coreboot/+/38127/3/src/mainboard/ibase/mb899/s... PS3, Line 74: // get_option(&cpufan_temperature, "cpufan_temperature"); Same: Why not remove these lines
Hello Frans Hendriks, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38127
to look at the new patch set (#4).
Change subject: mb/ibase/mb899: Remove dead code and do code reformating ......................................................................
mb/ibase/mb899: Remove dead code and do code reformating
Change-Id: Ic8c131b8dc6308ed813820f67477d157ccea5b05 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/ibase/mb899/early_init.c M src/mainboard/ibase/mb899/mainboard.c M src/mainboard/ibase/mb899/superio_hwm.c 3 files changed, 65 insertions(+), 59 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/38127/4
Hello Frans Hendriks, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38127
to look at the new patch set (#5).
Change subject: mb/ibase/mb899: Remove dead code and do code reformating ......................................................................
mb/ibase/mb899: Remove dead code and do code reformating
Change-Id: Ic8c131b8dc6308ed813820f67477d157ccea5b05 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/ibase/mb899/early_init.c M src/mainboard/ibase/mb899/mainboard.c M src/mainboard/ibase/mb899/superio_hwm.c 3 files changed, 75 insertions(+), 71 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/38127/5
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38127 )
Change subject: mb/ibase/mb899: Remove dead code and do code reformating ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38127/5/src/mainboard/ibase/mb899/e... File src/mainboard/ibase/mb899/early_init.c:
https://review.coreboot.org/c/coreboot/+/38127/5/src/mainboard/ibase/mb899/e... PS5, Line 75: pnp_write_config(dev, 0x30, 0x03); /* Enable GPIO3+4. pnp_set_enable is not sufficient */ line over 96 characters
Hello Frans Hendriks, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38127
to look at the new patch set (#6).
Change subject: mb/ibase/mb899: Remove dead code and do code reformating ......................................................................
mb/ibase/mb899: Remove dead code and do code reformating
Change-Id: Ic8c131b8dc6308ed813820f67477d157ccea5b05 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/ibase/mb899/early_init.c M src/mainboard/ibase/mb899/mainboard.c M src/mainboard/ibase/mb899/superio_hwm.c 3 files changed, 79 insertions(+), 73 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/38127/6
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38127 )
Change subject: mb/ibase/mb899: Remove dead code and do code reformating ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38127/6/src/mainboard/ibase/mb899/e... File src/mainboard/ibase/mb899/early_init.c:
https://review.coreboot.org/c/coreboot/+/38127/6/src/mainboard/ibase/mb899/e... PS6, Line 75: pnp_write_config(dev, 0x30, 0x03); /* Enable GPIO3+4. pnp_set_enable is not sufficient */ line over 96 characters
HAOUAS Elyes has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/38127 )
Change subject: mb/ibase/mb899: Remove dead code and do code reformating ......................................................................
Abandoned
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Frans Hendriks (Code Review)
-
HAOUAS Elyes (Code Review)