[PATCH v2 0/3] staging: mt7621-pci: extract PCI PHY part into a new 'pci-mt7621-phy' driver

[PATCH v2 0/3] staging: mt7621-pci: extract PCI PHY part into a new 'pci-mt7621-phy' driver

[Sergio Paracuellos]

There is a lot of code related with PHY part of the mt7621 PCI driver
that can be extracted to a new PHY generic driver to do the same function.

This patch series add a new 'mt7621-pci-phy' driver to handle those
and updates the mt7621-pci driver code to use generic kernel's PHY
API. This makes the code much better and readable.

Device tree has been also updated with new related bindinds for the
PCI PHY part.

This changes are only compile-tested.

Changes in v2:
    - PATCH 1: remap registers first and assign virtual address after
        for each port because they share the same address space.

Hope this helps.

Thanks in advance.

Best regards,
     Sergio Paracuellos


Sergio Paracuellos (3):
  staging: mt7621-pci-phy: add new driver for phy part of mt7621-pci
  staging: mt7621-pci: handle phy using new generic phy 'pci-mt7621-phy'
    driver
  staging: mt7621-dts: add pci-phy related bindings to board's device
    tree

 drivers/staging/Kconfig                       |   2 +
 drivers/staging/Makefile                      |   1 +
 drivers/staging/mt7621-dts/mt7621.dtsi        |  40 ++
 drivers/staging/mt7621-pci-phy/Kconfig        |   7 +
 drivers/staging/mt7621-pci-phy/Makefile       |   1 +
 drivers/staging/mt7621-pci-phy/TODO           |   4 +
 .../staging/mt7621-pci-phy/pci-mt7621-phy.c   | 411 ++++++++++++++++++
 drivers/staging/mt7621-pci/pci-mt7621.c       | 250 ++---------
 8 files changed, 498 insertions(+), 218 deletions(-)
 create mode 100644 drivers/staging/mt7621-pci-phy/Kconfig
 create mode 100644 drivers/staging/mt7621-pci-phy/Makefile
 create mode 100644 drivers/staging/mt7621-pci-phy/TODO
 create mode 100644 drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c

-- 
2.19.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 1/3] staging: mt7621-pci-phy: add new driver for phy part of mt7621-pci

[Sergio Paracuellos]

Phy part of the pci for this SoC can be handled using a generic phy
driver. This commit extracts phy part of the mt7621-pci into a new
'mt7621-pci-phy' driver.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/Kconfig                       |   2 +
 drivers/staging/Makefile                      |   1 +
 drivers/staging/mt7621-pci-phy/Kconfig        |   7 +
 drivers/staging/mt7621-pci-phy/Makefile       |   1 +
 drivers/staging/mt7621-pci-phy/TODO           |   4 +
 .../staging/mt7621-pci-phy/pci-mt7621-phy.c   | 411 ++++++++++++++++++
 6 files changed, 426 insertions(+)
 create mode 100644 drivers/staging/mt7621-pci-phy/Kconfig
 create mode 100644 drivers/staging/mt7621-pci-phy/Makefile
 create mode 100644 drivers/staging/mt7621-pci-phy/TODO
 create mode 100644 drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c

diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 7c015536360d..feec55e85347 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -106,6 +106,8 @@ source "drivers/staging/pi433/Kconfig"
 
 source "drivers/staging/mt7621-pci/Kconfig"
 
+source "drivers/staging/mt7621-pci-phy/Kconfig"
+
 source "drivers/staging/mt7621-pinctrl/Kconfig"
 
 source "drivers/staging/mt7621-spi/Kconfig"
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index a79b3fe20cf0..381891186572 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_BCM2835_VCHIQ)	+= vc04_services/
 obj-$(CONFIG_DRM_VBOXVIDEO)	+= vboxvideo/
 obj-$(CONFIG_PI433)		+= pi433/
 obj-$(CONFIG_SOC_MT7621)	+= mt7621-pci/
+obj-$(CONFIG_SOC_MT7621)	+= mt7621-pci-phy/
 obj-$(CONFIG_SOC_MT7621)	+= mt7621-pinctrl/
 obj-$(CONFIG_SOC_MT7621)	+= mt7621-spi/
 obj-$(CONFIG_SOC_MT7621)	+= mt7621-dma/
diff --git a/drivers/staging/mt7621-pci-phy/Kconfig b/drivers/staging/mt7621-pci-phy/Kconfig
new file mode 100644
index 000000000000..b9f6ab784ee8
--- /dev/null
+++ b/drivers/staging/mt7621-pci-phy/Kconfig
@@ -0,0 +1,7 @@
+config PCI_MT7621_PHY
+	tristate "MediaTek MT7621 PCI PHY Driver"
+	depends on RALINK && OF
+	select GENERIC_PHY
+	help
+	  Say 'Y' here to add support for MediaTek MT7621 PCI PHY driver,
+
diff --git a/drivers/staging/mt7621-pci-phy/Makefile b/drivers/staging/mt7621-pci-phy/Makefile
new file mode 100644
index 000000000000..2b82ccfc28c6
--- /dev/null
+++ b/drivers/staging/mt7621-pci-phy/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SOC_MT7621)       += pci-mt7621-phy.o
diff --git a/drivers/staging/mt7621-pci-phy/TODO b/drivers/staging/mt7621-pci-phy/TODO
new file mode 100644
index 000000000000..a255e8f753eb
--- /dev/null
+++ b/drivers/staging/mt7621-pci-phy/TODO
@@ -0,0 +1,4 @@
+
+- general code review and cleanup
+
+Cc:  NeilBrown <neil@brown.name> and Sergio Paracuellos <sergio.paracuellos@gmail.com>
diff --git a/drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c b/drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c
new file mode 100644
index 000000000000..1571c41aec36
--- /dev/null
+++ b/drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Mediatek MT7621 PCI PHY Driver
+ * Author: Sergio Paracuellos <sergio.paracuellos@gmail.com>
+ */
+
+#include <dt-bindings/phy/phy.h>
+#include <linux/bitops.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <mt7621.h>
+#include <ralink_regs.h>
+
+#define RALINK_CLKCFG1			0x30
+#define CHIP_REV_MT7621_E2		0x0101
+
+#define PCIE_PORT_CLK_EN(x)		BIT(24 + (x))
+
+#define RG_PE1_PIPE_REG			0x02c
+#define RG_PE1_PIPE_RST			BIT(12)
+#define RG_PE1_PIPE_CMD_FRC		BIT(4)
+
+#define RG_P0_TO_P1_WIDTH		0x100
+#define RG_PE1_H_LCDDS_REG		0x49c
+#define RG_PE1_H_LCDDS_PCW		GENMASK(30, 0)
+#define RG_PE1_H_LCDDS_PCW_VAL(x)	((0x7fffffff & (x)) << 0)
+
+#define RG_PE1_FRC_H_XTAL_REG		0x400
+#define RG_PE1_FRC_H_XTAL_TYPE          BIT(8)
+#define RG_PE1_H_XTAL_TYPE              GENMASK(10, 9)
+#define RG_PE1_H_XTAL_TYPE_VAL(x)       ((0x3 & (x)) << 9)
+
+#define RG_PE1_FRC_PHY_REG		0x000
+#define RG_PE1_FRC_PHY_EN               BIT(4)
+#define RG_PE1_PHY_EN                   BIT(5)
+
+#define RG_PE1_H_PLL_REG		0x490
+#define RG_PE1_H_PLL_BC			GENMASK(23, 22)
+#define RG_PE1_H_PLL_BC_VAL(x)		((0x3 & (x)) << 22)
+#define RG_PE1_H_PLL_BP			GENMASK(21, 18)
+#define RG_PE1_H_PLL_BP_VAL(x)		((0xf & (x)) << 18)
+#define RG_PE1_H_PLL_IR			GENMASK(15, 12)
+#define RG_PE1_H_PLL_IR_VAL(x)		((0xf & (x)) << 12)
+#define RG_PE1_H_PLL_IC			GENMASK(11, 8)
+#define RG_PE1_H_PLL_IC_VAL(x)		((0xf & (x)) << 8)
+#define RG_PE1_H_PLL_PREDIV             GENMASK(7, 6)
+#define RG_PE1_H_PLL_PREDIV_VAL(x)      ((0x3 & (x)) << 6)
+#define RG_PE1_PLL_DIVEN		GENMASK(3, 1)
+#define RG_PE1_PLL_DIVEN_VAL(x)		((0x7 & (x)) << 1)
+
+#define RG_PE1_H_PLL_FBKSEL_REG		0x4bc
+#define RG_PE1_H_PLL_FBKSEL             GENMASK(5, 4)
+#define RG_PE1_H_PLL_FBKSEL_VAL(x)      ((0x3 & (x)) << 4)
+
+#define	RG_PE1_H_LCDDS_SSC_PRD_REG	0x4a4
+#define RG_PE1_H_LCDDS_SSC_PRD          GENMASK(15, 0)
+#define RG_PE1_H_LCDDS_SSC_PRD_VAL(x)   ((0xffff & (x)) << 0)
+
+#define RG_PE1_H_LCDDS_SSC_DELTA_REG	0x4a8
+#define RG_PE1_H_LCDDS_SSC_DELTA        GENMASK(11, 0)
+#define RG_PE1_H_LCDDS_SSC_DELTA_VAL(x) ((0xfff & (x)) << 0)
+#define RG_PE1_H_LCDDS_SSC_DELTA1       GENMASK(27, 16)
+#define RG_PE1_H_LCDDS_SSC_DELTA1_VAL(x) ((0xff & (x)) << 16)
+
+#define RG_PE1_LCDDS_CLK_PH_INV_REG	0x4a0
+#define RG_PE1_LCDDS_CLK_PH_INV		BIT(5)
+
+#define RG_PE1_H_PLL_BR_REG		0x4ac
+#define RG_PE1_H_PLL_BR			GENMASK(18, 16)
+#define RG_PE1_H_PLL_BR_VAL(x)		((0x7 & (x)) << 16)
+
+#define	RG_PE1_MSTCKDIV_REG		0x414
+#define RG_PE1_MSTCKDIV			GENMASK(7, 6)
+#define RG_PE1_MSTCKDIV_VAL(x)		((0x3 & (x)) << 6)
+
+#define RG_PE1_FRC_MSTCKDIV		BIT(5)
+
+struct mt7621_pci_phy_instance {
+	struct phy *phy;
+	void __iomem *port_base;
+	u32 index;
+	u8 type;
+};
+
+struct mt7621_pci_phy {
+	struct device *dev;
+	struct mt7621_pci_phy_instance **phys;
+	int nphys;
+};
+
+static inline u32 phy_read(struct mt7621_pci_phy_instance *instance,
+				      u32 reg)
+{
+	return readl(instance->port_base + reg);
+}
+
+static inline void phy_write(struct mt7621_pci_phy_instance *instance,
+					u32 val, u32 reg)
+{
+	writel(val, instance->port_base + reg);
+}
+
+static void mt7621_bypass_pipe_rst(struct mt7621_pci_phy *phy,
+				   struct mt7621_pci_phy_instance *instance)
+{
+	u32 offset = (instance->index != 1) ?
+		RG_PE1_PIPE_REG : RG_PE1_PIPE_REG + RG_P0_TO_P1_WIDTH;
+	u32 reg;
+
+	reg = phy_read(instance, offset);
+	reg &= ~(RG_PE1_PIPE_RST | RG_PE1_PIPE_CMD_FRC);
+	reg |= (RG_PE1_PIPE_RST | RG_PE1_PIPE_CMD_FRC);
+	phy_write(instance, reg, offset);
+}
+
+static void mt7621_set_phy_for_ssc(struct mt7621_pci_phy *phy,
+				   struct mt7621_pci_phy_instance *instance)
+{
+	struct device *dev = phy->dev;
+	u32 reg = rt_sysc_r32(SYSC_REG_SYSTEM_CONFIG0);
+	u32 offset;
+	u32 val;
+
+	reg = (reg >> 6) & 0x7;
+	/* Set PCIe Port PHY to disable SSC */
+	/* Debug Xtal Type */
+	val = phy_read(instance, RG_PE1_FRC_H_XTAL_REG);
+	val &= ~(RG_PE1_FRC_H_XTAL_TYPE | RG_PE1_H_XTAL_TYPE);
+	val |= RG_PE1_FRC_H_XTAL_TYPE;
+	val |= RG_PE1_H_XTAL_TYPE_VAL(0x00);
+	phy_write(instance, val, RG_PE1_FRC_H_XTAL_REG);
+
+	/* disable port */
+	offset = (instance->index != 1) ?
+		RG_PE1_FRC_PHY_REG : RG_PE1_FRC_PHY_REG + RG_P0_TO_P1_WIDTH;
+	val = phy_read(instance, offset);
+	val &= ~(RG_PE1_FRC_PHY_EN | RG_PE1_PHY_EN);
+	val |= RG_PE1_FRC_PHY_EN;
+	phy_write(instance, val, offset);
+
+	/* Set Pre-divider ratio (for host mode) */
+	val = phy_read(instance, RG_PE1_H_PLL_REG);
+	val &= ~(RG_PE1_H_PLL_PREDIV);
+
+	if (reg <= 5 && reg >= 3) { /* 40MHz Xtal */
+		val |= RG_PE1_H_PLL_PREDIV_VAL(0x01);
+		phy_write(instance, val, RG_PE1_H_PLL_REG);
+		dev_info(dev, "Xtal is 40MHz\n");
+	} else { /* 25MHz | 20MHz Xtal */
+		val |= RG_PE1_H_PLL_PREDIV_VAL(0x00);
+		phy_write(instance, val, RG_PE1_H_PLL_REG);
+		if (reg >= 6) {
+			dev_info(dev, "Xtal is 25MHz\n");
+
+			/* Select feedback clock */
+			val = phy_read(instance, RG_PE1_H_PLL_FBKSEL_REG);
+			val &= ~(RG_PE1_H_PLL_FBKSEL);
+			val |= RG_PE1_H_PLL_FBKSEL_VAL(0x01);
+			phy_write(instance, val, RG_PE1_H_PLL_FBKSEL_REG);
+
+			/* DDS NCPO PCW (for host mode) */
+			val = phy_read(instance, RG_PE1_H_LCDDS_SSC_PRD_REG);
+			val &= ~(RG_PE1_H_LCDDS_SSC_PRD);
+			val |= RG_PE1_H_LCDDS_SSC_PRD_VAL(0x18000000);
+			phy_write(instance, val, RG_PE1_H_LCDDS_SSC_PRD_REG);
+
+			/* DDS SSC dither period control */
+			val = phy_read(instance, RG_PE1_H_LCDDS_SSC_PRD_REG);
+			val &= ~(RG_PE1_H_LCDDS_SSC_PRD);
+			val |= RG_PE1_H_LCDDS_SSC_PRD_VAL(0x18d);
+			phy_write(instance, val, RG_PE1_H_LCDDS_SSC_PRD_REG);
+
+			/* DDS SSC dither amplitude control */
+			val = phy_read(instance, RG_PE1_H_LCDDS_SSC_DELTA_REG);
+			val &= ~(RG_PE1_H_LCDDS_SSC_DELTA |
+				 RG_PE1_H_LCDDS_SSC_DELTA1);
+			val |= RG_PE1_H_LCDDS_SSC_DELTA_VAL(0x4a);
+			val |= RG_PE1_H_LCDDS_SSC_DELTA1_VAL(0x4a);
+			phy_write(instance, val, RG_PE1_H_LCDDS_SSC_DELTA_REG);
+		} else {
+			dev_info(dev, "Xtal is 20MHz\n");
+		}
+	}
+
+	/* DDS clock inversion */
+	val = phy_read(instance, RG_PE1_LCDDS_CLK_PH_INV_REG);
+	val &= ~(RG_PE1_LCDDS_CLK_PH_INV);
+	val |= RG_PE1_LCDDS_CLK_PH_INV;
+	phy_write(instance, val, RG_PE1_LCDDS_CLK_PH_INV_REG);
+
+	/* Set PLL bits */
+	val = phy_read(instance, RG_PE1_H_PLL_REG);
+	val &= ~(RG_PE1_H_PLL_BC | RG_PE1_H_PLL_BP | RG_PE1_H_PLL_IR |
+		 RG_PE1_H_PLL_IC | RG_PE1_PLL_DIVEN);
+	val |= RG_PE1_H_PLL_BC_VAL(0x02);
+	val |= RG_PE1_H_PLL_BP_VAL(0x06);
+	val |= RG_PE1_H_PLL_IR_VAL(0x02);
+	val |= RG_PE1_H_PLL_IC_VAL(0x01);
+	val |= RG_PE1_PLL_DIVEN_VAL(0x02);
+	phy_write(instance, val, RG_PE1_H_PLL_REG);
+
+	val = phy_read(instance, RG_PE1_H_PLL_BR_REG);
+	val &= ~(RG_PE1_H_PLL_BR);
+	val |= RG_PE1_H_PLL_BR_VAL(0x00);
+	phy_write(instance, val, RG_PE1_H_PLL_BR_REG);
+
+	if (reg <= 5 && reg >= 3) { /* 40MHz Xtal */
+		/* set force mode enable of da_pe1_mstckdiv */
+		val = phy_read(instance, RG_PE1_MSTCKDIV_REG);
+		val &= ~(RG_PE1_MSTCKDIV | RG_PE1_FRC_MSTCKDIV);
+		val |= (RG_PE1_MSTCKDIV_VAL(0x01) | RG_PE1_FRC_MSTCKDIV);
+		phy_write(instance, val, RG_PE1_MSTCKDIV_REG);
+	}
+}
+
+static int mt7621_pci_phy_init(struct phy *phy)
+{
+	struct mt7621_pci_phy_instance *instance = phy_get_drvdata(phy);
+	struct mt7621_pci_phy *mphy = dev_get_drvdata(phy->dev.parent);
+	u32 chip_rev_id = rt_sysc_r32(SYSC_REG_CHIP_REV);
+
+	if ((chip_rev_id & 0xFFFF) == CHIP_REV_MT7621_E2)
+		mt7621_bypass_pipe_rst(mphy, instance);
+
+	mt7621_set_phy_for_ssc(mphy, instance);
+
+	return 0;
+}
+
+static int mt7621_pci_phy_power_on(struct phy *phy)
+{
+	struct mt7621_pci_phy_instance *instance = phy_get_drvdata(phy);
+	u32 offset = (instance->index != 1) ?
+		RG_PE1_FRC_PHY_REG : RG_PE1_FRC_PHY_REG + RG_P0_TO_P1_WIDTH;
+	u32 val;
+
+	/* Enable PHY and disable force mode */
+	val = phy_read(instance, offset);
+	val &= ~(RG_PE1_FRC_PHY_EN | RG_PE1_PHY_EN);
+	val |= (RG_PE1_FRC_PHY_EN | RG_PE1_PHY_EN);
+	phy_write(instance, val, offset);
+
+	return 0;
+}
+
+static int mt7621_pci_phy_power_off(struct phy *phy)
+{
+	struct mt7621_pci_phy_instance *instance = phy_get_drvdata(phy);
+	u32 offset = (instance->index != 1) ?
+		RG_PE1_FRC_PHY_REG : RG_PE1_FRC_PHY_REG + RG_P0_TO_P1_WIDTH;
+	u32 val;
+
+	/* Disable PHY */
+	val = phy_read(instance, offset);
+	val &= ~(RG_PE1_FRC_PHY_EN | RG_PE1_PHY_EN);
+	val |= RG_PE1_FRC_PHY_EN;
+	phy_write(instance, val, offset);
+
+	return 0;
+}
+
+static int mt7621_pci_phy_exit(struct phy *phy)
+{
+	struct mt7621_pci_phy_instance *instance = phy_get_drvdata(phy);
+
+	rt_sysc_m32(PCIE_PORT_CLK_EN(instance->index), 0, RALINK_CLKCFG1);
+
+	return 0;
+}
+
+static struct phy *mt7621_pci_phy_xlate(struct device *dev,
+					struct of_phandle_args *args)
+{
+	struct mt7621_pci_phy *phy = dev_get_drvdata(dev);
+	struct mt7621_pci_phy_instance *instance = NULL;
+	struct device_node *phy_np = args->np;
+	int index;
+
+	if (args->args_count != 1) {
+		dev_err(dev, "invalid number of cells in 'phy' property\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	for (index = 0; index < phy->nphys; index++) {
+		if (phy_np == phy->phys[index]->phy->dev.of_node) {
+			instance = phy->phys[index];
+			break;
+		}
+	}
+
+	if (!instance) {
+		dev_err(dev, "failed to find appropriate phy\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	instance->type = args->args[0];
+	if (instance->type != PHY_TYPE_PCIE) {
+		dev_err(dev, "unsupported device type: %d\n", instance->type);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return instance->phy;
+}
+
+static const struct phy_ops mt7621_pci_phy_ops = {
+	.init		= mt7621_pci_phy_init,
+	.exit		= mt7621_pci_phy_exit,
+	.power_on	= mt7621_pci_phy_power_on,
+	.power_off	= mt7621_pci_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static int mt7621_pci_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct device_node *child_np;
+	struct phy_provider *provider;
+	struct mt7621_pci_phy *phy;
+	struct resource res;
+	int port, ret;
+	void __iomem *port_base;
+
+	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	phy->nphys = of_get_child_count(np);
+	phy->phys = devm_kcalloc(dev, phy->nphys,
+				 sizeof(*phy->phys), GFP_KERNEL);
+	if (!phy->phys)
+		return -ENOMEM;
+
+	phy->dev = dev;
+	platform_set_drvdata(pdev, phy);
+
+	ret = of_address_to_resource(np, 0, &res);
+	if (ret) {
+		dev_err(dev, "failed to get address resource(id-%d)\n", port);
+		return ret;
+	}
+
+	port_base = devm_ioremap_resource(dev, &res);
+	if (IS_ERR(port_base)) {
+		dev_err(dev, "failed to remap phy regs\n");
+		return PTR_ERR(port_base);
+	}
+
+	port = 0;
+	for_each_child_of_node(np, child_np) {
+		struct mt7621_pci_phy_instance *instance;
+		struct phy *pphy;
+
+		instance = devm_kzalloc(dev, sizeof(*instance), GFP_KERNEL);
+		if (!instance) {
+			ret = -ENOMEM;
+			goto put_child;
+		}
+
+		phy->phys[port] = instance;
+
+		pphy = devm_phy_create(dev, child_np, &mt7621_pci_phy_ops);
+		if (IS_ERR(phy)) {
+			dev_err(dev, "failed to create phy\n");
+			ret = PTR_ERR(phy);
+			goto put_child;
+		}
+
+		instance->port_base = port_base;
+		instance->phy = pphy;
+		instance->index = port;
+		phy_set_drvdata(pphy, instance);
+		port++;
+	}
+
+	provider = devm_of_phy_provider_register(dev, mt7621_pci_phy_xlate);
+
+	return PTR_ERR_OR_ZERO(provider);
+
+put_child:
+	of_node_put(child_np);
+	return ret;
+}
+
+static const struct of_device_id mt7621_pci_phy_ids[] = {
+	{ .compatible = "mediatek,mt7621-pci-phy" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mt7621_pci_ids);
+
+static struct platform_driver mt7621_pci_phy_driver = {
+	.probe = mt7621_pci_phy_probe,
+	.driver = {
+		.name = "mt7621-pci-phy",
+		.of_match_table = of_match_ptr(mt7621_pci_phy_ids),
+	},
+};
+
+static int __init mt7621_pci_phy_drv_init(void)
+{
+	return platform_driver_register(&mt7621_pci_phy_driver);
+}
+
+arch_initcall(mt7621_pci_phy_drv_init);
+
+MODULE_AUTHOR("Sergio Paracuellos <sergio.paracuellos@gmail.com>");
+MODULE_DESCRIPTION("MediaTek MT7621 PCI PHY driver");
+MODULE_LICENSE("GPL");
-- 
2.19.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 2/3] staging: mt7621-pci: handle phy using new generic phy 'pci-mt7621-phy' driver

[Sergio Paracuellos]

Phy part of this driver has been moved to a new 'pci-mt7621-phy' driver.
Change code to use kernel's generic phy API removing also no more need
moved code.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-pci/pci-mt7621.c | 250 +++---------------------
 1 file changed, 32 insertions(+), 218 deletions(-)

diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c
index e720524ba5a3..507b8c68d20b 100644
--- a/drivers/staging/mt7621-pci/pci-mt7621.c
+++ b/drivers/staging/mt7621-pci/pci-mt7621.c
@@ -25,6 +25,7 @@
 #include <linux/of_pci.h>
 #include <linux/of_platform.h>
 #include <linux/pci.h>
+#include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/reset.h>
 #include <mt7621.h>
@@ -95,76 +96,12 @@
 #define RALINK_PCI_IO_MAP_BASE		0x1e160000
 #define MEMORY_BASE			0x0
 
-/* pcie phy related macros */
-#define RALINK_PCIEPHY_P0P1_CTL_OFFSET	0x9000
-#define RALINK_PCIEPHY_P2_CTL_OFFSET	0xA000
-
-#define RG_P0_TO_P1_WIDTH		0x100
-
-#define RG_PE1_PIPE_REG			0x02c
-#define RG_PE1_PIPE_RST			BIT(12)
-#define RG_PE1_PIPE_CMD_FRC		BIT(4)
-
-#define RG_PE1_H_LCDDS_REG		0x49c
-#define RG_PE1_H_LCDDS_PCW		GENMASK(30, 0)
-#define RG_PE1_H_LCDDS_PCW_VAL(x)	((0x7fffffff & (x)) << 0)
-
-#define RG_PE1_FRC_H_XTAL_REG		0x400
-#define RG_PE1_FRC_H_XTAL_TYPE          BIT(8)
-#define RG_PE1_H_XTAL_TYPE              GENMASK(10, 9)
-#define RG_PE1_H_XTAL_TYPE_VAL(x)       ((0x3 & (x)) << 9)
-
-#define RG_PE1_FRC_PHY_REG		0x000
-#define RG_PE1_FRC_PHY_EN               BIT(4)
-#define RG_PE1_PHY_EN                   BIT(5)
-
-#define RG_PE1_H_PLL_REG		0x490
-#define RG_PE1_H_PLL_BC			GENMASK(23, 22)
-#define RG_PE1_H_PLL_BC_VAL(x)		((0x3 & (x)) << 22)
-#define RG_PE1_H_PLL_BP			GENMASK(21, 18)
-#define RG_PE1_H_PLL_BP_VAL(x)		((0xf & (x)) << 18)
-#define RG_PE1_H_PLL_IR			GENMASK(15, 12)
-#define RG_PE1_H_PLL_IR_VAL(x)		((0xf & (x)) << 12)
-#define RG_PE1_H_PLL_IC			GENMASK(11, 8)
-#define RG_PE1_H_PLL_IC_VAL(x)		((0xf & (x)) << 8)
-#define RG_PE1_H_PLL_PREDIV             GENMASK(7, 6)
-#define RG_PE1_H_PLL_PREDIV_VAL(x)      ((0x3 & (x)) << 6)
-#define RG_PE1_PLL_DIVEN		GENMASK(3, 1)
-#define RG_PE1_PLL_DIVEN_VAL(x)		((0x7 & (x)) << 1)
-
-#define RG_PE1_H_PLL_FBKSEL_REG		0x4bc
-#define RG_PE1_H_PLL_FBKSEL             GENMASK(5, 4)
-#define RG_PE1_H_PLL_FBKSEL_VAL(x)      ((0x3 & (x)) << 4)
-
-#define	RG_PE1_H_LCDDS_SSC_PRD_REG	0x4a4
-#define RG_PE1_H_LCDDS_SSC_PRD          GENMASK(15, 0)
-#define RG_PE1_H_LCDDS_SSC_PRD_VAL(x)   ((0xffff & (x)) << 0)
-
-#define RG_PE1_H_LCDDS_SSC_DELTA_REG	0x4a8
-#define RG_PE1_H_LCDDS_SSC_DELTA        GENMASK(11, 0)
-#define RG_PE1_H_LCDDS_SSC_DELTA_VAL(x) ((0xfff & (x)) << 0)
-#define RG_PE1_H_LCDDS_SSC_DELTA1       GENMASK(27, 16)
-#define RG_PE1_H_LCDDS_SSC_DELTA1_VAL(x) ((0xff & (x)) << 16)
-
-#define RG_PE1_LCDDS_CLK_PH_INV_REG	0x4a0
-#define RG_PE1_LCDDS_CLK_PH_INV		BIT(5)
-
-#define RG_PE1_H_PLL_BR_REG		0x4ac
-#define RG_PE1_H_PLL_BR			GENMASK(18, 16)
-#define RG_PE1_H_PLL_BR_VAL(x)		((0x7 & (x)) << 16)
-
-#define	RG_PE1_MSTCKDIV_REG		0x414
-#define RG_PE1_MSTCKDIV			GENMASK(7, 6)
-#define RG_PE1_MSTCKDIV_VAL(x)		((0x3 & (x)) << 6)
-
-#define RG_PE1_FRC_MSTCKDIV		BIT(5)
-
 /**
  * struct mt7621_pcie_port - PCIe port information
  * @base: I/O mapped register base
  * @list: port list
  * @pcie: pointer to PCIe host info
- * @phy_reg_offset: offset to related phy registers
+ * @phy: pointer to PHY control block
  * @pcie_rst: pointer to port reset control
  * @slot: port slot
  * @enabled: indicates if port is enabled
@@ -173,7 +110,7 @@ struct mt7621_pcie_port {
 	void __iomem *base;
 	struct list_head list;
 	struct mt7621_pcie *pcie;
-	u32 phy_reg_offset;
+	struct phy *phy;
 	struct reset_control *pcie_rst;
 	u32 slot;
 	bool enabled;
@@ -265,150 +202,6 @@ static void write_config(struct mt7621_pcie *pcie, unsigned int dev,
 	pcie_write(pcie, val, RALINK_PCI_CONFIG_DATA);
 }
 
-static void bypass_pipe_rst(struct mt7621_pcie_port *port)
-{
-	struct mt7621_pcie *pcie = port->pcie;
-	u32 phy_offset = port->phy_reg_offset;
-	u32 offset = (port->slot != 1) ?
-		phy_offset + RG_PE1_PIPE_REG :
-		phy_offset + RG_PE1_PIPE_REG + RG_P0_TO_P1_WIDTH;
-	u32 reg = pcie_read(pcie, offset);
-
-	reg &= ~(RG_PE1_PIPE_RST | RG_PE1_PIPE_CMD_FRC);
-	reg |= (RG_PE1_PIPE_RST | RG_PE1_PIPE_CMD_FRC);
-	pcie_write(pcie, reg, offset);
-}
-
-static void set_phy_for_ssc(struct mt7621_pcie_port *port)
-{
-	struct mt7621_pcie *pcie = port->pcie;
-	struct device *dev = pcie->dev;
-	u32 phy_offset = port->phy_reg_offset;
-	u32 reg = rt_sysc_r32(SYSC_REG_SYSTEM_CONFIG0);
-	u32 offset;
-	u32 val;
-
-	reg = (reg >> 6) & 0x7;
-	/* Set PCIe Port PHY to disable SSC */
-	/* Debug Xtal Type */
-	offset = phy_offset + RG_PE1_FRC_H_XTAL_REG;
-	val = pcie_read(pcie, offset);
-	val &= ~(RG_PE1_FRC_H_XTAL_TYPE | RG_PE1_H_XTAL_TYPE);
-	val |= RG_PE1_FRC_H_XTAL_TYPE;
-	val |= RG_PE1_H_XTAL_TYPE_VAL(0x00);
-	pcie_write(pcie, val, offset);
-
-	/* disable port */
-	offset = (port->slot != 1) ?
-		phy_offset + RG_PE1_FRC_PHY_REG :
-		phy_offset + RG_PE1_FRC_PHY_REG + RG_P0_TO_P1_WIDTH;
-	val = pcie_read(pcie, offset);
-	val &= ~(RG_PE1_FRC_PHY_EN | RG_PE1_PHY_EN);
-	val |= RG_PE1_FRC_PHY_EN;
-	pcie_write(pcie, val, offset);
-
-	/* Set Pre-divider ratio (for host mode) */
-	offset =  phy_offset + RG_PE1_H_PLL_REG;
-	val = pcie_read(pcie, offset);
-	val &= ~(RG_PE1_H_PLL_PREDIV);
-
-	if (reg <= 5 && reg >= 3) { /* 40MHz Xtal */
-		val |= RG_PE1_H_PLL_PREDIV_VAL(0x01);
-		pcie_write(pcie, val, offset);
-		dev_info(dev, "Xtal is 40MHz\n");
-	} else { /* 25MHz | 20MHz Xtal */
-		val |= RG_PE1_H_PLL_PREDIV_VAL(0x00);
-		pcie_write(pcie, val, offset);
-		if (reg >= 6) {
-			dev_info(dev, "Xtal is 25MHz\n");
-
-			/* Select feedback clock */
-			offset = phy_offset + RG_PE1_H_PLL_FBKSEL_REG;
-			val = pcie_read(pcie, offset);
-			val &= ~(RG_PE1_H_PLL_FBKSEL);
-			val |= RG_PE1_H_PLL_FBKSEL_VAL(0x01);
-			pcie_write(pcie, val, offset);
-
-			/* DDS NCPO PCW (for host mode) */
-			offset = phy_offset + RG_PE1_H_LCDDS_SSC_PRD_REG;
-			val = pcie_read(pcie, offset);
-			val &= ~(RG_PE1_H_LCDDS_SSC_PRD);
-			val |= RG_PE1_H_LCDDS_SSC_PRD_VAL(0x18000000);
-			pcie_write(pcie, val, offset);
-
-			/* DDS SSC dither period control */
-			offset = phy_offset + RG_PE1_H_LCDDS_SSC_PRD_REG;
-			val = pcie_read(pcie, offset);
-			val &= ~(RG_PE1_H_LCDDS_SSC_PRD);
-			val |= RG_PE1_H_LCDDS_SSC_PRD_VAL(0x18d);
-			pcie_write(pcie, val, offset);
-
-			/* DDS SSC dither amplitude control */
-			offset = phy_offset + RG_PE1_H_LCDDS_SSC_DELTA_REG;
-			val = pcie_read(pcie, offset);
-			val &= ~(RG_PE1_H_LCDDS_SSC_DELTA |
-				 RG_PE1_H_LCDDS_SSC_DELTA1);
-			val |= RG_PE1_H_LCDDS_SSC_DELTA_VAL(0x4a);
-			val |= RG_PE1_H_LCDDS_SSC_DELTA1_VAL(0x4a);
-			pcie_write(pcie, val, offset);
-		} else {
-			dev_info(dev, "Xtal is 20MHz\n");
-		}
-	}
-
-	/* DDS clock inversion */
-	offset = phy_offset + RG_PE1_LCDDS_CLK_PH_INV_REG;
-	val = pcie_read(pcie, offset);
-	val &= ~(RG_PE1_LCDDS_CLK_PH_INV);
-	val |= RG_PE1_LCDDS_CLK_PH_INV;
-	pcie_write(pcie, val, offset);
-
-	/* Set PLL bits */
-	offset = phy_offset + RG_PE1_H_PLL_REG;
-	val = pcie_read(pcie, offset);
-	val &= ~(RG_PE1_H_PLL_BC | RG_PE1_H_PLL_BP | RG_PE1_H_PLL_IR |
-		 RG_PE1_H_PLL_IC | RG_PE1_PLL_DIVEN);
-	val |= RG_PE1_H_PLL_BC_VAL(0x02);
-	val |= RG_PE1_H_PLL_BP_VAL(0x06);
-	val |= RG_PE1_H_PLL_IR_VAL(0x02);
-	val |= RG_PE1_H_PLL_IC_VAL(0x01);
-	val |= RG_PE1_PLL_DIVEN_VAL(0x02);
-	pcie_write(pcie, val, offset);
-
-	offset = phy_offset + RG_PE1_H_PLL_BR_REG;
-	val = pcie_read(pcie, offset);
-	val &= ~(RG_PE1_H_PLL_BR);
-	val |= RG_PE1_H_PLL_BR_VAL(0x00);
-	pcie_write(pcie, val, offset);
-
-	if (reg <= 5 && reg >= 3) { /* 40MHz Xtal */
-		/* set force mode enable of da_pe1_mstckdiv */
-		offset = phy_offset + RG_PE1_MSTCKDIV_REG;
-		val = pcie_read(pcie, offset);
-		val &= ~(RG_PE1_MSTCKDIV | RG_PE1_FRC_MSTCKDIV);
-		val |= (RG_PE1_MSTCKDIV_VAL(0x01) | RG_PE1_FRC_MSTCKDIV);
-		pcie_write(pcie, val, offset);
-	}
-
-	/* Enable PHY and disable force mode */
-	offset = (port->slot != 1) ?
-		phy_offset + RG_PE1_FRC_PHY_REG :
-		phy_offset + RG_PE1_FRC_PHY_REG + RG_P0_TO_P1_WIDTH;
-	val = pcie_read(pcie, offset);
-	val &= ~(RG_PE1_FRC_PHY_EN | RG_PE1_PHY_EN);
-	val |= (RG_PE1_FRC_PHY_EN | RG_PE1_PHY_EN);
-	pcie_write(pcie, val, offset);
-}
-
-static void mt7621_enable_phy(struct mt7621_pcie_port *port)
-{
-	u32 chip_rev_id = rt_sysc_r32(MT7621_CHIP_REV_ID);
-
-	if ((chip_rev_id & 0xFFFF) == CHIP_REV_MT7621_E2)
-		bypass_pipe_rst(port);
-	set_phy_for_ssc(port);
-}
-
 static inline void mt7621_control_assert(struct mt7621_pcie_port *port)
 {
 	u32 chip_rev_id = rt_sysc_r32(MT7621_CHIP_REV_ID);
@@ -510,7 +303,7 @@ static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
 	struct device *dev = pcie->dev;
 	struct device_node *pnode = dev->of_node;
 	struct resource regs;
-	char name[6];
+	char name[10];
 	int err;
 
 	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
@@ -534,11 +327,13 @@ static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
 		return PTR_ERR(port->pcie_rst);
 	}
 
+	snprintf(name, sizeof(name), "pcie-phy%d", slot);
+	port->phy = devm_phy_get(dev, name);
+	if (IS_ERR(port->phy))
+		return PTR_ERR(port->phy);
+
 	port->slot = slot;
 	port->pcie = pcie;
-	port->phy_reg_offset = (slot != 2) ?
-				RALINK_PCIEPHY_P0P1_CTL_OFFSET :
-				RALINK_PCIEPHY_P2_CTL_OFFSET;
 
 	INIT_LIST_HEAD(&port->list);
 	list_add_tail(&port->list, &pcie->ports);
@@ -588,6 +383,7 @@ static int mt7621_pcie_init_port(struct mt7621_pcie_port *port)
 	struct device *dev = pcie->dev;
 	u32 slot = port->slot;
 	u32 val = 0;
+	int err;
 
 	/*
 	 * Any MT7621 Ralink pcie controller that doesn't have 0x0101 at
@@ -598,18 +394,36 @@ static int mt7621_pcie_init_port(struct mt7621_pcie_port *port)
 	val = read_config(pcie, slot, PCIE_FTS_NUM);
 	dev_info(dev, "Port %d N_FTS = %x\n", (unsigned int)val, slot);
 
+	err = phy_init(port->phy);
+	if (err) {
+		dev_err(dev, "failed to initialize port%d phy\n", slot);
+		goto err_phy_init;
+	}
+
+	err = phy_power_on(port->phy);
+	if (err) {
+		dev_err(dev, "failed to power on port%d phy\n", slot);
+		goto err_phy_on;
+	}
+
 	if ((pcie_port_read(port, RALINK_PCI_STATUS) & PCIE_PORT_LINKUP) == 0) {
 		dev_err(dev, "pcie%d no card, disable it (RST & CLK)\n", slot);
 		mt7621_control_assert(port);
-		rt_sysc_m32(PCIE_PORT_CLK_EN(slot), 0, RALINK_CLKCFG1);
 		port->enabled = false;
-	} else {
-		port->enabled = true;
+		err = -ENODEV;
+		goto err_no_link_up;
 	}
 
-	mt7621_enable_phy(port);
+	port->enabled = true;
 
 	return 0;
+
+err_no_link_up:
+	phy_power_off(port->phy);
+err_phy_on:
+	phy_exit(port->phy);
+err_phy_init:
+	return err;
 }
 
 static void mt7621_pcie_init_ports(struct mt7621_pcie *pcie)
-- 
2.19.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 3/3] staging: mt7621-dts: add pci-phy related bindings to board's device tree

[Sergio Paracuellos]

New driver for pci phy has been added, as well as. pci driver has been
changed to use kernel's generic PHY API. Add related PCI PHY bindings
accordly.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-dts/mt7621.dtsi | 40 ++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi b/drivers/staging/mt7621-dts/mt7621.dtsi
index 71f069d59ad8..60ddfb7699b0 100644
--- a/drivers/staging/mt7621-dts/mt7621.dtsi
+++ b/drivers/staging/mt7621-dts/mt7621.dtsi
@@ -424,6 +424,10 @@
 		reset-names = "pcie0", "pcie1", "pcie2";
 		clocks = <&clkctrl 24 &clkctrl 25 &clkctrl 26>;
 		clock-names = "pcie0", "pcie1", "pcie2";
+		phys = <&pcie0_port PHY_TYPE_PCIE>,
+		       <&pcie1_port PHY_TYPE_PCIE>,
+		       <&pcie2_port PHY_TYPE_PCIE>;
+		phy-names = "pcie-phy0", "pcie-phy1", "pcie-phy2";
 
 		pcie@0,0 {
 			reg = <0x0000 0 0 0 0>;
@@ -449,4 +453,40 @@
 			bus-range = <0x00 0xff>;
 		};
 	};
+
+	pcie0_phy: pcie-phy@1a149000 {
+		compatible = "mediatek,mt7621-pci-phy";
+		reg = <0x1a149000 0x0700>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		ranges;
+		status = "disabled";
+
+		pcie0_port: pcie-phy@0 {
+			reg = <0>;
+			#phy-cells = <1>;
+			status = "okay";
+		};
+
+		pcie1_port: pcie-phy@1 {
+			reg = <1>;
+			#phy-cells = <1>;
+			status = "okay";
+		};
+	};
+
+	pcie1_phy: pcie-phy@1a14a000 {
+		compatible = "mediatek,mt7621-pci-phy";
+		reg = <0x1a14a000 0x0700>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		ranges;
+		status = "disabled";
+
+		pcie2_port: pcie-phy@0 {
+			reg = <0>;
+			#phy-cells = <1>;
+			status = "okay";
+		};
+	};
 };
-- 
2.19.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 0/3] staging: mt7621-pci: extract PCI PHY part into a new 'pci-mt7621-phy' driver

[Dan Carpenter]

Send a reply to the v1 thread so we don't apply that by mistake.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 0/3] staging: mt7621-pci: extract PCI PHY part into a new 'pci-mt7621-phy' driver

[Sergio Paracuellos]

Hi Dan,

On Wed, Jan 2, 2019 at 10:41 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> Send a reply to the v1 thread so we don't apply that by mistake.

Thanks for letting me know. I have just sent the mail reply to v1.

>
> regards,
> dan carpenter
>

Best regards,
    Sergio Paracuellos
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 3/3] staging: mt7621-dts: add pci-phy related bindings to board's device tree

[NeilBrown]

[-- Attachment #1.1: Type: text/plain, Size: 2585 bytes --]

On Mon, Dec 24 2018, Sergio Paracuellos wrote:

> New driver for pci phy has been added, as well as. pci driver has been
> changed to use kernel's generic PHY API. Add related PCI PHY bindings
> accordly.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  drivers/staging/mt7621-dts/mt7621.dtsi | 40 ++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi b/drivers/staging/mt7621-dts/mt7621.dtsi
> index 71f069d59ad8..60ddfb7699b0 100644
> --- a/drivers/staging/mt7621-dts/mt7621.dtsi
> +++ b/drivers/staging/mt7621-dts/mt7621.dtsi
> @@ -424,6 +424,10 @@
>  		reset-names = "pcie0", "pcie1", "pcie2";
>  		clocks = <&clkctrl 24 &clkctrl 25 &clkctrl 26>;
>  		clock-names = "pcie0", "pcie1", "pcie2";
> +		phys = <&pcie0_port PHY_TYPE_PCIE>,
> +		       <&pcie1_port PHY_TYPE_PCIE>,
> +		       <&pcie2_port PHY_TYPE_PCIE>;

This doesn't compile - PHY_TYPE_PCIE is unknown.
If I add
#include <dt-bindings/phy/phy.h>

it gets further.


> +		phy-names = "pcie-phy0", "pcie-phy1", "pcie-phy2";
>  
>  		pcie@0,0 {
>  			reg = <0x0000 0 0 0 0>;
> @@ -449,4 +453,40 @@
>  			bus-range = <0x00 0xff>;
>  		};
>  	};
> +
> +	pcie0_phy: pcie-phy@1a149000 {
> +		compatible = "mediatek,mt7621-pci-phy";
> +		reg = <0x1a149000 0x0700>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;

This causes warning
drivers/staging/mt7621-dts/gbpc1.dtb: Warning (ranges_format): /pcie-phy@1a149000:ranges: empty "ranges" property but its #size-cells (0) differs from / (1)
drivers/staging/mt7621-dts/gbpc1.dtb: Warning (ranges_format): /pcie-phy@1a14a000:ranges: empty "ranges" property but its #size-cells (0) differs from / (1)

Whether I leave it as <0>, or change it to <1>, booting results in

[    0.600602] mt7621-pci 1e140000.pcie: Parsing DT failed

I haven't tried to dig into why it fails.

Thanks,
NeilBrown


> +		ranges;
> +		status = "disabled";
> +
> +		pcie0_port: pcie-phy@0 {
> +			reg = <0>;
> +			#phy-cells = <1>;
> +			status = "okay";
> +		};
> +
> +		pcie1_port: pcie-phy@1 {
> +			reg = <1>;
> +			#phy-cells = <1>;
> +			status = "okay";
> +		};
> +	};
> +
> +	pcie1_phy: pcie-phy@1a14a000 {
> +		compatible = "mediatek,mt7621-pci-phy";
> +		reg = <0x1a14a000 0x0700>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		ranges;
> +		status = "disabled";
> +
> +		pcie2_port: pcie-phy@0 {
> +			reg = <0>;
> +			#phy-cells = <1>;
> +			status = "okay";
> +		};
> +	};
>  };
> -- 
> 2.19.1

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 1/3] staging: mt7621-pci-phy: add new driver for phy part of mt7621-pci

[NeilBrown]

[-- Attachment #1.1: Type: text/plain, Size: 3740 bytes --]

On Mon, Dec 24 2018, Sergio Paracuellos wrote:

> Phy part of the pci for this SoC can be handled using a generic phy
> driver. This commit extracts phy part of the mt7621-pci into a new
> 'mt7621-pci-phy' driver.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  drivers/staging/Kconfig                       |   2 +
>  drivers/staging/Makefile                      |   1 +
>  drivers/staging/mt7621-pci-phy/Kconfig        |   7 +
>  drivers/staging/mt7621-pci-phy/Makefile       |   1 +
>  drivers/staging/mt7621-pci-phy/TODO           |   4 +
>  .../staging/mt7621-pci-phy/pci-mt7621-phy.c   | 411 ++++++++++++++++++
>  6 files changed, 426 insertions(+)
>  create mode 100644 drivers/staging/mt7621-pci-phy/Kconfig
>  create mode 100644 drivers/staging/mt7621-pci-phy/Makefile
>  create mode 100644 drivers/staging/mt7621-pci-phy/TODO
>  create mode 100644 drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c
>
> diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
> index 7c015536360d..feec55e85347 100644
> --- a/drivers/staging/Kconfig
> +++ b/drivers/staging/Kconfig
> @@ -106,6 +106,8 @@ source "drivers/staging/pi433/Kconfig"
>  
>  source "drivers/staging/mt7621-pci/Kconfig"
>  
> +source "drivers/staging/mt7621-pci-phy/Kconfig"
> +
>  source "drivers/staging/mt7621-pinctrl/Kconfig"
>  
>  source "drivers/staging/mt7621-spi/Kconfig"
> diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
> index a79b3fe20cf0..381891186572 100644
> --- a/drivers/staging/Makefile
> +++ b/drivers/staging/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_BCM2835_VCHIQ)	+= vc04_services/
>  obj-$(CONFIG_DRM_VBOXVIDEO)	+= vboxvideo/
>  obj-$(CONFIG_PI433)		+= pi433/
>  obj-$(CONFIG_SOC_MT7621)	+= mt7621-pci/
> +obj-$(CONFIG_SOC_MT7621)	+= mt7621-pci-phy/
>  obj-$(CONFIG_SOC_MT7621)	+= mt7621-pinctrl/
>  obj-$(CONFIG_SOC_MT7621)	+= mt7621-spi/
>  obj-$(CONFIG_SOC_MT7621)	+= mt7621-dma/
> diff --git a/drivers/staging/mt7621-pci-phy/Kconfig b/drivers/staging/mt7621-pci-phy/Kconfig
> new file mode 100644
> index 000000000000..b9f6ab784ee8
> --- /dev/null
> +++ b/drivers/staging/mt7621-pci-phy/Kconfig
> @@ -0,0 +1,7 @@
> +config PCI_MT7621_PHY
> +	tristate "MediaTek MT7621 PCI PHY Driver"
> +	depends on RALINK && OF
> +	select GENERIC_PHY
> +	help
> +	  Say 'Y' here to add support for MediaTek MT7621 PCI PHY driver,
> +

I wonder if this should default to 'y' if SOC_MT7621 && PCI.

I cannot build if PCI_MT7621 is 'n' as pcibios_map_irq() is wanted but
not available.

I currently have the following patch, but maybe it should stay as
'tristate' - not sure.

Thanks,
NeilBrown

From: NeilBrown <neil@brown.name>
Date: Sun, 30 Dec 2018 20:00:57 +1100
Subject: [PATCH] staging: mt7621-pci: PCI driver is not optional when
 relevant.

The mt7621-pci driver must be enabled for SOC_MT7621 if PCI is
enabled, else a link failure results as pcibios_map_irq() is not
defined.

In any other circumstance, the driver is not relevant.
So make it non-selectable and provide an appropriate default.

Signed-off-by: NeilBrown <neil@brown.name>
---
 drivers/staging/mt7621-pci/Kconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/mt7621-pci/Kconfig b/drivers/staging/mt7621-pci/Kconfig
index d33533872a16..2d64a4823055 100644
--- a/drivers/staging/mt7621-pci/Kconfig
+++ b/drivers/staging/mt7621-pci/Kconfig
@@ -1,6 +1,5 @@
 config PCI_MT7621
-	tristate "MediaTek MT7621 PCI Controller"
-	depends on RALINK
+	def_bool y if SOC_MT7621 && PCI
 	select PCI_DRIVERS_GENERIC
 	help
 	  This selects a driver for the MediaTek MT7621 PCI Controller.
-- 
2.14.0.rc0.dirty


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 1/3] staging: mt7621-pci-phy: add new driver for phy part of mt7621-pci

[Sergio Paracuellos]

Hi Neil.

Thanks for the feedback.

On Thu, Jan 3, 2019 at 6:19 AM NeilBrown <neil@brown.name> wrote:
>
> On Mon, Dec 24 2018, Sergio Paracuellos wrote:
>
> > Phy part of the pci for this SoC can be handled using a generic phy
> > driver. This commit extracts phy part of the mt7621-pci into a new
> > 'mt7621-pci-phy' driver.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  drivers/staging/Kconfig                       |   2 +
> >  drivers/staging/Makefile                      |   1 +
> >  drivers/staging/mt7621-pci-phy/Kconfig        |   7 +
> >  drivers/staging/mt7621-pci-phy/Makefile       |   1 +
> >  drivers/staging/mt7621-pci-phy/TODO           |   4 +
> >  .../staging/mt7621-pci-phy/pci-mt7621-phy.c   | 411 ++++++++++++++++++
> >  6 files changed, 426 insertions(+)
> >  create mode 100644 drivers/staging/mt7621-pci-phy/Kconfig
> >  create mode 100644 drivers/staging/mt7621-pci-phy/Makefile
> >  create mode 100644 drivers/staging/mt7621-pci-phy/TODO
> >  create mode 100644 drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c
> >
> > diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
> > index 7c015536360d..feec55e85347 100644
> > --- a/drivers/staging/Kconfig
> > +++ b/drivers/staging/Kconfig
> > @@ -106,6 +106,8 @@ source "drivers/staging/pi433/Kconfig"
> >
> >  source "drivers/staging/mt7621-pci/Kconfig"
> >
> > +source "drivers/staging/mt7621-pci-phy/Kconfig"
> > +
> >  source "drivers/staging/mt7621-pinctrl/Kconfig"
> >
> >  source "drivers/staging/mt7621-spi/Kconfig"
> > diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
> > index a79b3fe20cf0..381891186572 100644
> > --- a/drivers/staging/Makefile
> > +++ b/drivers/staging/Makefile
> > @@ -43,6 +43,7 @@ obj-$(CONFIG_BCM2835_VCHIQ) += vc04_services/
> >  obj-$(CONFIG_DRM_VBOXVIDEO)  += vboxvideo/
> >  obj-$(CONFIG_PI433)          += pi433/
> >  obj-$(CONFIG_SOC_MT7621)     += mt7621-pci/
> > +obj-$(CONFIG_SOC_MT7621)     += mt7621-pci-phy/
> >  obj-$(CONFIG_SOC_MT7621)     += mt7621-pinctrl/
> >  obj-$(CONFIG_SOC_MT7621)     += mt7621-spi/
> >  obj-$(CONFIG_SOC_MT7621)     += mt7621-dma/
> > diff --git a/drivers/staging/mt7621-pci-phy/Kconfig b/drivers/staging/mt7621-pci-phy/Kconfig
> > new file mode 100644
> > index 000000000000..b9f6ab784ee8
> > --- /dev/null
> > +++ b/drivers/staging/mt7621-pci-phy/Kconfig
> > @@ -0,0 +1,7 @@
> > +config PCI_MT7621_PHY
> > +     tristate "MediaTek MT7621 PCI PHY Driver"
> > +     depends on RALINK && OF
> > +     select GENERIC_PHY
> > +     help
> > +       Say 'Y' here to add support for MediaTek MT7621 PCI PHY driver,
> > +
>
> I wonder if this should default to 'y' if SOC_MT7621 && PCI.
>
> I cannot build if PCI_MT7621 is 'n' as pcibios_map_irq() is wanted but
> not available.

I think we should select 'PCI_MT7621_PHY' if PCI_MT7621 is selected.

>
> I currently have the following patch, but maybe it should stay as
> 'tristate' - not sure.
>
> Thanks,
> NeilBrown
>
> From: NeilBrown <neil@brown.name>
> Date: Sun, 30 Dec 2018 20:00:57 +1100
> Subject: [PATCH] staging: mt7621-pci: PCI driver is not optional when
>  relevant.
>
> The mt7621-pci driver must be enabled for SOC_MT7621 if PCI is
> enabled, else a link failure results as pcibios_map_irq() is not
> defined.
>
> In any other circumstance, the driver is not relevant.
> So make it non-selectable and provide an appropriate default.
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
>  drivers/staging/mt7621-pci/Kconfig | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/mt7621-pci/Kconfig b/drivers/staging/mt7621-pci/Kconfig
> index d33533872a16..2d64a4823055 100644
> --- a/drivers/staging/mt7621-pci/Kconfig
> +++ b/drivers/staging/mt7621-pci/Kconfig
> @@ -1,6 +1,5 @@
>  config PCI_MT7621
> -       tristate "MediaTek MT7621 PCI Controller"
> -       depends on RALINK
> +       def_bool y if SOC_MT7621 && PCI
>         select PCI_DRIVERS_GENERIC
>         help
>           This selects a driver for the MediaTek MT7621 PCI Controller.
> --

The patch looks good to me.

> 2.14.0.rc0.dirty
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 3/3] staging: mt7621-dts: add pci-phy related bindings to board's device tree

[Sergio Paracuellos]

On Thu, Jan 3, 2019 at 6:15 AM NeilBrown <neil@brown.name> wrote:
>
> On Mon, Dec 24 2018, Sergio Paracuellos wrote:
>
> > New driver for pci phy has been added, as well as. pci driver has been
> > changed to use kernel's generic PHY API. Add related PCI PHY bindings
> > accordly.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  drivers/staging/mt7621-dts/mt7621.dtsi | 40 ++++++++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> >
> > diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi b/drivers/staging/mt7621-dts/mt7621.dtsi
> > index 71f069d59ad8..60ddfb7699b0 100644
> > --- a/drivers/staging/mt7621-dts/mt7621.dtsi
> > +++ b/drivers/staging/mt7621-dts/mt7621.dtsi
> > @@ -424,6 +424,10 @@
> >               reset-names = "pcie0", "pcie1", "pcie2";
> >               clocks = <&clkctrl 24 &clkctrl 25 &clkctrl 26>;
> >               clock-names = "pcie0", "pcie1", "pcie2";
> > +             phys = <&pcie0_port PHY_TYPE_PCIE>,
> > +                    <&pcie1_port PHY_TYPE_PCIE>,
> > +                    <&pcie2_port PHY_TYPE_PCIE>;
>
> This doesn't compile - PHY_TYPE_PCIE is unknown.
> If I add
> #include <dt-bindings/phy/phy.h>

It seems that my device tree is not being compiled with the rest of
the kernel tree which is weird, so sorry
for this. I though it was being compiling correctly.

>
> it gets further.
>
>
> > +             phy-names = "pcie-phy0", "pcie-phy1", "pcie-phy2";
> >
> >               pcie@0,0 {
> >                       reg = <0x0000 0 0 0 0>;
> > @@ -449,4 +453,40 @@
> >                       bus-range = <0x00 0xff>;
> >               };
> >       };
> > +
> > +     pcie0_phy: pcie-phy@1a149000 {
> > +             compatible = "mediatek,mt7621-pci-phy";
> > +             reg = <0x1a149000 0x0700>;
> > +             #address-cells = <1>;
> > +             #size-cells = <0>;
>
> This causes warning
> drivers/staging/mt7621-dts/gbpc1.dtb: Warning (ranges_format): /pcie-phy@1a149000:ranges: empty "ranges" property but its #size-cells (0) differs from / (1)
> drivers/staging/mt7621-dts/gbpc1.dtb: Warning (ranges_format): /pcie-phy@1a14a000:ranges: empty "ranges" property but its #size-cells (0) differs from / (1)
>
> Whether I leave it as <0>, or change it to <1>, booting results in
>
> [    0.600602] mt7621-pci 1e140000.pcie: Parsing DT failed

Uhmmm... The only place this can fail is getting the phy here (from
PATCH 2) but the code seems to be correct:

+ snprintf(name, sizeof(name), "pcie-phy%d", slot);
+ port->phy = devm_phy_get(dev, name);
+ if (IS_ERR(port->phy))
+          return PTR_ERR(port->phy);
+

So it seems this is getting wrong stuff from DT... I'll check the DT
warnings to check if could or not be related.

>
> I haven't tried to dig into why it fails.
>
> Thanks,
> NeilBrown

Thanks for testing this.

Best regards,
    Sergio Paracuellos
>
>
> > +             ranges;
> > +             status = "disabled";
> > +
> > +             pcie0_port: pcie-phy@0 {
> > +                     reg = <0>;
> > +                     #phy-cells = <1>;
> > +                     status = "okay";
> > +             };
> > +
> > +             pcie1_port: pcie-phy@1 {
> > +                     reg = <1>;
> > +                     #phy-cells = <1>;
> > +                     status = "okay";
> > +             };
> > +     };
> > +
> > +     pcie1_phy: pcie-phy@1a14a000 {
> > +             compatible = "mediatek,mt7621-pci-phy";
> > +             reg = <0x1a14a000 0x0700>;
> > +             #address-cells = <1>;
> > +             #size-cells = <0>;
> > +             ranges;
> > +             status = "disabled";
> > +
> > +             pcie2_port: pcie-phy@0 {
> > +                     reg = <0>;
> > +                     #phy-cells = <1>;
> > +                     status = "okay";
> > +             };
> > +     };
> >  };
> > --
> > 2.19.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 3/3] staging: mt7621-dts: add pci-phy related bindings to board's device tree

[Sergio Paracuellos]

Hi Neil,

On Thu, Jan 3, 2019 at 7:40 AM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> On Thu, Jan 3, 2019 at 6:15 AM NeilBrown <neil@brown.name> wrote:
> >
> > On Mon, Dec 24 2018, Sergio Paracuellos wrote:
> >
> > > New driver for pci phy has been added, as well as. pci driver has been
> > > changed to use kernel's generic PHY API. Add related PCI PHY bindings
> > > accordly.
> > >
> > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > ---
> > >  drivers/staging/mt7621-dts/mt7621.dtsi | 40 ++++++++++++++++++++++++++
> > >  1 file changed, 40 insertions(+)
> > >
> > > diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi b/drivers/staging/mt7621-dts/mt7621.dtsi
> > > index 71f069d59ad8..60ddfb7699b0 100644
> > > --- a/drivers/staging/mt7621-dts/mt7621.dtsi
> > > +++ b/drivers/staging/mt7621-dts/mt7621.dtsi
> > > @@ -424,6 +424,10 @@
> > >               reset-names = "pcie0", "pcie1", "pcie2";
> > >               clocks = <&clkctrl 24 &clkctrl 25 &clkctrl 26>;
> > >               clock-names = "pcie0", "pcie1", "pcie2";
> > > +             phys = <&pcie0_port PHY_TYPE_PCIE>,
> > > +                    <&pcie1_port PHY_TYPE_PCIE>,
> > > +                    <&pcie2_port PHY_TYPE_PCIE>;
> >
> > This doesn't compile - PHY_TYPE_PCIE is unknown.
> > If I add
> > #include <dt-bindings/phy/phy.h>
>
> It seems that my device tree is not being compiled with the rest of
> the kernel tree which is weird, so sorry
> for this. I though it was being compiling correctly.

So I fixed this. I don't know why but in some moment
'CONFIG_DTB_GNUBEE1' disappeared from my config. Now it is ok again.
Thanks and sorry for inconvenience.

>
> >
> > it gets further.
> >
> >
> > > +             phy-names = "pcie-phy0", "pcie-phy1", "pcie-phy2";
> > >
> > >               pcie@0,0 {
> > >                       reg = <0x0000 0 0 0 0>;
> > > @@ -449,4 +453,40 @@
> > >                       bus-range = <0x00 0xff>;
> > >               };
> > >       };
> > > +
> > > +     pcie0_phy: pcie-phy@1a149000 {
> > > +             compatible = "mediatek,mt7621-pci-phy";
> > > +             reg = <0x1a149000 0x0700>;
> > > +             #address-cells = <1>;
> > > +             #size-cells = <0>;
> >
> > This causes warning
> > drivers/staging/mt7621-dts/gbpc1.dtb: Warning (ranges_format): /pcie-phy@1a149000:ranges: empty "ranges" property but its #size-cells (0) differs from / (1)
> > drivers/staging/mt7621-dts/gbpc1.dtb: Warning (ranges_format): /pcie-phy@1a14a000:ranges: empty "ranges" property but its #size-cells (0) differs from / (1)

Actually I think there is no real need for 'ranges' property here. So
the following patch should cleanly compile and gets the DT for the pci
phy to a very minimum one:

diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi
b/drivers/staging/mt7621-dts/mt7621.dtsi
index a63b84015814..a2754f016b4d 100644
--- a/drivers/staging/mt7621-dts/mt7621.dtsi
+++ b/drivers/staging/mt7621-dts/mt7621.dtsi
@@ -1,4 +1,5 @@
 #include <dt-bindings/interrupt-controller/mips-gic.h>
+#include <dt-bindings/phy/phy.h>

 / {
        #address-cells = <1>;
@@ -459,19 +460,15 @@
                reg = <0x1a149000 0x0700>;
                #address-cells = <1>;
                #size-cells = <0>;
-               ranges;
-               status = "disabled";

                pcie0_port: pcie-phy@0 {
                        reg = <0>;
                        #phy-cells = <1>;
-                       status = "okay";
                };

                pcie1_port: pcie-phy@1 {
                        reg = <1>;
                        #phy-cells = <1>;
-                       status = "okay";
                };
        };

@@ -480,13 +477,10 @@
                reg = <0x1a14a000 0x0700>;
                #address-cells = <1>;
                #size-cells = <0>;
-               ranges;
-               status = "disabled";

                pcie2_port: pcie-phy@0 {
                        reg = <0>;
                        #phy-cells = <1>;
-                       status = "okay";
                };
        };

> >
> > Whether I leave it as <0>, or change it to <1>, booting results in
> >
> > [    0.600602] mt7621-pci 1e140000.pcie: Parsing DT failed
>
> Uhmmm... The only place this can fail is getting the phy here (from
> PATCH 2) but the code seems to be correct:
>
> + snprintf(name, sizeof(name), "pcie-phy%d", slot);
> + port->phy = devm_phy_get(dev, name);
> + if (IS_ERR(port->phy))
> +          return PTR_ERR(port->phy);
> +
>
> So it seems this is getting wrong stuff from DT... I'll check the DT
> warnings to check if could or not be related.

I reviewed the code and I think it should work.
Is the phy driver being loaded properly? Is any trace for the pci-phy
part in the log that put me in the way of what to check for
correctness?

Thanks in advance.

Best regards,
     Sergio Paracuellos

>
> >
> > I haven't tried to dig into why it fails.
> >
> > Thanks,
> > NeilBrown
>
> Thanks for testing this.
>
> Best regards,
>     Sergio Paracuellos
> >
> >
> > > +             ranges;
> > > +             status = "disabled";
> > > +
> > > +             pcie0_port: pcie-phy@0 {
> > > +                     reg = <0>;
> > > +                     #phy-cells = <1>;
> > > +                     status = "okay";
> > > +             };
> > > +
> > > +             pcie1_port: pcie-phy@1 {
> > > +                     reg = <1>;
> > > +                     #phy-cells = <1>;
> > > +                     status = "okay";
> > > +             };
> > > +     };
> > > +
> > > +     pcie1_phy: pcie-phy@1a14a000 {
> > > +             compatible = "mediatek,mt7621-pci-phy";
> > > +             reg = <0x1a14a000 0x0700>;
> > > +             #address-cells = <1>;
> > > +             #size-cells = <0>;
> > > +             ranges;
> > > +             status = "disabled";
> > > +
> > > +             pcie2_port: pcie-phy@0 {
> > > +                     reg = <0>;
> > > +                     #phy-cells = <1>;
> > > +                     status = "okay";
> > > +             };
> > > +     };
> > >  };
> > > --
> > > 2.19.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel