Messages in this thread |  | | Date | Thu, 09 Nov 2000 12:32:23 -0500 | From | Jeff Garzik <> | Subject | Re: OOPS loading cs46xx module, test11-pre1 |
| |
Christoph Hellwig wrote: > > On Thu, Nov 09, 2000 at 12:02:45PM -0500, Jeff Garzik wrote: > > > I have an (untested) update for the cs46xx driver in Linux 2.4. > > > It includes Nils' 2.2 changes, use of initcalls (so compiled-in > > > should work) and use of the 2.4 PCI interface. > > > > Patch Generally looks ok. Comments: > > > > 1) This code is weird: > > > if (copy_to_user(buffer, dmabuf->rawbuf + swptr, cnt)) { > > >- if (!ret) ret = -EFAULT; > > >- return ret; > > >+ if (!ret) > > >+ ret = -EFAULT; > > >+ break; > > > } > > > > If you have an error condition (ret != 0), then you should not attempt > > the copy_to_user at all. > > If you do not have an error condition, you should unconditionally assign > > -EFAULT to 'ret'. > > Makes sense. But I did not change the logic at all ... > Ask the Author (Alan or Nils) what this is about.
Now that I have looked at the code around it, it looks ok. It returns the bytes copied to userspace so far, like a good read function should.
> > 6) remove check for pci_present(), redundant > > Lot's of pci drivers do this. Anyway I will removed this in the next version.
It's been redundant since the 2.1.x days, maybe even before that. There -might- be a rare case where a driver really needs to check for the presence of a PCI BIOS. Maybe. But for all cases where you have
if (!pci_present()) probe_pci=0; /* ... */ if (probe_pci) { pci_register_driver(...) or pdev = pci_find_device(...); }
you can remove the pci_present check. All PCI probe functions return sane values in the absence of PCI support (or PCI devices).
> > of course for cs46xx, you will want to keep the version printk... > > > > 8) xxx_MODULE_NAME was a dumb and overly-lengthy creation of mine. Use > > instead MODNAME. > > It doesn't really matter ...
besides perpetuating a bad idea, that is.
Jeff
-- Jeff Garzik | Building 1024 | Would you like a Twinkie? MandrakeSoft | - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/
|  |