| Submitter | Myles Watson |
|---|---|
| Date | 2010-06-21 13:36:39 |
| Message ID | <AANLkTinoB8MGsxP65w2pwjmv3DPje9cRLaWJ2UNXOyyX@mail.gmail.com> |
| Download | mbox | patch |
| Permalink | /patch/1533/ |
| State | Superseded |
| Headers | show |
Comments
On Mon, Jun 21, 2010 at 07:36:39AM -0600, Myles Watson wrote: > On Mon, Jun 21, 2010 at 5:59 AM, Myles Watson <mylesgw@gmail.com> wrote: > > On Sun, Jun 20, 2010 at 8:11 PM, Ward Vandewege <ward@gnu.org> wrote: > >> Hi Myles, > >> > >> Everything seems fine with either patch - but there are some differences in > >> the boot output. > >> > >> I also ran the 'sensors' command. > >> > >> Output here: > >> > >> http://ward.vandewege.net/coreboot/s2881/20100617-myles/ > >> > >> I ran 4 tests: stock r5635 (head), stock r5632 (revision prior to this > >> changeset), r5635 + patch 1 and r5635 + patch 2. > > Thanks for testing. > > > > It looks like only 5632 has the "ADT7463 properly initialized" > > message. One problem is that patch 2 was meant to be applied after > > patch 1, so the device didn't end up in the tree for that run. I'll > > have to think about why the only message from the new device with > > patch 1 is "I2C: 00:d0 missing read_resources" For some reason it > > doesn't look like it got the correct ops. > > > > I wonder why the temperature values look right in all cases. Does it > > need to be cold booted in order for the initialization to be needed? > > The ADM1027 doesn't expect to have children, so it has no scan_bus > method. I had thought that the ADM1027 was some kind of a controller > for the ADT4763, but it looks like the same type of device. Is there > really an ADM1027 on your board? I don't see it in your sensors > output. > > So... the first two patches are the same as before. The third patch > adds a scan_bus method to the ADM1027 so that the ADT4763 can be > initialized, and the fourth patch replaces the ADM1027 with the > ADT4763 in the device tree, and removes the third patch. > > I'd be interested in head + 1 + 2 + 3, and head + 1 + 2 + 3 + 4. See http://ward.vandewege.net/coreboot/s2881/20100621-myles/ The temperature differences (higher readouts in head + 1 + 2 + 3 + 4 seem to be the consequence of less than optimal cooling of the board - and the fact that it was entirely cooled off before the first boot (head + 1 + 2 + 3) only. Thanks, Ward.
> > The ADM1027 doesn't expect to have children, so it has no scan_bus > > method. I had thought that the ADM1027 was some kind of a controller > > for the ADT4763, but it looks like the same type of device. Is there > > really an ADM1027 on your board? I don't see it in your sensors > > output. > > > > So... the first two patches are the same as before. The third patch > > adds a scan_bus method to the ADM1027 so that the ADT4763 can be > > initialized, and the fourth patch replaces the ADM1027 with the > > ADT4763 in the device tree, and removes the third patch. > > > > I'd be interested in head + 1 + 2 + 3, and head + 1 + 2 + 3 + 4. > > See > > http://ward.vandewege.net/coreboot/s2881/20100621-myles/ Thanks for testing Ward! As far as I can see, both worked, but 1+2+3+4 is cleaner. It doesn't look like there is an ADM1027 on your board. Is there something missing before an Ack & commit? Thanks, Myles
On Tue, Jun 22, 2010 at 01:30:45PM -0600, Myles Watson wrote: > > > The ADM1027 doesn't expect to have children, so it has no scan_bus > > > method. I had thought that the ADM1027 was some kind of a controller > > > for the ADT4763, but it looks like the same type of device. Is there > > > really an ADM1027 on your board? I don't see it in your sensors > > > output. > > > > > > So... the first two patches are the same as before. The third patch > > > adds a scan_bus method to the ADM1027 so that the ADT4763 can be > > > initialized, and the fourth patch replaces the ADM1027 with the > > > ADT4763 in the device tree, and removes the third patch. > > > > > > I'd be interested in head + 1 + 2 + 3, and head + 1 + 2 + 3 + 4. > > > > See > > > > http://ward.vandewege.net/coreboot/s2881/20100621-myles/ > > Thanks for testing Ward! As far as I can see, both worked, but 1+2+3+4 is > cleaner. It doesn't look like there is an ADM1027 on your board. > > Is there something missing before an Ack & commit? I think it's good. Thanks for writing the patches! Acked-by: Ward Vandewege <ward@gnu.org> Thanks, Ward.
> I think it's good. Thanks for writing the patches! No problem. I've wanted the initialization order to be fixed for a long time. It's good to have it done. > Acked-by: Ward Vandewege <ward@gnu.org> Rev 5641. Thanks, Myles
Patch
Index: svn/src/mainboard/tyan/s2881/Makefile.inc =================================================================== --- svn.orig/src/mainboard/tyan/s2881/Makefile.inc +++ svn/src/mainboard/tyan/s2881/Makefile.inc @@ -1,2 +1 @@ -obj-y += ../../../drivers/i2c/adm1027/adm1027.o obj-y += ../../../drivers/i2c/adt7463/adt7463.o Index: svn/src/mainboard/tyan/s2881/devicetree.cb =================================================================== --- svn.orig/src/mainboard/tyan/s2881/devicetree.cb +++ svn/src/mainboard/tyan/s2881/devicetree.cb @@ -102,13 +102,8 @@ chip northbridge/amd/amdk8/root_complex chip drivers/generic/generic #dimm 1-1-1 device i2c 57 on end end - chip drivers/i2c/adm1027 # ADT7463A CPU0/1 temp, CPU1 vid, SYS FAN 1/2/3 - device i2c 2d on - chip drivers/i2c/adt7463 - device i2c d0 on - end - end - end + chip drivers/i2c/adt7463 # CPU0/1 temp, CPU1 vid, SYS FAN 1/2/3 + device i2c 2d on end end chip drivers/generic/generic # Winbond HWM 0x54 CPU0/1 VRM temp, SYSFAN 4,CPU0 vid, CPU0/1 FAN device i2c 2a on end Index: svn/src/drivers/i2c/adm1027/adm1027.c =================================================================== --- svn.orig/src/drivers/i2c/adm1027/adm1027.c +++ svn/src/drivers/i2c/adm1027/adm1027.c @@ -56,7 +56,6 @@ static void adm1027_noop(device_t dummy) } static struct device_operations adm1027_operations = { - .scan_bus = scan_static_bus, .read_resources = adm1027_noop, .set_resources = adm1027_noop, .enable_resources = adm1027_noop,