Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1623019 > unrolled thread

[PATCH 0/8] i.MX7 PCIe related device tree changes

Started byAndrey Smirnov <andrew.smirnov@gmail.com>
First post2017-04-13 15:40 +0200
Last post2017-04-19 20:30 +0200
Articles 4 on this page of 24 — 5 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/8] i.MX7 PCIe related device tree changes Andrey Smirnov <andrew.smirnov@gmail.com> - 2017-04-13 15:40 +0200
    [PATCH 1/8] Revert "ARM: dts: imx: Remove unexistant property" Andrey Smirnov <andrew.smirnov@gmail.com> - 2017-04-13 15:40 +0200
    [PATCH 4/8] ARM: dts: imx7s: Add node for GPC Andrey Smirnov <andrew.smirnov@gmail.com> - 2017-04-13 15:40 +0200
      Re: [PATCH 4/8] ARM: dts: imx7s: Add node for GPC Tyler Baker <tyler.baker@linaro.org> - 2017-04-13 21:10 +0200
        Re: [PATCH 4/8] ARM: dts: imx7s: Add node for GPC Tyler Baker <tyler.baker@linaro.org> - 2017-04-13 21:30 +0200
          Re: [PATCH 4/8] ARM: dts: imx7s: Add node for GPC Fabio Estevam <festevam@gmail.com> - 2017-04-13 22:00 +0200
            Re: [PATCH 4/8] ARM: dts: imx7s: Add node for GPC Tyler Baker <tyler.baker@linaro.org> - 2017-04-13 22:20 +0200
              Re: [PATCH 4/8] ARM: dts: imx7s: Add node for GPC Fabio Estevam <festevam@gmail.com> - 2017-04-13 23:00 +0200
                Re: [PATCH 4/8] ARM: dts: imx7s: Add node for GPC Andrey Smirnov <andrew.smirnov@gmail.com> - 2017-04-13 23:30 +0200
                Re: [PATCH 4/8] ARM: dts: imx7s: Add node for GPC Tyler Baker <tyler.baker@linaro.org> - 2017-04-13 23:40 +0200
                  Re: [PATCH 4/8] ARM: dts: imx7s: Add node for GPC Fabio Estevam <festevam@gmail.com> - 2017-04-14 00:10 +0200
        Re: [PATCH 4/8] ARM: dts: imx7s: Add node for GPC Fabio Estevam <festevam@gmail.com> - 2017-04-13 21:30 +0200
        Re: [PATCH 4/8] ARM: dts: imx7s: Add node for GPC Andrey Smirnov <andrew.smirnov@gmail.com> - 2017-04-13 23:30 +0200
      Re: [PATCH 4/8] ARM: dts: imx7s: Add node for GPC Shawn Guo <shawnguo@kernel.org> - 2017-04-14 05:50 +0200
        Re: [PATCH 4/8] ARM: dts: imx7s: Add node for GPC Andrey Smirnov <andrew.smirnov@gmail.com> - 2017-04-14 17:30 +0200
          Re: [PATCH 4/8] ARM: dts: imx7s: Add node for GPC Dong Aisheng <dongas86@gmail.com> - 2017-04-14 18:00 +0200
            Re: [PATCH 4/8] ARM: dts: imx7s: Add node for GPC Andrey Smirnov <andrew.smirnov@gmail.com> - 2017-04-14 18:00 +0200
    [PATCH 3/8] ARM: dts: imx7s: Adjust anatop-enable-bit for 'reg_1p0d' Andrey Smirnov <andrew.smirnov@gmail.com> - 2017-04-13 15:40 +0200
      Re: [PATCH 3/8] ARM: dts: imx7s: Adjust anatop-enable-bit for  'reg_1p0d' Shawn Guo <shawnguo@kernel.org> - 2017-04-14 05:30 +0200
        Re: [PATCH 3/8] ARM: dts: imx7s: Adjust anatop-enable-bit for 'reg_1p0d' Andrey Smirnov <andrew.smirnov@gmail.com> - 2017-04-14 16:40 +0200
      Re: [PATCH 3/8] ARM: dts: imx7s: Adjust anatop-enable-bit for  'reg_1p0d' Dong Aisheng <dongas86@gmail.com> - 2017-04-14 17:40 +0200
        Re: [PATCH 3/8] ARM: dts: imx7s: Adjust anatop-enable-bit for 'reg_1p0d' Andrey Smirnov <andrew.smirnov@gmail.com> - 2017-04-14 18:10 +0200
    [PATCH 2/8] ARM: dts: imx6: Specify 'anatop-enable-bit' where appropriate Andrey Smirnov <andrew.smirnov@gmail.com> - 2017-04-13 15:40 +0200
    Re: [PATCH 0/8] i.MX7 PCIe related device tree changes Tyler Baker <tyler.baker@linaro.org> - 2017-04-19 20:30 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1623731 — Re: [PATCH 3/8] ARM: dts: imx7s: Adjust anatop-enable-bit for 'reg_1p0d'

FromDong Aisheng <dongas86@gmail.com>
Date2017-04-14 17:40 +0200
SubjectRe: [PATCH 3/8] ARM: dts: imx7s: Adjust anatop-enable-bit for 'reg_1p0d'
Message-ID<twbui-2Mv-25@gated-at.bofh.it>
In reply to#1623027
On Thu, Apr 13, 2017 at 06:32:37AM -0700, Andrey Smirnov wrote:
> In PMU_REG_1P0Dn ENABLE_LINREG is bit 0. Bit 31 is called OVERRIDE and
> it serves the function of granting permission to GPC IP block to alter
> various bit-fields of the register. The reason why this property, that
> trickeld here from Freescale BSP, is set to 31 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

Yes, this does is a bit of semantic stretch.
FSL using is combined with regulator notify and that do bring a bit
of complexity.

I'm not sure if it's good to introduce another anatop-override-bit
to separate, but i'm a bit scare since there's already many....

> 
> 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: 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
> Cc: linux-arm-kernel@lists.infradead.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 22c9788..8fee299 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -516,7 +516,7 @@
>  					anatop-min-bit-val = <8>;
>  					anatop-min-voltage = <800000>;
>  					anatop-max-voltage = <1200000>;
> -					anatop-enable-bit = <31>;
> +					anatop-enable-bit = <0>;

The change of this line seems already exist in patch 1.

Regards
Dong Aisheng

[toc] | [prev] | [next] | [standalone]

#1623756 — Re: [PATCH 3/8] ARM: dts: imx7s: Adjust anatop-enable-bit for 'reg_1p0d'

FromAndrey Smirnov <andrew.smirnov@gmail.com>
Date2017-04-14 18:10 +0200
SubjectRe: [PATCH 3/8] ARM: dts: imx7s: Adjust anatop-enable-bit for 'reg_1p0d'
Message-ID<twbXk-3bL-23@gated-at.bofh.it>
In reply to#1623731
On Fri, Apr 14, 2017 at 8:32 AM, Dong Aisheng <dongas86@gmail.com> wrote:
> On Thu, Apr 13, 2017 at 06:32:37AM -0700, Andrey Smirnov wrote:
>> In PMU_REG_1P0Dn ENABLE_LINREG is bit 0. Bit 31 is called OVERRIDE and
>> it serves the function of granting permission to GPC IP block to alter
>> various bit-fields of the register. The reason why this property, that
>> trickeld here from Freescale BSP, is set to 31 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
>
> Yes, this does is a bit of semantic stretch.
> FSL using is combined with regulator notify and that do bring a bit
> of complexity.
>
> I'm not sure if it's good to introduce another anatop-override-bit
> to separate, but i'm a bit scare since there's already many....
>

All of those Freescale specific events are replaced by GPCv2 power
domain driver that we discussed in another thread. Since regulator
driver for ANADIG sets up all of the voltages manually (or, more
specifically, GPCv2 driver sets them up via regulator API) I didn't
see any reason to use OVERRIDE instead of just ENABLE.

From reading the RM it seems that main reason for using OVERRIDE as
opposed to ENABLE would be to leverage advanced hardware power
management capabilities of the SoC which I don't think are implemented
in upstream kernel. Do you think there's a use-case for
anatop-override-bit property?

>>
>> 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: 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
>> Cc: linux-arm-kernel@lists.infradead.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 22c9788..8fee299 100644
>> --- a/arch/arm/boot/dts/imx7s.dtsi
>> +++ b/arch/arm/boot/dts/imx7s.dtsi
>> @@ -516,7 +516,7 @@
>>                                       anatop-min-bit-val = <8>;
>>                                       anatop-min-voltage = <800000>;
>>                                       anatop-max-voltage = <1200000>;
>> -                                     anatop-enable-bit = <31>;
>> +                                     anatop-enable-bit = <0>;
>
> The change of this line seems already exist in patch 1.

I am going to squash all three patches into a single one.

Thanks,
Andrey Smirnov

[toc] | [prev] | [next] | [standalone]

#1623029 — [PATCH 2/8] ARM: dts: imx6: Specify 'anatop-enable-bit' where appropriate

FromAndrey Smirnov <andrew.smirnov@gmail.com>
Date2017-04-13 15:40 +0200
Subject[PATCH 2/8] ARM: dts: imx6: Specify 'anatop-enable-bit' where appropriate
Message-ID<tvN8C-3CF-33@gated-at.bofh.it>
In reply to#1623019
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: 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
Cc: linux-arm-kernel@lists.infradead.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 e426faa..0576ef6 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -644,6 +644,7 @@
 					anatop-min-bit-val = <4>;
 					anatop-min-voltage = <800000>;
 					anatop-max-voltage = <1375000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-3p0 {
@@ -658,6 +659,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2625000>;
 					anatop-max-voltage = <3400000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-2p5 {
@@ -672,6 +674,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2100000>;
 					anatop-max-voltage = <2875000>;
+					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 cc9572e..3243af4 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -530,6 +530,7 @@
 					anatop-min-bit-val = <4>;
 					anatop-min-voltage = <800000>;
 					anatop-max-voltage = <1375000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-3p0 {
@@ -544,6 +545,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2625000>;
 					anatop-max-voltage = <3400000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-2p5 {
@@ -558,6 +560,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 3f1416b..f16b9df 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -587,6 +587,7 @@
 					anatop-min-bit-val = <4>;
 					anatop-min-voltage = <800000>;
 					anatop-max-voltage = <1375000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-3p0 {
@@ -601,6 +602,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2625000>;
 					anatop-max-voltage = <3400000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-2p5 {
@@ -615,6 +617,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

[toc] | [prev] | [next] | [standalone]

#1626683

FromTyler Baker <tyler.baker@linaro.org>
Date2017-04-19 20:30 +0200
Message-ID<ty2wy-6N4-23@gated-at.bofh.it>
In reply to#1623019
On 13 April 2017 at 06:32, Andrey Smirnov <andrew.smirnov@gmail.com> wrote:
> Shawn, everyone:
>
> This series includes changes made to device-tree in order to support
> PCIe on i.MX7 platform. They include:
>
>      - Bringing 'anatop-enable-bit' property of ANATOP regulators back
>        and extending it to all of the HW it is applicable to
>
>      - Adding GPCv2 node for i.MX7 (which was missing, despite the
>        irqchip driver for it being in the tree for quite some time)
>
>      - Adding a PCIe node for i.MX7
>
>      - Adding GPIO expander used by PCIe and enabling PCIe node from
>        above on i.MX7 based Sabre board
>
> As usual, feedback is welcome.
>
> Thanks,
> Andrey Smrinov
>
> Andrey Smirnov (8):
>   Revert "ARM: dts: imx: Remove unexistant property"
>   ARM: dts: imx6: Specify 'anatop-enable-bit' where appropriate
>   ARM: dts: imx7s: Adjust anatop-enable-bit for 'reg_1p0d'
>   ARM: dts: imx7s: Add node for GPC
>   ARM: dts: imx7s: Mark 'gpr' compatible with i.MX6 variant
>   ARM: dts: imx7d-sdb: Add GPIO expander node
>   ARM: dts: imx7d: Add node for PCIe controller
>   ARM: dts: imx7d-sdb: Enable PCIe peripheral

FWIW:

Tested-by: Tyler Baker <tyler.baker@linaro.org>

This whole series on top of v4.11-rc7, with the addition of the iMX7
GPCv2 and reset driver. Tested on a imx7d-cl-som with a CUK Killer
Doubleshot Wireless-AC 1535 wlan/bt radio using a mini pcie to m2
adapter. Confirmed that the radio was able to associate with an AP
using WPA2, and connected to multiple devices using 6lowpan over BLE.

Tyler

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web