PCI: dra7xx: Support PCIe x2 lane mode
Subsystems (4)
v2 cover PCI: dra7xx: Support PCIe x2 lane mode
Previous version of this series can be found here @ [1]
Patch series adds support to enable x2 lane mode in dra74/dra76 and dra72 based boards in pci-dra7xx driver. It introduces new compatible strings in order to enable x2 lane mode support.
Changes from v1:
*) Added ti prefix to syscon-lane-conf and syscon-lane-sel as
suggested to Rob
*) Merged "PCI: dwc: dra7xx: Add support for SoC specific compatible
strings" and "PCI: dwc: pci-dra7xx: Enable x2 mode support" into a single patch.
*) Fixed $subject as suggested by Bjorn
*) Added x2 lane mode support for DRA72x
The dts changes and phy changes will be sent as a separate series.
[1] -> https://lkml.org/lkml/2017/10/10/276
Kishon Vijay Abraham I (3):
dt-bindings: PCI: dra7xx: Add SoC specific compatible strings
dt-bindings: PCI: dra7xx: Add properties to enable x2 lane in dra7
PCI: dra7xx: Enable x2 mode support for dra74x, dra76x and dra72x
Documentation/devicetree/bindings/pci/ti-pci.txt | 14 +++- drivers/pci/dwc/pci-dra7xx.c | 90 ++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 2 deletions(-)
Signature · 1 line
2.11.0
PATCH v2 1/3 dt-bindings: PCI: dra7xx: Add SoC specific compatible strings
Add new compatible strings for dra74x SoC (also used by dra76x) and dra72x. This can be used to perform SoC specific configuration (like configuring PCIe in x2 lane mode).
--- Documentation/devicetree/bindings/pci/ti-pci.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/pci/ti-pci.txt +6 -2
@@ -1,8 +1,12 @@ TI PCI Controllers PCIe DesignWare Controller - - compatible: Should be "ti,dra7-pcie" for RC - Should be "ti,dra7-pcie-ep" for EP + - compatible: Should be "ti,dra7-pcie" for RC (deprecated) + Should be "ti,dra7-pcie-ep" for EP (deprecated) + Should be "ti,dra746-pcie-rc" for dra74x/dra76 in RC mode + Should be "ti,dra746-pcie-ep" for dra74x/dra76 in EP mode + Should be "ti,dra726-pcie-rc" for dra72x in RC mode + Should be "ti,dra726-pcie-ep" for dra72x in EP mode - phys : list of PHY specifiers (used by generic PHY framework) - phy-names : must be "pcie-phy0", "pcie-phy1", "pcie-phyN".. based on the number of PHYs as specified in *phys* property.
Signature · 1 line
2.11.0
PATCH v2 2/3 dt-bindings: PCI: dra7xx: Add properties to enable x2 lane in dra7
In reply to Kishon Vijay Abraham IAdd syscon properties required for configuring PCIe in x2 lane mode.
--- Documentation/devicetree/bindings/pci/ti-pci.txt | 6 ++++++ 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/pci/ti-pci.txt +6 -0
@@ -13,6 +13,12 @@ PCIe DesignWare Controller - ti,hwmods : Name of the hwmod associated to the pcie, "pcie<X>", where <X> is the instance number of the pcie from the HW spec. - num-lanes as specified in ../designware-pcie.txt + - ti,syscon-lane-conf : phandle/offset pair. Phandle to the system control + module and the register offset to specify 1 lane or + 2 lane. + - ti,syscon-lane-sel : phandle/offset pair. Phandle to the system control + module and the register offset to specify lane + selection. HOST MODE =========
Signature · 1 line
2.11.0
Rob Herring
maintainerRe: [PATCH v2 2/3] dt-bindings: PCI: dra7xx: Add properties to enable x2 lane in dra7
Quoted 13 lines · Kishon Vijay Abraham I
Add syscon properties required for configuring PCIe in x2 lane mode.
Signed-off-by Kishon Vijay Abraham I <[email protected]>Signed-off-by Sekhar Nori <[email protected]>--- Documentation/devicetree/bindings/pci/ti-pci.txt | 6 ++++++ 1 file changed, 6 insertions(+)Documentation/devicetree/bindings/pci/ti-pci.txt +0
-0
Documentation/devicetree/bindings/pci/ti-pci.txt PCIe DesignWare Controller · line 21
Quoted 9 lines
Documentation/devicetree/bindings/pci/ti-pci.txt +6
-0@@ -13,6 +13,12 @@ PCIe DesignWare Controller - ti,hwmods : Name of the hwmod associated to the pcie, "pcie<X>", where <X> is the instance number of the pcie from the HW spec. - num-lanes as specified in ../designware-pcie.txt + - ti,syscon-lane-conf : phandle/offset pair. Phandle to the system control + module and the register offset to specify 1 lane or + 2 lane. + - ti,syscon-lane-sel : phandle/offset pair. Phandle to the system control + module and the register offset to specify lane + selection.
Adding a property for every syscon register doesn't really scale and doesn't work if the register layout changes.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] dt-bindings: PCI: dra7xx: Add properties to enable x2 lane in dra7
Hi Rob,
Quoted 18 lines · Rob Herring
Quoted 17 lines · Kishon Vijay Abraham I
Add syscon properties required for configuring PCIe in x2 lane mode.
Signed-off-by Kishon Vijay Abraham I <[email protected]>Signed-off-by Sekhar Nori <[email protected]>--- Documentation/devicetree/bindings/pci/ti-pci.txt | 6 ++++++ 1 file changed, 6 insertions(+)Documentation/devicetree/bindings/pci/ti-pci.txt +1
-0@@ -13,6 +13,12 @@ PCIe DesignWare Controller - ti,hwmods : Name of the hwmod associated to the pcie, "pcie<X>", where <X> is the instance number of the pcie from the HW spec. - num-lanes as specified in ../designware-pcie.txt + - ti,syscon-lane-conf : phandle/offset pair. Phandle to the system control
Quoted 8 lines
Documentation/devicetree/bindings/pci/ti-pci.txt PCIe DesignWare Controller · line 21
Quoted 5 lines
Documentation/devicetree/bindings/pci/ti-pci.txt +5
-0+ module and the register offset to specify 1 lane or + 2 lane. + - ti,syscon-lane-sel : phandle/offset pair. Phandle to the system control + module and the register offset to specify lane + selection.Adding a property for every syscon register doesn't really scale and doesn't work if the register layout changes.
The register layout doesn't really change between silicon revisions and for new SoCs, the phandle and the register offset for that SoC will have to be populated again.
Having said that, I'm not aware of any other alternative here.
Thanks
Kishon
Re: [PATCH v2 2/3] dt-bindings: PCI: dra7xx: Add properties to enable x2 lane in dra7
Quoted 30 lines · Kishon Vijay Abraham I
Hi Rob,
Quoted 18 lines · Rob Herring
Quoted 17 lines · Kishon Vijay Abraham I
Add syscon properties required for configuring PCIe in x2 lane mode.
Signed-off-by Kishon Vijay Abraham I <[email protected]>Signed-off-by Sekhar Nori <[email protected]>--- Documentation/devicetree/bindings/pci/ti-pci.txt | 6 ++++++ 1 file changed, 6 insertions(+)Documentation/devicetree/bindings/pci/ti-pci.txt +1
-0@@ -13,6 +13,12 @@ PCIe DesignWare Controller - ti,hwmods : Name of the hwmod associated to the pcie, "pcie<X>", where <X> is the instance number of the pcie from the HW spec. - num-lanes as specified in ../designware-pcie.txt + - ti,syscon-lane-conf : phandle/offset pair. Phandle to the system controlQuoted 8 lines
Documentation/devicetree/bindings/pci/ti-pci.txt PCIe DesignWare Controller · line 21
Quoted 5 lines
Documentation/devicetree/bindings/pci/ti-pci.txt +5
-0+ module and the register offset to specify 1 lane or + 2 lane. + - ti,syscon-lane-sel : phandle/offset pair. Phandle to the system control + module and the register offset to specify lane + selection.Adding a property for every syscon register doesn't really scale and doesn't work if the register layout changes.
Quoted 5 lines
The register layout doesn't really change between silicon revisions and for new SoCs, the phandle and the register offset for that SoC will have to be populated again.
Having said that, I'm not aware of any other alternative here.
Sorry I did not realize this is still open. Sounds like I need to revert commit 4ece93c020e3 ("ARM: dts: dra7: Add properties to enable PCIe x2 lane mode"), let me know if that is not the case.
Regards,
Tony
Rob Herring
maintainerRe: [PATCH v2 2/3] dt-bindings: PCI: dra7xx: Add properties to enable x2 lane in dra7
Quoted 31 lines · Tony Lindgren
Quoted 30 lines · Kishon Vijay Abraham I
Hi Rob,
Quoted 26 lines · Rob Herring
Quoted 13 lines · Kishon Vijay Abraham I
Add syscon properties required for configuring PCIe in x2 lane mode.
Signed-off-by Kishon Vijay Abraham I <[email protected]>Signed-off-by Sekhar Nori <[email protected]>--- Documentation/devicetree/bindings/pci/ti-pci.txt | 6 ++++++ 1 file changed, 6 insertions(+)Documentation/devicetree/bindings/pci/ti-pci.txt +0
-0Documentation/devicetree/bindings/pci/ti-pci.txt PCIe DesignWare Controller · line 21
Quoted 9 lines
Documentation/devicetree/bindings/pci/ti-pci.txt +6
-0@@ -13,6 +13,12 @@ PCIe DesignWare Controller - ti,hwmods : Name of the hwmod associated to the pcie, "pcie<X>", where <X> is the instance number of the pcie from the HW spec. - num-lanes as specified in ../designware-pcie.txt + - ti,syscon-lane-conf : phandle/offset pair. Phandle to the system control + module and the register offset to specify 1 lane or + 2 lane. + - ti,syscon-lane-sel : phandle/offset pair. Phandle to the system control + module and the register offset to specify lane + selection.Adding a property for every syscon register doesn't really scale and doesn't work if the register layout changes.
Quoted 3 lines
Quoted 3 lines
The register layout doesn't really change between silicon revisions and for new SoCs, the phandle and the register offset for that SoC will have to be populated again.
And what about SoCs that don't exist yet?
Quoted 1 line
Quoted 1 line
Having said that, I'm not aware of any other alternative here.
What would you do if you had 20 different syscon registers to configure? Add 20 properties? No, you would have per SoC functions in the driver to handle the different cases.
Quoted 3 lines
Sorry I did not realize this is still open. Sounds like I need to revert commit
4ece93c020e3("ARM: dts: dra7: Add properties to enable PCIe x2 lane mode"), let me know if that is not the case.
It's fine, I guess. Keep adding more syscon phandles and then I'll NAK it (if I remember :)).
Rob
Re: [PATCH v2 2/3] dt-bindings: PCI: dra7xx: Add properties to enable x2 lane in dra7
Quoted 36 lines · Rob Herring
Quoted 31 lines · Tony Lindgren
Quoted 30 lines · Kishon Vijay Abraham I
Hi Rob,
Quoted 26 lines · Rob Herring
Quoted 13 lines · Kishon Vijay Abraham I
Add syscon properties required for configuring PCIe in x2 lane mode.
Signed-off-by Kishon Vijay Abraham I <[email protected]>Signed-off-by Sekhar Nori <[email protected]>--- Documentation/devicetree/bindings/pci/ti-pci.txt | 6 ++++++ 1 file changed, 6 insertions(+)Documentation/devicetree/bindings/pci/ti-pci.txt +0
-0Documentation/devicetree/bindings/pci/ti-pci.txt PCIe DesignWare Controller · line 21
Quoted 9 lines
Documentation/devicetree/bindings/pci/ti-pci.txt +6
-0@@ -13,6 +13,12 @@ PCIe DesignWare Controller - ti,hwmods : Name of the hwmod associated to the pcie, "pcie<X>", where <X> is the instance number of the pcie from the HW spec. - num-lanes as specified in ../designware-pcie.txt + - ti,syscon-lane-conf : phandle/offset pair. Phandle to the system control + module and the register offset to specify 1 lane or + 2 lane. + - ti,syscon-lane-sel : phandle/offset pair. Phandle to the system control + module and the register offset to specify lane + selection.Adding a property for every syscon register doesn't really scale and doesn't work if the register layout changes.
Quoted 3 lines
Quoted 3 lines
The register layout doesn't really change between silicon revisions and for new SoCs, the phandle and the register offset for that SoC will have to be populated again.
Quoted 7 lines
And what about SoCs that don't exist yet?
Quoted 1 line
Quoted 1 line
Having said that, I'm not aware of any other alternative here.
What would you do if you had 20 different syscon registers to configure? Add 20 properties? No, you would have per SoC functions in the driver to handle the different cases.
Ideally these syscon registers would be managed by some Linux generic framework such as clock/regulator/mux/phy.
But yeah, if that does not work, then setting a SoC specific configuration function based on the compatible value makes sense to me.
Quoted 6 lines
Quoted 3 lines
Sorry I did not realize this is still open. Sounds like I need to revert commit
4ece93c020e3("ARM: dts: dra7: Add properties to enable PCIe x2 lane mode"), let me know if that is not the case.It's fine, I guess. Keep adding more syscon phandles and then I'll NAK it (if I remember :)).
Already reverted, thanks for the comments.
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] dt-bindings: PCI: dra7xx: Add properties to enable x2 lane in dra7
Hi,
Quoted 45 lines · Tony Lindgren
Quoted 36 lines · Rob Herring
Quoted 31 lines · Tony Lindgren
Quoted 30 lines · Kishon Vijay Abraham I
Hi Rob,
Quoted 26 lines · Rob Herring
Quoted 13 lines · Kishon Vijay Abraham I
Add syscon properties required for configuring PCIe in x2 lane mode.
Signed-off-by Kishon Vijay Abraham I <[email protected]>Signed-off-by Sekhar Nori <[email protected]>--- Documentation/devicetree/bindings/pci/ti-pci.txt | 6 ++++++ 1 file changed, 6 insertions(+)Documentation/devicetree/bindings/pci/ti-pci.txt +0
-0Documentation/devicetree/bindings/pci/ti-pci.txt PCIe DesignWare Controller · line 21
Quoted 9 lines
Documentation/devicetree/bindings/pci/ti-pci.txt +6
-0@@ -13,6 +13,12 @@ PCIe DesignWare Controller - ti,hwmods : Name of the hwmod associated to the pcie, "pcie<X>", where <X> is the instance number of the pcie from the HW spec. - num-lanes as specified in ../designware-pcie.txt + - ti,syscon-lane-conf : phandle/offset pair. Phandle to the system control + module and the register offset to specify 1 lane or + 2 lane. + - ti,syscon-lane-sel : phandle/offset pair. Phandle to the system control + module and the register offset to specify lane + selection.Adding a property for every syscon register doesn't really scale and doesn't work if the register layout changes.
Quoted 3 lines
Quoted 3 lines
The register layout doesn't really change between silicon revisions and for new SoCs, the phandle and the register offset for that SoC will have to be populated again.
Quoted 7 lines
And what about SoCs that don't exist yet?
Quoted 1 line
Quoted 1 line
Having said that, I'm not aware of any other alternative here.
What would you do if you had 20 different syscon registers to configure? Add 20 properties? No, you would have per SoC functions in the driver to handle the different cases.
Quoted 6 lines
Ideally these syscon registers would be managed by some Linux generic framework such as clock/regulator/mux/phy.
But yeah, if that does not work, then setting a SoC specific configuration function based on the compatible value makes sense to me.
All right. I'll add a function for syscon configuration in pci-dra7xx.
Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 3/3 PCI: dra7xx: Enable x2 mode support for dra74x, dra76x and dra72x
In reply to Kishon Vijay Abraham Idra74x/dra76x and dra72x has separate compatible strings. Add support for these compatible strings in pci-dra7xx driver to perform syscon configurations required to get x2 mode working.
--- drivers/pci/dwc/pci-dra7xx.c | 90 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+)
drivers/pci/dwc/pci-dra7xx.c +90 -0
@@ -83,11 +83,15 @@#define MSI_REQ_GRANT BIT(0)#define MSI_VECTOR_SHIFT 7 +#define PCIE_1LANE_2LANE_SELECTION BIT(13) +#define PCIE_B1C0_MODE_SEL BIT(2) +struct dra7xx_pcie {struct dw_pcie *pci;void __iomem *base;/* DT ti_conf */ int phy_count;/* DT phy-names count */ struct phy **phy; +u32 *b1c0_mask;int link_gen;struct irq_domain *irq_domain;enum dw_pcie_device_mode mode;
@@ -95,6 +99,7 @@ struct dra7xx_pcie {struct dra7xx_pcie_of_data {enum dw_pcie_device_mode mode; +u32 b1co_mode_sel_mask; };#define to_dra7xx_pcie (x) dev_get_drvdata((x)->dev)
@@ -533,6 +538,26 @@ static const struct dra7xx_pcie_of_data dra7xx_pcie_ep_of_data = { .mode = DW_PCIE_EP_TYPE, }; +static const struct dra7xx_pcie_of_data dra746_pcie_rc_of_data = { + .b1co_mode_sel_mask =BIT (2 ), + .mode =DW_PCIE_RC_TYPE , +}; + +static const struct dra7xx_pcie_of_data dra726_pcie_rc_of_data = { + .b1co_mode_sel_mask =GENMASK (3 ,2 ), + .mode =DW_PCIE_RC_TYPE , +}; + +static const struct dra7xx_pcie_of_data dra746_pcie_ep_of_data = { + .b1co_mode_sel_mask =BIT (2 ), + .mode =DW_PCIE_EP_TYPE , +}; + +static const struct dra7xx_pcie_of_data dra726_pcie_ep_of_data = { + .b1co_mode_sel_mask =GENMASK (3 ,2 ), + .mode =DW_PCIE_EP_TYPE , +}; +static const struct of_device_id of_dra7xx_pcie_match[] = { { .compatible ="ti,dra7-pcie" ,
@@ -542,6 +567,22 @@ static const struct of_device_id of_dra7xx_pcie_match[] = { .compatible = "ti,dra7-pcie-ep", .data = &dra7xx_pcie_ep_of_data, }, + { + .compatible = "ti,dra746-pcie-rc", + .data = &dra746_pcie_rc_of_data, + }, + { + .compatible = "ti,dra726-pcie-rc", + .data = &dra726_pcie_rc_of_data, + }, + { + .compatible = "ti,dra746-pcie-ep", + .data = &dra746_pcie_ep_of_data, + }, + { + .compatible = "ti,dra726-pcie-ep", + .data = &dra726_pcie_ep_of_data, + }, {}, };
@@ -587,6 +628,47 @@ static int dra7xx_pcie_ep_unaligned_memaccess(struct device *dev)return ret; } +static int dra7xx_pcie_configure_two_lane (struct device *dev, +u32 b1co_mode_sel_mask) +{ +struct device_node *np = dev->of_node; +struct regmap *pcie_syscon; +unsigned int pcie_reg; + + pcie_syscon =syscon_regmap_lookup_by_phandle (np, +"ti,syscon-lane-conf" ); +if (IS_ERR (pcie_syscon)) { +dev_err (dev,"unable to get ti,syscon-lane-conf\n" ); +return -EINVAL ; + } + +if (of_property_read_u32_index (np,"ti,syscon-lane-conf" ,1 , + &pcie_reg)) { +dev_err (dev,"couldn't get lane configuration reg offset\n" ); +return -EINVAL ; + } + +regmap_update_bits (pcie_syscon, pcie_reg,PCIE_1LANE_2LANE_SELECTION , +PCIE_1LANE_2LANE_SELECTION ); + + pcie_syscon =syscon_regmap_lookup_by_phandle (np,"ti,syscon-lane-sel" ); +if (IS_ERR (pcie_syscon)) { +dev_err (dev,"unable to get ti,syscon-lane-sel\n" ); +return -EINVAL ; + } + +if (of_property_read_u32_index (np,"ti,syscon-lane-sel" ,1 , + &pcie_reg)) { +dev_err (dev,"couldn't get lane selection reg offset\n" ); +return -EINVAL ; + } + +regmap_update_bits (pcie_syscon, pcie_reg, b1co_mode_sel_mask, +PCIE_B1C0_MODE_SEL ); + +return 0 ; +} +static int __init dra7xx_pcie_probe (struct platform_device *pdev) {u32 reg;
@@ -608,6 +690,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)const struct of_device_id *match;const struct dra7xx_pcie_of_data *data;enum dw_pcie_device_mode mode; +u32 b1co_mode_sel_mask; match =of_match_device (of_match_ptr (of_dra7xx_pcie_match), dev);if (!match)
@@ -615,6 +698,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev) data = (struct dra7xx_pcie_of_data *)match->data; mode = (enum dw_pcie_device_mode )data->mode; + b1co_mode_sel_mask = data->b1co_mode_sel_mask; dra7xx =devm_kzalloc (dev,sizeof (*dra7xx),GFP_KERNEL );if (!dra7xx)
@@ -673,6 +757,12 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev) dra7xx->pci = pci; dra7xx->phy_count = phy_count; +if (phy_count ==2 ) { + ret =dra7xx_pcie_configure_two_lane (dev, b1co_mode_sel_mask); +if (ret <0 ) + goto err_link; + } + ret =dra7xx_pcie_enable_phy (dra7xx);if (ret) {dev_err (dev,"failed to enable phy\n" );
Signature · 1 line
2.11.0
Re: [PATCH v2 3/3] PCI: dra7xx: Enable x2 mode support for dra74x, dra76x and dra72x
Quoted 1 line · Kishon Vijay Abraham I
dra74x/dra76x and dra72x has separate compatible strings. Add support
s/has/have
Quoted 15 lines
for these compatible strings in pci-dra7xx driver to perform syscon configurations required to get x2 mode working.
Signed-off-by Kishon Vijay Abraham I <[email protected]>--- drivers/pci/dwc/pci-dra7xx.c | 90 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+)drivers/pci/dwc/pci-dra7xx.c +0
-0@@ -83,11 +83,15 @@#define MSI_REQ_GRANT BIT(0)#define MSI_VECTOR_SHIFT 7
drivers/pci/dwc/pci-dra7xx.c line 94
Quoted 10 lines
drivers/pci/dwc/pci-dra7xx.c +4
-0+#define PCIE_1LANE_2LANE_SELECTION BIT(13) +#define PCIE_B1C0_MODE_SEL BIT(2) +struct dra7xx_pcie {struct dw_pcie *pci;void __iomem *base;/* DT ti_conf */ int phy_count;/* DT phy-names count */ struct phy **phy; +u32 *b1c0_mask;
This looks unused in the current patch.
Quoted 12 lines
drivers/pci/dwc/pci-dra7xx.c +1
-0int link_gen;struct irq_domain *irq_domain;enum dw_pcie_device_mode mode;@@ -95,6 +99,7 @@ struct dra7xx_pcie {struct dra7xx_pcie_of_data {enum dw_pcie_device_mode mode; +u32 b1co_mode_sel_mask; };#define to_dra7xx_pcie (x) dev_get_drvdata((x)->dev)
drivers/pci/dwc/pci-dra7xx.c static const struct dra7xx_pcie_of_data dra7xx_pcie_ep_of_data = { · line 542
Quoted 5 lines
drivers/pci/dwc/pci-dra7xx.c +2
-0@@ -533,6 +538,26 @@ static const struct dra7xx_pcie_of_data dra7xx_pcie_ep_of_data = { .mode = DW_PCIE_EP_TYPE, }; +static const struct dra7xx_pcie_of_data dra746_pcie_rc_of_data = { + .b1co_mode_sel_mask =BIT (2 ),
Nit: DT guys are more familiar than me on how this data should be encoded but maybe bit offset + length can be better ? I do not know, I have no problem leaving them as masks.
Quoted 60 lines
drivers/pci/dwc/pci-dra7xx.c +46
-0+ .mode = DW_PCIE_RC_TYPE, +}; + +static const struct dra7xx_pcie_of_data dra726_pcie_rc_of_data = { + .b1co_mode_sel_mask =GENMASK (3 ,2 ), + .mode =DW_PCIE_RC_TYPE , +}; + +static const struct dra7xx_pcie_of_data dra746_pcie_ep_of_data = { + .b1co_mode_sel_mask =BIT (2 ), + .mode =DW_PCIE_EP_TYPE , +}; + +static const struct dra7xx_pcie_of_data dra726_pcie_ep_of_data = { + .b1co_mode_sel_mask =GENMASK (3 ,2 ), + .mode =DW_PCIE_EP_TYPE , +}; +static const struct of_device_id of_dra7xx_pcie_match[] = { { .compatible ="ti,dra7-pcie" ,@@ -542,6 +567,22 @@ static const struct of_device_id of_dra7xx_pcie_match[] = { .compatible = "ti,dra7-pcie-ep", .data = &dra7xx_pcie_ep_of_data, }, + { + .compatible = "ti,dra746-pcie-rc", + .data = &dra746_pcie_rc_of_data, + }, + { + .compatible = "ti,dra726-pcie-rc", + .data = &dra726_pcie_rc_of_data, + }, + { + .compatible = "ti,dra746-pcie-ep", + .data = &dra746_pcie_ep_of_data, + }, + { + .compatible = "ti,dra726-pcie-ep", + .data = &dra726_pcie_ep_of_data, + }, {}, };@@ -587,6 +628,47 @@ static int dra7xx_pcie_ep_unaligned_memaccess(struct device *dev)return ret; } +static int dra7xx_pcie_configure_two_lane (struct device *dev, +u32 b1co_mode_sel_mask) +{ +struct device_node *np = dev->of_node; +struct regmap *pcie_syscon; +unsigned int pcie_reg; + + pcie_syscon =syscon_regmap_lookup_by_phandle (np, +"ti,syscon-lane-conf" ); +if (IS_ERR (pcie_syscon)) { +dev_err (dev,"unable to get ti,syscon-lane-conf\n" ); +return -EINVAL ;
drivers/pci/dwc/pci-dra7xx.c static int dra7xx_pcie_ep_unaligned_memaccess(struct device *dev) · line 652
Quoted 10 lines
drivers/pci/dwc/pci-dra7xx.c +10
-0+ } + +if (of_property_read_u32_index (np,"ti,syscon-lane-conf" ,1 , + &pcie_reg)) { +dev_err (dev,"couldn't get lane configuration reg offset\n" ); +return -EINVAL ; + } + +regmap_update_bits (pcie_syscon, pcie_reg,PCIE_1LANE_2LANE_SELECTION , +PCIE_1LANE_2LANE_SELECTION );
I do not know if this can create issues but the regmap should clear those bits in the error path ?
Quoted 23 lines
drivers/pci/dwc/pci-dra7xx.c +19
-0+ + pcie_syscon =syscon_regmap_lookup_by_phandle (np,"ti,syscon-lane-sel" ); +if (IS_ERR (pcie_syscon)) { +dev_err (dev,"unable to get ti,syscon-lane-sel\n" ); +return -EINVAL ; + } + +if (of_property_read_u32_index (np,"ti,syscon-lane-sel" ,1 , + &pcie_reg)) { +dev_err (dev,"couldn't get lane selection reg offset\n" ); +return -EINVAL ; + } + +regmap_update_bits (pcie_syscon, pcie_reg, b1co_mode_sel_mask, +PCIE_B1C0_MODE_SEL ); + +return 0 ; +} +static int __init dra7xx_pcie_probe (struct platform_device *pdev) {u32 reg;
drivers/pci/dwc/pci-dra7xx.c static int __init dra7xx_pcie_probe(struct platform_device *pdev) · line 693
Quoted 4 lines
drivers/pci/dwc/pci-dra7xx.c +1
-0@@ -608,6 +690,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)const struct of_device_id *match;const struct dra7xx_pcie_of_data *data;enum dw_pcie_device_mode mode; +u32 b1co_mode_sel_mask;
Again, do you need this temporary variable (given that you pass the value below) ?
Thanks,
Lorenzo
Quoted 26 lines
drivers/pci/dwc/pci-dra7xx.c +7
-0match =of_match_device (of_match_ptr (of_dra7xx_pcie_match), dev);if (!match)@@ -615,6 +698,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev) data = (struct dra7xx_pcie_of_data *)match->data; mode = (enum dw_pcie_device_mode )data->mode; + b1co_mode_sel_mask = data->b1co_mode_sel_mask; dra7xx =devm_kzalloc (dev,sizeof (*dra7xx),GFP_KERNEL );if (!dra7xx)@@ -673,6 +757,12 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev) dra7xx->pci = pci; dra7xx->phy_count = phy_count; +if (phy_count ==2 ) { + ret =dra7xx_pcie_configure_two_lane (dev, b1co_mode_sel_mask); +if (ret <0 ) + goto err_link; + } + ret =dra7xx_pcie_enable_phy (dra7xx);if (ret) {dev_err (dev,"failed to enable phy\n" );Signature · 1 line
2.11.0
Re: [PATCH v2 3/3] PCI: dra7xx: Enable x2 mode support for dra74x, dra76x and dra72x
Hi Lorenzo,
Quoted 21 lines · Lorenzo Pieralisi
Quoted 1 line · Kishon Vijay Abraham I
dra74x/dra76x and dra72x has separate compatible strings. Add support
s/has/have
Quoted 16 lines
for these compatible strings in pci-dra7xx driver to perform syscon configurations required to get x2 mode working.
Signed-off-by Kishon Vijay Abraham I <[email protected]>--- drivers/pci/dwc/pci-dra7xx.c | 90 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+)drivers/pci/dwc/pci-dra7xx.c +0
-0@@ -83,11 +83,15 @@#define MSI_REQ_GRANT BIT(0)#define MSI_VECTOR_SHIFT 7
Quoted 11 lines
drivers/pci/dwc/pci-dra7xx.c line 94
Quoted 9 lines
drivers/pci/dwc/pci-dra7xx.c +4
-0+#define PCIE_1LANE_2LANE_SELECTION BIT(13) +#define PCIE_B1C0_MODE_SEL BIT(2) +struct dra7xx_pcie {struct dw_pcie *pci;void __iomem *base;/* DT ti_conf */ int phy_count;/* DT phy-names count */ struct phy **phy; +u32 *b1c0_mask;This looks unused in the current patch.
yeah, I'll remove it while sending the next revision.
Quoted 88 lines
Quoted 13 lines
drivers/pci/dwc/pci-dra7xx.c +1
-0int link_gen;struct irq_domain *irq_domain;enum dw_pcie_device_mode mode;@@ -95,6 +99,7 @@ struct dra7xx_pcie {struct dra7xx_pcie_of_data {enum dw_pcie_device_mode mode; +u32 b1co_mode_sel_mask; };#define to_dra7xx_pcie (x) dev_get_drvdata((x)->dev)drivers/pci/dwc/pci-dra7xx.c static const struct dra7xx_pcie_of_data dra7xx_pcie_ep_of_data = { · line 542
Quoted 5 lines
drivers/pci/dwc/pci-dra7xx.c +2
-0@@ -533,6 +538,26 @@ static const struct dra7xx_pcie_of_data dra7xx_pcie_ep_of_data = { .mode = DW_PCIE_EP_TYPE, }; +static const struct dra7xx_pcie_of_data dra746_pcie_rc_of_data = { + .b1co_mode_sel_mask =BIT (2 ),Nit: DT guys are more familiar than me on how this data should be encoded but maybe bit offset + length can be better ? I do not know, I have no problem leaving them as masks.
Quoted 65 lines
drivers/pci/dwc/pci-dra7xx.c +51
-0+ .mode = DW_PCIE_RC_TYPE, +}; + +static const struct dra7xx_pcie_of_data dra726_pcie_rc_of_data = { + .b1co_mode_sel_mask =GENMASK (3 ,2 ), + .mode =DW_PCIE_RC_TYPE , +}; + +static const struct dra7xx_pcie_of_data dra746_pcie_ep_of_data = { + .b1co_mode_sel_mask =BIT (2 ), + .mode =DW_PCIE_EP_TYPE , +}; + +static const struct dra7xx_pcie_of_data dra726_pcie_ep_of_data = { + .b1co_mode_sel_mask =GENMASK (3 ,2 ), + .mode =DW_PCIE_EP_TYPE , +}; +static const struct of_device_id of_dra7xx_pcie_match[] = { { .compatible ="ti,dra7-pcie" ,@@ -542,6 +567,22 @@ static const struct of_device_id of_dra7xx_pcie_match[] = { .compatible = "ti,dra7-pcie-ep", .data = &dra7xx_pcie_ep_of_data, }, + { + .compatible = "ti,dra746-pcie-rc", + .data = &dra746_pcie_rc_of_data, + }, + { + .compatible = "ti,dra726-pcie-rc", + .data = &dra726_pcie_rc_of_data, + }, + { + .compatible = "ti,dra746-pcie-ep", + .data = &dra746_pcie_ep_of_data, + }, + { + .compatible = "ti,dra726-pcie-ep", + .data = &dra726_pcie_ep_of_data, + }, {}, };@@ -587,6 +628,47 @@ static int dra7xx_pcie_ep_unaligned_memaccess(struct device *dev)return ret; } +static int dra7xx_pcie_configure_two_lane (struct device *dev, +u32 b1co_mode_sel_mask) +{ +struct device_node *np = dev->of_node; +struct regmap *pcie_syscon; +unsigned int pcie_reg; + + pcie_syscon =syscon_regmap_lookup_by_phandle (np, +"ti,syscon-lane-conf" ); +if (IS_ERR (pcie_syscon)) { +dev_err (dev,"unable to get ti,syscon-lane-conf\n" ); +return -EINVAL ; + } + +if (of_property_read_u32_index (np,"ti,syscon-lane-conf" ,1 , + &pcie_reg)) { +dev_err (dev,"couldn't get lane configuration reg offset\n" );
Quoted 8 lines
drivers/pci/dwc/pci-dra7xx.c static int dra7xx_pcie_ep_unaligned_memaccess(struct device *dev) · line 652
Quoted 5 lines
drivers/pci/dwc/pci-dra7xx.c +5
-0+return -EINVAL ; + } + +regmap_update_bits (pcie_syscon, pcie_reg,PCIE_1LANE_2LANE_SELECTION , +PCIE_1LANE_2LANE_SELECTION );I do not know if this can create issues but the regmap should clear those bits in the error path ?
hmm yeah, maybe we should try to fall back to 1 lane mode.
Quoted 23 lines
Quoted 23 lines
drivers/pci/dwc/pci-dra7xx.c +19
-0+ + pcie_syscon =syscon_regmap_lookup_by_phandle (np,"ti,syscon-lane-sel" ); +if (IS_ERR (pcie_syscon)) { +dev_err (dev,"unable to get ti,syscon-lane-sel\n" ); +return -EINVAL ; + } + +if (of_property_read_u32_index (np,"ti,syscon-lane-sel" ,1 , + &pcie_reg)) { +dev_err (dev,"couldn't get lane selection reg offset\n" ); +return -EINVAL ; + } + +regmap_update_bits (pcie_syscon, pcie_reg, b1co_mode_sel_mask, +PCIE_B1C0_MODE_SEL ); + +return 0 ; +} +static int __init dra7xx_pcie_probe (struct platform_device *pdev) {u32 reg;
Quoted 8 lines
drivers/pci/dwc/pci-dra7xx.c static int __init dra7xx_pcie_probe(struct platform_device *pdev) · line 693
Quoted 5 lines
drivers/pci/dwc/pci-dra7xx.c +1
-0@@ -608,6 +690,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)const struct of_device_id *match;const struct dra7xx_pcie_of_data *data;enum dw_pcie_device_mode mode; +u32 b1co_mode_sel_mask;Again, do you need this temporary variable (given that you pass the value below) ?
I used so that we don't cross the 80 line character limit below.
Thanks
Kishon