[PATCH 1/2] ARM: dts: imx6: Specify 'anatop-enable-bit' where appropriate

[PATCH 1/2] ARM: dts: imx6: Specify 'anatop-enable-bit' where appropriate

[Andrey Smirnov]

ENABLE_LINREG bit is implemented by 3P0, 1P1 and 2P5 regulators on
i.MX6. This property is present in similar code in Fresscale BSP and
made its way upstream in imx6ul.dtsi, so this patch adds this property
to the rest of i.MX6 family for completness.

Cc: yurovsky@gmail.com
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/boot/dts/imx6qdl.dtsi | 3 +++
 arch/arm/boot/dts/imx6sl.dtsi  | 3 +++
 arch/arm/boot/dts/imx6sx.dtsi  | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 89b834f..9702e18 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -635,6 +635,7 @@
 					anatop-min-bit-val = <4>;
 					anatop-min-voltage = <800000>;
 					anatop-max-voltage = <1375000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-3p0 {
@@ -649,6 +650,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2625000>;
 					anatop-max-voltage = <3400000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-2p5 {
@@ -663,6 +665,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2000000>;
 					anatop-max-voltage = <2750000>;
+					anatop-enable-bit = <0>;
 				};
 
 				reg_arm: regulator-vddcore {
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 19cbd87..a478a06f 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -522,6 +522,7 @@
 					anatop-min-bit-val = <4>;
 					anatop-min-voltage = <800000>;
 					anatop-max-voltage = <1375000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-3p0 {
@@ -536,6 +537,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2625000>;
 					anatop-max-voltage = <3400000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-2p5 {
@@ -550,6 +552,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2100000>;
 					anatop-max-voltage = <2850000>;
+					anatop-enable-bit = <0>;
 				};
 
 				reg_arm: regulator-vddcore {
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 10f3330..3de3cca 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -578,6 +578,7 @@
 					anatop-min-bit-val = <4>;
 					anatop-min-voltage = <800000>;
 					anatop-max-voltage = <1375000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-3p0 {
@@ -592,6 +593,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2625000>;
 					anatop-max-voltage = <3400000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-2p5 {
@@ -606,6 +608,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2100000>;
 					anatop-max-voltage = <2875000>;
+					anatop-enable-bit = <0>;
 				};
 
 				reg_arm: regulator-vddcore {
-- 
2.9.3

[PATCH] regulator: anatop: Add support for "anatop-enable-bit"

[Andrey Smirnov]

Add code to support support for "anatop-enable-bit" device-tree
property. This property translates to LINREG_ENABLE bit in real hardware
and is present on 1p1, 2p5 and 3p0 regulators on i.MX6 and 1p0d regulator
on i.MX7.

Cc: yurovsky@gmail.com
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---

Note: "anatop-enable-bit" has already found its way into upstream tree
before this patch (probably not on purpose). See imx6ul.dtsi and
imx7s.dtsi for concrete examples

 .../devicetree/bindings/regulator/anatop-regulator.txt       |  1 +
 drivers/regulator/anatop-regulator.c                         | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/anatop-regulator.txt b/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
index 37c4ea0..1d58c8c 100644
--- a/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
@@ -14,6 +14,7 @@ Optional properties:
 - anatop-delay-bit-shift: Bit shift for the step time register
 - anatop-delay-bit-width: Number of bits used in the step time register
 - vin-supply: The supply for this regulator
+- anatop-enable-bit: Regulator enable bit offset
 
 Any property defined as part of the core regulator
 binding, defined in regulator.txt, can also be used.
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 3a6d029..b041f27 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -301,7 +301,19 @@ static int anatop_regulator_probe(struct platform_device *pdev)
 			return -EINVAL;
 		}
 	} else {
+		u32 enable_bit;
+
 		rdesc->ops = &anatop_rops;
+
+		if (!of_property_read_u32(np, "anatop-enable-bit",
+					  &enable_bit)) {
+			anatop_rops.enable  = regulator_enable_regmap;
+			anatop_rops.disable = regulator_disable_regmap;
+			anatop_rops.is_enabled = regulator_is_enabled_regmap;
+
+			rdesc->enable_reg = sreg->control_reg;
+			rdesc->enable_mask = BIT(enable_bit);
+		}
 	}
 
 	/* register regulator */
-- 
2.9.3

Re: [PATCH] regulator: anatop: Add support for "anatop-enable-bit"

[Rob Herring]

On Tue, Jan 10, 2017 at 08:30:14AM -0800, Andrey Smirnov wrote:
> Add code to support support for "anatop-enable-bit" device-tree
> property. This property translates to LINREG_ENABLE bit in real hardware
> and is present on 1p1, 2p5 and 3p0 regulators on i.MX6 and 1p0d regulator
> on i.MX7.
> 
> Cc: yurovsky@gmail.com
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
> 
> Note: "anatop-enable-bit" has already found its way into upstream tree
> before this patch (probably not on purpose). See imx6ul.dtsi and
> imx7s.dtsi for concrete examples

But since the kernel is not using it, it doesn't really mean we have to 
keep it. Yes, other OSs could be, but that's unlikely.

>  .../devicetree/bindings/regulator/anatop-regulator.txt       |  1 +
>  drivers/regulator/anatop-regulator.c                         | 12 ++++++++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/anatop-regulator.txt b/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
> index 37c4ea0..1d58c8c 100644
> --- a/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
> +++ b/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
> @@ -14,6 +14,7 @@ Optional properties:
>  - anatop-delay-bit-shift: Bit shift for the step time register
>  - anatop-delay-bit-width: Number of bits used in the step time register
>  - vin-supply: The supply for this regulator
> +- anatop-enable-bit: Regulator enable bit offset

Looks like we have several properties describing the register layout. 
IMO, we should drop all of them and use the compatible strings to 
determine those instead.

Rob

Re: [PATCH] regulator: anatop: Add support for "anatop-enable-bit"

[Rob Herring]

On Fri, Jan 13, 2017 at 11:07:42AM -0600, Rob Herring wrote:
> On Tue, Jan 10, 2017 at 08:30:14AM -0800, Andrey Smirnov wrote:
> > Add code to support support for "anatop-enable-bit" device-tree
> > property. This property translates to LINREG_ENABLE bit in real hardware
> > and is present on 1p1, 2p5 and 3p0 regulators on i.MX6 and 1p0d regulator
> > on i.MX7.
> > 
> > Cc: yurovsky@gmail.com
> > Cc: Liam Girdwood <lgirdwood@gmail.com>
> > Cc: Mark Brown <broonie@kernel.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Shawn Guo <shawnguo@kernel.org>
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> > 
> > Note: "anatop-enable-bit" has already found its way into upstream tree
> > before this patch (probably not on purpose). See imx6ul.dtsi and
> > imx7s.dtsi for concrete examples
> 
> But since the kernel is not using it, it doesn't really mean we have to 
> keep it. Yes, other OSs could be, but that's unlikely.
> 
> >  .../devicetree/bindings/regulator/anatop-regulator.txt       |  1 +
> >  drivers/regulator/anatop-regulator.c                         | 12 ++++++++++++
> >  2 files changed, 13 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/regulator/anatop-regulator.txt b/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
> > index 37c4ea0..1d58c8c 100644
> > --- a/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
> > +++ b/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
> > @@ -14,6 +14,7 @@ Optional properties:
> >  - anatop-delay-bit-shift: Bit shift for the step time register
> >  - anatop-delay-bit-width: Number of bits used in the step time register
> >  - vin-supply: The supply for this regulator
> > +- anatop-enable-bit: Regulator enable bit offset
> 
> Looks like we have several properties describing the register layout. 
> IMO, we should drop all of them and use the compatible strings to 
> determine those instead.

NM, I see this on each regulator...

Acked-by: Rob Herring <robh@kernel.org>

Applied "regulator: anatop: Add support for "anatop-enable-bit"" to the regulator tree

[Mark Brown]

The patch

   regulator: anatop: Add support for "anatop-enable-bit"

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

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

>From ca7734ad77b4ffb83a03c80b8d64d40c7ef49263 Mon Sep 17 00:00:00 2001
From: Andrey Smirnov <andrew.smirnov@gmail.com>
Date: Tue, 10 Jan 2017 08:30:14 -0800
Subject: [PATCH] regulator: anatop: Add support for "anatop-enable-bit"

Add code to support support for "anatop-enable-bit" device-tree
property. This property translates to LINREG_ENABLE bit in real hardware
and is present on 1p1, 2p5 and 3p0 regulators on i.MX6 and 1p0d regulator
on i.MX7.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 .../devicetree/bindings/regulator/anatop-regulator.txt       |  1 +
 drivers/regulator/anatop-regulator.c                         | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/anatop-regulator.txt b/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
index 37c4ea076f88..1d58c8cfdbc0 100644
--- a/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
@@ -14,6 +14,7 @@ Optional properties:
 - anatop-delay-bit-shift: Bit shift for the step time register
 - anatop-delay-bit-width: Number of bits used in the step time register
 - vin-supply: The supply for this regulator
+- anatop-enable-bit: Regulator enable bit offset
 
 Any property defined as part of the core regulator
 binding, defined in regulator.txt, can also be used.
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 3a6d0290c54c..b041f277a38b 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -301,7 +301,19 @@ static int anatop_regulator_probe(struct platform_device *pdev)
 			return -EINVAL;
 		}
 	} else {
+		u32 enable_bit;
+
 		rdesc->ops = &anatop_rops;
+
+		if (!of_property_read_u32(np, "anatop-enable-bit",
+					  &enable_bit)) {
+			anatop_rops.enable  = regulator_enable_regmap;
+			anatop_rops.disable = regulator_disable_regmap;
+			anatop_rops.is_enabled = regulator_is_enabled_regmap;
+
+			rdesc->enable_reg = sreg->control_reg;
+			rdesc->enable_mask = BIT(enable_bit);
+		}
 	}
 
 	/* register regulator */
-- 
2.11.0

[PATCH 2/2] ARM: dts: imx7s: Adjust anatop-enable-bit for 'reg_1p0d'

[Andrey Smirnov]

In PMU_REG_1P0Dn ENABLE_LINREG is bit 0. Bit 31 is called OVERRIDE and
it serves the funtion of granting permission to GPC IP block to alter
various other bit-fields of the register. The reason why this property,
that trickeld here from Freescale BSP, is set up like that is because in
the code it came from it is used in conjunction with a notifier handler
for REGULATOR_EVENT_PRE_DO_ENABLE and REGULATOR_EVENT_PRE_DO_DISABLE
events (not found in upstream kernel) that triggers GPC to start
manipulating aforementioned other bitfields.

Since:
	a) none of the aforementioned machinery is implemented by
	   upstream
	b) using 'anatop-enable-bit' in that capacity is a bit of a
	   semantic stretch

simplify the situation by setting the value of 'anatop-enable-bit' to
point to ENABLE_LINREG (same as i.MX6).

Cc: yurovsky@gmail.com
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/boot/dts/imx7s.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 8ff2cbdd..c80d0db 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -509,7 +509,7 @@
 					anatop-min-bit-val = <8>;
 					anatop-min-voltage = <800000>;
 					anatop-max-voltage = <1200000>;
-					anatop-enable-bit = <31>;
+					anatop-enable-bit = <0>;
 				};
 			};
 
-- 
2.9.3

Re: [PATCH 1/2] ARM: dts: imx6: Specify 'anatop-enable-bit' where appropriate

[Fabio Estevam]

On Tue, Jan 10, 2017 at 2:30 PM, Andrey Smirnov
<andrew.smirnov@gmail.com> wrote:
> ENABLE_LINREG bit is implemented by 3P0, 1P1 and 2P5 regulators on
> i.MX6. This property is present in similar code in Fresscale BSP and
> made its way upstream in imx6ul.dtsi, so this patch adds this property
> to the rest of i.MX6 family for completness.

Please see:
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/patch/arch/arm/boot/dts/imx6ul.dtsi?id=27958ccdf29e9971732e02494b48be54b0691269

Re: [PATCH 1/2] ARM: dts: imx6: Specify 'anatop-enable-bit' where appropriate

[Andrey Smirnov]

On Tue, Jan 10, 2017 at 9:28 AM, Fabio Estevam <festevam@gmail.com> wrote:
> On Tue, Jan 10, 2017 at 2:30 PM, Andrey Smirnov
> <andrew.smirnov@gmail.com> wrote:
>> ENABLE_LINREG bit is implemented by 3P0, 1P1 and 2P5 regulators on
>> i.MX6. This property is present in similar code in Fresscale BSP and
>> made its way upstream in imx6ul.dtsi, so this patch adds this property
>> to the rest of i.MX6 family for completness.
>
> Please see:
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/patch/arch/arm/boot/dts/imx6ul.dtsi?id=27958ccdf29e9971732e02494b48be54b0691269

Fabio:

I submitted a patch implementing this property to LKML as well, see
https://www.spinics.net/lists/kernel/msg2418471.html

All of these patches are a part of a broader attempt to add PCIe
support for i.MX7, and on that platform this is a part of a 1P0D
regulator which supplies PCIe PHY.

I can rebase this patch set to take your commit into account, or else
let's discuss the best way to allow setting ENABLE_LINREG.

Thanks,
Andrey Smirnov