Previously a subnode to syscon node was added which has the
exact memory mapped address of pcie_ctrl but based on review comment
provided by Rob [1], the offset is now being passed as argument to
"ti,syscon-pcie-ctrl" phandle.
This series has both driver change and DT change. The driver change
should be merged first and the driver takes care of maintaining old
DT compatibility.
Changes frm v1:
*) Remove use of allOf in schema
*) Added Fixes tag
*) Maintain old DT compatibility
[1] -> http://lore.kernel.org/r/CAL_JsqKiUcO76bo1GoepWM1TusJWoty_BRy2hFSgtEVMqtrvvQ@mail.gmail.com
Kishon Vijay Abraham I (3):
dt-bindings: pci: ti,j721e: Fix "ti,syscon-pcie-ctrl" to take argument
PCI: j721e: Get offset within "syscon" from "ti,syscon-pcie-ctrl"
phandle arg
arm64: dts: ti: k3-j721e-main: Remove "syscon" nodes added for
pcieX_ctrl
.../bindings/pci/ti,j721e-pci-ep.yaml | 11 +++--
.../bindings/pci/ti,j721e-pci-host.yaml | 11 +++--
arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 48 ++++---------------
drivers/pci/controller/cadence/pci-j721e.c | 28 +++++++----
4 files changed, 41 insertions(+), 57 deletions(-)
--
2.17.1
Fix binding documentation of "ti,syscon-pcie-ctrl" to take phandle with
argument. The argument is the register offset within "syscon" used to
configure PCIe controller. This change is as discussed in [1]
[1] -> http://lore.kernel.org/r/CAL_JsqKiUcO76bo1GoepWM1TusJWoty_BRy2hFSgtEVMqtrvvQ@mail.gmail.com
Fixes: 431b53b81cdc ("dt-bindings: PCI: Add host mode dt-bindings for TI's J721E SoC")
Fixes: 45b39e928966 ("dt-bindings: PCI: Add EP mode dt-bindings for TI's J721E SoC")
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
.../devicetree/bindings/pci/ti,j721e-pci-ep.yaml | 11 +++++++----
.../devicetree/bindings/pci/ti,j721e-pci-host.yaml | 11 +++++++----
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml
index 3ae3e1a2d4b0..3766565cf258 100644
--- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml
@@ -29,9 +29,12 @@ properties:
- const: mem
ti,syscon-pcie-ctrl:
- description: Phandle to the SYSCON entry required for configuring PCIe mode
- and link speed.
- $ref: /schemas/types.yaml#/definitions/phandle
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ items:
+ - items:
+ - description: Phandle to the SYSCON entry
+ - description: pcie_ctrl register offset within SYSCON
+ description: Specifier for configuring PCIe mode and link speed.
power-domains:
maxItems: 1
@@ -80,7 +83,7 @@ examples:
<0x00 0x0d000000 0x00 0x00800000>,
<0x00 0x10000000 0x00 0x08000000>;
reg-names = "intd_cfg", "user_cfg", "reg", "mem";
- ti,syscon-pcie-ctrl = <&pcie0_ctrl>;
+ ti,syscon-pcie-ctrl = <&pcie0_ctrl 0x4070>;
max-link-speed = <3>;
num-lanes = <2>;
power-domains = <&k3_pds 239 TI_SCI_PD_EXCLUSIVE>;
diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
index ee7a8eade3f6..2b6a1a5eaf7a 100644
--- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
+++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
@@ -29,9 +29,12 @@ properties:
- const: cfg
ti,syscon-pcie-ctrl:
- description: Phandle to the SYSCON entry required for configuring PCIe mode
- and link speed.
- $ref: /schemas/types.yaml#/definitions/phandle
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ items:
+ - items:
+ - description: Phandle to the SYSCON entry
+ - description: pcie_ctrl register offset within SYSCON
+ description: Specifier for configuring PCIe mode and link speed.
power-domains:
maxItems: 1
@@ -90,7 +93,7 @@ examples:
<0x00 0x0d000000 0x00 0x00800000>,
<0x00 0x10000000 0x00 0x00001000>;
reg-names = "intd_cfg", "user_cfg", "reg", "cfg";
- ti,syscon-pcie-ctrl = <&pcie0_ctrl>;
+ ti,syscon-pcie-ctrl = <&pcie0_ctrl 0x4070>;
max-link-speed = <3>;
num-lanes = <2>;
power-domains = <&k3_pds 239 TI_SCI_PD_EXCLUSIVE>;
--
2.17.1
Get "syscon" pcie_ctrl offset from the argument of "ti,syscon-pcie-ctrl"
phandle. Previously a subnode to "syscon" node was added which has the
exact memory mapped address of pcie_ctrl but now the offset of pcie_ctrl
within "syscon" is now being passed as argument to "ti,syscon-pcie-ctrl"
phandle.
If the offset is not provided in "ti,syscon-pcie-ctrl", the
full memory mapped address of pcie_ctrl is used in order to maintain old
DT compatibility.
This change is as discussed in [1]
[1] -> http://lore.kernel.org/r/CAL_JsqKiUcO76bo1GoepWM1TusJWoty_BRy2hFSgtEVMqtrvvQ@mail.gmail.com
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/pci/controller/cadence/pci-j721e.c | 28 +++++++++++++++-------
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
index 586b9d69fa5e..dac1ac8a7615 100644
--- a/drivers/pci/controller/cadence/pci-j721e.c
+++ b/drivers/pci/controller/cadence/pci-j721e.c
@@ -12,6 +12,7 @@
#include <linux/irqchip/chained_irq.h>
#include <linux/irqdomain.h>
#include <linux/mfd/syscon.h>
+#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/of_irq.h>
#include <linux/pci.h>
@@ -153,7 +154,8 @@ static const struct cdns_pcie_ops j721e_pcie_ops = {
.link_up = j721e_pcie_link_up,
};
-static int j721e_pcie_set_mode(struct j721e_pcie *pcie, struct regmap *syscon)
+static int j721e_pcie_set_mode(struct j721e_pcie *pcie, struct regmap *syscon,
+ unsigned int offset)
{
struct device *dev = pcie->dev;
u32 mask = J721E_MODE_RC;
@@ -164,7 +166,7 @@ static int j721e_pcie_set_mode(struct j721e_pcie *pcie, struct regmap *syscon)
if (mode == PCI_MODE_RC)
val = J721E_MODE_RC;
- ret = regmap_update_bits(syscon, 0, mask, val);
+ ret = regmap_update_bits(syscon, offset, mask, val);
if (ret)
dev_err(dev, "failed to set pcie mode\n");
@@ -172,7 +174,7 @@ static int j721e_pcie_set_mode(struct j721e_pcie *pcie, struct regmap *syscon)
}
static int j721e_pcie_set_link_speed(struct j721e_pcie *pcie,
- struct regmap *syscon)
+ struct regmap *syscon, unsigned int offset)
{
struct device *dev = pcie->dev;
struct device_node *np = dev->of_node;
@@ -185,7 +187,7 @@ static int j721e_pcie_set_link_speed(struct j721e_pcie *pcie,
link_speed = 2;
val = link_speed - 1;
- ret = regmap_update_bits(syscon, 0, GENERATION_SEL_MASK, val);
+ ret = regmap_update_bits(syscon, offset, GENERATION_SEL_MASK, val);
if (ret)
dev_err(dev, "failed to set link speed\n");
@@ -193,7 +195,7 @@ static int j721e_pcie_set_link_speed(struct j721e_pcie *pcie,
}
static int j721e_pcie_set_lane_count(struct j721e_pcie *pcie,
- struct regmap *syscon)
+ struct regmap *syscon, unsigned int offset)
{
struct device *dev = pcie->dev;
u32 lanes = pcie->num_lanes;
@@ -201,7 +203,7 @@ static int j721e_pcie_set_lane_count(struct j721e_pcie *pcie,
int ret;
val = LANE_COUNT(lanes - 1);
- ret = regmap_update_bits(syscon, 0, LANE_COUNT_MASK, val);
+ ret = regmap_update_bits(syscon, offset, LANE_COUNT_MASK, val);
if (ret)
dev_err(dev, "failed to set link count\n");
@@ -212,6 +214,8 @@ static int j721e_pcie_ctrl_init(struct j721e_pcie *pcie)
{
struct device *dev = pcie->dev;
struct device_node *node = dev->of_node;
+ struct of_phandle_args args;
+ unsigned int offset = 0;
struct regmap *syscon;
int ret;
@@ -221,19 +225,25 @@ static int j721e_pcie_ctrl_init(struct j721e_pcie *pcie)
return PTR_ERR(syscon);
}
- ret = j721e_pcie_set_mode(pcie, syscon);
+ /* Do not error out to maintain old DT compatibility */
+ ret = of_parse_phandle_with_fixed_args(node, "ti,syscon-pcie-ctrl", 1,
+ 0, &args);
+ if (!ret)
+ offset = args.args[0];
+
+ ret = j721e_pcie_set_mode(pcie, syscon, offset);
if (ret < 0) {
dev_err(dev, "Failed to set pci mode\n");
return ret;
}
- ret = j721e_pcie_set_link_speed(pcie, syscon);
+ ret = j721e_pcie_set_link_speed(pcie, syscon, offset);
if (ret < 0) {
dev_err(dev, "Failed to set link speed\n");
return ret;
}
- ret = j721e_pcie_set_lane_count(pcie, syscon);
+ ret = j721e_pcie_set_lane_count(pcie, syscon, offset);
if (ret < 0) {
dev_err(dev, "Failed to set num-lanes\n");
return ret;
--
2.17.1
Remove "syscon" nodes added for pcieX_ctrl and have the PCIe node
point to the parent with an offset argument. This change is as discussed in [1]
[1] -> http://lore.kernel.org/r/CAL_JsqKiUcO76bo1GoepWM1TusJWoty_BRy2hFSgtEVMqtrvvQ@mail.gmail.com
Fixes: 4e5833884f66 ("arm64: dts: ti: k3-j721e-main: Add PCIe device tree nodes")
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 48 ++++-------------------
1 file changed, 8 insertions(+), 40 deletions(-)
diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
index 620e69e42974..23a0024dda79 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
@@ -28,38 +28,6 @@
#size-cells = <1>;
ranges = <0x0 0x0 0x00100000 0x1c000>;
- pcie0_ctrl: syscon@4070 {
- compatible = "ti,j721e-system-controller", "syscon", "simple-mfd";
- reg = <0x00004070 0x4>;
- #address-cells = <1>;
- #size-cells = <1>;
- ranges = <0x4070 0x4070 0x4>;
- };
-
- pcie1_ctrl: syscon@4074 {
- compatible = "ti,j721e-system-controller", "syscon", "simple-mfd";
- reg = <0x00004074 0x4>;
- #address-cells = <1>;
- #size-cells = <1>;
- ranges = <0x4074 0x4074 0x4>;
- };
-
- pcie2_ctrl: syscon@4078 {
- compatible = "ti,j721e-system-controller", "syscon", "simple-mfd";
- reg = <0x00004078 0x4>;
- #address-cells = <1>;
- #size-cells = <1>;
- ranges = <0x4078 0x4078 0x4>;
- };
-
- pcie3_ctrl: syscon@407c {
- compatible = "ti,j721e-system-controller", "syscon", "simple-mfd";
- reg = <0x0000407c 0x4>;
- #address-cells = <1>;
- #size-cells = <1>;
- ranges = <0x407c 0x407c 0x4>;
- };
-
serdes_ln_ctrl: mux@4080 {
compatible = "mmio-mux";
reg = <0x00004080 0x50>;
@@ -619,7 +587,7 @@
interrupt-names = "link_state";
interrupts = <GIC_SPI 318 IRQ_TYPE_EDGE_RISING>;
device_type = "pci";
- ti,syscon-pcie-ctrl = <&pcie0_ctrl>;
+ ti,syscon-pcie-ctrl = <&scm_conf 0x4070>;
max-link-speed = <3>;
num-lanes = <2>;
power-domains = <&k3_pds 239 TI_SCI_PD_EXCLUSIVE>;
@@ -646,7 +614,7 @@
reg-names = "intd_cfg", "user_cfg", "reg", "mem";
interrupt-names = "link_state";
interrupts = <GIC_SPI 318 IRQ_TYPE_EDGE_RISING>;
- ti,syscon-pcie-ctrl = <&pcie0_ctrl>;
+ ti,syscon-pcie-ctrl = <&scm_conf 0x4070>;
max-link-speed = <3>;
num-lanes = <2>;
power-domains = <&k3_pds 239 TI_SCI_PD_EXCLUSIVE>;
@@ -668,7 +636,7 @@
interrupt-names = "link_state";
interrupts = <GIC_SPI 330 IRQ_TYPE_EDGE_RISING>;
device_type = "pci";
- ti,syscon-pcie-ctrl = <&pcie1_ctrl>;
+ ti,syscon-pcie-ctrl = <&scm_conf 0x4074>;
max-link-speed = <3>;
num-lanes = <2>;
power-domains = <&k3_pds 240 TI_SCI_PD_EXCLUSIVE>;
@@ -695,7 +663,7 @@
reg-names = "intd_cfg", "user_cfg", "reg", "mem";
interrupt-names = "link_state";
interrupts = <GIC_SPI 330 IRQ_TYPE_EDGE_RISING>;
- ti,syscon-pcie-ctrl = <&pcie1_ctrl>;
+ ti,syscon-pcie-ctrl = <&scm_conf 0x4074>;
max-link-speed = <3>;
num-lanes = <2>;
power-domains = <&k3_pds 240 TI_SCI_PD_EXCLUSIVE>;
@@ -717,7 +685,7 @@
interrupt-names = "link_state";
interrupts = <GIC_SPI 342 IRQ_TYPE_EDGE_RISING>;
device_type = "pci";
- ti,syscon-pcie-ctrl = <&pcie2_ctrl>;
+ ti,syscon-pcie-ctrl = <&scm_conf 0x4078>;
max-link-speed = <3>;
num-lanes = <2>;
power-domains = <&k3_pds 241 TI_SCI_PD_EXCLUSIVE>;
@@ -744,7 +712,7 @@
reg-names = "intd_cfg", "user_cfg", "reg", "mem";
interrupt-names = "link_state";
interrupts = <GIC_SPI 342 IRQ_TYPE_EDGE_RISING>;
- ti,syscon-pcie-ctrl = <&pcie2_ctrl>;
+ ti,syscon-pcie-ctrl = <&scm_conf 0x4078>;
max-link-speed = <3>;
num-lanes = <2>;
power-domains = <&k3_pds 241 TI_SCI_PD_EXCLUSIVE>;
@@ -766,7 +734,7 @@
interrupt-names = "link_state";
interrupts = <GIC_SPI 354 IRQ_TYPE_EDGE_RISING>;
device_type = "pci";
- ti,syscon-pcie-ctrl = <&pcie3_ctrl>;
+ ti,syscon-pcie-ctrl = <&scm_conf 0x407c>;
max-link-speed = <3>;
num-lanes = <2>;
power-domains = <&k3_pds 242 TI_SCI_PD_EXCLUSIVE>;
@@ -793,7 +761,7 @@
reg-names = "intd_cfg", "user_cfg", "reg", "mem";
interrupt-names = "link_state";
interrupts = <GIC_SPI 354 IRQ_TYPE_EDGE_RISING>;
- ti,syscon-pcie-ctrl = <&pcie3_ctrl>;
+ ti,syscon-pcie-ctrl = <&scm_conf 0x407c>;
max-link-speed = <3>;
num-lanes = <2>;
power-domains = <&k3_pds 242 TI_SCI_PD_EXCLUSIVE>;
--
2.17.1
On Fri, Dec 4, 2020 at 1:52 AM Kishon Vijay Abraham I <[email protected]> wrote:
>
> Fix binding documentation of "ti,syscon-pcie-ctrl" to take phandle with
> argument. The argument is the register offset within "syscon" used to
> configure PCIe controller. This change is as discussed in [1]
>
> [1] -> http://lore.kernel.org/r/CAL_JsqKiUcO76bo1GoepWM1TusJWoty_BRy2hFSgtEVMqtrvvQ@mail.gmail.com
>
> Fixes: 431b53b81cdc ("dt-bindings: PCI: Add host mode dt-bindings for TI's J721E SoC")
> Fixes: 45b39e928966 ("dt-bindings: PCI: Add EP mode dt-bindings for TI's J721E SoC")
> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> ---
> .../devicetree/bindings/pci/ti,j721e-pci-ep.yaml | 11 +++++++----
> .../devicetree/bindings/pci/ti,j721e-pci-host.yaml | 11 +++++++----
> 2 files changed, 14 insertions(+), 8 deletions(-)
Reviewed-by: Rob Herring <[email protected]>
On Fri, Dec 4, 2020 at 1:52 AM Kishon Vijay Abraham I <[email protected]> wrote:
>
> Get "syscon" pcie_ctrl offset from the argument of "ti,syscon-pcie-ctrl"
> phandle. Previously a subnode to "syscon" node was added which has the
> exact memory mapped address of pcie_ctrl but now the offset of pcie_ctrl
> within "syscon" is now being passed as argument to "ti,syscon-pcie-ctrl"
> phandle.
>
> If the offset is not provided in "ti,syscon-pcie-ctrl", the
> full memory mapped address of pcie_ctrl is used in order to maintain old
> DT compatibility.
>
> This change is as discussed in [1]
>
> [1] -> http://lore.kernel.org/r/CAL_JsqKiUcO76bo1GoepWM1TusJWoty_BRy2hFSgtEVMqtrvvQ@mail.gmail.com
>
> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> ---
> drivers/pci/controller/cadence/pci-j721e.c | 28 +++++++++++++++-------
> 1 file changed, 19 insertions(+), 9 deletions(-)
Reviewed-by: Rob Herring <[email protected]>
On Fri, Dec 4, 2020 at 1:52 AM Kishon Vijay Abraham I <[email protected]> wrote:
>
> Remove "syscon" nodes added for pcieX_ctrl and have the PCIe node
> point to the parent with an offset argument. This change is as discussed in [1]
>
> [1] -> http://lore.kernel.org/r/CAL_JsqKiUcO76bo1GoepWM1TusJWoty_BRy2hFSgtEVMqtrvvQ@mail.gmail.com
>
> Fixes: 4e5833884f66 ("arm64: dts: ti: k3-j721e-main: Add PCIe device tree nodes")
> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> ---
> arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 48 ++++-------------------
> 1 file changed, 8 insertions(+), 40 deletions(-)
Reviewed-by: Rob Herring <[email protected]>
Hi Lorenzo,
On 04/12/20 1:21 pm, Kishon Vijay Abraham I wrote:
> Previously a subnode to syscon node was added which has the
> exact memory mapped address of pcie_ctrl but based on review comment
> provided by Rob [1], the offset is now being passed as argument to
> "ti,syscon-pcie-ctrl" phandle.
>
> This series has both driver change and DT change. The driver change
> should be merged first and the driver takes care of maintaining old
> DT compatibility.
Can you queue the 1st two patches of this series for this merge window?
I'll ask NM to queue the DTS patch. Let me know if you want me to resend
only the first two patches as a separate series.
Thank You,
Kishon
>
> Changes frm v1:
> *) Remove use of allOf in schema
> *) Added Fixes tag
> *) Maintain old DT compatibility
>
> [1] -> http://lore.kernel.org/r/CAL_JsqKiUcO76bo1GoepWM1TusJWoty_BRy2hFSgtEVMqtrvvQ@mail.gmail.com
>
> Kishon Vijay Abraham I (3):
> dt-bindings: pci: ti,j721e: Fix "ti,syscon-pcie-ctrl" to take argument
> PCI: j721e: Get offset within "syscon" from "ti,syscon-pcie-ctrl"
> phandle arg
> arm64: dts: ti: k3-j721e-main: Remove "syscon" nodes added for
> pcieX_ctrl
>
> .../bindings/pci/ti,j721e-pci-ep.yaml | 11 +++--
> .../bindings/pci/ti,j721e-pci-host.yaml | 11 +++--
> arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 48 ++++---------------
> drivers/pci/controller/cadence/pci-j721e.c | 28 +++++++----
> 4 files changed, 41 insertions(+), 57 deletions(-)
>
Hi Lorenzo,
On 10/12/20 12:17 pm, Kishon Vijay Abraham I wrote:
> Hi Lorenzo,
>
> On 04/12/20 1:21 pm, Kishon Vijay Abraham I wrote:
>> Previously a subnode to syscon node was added which has the
>> exact memory mapped address of pcie_ctrl but based on review comment
>> provided by Rob [1], the offset is now being passed as argument to
>> "ti,syscon-pcie-ctrl" phandle.
>>
>> This series has both driver change and DT change. The driver change
>> should be merged first and the driver takes care of maintaining old
>> DT compatibility.
>
> Can you queue the 1st two patches of this series for this merge window?
> I'll ask NM to queue the DTS patch. Let me know if you want me to resend
> only the first two patches as a separate series.
Never mind, I'll resend the pending patches for which I have already got
Acks from Rob.
Thank You,
Kishon
>
> Thank You,
> Kishon
>
>>
>> Changes frm v1:
>> *) Remove use of allOf in schema
>> *) Added Fixes tag
>> *) Maintain old DT compatibility
>>
>> [1] -> http://lore.kernel.org/r/CAL_JsqKiUcO76bo1GoepWM1TusJWoty_BRy2hFSgtEVMqtrvvQ@mail.gmail.com
>>
>> Kishon Vijay Abraham I (3):
>> dt-bindings: pci: ti,j721e: Fix "ti,syscon-pcie-ctrl" to take argument
>> PCI: j721e: Get offset within "syscon" from "ti,syscon-pcie-ctrl"
>> phandle arg
>> arm64: dts: ti: k3-j721e-main: Remove "syscon" nodes added for
>> pcieX_ctrl
>>
>> .../bindings/pci/ti,j721e-pci-ep.yaml | 11 +++--
>> .../bindings/pci/ti,j721e-pci-host.yaml | 11 +++--
>> arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 48 ++++---------------
>> drivers/pci/controller/cadence/pci-j721e.c | 28 +++++++----
>> 4 files changed, 41 insertions(+), 57 deletions(-)
>>