See patch.
Uwe.
-----Original Message----- From: coreboot-bounces@coreboot.org [mailto:coreboot-bounces@coreboot.org] On Behalf Of Uwe Hermann Sent: Saturday, November 15, 2008 10:12 AM To: coreboot@coreboot.org Subject: [coreboot] [PATCH] v3: Fix parts of the Winbond W83627HF dts
See patch. Fix some incorrect entries in superio/winbond/w83627hf/dts.
The hardware monitor defaults as per datasheet are 0x0000 / 0, but on hardware that uses this functionality it seems to be 0x290 / 5 often.>
I think that means that it should be 0x290 /5 in the dts. The dts should reflect the common case in actual use, not the power on defaults of the registers. Hopefully that will minimize the need to override defaults in the mainboard dts.
I'll Ack the other changes though.
Acked-by: Myles Watson mylesgw@gmail.com
Thanks, Myles
On Sat, Nov 15, 2008 at 02:04:38PM -0700, Myles Watson wrote:
The hardware monitor defaults as per datasheet are 0x0000 / 0, but on hardware that uses this functionality it seems to be 0x290 / 5 often.>
I think that means that it should be 0x290 /5 in the dts. The dts should reflect the common case in actual use, not the power on defaults of the registers.
Yeah, I guess you're right. Fixed.
Hopefully that will minimize the need to override defaults in the mainboard dts.
I don't know, I have mixed feeling with the Super I/O dts files. I somehow preferred the v2 way of setting all values explicitly in the board's Config.lb (now the board's dts). It's not very likely that any of the Super I/O values are the same as the defaults we set. My bet is that _all_ boards will override _all_ LDN values (and that makes the board dts a lot more readable and easier to understand too, IMHO).
I'll Ack the other changes though.
Acked-by: Myles Watson mylesgw@gmail.com
Thanks, r1034.
Uwe.
Uwe Hermann wrote:
Hopefully that will minimize the need to override defaults in the mainboard dts.
I don't know, I have mixed feeling with the Super I/O dts files. I somehow preferred the v2 way of setting all values explicitly in the board's Config.lb (now the board's dts).
I am completely with Myles. I want useful defaults in superio/dtses.
It's not very likely that any of the Super I/O values are the same as the defaults we set.
? These are mostly legacy devices that have to stay at known addresses for best compatibility.
My bet is that _all_ boards will override _all_ LDN values (and that makes the board dts a lot more readable and easier to understand too, IMHO).
I think that's a sad goal. How it turns out depends on what we do. If we have sane defaults in superio/dts then no mainboard/dtses need to bother. That is as it should be.
//Peter
Hey Uwe if you get a chance and want to fix the thg too, I'll be happy :-)
ron
-----Original Message----- From: Uwe Hermann [mailto:uwe@hermann-uwe.de] Sent: Sunday, November 16, 2008 6:51 AM To: Myles Watson Cc: coreboot@coreboot.org Subject: Re: [coreboot] [PATCH] v3: Fix parts of the Winbond W83627HF dts
On Sat, Nov 15, 2008 at 02:04:38PM -0700, Myles Watson wrote:
The hardware monitor defaults as per datasheet are 0x0000 / 0, but on hardware that uses this functionality it seems to be 0x290 / 5 often.>
I think that means that it should be 0x290 /5 in the dts. The dts
should
reflect the common case in actual use, not the power on defaults of the registers.
Yeah, I guess you're right. Fixed.
Hopefully that will minimize the need to override defaults in the mainboard dts.
I don't know, I have mixed feeling with the Super I/O dts files. I somehow preferred the v2 way of setting all values explicitly in the board's Config.lb (now the board's dts). It's not very likely that any of the Super I/O values are the same as the defaults we set. My bet is that _all_ boards will override _all_ LDN values (and that makes the board dts a lot more readable and easier to understand too, IMHO).
I can see your point. The thing I like about the way it is now is that you can put all that information in the board's dts if you want to.
Thanks, Myles