RE: RFC: bare pci configuration access functions ?

RE: RFC: bare pci configuration access functions ?

[Lee, Jung-Ik]

Hi Greg,

> > > > Will it be desirable to have bare global pci config access
> > > > functions as seen in i386/ia64 pci codes ? It's clean and needs
> > > > just what it takes - seg, bus, dev, func, where, value, 
> and size.
> > > 
> > > No, I do not think so.  I think the way 2.5 does this is 
> the correct
> > > way.  
> > 
> > From PCI's own context, it's perfectly right since this way 
> encapsulate
> > access method to the object(pci, pci-express, ...) ala 
> we're in that object
> > context.
> > But with the same object concept, mandating pci_bus struct 
> for any pci
> > config access seems cruel, because others could be affected 
> on changes in
> > pci objects as we are seeing between 2.4 and 2.5.
> 
> Almost no-one in the kernel should be directly accessing pci config
> space without having a pci_bus structure at the minimum.The 
> exceptions
> are the pci core, and the pci hotplug code.  Now, if we move the
> majority of the pci hotplug resource code into the pci core, then only
> one place would need it.  Then there would not be a need to have these
> types of functions exported at all.  ACPI is a arch specific way of
> setting up the pci space, so it too needs to be able to do 
> this a little
> bit (not a lot, like it currently does.)

Platform management, early console access, acpi, hotplug io-node w/ root,...
pci_bus based access is useless before pci driver is initialized.
All exceptions will be forced to use fake structs...
Sounds we need to be ready to live with all exceptions here too :)
Or just to make them all happy with that simple bare functions.

> 
> So yes, it is cruel to not have this ability, but it is cruel for a
> reason :)
> 
> > > We could just force every arch to export the same 
> functions that i386
> > > and ia64 does, that shouldn't be a big deal.  
> > 
> > Right. It becomes just a matter of unifying APIs if other 
> architecture have
> > own low level bare pci config access functions.
> 
> Ok, mind trying to make up a patch for 2.4 that does this to see how
> feasible it really is?

OK, if simple and pure pci config access is not possible in Linux land,
let pci driver fake itself, not everyone else :)
Just export the two APIs like pci_config_{read|write}(s,b,d,f,s,v),
or the ones in acpi driver. Hide the fake pci_bus manipulation in them. 
This way is way better than having everyone fake pci driver ;-)

Thanks,
J.I.

Re: RFC: bare pci configuration access functions ?

[Greg KH]

On Thu, Oct 31, 2002 at 06:39:26PM -0800, Lee, Jung-Ik wrote:
> 
> Platform management, early console access, acpi, hotplug io-node w/ root,...
> pci_bus based access is useless before pci driver is initialized.
> All exceptions will be forced to use fake structs...
> Sounds we need to be ready to live with all exceptions here too :)
> Or just to make them all happy with that simple bare functions.

Ok, let's make them happy with bare functions, _if_ we have to.  Places
that do not have to will be gleefully pointed out and mocked :)

> OK, if simple and pure pci config access is not possible in Linux land,
> let pci driver fake itself, not everyone else :)
> Just export the two APIs like pci_config_{read|write}(s,b,d,f,s,v),
> or the ones in acpi driver. Hide the fake pci_bus manipulation in them. 
> This way is way better than having everyone fake pci driver ;-)

I agree.  But can we do this for all archs?  I don't know, and look
forward to your patch proving this will work.  Without all arch support
of this, I can't justify only exporting the functions for i386 and ia64.

thanks,

greg k-h

RE: RFC: bare pci configuration access functions ?

[Lee, Jung-Ik]

> Ah, so exporting those types of functions is not practical?  Oh well...

It simply needs changes to every arch codes in architecture specific way, as
I explained in #2. This is practical, or not depending on how we view :)
Better not go on with this since there's lighter, better known solution.

> > There could be two ways to achieve bare pci config accesses for all
> > architectures.
> 
> <snip>
> 
> Wait, again I'm confused.  Let's go over the main points here:
> 
>  - for 2.5 everyone uses the pci_bus_read_config* and
>    pci_bus_write_config* functions and is happy.  Well ACPI 
> isn't happy,
>    but the code there currently works, so let's leave it at that.

See how each and every php drivers in 2.5 manage this differently only to
fake pci driver for non-existing pci_bus w/ own overhead. It's ugly.

> 
>  - for 2.4 we don't have the pci_bus* functions, so we need to do
>    something.  I originally wanted to look into exporting the
>    pci_config_* function pointers, but you said that doesn't look
>    possible based on the different arch specific implementation.

The simplest, sound way of exporting bare pci config access function is what
I proposed as #1. It doesn't need any change on arch pci codes.
More specifically, pci_bus_read_config_##size(pci_bus, ...) is simply
replaced with
pci_read_config_##size(pci_dev, ...).

>    
>  - Because of this, you just proposed a patch, yet your patch uses the
>    pci_bus_* functions which are not present on 2.4.  If they were,
>    everyone would be happy again, and not need such a patch, right?

I proposed pure PCI config access method required by some kernel components,
not just to get around the absence of pci_bus in 2.4, but also to address
the ugliness of having everyone fake pci driver w/ individual's overhead.
The #1 proposed is the solution for all architectures w/o any change in
existing arch pci codes, while the concept has already been proved.

Below is copied from previous email thread.
-------------------------------
>> OK, if simple and pure pci config access is not possible in Linux land,
>> let pci driver fake itself, not everyone else :)
>> Just export the two APIs like pci_config_{read|write}(s,b,d,f,s,v),
>> or the ones in acpi driver. Hide the fake pci_bus manipulation in them. 
>> This way is way better than having everyone fake pci driver ;-)

>I agree.  But can we do this for all archs?  I don't know, and look
>forward to your patch proving this will work.  Without all arch support
>of this, I can't justify only exporting the functions for i386 and ia64.
------------------------------

Thanks,
J.I.

RE: RFC: bare pci configuration access functions ?

[Lee, Jung-Ik]

[-- Attachment #1: Type: text/plain, Size: 4648 bytes --]

Hi Greg,

> What's wrong with the _existing_ pci_config_read() and
> pci_config_write() function pointers that ia64 and i386 have? 
>  Can't you
> just look into if the other archs can set them to the proper 
> function in
> their pci init functions too?

Other architectures' PCI config access methods vary and require their own
address mappings, etc.

There could be two ways to achieve bare pci config accesses for all
architectures.

#1. upper level functions of pci_ops functions.
	i.e., Just export APIs like pci_config_read_bare(s, b, d, f, l, s,
v) and have it call pci_ops's pci_bus_read_config_##size(). Since all
architecture supports pci_ops, this solution is architecture independent and
no changes are needed in existing arch pci codes.
Actually, this is how drivers/pci/syscall.c is implemented. Only diff is it
allows accesses to unpopulated pci config spaces w/ temporary pci_bus
structure inherited from root, as needed by acpi, php driver, etc.

#2. lower level functions of pci_ops functions/or separate functions from
pci_ops.
	This is how I386 and IA64's pci_config_{read|write} are implemented.
pci_ops functions call these. To achieve this, every architecture pci code
need changes and the changes are all architecture specific as they need addr
mapping, etc.

I prefer #1 since (1)it is small and does not require changes in arch pci
codes, (2)the concept has already been proved(temp pci_bus struct, in both
acpi and php driver), and (3)it is common to all architecture.

Patch below (also attached) is a reference implementation of #1, and
verified with cpqphp driver.
I put the code in drivers/hotplug/pci_hotplug_util.c for testing convenience
purpose.
Don't pay attention to function names, but the logic it is based on. I don't
like the names either :)

Any comment, ideas would be appreciated.
thanks,
J.I.

diff -urN hotplug/pci_hotplug_util.c hotplug_all_1030/pci_hotplug_util.c
--- hotplug/pci_hotplug_util.c	Wed Oct 30 16:43:07 2002
+++ hotplug_all_1030/pci_hotplug_util.c	Fri Nov  1 18:56:34 2002
@@ -153,6 +153,124 @@
 	return result;
 }
 
+#define	BARE_PCI_ACCESS
+#ifdef	BARE_PCI_ACCESS
+static struct pci_bus *root_pci_bus;
+static struct pci_bus *get_temporary_pci_bus(int busnum)
+{
+	static int first = 1;
+	struct pci_bus *pbus;
+	struct list_head *lh;
+
+	if (first) {
+		list_for_each (lh, &pci_root_buses) {
+			pbus = (struct pci_bus *) pci_bus_b(lh);
+			if (pbus)
+				if (pbus->number == 0) {
+					root_pci_bus = pbus;
+					first = 0;
+				}
+		}
+	}
+	if (!root_pci_bus)
+		return NULL;
+
+	pbus = kmalloc(sizeof(struct pci_bus), GFP_KERNEL);
+	if (pbus) {
+		memset(pbus, 0, sizeof(struct pci_bus));
+		pbus->number = busnum;
+		pbus->ops = root_pci_bus->ops;
+	}
+
+	return pbus;
+}
+
+static int get_pci_bus (
+	int segnum, int busnum, int devnum, int funcnum, struct pci_bus
**pbus)
+{
+	struct pci_dev *pdev = pci_find_slot(busnum, PCI_DEVFN(devnum,
funcnum));
+	int is_temp = 0;
+
+	if (pdev && pdev->bus)
+		*pbus = pdev->bus;
+	else {
+		*pbus = get_temporary_pci_bus(busnum);
+		is_temp++;
+	}
+	return is_temp;
+}
+
+int pci_config_bare_read (int segnum, int busnum, int devnum, int funcnum,
+	int loc, int size, u32 *value)
+{
+	struct pci_bus *pbus;
+	int retval, is_temp = get_pci_bus(segnum, busnum, devnum, funcnum,
&pbus);
+
+	if (!pbus)
+		return -ENODEV;
+
+	*value = 0x00;
+
+	switch (size) {
+	case 1:
+		retval = pci_bus_read_config_byte
+				(pbus, PCI_DEVFN(devnum, funcnum), loc, (u8
*)value);
+		break;
+	case 2:
+		retval = pci_bus_read_config_word
+				(pbus, PCI_DEVFN(devnum, funcnum), loc, (u16
*)value);
+		break;
+	case 4:
+		retval = pci_bus_read_config_dword
+				(pbus, PCI_DEVFN(devnum, funcnum), loc,
value);
+		break;
+	default:
+		retval = -ENODEV;
+		break;
+	}
+
+	if (is_temp)
+		kfree(pbus);
+
+	return retval;
+}
+
+int pci_config_bare_write (int segnum, int busnum, int devnum, int funcnum,
+	int loc, int size, u32 value)
+{
+	struct pci_bus *pbus;
+	int retval, is_temp = get_pci_bus(segnum, busnum, devnum, funcnum,
&pbus);
+
+	if (!pbus)
+		return -ENODEV;
+
+	switch (size) {
+	case 1:
+		retval = pci_bus_write_config_byte
+				(pbus, PCI_DEVFN(devnum, funcnum), loc,
(u8)value);
+		break;
+	case 2:
+		retval = pci_bus_write_config_word
+				(pbus, PCI_DEVFN(devnum, funcnum), loc,
(u16)value);
+		break;
+	case 4:
+		retval = pci_bus_write_config_dword
+				(pbus, PCI_DEVFN(devnum, funcnum), loc,
value);
+		break;
+	default:
+		retval = -ENODEV;
+		break;
+	}
+
+	if (is_temp)
+		kfree(pbus);
+
+	return retval;
+}
+
+EXPORT_SYMBOL(pci_config_bare_read);
+EXPORT_SYMBOL(pci_config_bare_write);
+#endif
 
 EXPORT_SYMBOL(pci_visit_dev);
 


[-- Attachment #2: bare_pci.diff --]
[-- Type: application/octet-stream, Size: 2807 bytes --]

diff -urN hotplug/pci_hotplug_util.c hotplug_all_1030/pci_hotplug_util.c
--- hotplug/pci_hotplug_util.c	Wed Oct 30 16:43:07 2002
+++ hotplug_all_1030/pci_hotplug_util.c	Fri Nov  1 18:56:34 2002
@@ -153,6 +153,124 @@
 	return result;
 }
 
+#define	BARE_PCI_ACCESS
+#ifdef	BARE_PCI_ACCESS
+static struct pci_bus *root_pci_bus;
+static struct pci_bus *get_temporary_pci_bus(int busnum)
+{
+	static int first = 1;
+	struct pci_bus *pbus;
+	struct list_head *lh;
+
+	if (first) {
+		list_for_each (lh, &pci_root_buses) {
+			pbus = (struct pci_bus *) pci_bus_b(lh);
+			if (pbus)
+				if (pbus->number == 0) {
+					root_pci_bus = pbus;
+					first = 0;
+				}
+		}
+	}
+	if (!root_pci_bus)
+		return NULL;
+
+	pbus = kmalloc(sizeof(struct pci_bus), GFP_KERNEL);
+	if (pbus) {
+		memset(pbus, 0, sizeof(struct pci_bus));
+		pbus->number = busnum;
+		pbus->ops = root_pci_bus->ops;
+	}
+
+	return pbus;
+}
+
+static int get_pci_bus (
+	int segnum, int busnum, int devnum, int funcnum, struct pci_bus **pbus)
+{
+	struct pci_dev *pdev = pci_find_slot(busnum, PCI_DEVFN(devnum, funcnum));
+	int is_temp = 0;
+
+	if (pdev && pdev->bus)
+		*pbus = pdev->bus;
+	else {
+		*pbus = get_temporary_pci_bus(busnum);
+		is_temp++;
+	}
+	return is_temp;
+}
+
+int pci_config_bare_read (int segnum, int busnum, int devnum, int funcnum,
+	int loc, int size, u32 *value)
+{
+	struct pci_bus *pbus;
+	int retval, is_temp = get_pci_bus(segnum, busnum, devnum, funcnum, &pbus);
+
+	if (!pbus)
+		return -ENODEV;
+
+	*value = 0x00;
+
+	switch (size) {
+	case 1:
+		retval = pci_bus_read_config_byte
+				(pbus, PCI_DEVFN(devnum, funcnum), loc, (u8 *)value);
+		break;
+	case 2:
+		retval = pci_bus_read_config_word
+				(pbus, PCI_DEVFN(devnum, funcnum), loc, (u16 *)value);
+		break;
+	case 4:
+		retval = pci_bus_read_config_dword
+				(pbus, PCI_DEVFN(devnum, funcnum), loc, value);
+		break;
+	default:
+		retval = -ENODEV;
+		break;
+	}
+
+	if (is_temp)
+		kfree(pbus);
+
+	return retval;
+}
+
+int pci_config_bare_write (int segnum, int busnum, int devnum, int funcnum,
+	int loc, int size, u32 value)
+{
+	struct pci_bus *pbus;
+	int retval, is_temp = get_pci_bus(segnum, busnum, devnum, funcnum, &pbus);
+
+	if (!pbus)
+		return -ENODEV;
+
+	switch (size) {
+	case 1:
+		retval = pci_bus_write_config_byte
+				(pbus, PCI_DEVFN(devnum, funcnum), loc, (u8)value);
+		break;
+	case 2:
+		retval = pci_bus_write_config_word
+				(pbus, PCI_DEVFN(devnum, funcnum), loc, (u16)value);
+		break;
+	case 4:
+		retval = pci_bus_write_config_dword
+				(pbus, PCI_DEVFN(devnum, funcnum), loc, value);
+		break;
+	default:
+		retval = -ENODEV;
+		break;
+	}
+
+	if (is_temp)
+		kfree(pbus);
+
+	return retval;
+}
+
+EXPORT_SYMBOL(pci_config_bare_read);
+EXPORT_SYMBOL(pci_config_bare_write);
+#endif
 
 EXPORT_SYMBOL(pci_visit_dev);
 

Re: RFC: bare pci configuration access functions ?

[Greg KH]

On Mon, Nov 04, 2002 at 12:17:45PM -0800, Lee, Jung-Ik wrote:
> Hi Greg,
> 
> > What's wrong with the _existing_ pci_config_read() and
> > pci_config_write() function pointers that ia64 and i386 have? 
> >  Can't you
> > just look into if the other archs can set them to the proper 
> > function in
> > their pci init functions too?
> 
> Other architectures' PCI config access methods vary and require their own
> address mappings, etc.

Ah, so exporting those types of functions is not pratical?  Oh well...

> There could be two ways to achieve bare pci config accesses for all
> architectures.

<snip>

Wait, again I'm confused.  Let's go over the main points here:

 - for 2.5 everyone uses the pci_bus_read_config* and
   pci_bus_write_config* functions and is happy.  Well ACPI isn't happy,
   but the code there currently works, so let's leave it at that.

 - for 2.4 we don't have the pci_bus* functions, so we need to do
   something.  I originally wanted to look into exporting the
   pci_config_* function pointers, but you said that doesn't look
   possible based on the different arch specific implementation.
   
 - Because of this, you just proposed a patch, yet your patch uses the
   pci_bus_* functions which are not present on 2.4.  If they were,
   everyone would be happy again, and not need such a patch, right?

Confused,

greg k-h

RE: RFC: bare pci configuration access functions ?

[Lee, Jung-Ik]

Minor fix to the code.
A patch to a flying patch ;-)

Thanks,
J.I.
=====================================================
static struct pci_bus *get_pci_bus(s, b, d, f)
{
	struct pci_bus *bus;
	struct pci_dev *dev = pci_find_slot(s, b, d+f);

	if (dev && dev->bus)
		bus = dev->bus;
	else	// dup pci_bus w/ root & set bus->number=b
		bus = get_fake_pci_bus(b);

	return bus;
}

int pci_config_{read|write}(
#ifdef WANT_PCI_BUS_PARAM
		pci_bus, 
#endif
			s, b, d, f, w, size, v)
{
	struct pci_bus *bus;
#ifdef WANT_PCI_BUS_PARAM
	if (valid(pci_bus))
		bus = pci_bus;
	else
#endif
	bus = get_pci_bus(s, b, d, f);
	if (!bus)
		return error;

	switch (size) {
	case byte:
		ret = pci_bus_{read|write}_##size (bus, d+f, w, v);
		break;
	...
	}
	return ret;
}

EXPORT_SYMBOL(pci_config_{read|write});
> > 
> > > OK, if simple and pure pci config access is not possible in 
> > Linux land,
> > > let pci driver fake itself, not everyone else :)
> > > Just export the two APIs like 
> pci_config_{read|write}(s,b,d,f,s,v),
> > > or the ones in acpi driver. Hide the fake pci_bus 
> > manipulation in them. 
> > > This way is way better than having everyone fake pci driver ;-)
> > 
> > I agree.  But can we do this for all archs?  I don't know, and look
> > forward to your patch proving this will work.  Without all 
> > arch support
> > of this, I can't justify only exporting the functions for 
> > i386 and ia64.
> 
> How about the followings ?
> It's for all architecture.
> 
> thanks,
> J.I.
> 
> static struct pci_bus *get_pci_bus(s, b, d, f)
> {
> 	struct pci_bus *bus;
> 	struct pci_dev *dev = pci_find_slot(s, b, d+f);
> 
> 	if (dev && dev->bus)
> 		bus = dev->bus;
> 	else	// dup pci_bus w/ root & set bus->number=b
> 		bus = get_fake_pci_bus(b);
> 
> 	return bus;
> }
> 
> int pci_config_{read|write}(
> #ifdef WANT_PCI_BUS_PARAM
> 		pci_bus, 
> #endif
> 			s, b, d, f, w, size, v)
> {
> 	struct pci_bus *bus;
> #ifdef WANT_PCI_BUS_PARAM
> 	if (!valid(pci_bus))	// null or invalid
> #endif
> 	bus = get_pci_bus(s, b, d, f);
> 	if (!bus)
> 		return error;
> 
> 	switch (size) {
> 	case byte:
> 		ret = pci_bus_{read|write}_##size (bus, d+f, w, v);
> 		break;
> 	...
> 	}
> 
> 	return ret;
> }
> 
> EXPORT_SYMBOL(pci_config_{read|write});
> 

Re: RFC: bare pci configuration access functions ?

[Greg KH]

On Thu, Oct 31, 2002 at 08:52:24PM -0800, Lee, Jung-Ik wrote:
> Minor fix to the code.
> A patch to a flying patch ;-)

Ok, maybe just because I've eaten too much candy tonight, but I do not
understand where you are trying to go with this odd pseudo code.

What's wrong with the _existing_ pci_config_read() and
pci_config_write() function pointers that ia64 and i386 have?  Can't you
just look into if the other archs can set them to the proper function in
their pci init functions too?

thanks,

greg k-h

RE: RFC: bare pci configuration access functions ?

[Lee, Jung-Ik]

> On Thu, Oct 31, 2002 at 06:39:26PM -0800, Lee, Jung-Ik wrote:
> > 
> > Platform management, early console access, acpi, hotplug 
> io-node w/ root,...
> > pci_bus based access is useless before pci driver is initialized.
> > All exceptions will be forced to use fake structs...
> > Sounds we need to be ready to live with all exceptions here too :)
> > Or just to make them all happy with that simple bare functions.
> 
> Ok, let's make them happy with bare functions, _if_ we have 
> to.  Places
> that do not have to will be gleefully pointed out and mocked :)
> 
> > OK, if simple and pure pci config access is not possible in 
> Linux land,
> > let pci driver fake itself, not everyone else :)
> > Just export the two APIs like pci_config_{read|write}(s,b,d,f,s,v),
> > or the ones in acpi driver. Hide the fake pci_bus 
> manipulation in them. 
> > This way is way better than having everyone fake pci driver ;-)
> 
> I agree.  But can we do this for all archs?  I don't know, and look
> forward to your patch proving this will work.  Without all 
> arch support
> of this, I can't justify only exporting the functions for 
> i386 and ia64.

How about the followings ?
It's for all architecture.

thanks,
J.I.

static struct pci_bus *get_pci_bus(s, b, d, f)
{
	struct pci_bus *bus;
	struct pci_dev *dev = pci_find_slot(s, b, d+f);

	if (dev && dev->bus)
		bus = dev->bus;
	else	// dup pci_bus w/ root & set bus->number=b
		bus = get_fake_pci_bus(b);

	return bus;
}

int pci_config_{read|write}(
#ifdef WANT_PCI_BUS_PARAM
		pci_bus, 
#endif
			s, b, d, f, w, size, v)
{
	struct pci_bus *bus;
#ifdef WANT_PCI_BUS_PARAM
	if (!valid(pci_bus))	// null or invalid
#endif
	bus = get_pci_bus(s, b, d, f);
	if (!bus)
		return error;

	switch (size) {
	case byte:
		ret = pci_bus_{read|write}_##size (bus, d+f, w, v);
		break;
	...
	}

	return ret;
}

EXPORT_SYMBOL(pci_config_{read|write});

RFC: bare pci configuration access functions ?

[Lee, Jung-Ik]

Need:
====
	Some kernel drivers/components such as hotplug pci/io-node drivers,
ACPI driver, some console drivers, etc **need bare pci configuration space
access** before either pci driver is initialized or struct pci_dev is
constructed.

ACPI needs this for ACPI/PCI population, hotplug pci driver for populating
hot-added pci hierarchy. As more drivers are cross ported over to wider
architectures, this would become wider need. Help me if others need this
too.


Current pci configuration access functions:
==========================================
	Current pci configuration access functions is based on "struct
pci_ops" from "struct pci_bus".
 pci_{read|write}_config_{byte|word|dword}(pci_dev, where, val);
 pci_bus_{read|write}_config_{byte|word|dword}(pci_bus, devfn, where, val);

Issue:
=====
	Current functions need pci_ops and pci_bus struct, which are not
constructed yet for the above cases.

Current solutions:
=================
(1) i386 and ia64 kernel provides global bare pci config access functions
like:
 pci_config_{read|write}(seg, bus, dev, func, where, size, val);
	Acpi driver uses these.

(2) Alternative is to allocate temporary pci_dev/pci_bus structs and copy
parent's or root's, and modify the struct.
	Hotplug pci driver uses this.


Question:
========
Will it be desirable to have bare global pci config access functions as seen
in i386/ia64 pci codes ? It's clean and needs just what it takes - seg, bus,
dev, func, where, value, and size.
Or, do we keep original functions with temporary structs ? It takes extra
care for temporary structs, but it's with pci context.

Request for comments.
thanks,
J.I.

Re: RFC: bare pci configuration access functions ?

[Greg KH]

Wait, first off, are we talking about 2.4, or 2.5 here?  For 2.5 I think
everything is covered, right?

On Thu, Oct 31, 2002 at 11:29:19AM -0800, Lee, Jung-Ik wrote:
> Question:
> ========
> Will it be desirable to have bare global pci config access functions as seen
> in i386/ia64 pci codes ? It's clean and needs just what it takes - seg, bus,
> dev, func, where, value, and size.

No, I do not think so.  I think the way 2.5 does this is the correct
way.  But as I did that patch, I might be a bit biased :)

We could just force every arch to export the same functions that i386
and ia64 does, that shouldn't be a big deal.  I think this would solve
the problem on 2.4 for pci hotplug, as ACPI is already "cheating" and
doing this right now...

thanks,

greg k-h

RE: RFC: bare pci configuration access functions ?

[Lee, Jung-Ik]

> Wait, first off, are we talking about 2.4, or 2.5 here?  

About both and beyond :)

> For 2.5 I think
> everything is covered, right?

Right.

> > Will it be desirable to have bare global pci config access 
> functions as seen
> > in i386/ia64 pci codes ? It's clean and needs just what it 
> takes - seg, bus,
> > dev, func, where, value, and size.
> 
> No, I do not think so.  I think the way 2.5 does this is the correct
> way.  

>From PCI's own context, it's perfectly right since this way encapsulate
access method to the object(pci, pci-express, ...) ala we're in that object
context.
But with the same object concept, mandating pci_bus struct for any pci
config access seems cruel, because others could be affected on changes in
pci objects as we are seeing between 2.4 and 2.5.

Tome, pci config access is simply about PCI/PCI-Express specs, and hence
access should be made with just what it takes - seg, bus, dev, ..., w/o any
implementation dependencies.

> But as I did that patch, I might be a bit biased :)
> 
> We could just force every arch to export the same functions that i386
> and ia64 does, that shouldn't be a big deal.  

Right. It becomes just a matter of unifying APIs if other architecture have
own low level bare pci config access functions.

> I think this would solve
> the problem on 2.4 for pci hotplug, as ACPI is already "cheating" and
> doing this right now...
> 
> thanks,
> 
> greg k-h
> 

Re: RFC: bare pci configuration access functions ?

[Greg KH]

On Thu, Oct 31, 2002 at 05:55:23PM -0800, Lee, Jung-Ik wrote:
> > Wait, first off, are we talking about 2.4, or 2.5 here?  
> 
> About both and beyond :)
> 
> > For 2.5 I think everything is covered, right?
> 
> Right.

Then "beyond" is already covered :)

> > > Will it be desirable to have bare global pci config access
> > > functions as seen in i386/ia64 pci codes ? It's clean and needs
> > > just what it takes - seg, bus, dev, func, where, value, and size.
> > 
> > No, I do not think so.  I think the way 2.5 does this is the correct
> > way.  
> 
> From PCI's own context, it's perfectly right since this way encapsulate
> access method to the object(pci, pci-express, ...) ala we're in that object
> context.
> But with the same object concept, mandating pci_bus struct for any pci
> config access seems cruel, because others could be affected on changes in
> pci objects as we are seeing between 2.4 and 2.5.

Almost no-one in the kernel should be directly accessing pci config
space without having a pci_bus structure at the minimum.  The exceptions
are the pci core, and the pci hotplug code.  Now, if we move the
majority of the pci hotplug resource code into the pci core, then only
one place would need it.  Then there would not be a need to have these
types of functions exported at all.  ACPI is a arch specific way of
setting up the pci space, so it too needs to be able to do this a little
bit (not a lot, like it currently does.)

So yes, it is cruel to not have this ability, but it is cruel for a
reason :)

> > We could just force every arch to export the same functions that i386
> > and ia64 does, that shouldn't be a big deal.  
> 
> Right. It becomes just a matter of unifying APIs if other architecture have
> own low level bare pci config access functions.

Ok, mind trying to make up a patch for 2.4 that does this to see how
feasible it really is?

thanks,

greg k-h