* Add ltc3562 voltage regulator driver @ 2014-10-29 8:15 Mike Looijmans 2014-10-29 8:16 ` [PATCH] " Mike Looijmans 2014-10-29 10:03 ` Mark Brown 0 siblings, 2 replies; 31+ messages in thread From: Mike Looijmans @ 2014-10-29 8:15 UTC (permalink / raw) To: lgirdwood, broonie; +Cc: linux-kernel This patch adds the LTC3562 I2C controlled voltage regulator to the kernel. The driver was tested and developed on a Topic Miami board where this chip supplies the IO voltages for the FPGA pins. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH] Add ltc3562 voltage regulator driver 2014-10-29 8:15 Add ltc3562 voltage regulator driver Mike Looijmans @ 2014-10-29 8:16 ` Mike Looijmans 2014-10-29 12:30 ` Mark Brown 2014-10-30 11:26 ` [PATCH v2] " Mike Looijmans 2014-10-29 10:03 ` Mark Brown 1 sibling, 2 replies; 31+ messages in thread From: Mike Looijmans @ 2014-10-29 8:16 UTC (permalink / raw) To: lgirdwood, broonie; +Cc: linux-kernel, Mike Looijmans The ltc3562 is an I2C controlled regulator supporting 4 independent outputs. Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl> --- drivers/regulator/Kconfig | 7 + drivers/regulator/Makefile | 1 + drivers/regulator/ltc3562.c | 387 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 395 insertions(+) create mode 100644 drivers/regulator/ltc3562.c diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig index 55d7b7b..4012601 100644 --- a/drivers/regulator/Kconfig +++ b/drivers/regulator/Kconfig @@ -300,6 +300,13 @@ config REGULATOR_LP8788 help This driver supports LP8788 voltage regulator chip. +config REGULATOR_LTC3562 + tristate "LCT3562 quad output voltage regulator" + depends on I2C + help + This enables support for the LCT3562 quad output voltage regulator + controlled via I2C. + config REGULATOR_LTC3589 tristate "LTC3589 8-output voltage regulator" depends on I2C diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile index 1029ed3..9a15146 100644 --- a/drivers/regulator/Makefile +++ b/drivers/regulator/Makefile @@ -41,6 +41,7 @@ obj-$(CONFIG_REGULATOR_LP872X) += lp872x.o obj-$(CONFIG_REGULATOR_LP8788) += lp8788-buck.o obj-$(CONFIG_REGULATOR_LP8788) += lp8788-ldo.o obj-$(CONFIG_REGULATOR_LP8755) += lp8755.o +obj-$(CONFIG_REGULATOR_LTC3562) += ltc3562.o obj-$(CONFIG_REGULATOR_LTC3589) += ltc3589.o obj-$(CONFIG_REGULATOR_MAX14577) += max14577.o obj-$(CONFIG_REGULATOR_MAX1586) += max1586.o diff --git a/drivers/regulator/ltc3562.c b/drivers/regulator/ltc3562.c new file mode 100644 index 0000000..be111bc --- /dev/null +++ b/drivers/regulator/ltc3562.c @@ -0,0 +1,387 @@ +/* + * Regulator driver for Linear Technology LTC3562 + * + * Copyright (C) 2014 Topic Embedded Products + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/bug.h> +#include <linux/err.h> +#include <linux/i2c.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/regulator/driver.h> +#include <linux/regulator/machine.h> +#include <linux/slab.h> +#include <linux/regulator/of_regulator.h> + +#define LTC3562_NUM_REGULATORS 4 + +#define REGULATOR_TYPE_A_NUM_VOLTAGES 16 +#define REGULATOR_TYPE_A_MIN_UV 425000 +#define REGULATOR_TYPE_A_UV_STEP 25000 + +#define REGULATOR_TYPE_B_NUM_VOLTAGES 128 +#define REGULATOR_TYPE_B_MIN_UV 600000 +#define REGULATOR_TYPE_B_UV_STEP 25000 + +struct ltc3562_status { + u8 shadow_addr; /* Shadow register of address byte */ + u8 shadow_data; /* Shadow register of data byte */ + u8 voltage_set; /* Check if voltage has been set before */ +}; + +struct ltc3562 { + struct device *dev; + struct i2c_client *i2c; + struct mutex mutex; + struct regulator_dev *rdev[LTC3562_NUM_REGULATORS]; + struct ltc3562_status rstatus[LTC3562_NUM_REGULATORS]; +}; + +static int ltc3562_write(struct i2c_client *i2c, u8 reg_a, u8 reg_b); +static int ltc3562_dummy_write(struct i2c_client *i2c); + +#define LTC3562_R400B_ID 0x00 +#define LTC3562_R600B_ID 0x01 +#define LTC3562_R400A_ID 0x02 +#define LTC3562_R600A_ID 0x03 + +#define OPERATING_MODE_MASK 0x03 + +#define PROGRAM_R400B_MASK 0x10 +#define PROGRAM_R600B_MASK 0x20 +#define PROGRAM_R400A_MASK 0x40 +#define PROGRAM_R600A_MASK 0x80 + +#define OPERATING_MODE_PULSE_SKIP 0x00 +#define OPERATING_MODE_LDO 0x01 +#define OPERATING_MODE_FOCED_BURST 0x02 +#define OPERATING_MODE_BURST 0x03 + +#define REGULATOR_ENABLE_BIT 0x80 + + +static int ltc3562_regulator_enable(struct regulator_dev *dev) +{ + int ret, v_index; + unsigned int v_default; + struct ltc3562 *ltc3562 = rdev_get_drvdata(dev); + int id = rdev_get_id(dev); + struct ltc3562_status *status = <c3562->rstatus[id]; + + mutex_lock(<c3562->mutex); + + if (!status->voltage_set) { + if (of_property_read_u32(dev->dev.of_node, + "ltc3562-default-voltage", &v_default) == 0) { + v_index = dev->desc->ops->map_voltage( + dev, v_default, v_default); + if (v_index > 0) + status->shadow_data = + v_index & (~REGULATOR_ENABLE_BIT); + status->voltage_set = 1; + } + } + + status->shadow_data |= REGULATOR_ENABLE_BIT; + + ret = ltc3562_write(ltc3562->i2c, + status->shadow_addr | PROGRAM_R400B_MASK << id, + status->shadow_data); + + mutex_unlock(<c3562->mutex); + + return ret; +} + +static int ltc3562_regulator_disable(struct regulator_dev *dev) +{ + int ret; + struct ltc3562 *ltc3562 = rdev_get_drvdata(dev); + int id = rdev_get_id(dev); + struct ltc3562_status *status = <c3562->rstatus[id]; + + mutex_lock(<c3562->mutex); + + status->shadow_data &= ~(REGULATOR_ENABLE_BIT); + + ret = ltc3562_write(ltc3562->i2c, + status->shadow_addr, status->shadow_data); + + mutex_unlock(<c3562->mutex); + + return ret; +} + +static int ltc3562_regulator_is_enabled(struct regulator_dev *dev) +{ + int ret; + struct ltc3562 *ltc3562 = rdev_get_drvdata(dev); + int id = rdev_get_id(dev); + struct ltc3562_status *status = <c3562->rstatus[id]; + + mutex_lock(<c3562->mutex); + + ret = (status->shadow_data & REGULATOR_ENABLE_BIT) ? 1 : 0; + + mutex_unlock(<c3562->mutex); + + return ret; +} + +static int ltc3562_set_voltage_sel(struct regulator_dev *dev, + unsigned int selector) +{ + int ret; + struct ltc3562 *ltc3562 = rdev_get_drvdata(dev); + int id = rdev_get_id(dev); + struct ltc3562_status *status = <c3562->rstatus[id]; + + mutex_lock(<c3562->mutex); + + status->shadow_data = + (status->shadow_data & REGULATOR_ENABLE_BIT) | selector; + status->voltage_set = 1; + + ret = ltc3562_write(ltc3562->i2c, + status->shadow_addr, status->shadow_data); + + mutex_unlock(<c3562->mutex); + + return ret; +} + +static int ltc3562_get_voltage_sel(struct regulator_dev *dev) +{ + int ret; + struct ltc3562 *ltc3562 = rdev_get_drvdata(dev); + int id = rdev_get_id(dev); + struct ltc3562_status *status = <c3562->rstatus[id]; + + mutex_lock(<c3562->mutex); + + ret = status->shadow_data & ~(REGULATOR_ENABLE_BIT); + + mutex_unlock(<c3562->mutex); + + return ret; +} + +static struct regulator_ops ltc3562_regulator_ops = { + .is_enabled = ltc3562_regulator_is_enabled, + .enable = ltc3562_regulator_enable, + .disable = ltc3562_regulator_disable, + .set_voltage_sel = ltc3562_set_voltage_sel, + .get_voltage_sel = ltc3562_get_voltage_sel, + .list_voltage = regulator_list_voltage_linear, + .map_voltage = regulator_map_voltage_linear, +}; + +static struct regulator_desc ltc3562_regulators[] = { + { + .name = "R400B", + .id = LTC3562_R400B_ID, + .ops = <c3562_regulator_ops, + .min_uV = REGULATOR_TYPE_B_MIN_UV, + .uV_step = REGULATOR_TYPE_B_UV_STEP, + .n_voltages = REGULATOR_TYPE_B_NUM_VOLTAGES, + .type = REGULATOR_VOLTAGE, + .owner = THIS_MODULE, + }, + { + .name = "R600B", + .id = LTC3562_R600B_ID, + .ops = <c3562_regulator_ops, + .min_uV = REGULATOR_TYPE_B_MIN_UV, + .uV_step = REGULATOR_TYPE_B_UV_STEP, + .n_voltages = REGULATOR_TYPE_B_NUM_VOLTAGES, + .type = REGULATOR_VOLTAGE, + .owner = THIS_MODULE, + }, + { + .name = "R400A", + .id = LTC3562_R400A_ID, + .ops = <c3562_regulator_ops, + .min_uV = REGULATOR_TYPE_A_MIN_UV, + .uV_step = REGULATOR_TYPE_A_UV_STEP, + .n_voltages = REGULATOR_TYPE_A_NUM_VOLTAGES, + .type = REGULATOR_VOLTAGE, + .owner = THIS_MODULE, + }, + { + .name = "R600A", + .id = LTC3562_R600A_ID, + .ops = <c3562_regulator_ops, + .min_uV = REGULATOR_TYPE_A_MIN_UV, + .uV_step = REGULATOR_TYPE_A_UV_STEP, + .n_voltages = REGULATOR_TYPE_A_NUM_VOLTAGES, + .type = REGULATOR_VOLTAGE, + .owner = THIS_MODULE, + }, +}; + + +static int ltc3562_write(struct i2c_client *i2c, u8 reg_a, u8 reg_b) +{ + return i2c_smbus_write_byte_data(i2c, reg_a, reg_b); +} + +/* Write dummy data to detect presence of physical device */ +static int ltc3562_dummy_write(struct i2c_client *i2c) +{ + return ltc3562_write(i2c, 0, 0); +} + + +static int ltc3562_regulators_unregister(struct ltc3562 *ltc3562, + int num_regulators) +{ + while (--num_regulators >= 0) { + pr_debug("Unregistering regulator: %s\n", + ltc3562->rdev[num_regulators]->desc->name); + regulator_unregister(ltc3562->rdev[num_regulators]); + } + return 0; +} + +static int ltc3562_i2c_probe(struct i2c_client *i2c, + const struct i2c_device_id *id) +{ + int i, error; + unsigned int r1, r2; + unsigned long uV; + struct ltc3562 *ltc3562; + struct regulator_config rconfig = { }; + struct device_node *np_regulators, *np; + struct device_node *np_child; + + ltc3562 = devm_kzalloc(&i2c->dev, + sizeof(struct ltc3562), GFP_KERNEL); + if (ltc3562 == NULL) + return -ENOMEM; + + ltc3562->i2c = i2c; + + mutex_init(<c3562->mutex); + + if (ltc3562_dummy_write(i2c) < 0) { + dev_err(&i2c->dev, + "Could not find device LTC3562 on i2c bus\n"); + error = -ENODEV; + goto lbl_exit; + } + + np = of_node_get(i2c->dev.of_node); + np_regulators = of_get_child_by_name(np, "regulators"); + + if (np_regulators == NULL) { + dev_err(&i2c->dev, "Could not find regulators node\n"); + error = -EINVAL; + goto lbl_exit; + } + + for (i = 0; i < LTC3562_NUM_REGULATORS; ++i) { + np_child = of_get_child_by_name(np_regulators, + ltc3562_regulators[i].name); + if (np_child == NULL) { + dev_err(&i2c->dev, + "Could not find regulator data for %s in devicetree\n", + ltc3562_regulators[i].name); + error = -EINVAL; + goto lbl_cleanup; + } + + if ((ltc3562_regulators[i].id == LTC3562_R400A_ID) || + (ltc3562_regulators[i].id == LTC3562_R600A_ID)) { + /* If regulator is A type get resistor values */ + error = of_property_read_u32(np_child, + "ltc3562-A-r1", &r1); + if (error) { + dev_err(&i2c->dev, + "Resistor value ltc3562-A-r1 for A-Type regulator %s not defined\n", + ltc3562_regulators[i].name); + goto lbl_cleanup; + } + + error = of_property_read_u32(np_child, + "ltc3562-A-r2", &r2); + if (error) { + dev_err(&i2c->dev, + "Resistor value ltc3562-A-r2 for A-Type regulator %s not defined\n", + ltc3562_regulators[i].name); + goto lbl_cleanup; + } + uV = ltc3562_regulators[i].min_uV / 1000; + ltc3562_regulators[i].min_uV = + (uV + mult_frac(uV, r1, r2)) * 1000; + + uV = ltc3562_regulators[i].uV_step / 1000; + ltc3562_regulators[i].uV_step = + (uV + mult_frac(uV, r1, r2)) * 1000; + } + + rconfig.dev = &i2c->dev; + rconfig.init_data = + of_get_regulator_init_data(&i2c->dev, np_child); + rconfig.driver_data = ltc3562; + rconfig.of_node = np_child; + + ltc3562->rstatus[i].voltage_set = 0; + + ltc3562->rdev[i] = + regulator_register(<c3562_regulators[i], &rconfig); + + if (IS_ERR(ltc3562->rdev[i])) { + error = PTR_ERR(ltc3562->rdev[i]); + dev_err(&i2c->dev, + "could not register regulator, Error %d\n", + error); + goto lbl_cleanup; + } + } + + i2c_set_clientdata(i2c, ltc3562); + dev_dbg(&i2c->dev, "LTC3562 Driver loaded.\n"); + return 0; + +lbl_cleanup: + ltc3562_regulators_unregister(ltc3562, i); +lbl_exit: + return error; +} + +static int ltc3562_i2c_remove(struct i2c_client *i2c) +{ + struct ltc3562 *ltc3562 = i2c_get_clientdata(i2c); + + ltc3562_regulators_unregister(ltc3562, LTC3562_NUM_REGULATORS); + + return 0; +} + +static const struct i2c_device_id ltc3562_i2c_id[] = { + { "ltc3562", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, ltc3562_i2c_id); + +static struct i2c_driver ltc3562_i2c_driver = { + .driver = { + .name = "LTC3562", + .owner = THIS_MODULE, + }, + .probe = ltc3562_i2c_probe, + .remove = ltc3562_i2c_remove, + .id_table = ltc3562_i2c_id, +}; + +module_i2c_driver(ltc3562_i2c_driver); + +MODULE_DESCRIPTION("LTC3562 Regulator Driver"); +MODULE_AUTHOR("auryn.verwegen@topic.nl"); +MODULE_AUTHOR("mike.looijmans@topic.nl"); +MODULE_LICENSE("GPL v2"); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH] Add ltc3562 voltage regulator driver 2014-10-29 8:16 ` [PATCH] " Mike Looijmans @ 2014-10-29 12:30 ` Mark Brown 2014-10-30 6:47 ` Mike Looijmans 2014-10-30 11:26 ` [PATCH v2] " Mike Looijmans 1 sibling, 1 reply; 31+ messages in thread From: Mark Brown @ 2014-10-29 12:30 UTC (permalink / raw) To: Mike Looijmans; +Cc: lgirdwood, linux-kernel [-- Attachment #1: Type: text/plain, Size: 511 bytes --] On Wed, Oct 29, 2014 at 09:16:00AM +0100, Mike Looijmans wrote: > + if (!status->voltage_set) { > + if (of_property_read_u32(dev->dev.of_node, > + "ltc3562-default-voltage", &v_default) == 0) { A couple of problems here: - This contains DT code but no DT bindings documentation; the binding documentation is mandatory for any new bindings. - It's not obvious why a "default voltage" property would be device specific - what is this for and why is it being added in an individual device driver? [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Add ltc3562 voltage regulator driver 2014-10-29 12:30 ` Mark Brown @ 2014-10-30 6:47 ` Mike Looijmans 2014-10-30 10:15 ` Mark Brown 0 siblings, 1 reply; 31+ messages in thread From: Mike Looijmans @ 2014-10-30 6:47 UTC (permalink / raw) To: Mark Brown; +Cc: lgirdwood, linux-kernel On 10/29/2014 01:30 PM, Mark Brown wrote: > On Wed, Oct 29, 2014 at 09:16:00AM +0100, Mike Looijmans wrote: > >> + if (!status->voltage_set) { >> + if (of_property_read_u32(dev->dev.of_node, >> + "ltc3562-default-voltage", &v_default) == 0) { > > A couple of problems here: > > - This contains DT code but no DT bindings documentation; the binding > documentation is mandatory for any new bindings. Should I submit a new patch to add the DT bindings and CC to the dt mailing list, or do you want it included in this patch? I was basically waiting for feedback on the driver first. > - It's not obvious why a "default voltage" property would be device > specific - what is this for and why is it being added in an > individual device driver? As for the "why" part: The default is used when there isn't a consumer available. It'll typically be combined with "always-on". When the output is used for say an MMC card, the consumer will ask for a specific voltage. When used to regulate the IO voltage of an FPGA bank (as is the case on our board), there isn't a single consumer to request a particular voltage, but the min and max will represent the range that the IO banks support (e.g. 1v8..3v3 or 1v2..1v8), because selecting voltages outside this range may damage the FPGA hardware. This allows a devicetree overlay to specify a different value for a different board or application, while still being able to restrict the range to what the hardware connected to the regulator supports. The "default voltage" may not be device specific, but regulator.txt doesn't have a property that describes what we needed here, because we can't get away with specifying min=max as fixed-regulator does. We also cannot assume that default=min or default=max, because that might harm hardware connected to the other side of the IO bank. I noticed after rebasing to mainline that there is now also a driver for a similar chip (similar in function, that is), the ltc3589, which requires similar properties like the resistor dividers. I think it would be good to use similar names in the ltc3562 driver. I'll do that first and submit a v2 patch, along with the DT description. Thank you for your feedback, Mike. Met vriendelijke groet / kind regards, Mike Looijmans System Expert TOPIC Embedded Systems Eindhovenseweg 32-C, NL-5683 KH Best Postbus 440, NL-5680 AK Best Telefoon: (+31) (0) 499 33 69 79 Telefax: (+31) (0) 499 33 69 70 E-mail: mike.looijmans@topic.nl Website: www.topic.nl Please consider the environment before printing this e-mail Topic zoekt gedreven (embedded) software specialisten! http://topic.nl/vacatures/topic-zoekt-software-engineers/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Add ltc3562 voltage regulator driver 2014-10-30 6:47 ` Mike Looijmans @ 2014-10-30 10:15 ` Mark Brown 2014-10-30 10:29 ` Mike Looijmans 0 siblings, 1 reply; 31+ messages in thread From: Mark Brown @ 2014-10-30 10:15 UTC (permalink / raw) To: Mike Looijmans; +Cc: lgirdwood, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1219 bytes --] On Thu, Oct 30, 2014 at 07:47:44AM +0100, Mike Looijmans wrote: > On 10/29/2014 01:30 PM, Mark Brown wrote: > >A couple of problems here: > > - This contains DT code but no DT bindings documentation; the binding > > documentation is mandatory for any new bindings. > Should I submit a new patch to add the DT bindings and CC to the dt mailing > list, or do you want it included in this patch? I was basically waiting for > feedback on the driver first. I'm not going to review a driver with bindings without those bindings being documented, it's a well documented requirement and apart from anything else it's not really possible to review the code that manages the bindings without seeing the bindings. > The "default voltage" may not be device specific, but regulator.txt doesn't > have a property that describes what we needed here, because we can't get > away with specifying min=max as fixed-regulator does. We also cannot assume > that default=min or default=max, because that might harm hardware connected > to the other side of the IO bank. No, if you need a generic property add it in the generic code rather than just hacking a device specific property into your driver. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Add ltc3562 voltage regulator driver 2014-10-30 10:15 ` Mark Brown @ 2014-10-30 10:29 ` Mike Looijmans 2014-10-30 10:53 ` Mike Looijmans 0 siblings, 1 reply; 31+ messages in thread From: Mike Looijmans @ 2014-10-30 10:29 UTC (permalink / raw) To: Mark Brown; +Cc: lgirdwood, linux-kernel On 10/30/2014 11:15 AM, Mark Brown wrote: > On Thu, Oct 30, 2014 at 07:47:44AM +0100, Mike Looijmans wrote: >> On 10/29/2014 01:30 PM, Mark Brown wrote: > >>> A couple of problems here: > >>> - This contains DT code but no DT bindings documentation; the binding >>> documentation is mandatory for any new bindings. > >> Should I submit a new patch to add the DT bindings and CC to the dt mailing >> list, or do you want it included in this patch? I was basically waiting for >> feedback on the driver first. > > I'm not going to review a driver with bindings without those bindings > being documented, it's a well documented requirement and apart from > anything else it's not really possible to review the code that manages > the bindings without seeing the bindings. > >> The "default voltage" may not be device specific, but regulator.txt doesn't >> have a property that describes what we needed here, because we can't get >> away with specifying min=max as fixed-regulator does. We also cannot assume >> that default=min or default=max, because that might harm hardware connected >> to the other side of the IO bank. > > No, if you need a generic property add it in the generic code rather > than just hacking a device specific property into your driver. So I should add "regulator-default-voltage" to the generic code? That would indeed be better than trying to do it into this driver. But would that need a separate patch for regulator core to add the property? Mike. Met vriendelijke groet / kind regards, Mike Looijmans System Expert TOPIC Embedded Systems Eindhovenseweg 32-C, NL-5683 KH Best Postbus 440, NL-5680 AK Best Telefoon: (+31) (0) 499 33 69 79 Telefax: (+31) (0) 499 33 69 70 E-mail: mike.looijmans@topic.nl Website: www.topic.nl Please consider the environment before printing this e-mail Topic zoekt gedreven (embedded) software specialisten! http://topic.nl/vacatures/topic-zoekt-software-engineers/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Add ltc3562 voltage regulator driver 2014-10-30 10:29 ` Mike Looijmans @ 2014-10-30 10:53 ` Mike Looijmans 2014-10-30 10:58 ` Mark Brown 0 siblings, 1 reply; 31+ messages in thread From: Mike Looijmans @ 2014-10-30 10:53 UTC (permalink / raw) To: Mark Brown; +Cc: lgirdwood, linux-kernel On 10/30/2014 11:29 AM, Mike Looijmans wrote: > On 10/30/2014 11:15 AM, Mark Brown wrote: >> On Thu, Oct 30, 2014 at 07:47:44AM +0100, Mike Looijmans wrote: >>> On 10/29/2014 01:30 PM, Mark Brown wrote: >> >>>> A couple of problems here: >> >>>> - This contains DT code but no DT bindings documentation; the binding >>>> documentation is mandatory for any new bindings. >> >>> Should I submit a new patch to add the DT bindings and CC to the dt mailing >>> list, or do you want it included in this patch? I was basically waiting for >>> feedback on the driver first. >> >> I'm not going to review a driver with bindings without those bindings >> being documented, it's a well documented requirement and apart from >> anything else it's not really possible to review the code that manages >> the bindings without seeing the bindings. >> >>> The "default voltage" may not be device specific, but regulator.txt doesn't >>> have a property that describes what we needed here, because we can't get >>> away with specifying min=max as fixed-regulator does. We also cannot assume >>> that default=min or default=max, because that might harm hardware connected >>> to the other side of the IO bank. >> >> No, if you need a generic property add it in the generic code rather >> than just hacking a device specific property into your driver. > > So I should add "regulator-default-voltage" to the generic code? That would > indeed be better than trying to do it into this driver. > > But would that need a separate patch for regulator core to add the property? Hmm, I looked into doing that, but that isn't trivial, and too many devices will suffer. Since default-voltage is unacceptable, I will remove it from the ltc3562 driver and just use the min==max setting in the devicetree to reach the same effect. The driver that will match IO voltages between SOM and carrier is far from being ready for submission anyway. I'll submit a v2 patch with all these things reworked soon. Mike. Met vriendelijke groet / kind regards, Mike Looijmans System Expert TOPIC Embedded Systems Eindhovenseweg 32-C, NL-5683 KH Best Postbus 440, NL-5680 AK Best Telefoon: (+31) (0) 499 33 69 79 Telefax: (+31) (0) 499 33 69 70 E-mail: mike.looijmans@topic.nl Website: www.topic.nl Please consider the environment before printing this e-mail Topic zoekt gedreven (embedded) software specialisten! http://topic.nl/vacatures/topic-zoekt-software-engineers/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Add ltc3562 voltage regulator driver 2014-10-30 10:53 ` Mike Looijmans @ 2014-10-30 10:58 ` Mark Brown 2014-10-30 11:31 ` Mike Looijmans 0 siblings, 1 reply; 31+ messages in thread From: Mark Brown @ 2014-10-30 10:58 UTC (permalink / raw) To: Mike Looijmans; +Cc: lgirdwood, linux-kernel [-- Attachment #1: Type: text/plain, Size: 679 bytes --] On Thu, Oct 30, 2014 at 11:53:37AM +0100, Mike Looijmans wrote: > On 10/30/2014 11:29 AM, Mike Looijmans wrote: > >So I should add "regulator-default-voltage" to the generic code? That would > >indeed be better than trying to do it into this driver. > >But would that need a separate patch for regulator core to add the property? Yes. > Hmm, I looked into doing that, but that isn't trivial, and too many devices > will suffer. Why would this have a negative effect on other devices? Unless somehow the other devices add the property they should not be affected by it; if users do add the property presumably that's because it's doing something useful. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Add ltc3562 voltage regulator driver 2014-10-30 10:58 ` Mark Brown @ 2014-10-30 11:31 ` Mike Looijmans 2014-10-30 12:04 ` Mark Brown 0 siblings, 1 reply; 31+ messages in thread From: Mike Looijmans @ 2014-10-30 11:31 UTC (permalink / raw) To: Mark Brown; +Cc: lgirdwood, linux-kernel On 10/30/2014 11:58 AM, Mark Brown wrote: > On Thu, Oct 30, 2014 at 11:53:37AM +0100, Mike Looijmans wrote: >> On 10/30/2014 11:29 AM, Mike Looijmans wrote: > >>> So I should add "regulator-default-voltage" to the generic code? That would >>> indeed be better than trying to do it into this driver. > >>> But would that need a separate patch for regulator core to add the property? > > Yes. > >> Hmm, I looked into doing that, but that isn't trivial, and too many devices >> will suffer. > > Why would this have a negative effect on other devices? Unless somehow > the other devices add the property they should not be affected by it; if > users do add the property presumably that's because it's doing something > useful. The "default" would end up in the constraints, thus adding extra fields to that struct. So far, all other drivers have accomplished this by setting min=max, and some experimenting proved that my driver is no exception to that. It even reduced the amount of code. Mike. Met vriendelijke groet / kind regards, Mike Looijmans System Expert TOPIC Embedded Systems Eindhovenseweg 32-C, NL-5683 KH Best Postbus 440, NL-5680 AK Best Telefoon: (+31) (0) 499 33 69 79 Telefax: (+31) (0) 499 33 69 70 E-mail: mike.looijmans@topic.nl Website: www.topic.nl Please consider the environment before printing this e-mail Topic zoekt gedreven (embedded) software specialisten! http://topic.nl/vacatures/topic-zoekt-software-engineers/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Add ltc3562 voltage regulator driver 2014-10-30 11:31 ` Mike Looijmans @ 2014-10-30 12:04 ` Mark Brown 0 siblings, 0 replies; 31+ messages in thread From: Mark Brown @ 2014-10-30 12:04 UTC (permalink / raw) To: Mike Looijmans; +Cc: lgirdwood, linux-kernel [-- Attachment #1: Type: text/plain, Size: 909 bytes --] On Thu, Oct 30, 2014 at 12:31:42PM +0100, Mike Looijmans wrote: > On 10/30/2014 11:58 AM, Mark Brown wrote: > >>Hmm, I looked into doing that, but that isn't trivial, and too many devices > >>will suffer. > >Why would this have a negative effect on other devices? Unless somehow > >the other devices add the property they should not be affected by it; if > >users do add the property presumably that's because it's doing something > >useful. > The "default" would end up in the constraints, thus adding extra fields to > that struct. Sure, but why is that a problem? > So far, all other drivers have accomplished this by setting min=max, and > some experimenting proved that my driver is no exception to that. It even > reduced the amount of code. Yes, if you don't need to both set an initial voltage and support voltage changing that works - it sounded like you needed both. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2] Add ltc3562 voltage regulator driver 2014-10-29 8:16 ` [PATCH] " Mike Looijmans 2014-10-29 12:30 ` Mark Brown @ 2014-10-30 11:26 ` Mike Looijmans 2014-10-30 16:51 ` Mark Brown 2014-11-04 6:50 ` [PATCH v3] " Mike Looijmans 1 sibling, 2 replies; 31+ messages in thread From: Mike Looijmans @ 2014-10-30 11:26 UTC (permalink / raw) To: broonie; +Cc: lgirdwood, linux-kernel, Mike Looijmans The ltc3562 is an I2C controlled regulator supporting 4 independent outputs. v2: Prefix "lltc" to devicetree properties. Use the same property names as the ltc3589 driver. Remove default-voltage property. Use devm_register_regulator. Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl> --- .../devicetree/bindings/regulator/ltc3562.txt | 56 ++++ drivers/regulator/Kconfig | 7 + drivers/regulator/Makefile | 1 + drivers/regulator/ltc3562.c | 337 ++++++++++++++++++++ 4 files changed, 401 insertions(+) create mode 100644 Documentation/devicetree/bindings/regulator/ltc3562.txt create mode 100644 drivers/regulator/ltc3562.c diff --git a/Documentation/devicetree/bindings/regulator/ltc3562.txt b/Documentation/devicetree/bindings/regulator/ltc3562.txt new file mode 100644 index 0000000..81ed923 --- /dev/null +++ b/Documentation/devicetree/bindings/regulator/ltc3562.txt @@ -0,0 +1,56 @@ +Linear Technology LTC3562 I2C controlled 4-output regulator + +Required properties: +- compatible: "ltc3562" +- reg: I2C slave address + +Required child node: +- regulators: Contains four regulator child nodes R400B, R600B, R400A, R600A, + specifying the initialization data as documented in + Documentation/devicetree/bindings/regulator/regulator.txt. + +Each regulator is defined using the standard binding for regulators. The +nodes for R400A and R600A additionally need to specify the resistor values of +their external feedback voltage dividers: + +Required properties (A-type only): +- lltc,fb-voltage-divider: An array of two integers containing the resistor + values R1 and R2 of the feedback voltage divider. Both values must remain in + the range 1..1000, only their quotient matters. + +Example: + + ltc3562: ltc3562@65 { + compatible = "ltc3562"; + reg = <0x65>; + regulators { + R400B_reg: R400B { + regulator-name = "R400B"; + regulator-min-microvolt = <600000>; + regulator-max-microvolt = <3300000>; + regulator-boot-on; + regulator-always-on; + }; + R600B_reg: R600B { + regulator-name = "R600B"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-boot-on; + regulator-always-on; + }; + R400A_reg: R400A { + regulator-name = "R400A"; + regulator-min-microvolt = <425000>; + regulator-max-microvolt = <8000000>; + lltc,fb-voltage-divider = <1, 1>; + }; + R600A_reg: R600A { + regulator-name = "R600A"; + regulator-min-microvolt = <425000>; + regulator-max-microvolt = <1800000>; + lltc,fb-voltage-divider = <316, 100>; + regulator-boot-on; + regulator-always-on; + }; + }; + }; diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig index 55d7b7b..4012601 100644 --- a/drivers/regulator/Kconfig +++ b/drivers/regulator/Kconfig @@ -300,6 +300,13 @@ config REGULATOR_LP8788 help This driver supports LP8788 voltage regulator chip. +config REGULATOR_LTC3562 + tristate "LCT3562 quad output voltage regulator" + depends on I2C + help + This enables support for the LCT3562 quad output voltage regulator + controlled via I2C. + config REGULATOR_LTC3589 tristate "LTC3589 8-output voltage regulator" depends on I2C diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile index 1029ed3..9a15146 100644 --- a/drivers/regulator/Makefile +++ b/drivers/regulator/Makefile @@ -41,6 +41,7 @@ obj-$(CONFIG_REGULATOR_LP872X) += lp872x.o obj-$(CONFIG_REGULATOR_LP8788) += lp8788-buck.o obj-$(CONFIG_REGULATOR_LP8788) += lp8788-ldo.o obj-$(CONFIG_REGULATOR_LP8755) += lp8755.o +obj-$(CONFIG_REGULATOR_LTC3562) += ltc3562.o obj-$(CONFIG_REGULATOR_LTC3589) += ltc3589.o obj-$(CONFIG_REGULATOR_MAX14577) += max14577.o obj-$(CONFIG_REGULATOR_MAX1586) += max1586.o diff --git a/drivers/regulator/ltc3562.c b/drivers/regulator/ltc3562.c new file mode 100644 index 0000000..b73eee7 --- /dev/null +++ b/drivers/regulator/ltc3562.c @@ -0,0 +1,337 @@ +/* + * Regulator driver for Linear Technology LTC3562 + * + * Copyright (C) 2014 Topic Embedded Products + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ +#include <linux/bug.h> +#include <linux/err.h> +#include <linux/i2c.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/regulator/driver.h> +#include <linux/regulator/machine.h> +#include <linux/slab.h> +#include <linux/regulator/of_regulator.h> + +#define LTC3562_NUM_REGULATORS 4 + +#define REGULATOR_TYPE_A_NUM_VOLTAGES 16 +#define REGULATOR_TYPE_A_MIN_UV 425000 +#define REGULATOR_TYPE_A_UV_STEP 25000 + +#define REGULATOR_TYPE_B_NUM_VOLTAGES 128 +#define REGULATOR_TYPE_B_MIN_UV 600000 +#define REGULATOR_TYPE_B_UV_STEP 25000 + +struct ltc3562_status { + u8 shadow_addr; /* Shadow register of address byte */ + u8 shadow_data; /* Shadow register of data byte */ +}; + +struct ltc3562 { + struct device *dev; + struct i2c_client *i2c; + struct mutex mutex; + struct regulator_dev *rdev[LTC3562_NUM_REGULATORS]; + struct ltc3562_status rstatus[LTC3562_NUM_REGULATORS]; +}; + +static int ltc3562_write(struct i2c_client *i2c, u8 reg_a, u8 reg_b); +static int ltc3562_dummy_write(struct i2c_client *i2c); + +#define LTC3562_R400B_ID 0x00 +#define LTC3562_R600B_ID 0x01 +#define LTC3562_R400A_ID 0x02 +#define LTC3562_R600A_ID 0x03 + +#define OPERATING_MODE_MASK 0x03 + +#define PROGRAM_R400B_MASK 0x10 +#define PROGRAM_R600B_MASK 0x20 +#define PROGRAM_R400A_MASK 0x40 +#define PROGRAM_R600A_MASK 0x80 + +#define OPERATING_MODE_PULSE_SKIP 0x00 +#define OPERATING_MODE_LDO 0x01 +#define OPERATING_MODE_FOCED_BURST 0x02 +#define OPERATING_MODE_BURST 0x03 + +#define REGULATOR_ENABLE_BIT 0x80 + +static int ltc3562_regulator_enable(struct regulator_dev *dev) +{ + int ret; + struct ltc3562 *ltc3562 = rdev_get_drvdata(dev); + int id = rdev_get_id(dev); + struct ltc3562_status *status = <c3562->rstatus[id]; + + mutex_lock(<c3562->mutex); + + status->shadow_data |= REGULATOR_ENABLE_BIT; + + ret = ltc3562_write(ltc3562->i2c, + status->shadow_addr | PROGRAM_R400B_MASK << id, + status->shadow_data); + + mutex_unlock(<c3562->mutex); + + return ret; +} + +static int ltc3562_regulator_disable(struct regulator_dev *dev) +{ + int ret; + struct ltc3562 *ltc3562 = rdev_get_drvdata(dev); + int id = rdev_get_id(dev); + struct ltc3562_status *status = <c3562->rstatus[id]; + + mutex_lock(<c3562->mutex); + + status->shadow_data &= ~(REGULATOR_ENABLE_BIT); + + ret = ltc3562_write(ltc3562->i2c, + status->shadow_addr, status->shadow_data); + + mutex_unlock(<c3562->mutex); + + return ret; +} + +static int ltc3562_regulator_is_enabled(struct regulator_dev *dev) +{ + int ret; + struct ltc3562 *ltc3562 = rdev_get_drvdata(dev); + int id = rdev_get_id(dev); + struct ltc3562_status *status = <c3562->rstatus[id]; + + mutex_lock(<c3562->mutex); + + ret = (status->shadow_data & REGULATOR_ENABLE_BIT) ? 1 : 0; + + mutex_unlock(<c3562->mutex); + + return ret; +} + +static int ltc3562_set_voltage_sel(struct regulator_dev *dev, + unsigned int selector) +{ + int ret; + struct ltc3562 *ltc3562 = rdev_get_drvdata(dev); + int id = rdev_get_id(dev); + struct ltc3562_status *status = <c3562->rstatus[id]; + + mutex_lock(<c3562->mutex); + + status->shadow_data = + (status->shadow_data & REGULATOR_ENABLE_BIT) | selector; + + ret = ltc3562_write(ltc3562->i2c, + status->shadow_addr, status->shadow_data); + + mutex_unlock(<c3562->mutex); + + return ret; +} + +static int ltc3562_get_voltage_sel(struct regulator_dev *dev) +{ + int ret; + struct ltc3562 *ltc3562 = rdev_get_drvdata(dev); + int id = rdev_get_id(dev); + struct ltc3562_status *status = <c3562->rstatus[id]; + + mutex_lock(<c3562->mutex); + + ret = status->shadow_data & ~(REGULATOR_ENABLE_BIT); + + mutex_unlock(<c3562->mutex); + + return ret; +} + +static struct regulator_ops ltc3562_regulator_ops = { + .is_enabled = ltc3562_regulator_is_enabled, + .enable = ltc3562_regulator_enable, + .disable = ltc3562_regulator_disable, + .set_voltage_sel = ltc3562_set_voltage_sel, + .get_voltage_sel = ltc3562_get_voltage_sel, + .list_voltage = regulator_list_voltage_linear, + .map_voltage = regulator_map_voltage_linear, +}; + +static struct regulator_desc ltc3562_regulators[] = { + { + .name = "R400B", + .id = LTC3562_R400B_ID, + .ops = <c3562_regulator_ops, + .min_uV = REGULATOR_TYPE_B_MIN_UV, + .uV_step = REGULATOR_TYPE_B_UV_STEP, + .n_voltages = REGULATOR_TYPE_B_NUM_VOLTAGES, + .type = REGULATOR_VOLTAGE, + .owner = THIS_MODULE, + }, + { + .name = "R600B", + .id = LTC3562_R600B_ID, + .ops = <c3562_regulator_ops, + .min_uV = REGULATOR_TYPE_B_MIN_UV, + .uV_step = REGULATOR_TYPE_B_UV_STEP, + .n_voltages = REGULATOR_TYPE_B_NUM_VOLTAGES, + .type = REGULATOR_VOLTAGE, + .owner = THIS_MODULE, + }, + { + .name = "R400A", + .id = LTC3562_R400A_ID, + .ops = <c3562_regulator_ops, + .min_uV = REGULATOR_TYPE_A_MIN_UV, + .uV_step = REGULATOR_TYPE_A_UV_STEP, + .n_voltages = REGULATOR_TYPE_A_NUM_VOLTAGES, + .type = REGULATOR_VOLTAGE, + .owner = THIS_MODULE, + }, + { + .name = "R600A", + .id = LTC3562_R600A_ID, + .ops = <c3562_regulator_ops, + .min_uV = REGULATOR_TYPE_A_MIN_UV, + .uV_step = REGULATOR_TYPE_A_UV_STEP, + .n_voltages = REGULATOR_TYPE_A_NUM_VOLTAGES, + .type = REGULATOR_VOLTAGE, + .owner = THIS_MODULE, + }, +}; + + +static int ltc3562_write(struct i2c_client *i2c, u8 reg_a, u8 reg_b) +{ + dev_dbg(&i2c->dev, "%s(%#x, %#x)\n", __func__, reg_a, reg_b); + return i2c_smbus_write_byte_data(i2c, reg_a, reg_b); +} + +/* Write dummy data to detect presence of physical device */ +static int ltc3562_dummy_write(struct i2c_client *i2c) +{ + return ltc3562_write(i2c, 0, 0); +} + +static int ltc3562_i2c_probe(struct i2c_client *i2c, + const struct i2c_device_id *id) +{ + int i, error; + unsigned long uV; + struct ltc3562 *ltc3562; + struct regulator_config rconfig = { }; + struct device_node *np_regulators, *np; + struct device_node *np_child; + u32 value; + + ltc3562 = devm_kzalloc(&i2c->dev, + sizeof(struct ltc3562), GFP_KERNEL); + if (ltc3562 == NULL) + return -ENOMEM; + + ltc3562->i2c = i2c; + + mutex_init(<c3562->mutex); + + if (ltc3562_dummy_write(i2c) < 0) { + dev_err(&i2c->dev, + "Could not find device LTC3562 on i2c bus\n"); + error = -ENODEV; + goto lbl_exit; + } + + np = of_node_get(i2c->dev.of_node); + np_regulators = of_get_child_by_name(np, "regulators"); + + if (np_regulators == NULL) { + dev_err(&i2c->dev, "Could not find regulators node\n"); + error = -EINVAL; + goto lbl_exit; + } + + for (i = 0; i < LTC3562_NUM_REGULATORS; ++i) { + np_child = of_get_child_by_name(np_regulators, + ltc3562_regulators[i].name); + if (np_child == NULL) { + dev_err(&i2c->dev, + "Could not find regulator data for %s in devicetree\n", + ltc3562_regulators[i].name); + return -EINVAL; + } + + if ((ltc3562_regulators[i].id == LTC3562_R400A_ID) || + (ltc3562_regulators[i].id == LTC3562_R600A_ID)) { + /* If regulator is A type get resistor values */ + u32 vdiv[2]; + + error = of_property_read_u32_array(np_child, + "lltc,fb-voltage-divider", vdiv, 2); + if (error) { + dev_err(&i2c->dev, + "Failed to parse voltage divider: %d\n", + error); + return error; + } + + uV = ltc3562_regulators[i].min_uV / 1000; + ltc3562_regulators[i].min_uV = + (uV + mult_frac(uV, vdiv[0], vdiv[1])) * 1000; + + uV = ltc3562_regulators[i].uV_step / 1000; + ltc3562_regulators[i].uV_step = + (uV + mult_frac(uV, vdiv[0], vdiv[1])) * 1000; + } + + rconfig.dev = &i2c->dev; + rconfig.init_data = + of_get_regulator_init_data(&i2c->dev, np_child); + rconfig.driver_data = ltc3562; + rconfig.of_node = np_child; + + ltc3562->rdev[i] = devm_regulator_register(&i2c->dev, + <c3562_regulators[i], &rconfig); + + if (IS_ERR(ltc3562->rdev[i])) { + error = PTR_ERR(ltc3562->rdev[i]); + dev_err(&i2c->dev, + "could not register regulator, Error %d\n", + error); + } + } + + i2c_set_clientdata(i2c, ltc3562); + dev_dbg(&i2c->dev, "LTC3562 Driver loaded\n"); + return 0; + +lbl_exit: + return error; +} + +static const struct i2c_device_id ltc3562_i2c_id[] = { + { "ltc3562", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, ltc3562_i2c_id); + +static struct i2c_driver ltc3562_i2c_driver = { + .driver = { + .name = "LTC3562", + .owner = THIS_MODULE, + }, + .probe = ltc3562_i2c_probe, + .id_table = ltc3562_i2c_id, +}; + +module_i2c_driver(ltc3562_i2c_driver); + +MODULE_DESCRIPTION("LTC3562 Regulator Driver"); +MODULE_AUTHOR("auryn.verwegen@topic.nl"); +MODULE_AUTHOR("mike.looijmans@topic.nl"); +MODULE_LICENSE("GPL v2"); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v2] Add ltc3562 voltage regulator driver 2014-10-30 11:26 ` [PATCH v2] " Mike Looijmans @ 2014-10-30 16:51 ` Mark Brown 2014-10-31 14:07 ` Mike Looijmans 2014-11-03 8:10 ` Mike Looijmans 2014-11-04 6:50 ` [PATCH v3] " Mike Looijmans 1 sibling, 2 replies; 31+ messages in thread From: Mark Brown @ 2014-10-30 16:51 UTC (permalink / raw) To: Mike Looijmans; +Cc: lgirdwood, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1938 bytes --] On Thu, Oct 30, 2014 at 12:26:55PM +0100, Mike Looijmans wrote: > The ltc3562 is an I2C controlled regulator supporting 4 independent > outputs. > > v2: Prefix "lltc" to devicetree properties. Use the same property names > as the ltc3589 driver. Remove default-voltage property. Use > devm_register_regulator. As covered in SubmittingPatches things like this should be after ---. > +Required properties: > +- compatible: "ltc3562" This needs a vendor in the compatible string. > +Required child node: > +- regulators: Contains four regulator child nodes R400B, R600B, R400A, R600A, > + specifying the initialization data as documented in > + Documentation/devicetree/bindings/regulator/regulator.txt. All regulator child nodes should be optional. > + R600A_reg: R600A { > + regulator-name = "R600A"; Remove these regulator-names, this is for the name of the supplies on the board not the regulator itself. > +static int ltc3562_write(struct i2c_client *i2c, u8 reg_a, u8 reg_b); > +static int ltc3562_dummy_write(struct i2c_client *i2c); This appears to be reimplementing regmap (including a cache). Please use that instead. Pretty much the entire driver could then be replaced with the regmap helpers, none of the operations look like they'd be needed, and you'd get the regmap diagnostic infrastructure. > + np = of_node_get(i2c->dev.of_node); > + np_regulators = of_get_child_by_name(np, "regulators"); > + np_child = of_get_child_by_name(np_regulators, > + ltc3562_regulators[i].name); > + if (np_child == NULL) { Use the core support for looking up constraints please - set regulators_node and so on. > +static struct i2c_driver ltc3562_i2c_driver = { > + .driver = { > + .name = "LTC3562", > + .owner = THIS_MODULE, > + }, > + .probe = ltc3562_i2c_probe, > + .id_table = ltc3562_i2c_id, > +}; You need to supply an of_match_table too. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] Add ltc3562 voltage regulator driver 2014-10-30 16:51 ` Mark Brown @ 2014-10-31 14:07 ` Mike Looijmans 2014-10-31 18:17 ` Mark Brown 2014-11-03 8:10 ` Mike Looijmans 1 sibling, 1 reply; 31+ messages in thread From: Mike Looijmans @ 2014-10-31 14:07 UTC (permalink / raw) To: Mark Brown; +Cc: lgirdwood, linux-kernel On 30-10-2014 17:51, Mark Brown wrote: > On Thu, Oct 30, 2014 at 12:26:55PM +0100, Mike Looijmans wrote: >> The ltc3562 is an I2C controlled regulator supporting 4 independent >> outputs. >> >> v2: Prefix "lltc" to devicetree properties. Use the same property names >> as the ltc3589 driver. Remove default-voltage property. Use >> devm_register_regulator. > > As covered in SubmittingPatches things like this should be after ---. > >> +Required properties: >> +- compatible: "ltc3562" > > This needs a vendor in the compatible string. Will do. >> +Required child node: >> +- regulators: Contains four regulator child nodes R400B, R600B, R400A, R600A, >> + specifying the initialization data as documented in >> + Documentation/devicetree/bindings/regulator/regulator.txt. > > All regulator child nodes should be optional. > >> + R600A_reg: R600A { >> + regulator-name = "R600A"; > > Remove these regulator-names, this is for the name of the supplies on > the board not the regulator itself. > >> +static int ltc3562_write(struct i2c_client *i2c, u8 reg_a, u8 reg_b); >> +static int ltc3562_dummy_write(struct i2c_client *i2c); > > This appears to be reimplementing regmap (including a cache). Please > use that instead. Pretty much the entire driver could then be replaced > with the regmap helpers, none of the operations look like they'd be > needed, and you'd get the regmap diagnostic infrastructure. The chip doesn't have an I2C register map, it uses "commands". It does not support read transactions at all, it will NACK those. The first byte contains a bit mask that tells which outputs are to be configured and in what mode, the next is the enable bit and setpoint value. The first byte already contains data, it's not just an address. I would have used regmap if I thought it'd help (if only for the diagnostics), but it simply doesn't fit here. If you insist, I can probably get it to fit on regmap, but that would still mean implementing some custom write method that translates between some non-existing registry format and what the chip really wants. >> + np = of_node_get(i2c->dev.of_node); >> + np_regulators = of_get_child_by_name(np, "regulators"); > >> + np_child = of_get_child_by_name(np_regulators, >> + ltc3562_regulators[i].name); >> + if (np_child == NULL) { > > Use the core support for looking up constraints please - set > regulators_node and so on. I'll dig into it, seems that there's more infrastructure here to use. >> +static struct i2c_driver ltc3562_i2c_driver = { >> + .driver = { >> + .name = "LTC3562", >> + .owner = THIS_MODULE, >> + }, >> + .probe = ltc3562_i2c_probe, >> + .id_table = ltc3562_i2c_id, >> +}; > > You need to supply an of_match_table too. Hmm, that would explain why I couldn't get it to work using "lltc,ltc3562" during our first attempts at the driver. -- Mike Looijmans ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] Add ltc3562 voltage regulator driver 2014-10-31 14:07 ` Mike Looijmans @ 2014-10-31 18:17 ` Mark Brown 0 siblings, 0 replies; 31+ messages in thread From: Mark Brown @ 2014-10-31 18:17 UTC (permalink / raw) To: Mike Looijmans; +Cc: lgirdwood, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1013 bytes --] On Fri, Oct 31, 2014 at 03:07:57PM +0100, Mike Looijmans wrote: > On 30-10-2014 17:51, Mark Brown wrote: > >This appears to be reimplementing regmap (including a cache). Please > >use that instead. Pretty much the entire driver could then be replaced > >with the regmap helpers, none of the operations look like they'd be > >needed, and you'd get the regmap diagnostic infrastructure. > The chip doesn't have an I2C register map, it uses "commands". > It does not support read transactions at all, it will NACK those. The first > byte contains a bit mask that tells which outputs are to be configured and > in what mode, the next is the enable bit and setpoint value. The first byte > already contains data, it's not just an address. If that's the case the code is really rather unclear about what it's doing - the use of "reg" for what's being written doesn't help, nor does the fact that what looks like a cache is being kept. The code should either use regmap or make it clear why that doesn't fit well. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] Add ltc3562 voltage regulator driver 2014-10-30 16:51 ` Mark Brown 2014-10-31 14:07 ` Mike Looijmans @ 2014-11-03 8:10 ` Mike Looijmans 2014-11-03 12:09 ` Mark Brown 1 sibling, 1 reply; 31+ messages in thread From: Mike Looijmans @ 2014-11-03 8:10 UTC (permalink / raw) To: Mark Brown; +Cc: lgirdwood, linux-kernel On 10/30/2014 05:51 PM, Mark Brown wrote: > On Thu, Oct 30, 2014 at 12:26:55PM +0100, Mike Looijmans wrote: >> + np = of_node_get(i2c->dev.of_node); >> + np_regulators = of_get_child_by_name(np, "regulators"); > >> + np_child = of_get_child_by_name(np_regulators, >> + ltc3562_regulators[i].name); >> + if (np_child == NULL) { > > Use the core support for looking up constraints please - set > regulators_node and so on. I've been reworking the driver, but this is the only feedback comment I didn't quite understand. What is it that I'm expected to do here? Probably just the name of the method I'm supposed to call here would be enough. The rest of your feedback I've processed and tested the results. Mike. Met vriendelijke groet / kind regards, Mike Looijmans System Expert TOPIC Embedded Systems Eindhovenseweg 32-C, NL-5683 KH Best Postbus 440, NL-5680 AK Best Telefoon: (+31) (0) 499 33 69 79 Telefax: (+31) (0) 499 33 69 70 E-mail: mike.looijmans@topic.nl Website: www.topic.nl Please consider the environment before printing this e-mail Topic zoekt gedreven (embedded) software specialisten! http://topic.nl/vacatures/topic-zoekt-software-engineers/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] Add ltc3562 voltage regulator driver 2014-11-03 8:10 ` Mike Looijmans @ 2014-11-03 12:09 ` Mark Brown 2014-11-03 14:48 ` Mike Looijmans 0 siblings, 1 reply; 31+ messages in thread From: Mark Brown @ 2014-11-03 12:09 UTC (permalink / raw) To: Mike Looijmans; +Cc: lgirdwood, linux-kernel [-- Attachment #1: Type: text/plain, Size: 658 bytes --] On Mon, Nov 03, 2014 at 09:10:08AM +0100, Mike Looijmans wrote: > On 10/30/2014 05:51 PM, Mark Brown wrote: > >>+ np_child = of_get_child_by_name(np_regulators, > >>+ ltc3562_regulators[i].name); > >>+ if (np_child == NULL) { > >Use the core support for looking up constraints please - set > >regulators_node and so on. > I've been reworking the driver, but this is the only feedback comment I > didn't quite understand. What is it that I'm expected to do here? Probably > just the name of the method I'm supposed to call here would be enough. No function calls, just use regulators_node. What is unclear about the functionality? [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] Add ltc3562 voltage regulator driver 2014-11-03 12:09 ` Mark Brown @ 2014-11-03 14:48 ` Mike Looijmans 2014-11-03 15:10 ` Mark Brown 0 siblings, 1 reply; 31+ messages in thread From: Mike Looijmans @ 2014-11-03 14:48 UTC (permalink / raw) To: Mark Brown; +Cc: lgirdwood, linux-kernel On 11/03/2014 01:09 PM, Mark Brown wrote: > On Mon, Nov 03, 2014 at 09:10:08AM +0100, Mike Looijmans wrote: >> On 10/30/2014 05:51 PM, Mark Brown wrote: > >>>> + np_child = of_get_child_by_name(np_regulators, >>>> + ltc3562_regulators[i].name); >>>> + if (np_child == NULL) { > >>> Use the core support for looking up constraints please - set >>> regulators_node and so on. > >> I've been reworking the driver, but this is the only feedback comment I >> didn't quite understand. What is it that I'm expected to do here? Probably >> just the name of the method I'm supposed to call here would be enough. > > No function calls, just use regulators_node. What is unclear about the > functionality? I don't understand what you mean by "regulators_node". "grep -R regulators_node *" in the kernel source tree returns no results, nor does http://lxr.free-electrons.com/ident?i=regulators_node Met vriendelijke groet / kind regards, Mike Looijmans System Expert TOPIC Embedded Systems Eindhovenseweg 32-C, NL-5683 KH Best Postbus 440, NL-5680 AK Best Telefoon: (+31) (0) 499 33 69 79 Telefax: (+31) (0) 499 33 69 70 E-mail: mike.looijmans@topic.nl Website: www.topic.nl Please consider the environment before printing this e-mail Topic zoekt gedreven (embedded) software specialisten! http://topic.nl/vacatures/topic-zoekt-software-engineers/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] Add ltc3562 voltage regulator driver 2014-11-03 14:48 ` Mike Looijmans @ 2014-11-03 15:10 ` Mark Brown 2014-11-03 17:38 ` Mike Looijmans 0 siblings, 1 reply; 31+ messages in thread From: Mark Brown @ 2014-11-03 15:10 UTC (permalink / raw) To: Mike Looijmans; +Cc: lgirdwood, linux-kernel [-- Attachment #1: Type: text/plain, Size: 555 bytes --] On Mon, Nov 03, 2014 at 03:48:56PM +0100, Mike Looijmans wrote: > On 11/03/2014 01:09 PM, Mark Brown wrote: > >No function calls, just use regulators_node. What is unclear about the > >functionality? > I don't understand what you mean by "regulators_node". > "grep -R regulators_node *" in the kernel source tree returns no results, > nor does http://lxr.free-electrons.com/ident?i=regulators_node You need to develop against current versions of the kernel, this is something that was merged into Linus' tree during the merge window. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] Add ltc3562 voltage regulator driver 2014-11-03 15:10 ` Mark Brown @ 2014-11-03 17:38 ` Mike Looijmans 2014-11-04 8:55 ` Mike Looijmans 0 siblings, 1 reply; 31+ messages in thread From: Mike Looijmans @ 2014-11-03 17:38 UTC (permalink / raw) To: Mark Brown; +Cc: lgirdwood, linux-kernel On 3-11-2014 16:10, Mark Brown wrote: > On Mon, Nov 03, 2014 at 03:48:56PM +0100, Mike Looijmans wrote: >> On 11/03/2014 01:09 PM, Mark Brown wrote: > >>> No function calls, just use regulators_node. What is unclear about the >>> functionality? > >> I don't understand what you mean by "regulators_node". > >> "grep -R regulators_node *" in the kernel source tree returns no results, >> nor does http://lxr.free-electrons.com/ident?i=regulators_node > > You need to develop against current versions of the kernel, this is > something that was merged into Linus' tree during the merge window. Is this an absolute show-stopper for you? With some effort I could move from our current 3.15 to 3.17, but even that wouldn't be recent enough then. I can justify spending a few days on getting the driver integrated into mainline, even if the initial version cost less than that; but moving everything to mainline is going to take weeks and the boss is definitely going to say "no" to that. -- Mike Looijmans ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] Add ltc3562 voltage regulator driver 2014-11-03 17:38 ` Mike Looijmans @ 2014-11-04 8:55 ` Mike Looijmans 2014-11-04 11:34 ` Mark Brown 0 siblings, 1 reply; 31+ messages in thread From: Mike Looijmans @ 2014-11-04 8:55 UTC (permalink / raw) To: Mark Brown; +Cc: lgirdwood, linux-kernel On 11/03/2014 06:38 PM, Mike Looijmans wrote: > On 3-11-2014 16:10, Mark Brown wrote: >> On Mon, Nov 03, 2014 at 03:48:56PM +0100, Mike Looijmans wrote: >>> On 11/03/2014 01:09 PM, Mark Brown wrote: >> >>>> No function calls, just use regulators_node. What is unclear about the >>>> functionality? >> >>> I don't understand what you mean by "regulators_node". >> >>> "grep -R regulators_node *" in the kernel source tree returns no results, >>> nor does http://lxr.free-electrons.com/ident?i=regulators_node >> >> You need to develop against current versions of the kernel, this is >> something that was merged into Linus' tree during the merge window. > > Is this an absolute show-stopper for you? > > With some effort I could move from our current 3.15 to 3.17, but even that > wouldn't be recent enough then. I can justify spending a few days on getting > the driver integrated into mainline, even if the initial version cost less > than that; but moving everything to mainline is going to take weeks and the > boss is definitely going to say "no" to that. More important than that: Since this chip powers the IO banks, we need it for our own products, and hence it has to work (also) on the current 3.15 kernel. Using APIs from 3.18 wouold render the driver useless to ourselves. Met vriendelijke groet / kind regards, Mike Looijmans System Expert TOPIC Embedded Systems Eindhovenseweg 32-C, NL-5683 KH Best Postbus 440, NL-5680 AK Best Telefoon: (+31) (0) 499 33 69 79 Telefax: (+31) (0) 499 33 69 70 E-mail: mike.looijmans@topic.nl Website: www.topic.nl Please consider the environment before printing this e-mail Topic zoekt gedreven (embedded) software specialisten! http://topic.nl/vacatures/topic-zoekt-software-engineers/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] Add ltc3562 voltage regulator driver 2014-11-04 8:55 ` Mike Looijmans @ 2014-11-04 11:34 ` Mark Brown 2014-11-04 12:47 ` Mike Looijmans 2014-11-04 13:35 ` Mike Looijmans 0 siblings, 2 replies; 31+ messages in thread From: Mark Brown @ 2014-11-04 11:34 UTC (permalink / raw) To: Mike Looijmans; +Cc: lgirdwood, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1340 bytes --] On Tue, Nov 04, 2014 at 09:55:14AM +0100, Mike Looijmans wrote: > On 11/03/2014 06:38 PM, Mike Looijmans wrote: > >>You need to develop against current versions of the kernel, this is > >>something that was merged into Linus' tree during the merge window. > >Is this an absolute show-stopper for you? > >With some effort I could move from our current 3.15 to 3.17, but even that > >wouldn't be recent enough then. I can justify spending a few days on getting > >the driver integrated into mainline, even if the initial version cost less > >than that; but moving everything to mainline is going to take weeks and the > >boss is definitely going to say "no" to that. It should be easy to backport the support into your current kernel, it's just a few commits and there hasn't been much development in this area of the code. Can you take a look at doing that please? > More important than that: Since this chip powers the IO banks, we need it > for our own products, and hence it has to work (also) on the current 3.15 > kernel. Using APIs from 3.18 wouold render the driver useless to ourselves. This is just not at all important upstream, sorry. The kernel moves on and internal APIs within the kernel move on so it is expected that code used in production won't be the latest and greatest upstream code. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] Add ltc3562 voltage regulator driver 2014-11-04 11:34 ` Mark Brown @ 2014-11-04 12:47 ` Mike Looijmans 2014-11-04 13:35 ` Mike Looijmans 1 sibling, 0 replies; 31+ messages in thread From: Mike Looijmans @ 2014-11-04 12:47 UTC (permalink / raw) To: Mark Brown; +Cc: lgirdwood, linux-kernel On 11/04/2014 12:34 PM, Mark Brown wrote: > On Tue, Nov 04, 2014 at 09:55:14AM +0100, Mike Looijmans wrote: >> On 11/03/2014 06:38 PM, Mike Looijmans wrote: > >>>> You need to develop against current versions of the kernel, this is >>>> something that was merged into Linus' tree during the merge window. > >>> Is this an absolute show-stopper for you? > >>> With some effort I could move from our current 3.15 to 3.17, but even that >>> wouldn't be recent enough then. I can justify spending a few days on getting >>> the driver integrated into mainline, even if the initial version cost less >>> than that; but moving everything to mainline is going to take weeks and the >>> boss is definitely going to say "no" to that. > > It should be easy to backport the support into your current kernel, it's > just a few commits and there hasn't been much development in this area > of the code. Can you take a look at doing that please? That option hadn't crossed my mind yet, I'll look into it. > >> More important than that: Since this chip powers the IO banks, we need it >> for our own products, and hence it has to work (also) on the current 3.15 >> kernel. Using APIs from 3.18 wouold render the driver useless to ourselves. > > This is just not at all important upstream, sorry. The kernel moves on > and internal APIs within the kernel move on so it is expected that code > used in production won't be the latest and greatest upstream code. I realize that, but then the result would be that I'll stop working on getting the driver into mainline and force customers to use a patch or use our fork of linux. Met vriendelijke groet / kind regards, Mike Looijmans System Expert TOPIC Embedded Systems Eindhovenseweg 32-C, NL-5683 KH Best Postbus 440, NL-5680 AK Best Telefoon: (+31) (0) 499 33 69 79 Telefax: (+31) (0) 499 33 69 70 E-mail: mike.looijmans@topic.nl Website: www.topic.nl Please consider the environment before printing this e-mail Topic zoekt gedreven (embedded) software specialisten! http://topic.nl/vacatures/topic-zoekt-software-engineers/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] Add ltc3562 voltage regulator driver 2014-11-04 11:34 ` Mark Brown 2014-11-04 12:47 ` Mike Looijmans @ 2014-11-04 13:35 ` Mike Looijmans 2014-11-04 19:47 ` Mark Brown 1 sibling, 1 reply; 31+ messages in thread From: Mike Looijmans @ 2014-11-04 13:35 UTC (permalink / raw) To: Mark Brown; +Cc: lgirdwood, linux-kernel On 11/04/2014 12:34 PM, Mark Brown wrote: > On Tue, Nov 04, 2014 at 09:55:14AM +0100, Mike Looijmans wrote: >> On 11/03/2014 06:38 PM, Mike Looijmans wrote: > >>>> You need to develop against current versions of the kernel, this is >>>> something that was merged into Linus' tree during the merge window. > >>> Is this an absolute show-stopper for you? > >>> With some effort I could move from our current 3.15 to 3.17, but even that >>> wouldn't be recent enough then. I can justify spending a few days on getting >>> the driver integrated into mainline, even if the initial version cost less >>> than that; but moving everything to mainline is going to take weeks and the >>> boss is definitely going to say "no" to that. > > It should be easy to backport the support into your current kernel, it's > just a few commits and there hasn't been much development in this area > of the code. Can you take a look at doing that please? I can cherry-pick about 4 commits to get the updated core into my 3.15 tree. I still need help with one thing that isn't clear to me though. The DT is parsed when calling regulator_register. But then how do I fetch my "private" settings in there BEFORE the regulator gets enabled? The feedback resistors and regulator mode must be set to the correct board before the output is enabled, otherwise it may damage the chip and its periferals. The only two drivers using the regulators_node (tps65217 and isl9305.c) don't have any private values in the DT. M. Met vriendelijke groet / kind regards, Mike Looijmans System Expert TOPIC Embedded Systems Eindhovenseweg 32-C, NL-5683 KH Best Postbus 440, NL-5680 AK Best Telefoon: (+31) (0) 499 33 69 79 Telefax: (+31) (0) 499 33 69 70 E-mail: mike.looijmans@topic.nl Website: www.topic.nl Please consider the environment before printing this e-mail Topic zoekt gedreven (embedded) software specialisten! http://topic.nl/vacatures/topic-zoekt-software-engineers/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] Add ltc3562 voltage regulator driver 2014-11-04 13:35 ` Mike Looijmans @ 2014-11-04 19:47 ` Mark Brown 2014-11-05 9:06 ` Krzysztof Kozlowski 2014-11-05 11:45 ` Mike Looijmans 0 siblings, 2 replies; 31+ messages in thread From: Mark Brown @ 2014-11-04 19:47 UTC (permalink / raw) To: Mike Looijmans Cc: lgirdwood, linux-kernel, Javier Martinez Canillas, Krzysztof Kozlowski [-- Attachment #1: Type: text/plain, Size: 620 bytes --] On Tue, Nov 04, 2014 at 02:35:50PM +0100, Mike Looijmans wrote: > I still need help with one thing that isn't clear to me though. The DT is > parsed when calling regulator_register. But then how do I fetch my "private" > settings in there BEFORE the regulator gets enabled? The feedback resistors > and regulator mode must be set to the correct board before the output is > enabled, otherwise it may damage the chip and its periferals. Add a callback for this. Javier and/or Krzysztof were looking at this for some other stuff, though I think they were intending to add core functionality for their specific feature. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] Add ltc3562 voltage regulator driver 2014-11-04 19:47 ` Mark Brown @ 2014-11-05 9:06 ` Krzysztof Kozlowski 2014-11-05 11:45 ` Mike Looijmans 1 sibling, 0 replies; 31+ messages in thread From: Krzysztof Kozlowski @ 2014-11-05 9:06 UTC (permalink / raw) To: Mark Brown Cc: Mike Looijmans, lgirdwood, linux-kernel, Javier Martinez Canillas On wto, 2014-11-04 at 19:47 +0000, Mark Brown wrote: > On Tue, Nov 04, 2014 at 02:35:50PM +0100, Mike Looijmans wrote: > > > I still need help with one thing that isn't clear to me though. The DT is > > parsed when calling regulator_register. But then how do I fetch my "private" > > settings in there BEFORE the regulator gets enabled? The feedback resistors > > and regulator mode must be set to the correct board before the output is > > enabled, otherwise it may damage the chip and its periferals. > > Add a callback for this. Javier and/or Krzysztof were looking at this > for some other stuff, though I think they were intending to add core > functionality for their specific feature. I needed a callback and added such. The callback was called on each DT node parsed. However I dropped that idea because it turned our to be unsuitable to my case. I needed to feed regulator_config.ena_gpio with data parsed from such callback. But the callback called from inside the regulator_register() has the regulator_config const. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] Add ltc3562 voltage regulator driver 2014-11-04 19:47 ` Mark Brown 2014-11-05 9:06 ` Krzysztof Kozlowski @ 2014-11-05 11:45 ` Mike Looijmans 1 sibling, 0 replies; 31+ messages in thread From: Mike Looijmans @ 2014-11-05 11:45 UTC (permalink / raw) To: Mark Brown Cc: lgirdwood, linux-kernel, Javier Martinez Canillas, Krzysztof Kozlowski On 11/04/2014 08:47 PM, Mark Brown wrote: > On Tue, Nov 04, 2014 at 02:35:50PM +0100, Mike Looijmans wrote: > >> I still need help with one thing that isn't clear to me though. The DT is >> parsed when calling regulator_register. But then how do I fetch my "private" >> settings in there BEFORE the regulator gets enabled? The feedback resistors >> and regulator mode must be set to the correct board before the output is >> enabled, otherwise it may damage the chip and its periferals. > > Add a callback for this. Javier and/or Krzysztof were looking at this > for some other stuff, though I think they were intending to add core > functionality for their specific feature. > I've run out of my hours-to-spend-on-driver-submission budget for this period , so it'll be at least a week before I can post a new version. The good news is that I have upgrading the kernel to 3.17 on the planning board now, so that'll at least reduce the version gap. Mike. Met vriendelijke groet / kind regards, Mike Looijmans System Expert TOPIC Embedded Systems Eindhovenseweg 32-C, NL-5683 KH Best Postbus 440, NL-5680 AK Best Telefoon: (+31) (0) 499 33 69 79 Telefax: (+31) (0) 499 33 69 70 E-mail: mike.looijmans@topic.nl Website: www.topic.nl Please consider the environment before printing this e-mail Topic zoekt gedreven (embedded) software specialisten! http://topic.nl/vacatures/topic-zoekt-software-engineers/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3] Add ltc3562 voltage regulator driver 2014-10-30 11:26 ` [PATCH v2] " Mike Looijmans 2014-10-30 16:51 ` Mark Brown @ 2014-11-04 6:50 ` Mike Looijmans 2014-11-04 20:26 ` Mark Brown 1 sibling, 1 reply; 31+ messages in thread From: Mike Looijmans @ 2014-11-04 6:50 UTC (permalink / raw) To: broonie; +Cc: lgirdwood, linux-kernel, Mike Looijmans The ltc3562 is an I2C controlled regulator supporting 4 independent outputs. Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl> --- v3: Add .of_match_table and prefix lltc Clarify why regmap cannot be used Add lltc,operating-mode (0..3) DT property regulator child nodes are optional .../devicetree/bindings/regulator/ltc3562.txt | 57 ++++ drivers/regulator/Kconfig | 7 + drivers/regulator/Makefile | 1 + drivers/regulator/ltc3562.c | 337 ++++++++++++++++++++ 4 files changed, 402 insertions(+) create mode 100644 Documentation/devicetree/bindings/regulator/ltc3562.txt create mode 100644 drivers/regulator/ltc3562.c diff --git a/Documentation/devicetree/bindings/regulator/ltc3562.txt b/Documentation/devicetree/bindings/regulator/ltc3562.txt new file mode 100644 index 0000000..d73c865 --- /dev/null +++ b/Documentation/devicetree/bindings/regulator/ltc3562.txt @@ -0,0 +1,57 @@ +Linear Technology LTC3562 I2C controlled 4-output regulator + +Required properties: +- compatible: "lltc,ltc3562" +- reg: I2C slave address + +Required child node: +- regulators: Contains optional regulator child nodes R400B, R600B, R400A + and R600A specifying the initialization data as documented in + Documentation/devicetree/bindings/regulator/regulator.txt. + +Each regulator is defined using the standard binding for regulators. The +nodes for R400A and R600A additionally need to specify the resistor values of +their external feedback voltage dividers: + +Required properties (A-type only): +- lltc,fb-voltage-divider: An array of two integers containing the resistor + values R1 and R2 of the feedback voltage divider. Both values must remain in + the range 1..1000, only their quotient matters. + +Optional properties: +- lltc,operating-mode: Operating mode as specified in table 3 of the datasheet. + This value is passed as the lower two bits of the first data byte, and sets + the operating mode: 0=Pulse-skip (default), 1=LDO, 2=Forced-burst, 3=Burst. + +Example: + + ltc3562: ltc3562@65 { + compatible = "ltc3562"; + reg = <0x65>; + regulators { + R400B_reg: R400B { + regulator-min-microvolt = <600000>; + regulator-max-microvolt = <3300000>; + regulator-boot-on; + regulator-always-on; + }; + R600B_reg: R600B { + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-boot-on; + regulator-always-on; + }; + R400A_reg: R400A { + regulator-min-microvolt = <425000>; + regulator-max-microvolt = <8000000>; + lltc,fb-voltage-divider = <1, 1>; + }; + R600A_reg: R600A { + regulator-min-microvolt = <425000>; + regulator-max-microvolt = <1800000>; + lltc,fb-voltage-divider = <316, 100>; + regulator-boot-on; + regulator-always-on; + }; + }; + }; diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig index 55d7b7b..4012601 100644 --- a/drivers/regulator/Kconfig +++ b/drivers/regulator/Kconfig @@ -300,6 +300,13 @@ config REGULATOR_LP8788 help This driver supports LP8788 voltage regulator chip. +config REGULATOR_LTC3562 + tristate "LCT3562 quad output voltage regulator" + depends on I2C + help + This enables support for the LCT3562 quad output voltage regulator + controlled via I2C. + config REGULATOR_LTC3589 tristate "LTC3589 8-output voltage regulator" depends on I2C diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile index 1029ed3..9a15146 100644 --- a/drivers/regulator/Makefile +++ b/drivers/regulator/Makefile @@ -41,6 +41,7 @@ obj-$(CONFIG_REGULATOR_LP872X) += lp872x.o obj-$(CONFIG_REGULATOR_LP8788) += lp8788-buck.o obj-$(CONFIG_REGULATOR_LP8788) += lp8788-ldo.o obj-$(CONFIG_REGULATOR_LP8755) += lp8755.o +obj-$(CONFIG_REGULATOR_LTC3562) += ltc3562.o obj-$(CONFIG_REGULATOR_LTC3589) += ltc3589.o obj-$(CONFIG_REGULATOR_MAX14577) += max14577.o obj-$(CONFIG_REGULATOR_MAX1586) += max1586.o diff --git a/drivers/regulator/ltc3562.c b/drivers/regulator/ltc3562.c new file mode 100644 index 0000000..69963f8 --- /dev/null +++ b/drivers/regulator/ltc3562.c @@ -0,0 +1,337 @@ +/* + * Regulator driver for Linear Technology LTC3562 + * + * Copyright (C) 2014 Topic Embedded Products + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ +#include <linux/bug.h> +#include <linux/err.h> +#include <linux/i2c.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/regulator/driver.h> +#include <linux/regulator/machine.h> +#include <linux/slab.h> +#include <linux/regulator/of_regulator.h> + +#define LTC3562_NUM_REGULATORS 4 + +#define REGULATOR_TYPE_A_NUM_VOLTAGES 16 +#define REGULATOR_TYPE_A_MIN_UV 425000 +#define REGULATOR_TYPE_A_UV_STEP 25000 + +#define REGULATOR_TYPE_B_NUM_VOLTAGES 128 +#define REGULATOR_TYPE_B_MIN_UV 600000 +#define REGULATOR_TYPE_B_UV_STEP 25000 + +/* the LTC3562 does not have a register map, instead it receives a two-byte + * command set. The first byte sets the mask for the output(s) to be programmed + * and the second byte hold the "enable" bit and the DAC code. */ +struct ltc3562_status { + u8 addr_mode; /* sub-address byte: program mask an operating mode */ + u8 enable_daccode; /* data byte: Enable bit and DAC code */ +}; + +struct ltc3562 { + struct device *dev; + struct i2c_client *i2c; + struct mutex mutex; + struct regulator_dev *rdev[LTC3562_NUM_REGULATORS]; + struct ltc3562_status rstatus[LTC3562_NUM_REGULATORS]; +}; + +#define LTC3562_R400B_ID 0x00 +#define LTC3562_R600B_ID 0x01 +#define LTC3562_R400A_ID 0x02 +#define LTC3562_R600A_ID 0x03 + +#define OPERATING_MODE_MASK 0x03 + +#define PROGRAM_R400B_MASK 0x10 +#define PROGRAM_R600B_MASK 0x20 +#define PROGRAM_R400A_MASK 0x40 +#define PROGRAM_R600A_MASK 0x80 + +#define OPERATING_MODE_PULSE_SKIP 0x00 +#define OPERATING_MODE_LDO 0x01 +#define OPERATING_MODE_FOCED_BURST 0x02 +#define OPERATING_MODE_BURST 0x03 + +#define REGULATOR_ENABLE_BIT 0x80 + +static int ltc3562_update(struct ltc3562 *ltc3562, + struct ltc3562_status *status) +{ + return i2c_smbus_write_byte_data( + ltc3562->i2c, status->addr_mode, status->enable_daccode); +} + +static int ltc3562_regulator_enable(struct regulator_dev *dev) +{ + int ret; + struct ltc3562 *ltc3562 = rdev_get_drvdata(dev); + int id = rdev_get_id(dev); + struct ltc3562_status *status = <c3562->rstatus[id]; + + mutex_lock(<c3562->mutex); + + status->enable_daccode |= REGULATOR_ENABLE_BIT; + ret = ltc3562_update(ltc3562, status); + + mutex_unlock(<c3562->mutex); + + return ret; +} + +static int ltc3562_regulator_disable(struct regulator_dev *dev) +{ + int ret; + struct ltc3562 *ltc3562 = rdev_get_drvdata(dev); + int id = rdev_get_id(dev); + struct ltc3562_status *status = <c3562->rstatus[id]; + + mutex_lock(<c3562->mutex); + + status->enable_daccode &= ~(REGULATOR_ENABLE_BIT); + ret = ltc3562_update(ltc3562, status); + + mutex_unlock(<c3562->mutex); + + return ret; +} + +static int ltc3562_regulator_is_enabled(struct regulator_dev *dev) +{ + int ret; + struct ltc3562 *ltc3562 = rdev_get_drvdata(dev); + int id = rdev_get_id(dev); + struct ltc3562_status *status = <c3562->rstatus[id]; + + mutex_lock(<c3562->mutex); + + ret = (status->enable_daccode & REGULATOR_ENABLE_BIT) ? 1 : 0; + + mutex_unlock(<c3562->mutex); + + return ret; +} + +static int ltc3562_set_voltage_sel(struct regulator_dev *dev, + unsigned int selector) +{ + int ret; + struct ltc3562 *ltc3562 = rdev_get_drvdata(dev); + int id = rdev_get_id(dev); + struct ltc3562_status *status = <c3562->rstatus[id]; + + mutex_lock(<c3562->mutex); + + status->enable_daccode = + (status->enable_daccode & REGULATOR_ENABLE_BIT) | selector; + ret = ltc3562_update(ltc3562, status); + + mutex_unlock(<c3562->mutex); + + return ret; +} + +static int ltc3562_get_voltage_sel(struct regulator_dev *dev) +{ + int ret; + struct ltc3562 *ltc3562 = rdev_get_drvdata(dev); + int id = rdev_get_id(dev); + struct ltc3562_status *status = <c3562->rstatus[id]; + + mutex_lock(<c3562->mutex); + + ret = status->enable_daccode & ~(REGULATOR_ENABLE_BIT); + + mutex_unlock(<c3562->mutex); + + return ret; +} + +static struct regulator_ops ltc3562_regulator_ops = { + .is_enabled = ltc3562_regulator_is_enabled, + .enable = ltc3562_regulator_enable, + .disable = ltc3562_regulator_disable, + .set_voltage_sel = ltc3562_set_voltage_sel, + .get_voltage_sel = ltc3562_get_voltage_sel, + .list_voltage = regulator_list_voltage_linear, + .map_voltage = regulator_map_voltage_linear, +}; + +static struct regulator_desc ltc3562_regulators[] = { + { + .name = "R400B", + .id = LTC3562_R400B_ID, + .ops = <c3562_regulator_ops, + .min_uV = REGULATOR_TYPE_B_MIN_UV, + .uV_step = REGULATOR_TYPE_B_UV_STEP, + .n_voltages = REGULATOR_TYPE_B_NUM_VOLTAGES, + .type = REGULATOR_VOLTAGE, + .owner = THIS_MODULE, + }, + { + .name = "R600B", + .id = LTC3562_R600B_ID, + .ops = <c3562_regulator_ops, + .min_uV = REGULATOR_TYPE_B_MIN_UV, + .uV_step = REGULATOR_TYPE_B_UV_STEP, + .n_voltages = REGULATOR_TYPE_B_NUM_VOLTAGES, + .type = REGULATOR_VOLTAGE, + .owner = THIS_MODULE, + }, + { + .name = "R400A", + .id = LTC3562_R400A_ID, + .ops = <c3562_regulator_ops, + .min_uV = REGULATOR_TYPE_A_MIN_UV, + .uV_step = REGULATOR_TYPE_A_UV_STEP, + .n_voltages = REGULATOR_TYPE_A_NUM_VOLTAGES, + .type = REGULATOR_VOLTAGE, + .owner = THIS_MODULE, + }, + { + .name = "R600A", + .id = LTC3562_R600A_ID, + .ops = <c3562_regulator_ops, + .min_uV = REGULATOR_TYPE_A_MIN_UV, + .uV_step = REGULATOR_TYPE_A_UV_STEP, + .n_voltages = REGULATOR_TYPE_A_NUM_VOLTAGES, + .type = REGULATOR_VOLTAGE, + .owner = THIS_MODULE, + }, +}; + +/* Write dummy data to detect presence of the physical device. */ +static int ltc3562_dummy_write(struct i2c_client *i2c) +{ + return i2c_smbus_write_byte_data(i2c, 0, 0); +} + +static int ltc3562_i2c_probe(struct i2c_client *i2c, + const struct i2c_device_id *id) +{ + int i, error; + unsigned long uV; + struct ltc3562 *ltc3562; + struct regulator_config rconfig = { }; + struct device_node *np_regulators, *np; + struct device_node *np_child; + u32 value; + + ltc3562 = devm_kzalloc(&i2c->dev, + sizeof(struct ltc3562), GFP_KERNEL); + if (ltc3562 == NULL) + return -ENOMEM; + + ltc3562->i2c = i2c; + i2c_set_clientdata(i2c, ltc3562); + + mutex_init(<c3562->mutex); + + if (ltc3562_dummy_write(i2c) < 0) { + dev_err(&i2c->dev, + "Could not find device LTC3562 on i2c bus\n"); + return -ENODEV; + } + + np = of_node_get(i2c->dev.of_node); + np_regulators = of_get_child_by_name(np, "regulators"); + + if (np_regulators == NULL) { + dev_err(&i2c->dev, "Could not find regulators node\n"); + return -EINVAL; + } + + for (i = 0; i < LTC3562_NUM_REGULATORS; ++i) { + np_child = of_get_child_by_name(np_regulators, + ltc3562_regulators[i].name); + if (np_child == NULL) + continue; + + if ((ltc3562_regulators[i].id == LTC3562_R400A_ID) || + (ltc3562_regulators[i].id == LTC3562_R600A_ID)) { + /* If regulator is A type get resistor values */ + u32 vdiv[2]; + + error = of_property_read_u32_array(np_child, + "lltc,fb-voltage-divider", vdiv, 2); + if (error) { + dev_err(&i2c->dev, + "Failed to parse voltage divider: %d\n", + error); + return error; + } + + uV = ltc3562_regulators[i].min_uV / 1000; + ltc3562_regulators[i].min_uV = + (uV + mult_frac(uV, vdiv[0], vdiv[1])) * 1000; + + uV = ltc3562_regulators[i].uV_step / 1000; + ltc3562_regulators[i].uV_step = + (uV + mult_frac(uV, vdiv[0], vdiv[1])) * 1000; + } + + /* Set operating mode and address mask */ + error = of_property_read_u32(np_child, + "lltc,operating-mode", &value); + if (error || (value > OPERATING_MODE_BURST)) + value = OPERATING_MODE_PULSE_SKIP; + ltc3562->rstatus[i].addr_mode = + (PROGRAM_R400B_MASK << i) | value; + + rconfig.dev = &i2c->dev; + rconfig.init_data = + of_get_regulator_init_data(&i2c->dev, np_child); + rconfig.driver_data = ltc3562; + rconfig.of_node = np_child; + + ltc3562->rdev[i] = devm_regulator_register(&i2c->dev, + <c3562_regulators[i], &rconfig); + + if (IS_ERR(ltc3562->rdev[i])) { + error = PTR_ERR(ltc3562->rdev[i]); + dev_err(&i2c->dev, + "could not register regulator, Error %d\n", + error); + } + } + + dev_dbg(&i2c->dev, "LTC3562 Driver loaded\n"); + + return 0; +} + +static const struct of_device_id ltc3562_match_id[] = { + { .compatible = "lltc,ltc3562", }, + {}, +}; +MODULE_DEVICE_TABLE(of, ltc3562_match_id); + +static const struct i2c_device_id ltc3562_i2c_id[] = { + { "ltc3562", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, ltc3562_i2c_id); + +static struct i2c_driver ltc3562_i2c_driver = { + .driver = { + .name = "LTC3562", + .owner = THIS_MODULE, + .of_match_table = of_match_ptr(ltc3562_match_id), + }, + .probe = ltc3562_i2c_probe, + .id_table = ltc3562_i2c_id, +}; + +module_i2c_driver(ltc3562_i2c_driver); + +MODULE_DESCRIPTION("LTC3562 Regulator Driver"); +MODULE_AUTHOR("auryn.verwegen@topic.nl"); +MODULE_AUTHOR("mike.looijmans@topic.nl"); +MODULE_LICENSE("GPL v2"); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v3] Add ltc3562 voltage regulator driver 2014-11-04 6:50 ` [PATCH v3] " Mike Looijmans @ 2014-11-04 20:26 ` Mark Brown 2014-11-05 11:41 ` Mike Looijmans 0 siblings, 1 reply; 31+ messages in thread From: Mark Brown @ 2014-11-04 20:26 UTC (permalink / raw) To: Mike Looijmans; +Cc: lgirdwood, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1345 bytes --] On Tue, Nov 04, 2014 at 07:50:45AM +0100, Mike Looijmans wrote: > v3: Add .of_match_table and prefix lltc > Clarify why regmap cannot be used > Add lltc,operating-mode (0..3) DT property > regulator child nodes are optional Leave out the mode setting in the DT for now please, Javier was looking at adding a standard property for this with a driver defined mapping to modes - you should implement the standard get_mode() and set_mode() (but can always do that as a followup). > +/* the LTC3562 does not have a register map, instead it receives a two-byte > + * command set. The first byte sets the mask for the output(s) to be programmed > + * and the second byte hold the "enable" bit and the DAC code. */ > +struct ltc3562_status { > + u8 addr_mode; /* sub-address byte: program mask an operating mode */ > + u8 enable_daccode; /* data byte: Enable bit and DAC code */ > +}; So, I managed to find a datasheet[1] and this does actually seem to be a standard register map. It looks like this is a 4x12 register map with the program bytes being essentially register addresses (called sub addresses in the datasheet), two pad bits (normally we'd include these in the data for convenience) and the rest of the bits data. What am I missing here? Otherwise this looks good. [1] http://cds.linear.com/docs/en/datasheet/3562fa.pdf [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3] Add ltc3562 voltage regulator driver 2014-11-04 20:26 ` Mark Brown @ 2014-11-05 11:41 ` Mike Looijmans 2014-11-05 13:34 ` Mark Brown 0 siblings, 1 reply; 31+ messages in thread From: Mike Looijmans @ 2014-11-05 11:41 UTC (permalink / raw) To: Mark Brown; +Cc: lgirdwood, linux-kernel On 11/04/2014 09:26 PM, Mark Brown wrote: > On Tue, Nov 04, 2014 at 07:50:45AM +0100, Mike Looijmans wrote: > >> v3: Add .of_match_table and prefix lltc >> Clarify why regmap cannot be used >> Add lltc,operating-mode (0..3) DT property >> regulator child nodes are optional > > Leave out the mode setting in the DT for now please, Javier was looking > at adding a standard property for this with a driver defined mapping to > modes - you should implement the standard get_mode() and set_mode() (but > can always do that as a followup). > >> +/* the LTC3562 does not have a register map, instead it receives a two-byte >> + * command set. The first byte sets the mask for the output(s) to be programmed >> + * and the second byte hold the "enable" bit and the DAC code. */ >> +struct ltc3562_status { >> + u8 addr_mode; /* sub-address byte: program mask an operating mode */ >> + u8 enable_daccode; /* data byte: Enable bit and DAC code */ >> +}; > > So, I managed to find a datasheet[1] and this does actually seem to be a > standard register map. It looks like this is a 4x12 register map with > the program bytes being essentially register addresses (called sub > addresses in the datasheet), two pad bits (normally we'd include these > in the data for convenience) and the rest of the bits data. What am I > missing here? > > Otherwise this looks good. > > [1] http://cds.linear.com/docs/en/datasheet/3562fa.pdf > Things that makes me think it's NOT a register map: - You cannot read from the device - You always have to send a complete command set (two bytes) - There is data (the regulator mode) in the address byte - The 'address' is not an index but a bitmask in bits 4..7 Yeah, I think you can force-feed this to regmap. It will just make the code considerably bigger and harder to understand, and as far as I know, the one and only advantage this will bring is that you can dump the current "registers" (4x10 bits of data) via /sys/kernel/debug/regmap. I fail to see the use for regmap here, but if your view on this is "regmap or burst" then I'll implement it. Mike. Met vriendelijke groet / kind regards, Mike Looijmans System Expert TOPIC Embedded Systems Eindhovenseweg 32-C, NL-5683 KH Best Postbus 440, NL-5680 AK Best Telefoon: (+31) (0) 499 33 69 79 Telefax: (+31) (0) 499 33 69 70 E-mail: mike.looijmans@topic.nl Website: www.topic.nl Please consider the environment before printing this e-mail Topic zoekt gedreven (embedded) software specialisten! http://topic.nl/vacatures/topic-zoekt-software-engineers/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3] Add ltc3562 voltage regulator driver 2014-11-05 11:41 ` Mike Looijmans @ 2014-11-05 13:34 ` Mark Brown 0 siblings, 0 replies; 31+ messages in thread From: Mark Brown @ 2014-11-05 13:34 UTC (permalink / raw) To: Mike Looijmans; +Cc: lgirdwood, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1873 bytes --] On Wed, Nov 05, 2014 at 12:41:34PM +0100, Mike Looijmans wrote: > On 11/04/2014 09:26 PM, Mark Brown wrote: > >So, I managed to find a datasheet[1] and this does actually seem to be a > >standard register map. It looks like this is a 4x12 register map with > >the program bytes being essentially register addresses (called sub > >addresses in the datasheet), two pad bits (normally we'd include these > >in the data for convenience) and the rest of the bits data. What am I > >missing here? > Things that makes me think it's NOT a register map: > - You cannot read from the device That's entirely normal for older devices, on larger process nodes the extra digital to implement read was expensive enough for people to care and I believe there were some IP issues with implementing read support. > - You always have to send a complete command set (two bytes) > - There is data (the regulator mode) in the address byte Having to send both a register and a value is the standard thing, and with older write only devices things frequently weren't full byte multiples, 7x9 was extremely common. > - The 'address' is not an index but a bitmask in bits 4..7 That's just a sparse register map, again very common (you'll also often see holes in the register map due to undocumented registers or with larger register addresses just to keep the map neat). > I fail to see the use for regmap here, but if your view on this is "regmap > or burst" then I'll implement it. The biggest advantage is that it means that a large proportion of the code in the driver can be removed and standard implementations in the core used instead. In general removing code is good and this means that if we do things like impement support for enabling and disabling multiple regulators with a single write then the driver will support this for free. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Add ltc3562 voltage regulator driver 2014-10-29 8:15 Add ltc3562 voltage regulator driver Mike Looijmans 2014-10-29 8:16 ` [PATCH] " Mike Looijmans @ 2014-10-29 10:03 ` Mark Brown 1 sibling, 0 replies; 31+ messages in thread From: Mark Brown @ 2014-10-29 10:03 UTC (permalink / raw) To: Mike Looijmans; +Cc: lgirdwood, linux-kernel [-- Attachment #1: Type: text/plain, Size: 433 bytes --] On Wed, Oct 29, 2014 at 09:15:59AM +0100, Mike Looijmans wrote: > This patch adds the LTC3562 I2C controlled voltage regulator to > the kernel. The driver was tested and developed on a Topic Miami > board where this chip supplies the IO voltages for the FPGA pins. Please don't send cover letters for single patches, if there's anything that needs to be added just put it in the changelog or after the --- if it shouldn't go there. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2014-11-05 13:34 UTC | newest] Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-10-29 8:15 Add ltc3562 voltage regulator driver Mike Looijmans 2014-10-29 8:16 ` [PATCH] " Mike Looijmans 2014-10-29 12:30 ` Mark Brown 2014-10-30 6:47 ` Mike Looijmans 2014-10-30 10:15 ` Mark Brown 2014-10-30 10:29 ` Mike Looijmans 2014-10-30 10:53 ` Mike Looijmans 2014-10-30 10:58 ` Mark Brown 2014-10-30 11:31 ` Mike Looijmans 2014-10-30 12:04 ` Mark Brown 2014-10-30 11:26 ` [PATCH v2] " Mike Looijmans 2014-10-30 16:51 ` Mark Brown 2014-10-31 14:07 ` Mike Looijmans 2014-10-31 18:17 ` Mark Brown 2014-11-03 8:10 ` Mike Looijmans 2014-11-03 12:09 ` Mark Brown 2014-11-03 14:48 ` Mike Looijmans 2014-11-03 15:10 ` Mark Brown 2014-11-03 17:38 ` Mike Looijmans 2014-11-04 8:55 ` Mike Looijmans 2014-11-04 11:34 ` Mark Brown 2014-11-04 12:47 ` Mike Looijmans 2014-11-04 13:35 ` Mike Looijmans 2014-11-04 19:47 ` Mark Brown 2014-11-05 9:06 ` Krzysztof Kozlowski 2014-11-05 11:45 ` Mike Looijmans 2014-11-04 6:50 ` [PATCH v3] " Mike Looijmans 2014-11-04 20:26 ` Mark Brown 2014-11-05 11:41 ` Mike Looijmans 2014-11-05 13:34 ` Mark Brown 2014-10-29 10:03 ` Mark Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).