2007-11-17 00:11:21

by Timur Tabi

[permalink] [raw]
Subject: Add a private_data pointer to struct device_attribute

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 <[email protected]>
---

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


2007-11-17 01:08:45

by Greg KH

[permalink] [raw]
Subject: Re: Add a private_data pointer to struct device_attribute

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 <[email protected]>
> ---
>
> 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

2007-11-17 01:19:32

by Mikael Pettersson

[permalink] [raw]
Subject: Re: Add a private_data pointer to struct device_attribute

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 <[email protected]>
> ---
>
> 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?

2007-11-17 14:02:23

by Timur Tabi

[permalink] [raw]
Subject: Re: Add a private_data pointer to struct device_attribute

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