Add Airoha EN7581 PCIE support
Subsystems (5)
cover Add Airoha EN7581 PCIE support
Introduce support for EN7581 SoC to mediatek-gen3 pcie driver
Lorenzo Bianconi (4):
dt-bindings: PCI: mediatek-gen3: add support for Airoha EN7581
PCI: mediatek-gen3: Add mtk_pcie_soc data structure
PCI: mediatek-gen3: rely on reset_bulk APIs for phy reset lines
PCI: mediatek-gen3: Add Airoha EN7581 support
.../bindings/pci/mediatek-pcie-gen3.yaml | 25 ++- drivers/pci/controller/Kconfig | 2 +- drivers/pci/controller/pcie-mediatek-gen3.c | 155 ++++++++++++++++-- 3 files changed, 161 insertions(+), 21 deletions(-)
Signature · 1 line
2.45.2
PATCH 1/4 dt-bindings: PCI: mediatek-gen3: add support for Airoha EN7581
Introduce Airoha EN7581 entry in mediatek-gen3 PCIe controller binding
--- .../bindings/pci/mediatek-pcie-gen3.yaml | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +20 -5
@@ -53,6 +53,7 @@ properties: - mediatek,mt8195-pcie - const: mediatek,mt8192-pcie - const: mediatek,mt8192-pcie + - const: airoha,en7581-pcie reg: maxItems: 1
@@ -76,20 +77,20 @@ properties: resets: minItems: 1 - maxItems: 2 + maxItems: 3 reset-names: minItems: 1 - maxItems: 2 + maxItems: 3 items: - enum: [ phy, mac ] + enum: [ phy, mac, phy-lane0, phy-lane1, phy-lane2 ] clocks: - minItems: 4 + minItems: 1 maxItems: 6 clock-names: - minItems: 4 + minItems: 1 maxItems: 6 assigned-clocks:
@@ -186,6 +187,20 @@ allOf: - const: tl_26m - const: peri_26m - const: top_133m + - if: + properties: + compatible: + const: airoha,en7581-pcie + then: + properties: + clock-names: + items: + - const: sys_ck + reset-names: + items: + - const: phy-lane0 + - const: phy-lane1 + - const: phy-lane2 unevaluatedProperties: false
Signature · 1 line
2.45.2
Re: [PATCH 1/4] dt-bindings: PCI: mediatek-gen3: add support for Airoha EN7581
Quoted 32 lines · Lorenzo Bianconi
Introduce Airoha EN7581 entry in mediatek-gen3 PCIe controller binding
Signed-off-by Lorenzo Bianconi <[email protected]>--- .../bindings/pci/mediatek-pcie-gen3.yaml | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-)Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +3
-3@@ -53,6 +53,7 @@ properties: - mediatek,mt8195-pcie - const: mediatek,mt8192-pcie - const: mediatek,mt8192-pcie + - const: airoha,en7581-pcie reg: maxItems: 1@@ -76,20 +77,20 @@ properties: resets: minItems: 1 - maxItems: 2 + maxItems: 3 reset-names: minItems: 1 - maxItems: 2 + maxItems: 3 items: - enum: [ phy, mac ]
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml properties: · line 93
Quoted 10 lines
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +3
-2+ enum: [ phy, mac, phy-lane0, phy-lane1, phy-lane2 ] clocks: - minItems: 4 + minItems: 1 maxItems: 6 clock-names: - minItems: 4 + minItems: 1
You've now relaxed the clock requirements for all devices, and permitted an extra reset on the existing platforms. You'll need to add some per-device min/maxItems constraints to solve that.
Thanks,
Conor.
Quoted 28 lines
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +14
-0maxItems: 6 assigned-clocks:@@ -186,6 +187,20 @@ allOf: - const: tl_26m - const: peri_26m - const: top_133m + - if: + properties: + compatible: + const: airoha,en7581-pcie + then: + properties: + clock-names: + items: + - const: sys_ck + reset-names: + items: + - const: phy-lane0 + - const: phy-lane1 + - const: phy-lane2 unevaluatedProperties: falseSignature · 1 line
2.45.2
Re: [PATCH 1/4] dt-bindings: PCI: mediatek-gen3: add support for Airoha EN7581
Quoted 40 lines · Lorenzo Bianconi
Quoted 39 lines
Introduce Airoha EN7581 entry in mediatek-gen3 PCIe controller binding
Signed-off-by Lorenzo Bianconi <[email protected]>--- .../bindings/pci/mediatek-pcie-gen3.yaml | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-)Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +5
-4@@ -53,6 +53,7 @@ properties: - mediatek,mt8195-pcie - const: mediatek,mt8192-pcie - const: mediatek,mt8192-pcie + - const: airoha,en7581-pcie reg: maxItems: 1@@ -76,20 +77,20 @@ properties: resets: minItems: 1 - maxItems: 2 + maxItems: 3 reset-names: minItems: 1 - maxItems: 2 + maxItems: 3 items: - enum: [ phy, mac ] + enum: [ phy, mac, phy-lane0, phy-lane1, phy-lane2 ] clocks: - minItems: 4 + minItems: 1 maxItems: 6
Quoted 7 lines
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml properties: · line 93
Quoted 3 lines
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +1
-1clock-names: - minItems: 4 + minItems: 1You've now relaxed the clock requirements for all devices, and permitted an extra reset on the existing platforms. You'll need to add some per-device min/maxItems constraints to solve that.
ack, I will fix it in v2.
Regards,
Lorenzo
Quoted 33 lines
Thanks,
Conor.Quoted 28 lines
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +14
-0maxItems: 6 assigned-clocks:@@ -186,6 +187,20 @@ allOf: - const: tl_26m - const: peri_26m - const: top_133m + - if: + properties: + compatible: + const: airoha,en7581-pcie + then: + properties: + clock-names: + items: + - const: sys_ck + reset-names: + items: + - const: phy-lane0 + - const: phy-lane1 + - const: phy-lane2 unevaluatedProperties: falseSignature · 1 line
2.45.2
PATCH 2/4 PCI: mediatek-gen3: Add mtk_pcie_soc data structure
In reply to Lorenzo BianconiIntroduce mtk_pcie_soc data structure in order to define multiple callbacks for each supported SoC. This is a preliminary patch to introduce EN7581 pcie support.
--- drivers/pci/controller/pcie-mediatek-gen3.c | 24 ++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-)
drivers/pci/controller/pcie-mediatek-gen3.c +21 -3
@@ -100,6 +100,16 @@#define PCIE_ATR_TLP_TYPE_MEM PCIE_ATR_TLP_TYPE(0)#define PCIE_ATR_TLP_TYPE_IO PCIE_ATR_TLP_TYPE(2) +struct mtk_gen3_pcie ; + +/** +* struct mtk_pcie_soc - differentiate between host generations +* @power_up: pcie power_up callback +*/ +struct mtk_pcie_soc { +int (*power_up)(struct mtk_gen3_pcie *pcie); +}; +/** * struct mtk_msi_set - MSI information for each set * @base: IO mapped register base
@@ -131,6 +141,7 @@ struct mtk_msi_set {* @msi_sets: MSI sets information * @lock: lock protecting IRQ bit map * @msi_irq_in_use: bit map for assigned MSI IRQ +* @soc: pointer to SoC-dependent operations */ struct mtk_gen3_pcie {struct device *dev;
@@ -151,6 +162,8 @@ struct mtk_gen3_pcie {struct mtk_msi_set msi_sets[PCIE_MSI_SET_NUM ];struct mutex lock;DECLARE_BITMAP (msi_irq_in_use,PCIE_MSI_IRQS_NUM ); + +const struct mtk_pcie_soc *soc; };/* LTSSM state in PCIE_LTSSM_STATUS_REG bit[28:24] */
@@ -904,7 +917,7 @@ static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie)usleep_range (10 ,20 );/* Don't touch the hardware registers before power up */ - err =mtk_pcie_power_up (pcie); + err = pcie->soc->power_up (pcie);if (err)return err;
@@ -939,6 +952,7 @@ static int mtk_pcie_probe(struct platform_device *pdev) pcie =pci_host_bridge_priv (host); pcie->dev = dev; + pcie->soc =of_device_get_match_data (dev);platform_set_drvdata (pdev, pcie); err =mtk_pcie_setup (pcie);
@@ -1054,7 +1068,7 @@ static int mtk_pcie_resume_noirq(struct device *dev)struct mtk_gen3_pcie *pcie =dev_get_drvdata (dev);int err; - err =mtk_pcie_power_up (pcie); + err = pcie->soc->power_up (pcie);if (err)return err;
@@ -1074,8 +1088,12 @@ static const struct dev_pm_ops mtk_pcie_pm_ops = { mtk_pcie_resume_noirq) }; +static const struct mtk_pcie_soc mtk_pcie_soc_mt8192 = { + .power_up = mtk_pcie_power_up, +}; +static const struct of_device_id mtk_pcie_of_match[] = { - { .compatible ="mediatek,mt8192-pcie" }, + { .compatible ="mediatek,mt8192-pcie" , .data = &mtk_pcie_soc_mt8192 }, {}, };MODULE_DEVICE_TABLE (of, mtk_pcie_of_match);
Signature · 1 line
2.45.2
AngeloGioacchino Del Regno
maintainerRe: [PATCH 2/4] PCI: mediatek-gen3: Add mtk_pcie_soc data structure
Il 21/06/24 16:48, Lorenzo Bianconi ha scritto:
Quoted 15 lines
Introduce
mtk_pcie_socdata structure in order to define multiple callbacks for each supported SoC. This is a preliminary patch to introduce EN7581 pcie support.Tested-by Zhengping Zhang <[email protected]>Signed-off-by Lorenzo Bianconi <[email protected]>--- drivers/pci/controller/pcie-mediatek-gen3.c | 24 ++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-)drivers/pci/controller/pcie-mediatek-gen3.c +0
-0
drivers/pci/controller/pcie-mediatek-gen3.c line 106
Quoted 7 lines
drivers/pci/controller/pcie-mediatek-gen3.c +4
-0@@ -100,6 +100,16 @@#define PCIE_ATR_TLP_TYPE_MEM PCIE_ATR_TLP_TYPE(0)#define PCIE_ATR_TLP_TYPE_IO PCIE_ATR_TLP_TYPE(2) +struct mtk_gen3_pcie ; + +/** +* struct mtk_pcie_soc - differentiate between host generations
mtk_gen3_pcie_pdata ?
Quoted 36 lines
drivers/pci/controller/pcie-mediatek-gen3.c +10
-1+* @power_up: pcie power_up callback +*/ +struct mtk_pcie_soc { +int (*power_up)(struct mtk_gen3_pcie *pcie); +}; +/** * struct mtk_msi_set - MSI information for each set * @base: IO mapped register base @@ -131,6 +141,7 @@ struct mtk_msi_set {* @msi_sets: MSI sets information * @lock: lock protecting IRQ bit map * @msi_irq_in_use: bit map for assigned MSI IRQ +* @soc: pointer to SoC-dependent operations */ struct mtk_gen3_pcie {struct device *dev;@@ -151,6 +162,8 @@ struct mtk_gen3_pcie {struct mtk_msi_set msi_sets[PCIE_MSI_SET_NUM ];struct mutex lock;DECLARE_BITMAP (msi_irq_in_use,PCIE_MSI_IRQS_NUM ); + +const struct mtk_pcie_soc *soc; };/* LTSSM state in PCIE_LTSSM_STATUS_REG bit[28:24] */ @@ -904,7 +917,7 @@ static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie)usleep_range (10 ,20 );/* Don't touch the hardware registers before power up */ - err =mtk_pcie_power_up (pcie); + err = pcie->soc->power_up (pcie);if (err)return err;
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_probe(struct platform_device *pdev) · line 955
Quoted 4 lines
drivers/pci/controller/pcie-mediatek-gen3.c +1
-0@@ -939,6 +952,7 @@ static int mtk_pcie_probe(struct platform_device *pdev) pcie =pci_host_bridge_priv (host); pcie->dev = dev; + pcie->soc =of_device_get_match_data (dev);
device_get_match_data() can also be used here :-)
Cheers,
Angelo
Re: [PATCH 2/4] PCI: mediatek-gen3: Add mtk_pcie_soc data structure
Quoted 11 lines
Il 21/06/24 16:48, Lorenzo Bianconi ha scritto:
Quoted 10 lines
Introduce
mtk_pcie_socdata structure in order to define multiple callbacks for each supported SoC. This is a preliminary patch to introduce EN7581 pcie support.Tested-by Zhengping Zhang <[email protected]>Signed-off-by Lorenzo Bianconi <[email protected]>--- drivers/pci/controller/pcie-mediatek-gen3.c | 24 ++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-)
Quoted 13 lines
drivers/pci/controller/pcie-mediatek-gen3.c line 105
Quoted 11 lines
drivers/pci/controller/pcie-mediatek-gen3.c +4
-0@@ -100,6 +100,16 @@#define PCIE_ATR_TLP_TYPE_MEM PCIE_ATR_TLP_TYPE(0)#define PCIE_ATR_TLP_TYPE_IO PCIE_ATR_TLP_TYPE(2) +struct mtk_gen3_pcie ; + +/** +* struct mtk_pcie_soc - differentiate between host generations
mtk_gen3_pcie_pdata?
ack, I will fix it in v2.
Quoted 31 lines
Quoted 31 lines
drivers/pci/controller/pcie-mediatek-gen3.c +10
-1+* @power_up: pcie power_up callback +*/ +struct mtk_pcie_soc { +int (*power_up)(struct mtk_gen3_pcie *pcie); +}; +/** * struct mtk_msi_set - MSI information for each set * @base: IO mapped register base @@ -131,6 +141,7 @@ struct mtk_msi_set {* @msi_sets: MSI sets information * @lock: lock protecting IRQ bit map * @msi_irq_in_use: bit map for assigned MSI IRQ +* @soc: pointer to SoC-dependent operations */ struct mtk_gen3_pcie {struct device *dev;@@ -151,6 +162,8 @@ struct mtk_gen3_pcie {struct mtk_msi_set msi_sets[PCIE_MSI_SET_NUM ];struct mutex lock;DECLARE_BITMAP (msi_irq_in_use,PCIE_MSI_IRQS_NUM ); + +const struct mtk_pcie_soc *soc; };/* LTSSM state in PCIE_LTSSM_STATUS_REG bit[28:24] */ @@ -904,7 +917,7 @@ static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie)usleep_range (10 ,20 );/* Don't touch the hardware registers before power up */ - err =mtk_pcie_power_up (pcie); + err = pcie->soc->power_up (pcie);
Quoted 8 lines
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_probe(struct platform_device *pdev) · line 954
Quoted 6 lines
drivers/pci/controller/pcie-mediatek-gen3.c +1
-0if (err)return err;@@ -939,6 +952,7 @@ static int mtk_pcie_probe(struct platform_device *pdev) pcie =pci_host_bridge_priv (host); pcie->dev = dev; + pcie->soc =of_device_get_match_data (dev);
device_get_match_data()can also be used here :-)
ack, I will fix it in v2.
Regards,
Lorenzo
Quoted 4 lines
Cheers,
Angelo
PATCH 3/4 PCI: mediatek-gen3: rely on reset_bulk APIs for phy reset lines
In reply to Lorenzo BianconiUse reset_bulk APIs to manage phy reset lines. This is a preliminary patch in order to add Airoha EN7581 pcie support.
--- drivers/pci/controller/pcie-mediatek-gen3.c | 49 ++++++++++++++++----- 1 file changed, 37 insertions(+), 12 deletions(-)
drivers/pci/controller/pcie-mediatek-gen3.c +37 -12
@@ -100,14 +100,21 @@#define PCIE_ATR_TLP_TYPE_MEM PCIE_ATR_TLP_TYPE(0)#define PCIE_ATR_TLP_TYPE_IO PCIE_ATR_TLP_TYPE(2) +#define MAX_NUM_PHY_RSTS 1 +struct mtk_gen3_pcie ;/** * struct mtk_pcie_soc - differentiate between host generations * @power_up: pcie power_up callback +* @phy_resets: phy reset lines SoC data. */ struct mtk_pcie_soc {int (*power_up)(struct mtk_gen3_pcie *pcie); +struct { +const char *id[MAX_NUM_PHY_RSTS ]; +int num_rsts; + } phy_resets; };/**
@@ -128,7 +135,7 @@ struct mtk_msi_set {* @base: IO mapped register base * @reg_base: physical register base * @mac_reset: MAC reset control -* @phy_reset: PHY reset control +* @phy_resets: PHY reset controllers * @phy: PHY controller block * @clks: PCIe clocks * @num_clks: PCIe clocks count for this port
@@ -148,7 +155,7 @@ struct mtk_gen3_pcie {void __iomem *base;phys_addr_t reg_base;struct reset_control *mac_reset; -struct reset_control *phy_reset; +struct reset_control_bulk_data phy_resets[MAX_NUM_PHY_RSTS ];struct phy *phy;struct clk_bulk_data *clks;int num_clks;
@@ -790,8 +797,8 @@ static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie) {struct device *dev = pcie->dev;struct platform_device *pdev =to_platform_device (dev); +int i, ret, num_rsts = pcie->soc->phy_resets.num_rsts;struct resource *regs; -int ret; regs =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"pcie-mac" );if (!regs)
@@ -804,12 +811,13 @@ static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie) pcie->reg_base = regs->start; - pcie->phy_reset =devm_reset_control_get_optional_exclusive (dev,"phy" ); -if (IS_ERR (pcie->phy_reset)) { - ret =PTR_ERR (pcie->phy_reset); -if (ret != -EPROBE_DEFER ) -dev_err (dev,"failed to get PHY reset\n" ); +for (i =0 ; i < num_rsts; i++) + pcie->phy_resets[i].id = pcie->soc->phy_resets.id[i]; + ret =devm_reset_control_bulk_get_optional_shared (dev, num_rsts, + pcie->phy_resets); +if (ret) { +dev_err (dev,"failed to get PHY bulk reset\n" );return ret; }
@@ -846,7 +854,12 @@ static int mtk_pcie_power_up(struct mtk_gen3_pcie *pcie)int err;/* PHY power on and enable pipe clock */ -reset_control_deassert (pcie->phy_reset); + err =reset_control_bulk_deassert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets); +if (err) { +dev_err (dev,"failed to deassert PHYs\n" ); +return err; + } err =phy_init (pcie->phy);if (err) {
@@ -882,7 +895,8 @@ static int mtk_pcie_power_up(struct mtk_gen3_pcie *pcie) err_phy_on:phy_exit (pcie->phy); err_phy_init: -reset_control_assert (pcie->phy_reset); +reset_control_bulk_assert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets);return err; }
@@ -897,7 +911,8 @@ static void mtk_pcie_power_down(struct mtk_gen3_pcie *pcie)phy_power_off (pcie->phy);phy_exit (pcie->phy); -reset_control_assert (pcie->phy_reset); +reset_control_bulk_assert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets); }static int mtk_pcie_setup (struct mtk_gen3_pcie *pcie)
@@ -912,7 +927,13 @@ static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie)* The controller may have been left out of reset by the bootloader * so make sure that we get a clean start by asserting resets here. */ -reset_control_assert (pcie->phy_reset); +reset_control_bulk_deassert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets); +usleep_range (5000 ,10000 ); +reset_control_bulk_assert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets); +msleep (100 ); +reset_control_assert (pcie->mac_reset);usleep_range (10 ,20 );
@@ -1090,6 +1111,10 @@ static const struct dev_pm_ops mtk_pcie_pm_ops = {static const struct mtk_pcie_soc mtk_pcie_soc_mt8192 = { .power_up = mtk_pcie_power_up, + .phy_resets = { + .id[0 ] ="phy" , + .num_rsts =1 , + }, };static const struct of_device_id mtk_pcie_of_match[] = {
Signature · 1 line
2.45.2
Re: [PATCH 3/4] PCI: mediatek-gen3: rely on reset_bulk APIs for phy reset lines
Quoted 2 lines · Lorenzo Bianconi
Use
reset_bulkAPIs to manage phy reset lines. This is a preliminary patch in order to add Airoha EN7581 pcie support.
If you have occasion to revise this:
s/rely/Rely/ in subject
s/phy/PHY/ in subject and commit log
s/pcie/PCIe/ in commit log
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie) · line 935
Quoted 11 lines
drivers/pci/controller/pcie-mediatek-gen3.c +6
-1@@ -912,7 +927,13 @@ static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie)* The controller may have been left out of reset by the bootloader * so make sure that we get a clean start by asserting resets here. */ -reset_control_assert (pcie->phy_reset); +reset_control_bulk_deassert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets); +usleep_range (5000 ,10000 ); +reset_control_bulk_assert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets); +msleep (100 );
Where did these usleep and msleep numbers come from? They should use a #define that we can connect back to a spec.
These delays should also be mentioned in the commit log because it appears unrelated to the conversion to the reset_bulk API. Actually, it would be even better if they were in a separate patch, since it looks like a logically separate change.
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie) · line 938
Quoted 2 lines
drivers/pci/controller/pcie-mediatek-gen3.c +0
-0reset_control_assert (pcie->mac_reset);usleep_range (10 ,20 );
Unrelated to this patch, but it would be nice to have an explanation of this existing delay, too.
Bjorn
Re: [PATCH 3/4] PCI: mediatek-gen3: rely on reset_bulk APIs for phy reset lines
Quoted 9 lines · Lorenzo Bianconi
Quoted 2 lines
Use
reset_bulkAPIs to manage phy reset lines. This is a preliminary patch in order to add Airoha EN7581 pcie support.If you have occasion to revise this:
s/rely/Rely/ in subject
s/phy/PHY/ in subject and commit log
s/pcie/PCIe/ in commit log
ack, I will fix them in v2
Quoted 15 lines
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie) · line 935
Quoted 12 lines
drivers/pci/controller/pcie-mediatek-gen3.c +6
-1@@ -912,7 +927,13 @@ static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie)* The controller may have been left out of reset by the bootloader * so make sure that we get a clean start by asserting resets here. */ -reset_control_assert (pcie->phy_reset); +reset_control_bulk_deassert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets); +usleep_range (5000 ,10000 ); +reset_control_bulk_assert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets); +msleep (100 );Where did these usleep and msleep numbers come from? They should use a #define that we can connect back to a spec.
I think we can get rid of the first usleep_range() since we need to deassert the line just to avoid unbalance in deassert_count counter since the reset line is shared (the line is actually already de-assert). I will add a comment to clarify it.
Quoted 5 lines
These delays should also be mentioned in the commit log because it appears unrelated to the conversion to the
reset_bulkAPI. Actually, it would be even better if they were in a separate patch, since it looks like a logically separate change.
Regarding the msleep(100), it is not documented in the vendor sdk, I think it necessary to complete the reset before initialize the pcie-phy. Since it is required just for EN7581, I guess we can move it in mtk_pcie_en7581_power_up() (in patch 4/4) before the phy_init(). What do you think?
Regards,
Lorenzo
Quoted 8 lines
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie) · line 938
Quoted 3 lines
drivers/pci/controller/pcie-mediatek-gen3.c +0
-0reset_control_assert (pcie->mac_reset);usleep_range (10 ,20 );Unrelated to this patch, but it would be nice to have an explanation of this existing delay, too.
Bjorn
AngeloGioacchino Del Regno
maintainerRe: [PATCH 3/4] PCI: mediatek-gen3: rely on reset_bulk APIs for phy reset lines
In reply to Lorenzo BianconiIl 21/06/24 16:48, Lorenzo Bianconi ha scritto:
Quoted 21 lines
Use
reset_bulkAPIs to manage phy reset lines. This is a preliminary patch in order to add Airoha EN7581 pcie support.Tested-by Zhengping Zhang <[email protected]>Signed-off-by Lorenzo Bianconi <[email protected]>--- drivers/pci/controller/pcie-mediatek-gen3.c | 49 ++++++++++++++++----- 1 file changed, 37 insertions(+), 12 deletions(-)drivers/pci/controller/pcie-mediatek-gen3.c +2
-0@@ -100,14 +100,21 @@#define PCIE_ATR_TLP_TYPE_MEM PCIE_ATR_TLP_TYPE(0)#define PCIE_ATR_TLP_TYPE_IO PCIE_ATR_TLP_TYPE(2) +#define MAX_NUM_PHY_RSTS 1 +struct mtk_gen3_pcie ;
drivers/pci/controller/pcie-mediatek-gen3.c line 116
Quoted 10 lines
drivers/pci/controller/pcie-mediatek-gen3.c +4
-0/** * struct mtk_pcie_soc - differentiate between host generations * @power_up: pcie power_up callback +* @phy_resets: phy reset lines SoC data. */ struct mtk_pcie_soc {int (*power_up)(struct mtk_gen3_pcie *pcie); +struct { +const char *id[MAX_NUM_PHY_RSTS ]; +int num_rsts;
Well, it's just two chars after all, so "num_resets" looks better imo.
Quoted 35 lines
drivers/pci/controller/pcie-mediatek-gen3.c +4
-3+ } phy_resets; };/** @@ -128,7 +135,7 @@ struct mtk_msi_set {* @base: IO mapped register base * @reg_base: physical register base * @mac_reset: MAC reset control -* @phy_reset: PHY reset control +* @phy_resets: PHY reset controllers * @phy: PHY controller block * @clks: PCIe clocks * @num_clks: PCIe clocks count for this port @@ -148,7 +155,7 @@ struct mtk_gen3_pcie {void __iomem *base;phys_addr_t reg_base;struct reset_control *mac_reset; -struct reset_control *phy_reset; +struct reset_control_bulk_data phy_resets[MAX_NUM_PHY_RSTS ];struct phy *phy;struct clk_bulk_data *clks;int num_clks;@@ -790,8 +797,8 @@ static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie) {struct device *dev = pcie->dev;struct platform_device *pdev =to_platform_device (dev); + int i, ret, num_rsts = pcie->soc->phy_resets.num_rsts; > struct resource *regs; -int ret; regs =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"pcie-mac" );if (!regs)@@ -804,12 +811,13 @@ static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie) pcie->reg_base = regs->start;
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie) · line 818
Quoted 10 lines
drivers/pci/controller/pcie-mediatek-gen3.c +4
-5- pcie->phy_reset =devm_reset_control_get_optional_exclusive (dev,"phy" ); -if (IS_ERR (pcie->phy_reset)) { - ret =PTR_ERR (pcie->phy_reset); -if (ret != -EPROBE_DEFER ) -dev_err (dev,"failed to get PHY reset\n" ); +for (i =0 ; i < num_rsts; i++) + pcie->phy_resets[i].id = pcie->soc->phy_resets.id[i]; + ret =devm_reset_control_bulk_get_optional_shared (dev, num_rsts, + pcie->phy_resets);
92 columns is ok, you can use one line for that.
Quoted 20 lines
drivers/pci/controller/pcie-mediatek-gen3.c +8
-1+if (ret) { +dev_err (dev,"failed to get PHY bulk reset\n" );return ret; }@@ -846,7 +854,12 @@ static int mtk_pcie_power_up(struct mtk_gen3_pcie *pcie)int err;/* PHY power on and enable pipe clock */ -reset_control_deassert (pcie->phy_reset); + err =reset_control_bulk_deassert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets); +if (err) { +dev_err (dev,"failed to deassert PHYs\n" ); +return err; + } err =phy_init (pcie->phy);if (err) {
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_power_up(struct mtk_gen3_pcie *pcie) · line 899
Quoted 6 lines
drivers/pci/controller/pcie-mediatek-gen3.c +2
-1@@ -882,7 +895,8 @@ static int mtk_pcie_power_up(struct mtk_gen3_pcie *pcie) err_phy_on:phy_exit (pcie->phy); err_phy_init: -reset_control_assert (pcie->phy_reset); +reset_control_bulk_assert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets);
same here
drivers/pci/controller/pcie-mediatek-gen3.c static void mtk_pcie_power_down(struct mtk_gen3_pcie *pcie) · line 915
Quoted 10 lines
drivers/pci/controller/pcie-mediatek-gen3.c +2
-1return err; }@@ -897,7 +911,8 @@ static void mtk_pcie_power_down(struct mtk_gen3_pcie *pcie)phy_power_off (pcie->phy);phy_exit (pcie->phy); -reset_control_assert (pcie->phy_reset); +reset_control_bulk_assert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets);
ditto
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie) · line 931
Quoted 10 lines
drivers/pci/controller/pcie-mediatek-gen3.c +2
-1}static int mtk_pcie_setup (struct mtk_gen3_pcie *pcie)@@ -912,7 +927,13 @@ static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie)* The controller may have been left out of reset by the bootloader * so make sure that we get a clean start by asserting resets here. */ -reset_control_assert (pcie->phy_reset); +reset_control_bulk_deassert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets);
and again...
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie) · line 934
Quoted 3 lines
drivers/pci/controller/pcie-mediatek-gen3.c +3
-0+usleep_range (5000 ,10000 ); +reset_control_bulk_assert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets);
.... :-)
Cheers,
Angelo
Quoted 16 lines
drivers/pci/controller/pcie-mediatek-gen3.c +6
-0+msleep (100 ); +reset_control_assert (pcie->mac_reset);usleep_range (10 ,20 );@@ -1090,6 +1111,10 @@ static const struct dev_pm_ops mtk_pcie_pm_ops = {static const struct mtk_pcie_soc mtk_pcie_soc_mt8192 = { .power_up = mtk_pcie_power_up, + .phy_resets = { + .id[0 ] ="phy" , + .num_rsts =1 , + }, };static const struct of_device_id mtk_pcie_of_match[] = {
Re: [PATCH 3/4] PCI: mediatek-gen3: rely on reset_bulk APIs for phy reset lines
Quoted 24 lines
Il 21/06/24 16:48, Lorenzo Bianconi ha scritto:
Quoted 23 lines
Use
reset_bulkAPIs to manage phy reset lines. This is a preliminary patch in order to add Airoha EN7581 pcie support.Tested-by Zhengping Zhang <[email protected]>Signed-off-by Lorenzo Bianconi <[email protected]>--- drivers/pci/controller/pcie-mediatek-gen3.c | 49 ++++++++++++++++----- 1 file changed, 37 insertions(+), 12 deletions(-)drivers/pci/controller/pcie-mediatek-gen3.c +3
-0@@ -100,14 +100,21 @@#define PCIE_ATR_TLP_TYPE_MEM PCIE_ATR_TLP_TYPE(0)#define PCIE_ATR_TLP_TYPE_IO PCIE_ATR_TLP_TYPE(2) +#define MAX_NUM_PHY_RSTS 1 +struct mtk_gen3_pcie ;/** * struct mtk_pcie_soc - differentiate between host generations * @power_up: pcie power_up callback +* @phy_resets: phy reset lines SoC data.
Quoted 8 lines
drivers/pci/controller/pcie-mediatek-gen3.c line 116
Quoted 6 lines
drivers/pci/controller/pcie-mediatek-gen3.c +3
-0*/ struct mtk_pcie_soc {int (*power_up)(struct mtk_gen3_pcie *pcie); +struct { +const char *id[MAX_NUM_PHY_RSTS ]; +int num_rsts;Well, it's just two chars after all, so "
num_resets" looks better imo.
ack, fine. Naming is always hard :)
Quoted 35 lines
Quoted 35 lines
drivers/pci/controller/pcie-mediatek-gen3.c +4
-6+ } phy_resets; };/** @@ -128,7 +135,7 @@ struct mtk_msi_set {* @base: IO mapped register base * @reg_base: physical register base * @mac_reset: MAC reset control -* @phy_reset: PHY reset control +* @phy_resets: PHY reset controllers * @phy: PHY controller block * @clks: PCIe clocks * @num_clks: PCIe clocks count for this port @@ -148,7 +155,7 @@ struct mtk_gen3_pcie {void __iomem *base;phys_addr_t reg_base;struct reset_control *mac_reset; -struct reset_control *phy_reset; +struct reset_control_bulk_data phy_resets[MAX_NUM_PHY_RSTS ];struct phy *phy;struct clk_bulk_data *clks;int num_clks;@@ -790,8 +797,8 @@ static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie) {struct device *dev = pcie->dev;struct platform_device *pdev =to_platform_device (dev); + int i, ret, num_rsts = pcie->soc->phy_resets.num_rsts; > struct resource *regs; -int ret; regs =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"pcie-mac" );if (!regs)@@ -804,12 +811,13 @@ static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie) pcie->reg_base = regs->start; - pcie->phy_reset =devm_reset_control_get_optional_exclusive (dev,"phy" ); -if (IS_ERR (pcie->phy_reset)) { - ret =PTR_ERR (pcie->phy_reset);
Quoted 8 lines
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie) · line 818
Quoted 6 lines
drivers/pci/controller/pcie-mediatek-gen3.c +4
-2-if (ret != -EPROBE_DEFER ) -dev_err (dev,"failed to get PHY reset\n" ); +for (i =0 ; i < num_rsts; i++) + pcie->phy_resets[i].id = pcie->soc->phy_resets.id[i]; + ret =devm_reset_control_bulk_get_optional_shared (dev, num_rsts, + pcie->phy_resets);92 columns is ok, you can use one line for that.
I usually prefer to stay below 79 column limit, but I do not have a strong opinion about it. I will fix it and even all below.
Regards,
Lorenzo
Quoted 73 lines
Quoted 18 lines
drivers/pci/controller/pcie-mediatek-gen3.c +8
-1+if (ret) { +dev_err (dev,"failed to get PHY bulk reset\n" );return ret; }@@ -846,7 +854,12 @@ static int mtk_pcie_power_up(struct mtk_gen3_pcie *pcie)int err;/* PHY power on and enable pipe clock */ -reset_control_deassert (pcie->phy_reset); + err =reset_control_bulk_deassert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets); +if (err) { +dev_err (dev,"failed to deassert PHYs\n" ); +return err; + } err =phy_init (pcie->phy);if (err) {drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_power_up(struct mtk_gen3_pcie *pcie) · line 899
Quoted 6 lines
drivers/pci/controller/pcie-mediatek-gen3.c +2
-1@@ -882,7 +895,8 @@ static int mtk_pcie_power_up(struct mtk_gen3_pcie *pcie) err_phy_on:phy_exit (pcie->phy); err_phy_init: -reset_control_assert (pcie->phy_reset); +reset_control_bulk_assert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets);same here
drivers/pci/controller/pcie-mediatek-gen3.c static void mtk_pcie_power_down(struct mtk_gen3_pcie *pcie) · line 914
Quoted 8 lines
drivers/pci/controller/pcie-mediatek-gen3.c +2
-1return err; }@@ -897,7 +911,8 @@ static void mtk_pcie_power_down(struct mtk_gen3_pcie *pcie)phy_power_off (pcie->phy);phy_exit (pcie->phy); -reset_control_assert (pcie->phy_reset); +reset_control_bulk_assert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets);ditto
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie) · line 931
Quoted 9 lines
drivers/pci/controller/pcie-mediatek-gen3.c +2
-1}static int mtk_pcie_setup (struct mtk_gen3_pcie *pcie)@@ -912,7 +927,13 @@ static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie)* The controller may have been left out of reset by the bootloader * so make sure that we get a clean start by asserting resets here. */ -reset_control_assert (pcie->phy_reset); +reset_control_bulk_deassert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets);and again...
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie) · line 934
Quoted 3 lines
drivers/pci/controller/pcie-mediatek-gen3.c +3
-0+usleep_range (5000 ,10000 ); +reset_control_bulk_assert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets);.... :-)
Cheers,
AngeloQuoted 13 lines
drivers/pci/controller/pcie-mediatek-gen3.c +6
-0+msleep (100 ); +reset_control_assert (pcie->mac_reset);usleep_range (10 ,20 );@@ -1090,6 +1111,10 @@ static const struct dev_pm_ops mtk_pcie_pm_ops = {static const struct mtk_pcie_soc mtk_pcie_soc_mt8192 = { .power_up = mtk_pcie_power_up, + .phy_resets = { + .id[0 ] ="phy" , + .num_rsts =1 , + }, };static const struct of_device_id mtk_pcie_of_match[] = {
PATCH 4/4 PCI: mediatek-gen3: Add Airoha EN7581 support
In reply to Lorenzo BianconiIntroduce support for Airoha EN7581 pcie controller to mediatek-gen3 pcie controller driver.
--- drivers/pci/controller/Kconfig | 2 +- drivers/pci/controller/pcie-mediatek-gen3.c | 84 ++++++++++++++++++++- 2 files changed, 84 insertions(+), 2 deletions(-)
drivers/pci/controller/Kconfig +1 -1
@@ -196,7 +196,7 @@ config PCIE_MEDIATEK config PCIE_MEDIATEK_GEN3 tristate "MediaTek Gen3 PCIe controller" - depends on ARCH_MEDIATEK || COMPILE_TEST + depends on ARCH_AIROHA || ARCH_MEDIATEK || COMPILE_TEST depends on PCI_MSI help Adds support for PCIe Gen3 MAC controller for MediaTek SoCs.
drivers/pci/controller/pcie-mediatek-gen3.c +83 -1
128 lines not on this page · raw message
Signature · 1 line
2.45.2
Re: [PATCH 4/4] PCI: mediatek-gen3: Add Airoha EN7581 support
Quoted 2 lines · Lorenzo Bianconi
Introduce support for Airoha EN7581 pcie controller to mediatek-gen3 pcie controller driver.
s/pcie/PCIe/ (twice)
Quoted 33 lines
Tested-by Zhengping Zhang <[email protected]>Signed-off-by Lorenzo Bianconi <[email protected]>--- drivers/pci/controller/Kconfig | 2 +- drivers/pci/controller/pcie-mediatek-gen3.c | 84 ++++++++++++++++++++- 2 files changed, 84 insertions(+), 2 deletions(-)drivers/pci/controller/Kconfig +1
-1@@ -196,7 +196,7 @@ config PCIE_MEDIATEK config PCIE_MEDIATEK_GEN3 tristate "MediaTek Gen3 PCIe controller" - depends on ARCH_MEDIATEK || COMPILE_TEST + depends on ARCH_AIROHA || ARCH_MEDIATEK || COMPILE_TEST depends on PCI_MSI help Adds support for PCIe Gen3 MAC controller for MediaTek SoCs.drivers/pci/controller/pcie-mediatek-gen3.c +1
-0@@ -7,6 +7,7 @@*/ #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/delay.h> #include <linux/iopoll.h> #include <linux/irq.h>
drivers/pci/controller/pcie-mediatek-gen3.c line 26
Quoted 5 lines
drivers/pci/controller/pcie-mediatek-gen3.c +2
-0@@ -21,6 +22,8 @@#include <linux/pm_domain.h> #include <linux/pm_runtime.h> #include <linux/reset.h> +#include <linux/of_pci.h> +#include <linux/of_device.h>
Existing list of includes is sorted. Preserve that sorted order.
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie) · line 863
Quoted 8 lines
drivers/pci/controller/pcie-mediatek-gen3.c +8
-0+static int mtk_pcie_en7581_power_up (struct mtk_gen3_pcie *pcie) +{ +struct device *dev = pcie->dev; +int err; + +writel_relaxed (0x23020133 , pcie->base +0x10044 ); +writel_relaxed (0x50500032 , pcie->base +0x15030 ); +writel_relaxed (0x50500032 , pcie->base +0x15130 );
Magic. Needs #defines at least for the registers. These offsets are HUGE, far bigger than the existing offsets:
#define PCIE_CFGNUM_REG 0x140 #define PCIE_CFG_OFFSET_ADDR 0x1000 #define PCIE_TRANS_TABLE_BASE_REG 0x800 #define PCIE_MSI_SET_BASE_REG 0xc00 #define PCIE_MSI_SET_ADDR_HI_BASE 0xc80 #define PCIE_MSI_SET_ENABLE_REG 0x190 #define PCIE_INT_ENABLE_REG 0x180 #define PCIE_SETTING_REG 0x80 #define PCIE_PCI_IDS_1 0x9c #define PCIE_MISC_CTRL_REG 0x348 #define PCIE_RST_CTRL_REG 0x148 #define PCIE_LINK_STATUS_REG 0x154 #define PCIE_LTSSM_STATUS_REG 0x150 #define PCIE_INT_STATUS_REG 0x184
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie) · line 870
Quoted 6 lines
drivers/pci/controller/pcie-mediatek-gen3.c +6
-0+ err =phy_init (pcie->phy); +if (err) { +dev_err (dev,"failed to initialize PHY\n" ); +return err; + } +mdelay (30 );
Source? Cite the spec that requires this delay and add a #define if possible.
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie) · line 884
Quoted 13 lines
drivers/pci/controller/pcie-mediatek-gen3.c +13
-0+ err =phy_power_on (pcie->phy); +if (err) { +dev_err (dev,"failed to power on PHY\n" ); + goto err_phy_on; + } + + err =reset_control_bulk_deassert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets); +if (err) { +dev_err (dev,"failed to deassert PHYs\n" ); + goto err_phy_deassert; + } +usleep_range (5000 ,10000 );
Source?
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie) · line 897
Quoted 12 lines
drivers/pci/controller/pcie-mediatek-gen3.c +12
-0+pm_runtime_enable (dev); +pm_runtime_get_sync (dev); + + err =clk_bulk_prepare (pcie->num_clks, pcie->clks); +if (err) { +dev_err (dev,"failed to prepare clock\n" ); + goto err_clk_prepare; + } + +writel_relaxed (0x41474147 , pcie->base +PCIE_EQ_PRESET_01_REF ); +writel_relaxed (0x1018020f , pcie->base +PCIE_PIPE4_PIE8_REG ); +mdelay (10 );
Source?
Re: [PATCH 4/4] PCI: mediatek-gen3: Add Airoha EN7581 support
Quoted 6 lines · Lorenzo Bianconi
Quoted 2 lines
Introduce support for Airoha EN7581 pcie controller to mediatek-gen3 pcie controller driver.
s/pcie/PCIe/ (twice)
ack, I will fix them in v2
Quoted 27 lines
Quoted 27 lines
Tested-by Zhengping Zhang <[email protected]>Signed-off-by Lorenzo Bianconi <[email protected]>--- drivers/pci/controller/Kconfig | 2 +- drivers/pci/controller/pcie-mediatek-gen3.c | 84 ++++++++++++++++++++- 2 files changed, 84 insertions(+), 2 deletions(-)drivers/pci/controller/Kconfig +1
-1@@ -196,7 +196,7 @@ config PCIE_MEDIATEK config PCIE_MEDIATEK_GEN3 tristate "MediaTek Gen3 PCIe controller" - depends on ARCH_MEDIATEK || COMPILE_TEST + depends on ARCH_AIROHA || ARCH_MEDIATEK || COMPILE_TEST depends on PCI_MSI help Adds support for PCIe Gen3 MAC controller for MediaTek SoCs.drivers/pci/controller/pcie-mediatek-gen3.c +0
-0@@ -7,6 +7,7 @@*/
Quoted 13 lines
drivers/pci/controller/pcie-mediatek-gen3.c line 26
Quoted 11 lines
drivers/pci/controller/pcie-mediatek-gen3.c +3
-0#include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/delay.h> #include <linux/iopoll.h> #include <linux/irq.h> @@ -21,6 +22,8 @@#include <linux/pm_domain.h> #include <linux/pm_runtime.h> #include <linux/reset.h> +#include <linux/of_pci.h> +#include <linux/of_device.h> Existing list of includes is sorted. Preserve that sorted order.
ack, I will fix them in v2
Quoted 12 lines
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie) · line 863
Quoted 9 lines
drivers/pci/controller/pcie-mediatek-gen3.c +8
-0+static int mtk_pcie_en7581_power_up (struct mtk_gen3_pcie *pcie) +{ +struct device *dev = pcie->dev; +int err; + +writel_relaxed (0x23020133 , pcie->base +0x10044 ); +writel_relaxed (0x50500032 , pcie->base +0x15030 ); +writel_relaxed (0x50500032 , pcie->base +0x15130 );Magic. Needs #defines at least for the registers. These offsets are HUGE, far bigger than the existing offsets:
ack. Anyway they are in pcie mapped regions (I have not posted the dts changes yet):
pcie0: reg = <0x0 0x1fc00000 0x0 0x20000>;
pcie1: reg = <0x0 0x1fc20000 0x0 0x20000>;
But they are undocumented. I will try to get some info for them.
Quoted 16 lines
#define PCIE_CFGNUM_REG 0x140 #define PCIE_CFG_OFFSET_ADDR 0x1000 #define PCIE_TRANS_TABLE_BASE_REG 0x800 #define PCIE_MSI_SET_BASE_REG 0xc00 #define PCIE_MSI_SET_ADDR_HI_BASE 0xc80 #define PCIE_MSI_SET_ENABLE_REG 0x190 #define PCIE_INT_ENABLE_REG 0x180 #define PCIE_SETTING_REG 0x80 #define PCIE_PCI_IDS_1 0x9c #define PCIE_MISC_CTRL_REG 0x348 #define PCIE_RST_CTRL_REG 0x148 #define PCIE_LINK_STATUS_REG 0x154 #define PCIE_LTSSM_STATUS_REG 0x150 #define PCIE_INT_STATUS_REG 0x184
Quoted 9 lines
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie) · line 870
Quoted 6 lines
drivers/pci/controller/pcie-mediatek-gen3.c +6
-0+ err =phy_init (pcie->phy); +if (err) { +dev_err (dev,"failed to initialize PHY\n" ); +return err; + } +mdelay (30 );Source? Cite the spec that requires this delay and add a #define if possible.
They are undocumented in the vendor sdk, I will try some info for them (and even for the ones below).
Regards,
Lorenzo
Quoted 31 lines
Quoted 4 lines
drivers/pci/controller/pcie-mediatek-gen3.c +3
-0+ err =phy_power_on (pcie->phy); +if (err) { +dev_err (dev,"failed to power on PHY\n" );drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie) · line 884
Quoted 10 lines
drivers/pci/controller/pcie-mediatek-gen3.c +10
-0+ goto err_phy_on; + } + + err =reset_control_bulk_deassert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets); +if (err) { +dev_err (dev,"failed to deassert PHYs\n" ); + goto err_phy_deassert; + } +usleep_range (5000 ,10000 );Source?
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie) · line 897
Quoted 12 lines
drivers/pci/controller/pcie-mediatek-gen3.c +12
-0+pm_runtime_enable (dev); +pm_runtime_get_sync (dev); + + err =clk_bulk_prepare (pcie->num_clks, pcie->clks); +if (err) { +dev_err (dev,"failed to prepare clock\n" ); + goto err_clk_prepare; + } + +writel_relaxed (0x41474147 , pcie->base +PCIE_EQ_PRESET_01_REF ); +writel_relaxed (0x1018020f , pcie->base +PCIE_PIPE4_PIE8_REG ); +mdelay (10 );Source?
AngeloGioacchino Del Regno
maintainerRe: [PATCH 4/4] PCI: mediatek-gen3: Add Airoha EN7581 support
In reply to Lorenzo BianconiIl 21/06/24 16:48, Lorenzo Bianconi ha scritto:
Quoted 71 lines
Introduce support for Airoha EN7581 pcie controller to mediatek-gen3 pcie controller driver.
Tested-by Zhengping Zhang <[email protected]>Signed-off-by Lorenzo Bianconi <[email protected]>--- drivers/pci/controller/Kconfig | 2 +- drivers/pci/controller/pcie-mediatek-gen3.c | 84 ++++++++++++++++++++- 2 files changed, 84 insertions(+), 2 deletions(-)drivers/pci/controller/Kconfig +1
-1@@ -196,7 +196,7 @@ config PCIE_MEDIATEK config PCIE_MEDIATEK_GEN3 tristate "MediaTek Gen3 PCIe controller" - depends on ARCH_MEDIATEK || COMPILE_TEST + depends on ARCH_AIROHA || ARCH_MEDIATEK || COMPILE_TEST depends on PCI_MSI help Adds support for PCIe Gen3 MAC controller for MediaTek SoCs.drivers/pci/controller/pcie-mediatek-gen3.c +6
-1@@ -7,6 +7,7 @@*/ #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/delay.h> #include <linux/iopoll.h> #include <linux/irq.h> @@ -21,6 +22,8 @@#include <linux/pm_domain.h> #include <linux/pm_runtime.h> #include <linux/reset.h> +#include <linux/of_pci.h> +#include <linux/of_device.h> #include "../pci.h" @@ -29,6 +32,7 @@#define PCI_CLASS (class) (class << 8)#define PCIE_RC_MODE BIT(0) +#define PCIE_EQ_PRESET_01_REF 0x100#define PCIE_CFGNUM_REG 0x140#define PCIE_CFG_DEVFN (devfn) ((devfn) & GENMASK(7, 0))#define PCIE_CFG_BUS (bus) (((bus) << 8) & GENMASK(15, 8))@@ -68,6 +72,7 @@#define PCIE_MSI_SET_ENABLE_REG 0x190#define PCIE_MSI_SET_ENABLE GENMASK(PCIE_MSI_SET_NUM - 1, 0) +#define PCIE_PIPE4_PIE8_REG 0x338#define PCIE_MSI_SET_BASE_REG 0xc00#define PCIE_MSI_SET_OFFSET 0x10#define PCIE_MSI_SET_STATUS_OFFSET 0x04@@ -100,7 +105,7 @@#define PCIE_ATR_TLP_TYPE_MEM PCIE_ATR_TLP_TYPE(0)#define PCIE_ATR_TLP_TYPE_IO PCIE_ATR_TLP_TYPE(2) -#define MAX_NUM_PHY_RSTS 1 +#define MAX_NUM_PHY_RSTS 3struct mtk_gen3_pcie ;@@ -848,6 +853,72 @@ static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie)return 0 ;
drivers/pci/controller/pcie-mediatek-gen3.c static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie) · line 863
Quoted 10 lines
drivers/pci/controller/pcie-mediatek-gen3.c +8
-0} +static int mtk_pcie_en7581_power_up (struct mtk_gen3_pcie *pcie) +{ +struct device *dev = pcie->dev; +int err; + +writel_relaxed (0x23020133 , pcie->base +0x10044 ); +writel_relaxed (0x50500032 , pcie->base +0x15030 ); +writel_relaxed (0x50500032 , pcie->base +0x15130 );
Can anyone from MediaTek/Airoha help to identify those registers and the layout so that we can avoid using magic numbers around?
Please.
Regards,
Angelo
Quoted 81 lines
drivers/pci/controller/pcie-mediatek-gen3.c +69
-0+ + err =phy_init (pcie->phy); +if (err) { +dev_err (dev,"failed to initialize PHY\n" ); +return err; + } +mdelay (30 ); + + err =phy_power_on (pcie->phy); +if (err) { +dev_err (dev,"failed to power on PHY\n" ); + goto err_phy_on; + } + + err =reset_control_bulk_deassert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets); +if (err) { +dev_err (dev,"failed to deassert PHYs\n" ); + goto err_phy_deassert; + } +usleep_range (5000 ,10000 ); + +pm_runtime_enable (dev); +pm_runtime_get_sync (dev); + + err =clk_bulk_prepare (pcie->num_clks, pcie->clks); +if (err) { +dev_err (dev,"failed to prepare clock\n" ); + goto err_clk_prepare; + } + +writel_relaxed (0x41474147 , pcie->base +PCIE_EQ_PRESET_01_REF ); +writel_relaxed (0x1018020f , pcie->base +PCIE_PIPE4_PIE8_REG ); +mdelay (10 ); + + err =clk_bulk_enable (pcie->num_clks, pcie->clks); +if (err) { +dev_err (dev,"failed to prepare clock\n" ); + goto err_clk_enable; + } + +return 0 ; + +err_clk_enable: +clk_bulk_unprepare (pcie->num_clks, pcie->clks); +err_clk_prepare: +pm_runtime_put_sync (dev); +pm_runtime_disable (dev); +reset_control_bulk_assert (pcie->soc->phy_resets.num_rsts, + pcie->phy_resets); +err_phy_deassert: +phy_power_off (pcie->phy); +err_phy_on: +phy_exit (pcie->phy); + +return err; +} +static int mtk_pcie_power_up (struct mtk_gen3_pcie *pcie) {struct device *dev = pcie->dev;@@ -1117,8 +1188,19 @@ static const struct mtk_pcie_soc mtk_pcie_soc_mt8192 = { }, }; +static const struct mtk_pcie_soc mtk_pcie_soc_en7581 = { + .power_up = mtk_pcie_en7581_power_up, + .phy_resets = { + .id[0 ] ="phy-lane0" , + .id[1 ] ="phy-lane1" , + .id[2 ] ="phy-lane2" , + .num_rsts =3 , + }, +}; +static const struct of_device_id mtk_pcie_of_match[] = { { .compatible ="mediatek,mt8192-pcie" , .data = &mtk_pcie_soc_mt8192 }, + { .compatible ="airoha,en7581-pcie" , .data = &mtk_pcie_soc_en7581 }, {}, };MODULE_DEVICE_TABLE (of, mtk_pcie_of_match);