Re: [PATCH] fix improper use of pci_module_init() in drivers/char/agp/amd64-agp.c

From: Andi Kleen (ak_at_suse.de)
Date: 09/30/04

  • Next message: Bartlomiej Zolnierkiewicz: "Re: PATCH: (Test) it8212 driver for 2.6.9rc3"
    Date:	Thu, 30 Sep 2004 22:06:30 +0200
    To: Greg KH <greg@kroah.com>
    
    

    On Thu, Sep 30, 2004 at 12:59:05PM -0700, Greg KH wrote:
    > On Thu, Sep 30, 2004 at 09:20:09PM +0200, Andi Kleen wrote:
    > > On Thu, Sep 30, 2004 at 11:42:48AM -0700, Greg KH wrote:
    > > > Hi,
    > > >
    > > > In going through the tree and auditing the usage of pci_module_init(), I
    > > > noticed that the amd64-agp driver was assuming that the return value of
    > > > this function could be greater than 0 (which is what could happen in 2.2
    > > > and 2.4 kernels.) As this is no longer true, I think the following
    > > > patch is correct.
    > > >
    > > > I can add this to my bk-pci tree if you wish, otherwise feel free to
    > > > send it upwards.
    > >
    > > There needs to be some replacement for it, you cannot just delete
    > > the code.
    >
    > But that code has not ever run, since early 2.5 days. Don't tell me
    > people are used to it :)

    No, but it's needed for new chipsets that are not in the PCI tables.
    People probably didn't complain because we're covering the current
    generation of K8 AGP bridges, but there should be new ones soon

    2.4 had similar code.

    >
    > > The idea is to run it as fallback when no devices are found.
    > >
    > > How about this patch?
    >
    > That does not work the way you are asking it to work. pci_module_init()
    > is just a replacement for pci_register_driver these days. It will
    > return either "0" if the driver is successfully registered, or a
    > negative value if something bad happened. It will not return the number
    > of devices that this driver bound to.
    >
    > So, if no devices are in the system, it will return 0, and again, the
    > code you are wanting to run, will not.

    Oh, yes I forgot that hotplug makes everything simple complicated.

    >
    > So, how about using the new pci_dev_present() call instead? That should
    > be what you want, right?

    % grep pci_dev_present include/linux/*
    %

    This patch will probably do and doesn't rely on any nonexisting
    calls.

    -Andi

    ----------------------------------------------------------------------

    Fix fallback code in K8 AGP driver.

    Problem pointed out by Greg KH

    Signed-off-by: Andi Kleen <ak@suse.de>

    diff -u linux-2.6.9rc3-work/drivers/char/agp/amd64-agp.c-o linux-2.6.9rc3-work/drivers/char/agp/amd64-agp.c
    --- linux-2.6.9rc3-work/drivers/char/agp/amd64-agp.c-o 2004-09-30 10:35:07.000000000 +0200
    +++ linux-2.6.9rc3-work/drivers/char/agp/amd64-agp.c 2004-09-30 22:04:56.000000000 +0200
    @@ -55,6 +55,8 @@
     static int gart_iterator;
     #define for_each_nb() for(gart_iterator=0;gart_iterator<nr_garts;gart_iterator++)
     
    +static int num_bridges;
    +
     static void flush_amd64_tlb(struct pci_dev *dev)
     {
             u32 tmp;
    @@ -514,6 +516,7 @@
             }
     
             pci_set_drvdata(pdev, bridge);
    + num_bridges++;
             return agp_add_bridge(bridge);
     }
     
    @@ -627,7 +630,8 @@
             int err = 0;
             if (agp_off)
                     return -EINVAL;
    - if (pci_module_init(&agp_amd64_pci_driver) > 0) {
    + pci_module_init(&agp_amd64_pci_driver);
    + if (num_bridges == 0) {
                     struct pci_dev *dev;
                     if (!agp_try_unsupported && !agp_try_unsupported_boot) {
                             printk(KERN_INFO PFX "No supported AGP bridge found.\n");

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/


  • Next message: Bartlomiej Zolnierkiewicz: "Re: PATCH: (Test) it8212 driver for 2.6.9rc3"

    Relevant Pages

    • Re: 2.6.6-mm5 oops mounting ext3 or reiserfs with -o barrier
      ... So here's a complete patch, that both fixes the code bug and adds some ... support for fool proof checking proper flush support in the ... static int write_cache ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: 2.6.10-mm3: swsusp: out of memory on resume (was: Re: Ho ho ho - Linux v2.6.10)
      ... Try Lukas's patch, ... +static int __init does_collide_order(unsigned long addr, ... To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ ...
      (Linux-Kernel)
    • [PATCH] rmap 35 mmap.c cleanups
      ... Before some real vma_merge work in mmap.c in the next patch, ... -static int browse_rb(struct rb_root *root) { ... +static int browse_rb ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: [PATCH] PPC64 iSeries virtual DVD-RAM
      ... This version adresses Jens comments on the ... The patch is against 2.6.8.1 but sould apply to just about anything. ... +static int viocd_packet(struct cdrom_device_info *cdi, ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • [2.6 patch] misc drivers/block/cciss* cleanups (fwd)
      ... The patch forwarded below still applies and compiles against 2.6.10-mm2. ... misc drivers/block/cciss* cleanups ... +static int __init init_cciss ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)