On Fri, Jun 24, 2016 at 3:20 AM, <meg...@megous.com> wrote: > From: Ondrej Jirman <meg...@megous.com> > > SY8106A is I2C attached single output voltage regulator > made by Silergy. > > Signed-off-by: Ondrej Jirman <meg...@megous.com> > --- > drivers/regulator/Kconfig | 8 +- > drivers/regulator/Makefile | 2 +- > drivers/regulator/sy8106a-regulator.c | 153 > ++++++++++++++++++++++++++++++++++ > 3 files changed, 161 insertions(+), 2 deletions(-) > create mode 100644 drivers/regulator/sy8106a-regulator.c > > diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig > index 144cbf5..fc3fae2 100644 > --- a/drivers/regulator/Kconfig > +++ b/drivers/regulator/Kconfig > @@ -860,5 +860,11 @@ config REGULATOR_WM8994 > This driver provides support for the voltage regulators on the > WM8994 CODEC. > > -endif > +config REGULATOR_SY8106A > + tristate "Silergy SY8106A" > + depends on I2C
Maybe you should also depend on OF since the driver is going to crippled without any constraints set, or (OF || COMPILE_TEST) if you want some compile test coverage. > + select REGMAP_I2C > + help > + This driver provides support for the voltage regulator SY8106A. > > +endif > diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile > index 85a1d44..f382095 100644 > --- a/drivers/regulator/Makefile > +++ b/drivers/regulator/Makefile > @@ -110,6 +110,6 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o > obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o > obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o > obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o > - > +obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o Follow the existing ordering in the Makefile. > > ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG > diff --git a/drivers/regulator/sy8106a-regulator.c > b/drivers/regulator/sy8106a-regulator.c > new file mode 100644 > index 0000000..34bd69c > --- /dev/null > +++ b/drivers/regulator/sy8106a-regulator.c > @@ -0,0 +1,153 @@ > +/* > + * sy8106a-regulator.c - Regulator device driver for SY8106A > + * > + * Copyright (C) 2016 Ondřej Jirman <meg...@megous.com> > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Library General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Library General Public License for more details. > + * > + * You should have received a copy of the GNU Library General Public > + * License along with this library; if not, write to the > + * Free Software Foundation, Inc., 51 Franklin St, Fifth Floor, > + * Boston, MA 02110-1301, USA. > + */ > + > +#include <linux/err.h> > +#include <linux/i2c.h> > +#include <linux/module.h> > +#include <linux/init.h> Do you need this one? > +#include <linux/regulator/driver.h> > +#include <linux/regulator/machine.h> And this one? > +#include <linux/regulator/of_regulator.h> > +#include <linux/regmap.h> Sort alphabetically please. > + > +#define SY8106A_REG_VOUT1_SEL 0x01 > +#define SY8106A_REG_VOUT_COM 0x02 > +#define SY8106A_REG_VOUT1_SEL_MASK 0x7f > +#define SY8106A_DISABLE_REG 0x01 BIT(0) would be clearer. > + > +struct sy8106a { > + struct regulator_dev *rdev; > + struct regmap *regmap; > +}; > + > +static const struct regmap_config sy8106a_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > +}; > + > +static int sy8106a_set_voltage_sel(struct regulator_dev *rdev, unsigned sel) > +{ > + return regmap_update_bits(rdev->regmap, rdev->desc->vsel_reg, > + 0xff, sel | 0x80); Can you use .apply_bit / .apply_reg with regulator_set_voltage_sel_regmap? > +} > + > +static const struct regulator_ops sy8106a_ops = { > + .is_enabled = regulator_is_enabled_regmap, > + .set_voltage_sel = sy8106a_set_voltage_sel, > + .set_voltage_time_sel = regulator_set_voltage_time_sel, > + .get_voltage_sel = regulator_get_voltage_sel_regmap, > + .list_voltage = regulator_list_voltage_linear, > +}; > + > +/* Default limits measured in millivolts and milliamps */ > +#define SY8106A_MIN_MV 680 > +#define SY8106A_MAX_MV 1950 > +#define SY8106A_STEP_MV 10 > + > +static const struct regulator_desc sy8106a_reg = { > + .name = "SY8106A", > + .id = 0, > + .ops = &sy8106a_ops, > + .type = REGULATOR_VOLTAGE, > + .n_voltages = ((SY8106A_MAX_MV - SY8106A_MIN_MV) / SY8106A_STEP_MV) + > 1, > + .min_uV = (SY8106A_MIN_MV * 1000), > + .uV_step = (SY8106A_STEP_MV * 1000), > + .vsel_reg = SY8106A_REG_VOUT1_SEL, > + .vsel_mask = SY8106A_REG_VOUT1_SEL_MASK, > + .enable_reg = SY8106A_REG_VOUT_COM, > + .enable_mask = SY8106A_DISABLE_REG, > + .disable_val = SY8106A_DISABLE_REG, > + .enable_is_inverted = 1, > + .owner = THIS_MODULE, > +}; > + > +/* > + * I2C driver interface functions > + */ > +static int sy8106a_i2c_probe(struct i2c_client *i2c, > + const struct i2c_device_id *id) > +{ > + struct sy8106a *chip; > + struct device *dev = &i2c->dev; > + struct regulator_dev *rdev = NULL; > + struct regulator_config config = { }; > + unsigned int selector; > + int error; > + > + chip = devm_kzalloc(&i2c->dev, sizeof(struct sy8106a), GFP_KERNEL); > + if (!chip) > + return -ENOMEM; > + > + chip->regmap = devm_regmap_init_i2c(i2c, &sy8106a_regmap_config); > + if (IS_ERR(chip->regmap)) { > + error = PTR_ERR(chip->regmap); > + dev_err(&i2c->dev, "Failed to allocate register map: %d\n", > + error); > + return error; > + } > + > + config.dev = &i2c->dev; > + config.init_data = of_get_regulator_init_data(dev, dev->of_node, > &sy8106a_reg); Check for possible failures? > + config.driver_data = chip; > + config.regmap = chip->regmap; > + config.of_node = dev->of_node; > + > + /* Probe regulator */ > + error = regmap_read(chip->regmap, SY8106A_REG_VOUT1_SEL, &selector); > + dev_info(&i2c->dev, "SY8106A voltage at boot: %u mV\n", > SY8106A_MIN_MV + SY8106A_STEP_MV * (selector & SY8106A_REG_VOUT1_SEL_MASK)); > + if (error) { > + dev_err(&i2c->dev, "Failed to read voltage at probe time: > %d\n", error); > + return error; > + } > + > + rdev = devm_regulator_register(&i2c->dev, &sy8106a_reg, &config); > + if (IS_ERR(rdev)) { > + dev_err(&i2c->dev, "Failed to register SY8106A regulator\n"); > + return PTR_ERR(rdev); > + } > + > + chip->rdev = rdev; > + > + i2c_set_clientdata(i2c, chip); > + > + return 0; > +} > + > +static const struct i2c_device_id sy8106a_i2c_id[] = { > + {"sy8106a", 0}, > + {}, > +}; > + Extra line. > +MODULE_DEVICE_TABLE(i2c, sy8106a_i2c_id); IMHO probing explicitly through DT (of_device_id) would be better. Regards ChenYu > + > +static struct i2c_driver sy8106a_regulator_driver = { > + .driver = { > + .name = "sy8106a", > + }, > + .probe = sy8106a_i2c_probe, > + .id_table = sy8106a_i2c_id, > +}; > + > +module_i2c_driver(sy8106a_regulator_driver); > + > +MODULE_AUTHOR("Ondřej Jirman <meg...@megous.com>"); > +MODULE_DESCRIPTION("Regulator device driver for Silergy SY8106A"); > +MODULE_LICENSE("GPL v2"); > -- > 2.9.0 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel