Re: [PATCH 4/6] regulator: regulator core.
On Sun, 2008-02-24 at 09:51 +0100, Pavel Machek wrote: > And what? You can't measure anything but current in amps, so ambiguity > is not a problem. It not is the credible validation of the language English. Thanking you kindly. -Ian -- 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/
Re: [PATCH 4/6] regulator: regulator core.
On Sat 2008-02-23 19:26:36, ian wrote: > > On Sat, 2008-02-23 at 10:18 -0800, Andrew Morton wrote: > > > Renaming is not an option - "current" is an electronic term for > > which > > > there is no alternative. > > > > amps, milliamps, > > Are units of current, not current itself. And what? You can't measure anything but current in amps, so ambiguity is not a problem. -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- 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/
Re: [PATCH 4/6] regulator: regulator core.
On Sun, Feb 24, 2008 at 10:23:49AM +, Mark Brown wrote:
> On Sat, Feb 23, 2008 at 09:52:17AM +, Russell King - ARM Linux wrote:
> > On Sat, Feb 23, 2008 at 12:05:38AM -0800, Andrew Morton wrote:
>
> > > > +#undef current
>
> > > err, no ;) Please rename your stuff.
>
> > Renaming is not an option - "current" is an electronic term for which
> > there is no alternative. The real problem is this __ATTR macro crap:
>
> Indeed, and this is already being worked around by
> drivers/power/power_supply_sysfs.c:
>
> #define POWER_SUPPLY_ATTR(_name)\
> { \
> .attr = { .name = #_name, .mode = 0444, .owner = THIS_MODULE }, \
My argument is - why bother with stuff like this, why not just pass
"_name" as the string itself, rather than using the preprocessor to
turn symbols into strings thereby suffering these nasty interactions.
It's not like "_name" is used for anything other than generating a
string.
--
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/
Re: [PATCH 4/6] regulator: regulator core.
On Sat, Feb 23, 2008 at 09:52:17AM +, Russell King - ARM Linux wrote:
> On Sat, Feb 23, 2008 at 12:05:38AM -0800, Andrew Morton wrote:
> > > +#undef current
> > err, no ;) Please rename your stuff.
> Renaming is not an option - "current" is an electronic term for which
> there is no alternative. The real problem is this __ATTR macro crap:
Indeed, and this is already being worked around by
drivers/power/power_supply_sysfs.c:
#define POWER_SUPPLY_ATTR(_name)\
{ \
.attr = { .name = #_name, .mode = 0444, .owner = THIS_MODULE }, \
.show = power_supply_show_property, \
.store = NULL, \
}
--
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/
Re: [PATCH 4/6] regulator: regulator core.
On Sat, 23 Feb 2008 19:26:36 + ian <[EMAIL PROTECTED]> wrote: > On Sat, 2008-02-23 at 10:18 -0800, Andrew Morton wrote: > > > Renaming is not an option - "current" is an electronic term for > > which > > > there is no alternative. > > > > amps, milliamps, > > Are units of current, not current itself. Right. And this is an argument *in favour* of naming the sysfs files "amps" (actually microamps), not "current". It conveys more information. -- 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/
Re: [PATCH 4/6] regulator: regulator core.
On Sat, 2008-02-23 at 10:18 -0800, Andrew Morton wrote: > > Renaming is not an option - "current" is an electronic term for > which > > there is no alternative. > > amps, milliamps, Are units of current, not current itself. > amperage (the latter of which I've always disliked) Ugh. -- 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/
Re: [PATCH 4/6] regulator: regulator core.
On Sat, 23 Feb 2008 09:52:17 + Russell King - ARM Linux <[EMAIL PROTECTED]>
wrote:
> On Sat, Feb 23, 2008 at 12:05:38AM -0800, Andrew Morton wrote:
> > On Wed, 20 Feb 2008 17:09:11 + Liam Girdwood <[EMAIL PROTECTED]> wrote:
> >
> > > This patch provides the regulator framework core. The core also provides a
> > > sysfs interface for userspace information.
> > >
> > > ...
> > >
> > > +
> > > +/* We need to undef the current macro (from include/asm/current.h)
> > > otherwise
> > > + * our "current" sysfs entry becomes "(get_current())".
> > > + */
> > > +#undef current
> >
> > err, no ;) Please rename your stuff.
>
> Renaming is not an option - "current" is an electronic term for which
> there is no alternative.
amps, milliamps, amperage (the latter of which I've always disliked)
> The real problem is this __ATTR macro crap:
>
> + __ATTR(current, 0444, regulator_uA_show, NULL),
This interface would actually be *improved* if that sysfs file were named
"microamps" rather than "current". Because its units become
self-documenting. Similarly, s/voltage/millivolts/g
> #define __ATTR(_name,_mode,_show,_store) { \
> .attr = {.name = __stringify(_name), .mode = _mode }, \
> .show = _show,\
> .store = _store, \
> }
>
> If __ATTR allowed the name to be passed as a string, instead of stupidly
> stringifying it, then this wouldn't be needed. So I suggest that the
> #undef stands _until_ we fix this stupid macro.
yeah, macros suck.
> Or we rename the
> "current" macro to something that doesn't clash with accepted scientific
> and engineering terminology.
--
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/
Re: [PATCH 4/6] regulator: regulator core.
On Sat, Feb 23, 2008 at 12:05:38AM -0800, Andrew Morton wrote:
> On Wed, 20 Feb 2008 17:09:11 + Liam Girdwood <[EMAIL PROTECTED]> wrote:
>
> > This patch provides the regulator framework core. The core also provides a
> > sysfs interface for userspace information.
> >
> > ...
> >
> > +
> > +/* We need to undef the current macro (from include/asm/current.h)
> > otherwise
> > + * our "current" sysfs entry becomes "(get_current())".
> > + */
> > +#undef current
>
> err, no ;) Please rename your stuff.
Renaming is not an option - "current" is an electronic term for which
there is no alternative. The real problem is this __ATTR macro crap:
+ __ATTR(current, 0444, regulator_uA_show, NULL),
#define __ATTR(_name,_mode,_show,_store) { \
.attr = {.name = __stringify(_name), .mode = _mode }, \
.show = _show,\
.store = _store, \
}
If __ATTR allowed the name to be passed as a string, instead of stupidly
stringifying it, then this wouldn't be needed. So I suggest that the
#undef stands _until_ we fix this stupid macro. Or we rename the
"current" macro to something that doesn't clash with accepted scientific
and engineering terminology.
--
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/
Re: [PATCH 4/6] regulator: regulator core.
On Wed, 20 Feb 2008 17:09:11 + Liam Girdwood <[EMAIL PROTECTED]> wrote:
> This patch provides the regulator framework core. The core also provides a
> sysfs interface for userspace information.
>
> ...
>
> +
> +/* We need to undef the current macro (from include/asm/current.h) otherwise
> + * our "current" sysfs entry becomes "(get_current())".
> + */
> +#undef current
err, no ;) Please rename your stuff.
> +static DEFINE_MUTEX(list_mutex);
> +static LIST_HEAD(regulator_cdevs);
> +
> +/**
> + * struct regulator_cdev
> + *
> + * Voltage / Current regulator class device. One for each regulator.
> + */
> +struct regulator_cdev {
> + struct regulator_desc *desc;
> + int use_count;
> +
> + struct list_head list;
> + struct list_head consumer_list;
> + struct blocking_notifier_head notifier;
> + struct mutex mutex;
> + struct module *owner;
> + struct class_device cdev;
> + struct regulation_constraints *constraints;
> + struct regulator_cdev *parent; /* for tree */
> +
> + void *reg_data; /* regulator_cdev data */
> + void *vendor; /* regulator_cdev vendor extensions */
> +};
> +#define to_rcdev(cd) \
> + container_of(cd, struct regulator_cdev, cdev)
This could be a C function?
> +/*
> + * struct regulator
> + *
> + * One for each consumer device.
> + */
> +struct regulator {
> + struct device *dev;
> + struct list_head list;
> + int uA_load;
> + int uV_required;
> + int enabled;
> + struct device_attribute dev_attr;
> + struct regulator_cdev *rcdev;
> +};
> +
> +static int _regulator_is_enabled(struct regulator_cdev *rcdev);
> +static int _regulator_disable(struct regulator_cdev *rcdev);
> +static int _regulator_get_voltage(struct regulator_cdev *rcdev);
> +static int _regulator_get_current(struct regulator_cdev *rcdev);
> +static unsigned int _regulator_get_mode(struct regulator_cdev *rcdev);
> +
> +static struct regulator *get_regulator_load(struct device *dev)
> +{
> + struct regulator *regulator = NULL;
> + struct regulator_cdev *rcdev;
> +
> + list_for_each_entry(rcdev, ®ulator_cdevs, list) {
> + list_for_each_entry(regulator, &rcdev->consumer_list, list) {
> + if (regulator->dev == dev)
> + return regulator;
> + }
> + }
> + return NULL;
> +}
afacit list_mutex is not held here.
> +static int regulator_check_voltage(struct regulator_cdev *rcdev, int uV)
> +{
> + if (!rcdev->constraints) {
> + printk(KERN_ERR "%s: no constraints for %s\n", __func__,
> + rcdev->desc->name);
> + return -ENODEV;
> + }
> + if (!rcdev->constraints->valid_ops_mask & REGULATOR_CHANGE_VOLTAGE) {
> + printk(KERN_ERR "%s: operation not allowed for %s\n",
> + __func__, rcdev->desc->name);
> + return -EPERM;
> + }
> + if (uV > rcdev->constraints->max_uV ||
> + uV < rcdev->constraints->min_uV) {
> + printk(KERN_ERR "%s: invalid voltage %duV for %s\n",
> + __func__, uV, rcdev->desc->name);
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int regulator_check_current(struct regulator_cdev *rcdev, int uA)
> +{
> + if (!rcdev->constraints) {
> + printk(KERN_ERR "%s: no constraints for %s\n", __func__,
> + rcdev->desc->name);
> + return -ENODEV;
> + }
> + if (!rcdev->constraints->valid_ops_mask & REGULATOR_CHANGE_CURRENT) {
Spot the bug.
> + printk(KERN_ERR "%s: operation not allowed for %s\n",
> + __func__, rcdev->desc->name);
> + return -EPERM;
> + }
> + if (uA > rcdev->constraints->max_uA ||
> + uA < rcdev->constraints->min_uA) {
> + printk(KERN_ERR "%s: invalid current %duA for %s\n",
> + __func__, uA, rcdev->desc->name);
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int regulator_check_mode(struct regulator_cdev *rcdev, int mode)
> +{
> + if (!rcdev->constraints) {
> + printk(KERN_ERR "%s: no constraints for %s\n", __func__,
> + rcdev->desc->name);
> + return -ENODEV;
> + }
> + if (!rcdev->constraints->valid_ops_mask & REGULATOR_CHANGE_MODE) {
Here too.
> + printk(KERN_ERR "%s: operation not allowed for %s\n",
> + __func__, rcdev->desc->name);
> + return -EPERM;
> + }
> + if (!rcdev->constraints->valid_modes_mask & mode) {
And again.
> + printk(KERN_ERR "%s: invalid mode %x for %s\n",
> + __func__, mode, rcdev->desc->name);
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int regulator_check_drms(struct regulator_cdev *rcdev)
> +{
> + if (!rcdev->constraints) {
> + printk(KERN_ERR "%s: no constraints for %s\
[PATCH 4/6] regulator: regulator core.
This patch provides the regulator framework core. The core also provides a
sysfs interface for userspace information.
Signed-off-by: Liam Girdwood <[EMAIL PROTECTED]>
---
drivers/regulator/reg-core.c | 1049 ++
1 files changed, 1049 insertions(+), 0 deletions(-)
create mode 100644 drivers/regulator/reg-core.c
diff --git a/drivers/regulator/reg-core.c b/drivers/regulator/reg-core.c
new file mode 100644
index 000..43610c6
--- /dev/null
+++ b/drivers/regulator/reg-core.c
@@ -0,0 +1,1049 @@
+/*
+ * regulator.c -- Voltage/Current Regulator framework.
+ *
+ * Copyright 2007 Wolfson Microelectronics PLC.
+ *
+ * Author: Liam Girdwood <[EMAIL PROTECTED]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define REGULATOR_VERSION "0.3"
+
+/* We need to undef the current macro (from include/asm/current.h) otherwise
+ * our "current" sysfs entry becomes "(get_current())".
+ */
+#undef current
+
+static DEFINE_MUTEX(list_mutex);
+static LIST_HEAD(regulator_cdevs);
+
+/**
+ * struct regulator_cdev
+ *
+ * Voltage / Current regulator class device. One for each regulator.
+ */
+struct regulator_cdev {
+ struct regulator_desc *desc;
+ int use_count;
+
+ struct list_head list;
+ struct list_head consumer_list;
+ struct blocking_notifier_head notifier;
+ struct mutex mutex;
+ struct module *owner;
+ struct class_device cdev;
+ struct regulation_constraints *constraints;
+ struct regulator_cdev *parent; /* for tree */
+
+ void *reg_data; /* regulator_cdev data */
+ void *vendor; /* regulator_cdev vendor extensions */
+};
+#define to_rcdev(cd) \
+ container_of(cd, struct regulator_cdev, cdev)
+
+/*
+ * struct regulator
+ *
+ * One for each consumer device.
+ */
+struct regulator {
+ struct device *dev;
+ struct list_head list;
+ int uA_load;
+ int uV_required;
+ int enabled;
+ struct device_attribute dev_attr;
+ struct regulator_cdev *rcdev;
+};
+
+static int _regulator_is_enabled(struct regulator_cdev *rcdev);
+static int _regulator_disable(struct regulator_cdev *rcdev);
+static int _regulator_get_voltage(struct regulator_cdev *rcdev);
+static int _regulator_get_current(struct regulator_cdev *rcdev);
+static unsigned int _regulator_get_mode(struct regulator_cdev *rcdev);
+
+static struct regulator *get_regulator_load(struct device *dev)
+{
+ struct regulator *regulator = NULL;
+ struct regulator_cdev *rcdev;
+
+ list_for_each_entry(rcdev, ®ulator_cdevs, list) {
+ list_for_each_entry(regulator, &rcdev->consumer_list, list) {
+ if (regulator->dev == dev)
+ return regulator;
+ }
+ }
+ return NULL;
+}
+
+static int regulator_check_voltage(struct regulator_cdev *rcdev, int uV)
+{
+ if (!rcdev->constraints) {
+ printk(KERN_ERR "%s: no constraints for %s\n", __func__,
+ rcdev->desc->name);
+ return -ENODEV;
+ }
+ if (!rcdev->constraints->valid_ops_mask & REGULATOR_CHANGE_VOLTAGE) {
+ printk(KERN_ERR "%s: operation not allowed for %s\n",
+ __func__, rcdev->desc->name);
+ return -EPERM;
+ }
+ if (uV > rcdev->constraints->max_uV ||
+ uV < rcdev->constraints->min_uV) {
+ printk(KERN_ERR "%s: invalid voltage %duV for %s\n",
+ __func__, uV, rcdev->desc->name);
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int regulator_check_current(struct regulator_cdev *rcdev, int uA)
+{
+ if (!rcdev->constraints) {
+ printk(KERN_ERR "%s: no constraints for %s\n", __func__,
+ rcdev->desc->name);
+ return -ENODEV;
+ }
+ if (!rcdev->constraints->valid_ops_mask & REGULATOR_CHANGE_CURRENT) {
+ printk(KERN_ERR "%s: operation not allowed for %s\n",
+ __func__, rcdev->desc->name);
+ return -EPERM;
+ }
+ if (uA > rcdev->constraints->max_uA ||
+ uA < rcdev->constraints->min_uA) {
+ printk(KERN_ERR "%s: invalid current %duA for %s\n",
+ __func__, uA, rcdev->desc->name);
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int regulator_check_mode(struct regulator_cdev *rcdev, int mode)
+{
+ if (!rcdev->constraints) {
+ printk(KERN_ERR "%s: no constraints for %s\n", __func__,
+ rcdev->desc->name);
+ return -ENODEV;
+
