Messages in this thread | | | Date | Mon, 2 Jun 2008 19:35:27 -0700 | From | Arjan van de Ven <> | Subject | Re: [PATCH] x86: Add PCI extended config space access for AMD Barcelona |
| |
On Mon, 2 Jun 2008 16:19:29 +0200 Robert Richter <robert.richter@amd.com> wrote:
> Arjan, > > On 28.05.08 12:02:53, Arjan van de Ven wrote: > > Comment 1: > > Can we make the 256/4096 thing conditional on actually having the > > feature somehow? (while not making the code TOO ugly) > > In the first version I had 2 functions also. The patch have had lots > of duplicate code or inline functions. Since the conditional check is > already in raw_pci_* I decided to not implement an additional check > and use only one function.
ok fair enough.
> > > Comment 2: > > The cpu_has_XXX is a bit dubious; while it's dependent on your cpu > > model right now, I'm a bit hesitant to consider a PCI feature > > something that belongs in the cpu_has_XXX namespace. (Yes I know > > PCI is moving into the cpu package, but on a logical level it seems > > just the wrong place). > > Do we need a platform_has_XXX namespace for things like this? > > An alternative implementation would be here to use a check something > like pci_probe & PCI_HAS_EXT_CFG. If needed, I will send an updated > patch.
I kind of prefer this, since logically this is a PCI not a CPU property. Would you mind doing this ? (not that your current patch is wrong, it's just nicer to keep CPU stuff with the CPU and PCI stuff with PCI :) (This wouldn't need to stop inclusion of your current patch, it can just be an incremental cleanup)
-- If you want to reach me at my work email, use arjan@linux.intel.com For development, discussion and tips for power savings, visit http://www.lesswatts.org
| |