Add a private_data pointer to struct device_attribute

Add a private_data pointer to struct device_attribute

[Timur Tabi]

A private data pointer in struct device_attribute allows the 'show' and 'store'
functions to access instance data.  This handy in situations where the
driver_data and platform_data pointers of 'struct device' are already used
for other purposes.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

Greg, can you tell me if you think this patch is a good idea?  It doesn't
appear to do any harm, and I'm working on an ALSA driver that could benefit
for this patch.  I think 2.6.25 would be a good target.

 include/linux/device.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index 2e15822..10708ee 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -344,6 +344,7 @@ struct device_attribute {
 			char *buf);
 	ssize_t (*store)(struct device *dev, struct device_attribute *attr,
 			 const char *buf, size_t count);
+	void *private_data;
 };
 
 #define DEVICE_ATTR(_name,_mode,_show,_store) \
-- 
1.5.2.4

Re: Add a private_data pointer to struct device_attribute

[Greg KH]

On Fri, Nov 16, 2007 at 06:11:00PM -0600, Timur Tabi wrote:
> A private data pointer in struct device_attribute allows the 'show' and 'store'
> functions to access instance data.  This handy in situations where the
> driver_data and platform_data pointers of 'struct device' are already used
> for other purposes.
> 
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
> 
> Greg, can you tell me if you think this patch is a good idea?  It doesn't
> appear to do any harm, and I'm working on an ALSA driver that could benefit
> for this patch.  I think 2.6.25 would be a good target.

Huh?  Why is this needed?  Can you give me a usage case for it?  Why
would you just not use the pointer to the attribute itself to identify
it?  All device specific information should be stored in the struct
device structure, as that is what describes the device, not the
attribute.

confused,

greg k-h

Re: Add a private_data pointer to struct device_attribute

[Timur Tabi]

Mikael Pettersson wrote:

> A common trick is to embed a generic struct inside a specific one
> containing add-on data fields, and then to map from the generic
> one to the specific one using container_of() in your ops (function
> pointers). This is both faster and less wasteful of memory than
> adding void *private all over the place.
> 
> Any reason that won't work here?

Yes, that will work.  Sorry, I should have thought of that myself, since I've 
used that trick a number of times before.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: Add a private_data pointer to struct device_attribute

[Mikael Pettersson]

On Fri, 16 Nov 2007 18:11:00 -0600, Timur Tabi wrote:
> A private data pointer in struct device_attribute allows the 'show' and 'store'
> functions to access instance data.  This handy in situations where the
> driver_data and platform_data pointers of 'struct device' are already used
> for other purposes.
> 
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
> 
> Greg, can you tell me if you think this patch is a good idea?  It doesn't
> appear to do any harm, and I'm working on an ALSA driver that could benefit
> for this patch.  I think 2.6.25 would be a good target.
> 
>  include/linux/device.h |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 2e15822..10708ee 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -344,6 +344,7 @@ struct device_attribute {
>  			char *buf);
>  	ssize_t (*store)(struct device *dev, struct device_attribute *attr,
>  			 const char *buf, size_t count);
> +	void *private_data;
>  };

A common trick is to embed a generic struct inside a specific one
containing add-on data fields, and then to map from the generic
one to the specific one using container_of() in your ops (function
pointers). This is both faster and less wasteful of memory than
adding void *private all over the place.

Any reason that won't work here?