LinuxLists.cc - [RFC 0/6] Driver core: Encourage use of const attributes

2009-12-17 18:14:27

Subject: [RFC 0/6] Driver core: Encourage use of const attributes

I know now's probably a bad time to be mentioning const issues
but I recently a reviewed a patch where 'const' was being removed
from a device_attribute structure, and I enquired why. The reason
was simply so that device_create_file() could be called, it taking
a non-const attribute pointer. Looking inside, all that function
did was pass a pointer to sysfs_create_file, which itself took a
const pointer. Non-constness was not required at all.

So here I offer a small patchset which I hope will permit and
encourage device and other attributes to be made const, and put
in read-only sections.

1-3 address the three attribute types which seemed to be trivially
const-able, and are the important part of the set.

4 adds a new macro to encourage the use of Const ATTRibutes,
and may need a better name. (I wanted to avoid RO, for example.)

5 and 6 are merely two quick examples of how easy it is to adopt
the new const convention. In reality, these structures have been
constant and treated as constant by the driver core all along, it's
just that one word was missing from a few important places.

I would hope to submit a patchset with 1-3 and a possibly modified
4. The migrations themselves will belong in different trees.

These have been compile/sparse tested against arm and x86_64 targets,
and through my usual suite of tests on the arm platform I use.

Regards,
Phil

Phil Carmody (6):
Driver core: device_attribute parameters can often be const*
Driver core: bin_attribute parameters can often be const*
Driver core: driver_attribute parameters can often be const*
Driver core: Easy macros to encourage const attributes
PM: Example of how easy it is to mark attributes const
gpio: Example of how easy it is to mark attributes const

drivers/base/core.c | 12 ++++++++----
drivers/base/driver.c | 4 ++--
drivers/gpio/gpiolib.c | 12 ++++++------
drivers/regulator/core.c | 38 +++++++++++++++++++-------------------
fs/sysfs/bin.c | 6 ++++--
include/linux/device.h | 20 ++++++++++++++------
include/linux/sysfs.h | 9 +++++----
7 files changed, 58 insertions(+), 43 deletions(-)

2009-12-17 18:14:19

by Phil Carmody

[permalink] [raw]

Subject: [RFC 1/6] Driver core: device_attribute parameters can often be const*

Most device_attributes are const, and are begging to be
put in a ro section. However, the create and remove
file interfaces were failing to propagate the const promise
which the only functions they call offer.

Signed-off-by: Phil Carmody <[email protected]>
---
Documentation/filesystems/sysfs.txt | 8 ++++----
drivers/base/core.c | 6 ++++--
include/linux/device.h | 4 ++--
3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/Documentation/filesystems/sysfs.txt b/Documentation/filesystems/sysfs.txt
index b245d52..a4ac2b9 100644
--- a/Documentation/filesystems/sysfs.txt
+++ b/Documentation/filesystems/sysfs.txt
@@ -91,8 +91,8 @@ struct device_attribute {
const char *buf, size_t count);
};

-int device_create_file(struct device *, struct device_attribute *);
-void device_remove_file(struct device *, struct device_attribute *);
+int device_create_file(struct device *, const struct device_attribute *);
+void device_remove_file(struct device *, const struct device_attribute *);

It also defines this helper for defining device attributes:

@@ -316,8 +316,8 @@ DEVICE_ATTR(_name, _mode, _show, _store);

Creation/Removal:

-int device_create_file(struct device *device, struct device_attribute * attr);
-void device_remove_file(struct device * dev, struct device_attribute * attr);
+int device_create_file(struct device *dev, const struct device_attribute * attr);
+void device_remove_file(struct device *dev, const struct device_attribute * attr);

- bus drivers (include/linux/device.h)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 6bee6af..c1e3cad 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -439,7 +439,8 @@ struct kset *devices_kset;
* @dev: device.
* @attr: device attribute descriptor.
*/
-int device_create_file(struct device *dev, struct device_attribute *attr)
+int device_create_file(struct device *dev,
+ const struct device_attribute *attr)
{
int error = 0;
if (dev)
@@ -452,7 +453,8 @@ int device_create_file(struct device *dev, struct device_attribute *attr)
* @dev: device.
* @attr: device attribute descriptor.
*/
-void device_remove_file(struct device *dev, struct device_attribute *attr)
+void device_remove_file(struct device *dev,
+ const struct device_attribute *attr)
{
if (dev)
sysfs_remove_file(&dev->kobj, &attr->attr);
diff --git a/include/linux/device.h b/include/linux/device.h
index 2ea3e49..9ecd9b0 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -319,9 +319,9 @@ struct device_attribute {
struct device_attribute dev_attr_##_name = __ATTR(_name, _mode, _show, _store)

extern int __must_check device_create_file(struct device *device,
- struct device_attribute *entry);
+ const struct device_attribute *entry);
extern void device_remove_file(struct device *dev,
- struct device_attribute *attr);
+ const struct device_attribute *attr);
extern int __must_check device_create_bin_file(struct device *dev,
struct bin_attribute *attr);
extern void device_remove_bin_file(struct device *dev,
--
1.6.0.4

2009-12-17 18:14:48

by Phil Carmody

[permalink] [raw]

Subject: [RFC 2/6] Driver core: bin_attribute parameters can often be const*

Many struct bin_attribute descriptors are purely read-only
structures, and there's no need to change them. Therefore
make the promise not to, which will let those descriptors
be put in a ro section.

Signed-off-by: Phil Carmody <[email protected]>
---
drivers/base/core.c | 6 ++++--
fs/sysfs/bin.c | 6 ++++--
include/linux/device.h | 6 +++---
include/linux/sysfs.h | 9 +++++----
4 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index c1e3cad..d0b2116 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -465,7 +465,8 @@ void device_remove_file(struct device *dev,
* @dev: device.
* @attr: device binary attribute descriptor.
*/
-int device_create_bin_file(struct device *dev, struct bin_attribute *attr)
+int device_create_bin_file(struct device *dev,
+ const struct bin_attribute *attr)
{
int error = -EINVAL;
if (dev)
@@ -479,7 +480,8 @@ EXPORT_SYMBOL_GPL(device_create_bin_file);
* @dev: device.
* @attr: device binary attribute descriptor.
*/
-void device_remove_bin_file(struct device *dev, struct bin_attribute *attr)
+void device_remove_bin_file(struct device *dev,
+ const struct bin_attribute *attr)
{
if (dev)
sysfs_remove_bin_file(&dev->kobj, attr);
diff --git a/fs/sysfs/bin.c b/fs/sysfs/bin.c
index 60c702b..a0a500a 100644
--- a/fs/sysfs/bin.c
+++ b/fs/sysfs/bin.c
@@ -483,7 +483,8 @@ void unmap_bin_file(struct sysfs_dirent *attr_sd)
* @attr: attribute descriptor.
*/

-int sysfs_create_bin_file(struct kobject * kobj, struct bin_attribute * attr)
+int sysfs_create_bin_file(struct kobject *kobj,
+ const struct bin_attribute *attr)
{
BUG_ON(!kobj || !kobj->sd || !attr);

@@ -497,7 +498,8 @@ int sysfs_create_bin_file(struct kobject * kobj, struct bin_attribute * attr)
* @attr: attribute descriptor.
*/

-void sysfs_remove_bin_file(struct kobject * kobj, struct bin_attribute * attr)
+void sysfs_remove_bin_file(struct kobject *kobj,
+ const struct bin_attribute *attr)
{
sysfs_hash_and_remove(kobj->sd, attr->attr.name);
}
diff --git a/include/linux/device.h b/include/linux/device.h
index 9ecd9b0..0823a29 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -319,13 +319,13 @@ struct device_attribute {
struct device_attribute dev_attr_##_name = __ATTR(_name, _mode, _show, _store)

extern int __must_check device_create_file(struct device *device,
- const struct device_attribute *entry);
+ const struct device_attribute *entry);
extern void device_remove_file(struct device *dev,
const struct device_attribute *attr);
extern int __must_check device_create_bin_file(struct device *dev,
- struct bin_attribute *attr);
+ const struct bin_attribute *attr);
extern void device_remove_bin_file(struct device *dev,
- struct bin_attribute *attr);
+ const struct bin_attribute *attr);
extern int device_schedule_callback_owner(struct device *dev,
void (*func)(struct device *dev), struct module *owner);

diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 9d68fed..cfa8308 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -99,8 +99,9 @@ int __must_check sysfs_chmod_file(struct kobject *kobj, struct attribute *attr,
void sysfs_remove_file(struct kobject *kobj, const struct attribute *attr);

int __must_check sysfs_create_bin_file(struct kobject *kobj,
- struct bin_attribute *attr);
-void sysfs_remove_bin_file(struct kobject *kobj, struct bin_attribute *attr);
+ const struct bin_attribute *attr);
+void sysfs_remove_bin_file(struct kobject *kobj,
+ const struct bin_attribute *attr);

int __must_check sysfs_create_link(struct kobject *kobj, struct kobject *target,
const char *name);
@@ -175,13 +176,13 @@ static inline void sysfs_remove_file(struct kobject *kobj,
}

static inline int sysfs_create_bin_file(struct kobject *kobj,
- struct bin_attribute *attr)
+ const struct bin_attribute *attr)
{
return 0;
}

static inline void sysfs_remove_bin_file(struct kobject *kobj,
- struct bin_attribute *attr)
+ const struct bin_attribute *attr)
{
}

--
1.6.0.4

2009-12-17 18:12:12

by Phil Carmody

[permalink] [raw]

Subject: [RFC 3/6] Driver core: driver_attribute parameters can often be const*

Many struct driver_attribute descriptors are purely read-only
structures, and there's no need to change them. Therefore make
the promise not to, which will let those descriptors be put in
a ro section.

Signed-off-by: Phil Carmody <[email protected]>
---
Documentation/driver-model/driver.txt | 4 ++--
Documentation/filesystems/sysfs.txt | 4 ++--
drivers/base/driver.c | 4 ++--
include/linux/device.h | 4 ++--
4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/driver-model/driver.txt b/Documentation/driver-model/driver.txt
index 60120fb..d2cd6fb 100644
--- a/Documentation/driver-model/driver.txt
+++ b/Documentation/driver-model/driver.txt
@@ -226,5 +226,5 @@ struct driver_attribute driver_attr_debug;
This can then be used to add and remove the attribute from the
driver's directory using:

-int driver_create_file(struct device_driver *, struct driver_attribute *);
-void driver_remove_file(struct device_driver *, struct driver_attribute *);
+int driver_create_file(struct device_driver *, const struct driver_attribute *);
+void driver_remove_file(struct device_driver *, const struct driver_attribute *);
diff --git a/Documentation/filesystems/sysfs.txt b/Documentation/filesystems/sysfs.txt
index a4ac2b9..931c806 100644
--- a/Documentation/filesystems/sysfs.txt
+++ b/Documentation/filesystems/sysfs.txt
@@ -358,7 +358,7 @@ DRIVER_ATTR(_name, _mode, _show, _store)

Creation/Removal:

-int driver_create_file(struct device_driver *, struct driver_attribute *);
-void driver_remove_file(struct device_driver *, struct driver_attribute *);
+int driver_create_file(struct device_driver *, const struct driver_attribute *);
+void driver_remove_file(struct device_driver *, const struct driver_attribute *);

diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index f367885..90c9fff 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -98,7 +98,7 @@ EXPORT_SYMBOL_GPL(driver_find_device);
* @attr: driver attribute descriptor.
*/
int driver_create_file(struct device_driver *drv,
- struct driver_attribute *attr)
+ const struct driver_attribute *attr)
{
int error;
if (drv)
@@ -115,7 +115,7 @@ EXPORT_SYMBOL_GPL(driver_create_file);
* @attr: driver attribute descriptor.
*/
void driver_remove_file(struct device_driver *drv,
- struct driver_attribute *attr)
+ const struct driver_attribute *attr)
{
if (drv)
sysfs_remove_file(&drv->p->kobj, &attr->attr);
diff --git a/include/linux/device.h b/include/linux/device.h
index 0823a29..d07f90f 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -166,9 +166,9 @@ struct driver_attribute driver_attr_##_name = \
__ATTR(_name, _mode, _show, _store)

extern int __must_check driver_create_file(struct device_driver *driver,
- struct driver_attribute *attr);
+ const struct driver_attribute *attr);
extern void driver_remove_file(struct device_driver *driver,
- struct driver_attribute *attr);
+ const struct driver_attribute *attr);

extern int __must_check driver_add_kobj(struct device_driver *drv,
struct kobject *kobj,
--
1.6.0.4

2009-12-17 18:13:53

by Phil Carmody

[permalink] [raw]

Subject: [RFC 4/6] Driver core: Easy macros to encourage const attributes

It's too easy to type DEVICE_ATTR and not think of adding
a 'const', make it just as easy to get const as part of the
macro.

Signed-off-by: Phil Carmody <[email protected]>
---
include/linux/device.h | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index d07f90f..ea8ced6 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -165,6 +165,10 @@ struct driver_attribute {
struct driver_attribute driver_attr_##_name = \
__ATTR(_name, _mode, _show, _store)

+#define DRIVER_CATTR(_name, _mode, _show, _store) \
+const struct driver_attribute driver_attr_##_name = \
+ __ATTR(_name, _mode, _show, _store)
+
extern int __must_check driver_create_file(struct device_driver *driver,
const struct driver_attribute *attr);
extern void driver_remove_file(struct device_driver *driver,
@@ -318,6 +322,10 @@ struct device_attribute {
#define DEVICE_ATTR(_name, _mode, _show, _store) \
struct device_attribute dev_attr_##_name = __ATTR(_name, _mode, _show, _store)

+#define DEVICE_CATTR(_name, _mode, _show, _store) \
+const struct device_attribute dev_attr_##_name = \
+ __ATTR(_name, _mode, _show, _store)
+
extern int __must_check device_create_file(struct device *device,
const struct device_attribute *entry);
extern void device_remove_file(struct device *dev,
--
1.6.0.4

2009-12-17 18:13:25

by Phil Carmody

[permalink] [raw]

Subject: [RFC 5/6] PM: Example of how easy it is to mark attributes const

These attributes can now be in ro sections.

Signed-off-by: Phil Carmody <[email protected]>
---
drivers/regulator/core.c | 38 +++++++++++++++++++-------------------
1 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index efe568d..f2b4376 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -215,7 +215,7 @@ static ssize_t regulator_uV_show(struct device *dev,

return ret;
}
-static DEVICE_ATTR(microvolts, 0444, regulator_uV_show, NULL);
+static DEVICE_CATTR(microvolts, 0444, regulator_uV_show, NULL);

static ssize_t regulator_uA_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -224,7 +224,7 @@ static ssize_t regulator_uA_show(struct device *dev,

return sprintf(buf, "%d\n", _regulator_get_current_limit(rdev));
}
-static DEVICE_ATTR(microamps, 0444, regulator_uA_show, NULL);
+static DEVICE_CATTR(microamps, 0444, regulator_uA_show, NULL);

static ssize_t regulator_name_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -264,7 +264,7 @@ static ssize_t regulator_opmode_show(struct device *dev,

return regulator_print_opmode(buf, _regulator_get_mode(rdev));
}
-static DEVICE_ATTR(opmode, 0444, regulator_opmode_show, NULL);
+static DEVICE_CATTR(opmode, 0444, regulator_opmode_show, NULL);

static ssize_t regulator_print_state(char *buf, int state)
{
@@ -288,7 +288,7 @@ static ssize_t regulator_state_show(struct device *dev,

return ret;
}
-static DEVICE_ATTR(state, 0444, regulator_state_show, NULL);
+static DEVICE_CATTR(state, 0444, regulator_state_show, NULL);

static ssize_t regulator_status_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -329,7 +329,7 @@ static ssize_t regulator_status_show(struct device *dev,

return sprintf(buf, "%s\n", label);
}
-static DEVICE_ATTR(status, 0444, regulator_status_show, NULL);
+static DEVICE_CATTR(status, 0444, regulator_status_show, NULL);

static ssize_t regulator_min_uA_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -341,7 +341,7 @@ static ssize_t regulator_min_uA_show(struct device *dev,

return sprintf(buf, "%d\n", rdev->constraints->min_uA);
}
-static DEVICE_ATTR(min_microamps, 0444, regulator_min_uA_show, NULL);
+static DEVICE_CATTR(min_microamps, 0444, regulator_min_uA_show, NULL);

static ssize_t regulator_max_uA_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -353,7 +353,7 @@ static ssize_t regulator_max_uA_show(struct device *dev,

return sprintf(buf, "%d\n", rdev->constraints->max_uA);
}
-static DEVICE_ATTR(max_microamps, 0444, regulator_max_uA_show, NULL);
+static DEVICE_CATTR(max_microamps, 0444, regulator_max_uA_show, NULL);

static ssize_t regulator_min_uV_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -365,7 +365,7 @@ static ssize_t regulator_min_uV_show(struct device *dev,

return sprintf(buf, "%d\n", rdev->constraints->min_uV);
}
-static DEVICE_ATTR(min_microvolts, 0444, regulator_min_uV_show, NULL);
+static DEVICE_CATTR(min_microvolts, 0444, regulator_min_uV_show, NULL);

static ssize_t regulator_max_uV_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -377,7 +377,7 @@ static ssize_t regulator_max_uV_show(struct device *dev,

return sprintf(buf, "%d\n", rdev->constraints->max_uV);
}
-static DEVICE_ATTR(max_microvolts, 0444, regulator_max_uV_show, NULL);
+static DEVICE_CATTR(max_microvolts, 0444, regulator_max_uV_show, NULL);

static ssize_t regulator_total_uA_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -392,7 +392,7 @@ static ssize_t regulator_total_uA_show(struct device *dev,
mutex_unlock(&rdev->mutex);
return sprintf(buf, "%d\n", uA);
}
-static DEVICE_ATTR(requested_microamps, 0444, regulator_total_uA_show, NULL);
+static DEVICE_CATTR(requested_microamps, 0444, regulator_total_uA_show, NULL);

static ssize_t regulator_num_users_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -422,7 +422,7 @@ static ssize_t regulator_suspend_mem_uV_show(struct device *dev,

return sprintf(buf, "%d\n", rdev->constraints->state_mem.uV);
}
-static DEVICE_ATTR(suspend_mem_microvolts, 0444,
+static DEVICE_CATTR(suspend_mem_microvolts, 0444,
regulator_suspend_mem_uV_show, NULL);

static ssize_t regulator_suspend_disk_uV_show(struct device *dev,
@@ -432,7 +432,7 @@ static ssize_t regulator_suspend_disk_uV_show(struct device *dev,

return sprintf(buf, "%d\n", rdev->constraints->state_disk.uV);
}
-static DEVICE_ATTR(suspend_disk_microvolts, 0444,
+static DEVICE_CATTR(suspend_disk_microvolts, 0444,
regulator_suspend_disk_uV_show, NULL);

static ssize_t regulator_suspend_standby_uV_show(struct device *dev,
@@ -442,7 +442,7 @@ static ssize_t regulator_suspend_standby_uV_show(struct device *dev,

return sprintf(buf, "%d\n", rdev->constraints->state_standby.uV);
}
-static DEVICE_ATTR(suspend_standby_microvolts, 0444,
+static DEVICE_CATTR(suspend_standby_microvolts, 0444,
regulator_suspend_standby_uV_show, NULL);

static ssize_t regulator_suspend_mem_mode_show(struct device *dev,
@@ -453,7 +453,7 @@ static ssize_t regulator_suspend_mem_mode_show(struct device *dev,
return regulator_print_opmode(buf,
rdev->constraints->state_mem.mode);
}
-static DEVICE_ATTR(suspend_mem_mode, 0444,
+static DEVICE_CATTR(suspend_mem_mode, 0444,
regulator_suspend_mem_mode_show, NULL);

static ssize_t regulator_suspend_disk_mode_show(struct device *dev,
@@ -464,7 +464,7 @@ static ssize_t regulator_suspend_disk_mode_show(struct device *dev,
return regulator_print_opmode(buf,
rdev->constraints->state_disk.mode);
}
-static DEVICE_ATTR(suspend_disk_mode, 0444,
+static DEVICE_CATTR(suspend_disk_mode, 0444,
regulator_suspend_disk_mode_show, NULL);

static ssize_t regulator_suspend_standby_mode_show(struct device *dev,
@@ -475,7 +475,7 @@ static ssize_t regulator_suspend_standby_mode_show(struct device *dev,
return regulator_print_opmode(buf,
rdev->constraints->state_standby.mode);
}
-static DEVICE_ATTR(suspend_standby_mode, 0444,
+static DEVICE_CATTR(suspend_standby_mode, 0444,
regulator_suspend_standby_mode_show, NULL);

static ssize_t regulator_suspend_mem_state_show(struct device *dev,
@@ -486,7 +486,7 @@ static ssize_t regulator_suspend_mem_state_show(struct device *dev,
return regulator_print_state(buf,
rdev->constraints->state_mem.enabled);
}
-static DEVICE_ATTR(suspend_mem_state, 0444,
+static DEVICE_CATTR(suspend_mem_state, 0444,
regulator_suspend_mem_state_show, NULL);

static ssize_t regulator_suspend_disk_state_show(struct device *dev,
@@ -497,7 +497,7 @@ static ssize_t regulator_suspend_disk_state_show(struct device *dev,
return regulator_print_state(buf,
rdev->constraints->state_disk.enabled);
}
-static DEVICE_ATTR(suspend_disk_state, 0444,
+static DEVICE_CATTR(suspend_disk_state, 0444,
regulator_suspend_disk_state_show, NULL);

static ssize_t regulator_suspend_standby_state_show(struct device *dev,
@@ -508,7 +508,7 @@ static ssize_t regulator_suspend_standby_state_show(struct device *dev,
return regulator_print_state(buf,
rdev->constraints->state_standby.enabled);
}
-static DEVICE_ATTR(suspend_standby_state, 0444,
+static DEVICE_CATTR(suspend_standby_state, 0444,
regulator_suspend_standby_state_show, NULL);

--
1.6.0.4

2009-12-17 18:13:14

by Phil Carmody

[permalink] [raw]

Subject: [RFC 6/6] gpio: Example of how easy it is to mark attributes const

These attributes may now be placed in ro sections.

Signed-off-by: Phil Carmody <[email protected]>
---
drivers/gpio/gpiolib.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 50de0f5..be99438 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -255,7 +255,7 @@ static ssize_t gpio_direction_store(struct device *dev,
return status ? : size;
}

-static const DEVICE_ATTR(direction, 0644,
+static DEVICE_CATTR(direction, 0644,
gpio_direction_show, gpio_direction_store);

static ssize_t gpio_value_show(struct device *dev,
@@ -303,7 +303,7 @@ static ssize_t gpio_value_store(struct device *dev,
return status;
}

-static /*const*/ DEVICE_ATTR(value, 0644,
+static DEVICE_CATTR(value, 0644,
gpio_value_show, gpio_value_store);

static irqreturn_t gpio_sysfs_irq(int irq, void *priv)
@@ -473,7 +473,7 @@ found:
return status;
}

-static DEVICE_ATTR(edge, 0644, gpio_edge_show, gpio_edge_store);
+static DEVICE_CATTR(edge, 0644, gpio_edge_show, gpio_edge_store);

static const struct attribute *gpio_attrs[] = {
&dev_attr_direction.attr,
@@ -499,7 +499,7 @@ static ssize_t chip_base_show(struct device *dev,

return sprintf(buf, "%d\n", chip->base);
}
-static DEVICE_ATTR(base, 0444, chip_base_show, NULL);
+static DEVICE_CATTR(base, 0444, chip_base_show, NULL);

static ssize_t chip_label_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -508,7 +508,7 @@ static ssize_t chip_label_show(struct device *dev,

return sprintf(buf, "%s\n", chip->label ? : "");
}
-static DEVICE_ATTR(label, 0444, chip_label_show, NULL);
+static DEVICE_CATTR(label, 0444, chip_label_show, NULL);

static ssize_t chip_ngpio_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -517,7 +517,7 @@ static ssize_t chip_ngpio_show(struct device *dev,

return sprintf(buf, "%u\n", chip->ngpio);
}
-static DEVICE_ATTR(ngpio, 0444, chip_ngpio_show, NULL);
+static DEVICE_CATTR(ngpio, 0444, chip_ngpio_show, NULL);

static const struct attribute *gpiochip_attrs[] = {
&dev_attr_base.attr,
--
1.6.0.4

2009-12-17 23:16:31

by Greg KH

[permalink] [raw]

Subject: Re: [RFC 0/6] Driver core: Encourage use of const attributes

On Thu, Dec 17, 2009 at 08:12:10PM +0200, Phil Carmody wrote:
> I know now's probably a bad time to be mentioning const issues
> but I recently a reviewed a patch where 'const' was being removed
> from a device_attribute structure, and I enquired why. The reason
> was simply so that device_create_file() could be called, it taking
> a non-const attribute pointer. Looking inside, all that function
> did was pass a pointer to sysfs_create_file, which itself took a
> const pointer. Non-constness was not required at all.
>
> So here I offer a small patchset which I hope will permit and
> encourage device and other attributes to be made const, and put
> in read-only sections.
>
> 1-3 address the three attribute types which seemed to be trivially
> const-able, and are the important part of the set.

I like these, very nice.

> 4 adds a new macro to encourage the use of Const ATTRibutes,
> and may need a better name. (I wanted to avoid RO, for example.)

Hm, is this really needed? How badly do things break if you change the
current attribute macros to use 'const'? What subsystems are not using
const?

> 5 and 6 are merely two quick examples of how easy it is to adopt
> the new const convention. In reality, these structures have been
> constant and treated as constant by the driver core all along, it's
> just that one word was missing from a few important places.
>
> I would hope to submit a patchset with 1-3 and a possibly modified
> 4. The migrations themselves will belong in different trees.

I'll be glad to take 1-3 now, and queue it up for .34.

thanks,

greg k-h

2009-12-18 00:42:33

by Stephen Rothwell

[permalink] [raw]

Subject: Re: [RFC 0/6] Driver core: Encourage use of const attributes

Hi Greg,

On Thu, 17 Dec 2009 15:16:21 -0800 Greg KH <[email protected]> wrote:
>
> I'll be glad to take 1-3 now, and queue it up for .34.

Any reason not to ask Linus to take them for .33 - that way the
conversions of callers can be done adhoc (and in more than just your
tree) for .34. These API changes should be perfectly safe (and the
compiler should tell us if they are not).

--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/

Attachments:

(No filename) (484.00 B)
(No filename) (198.00 B)
Download all attachments

2009-12-18 04:23:11

by Greg KH

[permalink] [raw]

Subject: Re: [RFC 0/6] Driver core: Encourage use of const attributes

On Fri, Dec 18, 2009 at 11:42:23AM +1100, Stephen Rothwell wrote:
> Hi Greg,
>
> On Thu, 17 Dec 2009 15:16:21 -0800 Greg KH <[email protected]> wrote:
> >
> > I'll be glad to take 1-3 now, and queue it up for .34.
>
> Any reason not to ask Linus to take them for .33 - that way the
> conversions of callers can be done adhoc (and in more than just your
> tree) for .34. These API changes should be perfectly safe (and the
> compiler should tell us if they are not).

Good idea, I'll queue them up to get in soon.

thanks,

greg k-h

2009-12-18 08:52:25

by Phil Carmody

[permalink] [raw]

Subject: Re: [RFC 0/6] Driver core: Encourage use of const attributes

On 18/12/09 00:16 +0100, ext Greg KH wrote:
> On Thu, Dec 17, 2009 at 08:12:10PM +0200, Phil Carmody wrote:
[...]> > So here I offer a small patchset which I hope will permit and
> > encourage device and other attributes to be made const, and put
> > in read-only sections.
> >
> > 1-3 address the three attribute types which seemed to be trivially
> > const-able, and are the important part of the set.
>
> I like these, very nice.

Thanks, I can possibly sniff a little bit deeper, and see if there are
any other obvious throwing-away-of-consts nearby.

> > 4 adds a new macro to encourage the use of Const ATTRibutes,
> > and may need a better name. (I wanted to avoid RO, for example.)
>
> Hm, is this really needed? How badly do things break if you change the
> current attribute macros to use 'const'? What subsystems are not using
> const?

That was the first thing that went through my mind, but I didn't want
to be too brave. He who dares wins though, and I can certainly give
that a try. allmodconfig is my friend.

> > 5 and 6 are merely two quick examples of how easy it is to adopt
> > the new const convention. In reality, these structures have been
> > constant and treated as constant by the driver core all along, it's
> > just that one word was missing from a few important places.
> >
> > I would hope to submit a patchset with 1-3 and a possibly modified
> > 4. The migrations themselves will belong in different trees.
>
> I'll be glad to take 1-3 now, and queue it up for .34.

They cleave cleanly at that point, so I'll resend with a [PATCH] prefix.

Many thanks, and seasons greetings,

Phil

2009-12-18 13:34:55

by Phil Carmody

[permalink] [raw]

Subject: [PATCH 0/3] Driver core: Encourage use of const attributes

2009-12-18 13:34:19

by Phil Carmody

[permalink] [raw]

Subject: [PATCH 1/3] Driver core: device_attribute parameters can often be const*

2009-12-18 13:34:39

by Phil Carmody

[permalink] [raw]

Subject: [PATCH 2/3] Driver core: bin_attribute parameters can often be const*

2009-12-18 13:34:25

by Phil Carmody

[permalink] [raw]

Subject: [PATCH 3/3] Driver core: driver_attribute parameters can often be const*

2009-12-18 17:09:45

by Greg KH

[permalink] [raw]

Subject: Re: [RFC 0/6] Driver core: Encourage use of const attributes

On Fri, Dec 18, 2009 at 10:52:21AM +0200, Phil Carmody wrote:
> On 18/12/09 00:16 +0100, ext Greg KH wrote:
> > On Thu, Dec 17, 2009 at 08:12:10PM +0200, Phil Carmody wrote:
> [...]> > So here I offer a small patchset which I hope will permit and
> > > encourage device and other attributes to be made const, and put
> > > in read-only sections.
> > >
> > > 1-3 address the three attribute types which seemed to be trivially
> > > const-able, and are the important part of the set.
> >
> > I like these, very nice.
>
> Thanks, I can possibly sniff a little bit deeper, and see if there are
> any other obvious throwing-away-of-consts nearby.

Great.

> > > 4 adds a new macro to encourage the use of Const ATTRibutes,
> > > and may need a better name. (I wanted to avoid RO, for example.)
> >
> > Hm, is this really needed? How badly do things break if you change the
> > current attribute macros to use 'const'? What subsystems are not using
> > const?
>
> That was the first thing that went through my mind, but I didn't want
> to be too brave. He who dares wins though, and I can certainly give
> that a try. allmodconfig is my friend.

That would be nice to try, let me know what you find.

> > > 5 and 6 are merely two quick examples of how easy it is to adopt
> > > the new const convention. In reality, these structures have been
> > > constant and treated as constant by the driver core all along, it's
> > > just that one word was missing from a few important places.
> > >
> > > I would hope to submit a patchset with 1-3 and a possibly modified
> > > 4. The migrations themselves will belong in different trees.
> >
> > I'll be glad to take 1-3 now, and queue it up for .34.
>
> They cleave cleanly at that point, so I'll resend with a [PATCH] prefix.

Got them, I'll go queue them up.

thanks again,

greg k-h