[coreboot] v3 Kontron warnings

[Myles Watson]

On Wed, Dec 10, 2008 at 11:12 AM, Carl-Daniel Hailfinger <
c-d.hailfinger.devel.2006 at gmx.net> wrote:

> On 10.12.2008 17:57, Myles Watson wrote:
> > This patch fixes some warnings in the Kontron build and a little bit of
> > whitespace.
> >
> > Signed-off-by: Myles Watson <mylesgw at gmail.com>
> >
>
> See my comments below. With the comments addressed, this is
> Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
> If you feel that addressing the comments needs more work, feel free to
> do a partial commit.


Rev 1068.

Thanks,
Myles


>
> Regards,
> Carl-Daniel
>
> > Thanks,
> > Myles
> > Index: svn/northbridge/intel/i945/northbridge.c
> > ===================================================================
> > --- svn.orig/northbridge/intel/i945/northbridge.c
> > +++ svn/northbridge/intel/i945/northbridge.c
> > @@ -179,17 +179,17 @@ static unsigned int i945_pci_domain_scan
> >  #warning get number of 945 pci domain ops
> >  struct device_operations i945_pci_domain_ops = {
> >       .id = {.type = DEVICE_ID_PCI,
> > -             {.pci = {.vendor = PCI_VENDOR_ID_INTEL,
> > -                           .device = 0x6789}}},
> > +            {.pci = {.vendor = PCI_VENDOR_ID_INTEL,
> > +                     .device = 0x6789}}},
> >       .constructor             = default_device_constructor,
> > -     .reset_bus              = pci_bus_reset,
> > +     .reset_bus               = pci_bus_reset,
> >       .phase3_scan             = i945_pci_domain_scan_bus,
> >       .phase4_read_resources   = I945_pci_domain_read_resources,
> >       .phase4_set_resources    = I945_pci_domain_set_resources,
> >       .phase5_enable_resources = enable_childrens_resources,
> >       .phase6_init             = NULL,
> >       .ops_pci                 = &pci_dev_ops_pci,
> > -     .ops_pci_bus      = &pci_cf8_conf1,     /* Do we want to use the
> memory mapped space here? */
> > +     .ops_pci_bus             = &pci_cf8_conf1,      /* Do we want to
> use the memory mapped space here? */
> >  };
> >
> >  static void mc_read_resources(struct device * dev)
> > @@ -212,8 +212,8 @@ static void mc_read_resources(struct dev
> >           IORESOURCE_MEM | IORESOURCE_FIXED | IORESOURCE_STORED |
> >           IORESOURCE_ASSIGNED;
> >
> > -     printk(BIOS_DEBUG, "Adding PCIe enhanced config space BAR
> 0x%08lx-0x%08lx.\n",
> > -                  (u64) resource->base, (u64) (resource->base +
> resource->size));
> > +     printk(BIOS_DEBUG, "Adding PCIe enhanced config space BAR
> 0x%08llx-0x%08llx.\n",
> > +            resource->base, resource->base + resource->size);
> >  }
> >
> >  static void mc_set_resources(struct device * dev)
> > Index: svn/northbridge/intel/i945/raminit.c
> > ===================================================================
> > --- svn.orig/northbridge/intel/i945/raminit.c
> > +++ svn/northbridge/intel/i945/raminit.c
> > @@ -1070,7 +1070,7 @@ static void sdram_rcomp_buffer_strength_
> >       };
> >
> >       const u8 * strength_multiplier;
> > -     const u8* const * slew_group_lookup;
> > +     const u8 * slew_group_lookup;
> >
>
> I'm curious. This is a change in semantics. Are you sure the code will
> work with that change? Can you get Stefan to confirm? He was a bit wary
> of changes to the Intel code because it exploded on seemingly obvious
> changes.


I can't confirm that it will work, but I think it is the correct change.
This pointer is assigned a const u8 *, then passed to a function as a const
u8 *.  That's its only usage.


>
>
>
> >       int idx;
> >
> >       /* Set Strength Multipliers */
> > Index: svn/southbridge/intel/i82801gx/pci.c
> > ===================================================================
> > --- svn.orig/southbridge/intel/i82801gx/pci.c
> > +++ svn/southbridge/intel/i82801gx/pci.c
> > @@ -57,7 +57,6 @@ static void pci_init(struct device *dev)
> >  static void ich_pci_dev_enable_resources(struct device *dev)
> >  {
> >       const struct pci_operations *ops;
> > -     u16 command;
> >
> >       /* Set the subsystem vendor and device id for mainboard devices */
> >       ops = ops_pci(dev);
> > @@ -72,6 +71,7 @@ static void ich_pci_dev_enable_resources
> >       }
> >
> >  #if 0
> > +     u16 command;
> >       /* If we write to PCI_COMMAND, on some systems
> >        * this will cause the ROM and APICs not being visible
> >        * anymore.
> > @@ -103,7 +103,7 @@ static void ich_pci_bus_enable_resources
> >       enable_childrens_resources(dev);
> >  }
> >
> > -static void set_subsystem(struct device * dev, u16 vendor, u16 device)
> > +static void set_subsystem(struct device * dev, unsigned vendor, unsigned
> device)
> >
>
> This change is the exact opposite of your set_subsystem patch.
>

Yes.  Lots of fast reviews :)  I won't include it.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20081210/f792b6f5/attachment.html>