On Wed, Jan 28, 2009 at 07:03:58PM +1100, Timothy S. Nelson wrote:
>
> This is my first kernel patch. Hopefully it's fine. Many thanks to those
> who helped with it (including answering my stupid questions). I'm hoping
> that Jesse, as maintainer, will pick it up from this list.
>
> diff -uprN linux-2.6.27/drivers/pci/pci-sysfs.c
> linux-2.6.27.rom/drivers/pci/pci-sysfs.c
> --- linux-2.6.27/drivers/pci/pci-sysfs.c 2008-10-10 09:13:53.000000000
> +1100
> +++ linux-2.6.27.rom/drivers/pci/pci-sysfs.c 2009-01-28 10:50:14.000000000
> +1100
Your patch is line-wrapped :(
> @@ -663,11 +663,20 @@ pci_write_rom(struct kobject *kobj, stru
> char *buf, loff_t off, size_t count)
> {
> struct pci_dev *pdev = to_pci_dev(container_of(kobj, struct device,
> kobj));
> + u16 command;
>
> if ((off == 0) && (*buf == '0') && (count == 2))
> pdev->rom_attr_enabled = 0;
> - else
> + else {
> pdev->rom_attr_enabled = 1;
> + pci_read_config_word(pdev, PCI_COMMAND, &command);
> + if (!(command & PCI_COMMAND_MEMORY)) {
> + printk(KERN_INFO "Suggest echoing 1 into "
> + "/sys/bus/pci/<deviceID>/enable to enable "
> + "device\n");
> + printk(KERN_INFO "before trying to do rom reading\n");
Please use dev_info() instead of "raw" printk() calls. It shows the
exact device that you are referring to. Also, don't split messages
across different printk calls, it could easily get very confusing if
other portions of the kernel are also printing messages.
And, putting "informational" messages in the kernel isn't usually
incouraged, that's what documentation is for :)
> + if (readb(image) != 0x55) {
> + printk(KERN_INFO " %s: Invalid ROM contents\n",
> + __func__);
> break;
s/printk/dev_err/ No need for the __func__ either, userspace doesn't
care about that.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
