<br><br><div class="gmail_quote">On Wed, Dec 10, 2008 at 11:12 AM, Carl-Daniel Hailfinger <span dir="ltr"><<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div><div></div><div class="Wj3C7c">On 10.12.2008 17:57, Myles Watson wrote:<br>
> This patch fixes some warnings in the Kontron build and a little bit of<br>
> whitespace.<br>
><br>
> Signed-off-by: Myles Watson <<a href="mailto:mylesgw@gmail.com">mylesgw@gmail.com</a>><br>
><br>
<br>
</div></div>See my comments below. With the comments addressed, this is<br>
Acked-by: Carl-Daniel Hailfinger <<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>><br>
If you feel that addressing the comments needs more work, feel free to<br>
do a partial commit.</blockquote><div><br>Rev 1068.<br><br>Thanks,<br>Myles<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
Regards,<br>
Carl-Daniel<br>
<br>
> Thanks,<br>
> Myles<br>
> Index: svn/northbridge/intel/i945/northbridge.c<br>
> ===================================================================<br>
> --- svn.orig/northbridge/intel/i945/northbridge.c<br>
> +++ svn/northbridge/intel/i945/northbridge.c<br>
> @@ -179,17 +179,17 @@ static unsigned int i945_pci_domain_scan<br>
>  #warning get number of 945 pci domain ops<br>
>  struct device_operations i945_pci_domain_ops = {<br>
>       .id = {.type = DEVICE_ID_PCI,<br>
> -             {.pci = {.vendor = PCI_VENDOR_ID_INTEL,<br>
> -                           .device = 0x6789}}},<br>
> +            {.pci = {.vendor = PCI_VENDOR_ID_INTEL,<br>
> +                     .device = 0x6789}}},<br>
>       .constructor             = default_device_constructor,<br>
> -     .reset_bus              = pci_bus_reset,<br>
> +     .reset_bus               = pci_bus_reset,<br>
>       .phase3_scan             = i945_pci_domain_scan_bus,<br>
>       .phase4_read_resources   = I945_pci_domain_read_resources,<br>
>       .phase4_set_resources    = I945_pci_domain_set_resources,<br>
>       .phase5_enable_resources = enable_childrens_resources,<br>
>       .phase6_init             = NULL,<br>
>       .ops_pci                 = &pci_dev_ops_pci,<br>
> -     .ops_pci_bus      = &pci_cf8_conf1,     /* Do we want to use the memory mapped space here? */<br>
> +     .ops_pci_bus             = &pci_cf8_conf1,      /* Do we want to use the memory mapped space here? */<br>
>  };<br>
><br>
>  static void mc_read_resources(struct device * dev)<br>
> @@ -212,8 +212,8 @@ static void mc_read_resources(struct dev<br>
>           IORESOURCE_MEM | IORESOURCE_FIXED | IORESOURCE_STORED |<br>
>           IORESOURCE_ASSIGNED;<br>
><br>
> -     printk(BIOS_DEBUG, "Adding PCIe enhanced config space BAR 0x%08lx-0x%08lx.\n",<br>
> -                  (u64) resource->base, (u64) (resource->base + resource->size));<br>
> +     printk(BIOS_DEBUG, "Adding PCIe enhanced config space BAR 0x%08llx-0x%08llx.\n",<br>
> +            resource->base, resource->base + resource->size);<br>
>  }<br>
><br>
>  static void mc_set_resources(struct device * dev)<br>
<div class="Ih2E3d">> Index: svn/northbridge/intel/i945/raminit.c<br>
> ===================================================================<br>
> --- svn.orig/northbridge/intel/i945/raminit.c<br>
> +++ svn/northbridge/intel/i945/raminit.c<br>
> @@ -1070,7 +1070,7 @@ static void sdram_rcomp_buffer_strength_<br>
>       };<br>
><br>
>       const u8 * strength_multiplier;<br>
> -     const u8* const * slew_group_lookup;<br>
> +     const u8 * slew_group_lookup;<br>
><br>
<br>
</div>I'm curious. This is a change in semantics. Are you sure the code will<br>
work with that change? Can you get Stefan to confirm? He was a bit wary<br>
of changes to the Intel code because it exploded on seemingly obvious<br>
changes.</blockquote><div><br>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.<br>
 <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br>
<div class="Ih2E3d"><br>
<br>
>       int idx;<br>
><br>
>       /* Set Strength Multipliers */<br>
> Index: svn/southbridge/intel/i82801gx/pci.c<br>
> ===================================================================<br>
> --- svn.orig/southbridge/intel/i82801gx/pci.c<br>
> +++ svn/southbridge/intel/i82801gx/pci.c<br>
> @@ -57,7 +57,6 @@ static void pci_init(struct device *dev)<br>
>  static void ich_pci_dev_enable_resources(struct device *dev)<br>
>  {<br>
>       const struct pci_operations *ops;<br>
> -     u16 command;<br>
><br>
>       /* Set the subsystem vendor and device id for mainboard devices */<br>
>       ops = ops_pci(dev);<br>
> @@ -72,6 +71,7 @@ static void ich_pci_dev_enable_resources<br>
>       }<br>
><br>
>  #if 0<br>
> +     u16 command;<br>
>       /* If we write to PCI_COMMAND, on some systems<br>
>        * this will cause the ROM and APICs not being visible<br>
>        * anymore.<br>
> @@ -103,7 +103,7 @@ static void ich_pci_bus_enable_resources<br>
>       enable_childrens_resources(dev);<br>
>  }<br>
><br>
</div><div class="Ih2E3d">> -static void set_subsystem(struct device * dev, u16 vendor, u16 device)<br>
> +static void set_subsystem(struct device * dev, unsigned vendor, unsigned device)<br>
><br>
<br>
</div>This change is the exact opposite of your set_subsystem patch.<br><div class="Ih2E3d"></div></blockquote><div><br>Yes.  Lots of fast reviews :)  I won't include it.<br></div></div><br>