Re: [PATCH v3 2/3] pmbus: add regulator support
From: atull
Date: Wed Sep 24 2014 - 17:14:15 EST
On Wed, 24 Sep 2014, Dinh Nguyen wrote:
> Hi Alan,
>
> On 9/24/14, 12:57 PM, atull@xxxxxxxxxxxxxxxxxxxxx wrote:
> > From: Alan Tull <atull@xxxxxxxxxxxxxxxxxxxxx>
> >
> > Add support for simple on/off control of each channel.
> >
> > To add regulator support, the pmbus part driver needs to add
> > regulator_desc information, of_regulator_match information,
> > and number of regulators to its pmbus_driver_info struct.
> >
> > regulator_desc can be declared using default macro for a
> > regulator (PMBUS_REGULATOR) that is in pmbus.h
> >
> > The regulator_init_data can be intialized from either
> > platform data or the device tree.
> >
> > Signed-off-by: Alan Tull <atull@xxxxxxxxxxxxxxxxxxxxx>
> >
> > v2: Remove '#include <linux/regulator/machine.h>'
> > Only one regulator per pmbus device
> > Get regulator_init_data from pdata or device tree
> >
> > v3: Support multiple regulators for each chip
> > Move most code to pmbus_core.c
> > fixed values for on/off
> > ---
> > drivers/hwmon/pmbus/pmbus.h | 27 ++++++++
> > drivers/hwmon/pmbus/pmbus_core.c | 133 ++++++++++++++++++++++++++++++++++++++
> > include/linux/i2c/pmbus.h | 4 ++
> > 3 files changed, 164 insertions(+)
> >
> > diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
> > index fa9beb3..74aa382 100644
> > --- a/drivers/hwmon/pmbus/pmbus.h
> > +++ b/drivers/hwmon/pmbus/pmbus.h
> > @@ -19,6 +19,9 @@
> > * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> > */
> >
> > +#include <linux/regulator/driver.h>
> > +#include <linux/regulator/of_regulator.h>
> > +
> > #ifndef PMBUS_H
> > #define PMBUS_H
> >
> > @@ -186,6 +189,12 @@
> > #define PMBUS_VIRT_STATUS_VMON (PMBUS_VIRT_BASE + 35)
> >
> > /*
> > + * OPERATION
> > + */
> > +#define PB_OPERATION_CONTROL_ON (1<<7)
> > +#define PB_OPERATION_CONTROL_SEQ_OFF (1<<6)
> > +
> > +/*
> > * CAPABILITY
> > */
> > #define PB_CAPABILITY_SMBALERT (1<<4)
> > @@ -365,8 +374,26 @@ struct pmbus_driver_info {
> > */
> > int (*identify)(struct i2c_client *client,
> > struct pmbus_driver_info *info);
> > +
> > + /* Regulator functionality, if supported by this chip driver. */
> > + int num_regulators;
> > + const struct regulator_desc *reg_desc;
> > + struct of_regulator_match *reg_matches;
> > };
> >
> > +/* Regulator ops */
> > +
> > +extern struct regulator_ops pmbus_regulator_regulator_ops;
> > +
> > +/* Macro for filling in array of struct regulator_desc */
> > +#define PMBUS_REGULATOR(_name, _id) \
> > + [_id] = { \
> > + .name = (_name # _id), \
> > + .id = (_id), \
> > + .ops = &pmbus_regulator_regulator_ops, \
> > + .owner = THIS_MODULE, \
> > + }
> > +
> > /* Function declarations */
> >
> > void pmbus_clear_cache(struct i2c_client *client);
> > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> > index d6c3701..9ab8bd4 100644
> > --- a/drivers/hwmon/pmbus/pmbus_core.c
> > +++ b/drivers/hwmon/pmbus/pmbus_core.c
> > @@ -29,6 +29,9 @@
> > #include <linux/hwmon-sysfs.h>
> > #include <linux/jiffies.h>
> > #include <linux/i2c/pmbus.h>
> > +#include <linux/regulator/of_regulator.h>
> > +#include <linux/regulator/driver.h>
> > +#include <linux/regulator/machine.h>
> > #include "pmbus.h"
> >
> > /*
> > @@ -1758,6 +1761,125 @@ static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,
> > return 0;
> > }
> >
> > +#if IS_ENABLED(CONFIG_REGULATOR)
> > +static int pmbus_regulator_is_enabled(struct regulator_dev *rdev)
> > +{
> > + struct device *dev = rdev_get_dev(rdev);
> > + struct i2c_client *client = to_i2c_client(dev->parent);
> > + u8 page = rdev_get_id(rdev);
> > + int ret;
> > +
> > + ret = pmbus_read_byte_data(client, page, PMBUS_OPERATION);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return !!(ret & PB_OPERATION_CONTROL_ON);
> > +}
> > +
> > +static int _pmbus_regulator_enable(struct regulator_dev *rdev, bool enable)
> > +{
> > + struct device *dev = rdev_get_dev(rdev);
> > + struct i2c_client *client = to_i2c_client(dev->parent);
> > + u8 val, page = rdev_get_id(rdev);
> > +
> > + if (enable)
> > + val = PB_OPERATION_CONTROL_ON;
> > + else
> > + val = 0;
> > +
> > + return pmbus_update_byte_data(client, page, PMBUS_OPERATION,
> > + PB_OPERATION_CONTROL_ON, val);
> > +}
> > +
> > +static int pmbus_regulator_enable(struct regulator_dev *rdev)
> > +{
> > + return _pmbus_regulator_enable(rdev, 1);
> > +}
> > +
> > +static int pmbus_regulator_disable(struct regulator_dev *rdev)
> > +{
> > + return _pmbus_regulator_enable(rdev, 0);
> > +}
> > +
> > +struct regulator_ops pmbus_regulator_regulator_ops = {
> > + .enable = pmbus_regulator_enable,
> > + .disable = pmbus_regulator_disable,
> > + .is_enabled = pmbus_regulator_is_enabled,
> > +};
> > +EXPORT_SYMBOL_GPL(pmbus_regulator_regulator_ops);
> > +
> > +#if IS_ENABLED(CONFIG_OF)
> > +static int pmbus_regulator_parse_dt(struct device *dev,
> > + const struct pmbus_driver_info *info)
> > +{
> > + struct device_node *np_regulators;
> > + int ret;
> > +
> > + if (!info->num_regulators)
> > + return 0;
> > +
> > + if (!info->reg_matches || !info->reg_desc)
> > + return -EINVAL;
> > +
> > + np_regulators = of_get_child_by_name(dev->of_node, "regulators");
> > + if (!np_regulators)
> > + return 0;
> > +
> > + ret = of_regulator_match(dev, np_regulators, info->reg_matches,
> > + info->num_regulators);
> > + of_node_put(np_regulators);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +#else
> > +static int pmbus_regulator_parse_dt(struct device *dev,
> > + const struct pmbus_driver_info *info)
> > +{
> > + return 0;
> > +}
> > +#endif
>
> This breaks the build if CONFIG_REGULATOR is not enabled:
>
> drivers/hwmon/pmbus/pmbus_core.c: In function âpmbus_do_probeâ:
> drivers/hwmon/pmbus/pmbus_core.c:1894:2: error: implicit declaration of
> function âpmbus_regulator_parse_dtâ [-Werror=implicit-function-declaration]
> cc1: some warnings being treated as errors
>
> Dinh
>
Hi Dinh,
Thanks, I will fix it. I had that working earlier, but obviously broke it again...
Alan