* [PATCH v2 0/2] Re-Enable support to disable switch regulators @ 2018-07-13 12:50 Marco Felsch 2018-07-13 12:50 ` [PATCH v2 1/3] dt-bindings: pfuze100: add optional disable switch-regulators binding Marco Felsch 2018-07-13 12:50 ` [PATCH v2 2/3] regulator: pfuze100: add support to en-/disable switch regulators Marco Felsch 0 siblings, 2 replies; 8+ messages in thread From: Marco Felsch @ 2018-07-13 12:50 UTC (permalink / raw) To: lgirdwood, broonie, robh+dt, mark.rutland Cc: fabio.estevam, Anson.Huang, kernel, devicetree, linux-kernel Hi, Anson had added the support to disable the switched regulators, but there were regressions [1] with old dtb's, so the commit was reverted [2]. At all, the support to disable the switch regulators seems to me to be a good feature. But we have to add a special dt-property to avoid regressions with older kernels. The property allows the user to decide if the switch regulators should be disabled or not. Since the revert patch is on Marks regulator repo, my patches are based on his repo too. Each commit has a changelog, so I ommit it here. I tested it on a custom board. Used Kernel: Repo: git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git Branch: regulator/for-4.19 Regards, Marco [1] https://patchwork.kernel.org/patch/10490381/ [2] https://patchwork.kernel.org/patch/10500333/ Marco Felsch (3): dt-bindings: pfuze100: add optional disable switch-regulators binding regulator: pfuze100: add support to en-/disable switch regulators .../bindings/regulator/pfuze100.txt | 11 ++++++ drivers/regulator/core.c | 2 ++ drivers/regulator/pfuze100-regulator.c | 36 +++++++++++++++++++ 3 files changed, 49 insertions(+) -- 2.18.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/3] dt-bindings: pfuze100: add optional disable switch-regulators binding 2018-07-13 12:50 [PATCH v2 0/2] Re-Enable support to disable switch regulators Marco Felsch @ 2018-07-13 12:50 ` Marco Felsch 2018-07-13 15:03 ` Mark Brown 2018-07-16 17:55 ` Rob Herring 2018-07-13 12:50 ` [PATCH v2 2/3] regulator: pfuze100: add support to en-/disable switch regulators Marco Felsch 1 sibling, 2 replies; 8+ messages in thread From: Marco Felsch @ 2018-07-13 12:50 UTC (permalink / raw) To: lgirdwood, broonie, robh+dt, mark.rutland Cc: fabio.estevam, Anson.Huang, kernel, devicetree, linux-kernel This binding is used to keep the backward compatibility with the current dtb's [1]. The binding informs the driver that the unused switch regulators can be disabled. If it is not specified, the driver doesn't disable the switch regulators. [1] https://patchwork.kernel.org/patch/10490381/ Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> --- Changes in V2: - add more information about the binding - rename binding and add vendor prefix .../devicetree/bindings/regulator/pfuze100.txt | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Documentation/devicetree/bindings/regulator/pfuze100.txt b/Documentation/devicetree/bindings/regulator/pfuze100.txt index 672c939045ff..2c46b8d368db 100644 --- a/Documentation/devicetree/bindings/regulator/pfuze100.txt +++ b/Documentation/devicetree/bindings/regulator/pfuze100.txt @@ -4,6 +4,17 @@ Required properties: - compatible: "fsl,pfuze100", "fsl,pfuze200", "fsl,pfuze3000", "fsl,pfuze3001" - reg: I2C slave address +Optional properties: +- fsl,pfuze-support-disable: Boolean, if present disable all unused switch + regulators to save power consumption. Attention, till 4.18 these regulators + were always on without specifying "regulator-always-on". So be sure to mark + import regulators as "regulator-always-on" (e.g. DDR ref, DDR supply). If + not present, the switched regualtors are always on and can't be disabled. + This binding is a workaround to keep backward compatibility with old dtb's + which rely on the fact that the switched regulators are always on and don't + mark them explicit as "regulator-always-on". On new dtbs this property should + always be present. + Required child node: - regulators: This is the list of child nodes that specify the regulator initialization data for defined regulators. Please refer to below doc -- 2.18.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: pfuze100: add optional disable switch-regulators binding 2018-07-13 12:50 ` [PATCH v2 1/3] dt-bindings: pfuze100: add optional disable switch-regulators binding Marco Felsch @ 2018-07-13 15:03 ` Mark Brown 2018-07-16 7:25 ` Marco Felsch 2018-07-16 17:55 ` Rob Herring 1 sibling, 1 reply; 8+ messages in thread From: Mark Brown @ 2018-07-13 15:03 UTC (permalink / raw) To: Marco Felsch Cc: lgirdwood, robh+dt, mark.rutland, fabio.estevam, Anson.Huang, kernel, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 424 bytes --] On Fri, Jul 13, 2018 at 02:50:01PM +0200, Marco Felsch wrote: > +Optional properties: > +- fsl,pfuze-support-disable: Boolean, if present disable all unused switch > + regulators to save power consumption. Attention, till 4.18 these regulators The property name sounds like it affects all the regulators but really it's just the sw ones - how about adding -sw at the end? Bit of a bikeshed but it does end up in an ABI. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: pfuze100: add optional disable switch-regulators binding 2018-07-13 15:03 ` Mark Brown @ 2018-07-16 7:25 ` Marco Felsch 2018-07-18 12:09 ` Mark Brown 0 siblings, 1 reply; 8+ messages in thread From: Marco Felsch @ 2018-07-16 7:25 UTC (permalink / raw) To: Mark Brown Cc: lgirdwood, robh+dt, mark.rutland, fabio.estevam, Anson.Huang, kernel, devicetree, linux-kernel Hi Mark, thanks for the review. On 18-07-13 16:03, Mark Brown wrote: > On Fri, Jul 13, 2018 at 02:50:01PM +0200, Marco Felsch wrote: > > > +Optional properties: > > +- fsl,pfuze-support-disable: Boolean, if present disable all unused switch > > + regulators to save power consumption. Attention, till 4.18 these regulators > > The property name sounds like it affects all the regulators but really > it's just the sw ones - how about adding -sw at the end? Bit of a > bikeshed but it does end up in an ABI. You're right, it sounds better. So the new binding will be fsl,pfuze-support-disable-sw. Are there any other comments about the implementation? Regards, Marco ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: pfuze100: add optional disable switch-regulators binding 2018-07-16 7:25 ` Marco Felsch @ 2018-07-18 12:09 ` Mark Brown 0 siblings, 0 replies; 8+ messages in thread From: Mark Brown @ 2018-07-18 12:09 UTC (permalink / raw) To: Marco Felsch Cc: lgirdwood, robh+dt, mark.rutland, fabio.estevam, Anson.Huang, kernel, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 225 bytes --] On Mon, Jul 16, 2018 at 09:25:16AM +0200, Marco Felsch wrote: > You're right, it sounds better. So the new binding will be > fsl,pfuze-support-disable-sw. Are there any other comments about the > implementation? Not AFAIR. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: pfuze100: add optional disable switch-regulators binding 2018-07-13 12:50 ` [PATCH v2 1/3] dt-bindings: pfuze100: add optional disable switch-regulators binding Marco Felsch 2018-07-13 15:03 ` Mark Brown @ 2018-07-16 17:55 ` Rob Herring 2018-07-18 14:58 ` Marco Felsch 1 sibling, 1 reply; 8+ messages in thread From: Rob Herring @ 2018-07-16 17:55 UTC (permalink / raw) To: Marco Felsch Cc: lgirdwood, broonie, mark.rutland, fabio.estevam, Anson.Huang, kernel, devicetree, linux-kernel On Fri, Jul 13, 2018 at 02:50:01PM +0200, Marco Felsch wrote: > This binding is used to keep the backward compatibility with the current > dtb's [1]. The binding informs the driver that the unused switch regulators > can be disabled. > If it is not specified, the driver doesn't disable the switch regulators. > > [1] https://patchwork.kernel.org/patch/10490381/ > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > --- > Changes in V2: > - add more information about the binding > - rename binding and add vendor prefix > > .../devicetree/bindings/regulator/pfuze100.txt | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/regulator/pfuze100.txt b/Documentation/devicetree/bindings/regulator/pfuze100.txt > index 672c939045ff..2c46b8d368db 100644 > --- a/Documentation/devicetree/bindings/regulator/pfuze100.txt > +++ b/Documentation/devicetree/bindings/regulator/pfuze100.txt > @@ -4,6 +4,17 @@ Required properties: > - compatible: "fsl,pfuze100", "fsl,pfuze200", "fsl,pfuze3000", "fsl,pfuze3001" > - reg: I2C slave address > > +Optional properties: > +- fsl,pfuze-support-disable: Boolean, if present disable all unused switch > + regulators to save power consumption. Attention, till 4.18 these regulators You shouldn't have kernel version info in bindings. > + were always on without specifying "regulator-always-on". So be sure to mark > + import regulators as "regulator-always-on" (e.g. DDR ref, DDR supply). If s/import/important/ > + not present, the switched regualtors are always on and can't be disabled. > + This binding is a workaround to keep backward compatibility with old dtb's > + which rely on the fact that the switched regulators are always on and don't > + mark them explicit as "regulator-always-on". On new dtbs this property should > + always be present. > + > Required child node: > - regulators: This is the list of child nodes that specify the regulator > initialization data for defined regulators. Please refer to below doc > -- > 2.18.0 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: pfuze100: add optional disable switch-regulators binding 2018-07-16 17:55 ` Rob Herring @ 2018-07-18 14:58 ` Marco Felsch 0 siblings, 0 replies; 8+ messages in thread From: Marco Felsch @ 2018-07-18 14:58 UTC (permalink / raw) To: Rob Herring Cc: lgirdwood, broonie, mark.rutland, fabio.estevam, Anson.Huang, kernel, devicetree, linux-kernel Hi Rob, thanks for the review. I will integrate it in a v3. Regards, Marco On 18-07-16 11:55, Rob Herring wrote: > On Fri, Jul 13, 2018 at 02:50:01PM +0200, Marco Felsch wrote: > > This binding is used to keep the backward compatibility with the current > > dtb's [1]. The binding informs the driver that the unused switch regulators > > can be disabled. > > If it is not specified, the driver doesn't disable the switch regulators. > > > > [1] https://patchwork.kernel.org/patch/10490381/ > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > > > --- > > Changes in V2: > > - add more information about the binding > > - rename binding and add vendor prefix > > > > .../devicetree/bindings/regulator/pfuze100.txt | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/regulator/pfuze100.txt b/Documentation/devicetree/bindings/regulator/pfuze100.txt > > index 672c939045ff..2c46b8d368db 100644 > > --- a/Documentation/devicetree/bindings/regulator/pfuze100.txt > > +++ b/Documentation/devicetree/bindings/regulator/pfuze100.txt > > @@ -4,6 +4,17 @@ Required properties: > > - compatible: "fsl,pfuze100", "fsl,pfuze200", "fsl,pfuze3000", "fsl,pfuze3001" > > - reg: I2C slave address > > > > +Optional properties: > > +- fsl,pfuze-support-disable: Boolean, if present disable all unused switch > > + regulators to save power consumption. Attention, till 4.18 these regulators > > You shouldn't have kernel version info in bindings. > > > + were always on without specifying "regulator-always-on". So be sure to mark > > + import regulators as "regulator-always-on" (e.g. DDR ref, DDR supply). If > > s/import/important/ > > > + not present, the switched regualtors are always on and can't be disabled. > > + This binding is a workaround to keep backward compatibility with old dtb's > > + which rely on the fact that the switched regulators are always on and don't > > + mark them explicit as "regulator-always-on". On new dtbs this property should > > + always be present. > > + > > Required child node: > > - regulators: This is the list of child nodes that specify the regulator > > initialization data for defined regulators. Please refer to below doc > > -- > > 2.18.0 > > > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5082 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/3] regulator: pfuze100: add support to en-/disable switch regulators 2018-07-13 12:50 [PATCH v2 0/2] Re-Enable support to disable switch regulators Marco Felsch 2018-07-13 12:50 ` [PATCH v2 1/3] dt-bindings: pfuze100: add optional disable switch-regulators binding Marco Felsch @ 2018-07-13 12:50 ` Marco Felsch 1 sibling, 0 replies; 8+ messages in thread From: Marco Felsch @ 2018-07-13 12:50 UTC (permalink / raw) To: lgirdwood, broonie, robh+dt, mark.rutland Cc: fabio.estevam, Anson.Huang, kernel, devicetree, linux-kernel Add enable/disable support for switch regulators on pfuze100. Based on commit 5fe156f1cab4 ("regulator: pfuze100: add enable/disable for switch") which is reverted due to boot regressions by commit 464a5686e6c9 ("regulator: Revert "regulator: pfuze100: add enable/disable for switch""). Disabling the switch regulators will only be done if the user specifies "fsl,pfuze-support-disable" in its device tree to keep backward compatibility with current dtb's [1]. [1] https://patchwork.kernel.org/patch/10490381/ Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> --- Changes in v2: - Don't trick the framework, use a seperate ops struct to register the correct callbacks. - Set the desc en/disable_val and enable_time only if it necessary - Change the dt property binding Changes since https://patchwork.kernel.org/patch/10405723/ - Use DT property to keep backward compatibility - Use the default register val 0x8 as enable_val drivers/regulator/pfuze100-regulator.c | 36 ++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/drivers/regulator/pfuze100-regulator.c b/drivers/regulator/pfuze100-regulator.c index cde6eda1d283..b70d16d5a2ff 100644 --- a/drivers/regulator/pfuze100-regulator.c +++ b/drivers/regulator/pfuze100-regulator.c @@ -17,6 +17,8 @@ #include <linux/slab.h> #include <linux/regmap.h> +#define PFUZE_FLAG_DISABLE_SW BIT(1) + #define PFUZE_NUMREGS 128 #define PFUZE100_VOL_OFFSET 0 #define PFUZE100_STANDBY_OFFSET 1 @@ -50,10 +52,12 @@ struct pfuze_regulator { struct regulator_desc desc; unsigned char stby_reg; unsigned char stby_mask; + bool sw_reg; }; struct pfuze_chip { int chip_id; + int flags; struct regmap *regmap; struct device *dev; struct pfuze_regulator regulator_descs[PFUZE100_MAX_REGULATOR]; @@ -170,6 +174,17 @@ static const struct regulator_ops pfuze100_sw_regulator_ops = { .set_ramp_delay = pfuze100_set_ramp_delay, }; +static const struct regulator_ops pfuze100_sw_disable_regulator_ops = { + .enable = regulator_enable_regmap, + .disable = regulator_disable_regmap, + .is_enabled = regulator_is_enabled_regmap, + .list_voltage = regulator_list_voltage_linear, + .set_voltage_sel = regulator_set_voltage_sel_regmap, + .get_voltage_sel = regulator_get_voltage_sel_regmap, + .set_voltage_time_sel = regulator_set_voltage_time_sel, + .set_ramp_delay = pfuze100_set_ramp_delay, +}; + static const struct regulator_ops pfuze100_swb_regulator_ops = { .enable = regulator_enable_regmap, .disable = regulator_disable_regmap, @@ -209,9 +224,12 @@ static const struct regulator_ops pfuze100_swb_regulator_ops = { .uV_step = (step), \ .vsel_reg = (base) + PFUZE100_VOL_OFFSET, \ .vsel_mask = 0x3f, \ + .enable_reg = (base) + PFUZE100_MODE_OFFSET, \ + .enable_mask = 0xf, \ }, \ .stby_reg = (base) + PFUZE100_STANDBY_OFFSET, \ .stby_mask = 0x3f, \ + .sw_reg = true, \ } #define PFUZE100_SWB_REG(_chip, _name, base, mask, voltages) \ @@ -471,6 +489,9 @@ static int pfuze_parse_regulators_dt(struct pfuze_chip *chip) if (!np) return -EINVAL; + if (of_property_read_bool(np, "fsl,pfuze-support-disable")) + chip->flags |= PFUZE_FLAG_DISABLE_SW; + parent = of_get_child_by_name(np, "regulators"); if (!parent) { dev_err(dev, "regulators node not found\n"); @@ -703,6 +724,21 @@ static int pfuze100_regulator_probe(struct i2c_client *client, } } + /* + * Allow SW regulators to turn off. Checking it trough a flag is + * a workaround to keep the backward compatibility with existing + * old dtb's which may relay on the fact that we didn't disable + * the switched regulator till yet. + */ + if (pfuze_chip->flags & PFUZE_FLAG_DISABLE_SW) { + if (pfuze_chip->regulator_descs[i].sw_reg) { + desc->ops = &pfuze100_sw_disable_regulator_ops; + desc->enable_val = 0x8; + desc->disable_val = 0x0; + desc->enable_time = 500; + } + } + config.dev = &client->dev; config.init_data = init_data; config.driver_data = pfuze_chip; -- 2.18.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-07-18 14:59 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-07-13 12:50 [PATCH v2 0/2] Re-Enable support to disable switch regulators Marco Felsch 2018-07-13 12:50 ` [PATCH v2 1/3] dt-bindings: pfuze100: add optional disable switch-regulators binding Marco Felsch 2018-07-13 15:03 ` Mark Brown 2018-07-16 7:25 ` Marco Felsch 2018-07-18 12:09 ` Mark Brown 2018-07-16 17:55 ` Rob Herring 2018-07-18 14:58 ` Marco Felsch 2018-07-13 12:50 ` [PATCH v2 2/3] regulator: pfuze100: add support to en-/disable switch regulators Marco Felsch
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).