[patch] add required info to not-matched vendor/device id

[Elia Yehuda]

Signed-off-by: Elia Yehuda

Index: src/devices/pci_rom.c
===================================================================
--- src/devices/pci_rom.c    (revision 3698)
+++ src/devices/pci_rom.c    (working copy)
@@ -67,8 +67,10 @@
     printk_spew("PCI ROM Image, Vendor %04x, Device %04x,\n",
             rom_data->vendor, rom_data->device);
     if (dev->vendor != rom_data->vendor || dev->device != rom_data->device)
{
-        printk_err("Device or Vendor ID mismatch Vendor %04x, Device
%04x\n",
-               rom_data->vendor, rom_data->device);
+        printk_err("Device or Vendor ID mismatch Vendor %04x, Device %04x "
+               "(Vendor %04x, Device %04x)\n",
+               rom_data->vendor, rom_data->device,
+               dev->vendor, dev->device);
         return NULL;
     }

[Carl-Daniel Hailfinger]

On 07.11.2008 03:47, Elia Yehuda wrote:
> Signed-off-by: Elia Yehuda

A small changelog (2 lines or so, more if you like) would be appreciated.
> Index: src/devices/pci_rom.c
>
> --- src/devices/pci_rom.c    (revision 3698)
> +++ src/devices/pci_rom.c    (working copy)
> @@ -67,8 +67,10 @@
>      printk_spew("PCI ROM Image, Vendor %04x, Device %04x,\n",
>              rom_data->vendor, rom_data->device);
>      if (dev->vendor != rom_data->vendor || dev->device != rom_data->device)
> {
>    printk_err("Device or Vendor ID mismatch Vendor %04x, Device
>
> %04x\n",
>           rom_data->vendor, rom_data->device);
>    printk_err("Device or Vendor ID mismatch Vendor %04x, Device %04x "
>           "(Vendor %04x, Device %04x)\n",
>           rom_data->vendor, rom_data->device,
>           dev->vendor, dev->device);
>    return NULL;
> }

The patch was mangled by gmail. AFAIK that's an unfixable bug with the
gmail web interface. Please attach the patch instead.

About the contents of the patch: The new message still requires people
to read the source code. How about:
"Device or Vendor ID mismatch: read Vendor %04x, Device %04x, expected
Vendor %04x, Device %04x\n"

Regards,
Carl-Daniel

[patch] add required info to not-matched vendor/device id

[Elia Yehuda]

On Fri, Nov 7, 2008 at 5:00 AM, Carl-Daniel Hailfinger <
[e-mail]> wrote:
> On 07.11.2008 03:47, Elia Yehuda wrote:
> > Signed-off-by: Elia Yehuda
>
> A small changelog (2 lines or so, more if you like) would be appreciated.

ok, i thought the title was informative enough... i'll try harder next
time!!! :-)
> > Index: src/devices/pci_rom.c
> >
> > --- src/devices/pci_rom.c    (revision 3698)
> > +++ src/devices/pci_rom.c    (working copy)
> > @@ -67,8 +67,10 @@
> >      printk_spew("PCI ROM Image, Vendor %04x, Device %04x,\n",
> >              rom_data->vendor, rom_data->device);
> >      if (dev->vendor != rom_data->vendor || dev->device !=
>
> rom_data->device)
> > {
> >    printk_err("Device or Vendor ID mismatch Vendor %04x, Device
> >
> > %04x\n",
> >           rom_data->vendor, rom_data->device);
> >    printk_err("Device or Vendor ID mismatch Vendor %04x, Device
>
> %04x "
> >           "(Vendor %04x, Device %04x)\n",
> >           rom_data->vendor, rom_data->device,
> >           dev->vendor, dev->device);
> >    return NULL;
> > }
>
> The patch was mangled by gmail. AFAIK that's an unfixable bug with the
> gmail web interface. Please attach the patch instead.

will do
> About the contents of the patch: The new message still requires people
> to read the source code. How about:
> "Device or Vendor ID mismatch: read Vendor %04x, Device %04x, expected
> Vendor %04x, Device %04x\n"

such a long line breaks the 80 column boundary. is that ok?
> Regards,
> Carl-Daniel

[Carl-Daniel Hailfinger]

On 07.11.2008 04:04, Elia Yehuda wrote:
> On Fri, Nov 7, 2008 at 5:00 AM, Carl-Daniel Hailfinger <
> [e-mail]> wrote:
> > On 07.11.2008 03:47, Elia Yehuda wrote:
> > > Signed-off-by: Elia Yehuda
> >
> > A small changelog (2 lines or so, more if you like) would be appreciated.
>
> ok, i thought the title was informative enough... i'll try harder next
> time!!! :-)

Rough guideline for changelogs is:
- Keep them in the mail body.
- Make sure that someone only reading the changelog (and not the code)
knows what's going on.

"Not-matched vendor/device id" can be anything. Please mention that this
is about PCI ROMs and that the mismatch occurs between the vendor and
device ID stored in the ROM and the PCI vendor and device ID of the
corresponding PCI device.
> > > Index: src/devices/pci_rom.c
> > >
> > > --- src/devices/pci_rom.c    (revision 3698)
> > > +++ src/devices/pci_rom.c    (working copy)
> > > @@ -67,8 +67,10 @@
> > >      printk_spew("PCI ROM Image, Vendor %04x, Device %04x,\n",
> > >              rom_data->vendor, rom_data->device);
> > >      if (dev->vendor != rom_data->vendor || dev->device !=
> >
> > rom_data->device)
> > > {
> > >    printk_err("Device or Vendor ID mismatch Vendor %04x, Device
> > >
> > > %04x\n",
> > >           rom_data->vendor, rom_data->device);
> > >    printk_err("Device or Vendor ID mismatch Vendor %04x, Device
> >
> > %04x "
> > >           "(Vendor %04x, Device %04x)\n",
> > >           rom_data->vendor, rom_data->device,
> > >           dev->vendor, dev->device);
> > >    return NULL;
> > > }
> >
> > The patch was mangled by gmail. AFAIK that's an unfixable bug with the
> > gmail web interface. Please attach the patch instead.
>
> will do

Thanks.
> > About the contents of the patch: The new message still requires people
> > to read the source code. How about:
> > "Device or Vendor ID mismatch: read Vendor %04x, Device %04x, expected
> > Vendor %04x, Device %04x\n"
>
> such a long line breaks the 80 column boundary. is that ok?

Of course you can break the line. My point was more about the message
itself than cosmetic linebreaks.

Regards.
Carl-Daniel

[patch] add required info to not-matched vendor/device id

[Elia Yehuda]

Signed-off-by: Elia Yehuda

This patch adds information in printk_err() about which vendor/device id
mismatches to which expected
vendor/device on pci_rom_probe().

Regards,
Elia.

On Fri, Nov 7, 2008 at 5:24 AM, Carl-Daniel Hailfinger <
[e-mail]> wrote:
> On 07.11.2008 04:04, Elia Yehuda wrote:
> > On Fri, Nov 7, 2008 at 5:00 AM, Carl-Daniel Hailfinger <
> > [e-mail]> wrote:
> > > On 07.11.2008 03:47, Elia Yehuda wrote:
> > > > Signed-off-by: Elia Yehuda
> > >
> > > A small changelog (2 lines or so, more if you like) would be
>
> appreciated.
> > >
> >
> > ok, i thought the title was informative enough... i'll try harder next
> > time!!! :-)
>
> Rough guideline for changelogs is:
> Keep them in the mail body.
> Make sure that someone only reading the changelog (and not the code)
>
> knows what's going on.
>
> "Not-matched vendor/device id" can be anything. Please mention that this
> is about PCI ROMs and that the mismatch occurs between the vendor and
> device ID stored in the ROM and the PCI vendor and device ID of the
> corresponding PCI device.
> > > > Index: src/devices/pci_rom.c
> > > >
> > > > --- src/devices/pci_rom.c    (revision 3698)
> > > > +++ src/devices/pci_rom.c    (working copy)
> > > > @@ -67,8 +67,10 @@
> > > >      printk_spew("PCI ROM Image, Vendor %04x, Device %04x,\n",
> > > >              rom_data->vendor, rom_data->device);
> > > >      if (dev->vendor != rom_data->vendor || dev->device !=
> > >
> > > rom_data->device)
> > > > {
> > > >    printk_err("Device or Vendor ID mismatch Vendor %04x, Device
> > > >
> > > > %04x\n",
> > > >           rom_data->vendor, rom_data->device);
> > > >    printk_err("Device or Vendor ID mismatch Vendor %04x, Device
> > >
> > > %04x "
> > > >           "(Vendor %04x, Device %04x)\n",
> > > >           rom_data->vendor, rom_data->device,
> > > >           dev->vendor, dev->device);
> > > >    return NULL;
> > > > }
> > >
> > > The patch was mangled by gmail. AFAIK that's an unfixable bug with the
> > > gmail web interface. Please attach the patch instead.
> >
> > will do
>
> Thanks.
> > > About the contents of the patch: The new message still requires people
> > > to read the source code. How about:
> > > "Device or Vendor ID mismatch: read Vendor %04x, Device %04x, expected
> > > Vendor %04x, Device %04x\n"
> >
> > such a long line breaks the 80 column boundary. is that ok?
>
> Of course you can break the line. My point was more about the message
> itself than cosmetic linebreaks.
>
> Regards.
> Carl-Daniel