[PATCH] regulator: wm8994: Add an off-on delay for WM8994 variant

[PATCH] regulator: wm8994: Add an off-on delay for WM8994 variant

[Jonathan Bakker]

As per Table 130 of the wm8994 datasheet at [1], there is an off-on
delay for LDO1 and LDO2.  In the wm8958 datasheet [2], I could not
find any reference to it.  I could not find a wm1811 datasheet to
double-check there, but as no one has complained presumably it works
without it.

This solves the issue on Samsung Aries boards with a wm8994 where
register writes fail when the device is powered off and back-on
quickly.

[1] https://statics.cirrus.com/pubs/proDatasheet/WM8994_Rev4.6.pdf
[2] https://statics.cirrus.com/pubs/proDatasheet/WM8958_v3.5.pdf

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 drivers/regulator/wm8994-regulator.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/wm8994-regulator.c b/drivers/regulator/wm8994-regulator.c
index cadea0344486..135fdf3f9fdd 100644
--- a/drivers/regulator/wm8994-regulator.c
+++ b/drivers/regulator/wm8994-regulator.c
@@ -70,7 +70,7 @@ static const struct regulator_ops wm8994_ldo2_ops = {
 	.set_voltage_sel = regulator_set_voltage_sel_regmap,
 };
 
-static const struct regulator_desc wm8994_ldo_desc[] = {
+static struct regulator_desc wm8994_ldo_desc[] = {
 	{
 		.name = "LDO1",
 		.id = 1,
@@ -167,6 +167,10 @@ static int wm8994_ldo_probe(struct platform_device *pdev)
 		ldo->init_data = *pdata->ldo[id].init_data;
 	}
 
+	/* WM8994 requires an off-on delay while others do not */
+	if (ldo->wm8994->type == WM8994)
+		wm8994_ldo_desc[id].off_on_delay = 36000;
+
 	/*
 	 * At this point the GPIO descriptor is handled over to the
 	 * regulator core and we need not worry about it on the
-- 
2.20.1

Re: [PATCH] regulator: wm8994: Add an off-on delay for WM8994 variant

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 577 bytes --]

On Sun, Mar 27, 2022 at 03:15:53PM -0700, Jonathan Bakker wrote:

>  
> +	/* WM8994 requires an off-on delay while others do not */
> +	if (ldo->wm8994->type == WM8994)
> +		wm8994_ldo_desc[id].off_on_delay = 36000;

You shouldn't modify the description - this wouldn't work in the
unusual situation where there were one of these devices and is a
bad pattern to set in case someone copies this to another device
where it's more likely there could be multiple instances.  It is
much better to provide two descriptions and select the one which
is needed at runtime.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2] regulator: wm8994: Add an off-on delay for WM8994 variant

[Jonathan Bakker]

As per Table 130 of the wm8994 datasheet at [1], there is an off-on
delay for LDO1 and LDO2.  In the wm8958 datasheet [2], I could not
find any reference to it.  I could not find a wm1811 datasheet to
double-check there, but as no one has complained presumably it works
without it.

This solves the issue on Samsung Aries boards with a wm8994 where
register writes fail when the device is powered off and back-on
quickly.

[1] https://statics.cirrus.com/pubs/proDatasheet/WM8994_Rev4.6.pdf
[2] https://statics.cirrus.com/pubs/proDatasheet/WM8958_v3.5.pdf

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
Changes in v2
- Duplicate regulator_desc, keep it const, and select which one
  instead of modifying the one existing one
---
 drivers/regulator/wm8994-regulator.c | 42 ++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/wm8994-regulator.c b/drivers/regulator/wm8994-regulator.c
index cadea0344486..40befdd9dfa9 100644
--- a/drivers/regulator/wm8994-regulator.c
+++ b/drivers/regulator/wm8994-regulator.c
@@ -71,6 +71,35 @@ static const struct regulator_ops wm8994_ldo2_ops = {
 };
 
 static const struct regulator_desc wm8994_ldo_desc[] = {
+	{
+		.name = "LDO1",
+		.id = 1,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = WM8994_LDO1_MAX_SELECTOR + 1,
+		.vsel_reg = WM8994_LDO_1,
+		.vsel_mask = WM8994_LDO1_VSEL_MASK,
+		.ops = &wm8994_ldo1_ops,
+		.min_uV = 2400000,
+		.uV_step = 100000,
+		.enable_time = 3000,
+		.off_on_delay = 36000,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "LDO2",
+		.id = 2,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = WM8994_LDO2_MAX_SELECTOR + 1,
+		.vsel_reg = WM8994_LDO_2,
+		.vsel_mask = WM8994_LDO2_VSEL_MASK,
+		.ops = &wm8994_ldo2_ops,
+		.enable_time = 3000,
+		.off_on_delay = 36000,
+		.owner = THIS_MODULE,
+	},
+};
+
+static const struct regulator_desc wm8958_ldo_desc[] = {
 	{
 		.name = "LDO1",
 		.id = 1,
@@ -172,9 +201,16 @@ static int wm8994_ldo_probe(struct platform_device *pdev)
 	 * regulator core and we need not worry about it on the
 	 * error path.
 	 */
-	ldo->regulator = devm_regulator_register(&pdev->dev,
-						 &wm8994_ldo_desc[id],
-						 &config);
+	if (ldo->wm8994->type == WM8994) {
+		ldo->regulator = devm_regulator_register(&pdev->dev,
+							 &wm8994_ldo_desc[id],
+							 &config);
+	} else {
+		ldo->regulator = devm_regulator_register(&pdev->dev,
+							 &wm8958_ldo_desc[id],
+							 &config);
+	}
+
 	if (IS_ERR(ldo->regulator)) {
 		ret = PTR_ERR(ldo->regulator);
 		dev_err(wm8994->dev, "Failed to register LDO%d: %d\n",
-- 
2.20.1

Re: [PATCH v2] regulator: wm8994: Add an off-on delay for WM8994 variant

[Charles Keepax]

On Sun, Mar 27, 2022 at 06:01:54PM -0700, Jonathan Bakker wrote:
> As per Table 130 of the wm8994 datasheet at [1], there is an off-on
> delay for LDO1 and LDO2.  In the wm8958 datasheet [2], I could not
> find any reference to it.  I could not find a wm1811 datasheet to
> double-check there, but as no one has complained presumably it works
> without it.
> 
> This solves the issue on Samsung Aries boards with a wm8994 where
> register writes fail when the device is powered off and back-on
> quickly.
> 
> [1] https://statics.cirrus.com/pubs/proDatasheet/WM8994_Rev4.6.pdf
> [2] https://statics.cirrus.com/pubs/proDatasheet/WM8958_v3.5.pdf
> 
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> ---

I can confirm the 1811 also doesn't mention the cycle time in the
datasheet. So that checks out.

Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Part of me wonders if we should just apply it to 1811 and 8958
anyway, I suspect they have the same IP for the LDO and its just
the datasheets never got updated. I had a go chasing the apps guys
here but we are a little short on people who remember details of
these parts. So I guess we trust the datasheets for now, unless
you have any strong feelings, Mark?

Thanks,
Charles

Re: [PATCH v2] regulator: wm8994: Add an off-on delay for WM8994 variant

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 643 bytes --]

On Mon, Mar 28, 2022 at 01:11:32PM +0000, Charles Keepax wrote:

> Part of me wonders if we should just apply it to 1811 and 8958
> anyway, I suspect they have the same IP for the LDO and its just
> the datasheets never got updated. I had a go chasing the apps guys
> here but we are a little short on people who remember details of
> these parts. So I guess we trust the datasheets for now, unless
> you have any strong feelings, Mark?

With something like this the on/off time may be a requirement of the
thing being powered rather than the regulator, the device might see
enough power loss to confuse state but not trip the power on reset.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] regulator: wm8994: Add an off-on delay for WM8994 variant

[Charles Keepax]

On Mon, Mar 28, 2022 at 03:36:53PM +0100, Mark Brown wrote:
> On Mon, Mar 28, 2022 at 01:11:32PM +0000, Charles Keepax wrote:
> 
> > Part of me wonders if we should just apply it to 1811 and 8958
> > anyway, I suspect they have the same IP for the LDO and its just
> > the datasheets never got updated. I had a go chasing the apps guys
> > here but we are a little short on people who remember details of
> > these parts. So I guess we trust the datasheets for now, unless
> > you have any strong feelings, Mark?
> 
> With something like this the on/off time may be a requirement of the
> thing being powered rather than the regulator, the device might see
> enough power loss to confuse state but not trip the power on reset.

Aye a good point. I am certainly happy to stick with trusting the
datasheets here.

Thanks,
Charles

Re: [PATCH v2] regulator: wm8994: Add an off-on delay for WM8994 variant

[Mark Brown]

On Sun, 27 Mar 2022 18:01:54 -0700, Jonathan Bakker wrote:
> As per Table 130 of the wm8994 datasheet at [1], there is an off-on
> delay for LDO1 and LDO2.  In the wm8958 datasheet [2], I could not
> find any reference to it.  I could not find a wm1811 datasheet to
> double-check there, but as no one has complained presumably it works
> without it.
> 
> This solves the issue on Samsung Aries boards with a wm8994 where
> register writes fail when the device is powered off and back-on
> quickly.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/1] regulator: wm8994: Add an off-on delay for WM8994 variant
      commit: 92d96b603738ec4f35cde7198c303ae264dd47cb

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark