[PATCH]change 'struct device' -> platform_data to firmware_data

[PATCH]change 'struct device' -> platform_data to firmware_data

[Li Shaohua]

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;
 
_

Re: [PATCH]change 'struct device' -> platform_data to firmware_data

[Deepak Saxena]

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 - dsaxena@plexity.net - http://www.plexity.net

If you complain once more, you'll meet an army of me. - Bjork

Re: [PATCH]change 'struct device' -> platform_data to firmware_data

[Greg KH]

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

Re: [PATCH]change 'struct device' -> platform_data to firmware_data

[Li Shaohua]

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

Re: [PATCH]change 'struct device' -> platform_data to firmware_data

[Kumar Gala]

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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>  Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH]change 'struct device' -> platform_data to firmware_data

[Li Shaohua]

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

Re: [PATCH]change 'struct device' -> platform_data to firmware_data

[Greg KH]

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