Hi,
This is the device core change required. Add .platform_bind method for
bus_type, so platform can do addition things when add a new device. A
case is ACPI, we want to utilize some ACPI methods for physical devices.
1. Why doesn't use 'platform_notify'?
Current device core has a 'platform_notify' mechanism, but it's not
sufficient for this. Only sepcific bus type know how to parse dev.bus_id
and know how to encode specific device's address into ACPI _ADR syntax.
2. Why adds new 'handle' in 'struct device'?
'Platform_data' is the best candidate, but a search shows some drivers
have used it. We can remove 'handle' after the drivers changes their
behavior.
Thanks,
Shaohua
---
2.6-root/drivers/base/bus.c | 2 ++
2.6-root/include/linux/device.h | 2 ++
2 files changed, 4 insertions(+)
diff -puN drivers/base/bus.c~devcore-platformbind drivers/base/bus.c
--- 2.6/drivers/base/bus.c~devcore-platformbind 2004-11-08
10:57:57.996568552 +0800
+++ 2.6-root/drivers/base/bus.c 2004-11-08 11:02:42.045386568 +0800
@@ -463,6 +463,8 @@ int bus_add_device(struct device * dev)
list_add_tail(&dev->bus_list, &dev->bus->devices.list);
device_attach(dev);
up_write(&dev->bus->subsys.rwsem);
+ if (bus->platform_bind)
+ bus->platform_bind(dev);
device_add_attrs(bus, dev);
sysfs_create_link(&bus->devices.kobj, &dev->kobj, dev->bus_id);
}
diff -puN include/linux/device.h~devcore-platformbind
include/linux/device.h
--- 2.6/include/linux/device.h~devcore-platformbind 2004-11-08
10:57:57.998568248 +0800
+++ 2.6-root/include/linux/device.h 2004-11-08 10:57:58.001567792 +0800
@@ -63,6 +63,7 @@ struct bus_type {
int num_envp, char *buffer, int buffer_size);
int (*suspend)(struct device * dev, u32 state);
int (*resume)(struct device * dev);
+ int (*platform_bind)(struct device *dev);
};
extern int bus_register(struct bus_type * bus);
@@ -290,6 +291,7 @@ struct device {
override */
void (*release)(struct device * dev);
+ void *handle;
};
static inline struct device *
_
On Mon, Nov 08, 2004 at 12:11:11PM +0800, Li Shaohua wrote:
> Hi,
> This is the device core change required. Add .platform_bind method for
> bus_type, so platform can do addition things when add a new device. A
> case is ACPI, we want to utilize some ACPI methods for physical devices.
> 1. Why doesn't use 'platform_notify'?
> Current device core has a 'platform_notify' mechanism, but it's not
> sufficient for this. Only sepcific bus type know how to parse dev.bus_id
> and know how to encode specific device's address into ACPI _ADR syntax.
I don't see why platform_notify is not sufficient. This is the exact
reason it was added to the code.
> 2. Why adds new 'handle' in 'struct device'?
> 'Platform_data' is the best candidate, but a search shows some drivers
> have used it. We can remove 'handle' after the drivers changes their
> behavior.
No, fix the drivers. Don't add something that you are going to remove
later.
thanks,
greg k-h
On Tue, 2004-11-09 at 06:58, Greg KH wrote:
> On Mon, Nov 08, 2004 at 12:11:11PM +0800, Li Shaohua wrote:
> > Hi,
> > This is the device core change required. Add .platform_bind method for
> > bus_type, so platform can do addition things when add a new device. A
> > case is ACPI, we want to utilize some ACPI methods for physical devices.
> > 1. Why doesn't use 'platform_notify'?
> > Current device core has a 'platform_notify' mechanism, but it's not
> > sufficient for this. Only sepcific bus type know how to parse dev.bus_id
> > and know how to encode specific device's address into ACPI _ADR syntax.
>
> I don't see why platform_notify is not sufficient. This is the exact
> reason it was added to the code.
As I said in the email, we need know the bus type to decode and encode
address. If you use platform_notify, you must do something like this:
switch (dev->bus)
{
case pci_bus_type:
bind PCI devices with ACPI devices
break;
case ide_bus_type:
bind IDE devices with ACPI devices
break;
....
}
But note this method requires all bus types are build-in. If a bus type
is in a loadable module (such as IDE bus), the method will failed. I
searched current tree, only ARM implemented 'platform_notify', but ARM
only cares PCI bus, ACPI cares about all bus types.
>
> > 2. Why adds new 'handle' in 'struct device'?
> > 'Platform_data' is the best candidate, but a search shows some drivers
> > have used it. We can remove 'handle' after the drivers changes their
> > behavior.
>
> No, fix the drivers. Don't add something that you are going to remove
> later.
Ok, I'll try to fix them.
Thanks,
Shaohua
On Tue, 2004-11-09 at 08:50, Li Shaohua wrote:
> On Tue, 2004-11-09 at 06:58, Greg KH wrote:
> > On Mon, Nov 08, 2004 at 12:11:11PM +0800, Li Shaohua wrote:
> > > Hi,
> > > This is the device core change required. Add .platform_bind method for
> > > bus_type, so platform can do addition things when add a new device. A
> > > case is ACPI, we want to utilize some ACPI methods for physical devices.
> > > 1. Why doesn't use 'platform_notify'?
> > > Current device core has a 'platform_notify' mechanism, but it's not
> > > sufficient for this. Only sepcific bus type know how to parse dev.bus_id
> > > and know how to encode specific device's address into ACPI _ADR syntax.
> >
> > I don't see why platform_notify is not sufficient. This is the exact
> > reason it was added to the code.
> As I said in the email, we need know the bus type to decode and encode
> address. If you use platform_notify, you must do something like this:
> switch (dev->bus)
> {
> case pci_bus_type:
> bind PCI devices with ACPI devices
> break;
> case ide_bus_type:
> bind IDE devices with ACPI devices
> break;
> ....
> }
> But note this method requires all bus types are build-in. If a bus type
> is in a loadable module (such as IDE bus), the method will failed. I
> searched current tree, only ARM implemented 'platform_notify', but ARM
> only cares PCI bus, ACPI cares about all bus types.
> >
Oops, it's my bad. we can identify the bus type from bus_type->name, but
it looks like a little ugly. Why the bus_type hasn't a flag to identify
which bus it is?
Anyway, thanks Greg. I will add as you said.
Thanks,
Shaohua
On Tue, Nov 09, 2004 at 11:35:41AM +0800, Li Shaohua wrote:
> On Tue, 2004-11-09 at 08:50, Li Shaohua wrote:
> > On Tue, 2004-11-09 at 06:58, Greg KH wrote:
> > > On Mon, Nov 08, 2004 at 12:11:11PM +0800, Li Shaohua wrote:
> > > > Hi,
> > > > This is the device core change required. Add .platform_bind method for
> > > > bus_type, so platform can do addition things when add a new device. A
> > > > case is ACPI, we want to utilize some ACPI methods for physical devices.
> > > > 1. Why doesn't use 'platform_notify'?
> > > > Current device core has a 'platform_notify' mechanism, but it's not
> > > > sufficient for this. Only sepcific bus type know how to parse dev.bus_id
> > > > and know how to encode specific device's address into ACPI _ADR syntax.
> > >
> > > I don't see why platform_notify is not sufficient. This is the exact
> > > reason it was added to the code.
> > As I said in the email, we need know the bus type to decode and encode
> > address. If you use platform_notify, you must do something like this:
> > switch (dev->bus)
> > {
> > case pci_bus_type:
> > bind PCI devices with ACPI devices
> > break;
> > case ide_bus_type:
> > bind IDE devices with ACPI devices
> > break;
> > ....
> > }
> > But note this method requires all bus types are build-in. If a bus type
> > is in a loadable module (such as IDE bus), the method will failed. I
> > searched current tree, only ARM implemented 'platform_notify', but ARM
> > only cares PCI bus, ACPI cares about all bus types.
> > >
> Oops, it's my bad. we can identify the bus type from bus_type->name, but
> it looks like a little ugly. Why the bus_type hasn't a flag to identify
> which bus it is?
Because if you have a struct bus * you _have_ to know what type it is.
> Anyway, thanks Greg. I will add as you said.
Hm, hopefully Pat will chime in about what would be best for this, as he
created the platform_notify interface.
thanks,
greg k-h
On Tue, 2004-11-09 at 12:58, Greg KH wrote:
> On Tue, Nov 09, 2004 at 11:35:41AM +0800, Li Shaohua wrote:
> > On Tue, 2004-11-09 at 08:50, Li Shaohua wrote:
> > > On Tue, 2004-11-09 at 06:58, Greg KH wrote:
> > > > On Mon, Nov 08, 2004 at 12:11:11PM +0800, Li Shaohua wrote:
> > > > > Hi,
> > > > > This is the device core change required. Add .platform_bind method for
> > > > > bus_type, so platform can do addition things when add a new device. A
> > > > > case is ACPI, we want to utilize some ACPI methods for physical devices.
> > > > > 1. Why doesn't use 'platform_notify'?
> > > > > Current device core has a 'platform_notify' mechanism, but it's not
> > > > > sufficient for this. Only sepcific bus type know how to parse dev.bus_id
> > > > > and know how to encode specific device's address into ACPI _ADR syntax.
> > > >
> > > > I don't see why platform_notify is not sufficient. This is the exact
> > > > reason it was added to the code.
> > > As I said in the email, we need know the bus type to decode and encode
> > > address. If you use platform_notify, you must do something like this:
> > > switch (dev->bus)
> > > {
> > > case pci_bus_type:
> > > bind PCI devices with ACPI devices
> > > break;
> > > case ide_bus_type:
> > > bind IDE devices with ACPI devices
> > > break;
> > > ....
> > > }
> > > But note this method requires all bus types are build-in. If a bus type
> > > is in a loadable module (such as IDE bus), the method will failed. I
> > > searched current tree, only ARM implemented 'platform_notify', but ARM
> > > only cares PCI bus, ACPI cares about all bus types.
> > > >
> > Oops, it's my bad. we can identify the bus type from bus_type->name, but
> > it looks like a little ugly. Why the bus_type hasn't a flag to identify
> > which bus it is?
>
> Because if you have a struct bus * you _have_ to know what type it is.
>
> > Anyway, thanks Greg. I will add as you said.
>
> Hm, hopefully Pat will chime in about what would be best for this, as he
> created the platform_notify interface.
Ok, an updated version. Use 'platform_notify', but add a 'type' in
'struct bus_type'. Using bus_type->name to identify the bus type is
pretty much ugly and slow.
Thanks,
Shaohua
On Tue, Nov 09, 2004 at 05:03:01PM +0800, Li Shaohua wrote:
> On Tue, 2004-11-09 at 12:58, Greg KH wrote:
> > On Tue, Nov 09, 2004 at 11:35:41AM +0800, Li Shaohua wrote:
> > > On Tue, 2004-11-09 at 08:50, Li Shaohua wrote:
> > > > On Tue, 2004-11-09 at 06:58, Greg KH wrote:
> > > > > On Mon, Nov 08, 2004 at 12:11:11PM +0800, Li Shaohua wrote:
> > > > > > Hi,
> > > > > > This is the device core change required. Add .platform_bind method for
> > > > > > bus_type, so platform can do addition things when add a new device. A
> > > > > > case is ACPI, we want to utilize some ACPI methods for physical devices.
> > > > > > 1. Why doesn't use 'platform_notify'?
> > > > > > Current device core has a 'platform_notify' mechanism, but it's not
> > > > > > sufficient for this. Only sepcific bus type know how to parse dev.bus_id
> > > > > > and know how to encode specific device's address into ACPI _ADR syntax.
> > > > >
> > > > > I don't see why platform_notify is not sufficient. This is the exact
> > > > > reason it was added to the code.
> > > > As I said in the email, we need know the bus type to decode and encode
> > > > address. If you use platform_notify, you must do something like this:
> > > > switch (dev->bus)
> > > > {
> > > > case pci_bus_type:
> > > > bind PCI devices with ACPI devices
> > > > break;
> > > > case ide_bus_type:
> > > > bind IDE devices with ACPI devices
> > > > break;
> > > > ....
> > > > }
> > > > But note this method requires all bus types are build-in. If a bus type
> > > > is in a loadable module (such as IDE bus), the method will failed. I
> > > > searched current tree, only ARM implemented 'platform_notify', but ARM
> > > > only cares PCI bus, ACPI cares about all bus types.
> > > > >
> > > Oops, it's my bad. we can identify the bus type from bus_type->name, but
> > > it looks like a little ugly. Why the bus_type hasn't a flag to identify
> > > which bus it is?
> >
> > Because if you have a struct bus * you _have_ to know what type it is.
> >
> > > Anyway, thanks Greg. I will add as you said.
> >
> > Hm, hopefully Pat will chime in about what would be best for this, as he
> > created the platform_notify interface.
> Ok, an updated version. Use 'platform_notify', but add a 'type' in
> 'struct bus_type'. Using bus_type->name to identify the bus type is
> pretty much ugly and slow.
No, no "type" for a bus, sorry.
Maybe your other patches weren't so bad... If we implement them, can we
drop the platform notify stuff?
thanks,
greg k-h
On Wed, 2004-11-10 at 09:24, Greg KH wrote:
> > >
> > > Hm, hopefully Pat will chime in about what would be best for this, as he
> > > created the platform_notify interface.
> > Ok, an updated version. Use 'platform_notify', but add a 'type' in
> > 'struct bus_type'. Using bus_type->name to identify the bus type is
> > pretty much ugly and slow.
>
> No, no "type" for a bus, sorry.
>
> Maybe your other patches weren't so bad... If we implement them, can we
> drop the platform notify stuff?
Currently only ARM use 'platform_notify', and we can easily convert it
to use per-bus 'platform_bind'. One concern of per-bus 'platform_bind'
is we will have many '#ifdef ..' if many platforms implement their
per-bus 'platform_bind'.
Thanks,
Shaohua
On Wed, Nov 10, 2004 at 09:45:37AM +0800, Li Shaohua wrote:
> On Wed, 2004-11-10 at 09:24, Greg KH wrote:
> > Maybe your other patches weren't so bad... If we implement them, can we
> > drop the platform notify stuff?
> Currently only ARM use 'platform_notify', and we can easily convert it
> to use per-bus 'platform_bind'. One concern of per-bus 'platform_bind'
> is we will have many '#ifdef ..' if many platforms implement their
> per-bus 'platform_bind'.
Except none of the merged ARM platforms use platform_notify, and I haven't
seen any suggestion in the ARM world of why it would be needed.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core
On Wed, 2004-11-10 at 12:28, Russell King wrote:
> On Wed, Nov 10, 2004 at 09:45:37AM +0800, Li Shaohua wrote:
> > On Wed, 2004-11-10 at 09:24, Greg KH wrote:
> > > Maybe your other patches weren't so bad... If we implement them, can we
> > > drop the platform notify stuff?
> > Currently only ARM use 'platform_notify', and we can easily convert it
> > to use per-bus 'platform_bind'. One concern of per-bus 'platform_bind'
> > is we will have many '#ifdef ..' if many platforms implement their
> > per-bus 'platform_bind'.
>
> Except none of the merged ARM platforms use platform_notify, and I haven't
> seen any suggestion in the ARM world of why it would be needed.
Ok, let me summarize it. we now have two options:
1. using 'platform_notify'
platform_notify only has one parameter 'struct device', we must know the
exact bus type of a device. We can identify the bus type from its name
(such as 'pci', 'ide'), but it's quite some ugly. Or we can add a 'type'
flag in the 'struct bus_type' to indicate the exact bus type which Greg
doesn't like it. One shortcoming is the method hasn't good flexibility,
we must add a new type whenever a new bus type is added.
2. using per-bus type 'platform_bind'
Every bus type defines a 'platform_bind', so we know the exact bus type
naturally in platform_bind. The method can't handle special devices,
such as PCI root bridge, which hasn't a bus type, so no 'platform_bind'
is invoked for them. we must use some tricky methods to work around.
Another concern is the chaos if many platforms define 'platform_bind'
for a bus type, which isn't a big problem currently.
Greg, it seems you tend to option 2, isn't it?
Thanks,
Shaohua
On Thu, Nov 11, 2004 at 03:03:33PM +0800, Li Shaohua wrote:
> On Wed, 2004-11-10 at 12:28, Russell King wrote:
> > On Wed, Nov 10, 2004 at 09:45:37AM +0800, Li Shaohua wrote:
> > > On Wed, 2004-11-10 at 09:24, Greg KH wrote:
> > > > Maybe your other patches weren't so bad... If we implement them, can we
> > > > drop the platform notify stuff?
> > > Currently only ARM use 'platform_notify', and we can easily convert it
> > > to use per-bus 'platform_bind'. One concern of per-bus 'platform_bind'
> > > is we will have many '#ifdef ..' if many platforms implement their
> > > per-bus 'platform_bind'.
> >
> > Except none of the merged ARM platforms use platform_notify, and I haven't
> > seen any suggestion in the ARM world of why it would be needed.
> Ok, let me summarize it. we now have two options:
> 1. using 'platform_notify'
> platform_notify only has one parameter 'struct device', we must know the
> exact bus type of a device. We can identify the bus type from its name
> (such as 'pci', 'ide'), but it's quite some ugly. Or we can add a 'type'
> flag in the 'struct bus_type' to indicate the exact bus type which Greg
> doesn't like it. One shortcoming is the method hasn't good flexibility,
> we must add a new type whenever a new bus type is added.
Is there something wrong with doing dev->bus == &pci_bus_type for
example?
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core
On Thu, 2004-11-11 at 16:44, Russell King wrote:
> On Thu, Nov 11, 2004 at 03:03:33PM +0800, Li Shaohua wrote:
> > On Wed, 2004-11-10 at 12:28, Russell King wrote:
> > > On Wed, Nov 10, 2004 at 09:45:37AM +0800, Li Shaohua wrote:
> > > > On Wed, 2004-11-10 at 09:24, Greg KH wrote:
> > > > > Maybe your other patches weren't so bad... If we implement them, can we
> > > > > drop the platform notify stuff?
> > > > Currently only ARM use 'platform_notify', and we can easily convert it
> > > > to use per-bus 'platform_bind'. One concern of per-bus 'platform_bind'
> > > > is we will have many '#ifdef ..' if many platforms implement their
> > > > per-bus 'platform_bind'.
> > >
> > > Except none of the merged ARM platforms use platform_notify, and I haven't
> > > seen any suggestion in the ARM world of why it would be needed.
> > Ok, let me summarize it. we now have two options:
> > 1. using 'platform_notify'
> > platform_notify only has one parameter 'struct device', we must know the
> > exact bus type of a device. We can identify the bus type from its name
> > (such as 'pci', 'ide'), but it's quite some ugly. Or we can add a 'type'
> > flag in the 'struct bus_type' to indicate the exact bus type which Greg
> > doesn't like it. One shortcoming is the method hasn't good flexibility,
> > we must add a new type whenever a new bus type is added.
>
> Is there something wrong with doing dev->bus == &pci_bus_type for
> example?
It can't work if the bus type is in a loadable module.
Thanks,
Shaohua
On Thu, Nov 11, 2004 at 03:03:33PM +0800, Li Shaohua wrote:
> On Wed, 2004-11-10 at 12:28, Russell King wrote:
> > On Wed, Nov 10, 2004 at 09:45:37AM +0800, Li Shaohua wrote:
> > > On Wed, 2004-11-10 at 09:24, Greg KH wrote:
> > > > Maybe your other patches weren't so bad... If we implement them, can we
> > > > drop the platform notify stuff?
> > > Currently only ARM use 'platform_notify', and we can easily convert it
> > > to use per-bus 'platform_bind'. One concern of per-bus 'platform_bind'
> > > is we will have many '#ifdef ..' if many platforms implement their
> > > per-bus 'platform_bind'.
> >
> > Except none of the merged ARM platforms use platform_notify, and I haven't
> > seen any suggestion in the ARM world of why it would be needed.
> Ok, let me summarize it. we now have two options:
> 1. using 'platform_notify'
> platform_notify only has one parameter 'struct device', we must know the
> exact bus type of a device. We can identify the bus type from its name
> (such as 'pci', 'ide'), but it's quite some ugly. Or we can add a 'type'
> flag in the 'struct bus_type' to indicate the exact bus type which Greg
> doesn't like it. One shortcoming is the method hasn't good flexibility,
> we must add a new type whenever a new bus type is added.
> 2. using per-bus type 'platform_bind'
> Every bus type defines a 'platform_bind', so we know the exact bus type
> naturally in platform_bind. The method can't handle special devices,
> such as PCI root bridge, which hasn't a bus type, so no 'platform_bind'
> is invoked for them. we must use some tricky methods to work around.
> Another concern is the chaos if many platforms define 'platform_bind'
> for a bus type, which isn't a big problem currently.
> Greg, it seems you tend to option 2, isn't it?
I don't tend toward option 2, I just don't see much of any workable
option right now :(
thanks,
greg k-h