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
arch/arm/boot/dts/imx6qdl.dtsi | 3 +++
arch/arm/boot/dts/imx6sl.dtsi | 3 +++
arch/arm/boot/dts/imx6sx.dtsi | 3 +++
arch/arm/boot/dts/imx6ul.dtsi | 1 +
arch/arm/boot/dts/imx7d-sdb.dts | 39 +++++++++++++++++++++++++++++++++++++++
arch/arm/boot/dts/imx7d.dtsi | 38 ++++++++++++++++++++++++++++++++++++++
arch/arm/boot/dts/imx7s.dtsi | 31 +++++++++++++++++++++++++++++--
7 files changed, 116 insertions(+), 2 deletions(-)
--
2.9.3
Add node for U38, a 74LV595PW serial-in shift register that acts as a
GPIO expander on the board.
Cc: [email protected]
Cc: Sascha Hauer <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
arch/arm/boot/dts/imx7d-sdb.dts | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts
index 5be01a1..e0ff276 100644
--- a/arch/arm/boot/dts/imx7d-sdb.dts
+++ b/arch/arm/boot/dts/imx7d-sdb.dts
@@ -52,6 +52,30 @@
reg = <0x80000000 0x80000000>;
};
+ spi4 {
+ compatible = "spi-gpio";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_spi1>;
+ status = "okay";
+ gpio-sck = <&gpio1 13 0>;
+ gpio-mosi = <&gpio1 9 0>;
+ cs-gpios = <&gpio1 12 0>;
+ num-chipselects = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ gpio_spi: gpio_spi@0 {
+ compatible = "fairchild,74hc595";
+ gpio-controller;
+ #gpio-cells = <2>;
+ reg = <0>;
+ registers-number = <1>;
+ /* Enable PERI_3V3, SENSOR_RST_B and HDMI_RST*/
+ registers-default = /bits/ 8 <0x74>;
+ spi-max-frequency = <100000>;
+ };
+ };
+
regulators {
compatible = "simple-bus";
#address-cells = <1>;
@@ -642,5 +666,13 @@
fsl,pins = <
MX7D_PAD_LPSR_GPIO1_IO01__PWM1_OUT 0x110b0
>;
+
+ pinctrl_spi1: spi1grp {
+ fsl,pins = <
+ MX7D_PAD_GPIO1_IO09__GPIO1_IO9 0x59
+ MX7D_PAD_GPIO1_IO12__GPIO1_IO12 0x59
+ MX7D_PAD_GPIO1_IO13__GPIO1_IO13 0x59
+ >;
+ };
};
};
--
2.9.3
Cc: [email protected]
Cc: Sascha Hauer <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
arch/arm/boot/dts/imx7d.dtsi | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index f6dee41..bbe23e4 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -42,6 +42,7 @@
*/
#include "imx7s.dtsi"
+#include <dt-bindings/reset/imx7-reset.h>
/ {
cpus {
@@ -127,6 +128,43 @@
fsl,num-rx-queues=<3>;
status = "disabled";
};
+
+ pcie: pcie@0x33800000 {
+ compatible = "fsl,imx7d-pcie", "snps,dw-pcie";
+ reg = <0x33800000 0x4000>,
+ <0x4ff00000 0x80000>;
+ reg-names = "dbi", "config";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ device_type = "pci";
+ bus-range = <0x00 0xff>;
+ ranges = <0x81000000 0 0 0x4ff80000 0 0x00010000 /* downstream I/O */
+ 0x82000000 0 0x40000000 0x40000000 0 0x0ff00000>; /* non-prefetchable memory */
+ num-lanes = <1>;
+ interrupts = <GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "msi";
+ #interrupt-cells = <1>;
+ interrupt-map-mask = <0 0 0 0x7>;
+ interrupt-map = <0 0 0 1 &intc GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 2 &intc GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 3 &intc GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 4 &intc GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks IMX7D_PCIE_CTRL_ROOT_CLK>,
+ <&clks IMX7D_PLL_ENET_MAIN_100M_CLK>,
+ <&clks IMX7D_PCIE_PHY_ROOT_CLK>;
+ clock-names = "pcie", "pcie_bus", "pcie_phy";
+ assigned-clocks = <&clks IMX7D_PCIE_CTRL_ROOT_SRC>,
+ <&clks IMX7D_PCIE_PHY_ROOT_SRC>;
+ assigned-clock-parents = <&clks IMX7D_PLL_ENET_MAIN_250M_CLK>,
+ <&clks IMX7D_PLL_ENET_MAIN_100M_CLK>;
+
+ fsl,max-link-speed = <2>;
+ power-domains = <&pgc_pcie_phy>;
+ resets = <&src IMX7_RESET_PCIEPHY>,
+ <&src IMX7_RESET_PCIE_CTRL_APPS_EN>;
+ reset-names = "pciephy", "apps";
+ status = "disabled";
+ };
};
&ca_funnel_ports {
--
2.9.3
List GPR block as compatible "fsl,imx6q-iomuxc-gpr" to support drivers
requesting it that way (PCIe driver is one example).
Cc: [email protected]
Cc: Sascha Hauer <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
arch/arm/boot/dts/imx7s.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 1a7058f..cc23478 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -491,7 +491,8 @@
};
gpr: iomuxc-gpr@30340000 {
- compatible = "fsl,imx7d-iomuxc-gpr", "syscon";
+ compatible = "fsl,imx7d-iomuxc-gpr",
+ "fsl,imx6q-iomuxc-gpr", "syscon";
reg = <0x30340000 0x10000>;
};
--
2.9.3
Enable PCIe peripheral on this board.
Cc: [email protected]
Cc: Sascha Hauer <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
arch/arm/boot/dts/imx7d-sdb.dts | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts
index e0ff276..f77e26a 100644
--- a/arch/arm/boot/dts/imx7d-sdb.dts
+++ b/arch/arm/boot/dts/imx7d-sdb.dts
@@ -352,6 +352,13 @@
};
};
+&pcie {
+ pinctrl-names = "default";
+ reset-gpio = <&gpio_spi 1 GPIO_ACTIVE_LOW>;
+ disable-gpio = <&gpio_spi 0 GPIO_ACTIVE_LOW>;
+ status = "okay";
+};
+
&pwm1 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pwm1>;
--
2.9.3
Add node for GPC and specify as a parent interrupt controller for SoC bus.
Cc: [email protected]
Cc: Sascha Hauer <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
arch/arm/boot/dts/imx7s.dtsi | 27 ++++++++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 8fee299..1a7058f 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -42,6 +42,7 @@
*/
#include <dt-bindings/clock/imx7d-clock.h>
+#include <dt-bindings/power/imx7-power.h>
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/input/input.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
@@ -119,7 +120,7 @@
#address-cells = <1>;
#size-cells = <1>;
compatible = "simple-bus";
- interrupt-parent = <&intc>;
+ interrupt-parent = <&gpc>;
ranges;
funnel@30041000 {
@@ -301,6 +302,7 @@
interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
#interrupt-cells = <3>;
interrupt-controller;
+ interrupt-parent = <&intc>;
reg = <0x31001000 0x1000>,
<0x31002000 0x2000>,
<0x31004000 0x2000>,
@@ -309,6 +311,7 @@
timer {
compatible = "arm,armv7-timer";
+ interrupt-parent = <&intc>;
interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
<GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
<GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
@@ -564,6 +567,28 @@
interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
#reset-cells = <1>;
};
+
+ gpc: gpc@303a0000 {
+ compatible = "fsl,imx7d-gpc";
+ reg = <0x303a0000 0x10000>;
+ interrupt-controller;
+ interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
+ #interrupt-cells = <3>;
+ interrupt-parent = <&intc>;
+ #power-domain-cells = <1>;
+
+ pgc {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pgc_pcie_phy: pgc-pcie-phy-domain {
+ #power-domain-cells = <0>;
+
+ reg = <IMX7_POWER_DOMAIN_PCIE_PHY>;
+ power-supply = <®_1p0d>;
+ };
+ };
+ };
};
aips2: aips-bus@30400000 {
--
2.9.3
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
simplify the situation by setting the value of 'anatop-enable-bit' to
point to ENABLE_LINREG (same as i.MX6).
Cc: [email protected]
Cc: Sascha Hauer <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
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>;
};
};
--
2.9.3
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: [email protected]
Cc: Sascha Hauer <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
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
Commit ca7734a ("regulator: anatop: Add support for
"anatop-enable-bit"") added code to support this particular binding
and 'anatop-enable-bit' is no longer an unused property.
This reverts commit 27958ccdf29e9971732e02494b48be54b0691269.
Cc: [email protected]
Cc: Sascha Hauer <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
arch/arm/boot/dts/imx6ul.dtsi | 1 +
arch/arm/boot/dts/imx7s.dtsi | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index b9d7d2d..6da2b77 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -542,6 +542,7 @@
anatop-min-bit-val = <0>;
anatop-min-voltage = <2625000>;
anatop-max-voltage = <3400000>;
+ anatop-enable-bit = <0>;
};
reg_arm: regulator-vddcore {
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index c4f12fd..22c9788 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -516,6 +516,7 @@
anatop-min-bit-val = <8>;
anatop-min-voltage = <800000>;
anatop-max-voltage = <1200000>;
+ anatop-enable-bit = <31>;
};
};
--
2.9.3
Hi Andrey,
On 13 April 2017 at 06:32, Andrey Smirnov <[email protected]> wrote:
> Add node for GPC and specify as a parent interrupt controller for SoC bus.
>
> Cc: [email protected]
> Cc: Sascha Hauer <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Andrey Smirnov <[email protected]>
> ---
> arch/arm/boot/dts/imx7s.dtsi | 27 ++++++++++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> index 8fee299..1a7058f 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -42,6 +42,7 @@
> */
>[0
> #include <dt-bindings/clock/imx7d-clock.h>
> +#include <dt-bindings/power/imx7-power.h>
> #include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/input/input.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> @@ -119,7 +120,7 @@
> #address-cells = <1>;
> #size-cells = <1>;
> compatible = "simple-bus";
> - interrupt-parent = <&intc>;
> + interrupt-parent = <&gpc>;
I've been testing your GPC/PCIe patch sets against v4.11-rc5 on my
imx7d-cl-som-imx7, but hit a bit of a wall. When gpc is set as the
interrupt-parent for the soc, the kernel seems to hang and not produce
any output on the serial port[0]. I tried to enable earlyprintk, but
no luck getting a trace. Reversing this change, gets the board
booting[1], but obviously isn't using the gpc which is needed for PCIe
support as I understand it. I assume you've tested these changes on a
imx7d-sdb and are not seeing a similar issue? You can find the patches
I've picked on top of v4.11-rc5 here[2], any idea what might be the
issue?
Cheers,
Tyler
[0] https://hastebin.com/zetohetuwi.coffeescript
[1] https://hastebin.com/uvacuyicuh.sql
[2] https://github.com/EmbeddedAndroid/linux/commits/tracking-ltd-imx-dev
On Thu, Apr 13, 2017 at 4:03 PM, Tyler Baker <[email protected]> wrote:
> I've been testing your GPC/PCIe patch sets against v4.11-rc5 on my
> imx7d-cl-som-imx7, but hit a bit of a wall. When gpc is set as the
> interrupt-parent for the soc, the kernel seems to hang and not produce
> any output on the serial port[0]. I tried to enable earlyprintk, but
> no luck getting a trace. Reversing this change, gets the board
> booting[1], but obviously isn't using the gpc which is needed for PCIe
> support as I understand it. I assume you've tested these changes on a
> imx7d-sdb and are not seeing a similar issue? You can find the patches
> I've picked on top of v4.11-rc5 here[2], any idea what might be the
> issue?
Thanks for the report.
It seems this series depends on the drivers/soc/imx/gpcv2.c patches
that landed into linux-next.
In this case, it would be better to re-send this series after
4.12-rc11 is out to avoid the breakage.
On 13 April 2017 at 12:18, Fabio Estevam <[email protected]> wrote:
> On Thu, Apr 13, 2017 at 4:03 PM, Tyler Baker <[email protected]> wrote:
>
>> I've been testing your GPC/PCIe patch sets against v4.11-rc5 on my
>> imx7d-cl-som-imx7, but hit a bit of a wall. When gpc is set as the
>> interrupt-parent for the soc, the kernel seems to hang and not produce
>> any output on the serial port[0]. I tried to enable earlyprintk, but
>> no luck getting a trace. Reversing this change, gets the board
>> booting[1], but obviously isn't using the gpc which is needed for PCIe
>> support as I understand it. I assume you've tested these changes on a
>> imx7d-sdb and are not seeing a similar issue? You can find the patches
>> I've picked on top of v4.11-rc5 here[2], any idea what might be the
>> issue?
>
> Thanks for the report.
No problem.
> It seems this series depends on the drivers/soc/imx/gpcv2.c patches
> that landed into linux-next.
Are you referring to the following patches?
"dt-bindings: Add GPCv2 power gating driver"
"soc: imx: Add GPCv2 power gating driver"
I've pulled these patches from Shawn's tree to test with, but still
not able to get anything functional. Is there another series I should
be looking at?
Thanks,
Tyler
On Thu, Apr 13, 2017 at 4:24 PM, Tyler Baker <[email protected]> wrote:
> Are you referring to the following patches?
>
> "dt-bindings: Add GPCv2 power gating driver"
> "soc: imx: Add GPCv2 power gating driver"
>
> I've pulled these patches from Shawn's tree to test with, but still
> not able to get anything functional. Is there another series I should
> be looking at?
Yes, these are the ones I was thinking about. Maybe Andrey can help to
clarify then.
Thanks
On 13 April 2017 at 12:55, Fabio Estevam <[email protected]> wrote:
> On Thu, Apr 13, 2017 at 4:24 PM, Tyler Baker <[email protected]> wrote:
>
>
>> Are you referring to the following patches?
>>
>> "dt-bindings: Add GPCv2 power gating driver"
>> "soc: imx: Add GPCv2 power gating driver"
>>
>> I've pulled these patches from Shawn's tree to test with, but still
>> not able to get anything functional. Is there another series I should
>> be looking at?
>
> Yes, these are the ones I was thinking about. Maybe Andrey can help to
> clarify then.
I've rebased this series on the next-20170413 for sanity sake, and
realized there doesn't appear to be a way to select CONFIG_IMX_GPCV2.
I forced it using 'default y' and configured with imx_v6_v7_defconfig.
Now my board is booting. Before this series is applied, it may be good
to have CONFIG_IMX_GPCV2 selected specifically for iMX7 platform,
otherwise there will be boot regressions.
I'd encounter a backtrace with next-20170413 + imx_v6_v7_defconfig +
CONFIG_IMX_GPCV2=y
Backtrace:
[<c010c364>] (dump_backtrace) from [<c010c610>] (show_stack+0x18/0x1c)
r7:00000000 r6:600000d3 r5:00000000 r4:c0e273dc
[<c010c5f8>] (show_stack) from [<c04070a4>] (dump_stack+0xb4/0xe8)
[<c0406ff0>] (dump_stack) from [<c0169e04>] (register_lock_class+0x208/0x5ec)
r9:ef00d010 r8:ef00d010 r7:c1606448 r6:00000000 r5:00000000 r4:ffffe000
[<c0169bfc>] (register_lock_class) from [<c016da48>]
(__lock_acquire+0x7c/0x18d0)
r10:c0e0af40 r9:ef00d010 r8:c0e274cc r7:00000001 r6:600000d3 r5:c1606448
r4:ffffe000
[<c016d9cc>] (__lock_acquire) from [<c016fa4c>] (lock_acquire+0x70/0x90)
r10:00000000 r9:ef007e38 r8:00000001 r7:00000001 r6:600000d3 r5:00000000
r4:ffffe000
[<c016f9dc>] (lock_acquire) from [<c09accc8>] (_raw_spin_lock+0x30/0x40)
r8:600000d3 r7:ef007e10 r6:00000001 r5:ef007e10 r4:ef00d000
[<c09acc98>] (_raw_spin_lock) from [<c04403a4>] (imx_gpcv2_irq_unmask+0x1c/0x5c)
r4:ef00d000
[<c0440388>] (imx_gpcv2_irq_unmask) from [<c017e838>] (irq_enable+0x38/0x4c)
r5:00000000 r4:ef007e00
[<c017e800>] (irq_enable) from [<c017e8d0>] (irq_startup+0x84/0x88)
r5:00000000 r4:ef007e00
[<c017e84c>] (irq_startup) from [<c017cd7c>] (__setup_irq+0x538/0x5f4)
r7:ef007e60 r6:00000015 r5:ef007e00 r4:ef007d00
[<c017c844>] (__setup_irq) from [<c017ce98>] (setup_irq+0x60/0xd0)
r10:c0d5fa48 r9:efffcbc0 r8:ef007d00 r7:00000015 r6:ef007e10 r5:00000000
r4:ef007e00
[<c017ce38>] (setup_irq) from [<c0d4dfdc>] (_mxc_timer_init+0x1f8/0x248)
r9:efffcbc0 r8:00000003 r7:016e3600 r6:c0c69bbc r5:ef007c40 r4:ef007c00
[<c0d4dde4>] (_mxc_timer_init) from [<c0d4e0dc>] (mxc_timer_init_dt+0xb0/0xf8)
r7:00000000 r6:c1669e48 r5:ef7ebf7c r4:ef007c00
[<c0d4e02c>] (mxc_timer_init_dt) from [<c0d4e168>]
(imx6dl_timer_init_dt+0x14/0x18)
r9:efffcbc0 r8:c0e7b000 r7:c0c695c0 r6:c0d6fe18 r5:00000001 r4:ef7ebf7c
[<c0d4e154>] (imx6dl_timer_init_dt) from [<c0d4d158>]
(clocksource_probe+0x54/0xb0)
[<c0d4d104>] (clocksource_probe) from [<c0d04a2c>] (time_init+0x30/0x38)
r7:c0e07900 r6:c0e7b000 r5:ffffffff r4:00000000
[<c0d049fc>] (time_init) from [<c0d00bc8>] (start_kernel+0x220/0x3a0)
[<c0d009a8>] (start_kernel) from [<8000807c>] (0x8000807c)
r10:00000000 r9:410fc075 r8:8000406a r7:c0e0c958 r6:c0d5fa44 r5:c0e07918
r4:c0e7b294
Cheers,
Tyler
On Thu, Apr 13, 2017 at 5:13 PM, Tyler Baker <[email protected]> wrote:
> I've rebased this series on the next-20170413 for sanity sake, and
> realized there doesn't appear to be a way to select CONFIG_IMX_GPCV2.
> I forced it using 'default y' and configured with imx_v6_v7_defconfig.
> Now my board is booting. Before this series is applied, it may be good
> to have CONFIG_IMX_GPCV2 selected specifically for iMX7 platform,
> otherwise there will be boot regressions.
Something like this?
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index f03ef43..fe99a48 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -253,6 +253,7 @@ config RENESAS_H8S_INTC
config IMX_GPCV2
bool
+ def_bool y if SOC_IMX7D
select IRQ_DOMAIN
help
Enables the wakeup IRQs for IMX platforms with GPCv2 block
>
> I'd encounter a backtrace with next-20170413 + imx_v6_v7_defconfig +
> CONFIG_IMX_GPCV2=y
>
> Backtrace:
> [<c010c364>] (dump_backtrace) from [<c010c610>] (show_stack+0x18/0x1c)
but prior to fixing the Kconfig we need to fix this backtrace you reported.
Thanks
On Thu, Apr 13, 2017 at 12:03 PM, Tyler Baker <[email protected]> wrote:
> Hi Andrey,
>
> On 13 April 2017 at 06:32, Andrey Smirnov <[email protected]> wrote:
>> Add node for GPC and specify as a parent interrupt controller for SoC bus.
>>
>> Cc: [email protected]
>> Cc: Sascha Hauer <[email protected]>
>> Cc: Fabio Estevam <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Cc: Mark Rutland <[email protected]>
>> Cc: Russell King <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Andrey Smirnov <[email protected]>
>> ---
>> arch/arm/boot/dts/imx7s.dtsi | 27 ++++++++++++++++++++++++++-
>> 1 file changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
>> index 8fee299..1a7058f 100644
>> --- a/arch/arm/boot/dts/imx7s.dtsi
>> +++ b/arch/arm/boot/dts/imx7s.dtsi
>> @@ -42,6 +42,7 @@
>> */
>>[0
>> #include <dt-bindings/clock/imx7d-clock.h>
>> +#include <dt-bindings/power/imx7-power.h>
>> #include <dt-bindings/gpio/gpio.h>
>> #include <dt-bindings/input/input.h>
>> #include <dt-bindings/interrupt-controller/arm-gic.h>
>> @@ -119,7 +120,7 @@
>> #address-cells = <1>;
>> #size-cells = <1>;
>> compatible = "simple-bus";
>> - interrupt-parent = <&intc>;
>> + interrupt-parent = <&gpc>;
>
> I've been testing your GPC/PCIe patch sets against v4.11-rc5 on my
> imx7d-cl-som-imx7, but hit a bit of a wall. When gpc is set as the
> interrupt-parent for the soc, the kernel seems to hang and not produce
> any output on the serial port[0]. I tried to enable earlyprintk, but
> no luck getting a trace. Reversing this change, gets the board
> booting[1], but obviously isn't using the gpc which is needed for PCIe
> support as I understand it. I assume you've tested these changes on a
> imx7d-sdb and are not seeing a similar issue? You can find the patches
> I've picked on top of v4.11-rc5 here[2], any idea what might be the
> issue?
Hmm, this is something new and I don't think I've seen it(neither that
nor the backtrace from your following e-mail). Here's the kernel tree
as I've been testing it:
https://github.com/ndreys/linux/commits/imx7d/pcie-support-v8
note, however, that it is based on d0ec4e6 (tip of pci/next when I was
rebasing) of git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git,
which is not exactly v4.11-rc5
I'll rebase on top v4.11-r5 and see if I can get the same backtrace
you are getting and see if I can fix it.
Thanks and sorry for breaking things for you,
Andrey Smirnov
On Thu, Apr 13, 2017 at 1:49 PM, Fabio Estevam <[email protected]> wrote:
> On Thu, Apr 13, 2017 at 5:13 PM, Tyler Baker <[email protected]> wrote:
>
>> I've rebased this series on the next-20170413 for sanity sake, and
>> realized there doesn't appear to be a way to select CONFIG_IMX_GPCV2.
>> I forced it using 'default y' and configured with imx_v6_v7_defconfig.
>> Now my board is booting. Before this series is applied, it may be good
>> to have CONFIG_IMX_GPCV2 selected specifically for iMX7 platform,
>> otherwise there will be boot regressions.
>
> Something like this?
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index f03ef43..fe99a48 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -253,6 +253,7 @@ config RENESAS_H8S_INTC
>
> config IMX_GPCV2
> bool
> + def_bool y if SOC_IMX7D
> select IRQ_DOMAIN
> help
> Enables the wakeup IRQs for IMX platforms with GPCv2 block
>
FWIW, there's a patch for this that I haven't submitted yet (I plan to
submit it tomorrow):
https://github.com/ndreys/linux/commit/bd2de5be5c74bb35a0b8090f473862c9298a48b3
Thanks,
Andrey Smirnov
On 13 April 2017 at 13:49, Fabio Estevam <[email protected]> wrote:
> On Thu, Apr 13, 2017 at 5:13 PM, Tyler Baker <[email protected]> wrote:
>
>> I've rebased this series on the next-20170413 for sanity sake, and
>> realized there doesn't appear to be a way to select CONFIG_IMX_GPCV2.
>> I forced it using 'default y' and configured with imx_v6_v7_defconfig.
>> Now my board is booting. Before this series is applied, it may be good
>> to have CONFIG_IMX_GPCV2 selected specifically for iMX7 platform,
>> otherwise there will be boot regressions.
>
> Something like this?
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index f03ef43..fe99a48 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -253,6 +253,7 @@ config RENESAS_H8S_INTC
>
> config IMX_GPCV2
> bool
> + def_bool y if SOC_IMX7D
> select IRQ_DOMAIN
> help
> Enables the wakeup IRQs for IMX platforms with GPCv2 block
Exactly. LGTM.
>> I'd encounter a backtrace with next-20170413 + imx_v6_v7_defconfig +
>> CONFIG_IMX_GPCV2=y
>>
>> Backtrace:
>> [<c010c364>] (dump_backtrace) from [<c010c610>] (show_stack+0x18/0x1c)
>
> but prior to fixing the Kconfig we need to fix this backtrace you reported.
I dug into this a bit, and lockdep is unhappy about spin locks not
being initialized before use.
The following patch fixes the backtrace on my board. I'll submit this
patch in a moment.
From: Tyler Baker <[email protected]>
Date: Thu, 13 Apr 2017 14:29:49 -0700
Subject: [PATCH] irqchip/irq-imx-gpcv2: fix spinlock initialization
Call raw_spin_lock_init() before the spinlocks are used to prevent a
lockdep splat.
Signed-off-by: Tyler Baker <[email protected]>
---
drivers/irqchip/irq-imx-gpcv2.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/irqchip/irq-imx-gpcv2.c b/drivers/irqchip/irq-imx-gpcv2.c
index e13236f..9463f35 100644
--- a/drivers/irqchip/irq-imx-gpcv2.c
+++ b/drivers/irqchip/irq-imx-gpcv2.c
@@ -230,6 +230,8 @@ static int __init imx_gpcv2_irqchip_init(struct
device_node *node,
return -ENOMEM;
}
+ raw_spin_lock_init(&cd->rlock);
+
cd->gpc_base = of_iomap(node, 0);
if (!cd->gpc_base) {
pr_err("fsl-gpcv2: unable to map gpc registers\n");
--
2.9.3
Cheers,
Tyler
On Thu, Apr 13, 2017 at 6:35 PM, Tyler Baker <[email protected]> wrote:
> The following patch fixes the backtrace on my board. I'll submit this
> patch in a moment.
>
> From: Tyler Baker <[email protected]>
> Date: Thu, 13 Apr 2017 14:29:49 -0700
> Subject: [PATCH] irqchip/irq-imx-gpcv2: fix spinlock initialization
>
> Call raw_spin_lock_init() before the spinlocks are used to prevent a
> lockdep splat.
>
> Signed-off-by: Tyler Baker <[email protected]>
Yes, it makes sense:
Reviewed-by: Fabio Estevam <[email protected]>
Hi Andrey,
On Thu, Apr 13, 2017 at 10:32 AM, Andrey Smirnov
<[email protected]> wrote:
> diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts
> index 5be01a1..e0ff276 100644
> --- a/arch/arm/boot/dts/imx7d-sdb.dts
> +++ b/arch/arm/boot/dts/imx7d-sdb.dts
> @@ -52,6 +52,30 @@
> reg = <0x80000000 0x80000000>;
> };
>
> + spi4 {
Here you use spi4 label...
> + compatible = "spi-gpio";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_spi1>;
and here spi1, which is a bit confusing.
> + status = "okay";
> + gpio-sck = <&gpio1 13 0>;
> + gpio-mosi = <&gpio1 9 0>;
> + cs-gpios = <&gpio1 12 0>;
You could use GPIO_ACTIVE_HIGH flag for better readability.
On Thu, Apr 13, 2017 at 10:32 AM, Andrey Smirnov
<[email protected]> wrote:
> +&pcie {
> + pinctrl-names = "default";
> + reset-gpio = <&gpio_spi 1 GPIO_ACTIVE_LOW>;
> + disable-gpio = <&gpio_spi 0 GPIO_ACTIVE_LOW>;
I don't see this property in linux-next.
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
>
> simplify the situation by setting the value of 'anatop-enable-bit' to
> point to ENABLE_LINREG (same as i.MX6).
>
> Cc: [email protected]
> Cc: Sascha Hauer <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Andrey Smirnov <[email protected]>
Since patch 1 ~ 3 are all about adding anatop-enable-bit, can we squash
them into one patch?
Shawn
> ---
> 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>;
> };
> };
>
> --
> 2.9.3
>
On Thu, Apr 13, 2017 at 06:32:38AM -0700, Andrey Smirnov wrote:
> Add node for GPC and specify as a parent interrupt controller for SoC bus.
>
> Cc: [email protected]
> Cc: Sascha Hauer <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Andrey Smirnov <[email protected]>
> ---
> arch/arm/boot/dts/imx7s.dtsi | 27 ++++++++++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> index 8fee299..1a7058f 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -42,6 +42,7 @@
> */
>
> #include <dt-bindings/clock/imx7d-clock.h>
> +#include <dt-bindings/power/imx7-power.h>
> #include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/input/input.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> @@ -119,7 +120,7 @@
> #address-cells = <1>;
> #size-cells = <1>;
> compatible = "simple-bus";
> - interrupt-parent = <&intc>;
> + interrupt-parent = <&gpc>;
> ranges;
>
> funnel@30041000 {
> @@ -301,6 +302,7 @@
> interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> #interrupt-cells = <3>;
> interrupt-controller;
> + interrupt-parent = <&intc>;
> reg = <0x31001000 0x1000>,
> <0x31002000 0x2000>,
> <0x31004000 0x2000>,
> @@ -309,6 +311,7 @@
>
> timer {
> compatible = "arm,armv7-timer";
> + interrupt-parent = <&intc>;
> interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> @@ -564,6 +567,28 @@
> interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> #reset-cells = <1>;
> };
> +
> + gpc: gpc@303a0000 {
> + compatible = "fsl,imx7d-gpc";
> + reg = <0x303a0000 0x10000>;
> + interrupt-controller;
> + interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> + #interrupt-cells = <3>;
> + interrupt-parent = <&intc>;
> + #power-domain-cells = <1>;
> +
> + pgc {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pgc_pcie_phy: pgc-pcie-phy-domain {
The node name should be something generic and has a unit-address when
there is a 'reg' property in the node.
> + #power-domain-cells = <0>;
> +
Drop this newline.
Shawn
> + reg = <IMX7_POWER_DOMAIN_PCIE_PHY>;
> + power-supply = <®_1p0d>;
> + };
> + };
> + };
> };
>
> aips2: aips-bus@30400000 {
> --
> 2.9.3
>
On Thu, Apr 13, 2017 at 06:32:40AM -0700, Andrey Smirnov wrote:
> Add node for U38, a 74LV595PW serial-in shift register that acts as a
> GPIO expander on the board.
>
> Cc: [email protected]
> Cc: Sascha Hauer <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Andrey Smirnov <[email protected]>
> ---
> arch/arm/boot/dts/imx7d-sdb.dts | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts
> index 5be01a1..e0ff276 100644
> --- a/arch/arm/boot/dts/imx7d-sdb.dts
> +++ b/arch/arm/boot/dts/imx7d-sdb.dts
> @@ -52,6 +52,30 @@
> reg = <0x80000000 0x80000000>;
> };
>
> + spi4 {
> + compatible = "spi-gpio";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_spi1>;
> + status = "okay";
The 'status' is not needed in this case.
> + gpio-sck = <&gpio1 13 0>;
> + gpio-mosi = <&gpio1 9 0>;
> + cs-gpios = <&gpio1 12 0>;
> + num-chipselects = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + gpio_spi: gpio_spi@0 {
gpio-expander might be a better node name?
> + compatible = "fairchild,74hc595";
> + gpio-controller;
> + #gpio-cells = <2>;
> + reg = <0>;
> + registers-number = <1>;
> + /* Enable PERI_3V3, SENSOR_RST_B and HDMI_RST*/
> + registers-default = /bits/ 8 <0x74>;
I do not see this property is documented or supported by kernel.
> + spi-max-frequency = <100000>;
> + };
> + };
> +
> regulators {
> compatible = "simple-bus";
> #address-cells = <1>;
> @@ -642,5 +666,13 @@
> fsl,pins = <
> MX7D_PAD_LPSR_GPIO1_IO01__PWM1_OUT 0x110b0
> >;
> +
> + pinctrl_spi1: spi1grp {
> + fsl,pins = <
> + MX7D_PAD_GPIO1_IO09__GPIO1_IO9 0x59
> + MX7D_PAD_GPIO1_IO12__GPIO1_IO12 0x59
> + MX7D_PAD_GPIO1_IO13__GPIO1_IO13 0x59
> + >;
> + };
> };
> };
> --
> 2.9.3
>
On Thu, Apr 13, 2017 at 06:32:42AM -0700, Andrey Smirnov wrote:
> Enable PCIe peripheral on this board.
>
> Cc: [email protected]
> Cc: Sascha Hauer <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Andrey Smirnov <[email protected]>
> ---
> arch/arm/boot/dts/imx7d-sdb.dts | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts
> index e0ff276..f77e26a 100644
> --- a/arch/arm/boot/dts/imx7d-sdb.dts
> +++ b/arch/arm/boot/dts/imx7d-sdb.dts
> @@ -352,6 +352,13 @@
> };
> };
>
> +&pcie {
> + pinctrl-names = "default";
> + reset-gpio = <&gpio_spi 1 GPIO_ACTIVE_LOW>;
> + disable-gpio = <&gpio_spi 0 GPIO_ACTIVE_LOW>;
I do not see this disable-gpio is documented or supported.
Shawn
> + status = "okay";
> +};
> +
> &pwm1 {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_pwm1>;
> --
> 2.9.3
>
On Thu, Apr 13, 2017 at 8:28 PM, Shawn Guo <[email protected]> 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
>>
>> simplify the situation by setting the value of 'anatop-enable-bit' to
>> point to ENABLE_LINREG (same as i.MX6).
>>
>> Cc: [email protected]
>> Cc: Sascha Hauer <[email protected]>
>> Cc: Fabio Estevam <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Cc: Mark Rutland <[email protected]>
>> Cc: Russell King <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Andrey Smirnov <[email protected]>
>
> Since patch 1 ~ 3 are all about adding anatop-enable-bit, can we squash
> them into one patch?
OK. Will do in v2.
On Thu, Apr 13, 2017 at 8:40 PM, Shawn Guo <[email protected]> wrote:
> On Thu, Apr 13, 2017 at 06:32:38AM -0700, Andrey Smirnov wrote:
>> Add node for GPC and specify as a parent interrupt controller for SoC bus.
>>
>> Cc: [email protected]
>> Cc: Sascha Hauer <[email protected]>
>> Cc: Fabio Estevam <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Cc: Mark Rutland <[email protected]>
>> Cc: Russell King <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Andrey Smirnov <[email protected]>
>> ---
>> arch/arm/boot/dts/imx7s.dtsi | 27 ++++++++++++++++++++++++++-
>> 1 file changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
>> index 8fee299..1a7058f 100644
>> --- a/arch/arm/boot/dts/imx7s.dtsi
>> +++ b/arch/arm/boot/dts/imx7s.dtsi
>> @@ -42,6 +42,7 @@
>> */
>>
>> #include <dt-bindings/clock/imx7d-clock.h>
>> +#include <dt-bindings/power/imx7-power.h>
>> #include <dt-bindings/gpio/gpio.h>
>> #include <dt-bindings/input/input.h>
>> #include <dt-bindings/interrupt-controller/arm-gic.h>
>> @@ -119,7 +120,7 @@
>> #address-cells = <1>;
>> #size-cells = <1>;
>> compatible = "simple-bus";
>> - interrupt-parent = <&intc>;
>> + interrupt-parent = <&gpc>;
>> ranges;
>>
>> funnel@30041000 {
>> @@ -301,6 +302,7 @@
>> interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
>> #interrupt-cells = <3>;
>> interrupt-controller;
>> + interrupt-parent = <&intc>;
>> reg = <0x31001000 0x1000>,
>> <0x31002000 0x2000>,
>> <0x31004000 0x2000>,
>> @@ -309,6 +311,7 @@
>>
>> timer {
>> compatible = "arm,armv7-timer";
>> + interrupt-parent = <&intc>;
>> interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> @@ -564,6 +567,28 @@
>> interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
>> #reset-cells = <1>;
>> };
>> +
>> + gpc: gpc@303a0000 {
>> + compatible = "fsl,imx7d-gpc";
>> + reg = <0x303a0000 0x10000>;
>> + interrupt-controller;
>> + interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
>> + #interrupt-cells = <3>;
>> + interrupt-parent = <&intc>;
>> + #power-domain-cells = <1>;
>> +
>> + pgc {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + pgc_pcie_phy: pgc-pcie-phy-domain {
>
> The node name should be something generic and has a unit-address when
> there is a 'reg' property in the node.
>
I'll change it to pgc-power-domain@0, let me know if you want
something different.
>> + #power-domain-cells = <0>;
>> +
>
> Drop this newline.
>
OK. Will do in v2.
On Thu, Apr 13, 2017 at 3:20 PM, Fabio Estevam <[email protected]> wrote:
> Hi Andrey,
>
> On Thu, Apr 13, 2017 at 10:32 AM, Andrey Smirnov
> <[email protected]> wrote:
>
>> diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts
>> index 5be01a1..e0ff276 100644
>> --- a/arch/arm/boot/dts/imx7d-sdb.dts
>> +++ b/arch/arm/boot/dts/imx7d-sdb.dts
>> @@ -52,6 +52,30 @@
>> reg = <0x80000000 0x80000000>;
>> };
>>
>> + spi4 {
>
> Here you use spi4 label...
>
>> + compatible = "spi-gpio";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_spi1>;
>
> and here spi1, which is a bit confusing.
>
Agreed. It is confusing, will fix in v2.
>> + status = "okay";
>> + gpio-sck = <&gpio1 13 0>;
>> + gpio-mosi = <&gpio1 9 0>;
>> + cs-gpios = <&gpio1 12 0>;
>
> You could use GPIO_ACTIVE_HIGH flag for better readability.
Missed that, will fix in v2.
Thanks,
Andrey Smirnov
On Thu, Apr 13, 2017 at 8:47 PM, Shawn Guo <[email protected]> wrote:
> On Thu, Apr 13, 2017 at 06:32:40AM -0700, Andrey Smirnov wrote:
>> Add node for U38, a 74LV595PW serial-in shift register that acts as a
>> GPIO expander on the board.
>>
>> Cc: [email protected]
>> Cc: Sascha Hauer <[email protected]>
>> Cc: Fabio Estevam <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Cc: Mark Rutland <[email protected]>
>> Cc: Russell King <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Andrey Smirnov <[email protected]>
>> ---
>> arch/arm/boot/dts/imx7d-sdb.dts | 32 ++++++++++++++++++++++++++++++++
>> 1 file changed, 32 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts
>> index 5be01a1..e0ff276 100644
>> --- a/arch/arm/boot/dts/imx7d-sdb.dts
>> +++ b/arch/arm/boot/dts/imx7d-sdb.dts
>> @@ -52,6 +52,30 @@
>> reg = <0x80000000 0x80000000>;
>> };
>>
>> + spi4 {
>> + compatible = "spi-gpio";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_spi1>;
>> + status = "okay";
>
> The 'status' is not needed in this case.
>
Missed that, will fix in v2.
>> + gpio-sck = <&gpio1 13 0>;
>> + gpio-mosi = <&gpio1 9 0>;
>> + cs-gpios = <&gpio1 12 0>;
>> + num-chipselects = <1>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + gpio_spi: gpio_spi@0 {
>
> gpio-expander might be a better node name?
>
Yeah, I agree. I'll change it to extended_io: gpio-expander@0
("Extended IO" is how this part is called out on the schematic)
>> + compatible = "fairchild,74hc595";
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> + reg = <0>;
>> + registers-number = <1>;
>> + /* Enable PERI_3V3, SENSOR_RST_B and HDMI_RST*/
>> + registers-default = /bits/ 8 <0x74>;
>
> I do not see this property is documented or supported by kernel.
My bad, some downstream properties leakage. Will remove in v2.
Thanks,
Andrey Smirnov
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: [email protected]
> Cc: Sascha Hauer <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Andrey Smirnov <[email protected]>
> ---
> 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
On Thu, Apr 13, 2017 at 8:51 PM, Shawn Guo <[email protected]> wrote:
> On Thu, Apr 13, 2017 at 06:32:42AM -0700, Andrey Smirnov wrote:
>> Enable PCIe peripheral on this board.
>>
>> Cc: [email protected]
>> Cc: Sascha Hauer <[email protected]>
>> Cc: Fabio Estevam <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Cc: Mark Rutland <[email protected]>
>> Cc: Russell King <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Andrey Smirnov <[email protected]>
>> ---
>> arch/arm/boot/dts/imx7d-sdb.dts | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts
>> index e0ff276..f77e26a 100644
>> --- a/arch/arm/boot/dts/imx7d-sdb.dts
>> +++ b/arch/arm/boot/dts/imx7d-sdb.dts
>> @@ -352,6 +352,13 @@
>> };
>> };
>>
>> +&pcie {
>> + pinctrl-names = "default";
>> + reset-gpio = <&gpio_spi 1 GPIO_ACTIVE_LOW>;
>> + disable-gpio = <&gpio_spi 0 GPIO_ACTIVE_LOW>;
>
> I do not see this disable-gpio is documented or supported.
>
My bad, didn't notice it when taking the code from downstream tree.
Will remove in v2.
Thanks,
Andrey Smirnov
On Fri, Apr 14, 2017 at 08:19:44AM -0700, Andrey Smirnov wrote:
...
> >> + gpc: gpc@303a0000 {
> >> + compatible = "fsl,imx7d-gpc";
> >> + reg = <0x303a0000 0x10000>;
> >> + interrupt-controller;
> >> + interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> >> + #interrupt-cells = <3>;
> >> + interrupt-parent = <&intc>;
> >> + #power-domain-cells = <1>;
> >> +
> >> + pgc {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + pgc_pcie_phy: pgc-pcie-phy-domain {
> >
> > The node name should be something generic and has a unit-address when
> > there is a 'reg' property in the node.
> >
>
> I'll change it to pgc-power-domain@0, let me know if you want
> something different.
>
I think just power-domain@0 is ok.
And also better replace unit-address by macro.
Regards
Dong Aisheng
On Fri, Apr 14, 2017 at 8:49 AM, Dong Aisheng <[email protected]> wrote:
> On Fri, Apr 14, 2017 at 08:19:44AM -0700, Andrey Smirnov wrote:
> ...
>> >> + gpc: gpc@303a0000 {
>> >> + compatible = "fsl,imx7d-gpc";
>> >> + reg = <0x303a0000 0x10000>;
>> >> + interrupt-controller;
>> >> + interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
>> >> + #interrupt-cells = <3>;
>> >> + interrupt-parent = <&intc>;
>> >> + #power-domain-cells = <1>;
>> >> +
>> >> + pgc {
>> >> + #address-cells = <1>;
>> >> + #size-cells = <0>;
>> >> +
>> >> + pgc_pcie_phy: pgc-pcie-phy-domain {
>> >
>> > The node name should be something generic and has a unit-address when
>> > there is a 'reg' property in the node.
>> >
>>
>> I'll change it to pgc-power-domain@0, let me know if you want
>> something different.
>>
>
> I think just power-domain@0 is ok.
Fair enough. I'll do that.
> And also better replace unit-address by macro.
>
Good point. Will do.
Thanks,
Andrey Smirnov
On Thu, Apr 13, 2017 at 06:32:39AM -0700, Andrey Smirnov wrote:
> List GPR block as compatible "fsl,imx6q-iomuxc-gpr" to support drivers
> requesting it that way (PCIe driver is one example).
>
> Cc: [email protected]
> Cc: Sascha Hauer <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Andrey Smirnov <[email protected]>
> ---
> arch/arm/boot/dts/imx7s.dtsi | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> index 1a7058f..cc23478 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -491,7 +491,8 @@
> };
>
> gpr: iomuxc-gpr@30340000 {
> - compatible = "fsl,imx7d-iomuxc-gpr", "syscon";
> + compatible = "fsl,imx7d-iomuxc-gpr",
> + "fsl,imx6q-iomuxc-gpr", "syscon";
This looks wrong to me.
mx7d-iomux-gpr gets a big difference from mx6q-iomux-gpr and mostly
not compatible.
Regards
Dong Aisheng
On Fri, Apr 14, 2017 at 11:47:00AM +0800, Shawn Guo wrote:
> On Thu, Apr 13, 2017 at 06:32:40AM -0700, Andrey Smirnov wrote:
> > Add node for U38, a 74LV595PW serial-in shift register that acts as a
> > GPIO expander on the board.
> >
> > Cc: [email protected]
> > Cc: Sascha Hauer <[email protected]>
> > Cc: Fabio Estevam <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Cc: Mark Rutland <[email protected]>
> > Cc: Russell King <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Andrey Smirnov <[email protected]>
> > ---
> > arch/arm/boot/dts/imx7d-sdb.dts | 32 ++++++++++++++++++++++++++++++++
> > 1 file changed, 32 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts
> > index 5be01a1..e0ff276 100644
> > --- a/arch/arm/boot/dts/imx7d-sdb.dts
> > +++ b/arch/arm/boot/dts/imx7d-sdb.dts
> > @@ -52,6 +52,30 @@
> > reg = <0x80000000 0x80000000>;
> > };
> >
> > + spi4 {
> > + compatible = "spi-gpio";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_spi1>;
> > + status = "okay";
>
> The 'status' is not needed in this case.
>
> > + gpio-sck = <&gpio1 13 0>;
> > + gpio-mosi = <&gpio1 9 0>;
> > + cs-gpios = <&gpio1 12 0>;
> > + num-chipselects = <1>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + gpio_spi: gpio_spi@0 {
>
> gpio-expander might be a better node name?
>
> > + compatible = "fairchild,74hc595";
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + reg = <0>;
> > + registers-number = <1>;
> > + /* Enable PERI_3V3, SENSOR_RST_B and HDMI_RST*/
> > + registers-default = /bits/ 8 <0x74>;
>
> I do not see this property is documented or supported by kernel.
It's FSL internal invented property to do some trick on register
intialization and should be dropped.
Regards
Dong Aisheng
On Fri, Apr 14, 2017 at 8:32 AM, Dong Aisheng <[email protected]> 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: [email protected]
>> Cc: Sascha Hauer <[email protected]>
>> Cc: Fabio Estevam <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Cc: Mark Rutland <[email protected]>
>> Cc: Russell King <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Andrey Smirnov <[email protected]>
>> ---
>> 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
On Fri, Apr 14, 2017 at 8:56 AM, Dong Aisheng <[email protected]> wrote:
> On Thu, Apr 13, 2017 at 06:32:39AM -0700, Andrey Smirnov wrote:
>> List GPR block as compatible "fsl,imx6q-iomuxc-gpr" to support drivers
>> requesting it that way (PCIe driver is one example).
>>
>> Cc: [email protected]
>> Cc: Sascha Hauer <[email protected]>
>> Cc: Fabio Estevam <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Cc: Mark Rutland <[email protected]>
>> Cc: Russell King <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Andrey Smirnov <[email protected]>
>> ---
>> arch/arm/boot/dts/imx7s.dtsi | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
>> index 1a7058f..cc23478 100644
>> --- a/arch/arm/boot/dts/imx7s.dtsi
>> +++ b/arch/arm/boot/dts/imx7s.dtsi
>> @@ -491,7 +491,8 @@
>> };
>>
>> gpr: iomuxc-gpr@30340000 {
>> - compatible = "fsl,imx7d-iomuxc-gpr", "syscon";
>> + compatible = "fsl,imx7d-iomuxc-gpr",
>> + "fsl,imx6q-iomuxc-gpr", "syscon";
>
> This looks wrong to me.
> mx7d-iomux-gpr gets a big difference from mx6q-iomux-gpr and mostly
> not compatible.
>
AFAICT, there are no upstream drivers that bind to that string
directly and all of the "consumers" of this node request it as a
syscon device. The only code I could find that does so and that is
shared between i.MX7 and i.MX6Q is i.MX PCIe driver which
distinguishes between variants based on its own compatibility string.
Those two register files are different, true, but I don't think there
are any users who try to use them as if they were the same/compatible.
Thanks,
Andrey Smirnov
On 13 April 2017 at 06:32, Andrey Smirnov <[email protected]> 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 <[email protected]>
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