All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Mike Werner <werner@sgi.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [RFC][AGPGART]Allow multiple backends to be initialized
Date: Tue, 7 Dec 2004 09:02:44 +0000	[thread overview]
Message-ID: <20041207090244.GA13591@infradead.org> (raw)
In-Reply-To: <200412061740.52337.werner@sgi.com>

> -struct agp_bridge_data agp_bridge_dummy = { .type = NOT_SUPPORTED };
> -struct agp_bridge_data *agp_bridge = &agp_bridge_dummy;
> +agp_bridge_data_p agp_bridge;

Please don't intoroduce new new typedefs, especially not for pointer
types.

> +LIST_HEAD(agp_bridges);
>  EXPORT_SYMBOL(agp_bridge);
> -
> +EXPORT_SYMBOL(agp_bridges);

> -int agp_backend_acquire(void)
> +int agp_backend_acquire(struct pci_dev *pdev, agp_bridge_data_p 
> *acquired_bridge)
>  {
> -	if (agp_bridge->type == NOT_SUPPORTED)
> -		return -EINVAL;
> -	if (atomic_read(&agp_bridge->agp_in_use))
> +	agp_bridge_data_p bridge;
> +
> +	if (!pdev) {
> +		if (!agp_bridge)
> +			return -ENODEV;
> +		bridge = agp_bridge;
> +	} else {
> +        	list_for_each_entry(bridge, &agp_bridges, list) {
> +			int match=0;
> +			switch(pdev->class) {
> +				/* Standard bridges have a valid pci_dev */
> +				case PCI_CLASS_BRIDGE_HOST:
> +					if (bridge->dev==pdev)
> +						match=1;
> +					break;
> +				/* Non-standard bridges can use a devices pci_dev */
> +				default:
> +                			if (bridge->dev->bus==pdev->bus)
> +						match=1;
> +					break;
> +			}
> +                        if (match)
> +				break;
> +        	}
> +	}

Wrong interface.  Please pass in the pci_dev of the grpahics cards and
add a new method for lowlevel drivers to find the bridge.  For the normal
bridges (aka everything but the hp, alpha and your new driver) you'd do
a generic helper that just walks down the parent pointers until it finds
a class.

Also I'd suggest returning the found bridges as return value of the
function.

> -	printk(KERN_INFO PFX "AGP aperture is %dM @ 0x%lx\n",
> -	       size_value, bridge->gart_bus_addr);
> +	printk(KERN_INFO PFX "agp_bridge = 0x%lx: AGP aperture is %dM @ 0x%lx\n",
> +	       (unsigned long)bridge, size_value, bridge->gart_bus_addr);

pointers should be printed using %p, but this isn't a place where you should
print it at all.


  reply	other threads:[~2004-12-07  9:03 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-12-07  1:40 [RFC][AGPGART]Allow multiple backends to be initialized Mike Werner
2004-12-07  9:02 ` Christoph Hellwig [this message]
2004-12-07 18:17   ` Mike Werner
2004-12-08 17:59   ` Mike Werner
2004-12-09 20:10   ` Mike Werner
2004-12-10  5:02   ` Mike Werner
     [not found] <200411131826.31770.werner@sgi.com>
2004-11-14  9:44 ` [RFC][AGPGART] Allow " Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20041207090244.GA13591@infradead.org \
    --to=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=werner@sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.