Hi,
struct device->platform_data is designed for ACPI, BIOS or other
platform specific data, but some drivers misused the field which makes
adding ACPI handle in device core impossible. Greg suggested me changing
the name of the filed and so it breaks all such drivers, and then fix
them. I'll try to fix some, but it would be great if the driver authors
could do it.
Thanks,
Shaohua
---
2.5-root/include/linux/device.h | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
diff -puN include/linux/device.h~platform_data include/linux/device.h
--- 2.5/include/linux/device.h~platform_data 2005-01-12 10:41:35.446722944 +0800
+++ 2.5-root/include/linux/device.h 2005-01-12 10:42:37.762249544 +0800
@@ -265,7 +265,7 @@ struct device {
struct device_driver *driver; /* which driver has allocated this
device */
void *driver_data; /* data private to the driver */
- void *platform_data; /* Platform specific data (e.g. ACPI,
+ void *firmware_data; /* Platform specific data (e.g. ACPI,
BIOS data relevant to device) */
struct dev_pm_info power;
_
On Jan 12 2005, at 10:57, Li Shaohua was caught saying:
> Hi,
> struct device->platform_data is designed for ACPI, BIOS or other
> platform specific data, but some drivers misused the field which makes
> adding ACPI handle in device core impossible. Greg suggested me changing
> the name of the filed and so it breaks all such drivers, and then fix
> them. I'll try to fix some, but it would be great if the driver authors
> could do it.
Whatever the original purpose of this field was, it is now in use by
several drivers for ARM systems where we need to support multiple SOCs
with slight variations and we use the platform_data field to pass
board-specific information to the drivers. Before we had this, we were
putting ugly #ifdefs in drivers. Examples of drivers using this
include:
- serial/8250.c.
platform_data points to a plat_serial8250_port structure that
provides per-serial port information such as capabilities and
uart clk rate.
- mtd/maps/various ARM drivers
platform_data points to a flash_platform_data structure containing
information such as bus width and function pointers for some init
and shutdown work.
- i2c/busses/i2c-ixp4xx.c and i2c-ixp2000.c
The IXP4xx and IXP2000 chips do not have an I2C controller and
use GPIO pins to create a bit-bang "adapter". What I2C lines to
use is up to the board designer so we pass that information via
a custom data structure shared between the driver and the board
level code that contains the GPIO lines to use for SCL and SDA.
i2c-s3c2410.c also seems to use platform_data to pass board-specific
information to the driver.
So the question I have is whether we are using the field as intended
or we have overloaded it with our own purposes?
If we are doing things incorrectly, I am not argueing that our usage
has to the way it sits. We could create a new generic serial_device and
flash_device structures and subsystems for these, but that requires
rewriting drivers and board ports; however, we need enough time
to work with appropriate subsystem maintainers to do so. My suggestion
is to add a new firmware_data field for use by ACPI ATM while we
clean things up in ARM world if so required. Since ACPI is non-existent
on ARM systems, another option is that we keep using the renamed data
structure as we have been doing. /me votes for this option
Thoughts?
~Deepak
--
Deepak Saxena - [email protected] - http://www.plexity.net
If you complain once more, you'll meet an army of me. - Bjork
On Wed, Jan 12, 2005 at 10:57:06AM +0800, Li Shaohua wrote:
> Hi,
> struct device->platform_data is designed for ACPI, BIOS or other
> platform specific data, but some drivers misused the field which makes
> adding ACPI handle in device core impossible. Greg suggested me changing
> the name of the filed and so it breaks all such drivers, and then fix
> them. I'll try to fix some, but it would be great if the driver authors
> could do it.
No, the kernel has the "you break it, you fix it" rule. And as you want
to use platform_data for something other than the drivers that are
currently using it for, you need to fix everyone else up before I can
accept such a change.
And yes, one could argue that those drivers are "doing the wrong thing",
but hey, they did it first, as no one else was using this field, and it
solved a need for them. So you could successfully argue that they are
the correct ones here :)
thanks,
greg k-h
On Tue, Jan 11, 2005 at 07:54:46PM -0800, Deepak Saxena wrote:
>
> So the question I have is whether we are using the field as intended
> or we have overloaded it with our own purposes?
You have used it for your own purposes :)
But I would argue that you used it in a way that works, and solves a
real need. As the intent of this field was never really properly
conveyed, and since you were here first, hey, your implementation gets
to stay!
> If we are doing things incorrectly, I am not argueing that our usage
> has to the way it sits. We could create a new generic serial_device and
> flash_device structures and subsystems for these, but that requires
> rewriting drivers and board ports; however, we need enough time
> to work with appropriate subsystem maintainers to do so. My suggestion
> is to add a new firmware_data field for use by ACPI ATM while we
> clean things up in ARM world if so required. Since ACPI is non-existent
> on ARM systems, another option is that we keep using the renamed data
> structure as we have been doing. /me votes for this option
I like the "just add a firmware_data" field option too. It doesn't
break any existing code, and the term "firmware" tells driver authors to
back away from it and not touch it (and we need to add the proper
documentation saying this.)
thanks,
greg k-h
On Wed, 2005-01-12 at 13:06, Greg KH wrote:
>
> > If we are doing things incorrectly, I am not argueing that our usage
> > has to the way it sits. We could create a new generic serial_device and
> > flash_device structures and subsystems for these, but that requires
> > rewriting drivers and board ports; however, we need enough time
> > to work with appropriate subsystem maintainers to do so. My suggestion
> > is to add a new firmware_data field for use by ACPI ATM while we
> > clean things up in ARM world if so required. Since ACPI is non-existent
> > on ARM systems, another option is that we keep using the renamed data
> > structure as we have been doing. /me votes for this option
>
> I like the "just add a firmware_data" field option too. It doesn't
> break any existing code, and the term "firmware" tells driver authors to
> back away from it and not touch it (and we need to add the proper
> documentation saying this.)
If nobody insists on the intent of platform_data, I'll be glad to add a
new field. It makes things more easy.
Thanks,
Shaohua
Embedded PPC has followed in ARMs footsteps with the use of
platform_data for board specific information to be passed to drivers.
The plan is to grow its use in embedded PPC. It would seem introducing
a new field for ACPI information would be the least painful solution.
Can some clarify what kind of information ACPI needs. I'm asking
because firmware_data does not seem any more clear to me.
Also, we should really probably be a bit more specific in
Documentation/driver-model/device.txt once we decide the meaning of the
fields is, possible giving an example.
- kumar
On Jan 11, 2005, at 11:15 PM, Li Shaohua wrote:
> On Wed, 2005-01-12 at 13:06, Greg KH wrote:
> >
> > > If we are doing things incorrectly, I am not argueing that our
> usage
> > > has to the way it sits. We could create a new generic
> serial_device and
> > > flash_device structures and subsystems for these, but that requires
> > > rewriting drivers and board ports; however, we need enough time
> > > to work with appropriate subsystem maintainers to do so. My
> suggestion
> > > is to add a new firmware_data field for use by ACPI ATM while we
> > > clean things up in ARM world if so required.? Since ACPI is
> non-existent
> > > on ARM systems, another option is that we keep using the renamed
> data
> > > structure as we have been doing. /me votes for this option
> >
> > I like the "just add a firmware_data" field option too.? It doesn't
> > break any existing code, and the term "firmware" tells driver
> authors to
> > back away from it and not touch it (and we need to add the proper
> > documentation saying this.)
> If nobody insists on the intent of platform_data, I'll be glad to add
> a
> new field. It makes things more easy.
>
> Thanks,
> Shaohua
>
> -
> To unsubscribe from this list: send the line "unsubscribe
> linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at? http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at? http://www.tux.org/lkml/
On Wed, 2005-01-12 at 14:07, Kumar Gala wrote:
> Embedded PPC has followed in ARMs footsteps with the use of
> platform_data for board specific information to be passed to drivers.
> The plan is to grow its use in embedded PPC. It would seem introducing
> a new field for ACPI information would be the least painful solution.
I'll be happy to the solution too.
> Can some clarify what kind of information ACPI needs. I'm asking
> because firmware_data does not seem any more clear to me.
Most devices have an ACPI counterpart in ACPI based system. We plan to
make 'platform_data' point to an ACPI device, which will enables us to
utilize some ACPI features.
Thanks,
Shaohua
>
> Also, we should really probably be a bit more specific in
> Documentation/driver-model/device.txt once we decide the meaning of the
>
> > On Wed, 2005-01-12 at 13:06, Greg KH wrote:
> > >
> > > > If we are doing things incorrectly, I am not argueing that our
> > usage
> > > > has to the way it sits. We could create a new generic
> > serial_device and
> > > > flash_device structures and subsystems for these, but that requires
> > > > rewriting drivers and board ports; however, we need enough time
> > > > to work with appropriate subsystem maintainers to do so. My
> > suggestion
> > > > is to add a new firmware_data field for use by ACPI ATM while we
> > > > clean things up in ARM world if so required. Since ACPI is
> > non-existent
> > > > on ARM systems, another option is that we keep using the renamed
> > data
> > > > structure as we have been doing. /me votes for this option
> > >
> > > I like the "just add a firmware_data" field option too. It doesn't
> > > break any existing code, and the term "firmware" tells driver
> > authors to
> > > back away from it and not touch it (and we need to add the proper
> > > documentation saying this.)
> > If nobody insists on the intent of platform_data, I'll be glad to add
> > a
> > new field. It makes things more easy.
> >
> > Thanks,
> > Shaohua