[LinuxBIOS] [PATCH] [notready] my current v3 diff
Uwe Hermann
uwe at hermann-uwe.de
Thu Aug 23 00:33:17 CEST 2007
On Wed, Aug 22, 2007 at 07:14:09PM +0200, Carl-Daniel Hailfinger wrote:
> > Index: include/device/device.h
> > ===================================================================
> > --- include/device/device.h (Revision 476)
> > +++ include/device/device.h (Arbeitskopie)
> > @@ -202,7 +202,7 @@
> > struct resource resource[MAX_RESOURCES];
> > unsigned int resources;
> >
> > - /* link are (down sream) buses attached to the device, usually a leaf
> > + /* link are (downstream) buses attached to the device, usually a leaf
ACK.
> > * device with no children have 0 buses attached and a bridge has 1 bus
> > */
> > struct bus link[MAX_LINKS];
> > Index: mainboard/adl/msm800sev/stage1.c
> > ===================================================================
> > --- mainboard/adl/msm800sev/stage1.c (Revision 476)
> > +++ mainboard/adl/msm800sev/stage1.c (Arbeitskopie)
> > @@ -33,7 +33,9 @@
> > #include <southbridge/amd/cs5536/cs5536.h>
> > #include <superio/winbond/w83627hf/w83627hf.h>
> >
> > +/* This is wrong! SERIAL_DEV can't be >=0x10 because it's a PNP device number */
> > #define SERIAL_DEV 0x30
> > +#define SERIAL_IOBASE 0x3f8
This is probably the wrong location for this information anyway. This is
hardware property and should thus be in the dts, IMHO.
This includes the 0x2e, the 0x30 (I think), and the 0x3f8.
> >
> > void hardware_stage1(void)
> > {
> > @@ -49,6 +51,6 @@
> > * for cs5536
> > */
> > cs5536_disable_internal_uart();
> > - w83627hf_enable_serial(0x2e, 0x30, 0x3f8);
> > + w83627hf_enable_serial(0x2e, SERIAL_DEV, SERIAL_IOBASE);
> > printk(BIOS_DEBUG, "Done %s\n", __FUNCTION__);
> > }
> > Index: device/device.c
> > ===================================================================
> > --- device/device.c (Revision 476)
> > +++ device/device.c (Arbeitskopie)
> > @@ -260,8 +260,10 @@
> > for (curdev = bus->children; curdev; curdev = curdev->sibling) {
> > unsigned int links;
> > int i;
> > - printk(BIOS_SPEW, "%s: %s(%s) have_resources %d enabled %d\n",
> > + printk(BIOS_SPEW,
> > + "%s: %s(%s) dtsname %s have_resources %d enabled %d\n",
> > __func__, bus->dev->dtsname, dev_path(bus->dev),
> > + curdev->dtsname,
> > curdev->have_resources, curdev->enabled);
> > if (curdev->have_resources) {
> > continue;
> > @@ -402,7 +404,11 @@
> > min_align = 0;
> > base = bridge->base;
> >
> > - printk(BIOS_SPEW, "%s compute_allocate_%s: base: %08Lx size: %08Lx align: %d gran: %d\n", dev_path(bus->dev), (bridge->flags & IORESOURCE_IO) ? "io" : (bridge->flags & IORESOURCE_PREFETCH) ? "prefmem" : "mem", base, bridge->size, bridge->align, bridge->gran);
> > + printk(BIOS_SPEW,
> > + "%s compute_allocate_%s: base: %08llx size: %08llx align: %d gran: %d\n",
> > + dev_path(bus->dev),
> > + (bridge->flags & IORESOURCE_IO) ? "io" : (bridge->flags & IORESOURCE_PREFETCH) ? "prefmem" : "mem",
> > + base, bridge->size, bridge->align, bridge->gran);
> >
> > /* We want different minimum alignments for different kinds of
> > * resources. These minimums are not device type specific but
> > @@ -485,7 +491,7 @@
> > base += size;
> >
> > printk(BIOS_SPEW,
> > - "%s %02lx * [0x%08Lx - 0x%08Lx] %s\n",
> > + "%s %02lx * [0x%08llx - 0x%08llx] %s\n",
Yep, I guess so. If this works (didn't follow the last printk-fixes
thread too closely), then please make one patch for all of these and
related printk-fixes.
> > dev_path(dev),
> > resource->index,
> > resource->base,
> > @@ -503,7 +509,11 @@
> > */
> > bridge->size = align_up(base, bridge->gran) - bridge->base;
> >
> > - printk(BIOS_SPEW, "%s compute_allocate_%s: base: %08Lx size: %08Lx align: %d gran: %d done\n", dev_path(bus->dev), (bridge->flags & IORESOURCE_IO) ? "io" : (bridge->flags & IORESOURCE_PREFETCH) ? "prefmem" : "mem", base, bridge->size, bridge->align, bridge->gran);
> > + printk(BIOS_SPEW,
> > + "%s compute_allocate_%s: base: %08llx size: %08llx align: %d gran: %d done\n",
> > + dev_path(bus->dev),
> > + (bridge->flags & IORESOURCE_IO) ? "io" : (bridge->flags & IORESOURCE_PREFETCH) ? "prefmem" : "mem",
> > + base, bridge->size, bridge->align, bridge->gran);
> > }
> >
> > #if defined(CONFIG_PCI_OPTION_ROM_RUN) && CONFIG_PCI_OPTION_ROM_RUN == 1
> > Index: device/pnp_device.c
> > ===================================================================
> > --- device/pnp_device.c (Revision 476)
> > +++ device/pnp_device.c (Arbeitskopie)
> > @@ -87,7 +87,7 @@
> > {
> > if (!(resource->flags & IORESOURCE_ASSIGNED)) {
> > printk(BIOS_ERR,
> > - "ERROR: %s %02lx %s size: 0x%010Lx not assigned\n",
> > + "ERROR: %s %02lx %s size: 0x%010llx not assigned\n",
> > dev_path(dev), resource->index, resource_type(resource),
> > resource->size);
> > return;
> > Index: device/pci_device.c
> > ===================================================================
> > --- device/pci_device.c (Revision 476)
> > +++ device/pci_device.c (Arbeitskopie)
> > @@ -252,10 +252,10 @@
> > printk(BIOS_DEBUG, "%s %02x ->",
> > dev_path(dev), resource->index);
> > printk(BIOS_DEBUG,
> > - " value: 0x%08Lx zeroes: 0x%08Lx ones: 0x%08Lx attr: %08lx\n",
> > + " value: 0x%08llx zeroes: 0x%08llx ones: 0x%08llx attr: %08lx\n",
Are we sure %ll will never produce more than 8 digits? On all
architectures?
> > value, zeroes, ones, attr);
> > printk(BIOS_DEBUG,
> > - "%s %02x -> size: 0x%08Lx max: 0x%08Lx %s\n ",
> > + "%s %02x -> size: 0x%08llx max: 0x%08llx %s\n ",
> > dev_path(dev), resource->index, resource->size,
> > resource->limit, resource_type(resource));
> > }
> > @@ -456,7 +456,7 @@
> > /* Make certain the resource has actually been set. */
> > if (!(resource->flags & IORESOURCE_ASSIGNED)) {
> > printk(BIOS_ERR,
> > - "ERROR: %s %02lx %s size: 0x%010Lx not assigned\n",
> > + "ERROR: %s %02lx %s size: 0x%010llx not assigned\n",
> > dev_path(dev), resource->index, resource_type(resource),
> > resource->size);
> > return;
> > @@ -863,7 +863,7 @@
> > (*list)->path.type);
> > continue;
> > }
> > - printk(BIOS_SPEW, "%s: check dev %s it has devfn 0x%x\n",
> > + printk(BIOS_SPEW, "%s: check dev %s it has devfn 0x%02x\n",
ACK, same for all other %x -> %02x (etc.) changes.
> > __func__, (*list)->dtsname, (*list)->path.u.pci.devfn);
> > if ((*list)->path.u.pci.devfn == devfn) {
> > /* Unlink from the list. */
> > Index: device/device_util.c
> > ===================================================================
> > --- device/device_util.c (Revision 476)
> > +++ device/device_util.c (Arbeitskopie)
> > @@ -229,7 +229,7 @@
> > memcpy(buffer, "Root Device", 12);
> > break;
> > case DEVICE_ID_PCI:
> > - sprintf(buffer, "PCI: %02x:%02x", id->u.pci.vendor,
> > + sprintf(buffer, "PCI: %04x:%04x", id->u.pci.vendor,
> > id->u.pci.device);
> > break;
> > case DEVICE_ID_PNP:
> > @@ -243,7 +243,7 @@
> > id->u.apic.device);
> > break;
> > case DEVICE_ID_PCI_DOMAIN:
> > - sprintf(buffer, "PCI_DOMAIN: %02x:%02x",
> > + sprintf(buffer, "PCI_DOMAIN: %04x:%04x",
> > id->u.pci_domain.vendor,
> > id->u.pci_domain.device);
> > break;
> > @@ -602,7 +602,7 @@
> > #endif
> > }
> > printk(BIOS_DEBUG,
> > - "%s %02lx <- [0x%010Lx - 0x%010Lx] %s%s%s\n",
> > + "%s %02lx <- [0x%010llx - 0x%010llx] %s%s%s\n",
> > dev_path(dev),
> > resource->index,
> > base, end, buf, resource_type(resource), comment);
> > @@ -656,7 +656,7 @@
> > for (i = 0; i < curdev->resources; i++) {
> > struct resource *resource = &curdev->resource[i];
> > printk(BIOS_SPEW,
> > - "%s: dev %s, resource %d, flags %lx base 0x%Lx size 0x%Lx\n",
> > + "%s: dev %s, resource %d, flags %lx base 0x%llx size 0x%llx\n",
> > __func__, curdev->dtsname, i, resource->flags,
> > resource->base, resource->size);
> > /* If it isn't the right kind of resource ignore it. */
> > Index: lib/stage2.c
> > ===================================================================
> > --- lib/stage2.c (Revision 476)
> > +++ lib/stage2.c (Arbeitskopie)
> > @@ -31,8 +31,9 @@
> > /**
> > * Main function of the DRAM part of LinuxBIOS.
> > *
> > - * LinuxBIOS is divided into pre-DRAM part and DRAM part. The phases before
> > - * this part are phase 0 and phase 1. This part contains phases x through y.
> > + * LinuxBIOS is divided into pre-DRAM part and DRAM part. The stages before
> > + * this part are stage 0 and stage 1. This part contains stage 2, which
> > + * consists of phases 1 through 6.
Looks correct, but using "stages" _and_ "phases" will confuse the hell
out of everyone. I vote for only using stage 0-2, just drop the phases
terminology completely. Or maybe something like "which consists of
multiple steps"?
> > *
> > * Device Enumeration: in the dev_enumerate() phase.
> > *
> > @@ -53,6 +54,7 @@
> >
> > post_code(0x20);
> >
> > + /* TODO: Explain why we use printk here although it is impossible */
> > printk(BIOS_NOTICE, console_test);
Hm, good point. It works in QEMU, but that's not a good indicator in
this case.
> >
> > dev_init();
> > Index: lib/elfboot.c
> > ===================================================================
> > --- lib/elfboot.c (Revision 476)
> > +++ lib/elfboot.c (Arbeitskopie)
> > @@ -61,7 +61,7 @@
> > }
> > if (i == mem_entries) {
> > printk(BIOS_ERR, "No matching RAM area found for range:\n");
> > - printk(BIOS_ERR, " [0x%016Lx, 0x%016Lx)\n", start, end);
> > + printk(BIOS_ERR, " [0x%016llx, 0x%016llx)\n", start, end);
> > printk(BIOS_ERR, "RAM areas\n");
> > for(i = 0; i < mem_entries; i++) {
> > u64 mstart, mend;
> > @@ -69,7 +69,7 @@
> > mtype = mem->map[i].type;
> > mstart = unpack_lb64(mem->map[i].start);
> > mend = mstart + unpack_lb64(mem->map[i].size);
> > - printk(BIOS_ERR, " [0x%016Lx, 0x%016Lx) %s\n",
> > + printk(BIOS_ERR, " [0x%016llx, 0x%016llx) %s\n",
> > mstart, mend, (mtype == LB_MEM_RAM)?"RAM":"Reserved");
> >
> > }
> > Index: northbridge/intel/i440bxemulation/i440bx.c
> > ===================================================================
> > --- northbridge/intel/i440bxemulation/i440bx.c (Revision 476)
> > +++ northbridge/intel/i440bxemulation/i440bx.c (Arbeitskopie)
> > @@ -79,7 +79,7 @@
> > resource->size = ((resource_t) sizek) << 10;
> > resource->flags = IORESOURCE_MEM | IORESOURCE_CACHEABLE |
> > IORESOURCE_FIXED | IORESOURCE_STORED | IORESOURCE_ASSIGNED;
> > - printk(BIOS_DEBUG, "%s: add ram resoource %Ld bytes\n", __func__,
> > + printk(BIOS_DEBUG, "%s: add ram resource %lld bytes\n", __func__,
> > resource->size);
> > }
> >
> > Index: arch/x86/linuxbios_table.c
> > ===================================================================
> > --- arch/x86/linuxbios_table.c (Revision 476)
> > +++ arch/x86/linuxbios_table.c (Arbeitskopie)
> > @@ -177,7 +177,7 @@
> > {
> > int entries;
> >
> > - printk(BIOS_DEBUG, "%s: start 0x%Lx size 0x%Lx\n",
> > + printk(BIOS_DEBUG, "%s: start 0x%llx size 0x%llx\n",
> > __func__, start, size);
> >
> > entries = (mem->size - sizeof(*mem))/sizeof(mem->map[0]);
> > Index: arch/x86/pci_ops_mmconf.c
> > ===================================================================
> > --- arch/x86/pci_ops_mmconf.c (Revision 476)
> > +++ arch/x86/pci_ops_mmconf.c (Arbeitskopie)
> > @@ -18,32 +18,32 @@
> >
> > #include <mmio_conf.h>
> >
> > -static uint8_t pci_mmconf_read_config8(struct bus *pbus, int bus, int devfn, int where)
> > +static u8 pci_mmconf_read_config8(struct bus *pbus, int bus, int devfn, int where)
ACK (same for the other occurences).
> > {
> > return (read8x(PCI_MMIO_ADDR(bus, devfn, where)));
> > }
> >
> > -static uint16_t pci_mmconf_read_config16(struct bus *pbus, int bus, int devfn, int where)
> > +static u16 pci_mmconf_read_config16(struct bus *pbus, int bus, int devfn, int where)
> > {
> > return (read16x(PCI_MMIO_ADDR(bus, devfn, where)));
> > }
> >
> > -static uint32_t pci_mmconf_read_config32(struct bus *pbus, int bus, int devfn, int where)
> > +static u32 pci_mmconf_read_config32(struct bus *pbus, int bus, int devfn, int where)
> > {
> > return (read32x(PCI_MMIO_ADDR(bus, devfn, where)));
> > }
> >
> > -static void pci_mmconf_write_config8(struct bus *pbus, int bus, int devfn, int where, uint8_t value)
> > +static void pci_mmconf_write_config8(struct bus *pbus, int bus, int devfn, int where, u8 value)
> > {
> > write8x(PCI_MMIO_ADDR(bus, devfn, where), value);
> > }
> >
> > -static void pci_mmconf_write_config16(struct bus *pbus, int bus, int devfn, int where, uint16_t value)
> > +static void pci_mmconf_write_config16(struct bus *pbus, int bus, int devfn, int where, u16 value)
> > {
> > write8x(PCI_MMIO_ADDR(bus, devfn, where), value);
> > }
> >
> > -static void pci_mmconf_write_config32(struct bus *pbus, int bus, int devfn, int where, uint32_t value)
> > +static void pci_mmconf_write_config32(struct bus *pbus, int bus, int devfn, int where, u32 value)
> > {
> > write8x(PCI_MMIO_ADDR(bus, devfn, where), value);
> > }
Please repost all of this as two or three separate patches, each fixing
a single issue repository-wide. I think we can apply this soon.
Uwe.
--
http://www.hermann-uwe.de | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20070823/ca1dd23b/attachment.sig>
More information about the coreboot
mailing list