add the Amlogic Meson PCIe phy driver
v2 cover add the Amlogic Meson PCIe phy driver
This patcheset add the driver and dt-bindings for
the Meson-PCIE-PHY controller.
Changes since v1: [0]
- move 'apb' and 'port' reset to ctrl driver
- format correcting
[0] : https://lkml.org/lkml/2018/8/14/66
Yue Wang (2):
dt-bindings: phy: add DT bindings for Amlogic Meson PCIe Phy
controller
PCI: meson: add the Amlogic Meson PCIe phy driver
.../bindings/phy/amlogic,meson-pcie-phy.txt | 21 ++++ drivers/phy/amlogic/Kconfig | 8 ++ drivers/phy/amlogic/Makefile | 1 + drivers/phy/amlogic/phy-meson-axg-pcie.c | 124 +++++++++++++++++++++ 4 files changed, 154 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/amlogic,meson-pcie-phy.txt create mode 100644 drivers/phy/amlogic/phy-meson-axg-pcie.c
Signature · 1 line
2.7.4
PATCH v2 1/2 dt-bindings: phy: add DT bindings for Amlogic Meson PCIe Phy controller
From: Yue Wang <[email protected]>
The Meson-PCIE-PHY controller supports the 5-Gbps data rate of the PCI Express Gen 2 specification and is backward compatible with the 2.5-Gbps Gen 1.1 specification with only inferred idle detection supported on Amlogic SoCs.
--- .../bindings/phy/amlogic,meson-pcie-phy.txt | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/amlogic,meson-pcie-phy.txt
Documentation/devicetree/bindings/phy/amlogic,meson-pcie-phy.txt new file +21 -0
@@ -0,0 +1,21 @@ +* Amlogic Meson AXG PCIE PHY binding + +Required properties: +- compatible: Should be + - "amlogic,axg-pcie-phy" +- #phys-cells: must be 0 (see phy-bindings.txt in this directory) +- reg: The base address and length of the registers +- resets: phandle to the reset lines +- reset-names: must contain "phy" and "peripheral" + - "phy" PHY reset +Optional properties: +- phy-supply: see phy-bindings.txt in this directory + +Example: + pcie_phy: pcie-phy at ff644000 { + #phy-cells = <0>; + compatible = "amlogic,axg-pcie-phy"; + reg = <0x0 0xff644000 0x0 0x2000>; + resets = <&reset RESET_PCIE_PHY>; + reset-names = "phy"; + };
Signature · 1 line
2.7.4
Rob Herring
maintainerPATCH v2 1/2 dt-bindings: phy: add DT bindings for Amlogic Meson PCIe Phy controller
Quoted 7 lines · Hanjie Lin
From: Yue Wang <[email protected]>
The Meson-PCIE-PHY controller supports the 5-Gbps data rate of the PCI Express Gen 2 specification and is backward compatible with the 2.5-Gbps Gen 1.1 specification with only inferred idle detection supported on Amlogic SoCs.
Quoted 7 lines
Signed-off-by Hanjie Lin <[email protected]>Signed-off-by Yue Wang <[email protected]>--- .../bindings/phy/amlogic,meson-pcie-phy.txt | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/amlogic,meson-pcie-phy.txt
PATCH v2 1/2 dt-bindings: phy: add DT bindings for Amlogic Meson PCIe Phy controller
Quoted 16 lines · Rob Herring
Quoted 7 lines · Hanjie Lin
From: Yue Wang <[email protected]>
The Meson-PCIE-PHY controller supports the 5-Gbps data rate of the PCI Express Gen 2 specification and is backward compatible with the 2.5-Gbps Gen 1.1 specification with only inferred idle detection supported on Amlogic SoCs.
Quoted 7 lines
Signed-off-by Hanjie Lin <[email protected]>Signed-off-by Yue Wang <[email protected]>--- .../bindings/phy/amlogic,meson-pcie-phy.txt | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/amlogic,meson-pcie-phy.txt
Quoted 4 lines
Reviewed-by Rob Herring <[email protected]>.
Thanks for the review.
As described during the discussion [0], we consider it's too overkill to have a dedicated phy driver which only process reset line. So we will abandon phy driver and integrate phy reset into the controller driver int the next version.
[0] https://lkml.kernel.org/r/1535096165-45827-1-git-send-email-hanjie.lin at amlogic.
PATCH v2 2/2 PCI: meson: add the Amlogic Meson PCIe phy driver
In reply to Hanjie LinFrom: Yue Wang <[email protected]>
The Meson-PCIE-PHY controller supports the 5-Gbps data rate of the PCI Express Gen 2 specification and is backwardcompatible with the 2.5-Gbps Gen 1.1 specification with only inferred idle detection supported on AMLOGIC SoCs.
--- drivers/phy/amlogic/Kconfig | 8 ++ drivers/phy/amlogic/Makefile | 1 + drivers/phy/amlogic/phy-meson-axg-pcie.c | 124 +++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+) create mode 100644 drivers/phy/amlogic/phy-meson-axg-pcie.c
drivers/phy/amlogic/Kconfig +8 -0
@@ -36,3 +36,11 @@ config PHY_MESON_GXL_USB3 Enable this to support the Meson USB3 PHY and OTG detection IP block found in Meson GXL and GXM SoCs. If unsure, say N. + +config PHY_MESON_AXG_PCIE + bool "Meson AXG PCIe PHY driver" + depends on OF && (ARCH_MESON || COMPILE_TEST) + select GENERIC_PHY + help + Enable PCIe PHY support for Meson AXG SoC series. + This driver provides PHY interface for Meson PCIe controller.
drivers/phy/amlogic/Makefile +1 -0
@@ -1,3 +1,4 @@ obj-$(CONFIG_PHY_MESON8B_USB2) += phy-meson8b-usb2.o obj-$(CONFIG_PHY_MESON_GXL_USB2) += phy-meson-gxl-usb2.o obj-$(CONFIG_PHY_MESON_GXL_USB3) += phy-meson-gxl-usb3.o +obj-$(CONFIG_PHY_MESON_AXG_PCIE) += phy-meson-axg-pcie.o
drivers/phy/amlogic/phy-meson-axg-pcie.c new file +124 -0
124 lines not on this page · raw message
Signature · 1 line
2.7.4
Jerome Brunet
reviewerPATCH v2 2/2 PCI: meson: add the Amlogic Meson PCIe phy driver
Quoted 15 lines · Hanjie Lin
drivers/phy/amlogic/phy-meson-axg-pcie.c new file +15
-0+static int meson_pcie_phy_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct meson_pcie_phy *mphy; +struct meson_pcie_reset *mrst; +struct phy *generic_phy; +struct phy_provider *phy_provider; +struct resource *res; +const struct meson_pcie_phy_data *data; + + data =of_device_get_match_data (dev); +if (!data) +return -ENODEV ; + + mphy =devm_kzalloc (dev,sizeof (*mphy),GFP_KERNEL );
drivers/phy/amlogic/phy-meson-axg-pcie.c line 90
Quoted 10 lines
drivers/phy/amlogic/phy-meson-axg-pcie.c new file +10
-0+if (!mphy) +return -ENOMEM ; + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + mphy->phy_base =devm_ioremap_resource (dev, res); +if (IS_ERR (mphy->phy_base)) +return PTR_ERR (mphy->phy_base); + + mrst = &mphy->reset; + + mrst->phy =devm_reset_control_get_shared (dev,"phy" );
I thought you said there was only phy on this platform. If that's the case, what is this reset shared with ?
drivers/phy/amlogic/phy-meson-axg-pcie.c line 98
Quoted 8 lines
drivers/phy/amlogic/phy-meson-axg-pcie.c new file +8
-0+if (IS_ERR (mrst->phy)) { +if (PTR_ERR (mrst->phy) != -EPROBE_DEFER ) +dev_err (dev,"couldn't get phy reset\n" ); + +return PTR_ERR (mrst->phy); + } + +reset_control_deassert (mrst->phy);
Is it really necessary before init() is called by the consumer ?
Quoted 24 lines
drivers/phy/amlogic/phy-meson-axg-pcie.c new file +24
-0+ + mphy->data = data; + + generic_phy =devm_phy_create (dev, dev->of_node, mphy->data->ops); +if (IS_ERR (generic_phy)) { +if (PTR_ERR (generic_phy) != -EPROBE_DEFER ) +dev_err (dev,"failed to create PHY\n" ); + +return PTR_ERR (generic_phy); + } + +phy_set_drvdata (generic_phy, mphy); + phy_provider =devm_of_phy_provider_register (dev, of_phy_simple_xlate); + +return PTR_ERR_OR_ZERO (phy_provider); +} + +static struct platform_driver meson_pcie_phy_driver = { + .probe = meson_pcie_phy_probe, + .driver = { + .of_match_table = meson_pcie_phy_match, + .name ="meson-pcie-phy" , + } +};
PATCH v2 2/2 PCI: meson: add the Amlogic Meson PCIe phy driver
Quoted 21 lines · Jerome Brunet
Quoted 20 lines · Hanjie Lin
drivers/phy/amlogic/phy-meson-axg-pcie.c new file +20
-0+static int meson_pcie_phy_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct meson_pcie_phy *mphy; +struct meson_pcie_reset *mrst; +struct phy *generic_phy; +struct phy_provider *phy_provider; +struct resource *res; +const struct meson_pcie_phy_data *data; + + data =of_device_get_match_data (dev); +if (!data) +return -ENODEV ; + + mphy =devm_kzalloc (dev,sizeof (*mphy),GFP_KERNEL ); +if (!mphy) +return -ENOMEM ; + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + mphy->phy_base =devm_ioremap_resource (dev, res); +if (IS_ERR (mphy->phy_base))
Quoted 8 lines
drivers/phy/amlogic/phy-meson-axg-pcie.c line 90
Quoted 5 lines
drivers/phy/amlogic/phy-meson-axg-pcie.c new file +5
-0+return PTR_ERR (mphy->phy_base); + + mrst = &mphy->reset; + + mrst->phy =devm_reset_control_get_shared (dev,"phy" );I thought you said there was only phy on this platform. If that's the case, what is this reset shared with ?
Amlogic axg soc includes two pcie controllers and they share the same pcie phy. Because of two pcie controllers, meson_pcie_phy_probe() will be called two times. So, the phy reset must be shared.
Quoted 12 lines
drivers/phy/amlogic/phy-meson-axg-pcie.c line 98
Quoted 9 lines
drivers/phy/amlogic/phy-meson-axg-pcie.c new file +8
-0+if (IS_ERR (mrst->phy)) { +if (PTR_ERR (mrst->phy) != -EPROBE_DEFER ) +dev_err (dev,"couldn't get phy reset\n" ); + +return PTR_ERR (mrst->phy); + } + +reset_control_deassert (mrst->phy);Is it really necessary before
init()is called by the consumer ?
For shared reset we must deassert first before do assert, currently if we don't do this deassert, we will get the below warning when controller driver do the share reset.
int reset_control_assert(struct reset_control *rstc) {
...
if (rstc->shared) {
if (WARN_ON(atomic_read(&rstc->deassert_count) == 0))Quoted 19 lines
Quoted 19 lines
drivers/phy/amlogic/phy-meson-axg-pcie.c new file +19
-0+ + mphy->data = data; + + generic_phy =devm_phy_create (dev, dev->of_node, mphy->data->ops); +if (IS_ERR (generic_phy)) { +if (PTR_ERR (generic_phy) != -EPROBE_DEFER ) +dev_err (dev,"failed to create PHY\n" ); + +return PTR_ERR (generic_phy); + } + +phy_set_drvdata (generic_phy, mphy); + phy_provider =devm_of_phy_provider_register (dev, of_phy_simple_xlate); + +return PTR_ERR_OR_ZERO (phy_provider); +} + +static struct platform_driver meson_pcie_phy_driver = { + .probe = meson_pcie_phy_probe,
Quoted 9 lines
drivers/phy/amlogic/phy-meson-axg-pcie.c line 122
Quoted 5 lines
drivers/phy/amlogic/phy-meson-axg-pcie.c new file +5
-0+ .driver = { + .of_match_table = meson_pcie_phy_match, + .name = "meson-pcie-phy", + } +};.
Thanks for the review.
As described during the discussion [0], we consider it's too overkill to have a dedicated phy driver which only process reset line. So we will abandon phy driver and integrate phy reset into the controller driver int the next version.
[0] https://lkml.kernel.org/r/1535096165-45827-1-git-send-email-hanjie.lin at amlogic.
Jerome Brunet
reviewerPATCH v2 2/2 PCI: meson: add the Amlogic Meson PCIe phy driver
Quoted 6 lines · Hanjie Lin
Quoted 2 lines
I thought you said there was only phy on this platform. If that's the case, what is this reset shared with ?
Amlogic axg soc includes two pcie controllers and they share the same pcie phy. Because of two pcie controllers,
meson_pcie_phy_probe()will be called two times. So, the phy reset must be shared.
You are abusing the API then.
The phy should have exclusive control of its *own* reset line, not other device should claim this reset.
I should then manage the fact that it may have more than one consumer.