linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] arm64: dts: meson-g12: add support for PCIe
@ 2019-09-08 13:42 Neil Armstrong
  2019-09-08 13:42 ` [PATCH 1/6] dt-bindings: pci: amlogic,meson-pcie: Add G12A bindings Neil Armstrong
                   ` (5 more replies)
  0 siblings, 6 replies; 24+ messages in thread
From: Neil Armstrong @ 2019-09-08 13:42 UTC (permalink / raw)
  To: khilman, bhelgaas, lorenzo.pieralisi, yue.wang, kishon
  Cc: Neil Armstrong, maz, linux-kernel, repk, linux-pci,
	linux-amlogic, linux-arm-kernel

This patchset :
- updates the Amlogic PCI bindings for G12A
- reworks the Amlogic PCIe driver to make use of the
G12a USB3+PCIe Combo PHY instead of directly writing in
the PHY register
- adds the necessary operations to the G12a USB3+PCIe Combo PHY driver
- adds the PCIe Node for G12A, G12B and SM1 SoCs
- adds the commented support for the S922X, A311D and S905D3 based
VIM3 boards.

This patchset has been tested in a A311D VIM3 using a 128Go
TS128GMTE110S NVMe PCIe module.

For indication, here is a bonnie++ run as ext4 formatted:
     ------Sequential Output------ --Sequential Input- --Random-
     -Per Chr- --Block-- -Rewrite- -Per Chr- --Block-- --Seeks--
Size K/sec %CP K/sec %CP K/sec %CP K/sec %CP K/sec %CP /sec %CP
  4G 93865  99 312837  96 194487  23 102808  97 415501 21 +++++ +++

Neil Armstrong (6):
  dt-bindings: pci: amlogic,meson-pcie: Add G12A bindings
  PCI: amlogic: Fix probed clock names
  PCI: amlogic: meson: Add support for G12A
  phy: meson-g12a-usb3-pcie: Add support for PCIe mode
  arm64: dts: meson-g12a: Add PCIe node
  arm64: dts: khadas-vim3: add commented support for PCIe

 .../bindings/pci/amlogic,meson-pcie.txt       |  12 +-
 .../boot/dts/amlogic/meson-g12-common.dtsi    |  33 ++++++
 .../amlogic/meson-g12b-a311d-khadas-vim3.dts  |  22 ++++
 .../amlogic/meson-g12b-s922x-khadas-vim3.dts  |  22 ++++
 .../boot/dts/amlogic/meson-khadas-vim3.dtsi   |   4 +
 .../dts/amlogic/meson-sm1-khadas-vim3l.dts    |  22 ++++
 drivers/pci/controller/dwc/pci-meson.c        | 105 ++++++++++++++----
 .../phy/amlogic/phy-meson-g12a-usb3-pcie.c    |  70 ++++++++++--
 8 files changed, 258 insertions(+), 32 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/6] dt-bindings: pci: amlogic,meson-pcie: Add G12A bindings
  2019-09-08 13:42 [PATCH 0/6] arm64: dts: meson-g12: add support for PCIe Neil Armstrong
@ 2019-09-08 13:42 ` Neil Armstrong
  2019-09-11 12:22   ` Andrew Murray
  2019-09-13 14:36   ` [PATCH 1/6] dt-bindings: pci: amlogic, meson-pcie: " Rob Herring
  2019-09-08 13:42 ` [PATCH 2/6] PCI: amlogic: Fix probed clock names Neil Armstrong
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 24+ messages in thread
From: Neil Armstrong @ 2019-09-08 13:42 UTC (permalink / raw)
  To: khilman, bhelgaas, lorenzo.pieralisi, yue.wang, kishon, devicetree
  Cc: Neil Armstrong, maz, linux-kernel, repk, linux-pci,
	linux-amlogic, linux-arm-kernel

Add PCIE bindings for the Amlogic G12A SoC, the support is the same
but the PHY is shared with USB3 to control the differential lines.

Thus this adds a phy phandle to control the PHY, and sets invalid
MIPI clock as optional for G12A.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../devicetree/bindings/pci/amlogic,meson-pcie.txt   | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt
index efa2c8b9b85a..84fdc422792e 100644
--- a/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt
@@ -9,13 +9,16 @@ Additional properties are described here:
 
 Required properties:
 - compatible:
-	should contain "amlogic,axg-pcie" to identify the core.
+	should contain :
+	- "amlogic,axg-pcie" for AXG SoC Family
+	- "amlogic,g12a-pcie" for G12A SoC Family
+	to identify the core.
 - reg:
 	should contain the configuration address space.
 - reg-names: Must be
 	- "elbi"	External local bus interface registers
 	- "cfg"		Meson specific registers
-	- "phy"		Meson PCIE PHY registers
+	- "phy"		Meson PCIE PHY registers for AXG SoC Family
 	- "config"	PCIe configuration space
 - reset-gpios: The GPIO to generate PCIe PERST# assert and deassert signal.
 - clocks: Must contain an entry for each entry in clock-names.
@@ -23,12 +26,13 @@ Required properties:
 	- "pclk"       PCIe GEN 100M PLL clock
 	- "port"       PCIe_x(A or B) RC clock gate
 	- "general"    PCIe Phy clock
-	- "mipi"       PCIe_x(A or B) 100M ref clock gate
+	- "mipi"       PCIe_x(A or B) 100M ref clock gate for AXG SoC Family
 - resets: phandle to the reset lines.
 - reset-names: must contain "phy" "port" and "apb"
-       - "phy"         Share PHY reset
+       - "phy"         Share PHY reset for AXG SoC Family
        - "port"        Port A or B reset
        - "apb"         Share APB reset
+- phys: should contain a phandle to the shared phy for G12A SoC Family
 - device_type:
 	should be "pci". As specified in designware-pcie.txt
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/6] PCI: amlogic: Fix probed clock names
  2019-09-08 13:42 [PATCH 0/6] arm64: dts: meson-g12: add support for PCIe Neil Armstrong
  2019-09-08 13:42 ` [PATCH 1/6] dt-bindings: pci: amlogic,meson-pcie: Add G12A bindings Neil Armstrong
@ 2019-09-08 13:42 ` Neil Armstrong
  2019-09-11 10:59   ` Andrew Murray
  2019-09-08 13:42 ` [PATCH 3/6] PCI: amlogic: meson: Add support for G12A Neil Armstrong
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 24+ messages in thread
From: Neil Armstrong @ 2019-09-08 13:42 UTC (permalink / raw)
  To: khilman, bhelgaas, lorenzo.pieralisi, yue.wang, kishon
  Cc: Neil Armstrong, maz, linux-kernel, repk, linux-pci,
	linux-amlogic, linux-arm-kernel

Fix the clock names used in the probe function according
to the bindings.

Fixes: 9c0ef6d34fdb ("PCI: amlogic: Add the Amlogic Meson PCIe controller driver")
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/pci/controller/dwc/pci-meson.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
index 541f37a6f6a5..ab79990798f8 100644
--- a/drivers/pci/controller/dwc/pci-meson.c
+++ b/drivers/pci/controller/dwc/pci-meson.c
@@ -250,15 +250,15 @@ static int meson_pcie_probe_clocks(struct meson_pcie *mp)
 	if (IS_ERR(res->port_clk))
 		return PTR_ERR(res->port_clk);
 
-	res->mipi_gate = meson_pcie_probe_clock(dev, "pcie_mipi_en", 0);
+	res->mipi_gate = meson_pcie_probe_clock(dev, "mipi", 0);
 	if (IS_ERR(res->mipi_gate))
 		return PTR_ERR(res->mipi_gate);
 
-	res->general_clk = meson_pcie_probe_clock(dev, "pcie_general", 0);
+	res->general_clk = meson_pcie_probe_clock(dev, "general", 0);
 	if (IS_ERR(res->general_clk))
 		return PTR_ERR(res->general_clk);
 
-	res->clk = meson_pcie_probe_clock(dev, "pcie", 0);
+	res->clk = meson_pcie_probe_clock(dev, "pclk", 0);
 	if (IS_ERR(res->clk))
 		return PTR_ERR(res->clk);
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 3/6] PCI: amlogic: meson: Add support for G12A
  2019-09-08 13:42 [PATCH 0/6] arm64: dts: meson-g12: add support for PCIe Neil Armstrong
  2019-09-08 13:42 ` [PATCH 1/6] dt-bindings: pci: amlogic,meson-pcie: Add G12A bindings Neil Armstrong
  2019-09-08 13:42 ` [PATCH 2/6] PCI: amlogic: Fix probed clock names Neil Armstrong
@ 2019-09-08 13:42 ` Neil Armstrong
  2019-09-11 11:36   ` Andrew Murray
  2019-09-08 13:42 ` [PATCH 4/6] phy: meson-g12a-usb3-pcie: Add support for PCIe mode Neil Armstrong
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 24+ messages in thread
From: Neil Armstrong @ 2019-09-08 13:42 UTC (permalink / raw)
  To: khilman, bhelgaas, lorenzo.pieralisi, yue.wang, kishon
  Cc: Neil Armstrong, maz, linux-kernel, repk, linux-pci,
	linux-amlogic, linux-arm-kernel

Add support for the Amlogic G12A SoC using a separate shared PHY.

This adds support for fetching a PHY phandle and call the PHY init,
reset and power on/off calls instead of writing in the PHY register or
toggling the PHY reset line.

The MIPI clock is also made optional since it is used for setting up
the PHY reference clock chared with the DSI controller on AXG.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/pci/controller/dwc/pci-meson.c | 101 ++++++++++++++++++++-----
 1 file changed, 84 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
index ab79990798f8..3fadad381762 100644
--- a/drivers/pci/controller/dwc/pci-meson.c
+++ b/drivers/pci/controller/dwc/pci-meson.c
@@ -16,6 +16,7 @@
 #include <linux/reset.h>
 #include <linux/resource.h>
 #include <linux/types.h>
+#include <linux/phy/phy.h>
 
 #include "pcie-designware.h"
 
@@ -96,12 +97,18 @@ struct meson_pcie_rc_reset {
 	struct reset_control *apb;
 };
 
+struct meson_pcie_param {
+	bool has_shared_phy;
+};
+
 struct meson_pcie {
 	struct dw_pcie pci;
 	struct meson_pcie_mem_res mem_res;
 	struct meson_pcie_clk_res clk_res;
 	struct meson_pcie_rc_reset mrst;
 	struct gpio_desc *reset_gpio;
+	struct phy *phy;
+	const struct meson_pcie_param *param;
 };
 
 static struct reset_control *meson_pcie_get_reset(struct meson_pcie *mp,
@@ -123,10 +130,12 @@ static int meson_pcie_get_resets(struct meson_pcie *mp)
 {
 	struct meson_pcie_rc_reset *mrst = &mp->mrst;
 
-	mrst->phy = meson_pcie_get_reset(mp, "phy", PCIE_SHARED_RESET);
-	if (IS_ERR(mrst->phy))
-		return PTR_ERR(mrst->phy);
-	reset_control_deassert(mrst->phy);
+	if (!mp->param->has_shared_phy) {
+		mrst->phy = meson_pcie_get_reset(mp, "phy", PCIE_SHARED_RESET);
+		if (IS_ERR(mrst->phy))
+			return PTR_ERR(mrst->phy);
+		reset_control_deassert(mrst->phy);
+	}
 
 	mrst->port = meson_pcie_get_reset(mp, "port", PCIE_NORMAL_RESET);
 	if (IS_ERR(mrst->port))
@@ -180,6 +189,9 @@ static int meson_pcie_get_mems(struct platform_device *pdev,
 	if (IS_ERR(mp->mem_res.cfg_base))
 		return PTR_ERR(mp->mem_res.cfg_base);
 
+	if (mp->param->has_shared_phy)
+		return 0;
+
 	/* Meson SoC has two PCI controllers use same phy register*/
 	mp->mem_res.phy_base = meson_pcie_get_mem_shared(pdev, mp, "phy");
 	if (IS_ERR(mp->mem_res.phy_base))
@@ -188,19 +200,33 @@ static int meson_pcie_get_mems(struct platform_device *pdev,
 	return 0;
 }
 
-static void meson_pcie_power_on(struct meson_pcie *mp)
+static int meson_pcie_power_on(struct meson_pcie *mp)
 {
-	writel(MESON_PCIE_PHY_POWERUP, mp->mem_res.phy_base);
+	int ret = 0;
+
+	if (mp->param->has_shared_phy)
+		ret = phy_power_on(mp->phy);
+	else
+		writel(MESON_PCIE_PHY_POWERUP, mp->mem_res.phy_base);
+
+	return ret;
 }
 
-static void meson_pcie_reset(struct meson_pcie *mp)
+static int meson_pcie_reset(struct meson_pcie *mp)
 {
 	struct meson_pcie_rc_reset *mrst = &mp->mrst;
-
-	reset_control_assert(mrst->phy);
-	udelay(PCIE_RESET_DELAY);
-	reset_control_deassert(mrst->phy);
-	udelay(PCIE_RESET_DELAY);
+	int ret = 0;
+
+	if (mp->param->has_shared_phy) {
+		ret = phy_reset(mp->phy);
+		if (ret)
+			return ret;
+	} else {
+		reset_control_assert(mrst->phy);
+		udelay(PCIE_RESET_DELAY);
+		reset_control_deassert(mrst->phy);
+		udelay(PCIE_RESET_DELAY);
+	}
 
 	reset_control_assert(mrst->port);
 	reset_control_assert(mrst->apb);
@@ -208,6 +234,8 @@ static void meson_pcie_reset(struct meson_pcie *mp)
 	reset_control_deassert(mrst->port);
 	reset_control_deassert(mrst->apb);
 	udelay(PCIE_RESET_DELAY);
+
+	return 0;
 }
 
 static inline struct clk *meson_pcie_probe_clock(struct device *dev,
@@ -250,9 +278,11 @@ static int meson_pcie_probe_clocks(struct meson_pcie *mp)
 	if (IS_ERR(res->port_clk))
 		return PTR_ERR(res->port_clk);
 
-	res->mipi_gate = meson_pcie_probe_clock(dev, "mipi", 0);
-	if (IS_ERR(res->mipi_gate))
-		return PTR_ERR(res->mipi_gate);
+	if (!mp->param->has_shared_phy) {
+		res->mipi_gate = meson_pcie_probe_clock(dev, "mipi", 0);
+		if (IS_ERR(res->mipi_gate))
+			return PTR_ERR(res->mipi_gate);
+	}
 
 	res->general_clk = meson_pcie_probe_clock(dev, "general", 0);
 	if (IS_ERR(res->general_clk))
@@ -524,6 +554,7 @@ static const struct dw_pcie_ops dw_pcie_ops = {
 
 static int meson_pcie_probe(struct platform_device *pdev)
 {
+	const struct meson_pcie_param *match_data;
 	struct device *dev = &pdev->dev;
 	struct dw_pcie *pci;
 	struct meson_pcie *mp;
@@ -537,6 +568,20 @@ static int meson_pcie_probe(struct platform_device *pdev)
 	pci->dev = dev;
 	pci->ops = &dw_pcie_ops;
 
+	match_data = of_device_get_match_data(dev);
+	if (!match_data) {
+		dev_err(dev, "failed to get match data\n");
+		return -ENODEV;
+	}
+	mp->param = match_data;
+
+	if (mp->param->has_shared_phy) {
+		mp->phy = devm_phy_get(dev, "pcie");
+		if (IS_ERR(mp->phy)) {
+			return PTR_ERR(mp->phy);
+		}
+	}
+
 	mp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
 	if (IS_ERR(mp->reset_gpio)) {
 		dev_err(dev, "get reset gpio failed\n");
@@ -555,8 +600,17 @@ static int meson_pcie_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	meson_pcie_power_on(mp);
-	meson_pcie_reset(mp);
+	ret = meson_pcie_power_on(mp);
+	if (ret) {
+		dev_err(dev, "phy power on failed, %d\n", ret);
+		return ret;
+	}
+
+	ret = meson_pcie_reset(mp);
+	if (ret) {
+		dev_err(dev, "reset failed, %d\n", ret);
+		return ret;
+	}
 
 	ret = meson_pcie_probe_clocks(mp);
 	if (ret) {
@@ -575,9 +629,22 @@ static int meson_pcie_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static struct meson_pcie_param meson_pcie_axg_param = {
+	.has_shared_phy = false,
+};
+
+static struct meson_pcie_param meson_pcie_g12a_param = {
+	.has_shared_phy = true,
+};
+
 static const struct of_device_id meson_pcie_of_match[] = {
 	{
 		.compatible = "amlogic,axg-pcie",
+		.data = &meson_pcie_axg_param,
+	},
+	{
+		.compatible = "amlogic,g12a-pcie",
+		.data = &meson_pcie_g12a_param,
 	},
 	{},
 };
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 4/6] phy: meson-g12a-usb3-pcie: Add support for PCIe mode
  2019-09-08 13:42 [PATCH 0/6] arm64: dts: meson-g12: add support for PCIe Neil Armstrong
                   ` (2 preceding siblings ...)
  2019-09-08 13:42 ` [PATCH 3/6] PCI: amlogic: meson: Add support for G12A Neil Armstrong
@ 2019-09-08 13:42 ` Neil Armstrong
  2019-09-11 12:19   ` Andrew Murray
  2019-09-08 13:42 ` [PATCH 5/6] arm64: dts: meson-g12a: Add PCIe node Neil Armstrong
  2019-09-08 13:42 ` [PATCH 6/6] arm64: dts: khadas-vim3: add commented support for PCIe Neil Armstrong
  5 siblings, 1 reply; 24+ messages in thread
From: Neil Armstrong @ 2019-09-08 13:42 UTC (permalink / raw)
  To: khilman, bhelgaas, lorenzo.pieralisi, yue.wang, kishon
  Cc: Neil Armstrong, maz, linux-kernel, repk, linux-pci,
	linux-amlogic, linux-arm-kernel

This adds extended PCIe PHY functions for the Amlogic G12A
USB3+PCIE Combo PHY to support reset, power_on and power_off for
PCIe exclusively.

With these callbacks, we can handle all the needed operations of the
Amlogic PCIe controller driver.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../phy/amlogic/phy-meson-g12a-usb3-pcie.c    | 70 ++++++++++++++++---
 1 file changed, 61 insertions(+), 9 deletions(-)

diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c b/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
index ac322d643c7a..08e322789e59 100644
--- a/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
+++ b/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
@@ -50,6 +50,8 @@
 	#define PHY_R5_PHY_CR_ACK				BIT(16)
 	#define PHY_R5_PHY_BS_OUT				BIT(17)
 
+#define PCIE_RESET_DELAY					500
+
 struct phy_g12a_usb3_pcie_priv {
 	struct regmap		*regmap;
 	struct regmap		*regmap_cr;
@@ -196,6 +198,10 @@ static int phy_g12a_usb3_init(struct phy *phy)
 	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
 	int data, ret;
 
+	ret = reset_control_reset(priv->reset);
+	if (ret)
+		return ret;
+
 	/* Switch PHY to USB3 */
 	/* TODO figure out how to handle when PCIe was set in the bootloader */
 	regmap_update_bits(priv->regmap, PHY_R0,
@@ -272,24 +278,64 @@ static int phy_g12a_usb3_init(struct phy *phy)
 	return 0;
 }
 
-static int phy_g12a_usb3_pcie_init(struct phy *phy)
+static int phy_g12a_usb3_pcie_power_on(struct phy *phy)
+{
+	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
+
+	if (priv->mode == PHY_TYPE_USB3)
+		return 0;
+
+	regmap_update_bits(priv->regmap, PHY_R0,
+			   PHY_R0_PCIE_POWER_STATE,
+			   FIELD_PREP(PHY_R0_PCIE_POWER_STATE, 0x1c));
+
+	return 0;
+}
+
+static int phy_g12a_usb3_pcie_power_off(struct phy *phy)
+{
+	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
+
+	if (priv->mode == PHY_TYPE_USB3)
+		return 0;
+
+	regmap_update_bits(priv->regmap, PHY_R0,
+			   PHY_R0_PCIE_POWER_STATE,
+			   FIELD_PREP(PHY_R0_PCIE_POWER_STATE, 0x1d));
+
+	return 0;
+}
+
+static int phy_g12a_usb3_pcie_reset(struct phy *phy)
 {
 	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
 	int ret;
 
-	ret = reset_control_reset(priv->reset);
+	if (priv->mode == PHY_TYPE_USB3)
+		return 0;
+
+	ret = reset_control_assert(priv->reset);
 	if (ret)
 		return ret;
 
+	udelay(PCIE_RESET_DELAY);
+
+	ret = reset_control_deassert(priv->reset);
+	if (ret)
+		return ret;
+
+	udelay(PCIE_RESET_DELAY);
+
+	return 0;
+}
+
+static int phy_g12a_usb3_pcie_init(struct phy *phy)
+{
+	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
+
 	if (priv->mode == PHY_TYPE_USB3)
 		return phy_g12a_usb3_init(phy);
 
-	/* Power UP PCIE */
-	/* TODO figure out when the bootloader has set USB3 mode before */
-	regmap_update_bits(priv->regmap, PHY_R0,
-			   PHY_R0_PCIE_POWER_STATE,
-			   FIELD_PREP(PHY_R0_PCIE_POWER_STATE, 0x1c));
-
 	return 0;
 }
 
@@ -297,7 +343,10 @@ static int phy_g12a_usb3_pcie_exit(struct phy *phy)
 {
 	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
 
-	return reset_control_reset(priv->reset);
+	if (priv->mode == PHY_TYPE_USB3)
+		return reset_control_reset(priv->reset);
+
+	return 0;
 }
 
 static struct phy *phy_g12a_usb3_pcie_xlate(struct device *dev,
@@ -326,6 +375,9 @@ static struct phy *phy_g12a_usb3_pcie_xlate(struct device *dev,
 static const struct phy_ops phy_g12a_usb3_pcie_ops = {
 	.init		= phy_g12a_usb3_pcie_init,
 	.exit		= phy_g12a_usb3_pcie_exit,
+	.power_on	= phy_g12a_usb3_pcie_power_on,
+	.power_off	= phy_g12a_usb3_pcie_power_off,
+	.reset		= phy_g12a_usb3_pcie_reset,
 	.owner		= THIS_MODULE,
 };
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 5/6] arm64: dts: meson-g12a: Add PCIe node
  2019-09-08 13:42 [PATCH 0/6] arm64: dts: meson-g12: add support for PCIe Neil Armstrong
                   ` (3 preceding siblings ...)
  2019-09-08 13:42 ` [PATCH 4/6] phy: meson-g12a-usb3-pcie: Add support for PCIe mode Neil Armstrong
@ 2019-09-08 13:42 ` Neil Armstrong
  2019-09-08 13:42 ` [PATCH 6/6] arm64: dts: khadas-vim3: add commented support for PCIe Neil Armstrong
  5 siblings, 0 replies; 24+ messages in thread
From: Neil Armstrong @ 2019-09-08 13:42 UTC (permalink / raw)
  To: khilman, bhelgaas, lorenzo.pieralisi, yue.wang, kishon
  Cc: Neil Armstrong, maz, linux-kernel, repk, linux-pci,
	linux-amlogic, linux-arm-kernel

This adds the Amlogic G12A PCI Express controller node, also
using the USB3+PCIe Combo PHY.

The PHY mode selection is static, thus the USB3+PCIe Combo PHY
phandle would need to be removed from the USB control node if the
shared differentil lines are used for PCIe instead of USB3.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../boot/dts/amlogic/meson-g12-common.dtsi    | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
index 852cf9cf121b..7330dc37b7a6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
@@ -95,6 +95,39 @@
 		#size-cells = <2>;
 		ranges;
 
+		pcie: pcie@fc000000 {
+			compatible = "amlogic,g12a-pcie", "snps,dw-pcie";
+			reg = <0x0 0xfc000000 0x0 0x400000
+			       0x0 0xff648000 0x0 0x2000
+			       0x0 0xfc400000 0x0 0x200000>;
+			reg-names = "elbi", "cfg", "config";
+			interrupts = <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
+			#interrupt-cells = <1>;
+			interrupt-map-mask = <0 0 0 0>;
+			interrupt-map = <0 0 0 0 &gic GIC_SPI 223 IRQ_TYPE_LEVEL_HIGH>;
+			bus-range = <0x0 0xff>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+			device_type = "pci";
+			ranges = <0x81000000 0 0 0x0 0xfc600000 0 0x00100000
+				  0x82000000 0 0xfc700000 0x0 0xfc700000 0 0x1900000>;
+
+			clocks = <&clkc CLKID_PCIE_PHY
+				  &clkc CLKID_PCIE_COMB
+				  &clkc CLKID_PCIE_PLL>;
+			clock-names = "general",
+				      "pclk",
+				      "port";
+			resets = <&reset RESET_PCIE_CTRL_A>,
+				 <&reset RESET_PCIE_APB>;
+			reset-names = "port",
+				      "apb";
+			num-lanes = <1>;
+			phys = <&usb3_pcie_phy PHY_TYPE_PCIE>;
+			phy-names = "pcie";
+			status = "disabled";
+		};
+
 		ethmac: ethernet@ff3f0000 {
 			compatible = "amlogic,meson-axg-dwmac",
 				     "snps,dwmac-3.70a",
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 6/6] arm64: dts: khadas-vim3: add commented support for PCIe
  2019-09-08 13:42 [PATCH 0/6] arm64: dts: meson-g12: add support for PCIe Neil Armstrong
                   ` (4 preceding siblings ...)
  2019-09-08 13:42 ` [PATCH 5/6] arm64: dts: meson-g12a: Add PCIe node Neil Armstrong
@ 2019-09-08 13:42 ` Neil Armstrong
  2019-09-09 16:37   ` Marc Zyngier
  2019-09-11 12:50   ` Andrew Murray
  5 siblings, 2 replies; 24+ messages in thread
From: Neil Armstrong @ 2019-09-08 13:42 UTC (permalink / raw)
  To: khilman, bhelgaas, lorenzo.pieralisi, yue.wang, kishon
  Cc: Neil Armstrong, maz, linux-kernel, repk, linux-pci,
	linux-amlogic, linux-arm-kernel

The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
an USB3.0 Type A connector and a M.2 Key M slot.
The PHY driving these differential lines is shared between
the USB3.0 controller and the PCIe Controller, thus only
a single controller can use it.

The needed DT configuration when the MCU is configured to mux
the PCIe/USB3.0 differential lines to the M.2 Key M slot is
added commented and may uncommented to disable USB3.0 from the
USB Complex and enable the PCIe controller.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../amlogic/meson-g12b-a311d-khadas-vim3.dts  | 22 +++++++++++++++++++
 .../amlogic/meson-g12b-s922x-khadas-vim3.dts  | 22 +++++++++++++++++++
 .../boot/dts/amlogic/meson-khadas-vim3.dtsi   |  4 ++++
 .../dts/amlogic/meson-sm1-khadas-vim3l.dts    | 22 +++++++++++++++++++
 4 files changed, 70 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
index 3a6a1e0c1e32..0577b1435cbb 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
@@ -14,3 +14,25 @@
 / {
 	compatible = "khadas,vim3", "amlogic,a311d", "amlogic,g12b";
 };
+
+/*
+ * The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
+ * lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
+ * an USB3.0 Type A connector and a M.2 Key M slot.
+ * The PHY driving these differential lines is shared between
+ * the USB3.0 controller and the PCIe Controller, thus only
+ * a single controller can use it.
+ * If the MCU is configured to mux the PCIe/USB3.0 differential lines
+ * to the M.2 Key M slot, uncomment the following block to disable
+ * USB3.0 from the USB Complex and enable the PCIe controller.
+ */
+/*
+&pcie {
+	status = "okay";
+};
+
+&usb {
+	phys = <&usb2_phy0>, <&usb2_phy1>;
+	phy-names = "usb2-phy0", "usb2-phy1";
+};
+ */
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-s922x-khadas-vim3.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-s922x-khadas-vim3.dts
index b73deb282120..1ef5c2f04f67 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-s922x-khadas-vim3.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-s922x-khadas-vim3.dts
@@ -14,3 +14,25 @@
 / {
 	compatible = "khadas,vim3", "amlogic,s922x", "amlogic,g12b";
 };
+
+/*
+ * The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
+ * lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
+ * an USB3.0 Type A connector and a M.2 Key M slot.
+ * The PHY driving these differential lines is shared between
+ * the USB3.0 controller and the PCIe Controller, thus only
+ * a single controller can use it.
+ * If the MCU is configured to mux the PCIe/USB3.0 differential lines
+ * to the M.2 Key M slot, uncomment the following block to disable
+ * USB3.0 from the USB Complex and enable the PCIe controller.
+ */
+/*
+&pcie {
+	status = "okay";
+};
+
+&usb {
+	phys = <&usb2_phy0>, <&usb2_phy1>;
+	phy-names = "usb2-phy0", "usb2-phy1";
+};
+ */
diff --git a/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi b/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
index 8647da7d6609..eac5720dc15f 100644
--- a/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
@@ -246,6 +246,10 @@
 	linux,rc-map-name = "rc-khadas";
 };
 
+&pcie {
+	reset-gpios = <&gpio GPIOA_8 GPIO_ACTIVE_LOW>;
+};
+
 &pwm_ef {
         status = "okay";
         pinctrl-0 = <&pwm_e_pins>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
index 5233bd7cacfb..d9c7cbedce53 100644
--- a/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
@@ -68,3 +68,25 @@
 	clock-names = "clkin1";
 	status = "okay";
 };
+
+/*
+ * The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
+ * lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
+ * an USB3.0 Type A connector and a M.2 Key M slot.
+ * The PHY driving these differential lines is shared between
+ * the USB3.0 controller and the PCIe Controller, thus only
+ * a single controller can use it.
+ * If the MCU is configured to mux the PCIe/USB3.0 differential lines
+ * to the M.2 Key M slot, uncomment the following block to disable
+ * USB3.0 from the USB Complex and enable the PCIe controller.
+ */
+/*
+&pcie {
+	status = "okay";
+};
+
+&usb {
+	phys = <&usb2_phy0>, <&usb2_phy1>;
+	phy-names = "usb2-phy0", "usb2-phy1";
+};
+ */
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 6/6] arm64: dts: khadas-vim3: add commented support for PCIe
  2019-09-08 13:42 ` [PATCH 6/6] arm64: dts: khadas-vim3: add commented support for PCIe Neil Armstrong
@ 2019-09-09 16:37   ` Marc Zyngier
  2019-09-09 17:50     ` Neil Armstrong
  2019-09-11 12:50   ` Andrew Murray
  1 sibling, 1 reply; 24+ messages in thread
From: Marc Zyngier @ 2019-09-09 16:37 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: lorenzo.pieralisi, khilman, linux-kernel, kishon, repk,
	linux-pci, bhelgaas, linux-amlogic, yue.wang, linux-arm-kernel

On Sun, 08 Sep 2019 14:42:58 +0100,
Neil Armstrong <narmstrong@baylibre.com> wrote:
> 
> The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
> lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
> an USB3.0 Type A connector and a M.2 Key M slot.
> The PHY driving these differential lines is shared between
> the USB3.0 controller and the PCIe Controller, thus only
> a single controller can use it.
> 
> The needed DT configuration when the MCU is configured to mux
> the PCIe/USB3.0 differential lines to the M.2 Key M slot is
> added commented and may uncommented to disable USB3.0 from the
> USB Complex and enable the PCIe controller.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  .../amlogic/meson-g12b-a311d-khadas-vim3.dts  | 22 +++++++++++++++++++
>  .../amlogic/meson-g12b-s922x-khadas-vim3.dts  | 22 +++++++++++++++++++
>  .../boot/dts/amlogic/meson-khadas-vim3.dtsi   |  4 ++++
>  .../dts/amlogic/meson-sm1-khadas-vim3l.dts    | 22 +++++++++++++++++++
>  4 files changed, 70 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
> index 3a6a1e0c1e32..0577b1435cbb 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
> @@ -14,3 +14,25 @@
>  / {
>  	compatible = "khadas,vim3", "amlogic,a311d", "amlogic,g12b";
>  };
> +
> +/*
> + * The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
> + * lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
> + * an USB3.0 Type A connector and a M.2 Key M slot.
> + * The PHY driving these differential lines is shared between
> + * the USB3.0 controller and the PCIe Controller, thus only
> + * a single controller can use it.
> + * If the MCU is configured to mux the PCIe/USB3.0 differential lines
> + * to the M.2 Key M slot, uncomment the following block to disable
> + * USB3.0 from the USB Complex and enable the PCIe controller.
> + */
> +/*
> +&pcie {
> +	status = "okay";
> +};
> +
> +&usb {
> +	phys = <&usb2_phy0>, <&usb2_phy1>;
> +	phy-names = "usb2-phy0", "usb2-phy1";
> +};
> + */

Although you can't do much more than this here, I'd expect firmware on
the machine to provide the DT that matches its configuration. Is it
the way it actually works? Or is the user actually expected to edit
this file?

Thanks,

	M.

-- 
Jazz is not dead, it just smells funny.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 6/6] arm64: dts: khadas-vim3: add commented support for PCIe
  2019-09-09 16:37   ` Marc Zyngier
@ 2019-09-09 17:50     ` Neil Armstrong
  2019-09-10  9:12       ` Marc Zyngier
  0 siblings, 1 reply; 24+ messages in thread
From: Neil Armstrong @ 2019-09-09 17:50 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: lorenzo.pieralisi, khilman, linux-kernel, kishon, repk,
	linux-pci, bhelgaas, linux-amlogic, yue.wang, linux-arm-kernel

Hi Marc,

Le 09/09/2019 à 18:37, Marc Zyngier a écrit :
> On Sun, 08 Sep 2019 14:42:58 +0100,
> Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
>> lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
>> an USB3.0 Type A connector and a M.2 Key M slot.
>> The PHY driving these differential lines is shared between
>> the USB3.0 controller and the PCIe Controller, thus only
>> a single controller can use it.
>>
>> The needed DT configuration when the MCU is configured to mux
>> the PCIe/USB3.0 differential lines to the M.2 Key M slot is
>> added commented and may uncommented to disable USB3.0 from the
>> USB Complex and enable the PCIe controller.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  .../amlogic/meson-g12b-a311d-khadas-vim3.dts  | 22 +++++++++++++++++++
>>  .../amlogic/meson-g12b-s922x-khadas-vim3.dts  | 22 +++++++++++++++++++
>>  .../boot/dts/amlogic/meson-khadas-vim3.dtsi   |  4 ++++
>>  .../dts/amlogic/meson-sm1-khadas-vim3l.dts    | 22 +++++++++++++++++++
>>  4 files changed, 70 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
>> index 3a6a1e0c1e32..0577b1435cbb 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
>> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
>> @@ -14,3 +14,25 @@
>>  / {
>>  	compatible = "khadas,vim3", "amlogic,a311d", "amlogic,g12b";
>>  };
>> +
>> +/*
>> + * The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
>> + * lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
>> + * an USB3.0 Type A connector and a M.2 Key M slot.
>> + * The PHY driving these differential lines is shared between
>> + * the USB3.0 controller and the PCIe Controller, thus only
>> + * a single controller can use it.
>> + * If the MCU is configured to mux the PCIe/USB3.0 differential lines
>> + * to the M.2 Key M slot, uncomment the following block to disable
>> + * USB3.0 from the USB Complex and enable the PCIe controller.
>> + */
>> +/*
>> +&pcie {
>> +	status = "okay";
>> +};
>> +
>> +&usb {
>> +	phys = <&usb2_phy0>, <&usb2_phy1>;
>> +	phy-names = "usb2-phy0", "usb2-phy1";
>> +};
>> + */
> 
> Although you can't do much more than this here, I'd expect firmware on
> the machine to provide the DT that matches its configuration. Is it
> the way it actually works? Or is the user actually expected to edit
> this file?

It's the plan when initial VIM3 support will be merged in u-boot mainline,
and the MCU driver will be added aswell :
https://patchwork.ozlabs.org/cover/1156618/
A custom board support altering the DT will be added when this patchset
is merged upstream.

But since these are separate projects, leaving this as commented is ugly,
but necessary.

Thanks,
Neil

> 
> Thanks,
> 
> 	M.
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 6/6] arm64: dts: khadas-vim3: add commented support for PCIe
  2019-09-09 17:50     ` Neil Armstrong
@ 2019-09-10  9:12       ` Marc Zyngier
  2019-09-10  9:14         ` Neil Armstrong
  0 siblings, 1 reply; 24+ messages in thread
From: Marc Zyngier @ 2019-09-10  9:12 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: lorenzo.pieralisi, khilman, linux-kernel, kishon, repk,
	linux-pci, bhelgaas, linux-amlogic, yue.wang, linux-arm-kernel

On Mon, 09 Sep 2019 18:50:42 +0100,
Neil Armstrong <narmstrong@baylibre.com> wrote:
> 
> Hi Marc,
> 
> Le 09/09/2019 à 18:37, Marc Zyngier a écrit :
> > On Sun, 08 Sep 2019 14:42:58 +0100,
> > Neil Armstrong <narmstrong@baylibre.com> wrote:
> >>
> >> The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
> >> lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
> >> an USB3.0 Type A connector and a M.2 Key M slot.
> >> The PHY driving these differential lines is shared between
> >> the USB3.0 controller and the PCIe Controller, thus only
> >> a single controller can use it.
> >>
> >> The needed DT configuration when the MCU is configured to mux
> >> the PCIe/USB3.0 differential lines to the M.2 Key M slot is
> >> added commented and may uncommented to disable USB3.0 from the
> >> USB Complex and enable the PCIe controller.
> >>
> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >> ---
> >>  .../amlogic/meson-g12b-a311d-khadas-vim3.dts  | 22 +++++++++++++++++++
> >>  .../amlogic/meson-g12b-s922x-khadas-vim3.dts  | 22 +++++++++++++++++++
> >>  .../boot/dts/amlogic/meson-khadas-vim3.dtsi   |  4 ++++
> >>  .../dts/amlogic/meson-sm1-khadas-vim3l.dts    | 22 +++++++++++++++++++
> >>  4 files changed, 70 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
> >> index 3a6a1e0c1e32..0577b1435cbb 100644
> >> --- a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
> >> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
> >> @@ -14,3 +14,25 @@
> >>  / {
> >>  	compatible = "khadas,vim3", "amlogic,a311d", "amlogic,g12b";
> >>  };
> >> +
> >> +/*
> >> + * The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
> >> + * lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
> >> + * an USB3.0 Type A connector and a M.2 Key M slot.
> >> + * The PHY driving these differential lines is shared between
> >> + * the USB3.0 controller and the PCIe Controller, thus only
> >> + * a single controller can use it.
> >> + * If the MCU is configured to mux the PCIe/USB3.0 differential lines
> >> + * to the M.2 Key M slot, uncomment the following block to disable
> >> + * USB3.0 from the USB Complex and enable the PCIe controller.
> >> + */
> >> +/*
> >> +&pcie {
> >> +	status = "okay";
> >> +};
> >> +
> >> +&usb {
> >> +	phys = <&usb2_phy0>, <&usb2_phy1>;
> >> +	phy-names = "usb2-phy0", "usb2-phy1";
> >> +};
> >> + */
> > 
> > Although you can't do much more than this here, I'd expect firmware on
> > the machine to provide the DT that matches its configuration. Is it
> > the way it actually works? Or is the user actually expected to edit
> > this file?
> 
> It's the plan when initial VIM3 support will be merged in u-boot mainline,
> and the MCU driver will be added aswell :
> https://patchwork.ozlabs.org/cover/1156618/
> A custom board support altering the DT will be added when this patchset
> is merged upstream.
> 
> But since these are separate projects, leaving this as commented is ugly,
> but necessary.

I agree with the unfortunate necessity. However, could you please have
a comment here, indicating that the user isn't expected to change this
on their own, but instead rely on the firmware/bootloader to do it
accordingly?

Thanks,

	M.

-- 
Jazz is not dead, it just smells funny.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 6/6] arm64: dts: khadas-vim3: add commented support for PCIe
  2019-09-10  9:12       ` Marc Zyngier
@ 2019-09-10  9:14         ` Neil Armstrong
  0 siblings, 0 replies; 24+ messages in thread
From: Neil Armstrong @ 2019-09-10  9:14 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: lorenzo.pieralisi, khilman, linux-kernel, kishon, repk,
	linux-pci, bhelgaas, linux-amlogic, yue.wang, linux-arm-kernel

On 10/09/2019 11:12, Marc Zyngier wrote:
> On Mon, 09 Sep 2019 18:50:42 +0100,
> Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> Hi Marc,
>>
>> Le 09/09/2019 à 18:37, Marc Zyngier a écrit :
>>> On Sun, 08 Sep 2019 14:42:58 +0100,
>>> Neil Armstrong <narmstrong@baylibre.com> wrote:
>>>>
>>>> The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
>>>> lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
>>>> an USB3.0 Type A connector and a M.2 Key M slot.
>>>> The PHY driving these differential lines is shared between
>>>> the USB3.0 controller and the PCIe Controller, thus only
>>>> a single controller can use it.
>>>>
>>>> The needed DT configuration when the MCU is configured to mux
>>>> the PCIe/USB3.0 differential lines to the M.2 Key M slot is
>>>> added commented and may uncommented to disable USB3.0 from the
>>>> USB Complex and enable the PCIe controller.
>>>>
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>> ---
>>>>  .../amlogic/meson-g12b-a311d-khadas-vim3.dts  | 22 +++++++++++++++++++
>>>>  .../amlogic/meson-g12b-s922x-khadas-vim3.dts  | 22 +++++++++++++++++++
>>>>  .../boot/dts/amlogic/meson-khadas-vim3.dtsi   |  4 ++++
>>>>  .../dts/amlogic/meson-sm1-khadas-vim3l.dts    | 22 +++++++++++++++++++
>>>>  4 files changed, 70 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
>>>> index 3a6a1e0c1e32..0577b1435cbb 100644
>>>> --- a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
>>>> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
>>>> @@ -14,3 +14,25 @@
>>>>  / {
>>>>  	compatible = "khadas,vim3", "amlogic,a311d", "amlogic,g12b";
>>>>  };
>>>> +
>>>> +/*
>>>> + * The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
>>>> + * lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
>>>> + * an USB3.0 Type A connector and a M.2 Key M slot.
>>>> + * The PHY driving these differential lines is shared between
>>>> + * the USB3.0 controller and the PCIe Controller, thus only
>>>> + * a single controller can use it.
>>>> + * If the MCU is configured to mux the PCIe/USB3.0 differential lines
>>>> + * to the M.2 Key M slot, uncomment the following block to disable
>>>> + * USB3.0 from the USB Complex and enable the PCIe controller.
>>>> + */
>>>> +/*
>>>> +&pcie {
>>>> +	status = "okay";
>>>> +};
>>>> +
>>>> +&usb {
>>>> +	phys = <&usb2_phy0>, <&usb2_phy1>;
>>>> +	phy-names = "usb2-phy0", "usb2-phy1";
>>>> +};
>>>> + */
>>>
>>> Although you can't do much more than this here, I'd expect firmware on
>>> the machine to provide the DT that matches its configuration. Is it
>>> the way it actually works? Or is the user actually expected to edit
>>> this file?
>>
>> It's the plan when initial VIM3 support will be merged in u-boot mainline,
>> and the MCU driver will be added aswell :
>> https://patchwork.ozlabs.org/cover/1156618/
>> A custom board support altering the DT will be added when this patchset
>> is merged upstream.
>>
>> But since these are separate projects, leaving this as commented is ugly,
>> but necessary.
> 
> I agree with the unfortunate necessity. However, could you please have
> a comment here, indicating that the user isn't expected to change this
> on their own, but instead rely on the firmware/bootloader to do it
> accordingly?

Yes, sure.

Neil

> 
> Thanks,
> 
> 	M.
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/6] PCI: amlogic: Fix probed clock names
  2019-09-08 13:42 ` [PATCH 2/6] PCI: amlogic: Fix probed clock names Neil Armstrong
@ 2019-09-11 10:59   ` Andrew Murray
  0 siblings, 0 replies; 24+ messages in thread
From: Andrew Murray @ 2019-09-11 10:59 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: lorenzo.pieralisi, khilman, linux-pci, linux-kernel, kishon,
	repk, maz, bhelgaas, linux-amlogic, yue.wang, linux-arm-kernel

On Sun, Sep 08, 2019 at 01:42:54PM +0000, Neil Armstrong wrote:
> Fix the clock names used in the probe function according
> to the bindings.
> 
> Fixes: 9c0ef6d34fdb ("PCI: amlogic: Add the Amlogic Meson PCIe controller driver")
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

Reviewed-by: Andrew Murray <andrew.murray@arm.com>

> ---
>  drivers/pci/controller/dwc/pci-meson.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> index 541f37a6f6a5..ab79990798f8 100644
> --- a/drivers/pci/controller/dwc/pci-meson.c
> +++ b/drivers/pci/controller/dwc/pci-meson.c
> @@ -250,15 +250,15 @@ static int meson_pcie_probe_clocks(struct meson_pcie *mp)
>  	if (IS_ERR(res->port_clk))
>  		return PTR_ERR(res->port_clk);
>  
> -	res->mipi_gate = meson_pcie_probe_clock(dev, "pcie_mipi_en", 0);
> +	res->mipi_gate = meson_pcie_probe_clock(dev, "mipi", 0);
>  	if (IS_ERR(res->mipi_gate))
>  		return PTR_ERR(res->mipi_gate);
>  
> -	res->general_clk = meson_pcie_probe_clock(dev, "pcie_general", 0);
> +	res->general_clk = meson_pcie_probe_clock(dev, "general", 0);
>  	if (IS_ERR(res->general_clk))
>  		return PTR_ERR(res->general_clk);
>  
> -	res->clk = meson_pcie_probe_clock(dev, "pcie", 0);
> +	res->clk = meson_pcie_probe_clock(dev, "pclk", 0);
>  	if (IS_ERR(res->clk))
>  		return PTR_ERR(res->clk);
>  
> -- 
> 2.17.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/6] PCI: amlogic: meson: Add support for G12A
  2019-09-08 13:42 ` [PATCH 3/6] PCI: amlogic: meson: Add support for G12A Neil Armstrong
@ 2019-09-11 11:36   ` Andrew Murray
  2019-09-11 12:39     ` Neil Armstrong
  0 siblings, 1 reply; 24+ messages in thread
From: Andrew Murray @ 2019-09-11 11:36 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: lorenzo.pieralisi, khilman, linux-pci, linux-kernel, kishon,
	repk, maz, bhelgaas, linux-amlogic, yue.wang, linux-arm-kernel

On Sun, Sep 08, 2019 at 01:42:55PM +0000, Neil Armstrong wrote:
> Add support for the Amlogic G12A SoC using a separate shared PHY.
> 
> This adds support for fetching a PHY phandle and call the PHY init,
> reset and power on/off calls instead of writing in the PHY register or
> toggling the PHY reset line.
> 
> The MIPI clock is also made optional since it is used for setting up

Is it worth indicating here that the MIPI clock is *only required* for
the G12A (or controllers with a shared phy)? It's still required for
AXG. It's not optional for G12A - it's ignored.

> the PHY reference clock chared with the DSI controller on AXG.

s/chared/shared/

> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/pci/controller/dwc/pci-meson.c | 101 ++++++++++++++++++++-----
>  1 file changed, 84 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> index ab79990798f8..3fadad381762 100644
> --- a/drivers/pci/controller/dwc/pci-meson.c
> +++ b/drivers/pci/controller/dwc/pci-meson.c
> @@ -16,6 +16,7 @@
>  #include <linux/reset.h>
>  #include <linux/resource.h>
>  #include <linux/types.h>
> +#include <linux/phy/phy.h>
>  
>  #include "pcie-designware.h"
>  
> @@ -96,12 +97,18 @@ struct meson_pcie_rc_reset {
>  	struct reset_control *apb;
>  };
>  
> +struct meson_pcie_param {
> +	bool has_shared_phy;
> +};
> +
>  struct meson_pcie {
>  	struct dw_pcie pci;
>  	struct meson_pcie_mem_res mem_res;
>  	struct meson_pcie_clk_res clk_res;
>  	struct meson_pcie_rc_reset mrst;
>  	struct gpio_desc *reset_gpio;
> +	struct phy *phy;
> +	const struct meson_pcie_param *param;
>  };
>  
>  static struct reset_control *meson_pcie_get_reset(struct meson_pcie *mp,
> @@ -123,10 +130,12 @@ static int meson_pcie_get_resets(struct meson_pcie *mp)
>  {
>  	struct meson_pcie_rc_reset *mrst = &mp->mrst;
>  
> -	mrst->phy = meson_pcie_get_reset(mp, "phy", PCIE_SHARED_RESET);
> -	if (IS_ERR(mrst->phy))
> -		return PTR_ERR(mrst->phy);
> -	reset_control_deassert(mrst->phy);
> +	if (!mp->param->has_shared_phy) {
> +		mrst->phy = meson_pcie_get_reset(mp, "phy", PCIE_SHARED_RESET);
> +		if (IS_ERR(mrst->phy))
> +			return PTR_ERR(mrst->phy);
> +		reset_control_deassert(mrst->phy);
> +	}
>  
>  	mrst->port = meson_pcie_get_reset(mp, "port", PCIE_NORMAL_RESET);
>  	if (IS_ERR(mrst->port))
> @@ -180,6 +189,9 @@ static int meson_pcie_get_mems(struct platform_device *pdev,
>  	if (IS_ERR(mp->mem_res.cfg_base))
>  		return PTR_ERR(mp->mem_res.cfg_base);
>  
> +	if (mp->param->has_shared_phy)
> +		return 0;
> +

It may be more consistent if, rather than returning here, you wrapped
the following 3 lines by the if statement.

>  	/* Meson SoC has two PCI controllers use same phy register*/

I guess this comment should now be updated to refer to AXG?

>  	mp->mem_res.phy_base = meson_pcie_get_mem_shared(pdev, mp, "phy");
>  	if (IS_ERR(mp->mem_res.phy_base))
> @@ -188,19 +200,33 @@ static int meson_pcie_get_mems(struct platform_device *pdev,
>  	return 0;
>  }
>  
> -static void meson_pcie_power_on(struct meson_pcie *mp)
> +static int meson_pcie_power_on(struct meson_pcie *mp)
>  {
> -	writel(MESON_PCIE_PHY_POWERUP, mp->mem_res.phy_base);
> +	int ret = 0;
> +
> +	if (mp->param->has_shared_phy)
> +		ret = phy_power_on(mp->phy);

I haven't seen any phy_[init/exit] calls, should there be any?

> +	else
> +		writel(MESON_PCIE_PHY_POWERUP, mp->mem_res.phy_base);
> +
> +	return ret;
>  }
>  
> -static void meson_pcie_reset(struct meson_pcie *mp)
> +static int meson_pcie_reset(struct meson_pcie *mp)
>  {
>  	struct meson_pcie_rc_reset *mrst = &mp->mrst;
> -
> -	reset_control_assert(mrst->phy);
> -	udelay(PCIE_RESET_DELAY);
> -	reset_control_deassert(mrst->phy);
> -	udelay(PCIE_RESET_DELAY);
> +	int ret = 0;
> +
> +	if (mp->param->has_shared_phy) {
> +		ret = phy_reset(mp->phy);
> +		if (ret)
> +			return ret;
> +	} else {
> +		reset_control_assert(mrst->phy);
> +		udelay(PCIE_RESET_DELAY);
> +		reset_control_deassert(mrst->phy);
> +		udelay(PCIE_RESET_DELAY);
> +	}
>  
>  	reset_control_assert(mrst->port);
>  	reset_control_assert(mrst->apb);
> @@ -208,6 +234,8 @@ static void meson_pcie_reset(struct meson_pcie *mp)
>  	reset_control_deassert(mrst->port);
>  	reset_control_deassert(mrst->apb);
>  	udelay(PCIE_RESET_DELAY);
> +
> +	return 0;
>  }
>  
>  static inline struct clk *meson_pcie_probe_clock(struct device *dev,
> @@ -250,9 +278,11 @@ static int meson_pcie_probe_clocks(struct meson_pcie *mp)
>  	if (IS_ERR(res->port_clk))
>  		return PTR_ERR(res->port_clk);
>  
> -	res->mipi_gate = meson_pcie_probe_clock(dev, "mipi", 0);
> -	if (IS_ERR(res->mipi_gate))
> -		return PTR_ERR(res->mipi_gate);
> +	if (!mp->param->has_shared_phy) {
> +		res->mipi_gate = meson_pcie_probe_clock(dev, "mipi", 0);
> +		if (IS_ERR(res->mipi_gate))
> +			return PTR_ERR(res->mipi_gate);
> +	}
>  
>  	res->general_clk = meson_pcie_probe_clock(dev, "general", 0);
>  	if (IS_ERR(res->general_clk))
> @@ -524,6 +554,7 @@ static const struct dw_pcie_ops dw_pcie_ops = {
>  
>  static int meson_pcie_probe(struct platform_device *pdev)
>  {
> +	const struct meson_pcie_param *match_data;
>  	struct device *dev = &pdev->dev;
>  	struct dw_pcie *pci;
>  	struct meson_pcie *mp;
> @@ -537,6 +568,20 @@ static int meson_pcie_probe(struct platform_device *pdev)
>  	pci->dev = dev;
>  	pci->ops = &dw_pcie_ops;
>  
> +	match_data = of_device_get_match_data(dev);
> +	if (!match_data) {
> +		dev_err(dev, "failed to get match data\n");
> +		return -ENODEV;
> +	}
> +	mp->param = match_data;
> +
> +	if (mp->param->has_shared_phy) {
> +		mp->phy = devm_phy_get(dev, "pcie");
> +		if (IS_ERR(mp->phy)) {
> +			return PTR_ERR(mp->phy);
> +		}
> +	}
> +
>  	mp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
>  	if (IS_ERR(mp->reset_gpio)) {
>  		dev_err(dev, "get reset gpio failed\n");
> @@ -555,8 +600,17 @@ static int meson_pcie_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	meson_pcie_power_on(mp);
> -	meson_pcie_reset(mp);
> +	ret = meson_pcie_power_on(mp);
> +	if (ret) {
> +		dev_err(dev, "phy power on failed, %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = meson_pcie_reset(mp);
> +	if (ret) {
> +		dev_err(dev, "reset failed, %d\n", ret);
> +		return ret;
> +	}
>  
>  	ret = meson_pcie_probe_clocks(mp);
>  	if (ret) {
> @@ -575,9 +629,22 @@ static int meson_pcie_probe(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static struct meson_pcie_param meson_pcie_axg_param = {
> +	.has_shared_phy = false,
> +};
> +
> +static struct meson_pcie_param meson_pcie_g12a_param = {
> +	.has_shared_phy = true,
> +};
> +
>  static const struct of_device_id meson_pcie_of_match[] = {
>  	{
>  		.compatible = "amlogic,axg-pcie",
> +		.data = &meson_pcie_axg_param,
> +	},
> +	{
> +		.compatible = "amlogic,g12a-pcie",
> +		.data = &meson_pcie_g12a_param,

Here, we hard-code knowledge about the SOCs regarding if they have shared phys
or not. I guess the alternative would have been to assume there is a shared
phy if the DT has a phandle for it. I.e. instead of mp->param->has_shared_phy
everywhere you could test for mp->phy. Though I guess at least with the
current approach you guard against bad DTs, this seems OK.

Thanks,

Andrew Murray

>  	},
>  	{},
>  };
> -- 
> 2.17.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/6] phy: meson-g12a-usb3-pcie: Add support for PCIe mode
  2019-09-08 13:42 ` [PATCH 4/6] phy: meson-g12a-usb3-pcie: Add support for PCIe mode Neil Armstrong
@ 2019-09-11 12:19   ` Andrew Murray
  2019-09-11 12:45     ` Neil Armstrong
  0 siblings, 1 reply; 24+ messages in thread
From: Andrew Murray @ 2019-09-11 12:19 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: lorenzo.pieralisi, khilman, linux-pci, linux-kernel, kishon,
	repk, maz, bhelgaas, linux-amlogic, yue.wang, linux-arm-kernel

On Sun, Sep 08, 2019 at 01:42:56PM +0000, Neil Armstrong wrote:
> This adds extended PCIe PHY functions for the Amlogic G12A
> USB3+PCIE Combo PHY to support reset, power_on and power_off for
> PCIe exclusively.
> 
> With these callbacks, we can handle all the needed operations of the
> Amlogic PCIe controller driver.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  .../phy/amlogic/phy-meson-g12a-usb3-pcie.c    | 70 ++++++++++++++++---
>  1 file changed, 61 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c b/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
> index ac322d643c7a..08e322789e59 100644
> --- a/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
> +++ b/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
> @@ -50,6 +50,8 @@
>  	#define PHY_R5_PHY_CR_ACK				BIT(16)
>  	#define PHY_R5_PHY_BS_OUT				BIT(17)
>  
> +#define PCIE_RESET_DELAY					500
> +
>  struct phy_g12a_usb3_pcie_priv {
>  	struct regmap		*regmap;
>  	struct regmap		*regmap_cr;
> @@ -196,6 +198,10 @@ static int phy_g12a_usb3_init(struct phy *phy)
>  	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
>  	int data, ret;
>  
> +	ret = reset_control_reset(priv->reset);
> +	if (ret)
> +		return ret;
> +

Right, so we've moved this to apply to USB only, thus assuming PCI will
call .reset for its reset (why the asymmetry?).

Thanks,

Andrew Murray

>  	/* Switch PHY to USB3 */
>  	/* TODO figure out how to handle when PCIe was set in the bootloader */
>  	regmap_update_bits(priv->regmap, PHY_R0,
> @@ -272,24 +278,64 @@ static int phy_g12a_usb3_init(struct phy *phy)
>  	return 0;
>  }
>  
> -static int phy_g12a_usb3_pcie_init(struct phy *phy)
> +static int phy_g12a_usb3_pcie_power_on(struct phy *phy)
> +{
> +	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
> +
> +	if (priv->mode == PHY_TYPE_USB3)
> +		return 0;
> +
> +	regmap_update_bits(priv->regmap, PHY_R0,
> +			   PHY_R0_PCIE_POWER_STATE,
> +			   FIELD_PREP(PHY_R0_PCIE_POWER_STATE, 0x1c));
> +
> +	return 0;
> +}
> +
> +static int phy_g12a_usb3_pcie_power_off(struct phy *phy)
> +{
> +	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
> +
> +	if (priv->mode == PHY_TYPE_USB3)
> +		return 0;
> +
> +	regmap_update_bits(priv->regmap, PHY_R0,
> +			   PHY_R0_PCIE_POWER_STATE,
> +			   FIELD_PREP(PHY_R0_PCIE_POWER_STATE, 0x1d));
> +
> +	return 0;
> +}
> +
> +static int phy_g12a_usb3_pcie_reset(struct phy *phy)
>  {
>  	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
>  	int ret;
>  
> -	ret = reset_control_reset(priv->reset);
> +	if (priv->mode == PHY_TYPE_USB3)
> +		return 0;
> +
> +	ret = reset_control_assert(priv->reset);
>  	if (ret)
>  		return ret;
>  
> +	udelay(PCIE_RESET_DELAY);
> +
> +	ret = reset_control_deassert(priv->reset);
> +	if (ret)
> +		return ret;
> +
> +	udelay(PCIE_RESET_DELAY);
> +
> +	return 0;
> +}
> +
> +static int phy_g12a_usb3_pcie_init(struct phy *phy)
> +{
> +	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
> +
>  	if (priv->mode == PHY_TYPE_USB3)
>  		return phy_g12a_usb3_init(phy);
>  
> -	/* Power UP PCIE */
> -	/* TODO figure out when the bootloader has set USB3 mode before */
> -	regmap_update_bits(priv->regmap, PHY_R0,
> -			   PHY_R0_PCIE_POWER_STATE,
> -			   FIELD_PREP(PHY_R0_PCIE_POWER_STATE, 0x1c));
> -
>  	return 0;
>  }
>  
> @@ -297,7 +343,10 @@ static int phy_g12a_usb3_pcie_exit(struct phy *phy)
>  {
>  	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
>  
> -	return reset_control_reset(priv->reset);
> +	if (priv->mode == PHY_TYPE_USB3)
> +		return reset_control_reset(priv->reset);
> +
> +	return 0;
>  }
>  
>  static struct phy *phy_g12a_usb3_pcie_xlate(struct device *dev,
> @@ -326,6 +375,9 @@ static struct phy *phy_g12a_usb3_pcie_xlate(struct device *dev,
>  static const struct phy_ops phy_g12a_usb3_pcie_ops = {
>  	.init		= phy_g12a_usb3_pcie_init,
>  	.exit		= phy_g12a_usb3_pcie_exit,
> +	.power_on	= phy_g12a_usb3_pcie_power_on,
> +	.power_off	= phy_g12a_usb3_pcie_power_off,
> +	.reset		= phy_g12a_usb3_pcie_reset,
>  	.owner		= THIS_MODULE,
>  };
>  
> -- 
> 2.17.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/6] dt-bindings: pci: amlogic,meson-pcie: Add G12A bindings
  2019-09-08 13:42 ` [PATCH 1/6] dt-bindings: pci: amlogic,meson-pcie: Add G12A bindings Neil Armstrong
@ 2019-09-11 12:22   ` Andrew Murray
  2019-09-11 12:30     ` Neil Armstrong
  2019-09-13 14:36   ` [PATCH 1/6] dt-bindings: pci: amlogic, meson-pcie: " Rob Herring
  1 sibling, 1 reply; 24+ messages in thread
From: Andrew Murray @ 2019-09-11 12:22 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: devicetree, lorenzo.pieralisi, khilman, linux-pci, linux-kernel,
	kishon, repk, maz, bhelgaas, linux-amlogic, yue.wang,
	linux-arm-kernel

On Sun, Sep 08, 2019 at 01:42:53PM +0000, Neil Armstrong wrote:
> Add PCIE bindings for the Amlogic G12A SoC, the support is the same
> but the PHY is shared with USB3 to control the differential lines.
> 
> Thus this adds a phy phandle to control the PHY, and sets invalid
> MIPI clock as optional for G12A.

Perhaps reword to "Thus this adds a phy phandle to control the PHY,
and only requires a MIPI clock for AXG SoC Family".

Thanks,

Andrew Murray

> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  .../devicetree/bindings/pci/amlogic,meson-pcie.txt   | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt
> index efa2c8b9b85a..84fdc422792e 100644
> --- a/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt
> @@ -9,13 +9,16 @@ Additional properties are described here:
>  
>  Required properties:
>  - compatible:
> -	should contain "amlogic,axg-pcie" to identify the core.
> +	should contain :
> +	- "amlogic,axg-pcie" for AXG SoC Family
> +	- "amlogic,g12a-pcie" for G12A SoC Family
> +	to identify the core.
>  - reg:
>  	should contain the configuration address space.
>  - reg-names: Must be
>  	- "elbi"	External local bus interface registers
>  	- "cfg"		Meson specific registers
> -	- "phy"		Meson PCIE PHY registers
> +	- "phy"		Meson PCIE PHY registers for AXG SoC Family
>  	- "config"	PCIe configuration space
>  - reset-gpios: The GPIO to generate PCIe PERST# assert and deassert signal.
>  - clocks: Must contain an entry for each entry in clock-names.
> @@ -23,12 +26,13 @@ Required properties:
>  	- "pclk"       PCIe GEN 100M PLL clock
>  	- "port"       PCIe_x(A or B) RC clock gate
>  	- "general"    PCIe Phy clock
> -	- "mipi"       PCIe_x(A or B) 100M ref clock gate
> +	- "mipi"       PCIe_x(A or B) 100M ref clock gate for AXG SoC Family
>  - resets: phandle to the reset lines.
>  - reset-names: must contain "phy" "port" and "apb"
> -       - "phy"         Share PHY reset
> +       - "phy"         Share PHY reset for AXG SoC Family
>         - "port"        Port A or B reset
>         - "apb"         Share APB reset
> +- phys: should contain a phandle to the shared phy for G12A SoC Family
>  - device_type:
>  	should be "pci". As specified in designware-pcie.txt
>  
> -- 
> 2.17.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/6] dt-bindings: pci: amlogic,meson-pcie: Add G12A bindings
  2019-09-11 12:22   ` Andrew Murray
@ 2019-09-11 12:30     ` Neil Armstrong
  0 siblings, 0 replies; 24+ messages in thread
From: Neil Armstrong @ 2019-09-11 12:30 UTC (permalink / raw)
  To: Andrew Murray
  Cc: devicetree, lorenzo.pieralisi, khilman, linux-pci, linux-kernel,
	kishon, repk, maz, bhelgaas, linux-amlogic, yue.wang,
	linux-arm-kernel

Hi Andrew,

On 11/09/2019 14:22, Andrew Murray wrote:
> On Sun, Sep 08, 2019 at 01:42:53PM +0000, Neil Armstrong wrote:
>> Add PCIE bindings for the Amlogic G12A SoC, the support is the same
>> but the PHY is shared with USB3 to control the differential lines.
>>
>> Thus this adds a phy phandle to control the PHY, and sets invalid
>> MIPI clock as optional for G12A.
> 
> Perhaps reword to "Thus this adds a phy phandle to control the PHY,
> and only requires a MIPI clock for AXG SoC Family".

Sure, thanks,
Neil

> 
> Thanks,
> 
> Andrew Murray
> 
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  .../devicetree/bindings/pci/amlogic,meson-pcie.txt   | 12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt
>> index efa2c8b9b85a..84fdc422792e 100644
>> --- a/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt
>> @@ -9,13 +9,16 @@ Additional properties are described here:
>>  
>>  Required properties:
>>  - compatible:
>> -	should contain "amlogic,axg-pcie" to identify the core.
>> +	should contain :
>> +	- "amlogic,axg-pcie" for AXG SoC Family
>> +	- "amlogic,g12a-pcie" for G12A SoC Family
>> +	to identify the core.
>>  - reg:
>>  	should contain the configuration address space.
>>  - reg-names: Must be
>>  	- "elbi"	External local bus interface registers
>>  	- "cfg"		Meson specific registers
>> -	- "phy"		Meson PCIE PHY registers
>> +	- "phy"		Meson PCIE PHY registers for AXG SoC Family
>>  	- "config"	PCIe configuration space
>>  - reset-gpios: The GPIO to generate PCIe PERST# assert and deassert signal.
>>  - clocks: Must contain an entry for each entry in clock-names.
>> @@ -23,12 +26,13 @@ Required properties:
>>  	- "pclk"       PCIe GEN 100M PLL clock
>>  	- "port"       PCIe_x(A or B) RC clock gate
>>  	- "general"    PCIe Phy clock
>> -	- "mipi"       PCIe_x(A or B) 100M ref clock gate
>> +	- "mipi"       PCIe_x(A or B) 100M ref clock gate for AXG SoC Family
>>  - resets: phandle to the reset lines.
>>  - reset-names: must contain "phy" "port" and "apb"
>> -       - "phy"         Share PHY reset
>> +       - "phy"         Share PHY reset for AXG SoC Family
>>         - "port"        Port A or B reset
>>         - "apb"         Share APB reset
>> +- phys: should contain a phandle to the shared phy for G12A SoC Family
>>  - device_type:
>>  	should be "pci". As specified in designware-pcie.txt
>>  
>> -- 
>> 2.17.1
>>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/6] PCI: amlogic: meson: Add support for G12A
  2019-09-11 11:36   ` Andrew Murray
@ 2019-09-11 12:39     ` Neil Armstrong
  2019-09-11 12:58       ` Andrew Murray
  0 siblings, 1 reply; 24+ messages in thread
From: Neil Armstrong @ 2019-09-11 12:39 UTC (permalink / raw)
  To: Andrew Murray
  Cc: lorenzo.pieralisi, khilman, linux-pci, linux-kernel, kishon,
	repk, maz, bhelgaas, linux-amlogic, yue.wang, linux-arm-kernel

Hi Andrew,

On 11/09/2019 13:36, Andrew Murray wrote:
> On Sun, Sep 08, 2019 at 01:42:55PM +0000, Neil Armstrong wrote:
>> Add support for the Amlogic G12A SoC using a separate shared PHY.
>>
>> This adds support for fetching a PHY phandle and call the PHY init,
>> reset and power on/off calls instead of writing in the PHY register or
>> toggling the PHY reset line.
>>
>> The MIPI clock is also made optional since it is used for setting up
> 
> Is it worth indicating here that the MIPI clock is *only required* for
> the G12A (or controllers with a shared phy)? It's still required for
> AXG. It's not optional for G12A - it's ignored.

Indeed it's ignored, I'll reword it.

> 
>> the PHY reference clock chared with the DSI controller on AXG.
> 
> s/chared/shared/

Ack

> 
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  drivers/pci/controller/dwc/pci-meson.c | 101 ++++++++++++++++++++-----
>>  1 file changed, 84 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
>> index ab79990798f8..3fadad381762 100644
>> --- a/drivers/pci/controller/dwc/pci-meson.c
>> +++ b/drivers/pci/controller/dwc/pci-meson.c
>> @@ -16,6 +16,7 @@
>>  #include <linux/reset.h>
>>  #include <linux/resource.h>
>>  #include <linux/types.h>
>> +#include <linux/phy/phy.h>
>>  
>>  #include "pcie-designware.h"
>>  
>> @@ -96,12 +97,18 @@ struct meson_pcie_rc_reset {
>>  	struct reset_control *apb;
>>  };
>>  
>> +struct meson_pcie_param {
>> +	bool has_shared_phy;
>> +};
>> +
>>  struct meson_pcie {
>>  	struct dw_pcie pci;
>>  	struct meson_pcie_mem_res mem_res;
>>  	struct meson_pcie_clk_res clk_res;
>>  	struct meson_pcie_rc_reset mrst;
>>  	struct gpio_desc *reset_gpio;
>> +	struct phy *phy;
>> +	const struct meson_pcie_param *param;
>>  };
>>  
>>  static struct reset_control *meson_pcie_get_reset(struct meson_pcie *mp,
>> @@ -123,10 +130,12 @@ static int meson_pcie_get_resets(struct meson_pcie *mp)
>>  {
>>  	struct meson_pcie_rc_reset *mrst = &mp->mrst;
>>  
>> -	mrst->phy = meson_pcie_get_reset(mp, "phy", PCIE_SHARED_RESET);
>> -	if (IS_ERR(mrst->phy))
>> -		return PTR_ERR(mrst->phy);
>> -	reset_control_deassert(mrst->phy);
>> +	if (!mp->param->has_shared_phy) {
>> +		mrst->phy = meson_pcie_get_reset(mp, "phy", PCIE_SHARED_RESET);
>> +		if (IS_ERR(mrst->phy))
>> +			return PTR_ERR(mrst->phy);
>> +		reset_control_deassert(mrst->phy);
>> +	}
>>  
>>  	mrst->port = meson_pcie_get_reset(mp, "port", PCIE_NORMAL_RESET);
>>  	if (IS_ERR(mrst->port))
>> @@ -180,6 +189,9 @@ static int meson_pcie_get_mems(struct platform_device *pdev,
>>  	if (IS_ERR(mp->mem_res.cfg_base))
>>  		return PTR_ERR(mp->mem_res.cfg_base);
>>  
>> +	if (mp->param->has_shared_phy)
>> +		return 0;
>> +
> 
> It may be more consistent if, rather than returning here, you wrapped
> the following 3 lines by the if statement.

ok

> 
>>  	/* Meson SoC has two PCI controllers use same phy register*/
> 
> I guess this comment should now be updated to refer to AXG?

Indeed

> 
>>  	mp->mem_res.phy_base = meson_pcie_get_mem_shared(pdev, mp, "phy");
>>  	if (IS_ERR(mp->mem_res.phy_base))
>> @@ -188,19 +200,33 @@ static int meson_pcie_get_mems(struct platform_device *pdev,
>>  	return 0;
>>  }
>>  
>> -static void meson_pcie_power_on(struct meson_pcie *mp)
>> +static int meson_pcie_power_on(struct meson_pcie *mp)
>>  {
>> -	writel(MESON_PCIE_PHY_POWERUP, mp->mem_res.phy_base);
>> +	int ret = 0;
>> +
>> +	if (mp->param->has_shared_phy)
>> +		ret = phy_power_on(mp->phy);
> 
> I haven't seen any phy_[init/exit] calls, should there be any?

There is no _init() needed, but indeed we should still call them even it's
a no-op.

> 
>> +	else
>> +		writel(MESON_PCIE_PHY_POWERUP, mp->mem_res.phy_base);
>> +
>> +	return ret;
>>  }
>>  
>> -static void meson_pcie_reset(struct meson_pcie *mp)
>> +static int meson_pcie_reset(struct meson_pcie *mp)
>>  {
>>  	struct meson_pcie_rc_reset *mrst = &mp->mrst;
>> -
>> -	reset_control_assert(mrst->phy);
>> -	udelay(PCIE_RESET_DELAY);
>> -	reset_control_deassert(mrst->phy);
>> -	udelay(PCIE_RESET_DELAY);
>> +	int ret = 0;
>> +
>> +	if (mp->param->has_shared_phy) {
>> +		ret = phy_reset(mp->phy);
>> +		if (ret)
>> +			return ret;
>> +	} else {
>> +		reset_control_assert(mrst->phy);
>> +		udelay(PCIE_RESET_DELAY);
>> +		reset_control_deassert(mrst->phy);
>> +		udelay(PCIE_RESET_DELAY);
>> +	}
>>  
>>  	reset_control_assert(mrst->port);
>>  	reset_control_assert(mrst->apb);
>> @@ -208,6 +234,8 @@ static void meson_pcie_reset(struct meson_pcie *mp)
>>  	reset_control_deassert(mrst->port);
>>  	reset_control_deassert(mrst->apb);
>>  	udelay(PCIE_RESET_DELAY);
>> +
>> +	return 0;
>>  }
>>  
>>  static inline struct clk *meson_pcie_probe_clock(struct device *dev,
>> @@ -250,9 +278,11 @@ static int meson_pcie_probe_clocks(struct meson_pcie *mp)
>>  	if (IS_ERR(res->port_clk))
>>  		return PTR_ERR(res->port_clk);
>>  
>> -	res->mipi_gate = meson_pcie_probe_clock(dev, "mipi", 0);
>> -	if (IS_ERR(res->mipi_gate))
>> -		return PTR_ERR(res->mipi_gate);
>> +	if (!mp->param->has_shared_phy) {
>> +		res->mipi_gate = meson_pcie_probe_clock(dev, "mipi", 0);
>> +		if (IS_ERR(res->mipi_gate))
>> +			return PTR_ERR(res->mipi_gate);
>> +	}
>>  
>>  	res->general_clk = meson_pcie_probe_clock(dev, "general", 0);
>>  	if (IS_ERR(res->general_clk))
>> @@ -524,6 +554,7 @@ static const struct dw_pcie_ops dw_pcie_ops = {
>>  
>>  static int meson_pcie_probe(struct platform_device *pdev)
>>  {
>> +	const struct meson_pcie_param *match_data;
>>  	struct device *dev = &pdev->dev;
>>  	struct dw_pcie *pci;
>>  	struct meson_pcie *mp;
>> @@ -537,6 +568,20 @@ static int meson_pcie_probe(struct platform_device *pdev)
>>  	pci->dev = dev;
>>  	pci->ops = &dw_pcie_ops;
>>  
>> +	match_data = of_device_get_match_data(dev);
>> +	if (!match_data) {
>> +		dev_err(dev, "failed to get match data\n");
>> +		return -ENODEV;
>> +	}
>> +	mp->param = match_data;
>> +
>> +	if (mp->param->has_shared_phy) {
>> +		mp->phy = devm_phy_get(dev, "pcie");
>> +		if (IS_ERR(mp->phy)) {
>> +			return PTR_ERR(mp->phy);
>> +		}
>> +	}
>> +
>>  	mp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
>>  	if (IS_ERR(mp->reset_gpio)) {
>>  		dev_err(dev, "get reset gpio failed\n");
>> @@ -555,8 +600,17 @@ static int meson_pcie_probe(struct platform_device *pdev)
>>  		return ret;
>>  	}
>>  
>> -	meson_pcie_power_on(mp);
>> -	meson_pcie_reset(mp);
>> +	ret = meson_pcie_power_on(mp);
>> +	if (ret) {
>> +		dev_err(dev, "phy power on failed, %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = meson_pcie_reset(mp);
>> +	if (ret) {
>> +		dev_err(dev, "reset failed, %d\n", ret);
>> +		return ret;
>> +	}
>>  
>>  	ret = meson_pcie_probe_clocks(mp);
>>  	if (ret) {
>> @@ -575,9 +629,22 @@ static int meson_pcie_probe(struct platform_device *pdev)
>>  	return 0;
>>  }
>>  
>> +static struct meson_pcie_param meson_pcie_axg_param = {
>> +	.has_shared_phy = false,
>> +};
>> +
>> +static struct meson_pcie_param meson_pcie_g12a_param = {
>> +	.has_shared_phy = true,
>> +};
>> +
>>  static const struct of_device_id meson_pcie_of_match[] = {
>>  	{
>>  		.compatible = "amlogic,axg-pcie",
>> +		.data = &meson_pcie_axg_param,
>> +	},
>> +	{
>> +		.compatible = "amlogic,g12a-pcie",
>> +		.data = &meson_pcie_g12a_param,
> 
> Here, we hard-code knowledge about the SOCs regarding if they have shared phys
> or not. I guess the alternative would have been to assume there is a shared
> phy if the DT has a phandle for it. I.e. instead of mp->param->has_shared_phy
> everywhere you could test for mp->phy. Though I guess at least with the
> current approach you guard against bad DTs, this seems OK.

I could split with if(mp->phy) and .needs_mipi_clk, but overall it would
be the same, and I wouldn't know how to react if we forget the PHY in g12a DT
since we wouldn't have the PHY register memory zone.
On G12A, the PHY is mandatory unlike AXG.

And finally this MIPI clock is part of the PHY ref clock, so I think
it's fine to wrap it in the .has_shared_phy knowledge.

Thanks for the review,
Neil

> 
> Thanks,
> 
> Andrew Murray
> 
>>  	},
>>  	{},
>>  };
>> -- 
>> 2.17.1
>>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/6] phy: meson-g12a-usb3-pcie: Add support for PCIe mode
  2019-09-11 12:19   ` Andrew Murray
@ 2019-09-11 12:45     ` Neil Armstrong
  2019-09-11 12:59       ` Andrew Murray
  0 siblings, 1 reply; 24+ messages in thread
From: Neil Armstrong @ 2019-09-11 12:45 UTC (permalink / raw)
  To: Andrew Murray
  Cc: lorenzo.pieralisi, khilman, linux-pci, linux-kernel, kishon,
	repk, maz, bhelgaas, linux-amlogic, yue.wang, linux-arm-kernel

On 11/09/2019 14:19, Andrew Murray wrote:
> On Sun, Sep 08, 2019 at 01:42:56PM +0000, Neil Armstrong wrote:
>> This adds extended PCIe PHY functions for the Amlogic G12A
>> USB3+PCIE Combo PHY to support reset, power_on and power_off for
>> PCIe exclusively.
>>
>> With these callbacks, we can handle all the needed operations of the
>> Amlogic PCIe controller driver.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  .../phy/amlogic/phy-meson-g12a-usb3-pcie.c    | 70 ++++++++++++++++---
>>  1 file changed, 61 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c b/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
>> index ac322d643c7a..08e322789e59 100644
>> --- a/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
>> +++ b/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
>> @@ -50,6 +50,8 @@
>>  	#define PHY_R5_PHY_CR_ACK				BIT(16)
>>  	#define PHY_R5_PHY_BS_OUT				BIT(17)
>>  
>> +#define PCIE_RESET_DELAY					500
>> +
>>  struct phy_g12a_usb3_pcie_priv {
>>  	struct regmap		*regmap;
>>  	struct regmap		*regmap_cr;
>> @@ -196,6 +198,10 @@ static int phy_g12a_usb3_init(struct phy *phy)
>>  	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
>>  	int data, ret;
>>  
>> +	ret = reset_control_reset(priv->reset);
>> +	if (ret)
>> +		return ret;
>> +
> 
> Right, so we've moved this to apply to USB only, thus assuming PCI will
> call .reset for its reset (why the asymmetry?).

Exact, there is no power_on/power_off when USB3 mode is used, and vendor
always reset the PHY before switching to USB3, but for PCIe, it seems the
reset and the power_on must be done separately with the PCIe controller init
and reset in the middle.

I would prefer symmetry aswell :-/

Neil

> 
> Thanks,
> 
> Andrew Murray
> 
>>  	/* Switch PHY to USB3 */
>>  	/* TODO figure out how to handle when PCIe was set in the bootloader */
>>  	regmap_update_bits(priv->regmap, PHY_R0,
>> @@ -272,24 +278,64 @@ static int phy_g12a_usb3_init(struct phy *phy)
>>  	return 0;
>>  }
>>  
>> -static int phy_g12a_usb3_pcie_init(struct phy *phy)
>> +static int phy_g12a_usb3_pcie_power_on(struct phy *phy)
>> +{
>> +	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
>> +
>> +	if (priv->mode == PHY_TYPE_USB3)
>> +		return 0;
>> +
>> +	regmap_update_bits(priv->regmap, PHY_R0,
>> +			   PHY_R0_PCIE_POWER_STATE,
>> +			   FIELD_PREP(PHY_R0_PCIE_POWER_STATE, 0x1c));
>> +
>> +	return 0;
>> +}
>> +
>> +static int phy_g12a_usb3_pcie_power_off(struct phy *phy)
>> +{
>> +	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
>> +
>> +	if (priv->mode == PHY_TYPE_USB3)
>> +		return 0;
>> +
>> +	regmap_update_bits(priv->regmap, PHY_R0,
>> +			   PHY_R0_PCIE_POWER_STATE,
>> +			   FIELD_PREP(PHY_R0_PCIE_POWER_STATE, 0x1d));
>> +
>> +	return 0;
>> +}
>> +
>> +static int phy_g12a_usb3_pcie_reset(struct phy *phy)
>>  {
>>  	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
>>  	int ret;
>>  
>> -	ret = reset_control_reset(priv->reset);
>> +	if (priv->mode == PHY_TYPE_USB3)
>> +		return 0;
>> +
>> +	ret = reset_control_assert(priv->reset);
>>  	if (ret)
>>  		return ret;
>>  
>> +	udelay(PCIE_RESET_DELAY);
>> +
>> +	ret = reset_control_deassert(priv->reset);
>> +	if (ret)
>> +		return ret;
>> +
>> +	udelay(PCIE_RESET_DELAY);
>> +
>> +	return 0;
>> +}
>> +
>> +static int phy_g12a_usb3_pcie_init(struct phy *phy)
>> +{
>> +	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
>> +
>>  	if (priv->mode == PHY_TYPE_USB3)
>>  		return phy_g12a_usb3_init(phy);
>>  
>> -	/* Power UP PCIE */
>> -	/* TODO figure out when the bootloader has set USB3 mode before */
>> -	regmap_update_bits(priv->regmap, PHY_R0,
>> -			   PHY_R0_PCIE_POWER_STATE,
>> -			   FIELD_PREP(PHY_R0_PCIE_POWER_STATE, 0x1c));
>> -
>>  	return 0;
>>  }
>>  
>> @@ -297,7 +343,10 @@ static int phy_g12a_usb3_pcie_exit(struct phy *phy)
>>  {
>>  	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
>>  
>> -	return reset_control_reset(priv->reset);
>> +	if (priv->mode == PHY_TYPE_USB3)
>> +		return reset_control_reset(priv->reset);
>> +
>> +	return 0;
>>  }
>>  
>>  static struct phy *phy_g12a_usb3_pcie_xlate(struct device *dev,
>> @@ -326,6 +375,9 @@ static struct phy *phy_g12a_usb3_pcie_xlate(struct device *dev,
>>  static const struct phy_ops phy_g12a_usb3_pcie_ops = {
>>  	.init		= phy_g12a_usb3_pcie_init,
>>  	.exit		= phy_g12a_usb3_pcie_exit,
>> +	.power_on	= phy_g12a_usb3_pcie_power_on,
>> +	.power_off	= phy_g12a_usb3_pcie_power_off,
>> +	.reset		= phy_g12a_usb3_pcie_reset,
>>  	.owner		= THIS_MODULE,
>>  };
>>  
>> -- 
>> 2.17.1
>>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 6/6] arm64: dts: khadas-vim3: add commented support for PCIe
  2019-09-08 13:42 ` [PATCH 6/6] arm64: dts: khadas-vim3: add commented support for PCIe Neil Armstrong
  2019-09-09 16:37   ` Marc Zyngier
@ 2019-09-11 12:50   ` Andrew Murray
  2019-09-11 12:58     ` Neil Armstrong
  1 sibling, 1 reply; 24+ messages in thread
From: Andrew Murray @ 2019-09-11 12:50 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: lorenzo.pieralisi, khilman, linux-pci, linux-kernel, kishon,
	repk, maz, bhelgaas, linux-amlogic, yue.wang, linux-arm-kernel

On Sun, Sep 08, 2019 at 01:42:58PM +0000, Neil Armstrong wrote:
> The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
> lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
> an USB3.0 Type A connector and a M.2 Key M slot.
> The PHY driving these differential lines is shared between
> the USB3.0 controller and the PCIe Controller, thus only
> a single controller can use it.
> 
> The needed DT configuration when the MCU is configured to mux
> the PCIe/USB3.0 differential lines to the M.2 Key M slot is
> added commented and may uncommented to disable USB3.0 from the

*and may be*

> USB Complex and enable the PCIe controller.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  .../amlogic/meson-g12b-a311d-khadas-vim3.dts  | 22 +++++++++++++++++++
>  .../amlogic/meson-g12b-s922x-khadas-vim3.dts  | 22 +++++++++++++++++++
>  .../boot/dts/amlogic/meson-khadas-vim3.dtsi   |  4 ++++
>  .../dts/amlogic/meson-sm1-khadas-vim3l.dts    | 22 +++++++++++++++++++
>  4 files changed, 70 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
> index 3a6a1e0c1e32..0577b1435cbb 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
> @@ -14,3 +14,25 @@
>  / {
>  	compatible = "khadas,vim3", "amlogic,a311d", "amlogic,g12b";
>  };
> +
> +/*
> + * The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
> + * lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
> + * an USB3.0 Type A connector and a M.2 Key M slot.
> + * The PHY driving these differential lines is shared between
> + * the USB3.0 controller and the PCIe Controller, thus only
> + * a single controller can use it.
> + * If the MCU is configured to mux the PCIe/USB3.0 differential lines
> + * to the M.2 Key M slot, uncomment the following block to disable
> + * USB3.0 from the USB Complex and enable the PCIe controller.
> + */
> +/*
> +&pcie {
> +	status = "okay";
> +};
> +
> +&usb {
> +	phys = <&usb2_phy0>, <&usb2_phy1>;
> +	phy-names = "usb2-phy0", "usb2-phy1";
> +};

I assume there is no way other way to determine from the hardware which way
the mux is set?

Otherwise phy_g12a_usb3_pcie_xlate could determine the hardware mode, and
reject the phy instance with the wrong mode. Thus resulting in either the
PCI or USB to fail their probe. And avoiding the need to modify the DT on
boot.

Thanks,

Andrew Murray

> + */
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-s922x-khadas-vim3.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-s922x-khadas-vim3.dts
> index b73deb282120..1ef5c2f04f67 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-g12b-s922x-khadas-vim3.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-s922x-khadas-vim3.dts
> @@ -14,3 +14,25 @@
>  / {
>  	compatible = "khadas,vim3", "amlogic,s922x", "amlogic,g12b";
>  };
> +
> +/*
> + * The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
> + * lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
> + * an USB3.0 Type A connector and a M.2 Key M slot.
> + * The PHY driving these differential lines is shared between
> + * the USB3.0 controller and the PCIe Controller, thus only
> + * a single controller can use it.
> + * If the MCU is configured to mux the PCIe/USB3.0 differential lines
> + * to the M.2 Key M slot, uncomment the following block to disable
> + * USB3.0 from the USB Complex and enable the PCIe controller.
> + */
> +/*
> +&pcie {
> +	status = "okay";
> +};
> +
> +&usb {
> +	phys = <&usb2_phy0>, <&usb2_phy1>;
> +	phy-names = "usb2-phy0", "usb2-phy1";
> +};
> + */
> diff --git a/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi b/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
> index 8647da7d6609..eac5720dc15f 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
> @@ -246,6 +246,10 @@
>  	linux,rc-map-name = "rc-khadas";
>  };
>  
> +&pcie {
> +	reset-gpios = <&gpio GPIOA_8 GPIO_ACTIVE_LOW>;
> +};
> +
>  &pwm_ef {
>          status = "okay";
>          pinctrl-0 = <&pwm_e_pins>;
> diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
> index 5233bd7cacfb..d9c7cbedce53 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
> @@ -68,3 +68,25 @@
>  	clock-names = "clkin1";
>  	status = "okay";
>  };
> +
> +/*
> + * The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
> + * lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
> + * an USB3.0 Type A connector and a M.2 Key M slot.
> + * The PHY driving these differential lines is shared between
> + * the USB3.0 controller and the PCIe Controller, thus only
> + * a single controller can use it.
> + * If the MCU is configured to mux the PCIe/USB3.0 differential lines
> + * to the M.2 Key M slot, uncomment the following block to disable
> + * USB3.0 from the USB Complex and enable the PCIe controller.
> + */
> +/*
> +&pcie {
> +	status = "okay";
> +};
> +
> +&usb {
> +	phys = <&usb2_phy0>, <&usb2_phy1>;
> +	phy-names = "usb2-phy0", "usb2-phy1";
> +};
> + */
> -- 
> 2.17.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/6] PCI: amlogic: meson: Add support for G12A
  2019-09-11 12:39     ` Neil Armstrong
@ 2019-09-11 12:58       ` Andrew Murray
  0 siblings, 0 replies; 24+ messages in thread
From: Andrew Murray @ 2019-09-11 12:58 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: lorenzo.pieralisi, khilman, linux-pci, linux-kernel, kishon,
	repk, maz, bhelgaas, linux-amlogic, yue.wang, linux-arm-kernel

On Wed, Sep 11, 2019 at 02:39:42PM +0200, Neil Armstrong wrote:
> Hi Andrew,
> 
> On 11/09/2019 13:36, Andrew Murray wrote:
> > On Sun, Sep 08, 2019 at 01:42:55PM +0000, Neil Armstrong wrote:
> >> Add support for the Amlogic G12A SoC using a separate shared PHY.
> >>
> >> This adds support for fetching a PHY phandle and call the PHY init,
> >> reset and power on/off calls instead of writing in the PHY register or
> >> toggling the PHY reset line.
> >>
> >> The MIPI clock is also made optional since it is used for setting up
> > 
> > Is it worth indicating here that the MIPI clock is *only required* for
> > the G12A (or controllers with a shared phy)? It's still required for
> > AXG. It's not optional for G12A - it's ignored.
> 
> Indeed it's ignored, I'll reword it.
> 
> > 
> >> the PHY reference clock chared with the DSI controller on AXG.
> > 
> > s/chared/shared/
> 
> Ack
> 
> > 
> >>
> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >> ---
> >>  drivers/pci/controller/dwc/pci-meson.c | 101 ++++++++++++++++++++-----
> >>  1 file changed, 84 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> >> index ab79990798f8..3fadad381762 100644
> >> --- a/drivers/pci/controller/dwc/pci-meson.c
> >> +++ b/drivers/pci/controller/dwc/pci-meson.c
> >> @@ -16,6 +16,7 @@
> >>  #include <linux/reset.h>
> >>  #include <linux/resource.h>
> >>  #include <linux/types.h>
> >> +#include <linux/phy/phy.h>
> >>  
> >>  #include "pcie-designware.h"
> >>  
> >> @@ -96,12 +97,18 @@ struct meson_pcie_rc_reset {
> >>  	struct reset_control *apb;
> >>  };
> >>  
> >> +struct meson_pcie_param {
> >> +	bool has_shared_phy;
> >> +};
> >> +
> >>  struct meson_pcie {
> >>  	struct dw_pcie pci;
> >>  	struct meson_pcie_mem_res mem_res;
> >>  	struct meson_pcie_clk_res clk_res;
> >>  	struct meson_pcie_rc_reset mrst;
> >>  	struct gpio_desc *reset_gpio;
> >> +	struct phy *phy;
> >> +	const struct meson_pcie_param *param;
> >>  };
> >>  
> >>  static struct reset_control *meson_pcie_get_reset(struct meson_pcie *mp,
> >> @@ -123,10 +130,12 @@ static int meson_pcie_get_resets(struct meson_pcie *mp)
> >>  {
> >>  	struct meson_pcie_rc_reset *mrst = &mp->mrst;
> >>  
> >> -	mrst->phy = meson_pcie_get_reset(mp, "phy", PCIE_SHARED_RESET);
> >> -	if (IS_ERR(mrst->phy))
> >> -		return PTR_ERR(mrst->phy);
> >> -	reset_control_deassert(mrst->phy);
> >> +	if (!mp->param->has_shared_phy) {
> >> +		mrst->phy = meson_pcie_get_reset(mp, "phy", PCIE_SHARED_RESET);
> >> +		if (IS_ERR(mrst->phy))
> >> +			return PTR_ERR(mrst->phy);
> >> +		reset_control_deassert(mrst->phy);
> >> +	}
> >>  
> >>  	mrst->port = meson_pcie_get_reset(mp, "port", PCIE_NORMAL_RESET);
> >>  	if (IS_ERR(mrst->port))
> >> @@ -180,6 +189,9 @@ static int meson_pcie_get_mems(struct platform_device *pdev,
> >>  	if (IS_ERR(mp->mem_res.cfg_base))
> >>  		return PTR_ERR(mp->mem_res.cfg_base);
> >>  
> >> +	if (mp->param->has_shared_phy)
> >> +		return 0;
> >> +
> > 
> > It may be more consistent if, rather than returning here, you wrapped
> > the following 3 lines by the if statement.
> 
> ok
> 
> > 
> >>  	/* Meson SoC has two PCI controllers use same phy register*/
> > 
> > I guess this comment should now be updated to refer to AXG?
> 
> Indeed
> 
> > 
> >>  	mp->mem_res.phy_base = meson_pcie_get_mem_shared(pdev, mp, "phy");
> >>  	if (IS_ERR(mp->mem_res.phy_base))
> >> @@ -188,19 +200,33 @@ static int meson_pcie_get_mems(struct platform_device *pdev,
> >>  	return 0;
> >>  }
> >>  
> >> -static void meson_pcie_power_on(struct meson_pcie *mp)
> >> +static int meson_pcie_power_on(struct meson_pcie *mp)
> >>  {
> >> -	writel(MESON_PCIE_PHY_POWERUP, mp->mem_res.phy_base);
> >> +	int ret = 0;
> >> +
> >> +	if (mp->param->has_shared_phy)
> >> +		ret = phy_power_on(mp->phy);
> > 
> > I haven't seen any phy_[init/exit] calls, should there be any?
> 
> There is no _init() needed, but indeed we should still call them even it's
> a no-op.

Yes - and that makes it easier for someone to modify the phy driver and not
have to worry that there may be users that don't call init.

> 
> > 
> >> +	else
> >> +		writel(MESON_PCIE_PHY_POWERUP, mp->mem_res.phy_base);
> >> +
> >> +	return ret;
> >>  }
> >>  
> >> -static void meson_pcie_reset(struct meson_pcie *mp)
> >> +static int meson_pcie_reset(struct meson_pcie *mp)
> >>  {
> >>  	struct meson_pcie_rc_reset *mrst = &mp->mrst;
> >> -
> >> -	reset_control_assert(mrst->phy);
> >> -	udelay(PCIE_RESET_DELAY);
> >> -	reset_control_deassert(mrst->phy);
> >> -	udelay(PCIE_RESET_DELAY);
> >> +	int ret = 0;
> >> +
> >> +	if (mp->param->has_shared_phy) {
> >> +		ret = phy_reset(mp->phy);
> >> +		if (ret)
> >> +			return ret;
> >> +	} else {
> >> +		reset_control_assert(mrst->phy);
> >> +		udelay(PCIE_RESET_DELAY);
> >> +		reset_control_deassert(mrst->phy);
> >> +		udelay(PCIE_RESET_DELAY);
> >> +	}
> >>  
> >>  	reset_control_assert(mrst->port);
> >>  	reset_control_assert(mrst->apb);
> >> @@ -208,6 +234,8 @@ static void meson_pcie_reset(struct meson_pcie *mp)
> >>  	reset_control_deassert(mrst->port);
> >>  	reset_control_deassert(mrst->apb);
> >>  	udelay(PCIE_RESET_DELAY);
> >> +
> >> +	return 0;
> >>  }
> >>  
> >>  static inline struct clk *meson_pcie_probe_clock(struct device *dev,
> >> @@ -250,9 +278,11 @@ static int meson_pcie_probe_clocks(struct meson_pcie *mp)
> >>  	if (IS_ERR(res->port_clk))
> >>  		return PTR_ERR(res->port_clk);
> >>  
> >> -	res->mipi_gate = meson_pcie_probe_clock(dev, "mipi", 0);
> >> -	if (IS_ERR(res->mipi_gate))
> >> -		return PTR_ERR(res->mipi_gate);
> >> +	if (!mp->param->has_shared_phy) {
> >> +		res->mipi_gate = meson_pcie_probe_clock(dev, "mipi", 0);
> >> +		if (IS_ERR(res->mipi_gate))
> >> +			return PTR_ERR(res->mipi_gate);
> >> +	}
> >>  
> >>  	res->general_clk = meson_pcie_probe_clock(dev, "general", 0);
> >>  	if (IS_ERR(res->general_clk))
> >> @@ -524,6 +554,7 @@ static const struct dw_pcie_ops dw_pcie_ops = {
> >>  
> >>  static int meson_pcie_probe(struct platform_device *pdev)
> >>  {
> >> +	const struct meson_pcie_param *match_data;
> >>  	struct device *dev = &pdev->dev;
> >>  	struct dw_pcie *pci;
> >>  	struct meson_pcie *mp;
> >> @@ -537,6 +568,20 @@ static int meson_pcie_probe(struct platform_device *pdev)
> >>  	pci->dev = dev;
> >>  	pci->ops = &dw_pcie_ops;
> >>  
> >> +	match_data = of_device_get_match_data(dev);
> >> +	if (!match_data) {
> >> +		dev_err(dev, "failed to get match data\n");
> >> +		return -ENODEV;
> >> +	}
> >> +	mp->param = match_data;
> >> +
> >> +	if (mp->param->has_shared_phy) {
> >> +		mp->phy = devm_phy_get(dev, "pcie");
> >> +		if (IS_ERR(mp->phy)) {
> >> +			return PTR_ERR(mp->phy);
> >> +		}
> >> +	}
> >> +
> >>  	mp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> >>  	if (IS_ERR(mp->reset_gpio)) {
> >>  		dev_err(dev, "get reset gpio failed\n");
> >> @@ -555,8 +600,17 @@ static int meson_pcie_probe(struct platform_device *pdev)
> >>  		return ret;
> >>  	}
> >>  
> >> -	meson_pcie_power_on(mp);
> >> -	meson_pcie_reset(mp);
> >> +	ret = meson_pcie_power_on(mp);
> >> +	if (ret) {
> >> +		dev_err(dev, "phy power on failed, %d\n", ret);
> >> +		return ret;
> >> +	}
> >> +
> >> +	ret = meson_pcie_reset(mp);
> >> +	if (ret) {
> >> +		dev_err(dev, "reset failed, %d\n", ret);
> >> +		return ret;
> >> +	}
> >>  
> >>  	ret = meson_pcie_probe_clocks(mp);
> >>  	if (ret) {
> >> @@ -575,9 +629,22 @@ static int meson_pcie_probe(struct platform_device *pdev)
> >>  	return 0;
> >>  }
> >>  
> >> +static struct meson_pcie_param meson_pcie_axg_param = {
> >> +	.has_shared_phy = false,
> >> +};
> >> +
> >> +static struct meson_pcie_param meson_pcie_g12a_param = {
> >> +	.has_shared_phy = true,
> >> +};
> >> +
> >>  static const struct of_device_id meson_pcie_of_match[] = {
> >>  	{
> >>  		.compatible = "amlogic,axg-pcie",
> >> +		.data = &meson_pcie_axg_param,
> >> +	},
> >> +	{
> >> +		.compatible = "amlogic,g12a-pcie",
> >> +		.data = &meson_pcie_g12a_param,
> > 
> > Here, we hard-code knowledge about the SOCs regarding if they have shared phys
> > or not. I guess the alternative would have been to assume there is a shared
> > phy if the DT has a phandle for it. I.e. instead of mp->param->has_shared_phy
> > everywhere you could test for mp->phy. Though I guess at least with the
> > current approach you guard against bad DTs, this seems OK.
> 
> I could split with if(mp->phy) and .needs_mipi_clk, but overall it would
> be the same, and I wouldn't know how to react if we forget the PHY in g12a DT
> since we wouldn't have the PHY register memory zone.
> On G12A, the PHY is mandatory unlike AXG.

Indeed.

> 
> And finally this MIPI clock is part of the PHY ref clock, so I think
> it's fine to wrap it in the .has_shared_phy knowledge.

I feel like the naming of "mipi" is unfortunate as ideally it'd be something
like "ref" or similar. Especially if another SoC uses meson PCI, without a
shared phy but with a reference clock that isn't MIPI. But I don't think
anyone wants to change the existing bindings.

I think your current approach is robust, I have no objections.

> 
> Thanks for the review,

Thanks,

Andrew Murray

> Neil
> 
> > 
> > Thanks,
> > 
> > Andrew Murray
> > 
> >>  	},
> >>  	{},
> >>  };
> >> -- 
> >> 2.17.1
> >>
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 6/6] arm64: dts: khadas-vim3: add commented support for PCIe
  2019-09-11 12:50   ` Andrew Murray
@ 2019-09-11 12:58     ` Neil Armstrong
  2019-09-11 13:11       ` Andrew Murray
  0 siblings, 1 reply; 24+ messages in thread
From: Neil Armstrong @ 2019-09-11 12:58 UTC (permalink / raw)
  To: Andrew Murray
  Cc: lorenzo.pieralisi, khilman, linux-pci, linux-kernel, kishon,
	repk, maz, bhelgaas, linux-amlogic, yue.wang, linux-arm-kernel

On 11/09/2019 14:50, Andrew Murray wrote:
> On Sun, Sep 08, 2019 at 01:42:58PM +0000, Neil Armstrong wrote:
>> The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
>> lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
>> an USB3.0 Type A connector and a M.2 Key M slot.
>> The PHY driving these differential lines is shared between
>> the USB3.0 controller and the PCIe Controller, thus only
>> a single controller can use it.
>>
>> The needed DT configuration when the MCU is configured to mux
>> the PCIe/USB3.0 differential lines to the M.2 Key M slot is
>> added commented and may uncommented to disable USB3.0 from the
> 
> *and may be*
> 
>> USB Complex and enable the PCIe controller.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  .../amlogic/meson-g12b-a311d-khadas-vim3.dts  | 22 +++++++++++++++++++
>>  .../amlogic/meson-g12b-s922x-khadas-vim3.dts  | 22 +++++++++++++++++++
>>  .../boot/dts/amlogic/meson-khadas-vim3.dtsi   |  4 ++++
>>  .../dts/amlogic/meson-sm1-khadas-vim3l.dts    | 22 +++++++++++++++++++
>>  4 files changed, 70 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
>> index 3a6a1e0c1e32..0577b1435cbb 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
>> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
>> @@ -14,3 +14,25 @@
>>  / {
>>  	compatible = "khadas,vim3", "amlogic,a311d", "amlogic,g12b";
>>  };
>> +
>> +/*
>> + * The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
>> + * lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
>> + * an USB3.0 Type A connector and a M.2 Key M slot.
>> + * The PHY driving these differential lines is shared between
>> + * the USB3.0 controller and the PCIe Controller, thus only
>> + * a single controller can use it.
>> + * If the MCU is configured to mux the PCIe/USB3.0 differential lines
>> + * to the M.2 Key M slot, uncomment the following block to disable
>> + * USB3.0 from the USB Complex and enable the PCIe controller.
>> + */
>> +/*
>> +&pcie {
>> +	status = "okay";
>> +};
>> +
>> +&usb {
>> +	phys = <&usb2_phy0>, <&usb2_phy1>;
>> +	phy-names = "usb2-phy0", "usb2-phy1";
>> +};
> 
> I assume there is no way other way to determine from the hardware which way
> the mux is set?

No, it would be simpler :-/ The MUX is on-board and the MCU drives the MUX selection.

You can look at the https://dl.khadas.com/Hardware/VIM3/Schematic/VIM3_V11_Sch.pdf
The PCIE_EN signal is driven by the STM8S MCU.

> 
> Otherwise phy_g12a_usb3_pcie_xlate could determine the hardware mode, and
> reject the phy instance with the wrong mode. Thus resulting in either the
> PCI or USB to fail their probe. And avoiding the need to modify the DT on
> boot.

Yep, it would have been simpler this way. Maybe a board vendor will set a gpio ?
who knows, but for actual boards it's static or with 0ohm resistors, and for the
VIM3 we only know by asking the MCU.

Maybe we could add a fake PHY as a MCU MFD subdevice, wrapping calls to the
right PHY. But for now the MCU has no upstream driver anyway.

Neil

> 
> Thanks,
> 
> Andrew Murray
> 
>> + */
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-s922x-khadas-vim3.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-s922x-khadas-vim3.dts
>> index b73deb282120..1ef5c2f04f67 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-g12b-s922x-khadas-vim3.dts
>> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-s922x-khadas-vim3.dts
>> @@ -14,3 +14,25 @@
>>  / {
>>  	compatible = "khadas,vim3", "amlogic,s922x", "amlogic,g12b";
>>  };
>> +
>> +/*
>> + * The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
>> + * lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
>> + * an USB3.0 Type A connector and a M.2 Key M slot.
>> + * The PHY driving these differential lines is shared between
>> + * the USB3.0 controller and the PCIe Controller, thus only
>> + * a single controller can use it.
>> + * If the MCU is configured to mux the PCIe/USB3.0 differential lines
>> + * to the M.2 Key M slot, uncomment the following block to disable
>> + * USB3.0 from the USB Complex and enable the PCIe controller.
>> + */
>> +/*
>> +&pcie {
>> +	status = "okay";
>> +};
>> +
>> +&usb {
>> +	phys = <&usb2_phy0>, <&usb2_phy1>;
>> +	phy-names = "usb2-phy0", "usb2-phy1";
>> +};
>> + */
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi b/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
>> index 8647da7d6609..eac5720dc15f 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
>> @@ -246,6 +246,10 @@
>>  	linux,rc-map-name = "rc-khadas";
>>  };
>>  
>> +&pcie {
>> +	reset-gpios = <&gpio GPIOA_8 GPIO_ACTIVE_LOW>;
>> +};
>> +
>>  &pwm_ef {
>>          status = "okay";
>>          pinctrl-0 = <&pwm_e_pins>;
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
>> index 5233bd7cacfb..d9c7cbedce53 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
>> +++ b/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
>> @@ -68,3 +68,25 @@
>>  	clock-names = "clkin1";
>>  	status = "okay";
>>  };
>> +
>> +/*
>> + * The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
>> + * lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
>> + * an USB3.0 Type A connector and a M.2 Key M slot.
>> + * The PHY driving these differential lines is shared between
>> + * the USB3.0 controller and the PCIe Controller, thus only
>> + * a single controller can use it.
>> + * If the MCU is configured to mux the PCIe/USB3.0 differential lines
>> + * to the M.2 Key M slot, uncomment the following block to disable
>> + * USB3.0 from the USB Complex and enable the PCIe controller.
>> + */
>> +/*
>> +&pcie {
>> +	status = "okay";
>> +};
>> +
>> +&usb {
>> +	phys = <&usb2_phy0>, <&usb2_phy1>;
>> +	phy-names = "usb2-phy0", "usb2-phy1";
>> +};
>> + */
>> -- 
>> 2.17.1
>>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/6] phy: meson-g12a-usb3-pcie: Add support for PCIe mode
  2019-09-11 12:45     ` Neil Armstrong
@ 2019-09-11 12:59       ` Andrew Murray
  0 siblings, 0 replies; 24+ messages in thread
From: Andrew Murray @ 2019-09-11 12:59 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: lorenzo.pieralisi, khilman, linux-pci, linux-kernel, kishon,
	repk, maz, bhelgaas, linux-amlogic, yue.wang, linux-arm-kernel

On Wed, Sep 11, 2019 at 02:45:23PM +0200, Neil Armstrong wrote:
> On 11/09/2019 14:19, Andrew Murray wrote:
> > On Sun, Sep 08, 2019 at 01:42:56PM +0000, Neil Armstrong wrote:
> >> This adds extended PCIe PHY functions for the Amlogic G12A
> >> USB3+PCIE Combo PHY to support reset, power_on and power_off for
> >> PCIe exclusively.
> >>
> >> With these callbacks, we can handle all the needed operations of the
> >> Amlogic PCIe controller driver.
> >>
> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >> ---
> >>  .../phy/amlogic/phy-meson-g12a-usb3-pcie.c    | 70 ++++++++++++++++---
> >>  1 file changed, 61 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c b/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
> >> index ac322d643c7a..08e322789e59 100644
> >> --- a/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
> >> +++ b/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
> >> @@ -50,6 +50,8 @@
> >>  	#define PHY_R5_PHY_CR_ACK				BIT(16)
> >>  	#define PHY_R5_PHY_BS_OUT				BIT(17)
> >>  
> >> +#define PCIE_RESET_DELAY					500
> >> +
> >>  struct phy_g12a_usb3_pcie_priv {
> >>  	struct regmap		*regmap;
> >>  	struct regmap		*regmap_cr;
> >> @@ -196,6 +198,10 @@ static int phy_g12a_usb3_init(struct phy *phy)
> >>  	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
> >>  	int data, ret;
> >>  
> >> +	ret = reset_control_reset(priv->reset);
> >> +	if (ret)
> >> +		return ret;
> >> +
> > 
> > Right, so we've moved this to apply to USB only, thus assuming PCI will
> > call .reset for its reset (why the asymmetry?).
> 
> Exact, there is no power_on/power_off when USB3 mode is used, and vendor
> always reset the PHY before switching to USB3, but for PCIe, it seems the
> reset and the power_on must be done separately with the PCIe controller init
> and reset in the middle.
> 
> I would prefer symmetry aswell :-/

OK.

Thanks,

Andrew Murray

> 
> Neil
> 
> > 
> > Thanks,
> > 
> > Andrew Murray
> > 
> >>  	/* Switch PHY to USB3 */
> >>  	/* TODO figure out how to handle when PCIe was set in the bootloader */
> >>  	regmap_update_bits(priv->regmap, PHY_R0,
> >> @@ -272,24 +278,64 @@ static int phy_g12a_usb3_init(struct phy *phy)
> >>  	return 0;
> >>  }
> >>  
> >> -static int phy_g12a_usb3_pcie_init(struct phy *phy)
> >> +static int phy_g12a_usb3_pcie_power_on(struct phy *phy)
> >> +{
> >> +	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
> >> +
> >> +	if (priv->mode == PHY_TYPE_USB3)
> >> +		return 0;
> >> +
> >> +	regmap_update_bits(priv->regmap, PHY_R0,
> >> +			   PHY_R0_PCIE_POWER_STATE,
> >> +			   FIELD_PREP(PHY_R0_PCIE_POWER_STATE, 0x1c));
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int phy_g12a_usb3_pcie_power_off(struct phy *phy)
> >> +{
> >> +	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
> >> +
> >> +	if (priv->mode == PHY_TYPE_USB3)
> >> +		return 0;
> >> +
> >> +	regmap_update_bits(priv->regmap, PHY_R0,
> >> +			   PHY_R0_PCIE_POWER_STATE,
> >> +			   FIELD_PREP(PHY_R0_PCIE_POWER_STATE, 0x1d));
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int phy_g12a_usb3_pcie_reset(struct phy *phy)
> >>  {
> >>  	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
> >>  	int ret;
> >>  
> >> -	ret = reset_control_reset(priv->reset);
> >> +	if (priv->mode == PHY_TYPE_USB3)
> >> +		return 0;
> >> +
> >> +	ret = reset_control_assert(priv->reset);
> >>  	if (ret)
> >>  		return ret;
> >>  
> >> +	udelay(PCIE_RESET_DELAY);
> >> +
> >> +	ret = reset_control_deassert(priv->reset);
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	udelay(PCIE_RESET_DELAY);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int phy_g12a_usb3_pcie_init(struct phy *phy)
> >> +{
> >> +	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
> >> +
> >>  	if (priv->mode == PHY_TYPE_USB3)
> >>  		return phy_g12a_usb3_init(phy);
> >>  
> >> -	/* Power UP PCIE */
> >> -	/* TODO figure out when the bootloader has set USB3 mode before */
> >> -	regmap_update_bits(priv->regmap, PHY_R0,
> >> -			   PHY_R0_PCIE_POWER_STATE,
> >> -			   FIELD_PREP(PHY_R0_PCIE_POWER_STATE, 0x1c));
> >> -
> >>  	return 0;
> >>  }
> >>  
> >> @@ -297,7 +343,10 @@ static int phy_g12a_usb3_pcie_exit(struct phy *phy)
> >>  {
> >>  	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
> >>  
> >> -	return reset_control_reset(priv->reset);
> >> +	if (priv->mode == PHY_TYPE_USB3)
> >> +		return reset_control_reset(priv->reset);
> >> +
> >> +	return 0;
> >>  }
> >>  
> >>  static struct phy *phy_g12a_usb3_pcie_xlate(struct device *dev,
> >> @@ -326,6 +375,9 @@ static struct phy *phy_g12a_usb3_pcie_xlate(struct device *dev,
> >>  static const struct phy_ops phy_g12a_usb3_pcie_ops = {
> >>  	.init		= phy_g12a_usb3_pcie_init,
> >>  	.exit		= phy_g12a_usb3_pcie_exit,
> >> +	.power_on	= phy_g12a_usb3_pcie_power_on,
> >> +	.power_off	= phy_g12a_usb3_pcie_power_off,
> >> +	.reset		= phy_g12a_usb3_pcie_reset,
> >>  	.owner		= THIS_MODULE,
> >>  };
> >>  
> >> -- 
> >> 2.17.1
> >>
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 6/6] arm64: dts: khadas-vim3: add commented support for PCIe
  2019-09-11 12:58     ` Neil Armstrong
@ 2019-09-11 13:11       ` Andrew Murray
  0 siblings, 0 replies; 24+ messages in thread
From: Andrew Murray @ 2019-09-11 13:11 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: lorenzo.pieralisi, khilman, linux-pci, linux-kernel, kishon,
	repk, maz, bhelgaas, linux-amlogic, yue.wang, linux-arm-kernel

On Wed, Sep 11, 2019 at 02:58:18PM +0200, Neil Armstrong wrote:
> On 11/09/2019 14:50, Andrew Murray wrote:
> > On Sun, Sep 08, 2019 at 01:42:58PM +0000, Neil Armstrong wrote:
> >> The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
> >> lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
> >> an USB3.0 Type A connector and a M.2 Key M slot.
> >> The PHY driving these differential lines is shared between
> >> the USB3.0 controller and the PCIe Controller, thus only
> >> a single controller can use it.
> >>
> >> The needed DT configuration when the MCU is configured to mux
> >> the PCIe/USB3.0 differential lines to the M.2 Key M slot is
> >> added commented and may uncommented to disable USB3.0 from the
> > 
> > *and may be*
> > 
> >> USB Complex and enable the PCIe controller.
> >>
> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >> ---
> >>  .../amlogic/meson-g12b-a311d-khadas-vim3.dts  | 22 +++++++++++++++++++
> >>  .../amlogic/meson-g12b-s922x-khadas-vim3.dts  | 22 +++++++++++++++++++
> >>  .../boot/dts/amlogic/meson-khadas-vim3.dtsi   |  4 ++++
> >>  .../dts/amlogic/meson-sm1-khadas-vim3l.dts    | 22 +++++++++++++++++++
> >>  4 files changed, 70 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
> >> index 3a6a1e0c1e32..0577b1435cbb 100644
> >> --- a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
> >> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
> >> @@ -14,3 +14,25 @@
> >>  / {
> >>  	compatible = "khadas,vim3", "amlogic,a311d", "amlogic,g12b";
> >>  };
> >> +
> >> +/*
> >> + * The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
> >> + * lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
> >> + * an USB3.0 Type A connector and a M.2 Key M slot.
> >> + * The PHY driving these differential lines is shared between
> >> + * the USB3.0 controller and the PCIe Controller, thus only
> >> + * a single controller can use it.
> >> + * If the MCU is configured to mux the PCIe/USB3.0 differential lines
> >> + * to the M.2 Key M slot, uncomment the following block to disable
> >> + * USB3.0 from the USB Complex and enable the PCIe controller.
> >> + */
> >> +/*
> >> +&pcie {
> >> +	status = "okay";
> >> +};
> >> +
> >> +&usb {
> >> +	phys = <&usb2_phy0>, <&usb2_phy1>;
> >> +	phy-names = "usb2-phy0", "usb2-phy1";
> >> +};
> > 
> > I assume there is no way other way to determine from the hardware which way
> > the mux is set?
> 
> No, it would be simpler :-/ The MUX is on-board and the MCU drives the MUX selection.
> 
> You can look at the https://dl.khadas.com/Hardware/VIM3/Schematic/VIM3_V11_Sch.pdf
> The PCIE_EN signal is driven by the STM8S MCU.

Ah I see.

> 
> > 
> > Otherwise phy_g12a_usb3_pcie_xlate could determine the hardware mode, and
> > reject the phy instance with the wrong mode. Thus resulting in either the
> > PCI or USB to fail their probe. And avoiding the need to modify the DT on
> > boot.
> 
> Yep, it would have been simpler this way. Maybe a board vendor will set a gpio ?
> who knows, but for actual boards it's static or with 0ohm resistors, and for the
> VIM3 we only know by asking the MCU.
> 
> Maybe we could add a fake PHY as a MCU MFD subdevice, wrapping calls to the
> right PHY. But for now the MCU has no upstream driver anyway.

OK

Thanks,

Andrew Murray

> 
> Neil
> 
> > 
> > Thanks,
> > 
> > Andrew Murray
> > 
> >> + */
> >> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-s922x-khadas-vim3.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-s922x-khadas-vim3.dts
> >> index b73deb282120..1ef5c2f04f67 100644
> >> --- a/arch/arm64/boot/dts/amlogic/meson-g12b-s922x-khadas-vim3.dts
> >> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-s922x-khadas-vim3.dts
> >> @@ -14,3 +14,25 @@
> >>  / {
> >>  	compatible = "khadas,vim3", "amlogic,s922x", "amlogic,g12b";
> >>  };
> >> +
> >> +/*
> >> + * The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
> >> + * lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
> >> + * an USB3.0 Type A connector and a M.2 Key M slot.
> >> + * The PHY driving these differential lines is shared between
> >> + * the USB3.0 controller and the PCIe Controller, thus only
> >> + * a single controller can use it.
> >> + * If the MCU is configured to mux the PCIe/USB3.0 differential lines
> >> + * to the M.2 Key M slot, uncomment the following block to disable
> >> + * USB3.0 from the USB Complex and enable the PCIe controller.
> >> + */
> >> +/*
> >> +&pcie {
> >> +	status = "okay";
> >> +};
> >> +
> >> +&usb {
> >> +	phys = <&usb2_phy0>, <&usb2_phy1>;
> >> +	phy-names = "usb2-phy0", "usb2-phy1";
> >> +};
> >> + */
> >> diff --git a/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi b/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
> >> index 8647da7d6609..eac5720dc15f 100644
> >> --- a/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
> >> +++ b/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
> >> @@ -246,6 +246,10 @@
> >>  	linux,rc-map-name = "rc-khadas";
> >>  };
> >>  
> >> +&pcie {
> >> +	reset-gpios = <&gpio GPIOA_8 GPIO_ACTIVE_LOW>;
> >> +};
> >> +
> >>  &pwm_ef {
> >>          status = "okay";
> >>          pinctrl-0 = <&pwm_e_pins>;
> >> diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
> >> index 5233bd7cacfb..d9c7cbedce53 100644
> >> --- a/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
> >> +++ b/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
> >> @@ -68,3 +68,25 @@
> >>  	clock-names = "clkin1";
> >>  	status = "okay";
> >>  };
> >> +
> >> +/*
> >> + * The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
> >> + * lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
> >> + * an USB3.0 Type A connector and a M.2 Key M slot.
> >> + * The PHY driving these differential lines is shared between
> >> + * the USB3.0 controller and the PCIe Controller, thus only
> >> + * a single controller can use it.
> >> + * If the MCU is configured to mux the PCIe/USB3.0 differential lines
> >> + * to the M.2 Key M slot, uncomment the following block to disable
> >> + * USB3.0 from the USB Complex and enable the PCIe controller.
> >> + */
> >> +/*
> >> +&pcie {
> >> +	status = "okay";
> >> +};
> >> +
> >> +&usb {
> >> +	phys = <&usb2_phy0>, <&usb2_phy1>;
> >> +	phy-names = "usb2-phy0", "usb2-phy1";
> >> +};
> >> + */
> >> -- 
> >> 2.17.1
> >>
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/6] dt-bindings: pci: amlogic, meson-pcie: Add G12A bindings
  2019-09-08 13:42 ` [PATCH 1/6] dt-bindings: pci: amlogic,meson-pcie: Add G12A bindings Neil Armstrong
  2019-09-11 12:22   ` Andrew Murray
@ 2019-09-13 14:36   ` Rob Herring
  1 sibling, 0 replies; 24+ messages in thread
From: Rob Herring @ 2019-09-13 14:36 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: devicetree, lorenzo.pieralisi, Neil Armstrong, khilman,
	linux-pci, linux-kernel, kishon, repk, maz, bhelgaas,
	linux-amlogic, yue.wang, linux-arm-kernel

On Sun,  8 Sep 2019 13:42:53 +0000, Neil Armstrong wrote:
> Add PCIE bindings for the Amlogic G12A SoC, the support is the same
> but the PHY is shared with USB3 to control the differential lines.
> 
> Thus this adds a phy phandle to control the PHY, and sets invalid
> MIPI clock as optional for G12A.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  .../devicetree/bindings/pci/amlogic,meson-pcie.txt   | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2019-09-13 14:38 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-08 13:42 [PATCH 0/6] arm64: dts: meson-g12: add support for PCIe Neil Armstrong
2019-09-08 13:42 ` [PATCH 1/6] dt-bindings: pci: amlogic,meson-pcie: Add G12A bindings Neil Armstrong
2019-09-11 12:22   ` Andrew Murray
2019-09-11 12:30     ` Neil Armstrong
2019-09-13 14:36   ` [PATCH 1/6] dt-bindings: pci: amlogic, meson-pcie: " Rob Herring
2019-09-08 13:42 ` [PATCH 2/6] PCI: amlogic: Fix probed clock names Neil Armstrong
2019-09-11 10:59   ` Andrew Murray
2019-09-08 13:42 ` [PATCH 3/6] PCI: amlogic: meson: Add support for G12A Neil Armstrong
2019-09-11 11:36   ` Andrew Murray
2019-09-11 12:39     ` Neil Armstrong
2019-09-11 12:58       ` Andrew Murray
2019-09-08 13:42 ` [PATCH 4/6] phy: meson-g12a-usb3-pcie: Add support for PCIe mode Neil Armstrong
2019-09-11 12:19   ` Andrew Murray
2019-09-11 12:45     ` Neil Armstrong
2019-09-11 12:59       ` Andrew Murray
2019-09-08 13:42 ` [PATCH 5/6] arm64: dts: meson-g12a: Add PCIe node Neil Armstrong
2019-09-08 13:42 ` [PATCH 6/6] arm64: dts: khadas-vim3: add commented support for PCIe Neil Armstrong
2019-09-09 16:37   ` Marc Zyngier
2019-09-09 17:50     ` Neil Armstrong
2019-09-10  9:12       ` Marc Zyngier
2019-09-10  9:14         ` Neil Armstrong
2019-09-11 12:50   ` Andrew Murray
2019-09-11 12:58     ` Neil Armstrong
2019-09-11 13:11       ` Andrew Murray

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).