* [PATCH v2 2/2] phy: mediatek: Add PCIe PHY driver
@ 2022-03-18 9:54 ` Jianjun Wang
0 siblings, 0 replies; 52+ messages in thread
From: Jianjun Wang @ 2022-03-18 9:54 UTC (permalink / raw)
To: Chunfeng Yun, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
Matthias Brugger, Chen-Yu Tsai
Cc: Jianjun Wang, linux-arm-kernel, linux-mediatek, linux-phy,
devicetree, linux-kernel, rex-bc.chen, randy.wu, jieyy.yang,
chuanjia.liu, qizhong.cheng, jian.yang
Add PCIe GEN3 PHY driver support on MediaTek chipsets.
Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
---
drivers/phy/mediatek/Kconfig | 11 ++
drivers/phy/mediatek/Makefile | 1 +
drivers/phy/mediatek/phy-mtk-pcie.c | 246 ++++++++++++++++++++++++++++
3 files changed, 258 insertions(+)
create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
index 55f8e6c048ab..387ed1b3f2cc 100644
--- a/drivers/phy/mediatek/Kconfig
+++ b/drivers/phy/mediatek/Kconfig
@@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
select GENERIC_PHY
help
Support MIPI DSI for Mediatek SoCs.
+
+config PHY_MTK_PCIE
+ tristate "MediaTek PCIe-PHY Driver"
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ depends on OF
+ select GENERIC_PHY
+ help
+ Say 'Y' here to add support for MediaTek PCIe PHY driver.
+ This driver create the basic PHY instance and provides initialize
+ callback for PCIe GEN3 port, it supports software efuse
+ initialization.
diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
index ace660fbed3a..788c13147f63 100644
--- a/drivers/phy/mediatek/Makefile
+++ b/drivers/phy/mediatek/Makefile
@@ -6,6 +6,7 @@
obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
+obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
phy-mtk-hdmi-drv-y += phy-mtk-hdmi-mt2701.o
diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c b/drivers/phy/mediatek/phy-mtk-pcie.c
new file mode 100644
index 000000000000..0f5d7c7e2b7e
--- /dev/null
+++ b/drivers/phy/mediatek/phy-mtk-pcie.c
@@ -0,0 +1,246 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022 MediaTek Inc.
+ * Author: Jianjun Wang <jianjun.wang@mediatek.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/compiler_types.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include "phy-mtk-io.h"
+
+#define PEXTP_ANA_GLB_00_REG 0x9000
+#define PEXTP_ANA_LN0_TRX_REG 0xa000
+#define PEXTP_ANA_TX_OFFSET 0x04
+#define PEXTP_ANA_RX_OFFSET 0x3c
+#define PEXTP_ANA_LANE_OFFSET 0x100
+
+/* PEXTP_GLB_00_REG[28:24] Internal Resistor Selection of TX Bias Current */
+#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
+#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
+
+/* PEXTP_ANA_LN_RX_REG[3:0] RX impedance selection */
+#define EFUSE_LN_RX_SEL GENMASK(3, 0)
+#define EFUSE_LN_RX_VAL(x) (0xf & (x))
+
+/* PEXTP_ANA_LN_TX_REG[5:2] TX PMOS impedance selection */
+#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
+#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
+
+/* PEXTP_ANA_LN_TX_REG[11:8] TX NMOS impedance selection */
+#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
+#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
+
+/* Efuse data for each lane */
+struct mtk_pcie_lane_efuse {
+ u32 tx_pmos;
+ u32 tx_nmos;
+ u32 rx_data;
+ bool lane_efuse_supported;
+};
+
+struct mtk_pcie_phy {
+ struct device *dev;
+ struct phy *phy;
+ void __iomem *sif_base;
+
+ /*
+ * Support software efuse initialization,
+ * currently we only support 2 lane in maximum.
+ */
+ bool sw_efuse_supported;
+ u32 efuse_glb_intr;
+ struct mtk_pcie_lane_efuse efuse[2];
+};
+
+static void mtk_pcie_efuse_set_lane(struct mtk_pcie_phy *pcie_phy,
+ unsigned int lane)
+{
+ struct mtk_pcie_lane_efuse *data;
+ void __iomem *addr;
+
+ if (lane >= ARRAY_SIZE(pcie_phy->efuse)) {
+ dev_err(pcie_phy->dev,
+ "Requested lane number %d exceeds maximum %ld\n",
+ lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
+ return;
+ }
+
+ data = &pcie_phy->efuse[lane];
+ if (!data->lane_efuse_supported)
+ return;
+
+ addr = pcie_phy->sif_base + PEXTP_ANA_LN0_TRX_REG +
+ lane * PEXTP_ANA_LANE_OFFSET;
+
+ mtk_phy_update_bits(addr + PEXTP_ANA_TX_OFFSET, EFUSE_LN_TX_PMOS_SEL,
+ EFUSE_LN_TX_PMOS_VAL(data->tx_pmos));
+
+ mtk_phy_update_bits(addr + PEXTP_ANA_TX_OFFSET, EFUSE_LN_TX_NMOS_SEL,
+ EFUSE_LN_TX_NMOS_VAL(data->tx_nmos));
+
+ mtk_phy_update_bits(addr + PEXTP_ANA_RX_OFFSET, EFUSE_LN_RX_SEL,
+ EFUSE_LN_RX_VAL(data->rx_data));
+}
+
+/**
+ * mtk_pcie_phy_init() - Initialize the phy
+ * @phy: the phy to be initialized
+ *
+ * Initialize the phy by setting the efuse data.
+ * The hardware settings will be reset during suspend, it should be
+ * reinitialized when the consumer calls phy_init() again on resume.
+ */
+static int mtk_pcie_phy_init(struct phy *phy)
+{
+ struct mtk_pcie_phy *pcie_phy = phy_get_drvdata(phy);
+
+ if (!pcie_phy->sw_efuse_supported)
+ return 0;
+
+ /* Set global data */
+ mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_GLB_00_REG,
+ EFUSE_GLB_INTR_SEL,
+ EFUSE_GLB_INTR_VAL(pcie_phy->efuse_glb_intr));
+
+ mtk_pcie_efuse_set_lane(pcie_phy, 0);
+
+ mtk_pcie_efuse_set_lane(pcie_phy, 1);
+
+ return 0;
+}
+
+static const struct phy_ops mtk_pcie_phy_ops = {
+ .init = mtk_pcie_phy_init,
+ .owner = THIS_MODULE,
+};
+
+static int mtk_pcie_efuse_read_for_lane(struct mtk_pcie_phy *pcie_phy,
+ unsigned int lane)
+{
+ struct device *dev = pcie_phy->dev;
+ struct mtk_pcie_lane_efuse *data;
+ char efuse_id[15];
+ int ret;
+
+ if (lane >= ARRAY_SIZE(pcie_phy->efuse))
+ return dev_err_probe(pcie_phy->dev, -EINVAL,
+ "Requested lane number %d exceeds maximum %ld\n",
+ lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
+
+ data = &pcie_phy->efuse[lane];
+
+ snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_pmos", lane);
+ ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_pmos);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
+
+ snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_nmos", lane);
+ ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_nmos);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
+
+ snprintf(efuse_id, sizeof(efuse_id), "rx_ln%d", lane);
+ ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->rx_data);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
+
+ if (!(data->tx_pmos || data->tx_nmos || data->rx_data))
+ return dev_err_probe(dev, -EINVAL,
+ "No efuse data found for lane%d, but dts enable it\n",
+ lane);
+
+ data->lane_efuse_supported = true;
+
+ return 0;
+}
+
+static int mtk_pcie_read_efuse(struct mtk_pcie_phy *pcie_phy)
+{
+ struct device *dev = pcie_phy->dev;
+ bool nvmem_enabled;
+ int ret;
+
+ nvmem_enabled = device_property_read_bool(dev, "nvmem-cells");
+ if (!nvmem_enabled)
+ return -ENODEV;
+
+ ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr",
+ &pcie_phy->efuse_glb_intr);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to read glb_intr\n");
+
+ pcie_phy->sw_efuse_supported = true;
+
+ ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 0);
+ if (ret)
+ return ret;
+
+ ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 1);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int mtk_pcie_phy_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct phy_provider *provider;
+ struct mtk_pcie_phy *pcie_phy;
+
+ pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy), GFP_KERNEL);
+ if (!pcie_phy)
+ return -ENOMEM;
+
+ pcie_phy->dev = dev;
+
+ pcie_phy->sif_base = devm_platform_ioremap_resource_byname(pdev, "sif");
+ if (IS_ERR(pcie_phy->sif_base))
+ return dev_err_probe(dev, PTR_ERR(pcie_phy->sif_base),
+ "Failed to map phy-sif base\n");
+
+ pcie_phy->phy = devm_phy_create(dev, dev->of_node, &mtk_pcie_phy_ops);
+ if (IS_ERR(pcie_phy->phy))
+ return dev_err_probe(dev, PTR_ERR(pcie_phy->phy),
+ "Failed to create PCIe phy\n");
+
+ /*
+ * Failed to read the efuse data is not a fatal problem,
+ * ignore the failure and keep going.
+ */
+ mtk_pcie_read_efuse(pcie_phy);
+
+ phy_set_drvdata(pcie_phy->phy, pcie_phy);
+
+ provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+ if (IS_ERR(provider))
+ return dev_err_probe(dev, PTR_ERR(provider),
+ "PCIe phy probe failed\n");
+
+ return 0;
+}
+
+static const struct of_device_id mtk_pcie_phy_of_match[] = {
+ { .compatible = "mediatek,mt8195-pcie-phy" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, mtk_pcie_phy_of_match);
+
+static struct platform_driver mtk_pcie_phy_driver = {
+ .probe = mtk_pcie_phy_probe,
+ .driver = {
+ .name = "mtk-pcie-phy",
+ .of_match_table = mtk_pcie_phy_of_match,
+ },
+};
+module_platform_driver(mtk_pcie_phy_driver);
+
+MODULE_DESCRIPTION("MediaTek PCIe PHY driver");
+MODULE_AUTHOR("Jianjun Wang <jianjun.wang@mediatek.com>");
+MODULE_LICENSE("GPL v2");
--
2.18.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH v2 2/2] phy: mediatek: Add PCIe PHY driver
@ 2022-03-18 9:54 ` Jianjun Wang
0 siblings, 0 replies; 52+ messages in thread
From: Jianjun Wang @ 2022-03-18 9:54 UTC (permalink / raw)
To: Chunfeng Yun, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
Matthias Brugger, Chen-Yu Tsai
Cc: Jianjun Wang, linux-arm-kernel, linux-mediatek, linux-phy,
devicetree, linux-kernel, rex-bc.chen, randy.wu, jieyy.yang,
chuanjia.liu, qizhong.cheng, jian.yang
Add PCIe GEN3 PHY driver support on MediaTek chipsets.
Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
---
drivers/phy/mediatek/Kconfig | 11 ++
drivers/phy/mediatek/Makefile | 1 +
drivers/phy/mediatek/phy-mtk-pcie.c | 246 ++++++++++++++++++++++++++++
3 files changed, 258 insertions(+)
create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
index 55f8e6c048ab..387ed1b3f2cc 100644
--- a/drivers/phy/mediatek/Kconfig
+++ b/drivers/phy/mediatek/Kconfig
@@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
select GENERIC_PHY
help
Support MIPI DSI for Mediatek SoCs.
+
+config PHY_MTK_PCIE
+ tristate "MediaTek PCIe-PHY Driver"
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ depends on OF
+ select GENERIC_PHY
+ help
+ Say 'Y' here to add support for MediaTek PCIe PHY driver.
+ This driver create the basic PHY instance and provides initialize
+ callback for PCIe GEN3 port, it supports software efuse
+ initialization.
diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
index ace660fbed3a..788c13147f63 100644
--- a/drivers/phy/mediatek/Makefile
+++ b/drivers/phy/mediatek/Makefile
@@ -6,6 +6,7 @@
obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
+obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
phy-mtk-hdmi-drv-y += phy-mtk-hdmi-mt2701.o
diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c b/drivers/phy/mediatek/phy-mtk-pcie.c
new file mode 100644
index 000000000000..0f5d7c7e2b7e
--- /dev/null
+++ b/drivers/phy/mediatek/phy-mtk-pcie.c
@@ -0,0 +1,246 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022 MediaTek Inc.
+ * Author: Jianjun Wang <jianjun.wang@mediatek.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/compiler_types.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include "phy-mtk-io.h"
+
+#define PEXTP_ANA_GLB_00_REG 0x9000
+#define PEXTP_ANA_LN0_TRX_REG 0xa000
+#define PEXTP_ANA_TX_OFFSET 0x04
+#define PEXTP_ANA_RX_OFFSET 0x3c
+#define PEXTP_ANA_LANE_OFFSET 0x100
+
+/* PEXTP_GLB_00_REG[28:24] Internal Resistor Selection of TX Bias Current */
+#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
+#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
+
+/* PEXTP_ANA_LN_RX_REG[3:0] RX impedance selection */
+#define EFUSE_LN_RX_SEL GENMASK(3, 0)
+#define EFUSE_LN_RX_VAL(x) (0xf & (x))
+
+/* PEXTP_ANA_LN_TX_REG[5:2] TX PMOS impedance selection */
+#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
+#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
+
+/* PEXTP_ANA_LN_TX_REG[11:8] TX NMOS impedance selection */
+#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
+#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
+
+/* Efuse data for each lane */
+struct mtk_pcie_lane_efuse {
+ u32 tx_pmos;
+ u32 tx_nmos;
+ u32 rx_data;
+ bool lane_efuse_supported;
+};
+
+struct mtk_pcie_phy {
+ struct device *dev;
+ struct phy *phy;
+ void __iomem *sif_base;
+
+ /*
+ * Support software efuse initialization,
+ * currently we only support 2 lane in maximum.
+ */
+ bool sw_efuse_supported;
+ u32 efuse_glb_intr;
+ struct mtk_pcie_lane_efuse efuse[2];
+};
+
+static void mtk_pcie_efuse_set_lane(struct mtk_pcie_phy *pcie_phy,
+ unsigned int lane)
+{
+ struct mtk_pcie_lane_efuse *data;
+ void __iomem *addr;
+
+ if (lane >= ARRAY_SIZE(pcie_phy->efuse)) {
+ dev_err(pcie_phy->dev,
+ "Requested lane number %d exceeds maximum %ld\n",
+ lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
+ return;
+ }
+
+ data = &pcie_phy->efuse[lane];
+ if (!data->lane_efuse_supported)
+ return;
+
+ addr = pcie_phy->sif_base + PEXTP_ANA_LN0_TRX_REG +
+ lane * PEXTP_ANA_LANE_OFFSET;
+
+ mtk_phy_update_bits(addr + PEXTP_ANA_TX_OFFSET, EFUSE_LN_TX_PMOS_SEL,
+ EFUSE_LN_TX_PMOS_VAL(data->tx_pmos));
+
+ mtk_phy_update_bits(addr + PEXTP_ANA_TX_OFFSET, EFUSE_LN_TX_NMOS_SEL,
+ EFUSE_LN_TX_NMOS_VAL(data->tx_nmos));
+
+ mtk_phy_update_bits(addr + PEXTP_ANA_RX_OFFSET, EFUSE_LN_RX_SEL,
+ EFUSE_LN_RX_VAL(data->rx_data));
+}
+
+/**
+ * mtk_pcie_phy_init() - Initialize the phy
+ * @phy: the phy to be initialized
+ *
+ * Initialize the phy by setting the efuse data.
+ * The hardware settings will be reset during suspend, it should be
+ * reinitialized when the consumer calls phy_init() again on resume.
+ */
+static int mtk_pcie_phy_init(struct phy *phy)
+{
+ struct mtk_pcie_phy *pcie_phy = phy_get_drvdata(phy);
+
+ if (!pcie_phy->sw_efuse_supported)
+ return 0;
+
+ /* Set global data */
+ mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_GLB_00_REG,
+ EFUSE_GLB_INTR_SEL,
+ EFUSE_GLB_INTR_VAL(pcie_phy->efuse_glb_intr));
+
+ mtk_pcie_efuse_set_lane(pcie_phy, 0);
+
+ mtk_pcie_efuse_set_lane(pcie_phy, 1);
+
+ return 0;
+}
+
+static const struct phy_ops mtk_pcie_phy_ops = {
+ .init = mtk_pcie_phy_init,
+ .owner = THIS_MODULE,
+};
+
+static int mtk_pcie_efuse_read_for_lane(struct mtk_pcie_phy *pcie_phy,
+ unsigned int lane)
+{
+ struct device *dev = pcie_phy->dev;
+ struct mtk_pcie_lane_efuse *data;
+ char efuse_id[15];
+ int ret;
+
+ if (lane >= ARRAY_SIZE(pcie_phy->efuse))
+ return dev_err_probe(pcie_phy->dev, -EINVAL,
+ "Requested lane number %d exceeds maximum %ld\n",
+ lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
+
+ data = &pcie_phy->efuse[lane];
+
+ snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_pmos", lane);
+ ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_pmos);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
+
+ snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_nmos", lane);
+ ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_nmos);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
+
+ snprintf(efuse_id, sizeof(efuse_id), "rx_ln%d", lane);
+ ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->rx_data);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
+
+ if (!(data->tx_pmos || data->tx_nmos || data->rx_data))
+ return dev_err_probe(dev, -EINVAL,
+ "No efuse data found for lane%d, but dts enable it\n",
+ lane);
+
+ data->lane_efuse_supported = true;
+
+ return 0;
+}
+
+static int mtk_pcie_read_efuse(struct mtk_pcie_phy *pcie_phy)
+{
+ struct device *dev = pcie_phy->dev;
+ bool nvmem_enabled;
+ int ret;
+
+ nvmem_enabled = device_property_read_bool(dev, "nvmem-cells");
+ if (!nvmem_enabled)
+ return -ENODEV;
+
+ ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr",
+ &pcie_phy->efuse_glb_intr);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to read glb_intr\n");
+
+ pcie_phy->sw_efuse_supported = true;
+
+ ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 0);
+ if (ret)
+ return ret;
+
+ ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 1);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int mtk_pcie_phy_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct phy_provider *provider;
+ struct mtk_pcie_phy *pcie_phy;
+
+ pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy), GFP_KERNEL);
+ if (!pcie_phy)
+ return -ENOMEM;
+
+ pcie_phy->dev = dev;
+
+ pcie_phy->sif_base = devm_platform_ioremap_resource_byname(pdev, "sif");
+ if (IS_ERR(pcie_phy->sif_base))
+ return dev_err_probe(dev, PTR_ERR(pcie_phy->sif_base),
+ "Failed to map phy-sif base\n");
+
+ pcie_phy->phy = devm_phy_create(dev, dev->of_node, &mtk_pcie_phy_ops);
+ if (IS_ERR(pcie_phy->phy))
+ return dev_err_probe(dev, PTR_ERR(pcie_phy->phy),
+ "Failed to create PCIe phy\n");
+
+ /*
+ * Failed to read the efuse data is not a fatal problem,
+ * ignore the failure and keep going.
+ */
+ mtk_pcie_read_efuse(pcie_phy);
+
+ phy_set_drvdata(pcie_phy->phy, pcie_phy);
+
+ provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+ if (IS_ERR(provider))
+ return dev_err_probe(dev, PTR_ERR(provider),
+ "PCIe phy probe failed\n");
+
+ return 0;
+}
+
+static const struct of_device_id mtk_pcie_phy_of_match[] = {
+ { .compatible = "mediatek,mt8195-pcie-phy" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, mtk_pcie_phy_of_match);
+
+static struct platform_driver mtk_pcie_phy_driver = {
+ .probe = mtk_pcie_phy_probe,
+ .driver = {
+ .name = "mtk-pcie-phy",
+ .of_match_table = mtk_pcie_phy_of_match,
+ },
+};
+module_platform_driver(mtk_pcie_phy_driver);
+
+MODULE_DESCRIPTION("MediaTek PCIe PHY driver");
+MODULE_AUTHOR("Jianjun Wang <jianjun.wang@mediatek.com>");
+MODULE_LICENSE("GPL v2");
--
2.18.0
_______________________________________________
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] 52+ messages in thread* [PATCH v2 2/2] phy: mediatek: Add PCIe PHY driver
@ 2022-03-18 9:54 ` Jianjun Wang
0 siblings, 0 replies; 52+ messages in thread
From: Jianjun Wang @ 2022-03-18 9:54 UTC (permalink / raw)
To: Chunfeng Yun, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
Matthias Brugger, Chen-Yu Tsai
Cc: Jianjun Wang, linux-arm-kernel, linux-mediatek, linux-phy,
devicetree, linux-kernel, rex-bc.chen, randy.wu, jieyy.yang,
chuanjia.liu, qizhong.cheng, jian.yang
Add PCIe GEN3 PHY driver support on MediaTek chipsets.
Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
---
drivers/phy/mediatek/Kconfig | 11 ++
drivers/phy/mediatek/Makefile | 1 +
drivers/phy/mediatek/phy-mtk-pcie.c | 246 ++++++++++++++++++++++++++++
3 files changed, 258 insertions(+)
create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
index 55f8e6c048ab..387ed1b3f2cc 100644
--- a/drivers/phy/mediatek/Kconfig
+++ b/drivers/phy/mediatek/Kconfig
@@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
select GENERIC_PHY
help
Support MIPI DSI for Mediatek SoCs.
+
+config PHY_MTK_PCIE
+ tristate "MediaTek PCIe-PHY Driver"
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ depends on OF
+ select GENERIC_PHY
+ help
+ Say 'Y' here to add support for MediaTek PCIe PHY driver.
+ This driver create the basic PHY instance and provides initialize
+ callback for PCIe GEN3 port, it supports software efuse
+ initialization.
diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
index ace660fbed3a..788c13147f63 100644
--- a/drivers/phy/mediatek/Makefile
+++ b/drivers/phy/mediatek/Makefile
@@ -6,6 +6,7 @@
obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
+obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
phy-mtk-hdmi-drv-y += phy-mtk-hdmi-mt2701.o
diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c b/drivers/phy/mediatek/phy-mtk-pcie.c
new file mode 100644
index 000000000000..0f5d7c7e2b7e
--- /dev/null
+++ b/drivers/phy/mediatek/phy-mtk-pcie.c
@@ -0,0 +1,246 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022 MediaTek Inc.
+ * Author: Jianjun Wang <jianjun.wang@mediatek.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/compiler_types.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include "phy-mtk-io.h"
+
+#define PEXTP_ANA_GLB_00_REG 0x9000
+#define PEXTP_ANA_LN0_TRX_REG 0xa000
+#define PEXTP_ANA_TX_OFFSET 0x04
+#define PEXTP_ANA_RX_OFFSET 0x3c
+#define PEXTP_ANA_LANE_OFFSET 0x100
+
+/* PEXTP_GLB_00_REG[28:24] Internal Resistor Selection of TX Bias Current */
+#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
+#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
+
+/* PEXTP_ANA_LN_RX_REG[3:0] RX impedance selection */
+#define EFUSE_LN_RX_SEL GENMASK(3, 0)
+#define EFUSE_LN_RX_VAL(x) (0xf & (x))
+
+/* PEXTP_ANA_LN_TX_REG[5:2] TX PMOS impedance selection */
+#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
+#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
+
+/* PEXTP_ANA_LN_TX_REG[11:8] TX NMOS impedance selection */
+#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
+#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
+
+/* Efuse data for each lane */
+struct mtk_pcie_lane_efuse {
+ u32 tx_pmos;
+ u32 tx_nmos;
+ u32 rx_data;
+ bool lane_efuse_supported;
+};
+
+struct mtk_pcie_phy {
+ struct device *dev;
+ struct phy *phy;
+ void __iomem *sif_base;
+
+ /*
+ * Support software efuse initialization,
+ * currently we only support 2 lane in maximum.
+ */
+ bool sw_efuse_supported;
+ u32 efuse_glb_intr;
+ struct mtk_pcie_lane_efuse efuse[2];
+};
+
+static void mtk_pcie_efuse_set_lane(struct mtk_pcie_phy *pcie_phy,
+ unsigned int lane)
+{
+ struct mtk_pcie_lane_efuse *data;
+ void __iomem *addr;
+
+ if (lane >= ARRAY_SIZE(pcie_phy->efuse)) {
+ dev_err(pcie_phy->dev,
+ "Requested lane number %d exceeds maximum %ld\n",
+ lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
+ return;
+ }
+
+ data = &pcie_phy->efuse[lane];
+ if (!data->lane_efuse_supported)
+ return;
+
+ addr = pcie_phy->sif_base + PEXTP_ANA_LN0_TRX_REG +
+ lane * PEXTP_ANA_LANE_OFFSET;
+
+ mtk_phy_update_bits(addr + PEXTP_ANA_TX_OFFSET, EFUSE_LN_TX_PMOS_SEL,
+ EFUSE_LN_TX_PMOS_VAL(data->tx_pmos));
+
+ mtk_phy_update_bits(addr + PEXTP_ANA_TX_OFFSET, EFUSE_LN_TX_NMOS_SEL,
+ EFUSE_LN_TX_NMOS_VAL(data->tx_nmos));
+
+ mtk_phy_update_bits(addr + PEXTP_ANA_RX_OFFSET, EFUSE_LN_RX_SEL,
+ EFUSE_LN_RX_VAL(data->rx_data));
+}
+
+/**
+ * mtk_pcie_phy_init() - Initialize the phy
+ * @phy: the phy to be initialized
+ *
+ * Initialize the phy by setting the efuse data.
+ * The hardware settings will be reset during suspend, it should be
+ * reinitialized when the consumer calls phy_init() again on resume.
+ */
+static int mtk_pcie_phy_init(struct phy *phy)
+{
+ struct mtk_pcie_phy *pcie_phy = phy_get_drvdata(phy);
+
+ if (!pcie_phy->sw_efuse_supported)
+ return 0;
+
+ /* Set global data */
+ mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_GLB_00_REG,
+ EFUSE_GLB_INTR_SEL,
+ EFUSE_GLB_INTR_VAL(pcie_phy->efuse_glb_intr));
+
+ mtk_pcie_efuse_set_lane(pcie_phy, 0);
+
+ mtk_pcie_efuse_set_lane(pcie_phy, 1);
+
+ return 0;
+}
+
+static const struct phy_ops mtk_pcie_phy_ops = {
+ .init = mtk_pcie_phy_init,
+ .owner = THIS_MODULE,
+};
+
+static int mtk_pcie_efuse_read_for_lane(struct mtk_pcie_phy *pcie_phy,
+ unsigned int lane)
+{
+ struct device *dev = pcie_phy->dev;
+ struct mtk_pcie_lane_efuse *data;
+ char efuse_id[15];
+ int ret;
+
+ if (lane >= ARRAY_SIZE(pcie_phy->efuse))
+ return dev_err_probe(pcie_phy->dev, -EINVAL,
+ "Requested lane number %d exceeds maximum %ld\n",
+ lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
+
+ data = &pcie_phy->efuse[lane];
+
+ snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_pmos", lane);
+ ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_pmos);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
+
+ snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_nmos", lane);
+ ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_nmos);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
+
+ snprintf(efuse_id, sizeof(efuse_id), "rx_ln%d", lane);
+ ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->rx_data);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
+
+ if (!(data->tx_pmos || data->tx_nmos || data->rx_data))
+ return dev_err_probe(dev, -EINVAL,
+ "No efuse data found for lane%d, but dts enable it\n",
+ lane);
+
+ data->lane_efuse_supported = true;
+
+ return 0;
+}
+
+static int mtk_pcie_read_efuse(struct mtk_pcie_phy *pcie_phy)
+{
+ struct device *dev = pcie_phy->dev;
+ bool nvmem_enabled;
+ int ret;
+
+ nvmem_enabled = device_property_read_bool(dev, "nvmem-cells");
+ if (!nvmem_enabled)
+ return -ENODEV;
+
+ ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr",
+ &pcie_phy->efuse_glb_intr);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to read glb_intr\n");
+
+ pcie_phy->sw_efuse_supported = true;
+
+ ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 0);
+ if (ret)
+ return ret;
+
+ ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 1);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int mtk_pcie_phy_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct phy_provider *provider;
+ struct mtk_pcie_phy *pcie_phy;
+
+ pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy), GFP_KERNEL);
+ if (!pcie_phy)
+ return -ENOMEM;
+
+ pcie_phy->dev = dev;
+
+ pcie_phy->sif_base = devm_platform_ioremap_resource_byname(pdev, "sif");
+ if (IS_ERR(pcie_phy->sif_base))
+ return dev_err_probe(dev, PTR_ERR(pcie_phy->sif_base),
+ "Failed to map phy-sif base\n");
+
+ pcie_phy->phy = devm_phy_create(dev, dev->of_node, &mtk_pcie_phy_ops);
+ if (IS_ERR(pcie_phy->phy))
+ return dev_err_probe(dev, PTR_ERR(pcie_phy->phy),
+ "Failed to create PCIe phy\n");
+
+ /*
+ * Failed to read the efuse data is not a fatal problem,
+ * ignore the failure and keep going.
+ */
+ mtk_pcie_read_efuse(pcie_phy);
+
+ phy_set_drvdata(pcie_phy->phy, pcie_phy);
+
+ provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+ if (IS_ERR(provider))
+ return dev_err_probe(dev, PTR_ERR(provider),
+ "PCIe phy probe failed\n");
+
+ return 0;
+}
+
+static const struct of_device_id mtk_pcie_phy_of_match[] = {
+ { .compatible = "mediatek,mt8195-pcie-phy" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, mtk_pcie_phy_of_match);
+
+static struct platform_driver mtk_pcie_phy_driver = {
+ .probe = mtk_pcie_phy_probe,
+ .driver = {
+ .name = "mtk-pcie-phy",
+ .of_match_table = mtk_pcie_phy_of_match,
+ },
+};
+module_platform_driver(mtk_pcie_phy_driver);
+
+MODULE_DESCRIPTION("MediaTek PCIe PHY driver");
+MODULE_AUTHOR("Jianjun Wang <jianjun.wang@mediatek.com>");
+MODULE_LICENSE("GPL v2");
--
2.18.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [PATCH v2 2/2] phy: mediatek: Add PCIe PHY driver
2022-03-18 9:54 ` Jianjun Wang
(?)
(?)
@ 2022-03-18 9:59 ` Chen-Yu Tsai
-1 siblings, 0 replies; 52+ messages in thread
From: Chen-Yu Tsai @ 2022-03-18 9:59 UTC (permalink / raw)
To: Jianjun Wang
Cc: Chunfeng Yun, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
Matthias Brugger, linux-arm-kernel, linux-mediatek, linux-phy,
devicetree, linux-kernel, rex-bc.chen, Randy.Wu, jieyy.yang,
chuanjia.liu, qizhong.cheng, jian.yang
On Fri, Mar 18, 2022 at 5:54 PM Jianjun Wang <jianjun.wang@mediatek.com> wrote:
>
> Add PCIe GEN3 PHY driver support on MediaTek chipsets.
>
> Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> ---
> drivers/phy/mediatek/Kconfig | 11 ++
> drivers/phy/mediatek/Makefile | 1 +
> drivers/phy/mediatek/phy-mtk-pcie.c | 246 ++++++++++++++++++++++++++++
> 3 files changed, 258 insertions(+)
> create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
>
> diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
> index 55f8e6c048ab..387ed1b3f2cc 100644
> --- a/drivers/phy/mediatek/Kconfig
> +++ b/drivers/phy/mediatek/Kconfig
> @@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
> select GENERIC_PHY
> help
> Support MIPI DSI for Mediatek SoCs.
> +
> +config PHY_MTK_PCIE
> + tristate "MediaTek PCIe-PHY Driver"
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + depends on OF
> + select GENERIC_PHY
> + help
> + Say 'Y' here to add support for MediaTek PCIe PHY driver.
> + This driver create the basic PHY instance and provides initialize
> + callback for PCIe GEN3 port, it supports software efuse
> + initialization.
> diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
> index ace660fbed3a..788c13147f63 100644
> --- a/drivers/phy/mediatek/Makefile
> +++ b/drivers/phy/mediatek/Makefile
> @@ -6,6 +6,7 @@
> obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
> obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
> obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
> +obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
>
> phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
> phy-mtk-hdmi-drv-y += phy-mtk-hdmi-mt2701.o
> diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c b/drivers/phy/mediatek/phy-mtk-pcie.c
> new file mode 100644
> index 000000000000..0f5d7c7e2b7e
> --- /dev/null
> +++ b/drivers/phy/mediatek/phy-mtk-pcie.c
> @@ -0,0 +1,246 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 MediaTek Inc.
> + * Author: Jianjun Wang <jianjun.wang@mediatek.com>
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/compiler_types.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "phy-mtk-io.h"
> +
> +#define PEXTP_ANA_GLB_00_REG 0x9000
> +#define PEXTP_ANA_LN0_TRX_REG 0xa000
> +#define PEXTP_ANA_TX_OFFSET 0x04
> +#define PEXTP_ANA_RX_OFFSET 0x3c
> +#define PEXTP_ANA_LANE_OFFSET 0x100
> +
> +/* PEXTP_GLB_00_REG[28:24] Internal Resistor Selection of TX Bias Current */
> +#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
> +#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
> +
> +/* PEXTP_ANA_LN_RX_REG[3:0] RX impedance selection */
> +#define EFUSE_LN_RX_SEL GENMASK(3, 0)
> +#define EFUSE_LN_RX_VAL(x) (0xf & (x))
> +
> +/* PEXTP_ANA_LN_TX_REG[5:2] TX PMOS impedance selection */
> +#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
> +#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
> +
> +/* PEXTP_ANA_LN_TX_REG[11:8] TX NMOS impedance selection */
> +#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
> +#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
> +
> +/* Efuse data for each lane */
> +struct mtk_pcie_lane_efuse {
> + u32 tx_pmos;
> + u32 tx_nmos;
> + u32 rx_data;
> + bool lane_efuse_supported;
> +};
> +
> +struct mtk_pcie_phy {
> + struct device *dev;
> + struct phy *phy;
> + void __iomem *sif_base;
> +
> + /*
> + * Support software efuse initialization,
> + * currently we only support 2 lane in maximum.
> + */
> + bool sw_efuse_supported;
> + u32 efuse_glb_intr;
> + struct mtk_pcie_lane_efuse efuse[2];
> +};
> +
> +static void mtk_pcie_efuse_set_lane(struct mtk_pcie_phy *pcie_phy,
> + unsigned int lane)
> +{
> + struct mtk_pcie_lane_efuse *data;
> + void __iomem *addr;
> +
> + if (lane >= ARRAY_SIZE(pcie_phy->efuse)) {
> + dev_err(pcie_phy->dev,
> + "Requested lane number %d exceeds maximum %ld\n",
> + lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
> + return;
> + }
> +
> + data = &pcie_phy->efuse[lane];
> + if (!data->lane_efuse_supported)
> + return;
> +
> + addr = pcie_phy->sif_base + PEXTP_ANA_LN0_TRX_REG +
> + lane * PEXTP_ANA_LANE_OFFSET;
> +
> + mtk_phy_update_bits(addr + PEXTP_ANA_TX_OFFSET, EFUSE_LN_TX_PMOS_SEL,
> + EFUSE_LN_TX_PMOS_VAL(data->tx_pmos));
> +
> + mtk_phy_update_bits(addr + PEXTP_ANA_TX_OFFSET, EFUSE_LN_TX_NMOS_SEL,
> + EFUSE_LN_TX_NMOS_VAL(data->tx_nmos));
> +
> + mtk_phy_update_bits(addr + PEXTP_ANA_RX_OFFSET, EFUSE_LN_RX_SEL,
> + EFUSE_LN_RX_VAL(data->rx_data));
> +}
> +
> +/**
> + * mtk_pcie_phy_init() - Initialize the phy
> + * @phy: the phy to be initialized
> + *
> + * Initialize the phy by setting the efuse data.
> + * The hardware settings will be reset during suspend, it should be
> + * reinitialized when the consumer calls phy_init() again on resume.
> + */
> +static int mtk_pcie_phy_init(struct phy *phy)
> +{
> + struct mtk_pcie_phy *pcie_phy = phy_get_drvdata(phy);
> +
> + if (!pcie_phy->sw_efuse_supported)
> + return 0;
> +
> + /* Set global data */
> + mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_GLB_00_REG,
> + EFUSE_GLB_INTR_SEL,
> + EFUSE_GLB_INTR_VAL(pcie_phy->efuse_glb_intr));
> +
> + mtk_pcie_efuse_set_lane(pcie_phy, 0);
> +
> + mtk_pcie_efuse_set_lane(pcie_phy, 1);
> +
> + return 0;
> +}
> +
> +static const struct phy_ops mtk_pcie_phy_ops = {
> + .init = mtk_pcie_phy_init,
> + .owner = THIS_MODULE,
> +};
> +
> +static int mtk_pcie_efuse_read_for_lane(struct mtk_pcie_phy *pcie_phy,
> + unsigned int lane)
> +{
> + struct device *dev = pcie_phy->dev;
> + struct mtk_pcie_lane_efuse *data;
> + char efuse_id[15];
> + int ret;
> +
> + if (lane >= ARRAY_SIZE(pcie_phy->efuse))
> + return dev_err_probe(pcie_phy->dev, -EINVAL,
> + "Requested lane number %d exceeds maximum %ld\n",
> + lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
> +
> + data = &pcie_phy->efuse[lane];
> +
> + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_pmos", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_pmos);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_nmos", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_nmos);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + snprintf(efuse_id, sizeof(efuse_id), "rx_ln%d", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->rx_data);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + if (!(data->tx_pmos || data->tx_nmos || data->rx_data))
> + return dev_err_probe(dev, -EINVAL,
> + "No efuse data found for lane%d, but dts enable it\n",
> + lane);
> +
> + data->lane_efuse_supported = true;
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_read_efuse(struct mtk_pcie_phy *pcie_phy)
> +{
> + struct device *dev = pcie_phy->dev;
> + bool nvmem_enabled;
> + int ret;
> +
> + nvmem_enabled = device_property_read_bool(dev, "nvmem-cells");
> + if (!nvmem_enabled)
> + return -ENODEV;
> +
> + ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr",
> + &pcie_phy->efuse_glb_intr);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read glb_intr\n");
> +
> + pcie_phy->sw_efuse_supported = true;
> +
> + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 0);
> + if (ret)
> + return ret;
> +
> + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 1);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_phy_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct phy_provider *provider;
> + struct mtk_pcie_phy *pcie_phy;
> +
> + pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy), GFP_KERNEL);
> + if (!pcie_phy)
> + return -ENOMEM;
> +
> + pcie_phy->dev = dev;
> +
> + pcie_phy->sif_base = devm_platform_ioremap_resource_byname(pdev, "sif");
> + if (IS_ERR(pcie_phy->sif_base))
> + return dev_err_probe(dev, PTR_ERR(pcie_phy->sif_base),
> + "Failed to map phy-sif base\n");
> +
> + pcie_phy->phy = devm_phy_create(dev, dev->of_node, &mtk_pcie_phy_ops);
> + if (IS_ERR(pcie_phy->phy))
> + return dev_err_probe(dev, PTR_ERR(pcie_phy->phy),
> + "Failed to create PCIe phy\n");
> +
> + /*
> + * Failed to read the efuse data is not a fatal problem,
> + * ignore the failure and keep going.
> + */
> + mtk_pcie_read_efuse(pcie_phy);
You need to check the return value for -EPROBE_DEFER to defer probing.
> +
> + phy_set_drvdata(pcie_phy->phy, pcie_phy);
> +
> + provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> + if (IS_ERR(provider))
> + return dev_err_probe(dev, PTR_ERR(provider),
> + "PCIe phy probe failed\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mtk_pcie_phy_of_match[] = {
> + { .compatible = "mediatek,mt8195-pcie-phy" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, mtk_pcie_phy_of_match);
> +
> +static struct platform_driver mtk_pcie_phy_driver = {
> + .probe = mtk_pcie_phy_probe,
> + .driver = {
> + .name = "mtk-pcie-phy",
> + .of_match_table = mtk_pcie_phy_of_match,
> + },
> +};
> +module_platform_driver(mtk_pcie_phy_driver);
> +
> +MODULE_DESCRIPTION("MediaTek PCIe PHY driver");
> +MODULE_AUTHOR("Jianjun Wang <jianjun.wang@mediatek.com>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.18.0
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH v2 2/2] phy: mediatek: Add PCIe PHY driver
@ 2022-03-18 9:59 ` Chen-Yu Tsai
0 siblings, 0 replies; 52+ messages in thread
From: Chen-Yu Tsai @ 2022-03-18 9:59 UTC (permalink / raw)
To: Jianjun Wang
Cc: Chunfeng Yun, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
Matthias Brugger, linux-arm-kernel, linux-mediatek, linux-phy,
devicetree, linux-kernel, rex-bc.chen, Randy.Wu, jieyy.yang,
chuanjia.liu, qizhong.cheng, jian.yang
On Fri, Mar 18, 2022 at 5:54 PM Jianjun Wang <jianjun.wang@mediatek.com> wrote:
>
> Add PCIe GEN3 PHY driver support on MediaTek chipsets.
>
> Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> ---
> drivers/phy/mediatek/Kconfig | 11 ++
> drivers/phy/mediatek/Makefile | 1 +
> drivers/phy/mediatek/phy-mtk-pcie.c | 246 ++++++++++++++++++++++++++++
> 3 files changed, 258 insertions(+)
> create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
>
> diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
> index 55f8e6c048ab..387ed1b3f2cc 100644
> --- a/drivers/phy/mediatek/Kconfig
> +++ b/drivers/phy/mediatek/Kconfig
> @@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
> select GENERIC_PHY
> help
> Support MIPI DSI for Mediatek SoCs.
> +
> +config PHY_MTK_PCIE
> + tristate "MediaTek PCIe-PHY Driver"
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + depends on OF
> + select GENERIC_PHY
> + help
> + Say 'Y' here to add support for MediaTek PCIe PHY driver.
> + This driver create the basic PHY instance and provides initialize
> + callback for PCIe GEN3 port, it supports software efuse
> + initialization.
> diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
> index ace660fbed3a..788c13147f63 100644
> --- a/drivers/phy/mediatek/Makefile
> +++ b/drivers/phy/mediatek/Makefile
> @@ -6,6 +6,7 @@
> obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
> obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
> obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
> +obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
>
> phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
> phy-mtk-hdmi-drv-y += phy-mtk-hdmi-mt2701.o
> diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c b/drivers/phy/mediatek/phy-mtk-pcie.c
> new file mode 100644
> index 000000000000..0f5d7c7e2b7e
> --- /dev/null
> +++ b/drivers/phy/mediatek/phy-mtk-pcie.c
> @@ -0,0 +1,246 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 MediaTek Inc.
> + * Author: Jianjun Wang <jianjun.wang@mediatek.com>
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/compiler_types.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "phy-mtk-io.h"
> +
> +#define PEXTP_ANA_GLB_00_REG 0x9000
> +#define PEXTP_ANA_LN0_TRX_REG 0xa000
> +#define PEXTP_ANA_TX_OFFSET 0x04
> +#define PEXTP_ANA_RX_OFFSET 0x3c
> +#define PEXTP_ANA_LANE_OFFSET 0x100
> +
> +/* PEXTP_GLB_00_REG[28:24] Internal Resistor Selection of TX Bias Current */
> +#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
> +#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
> +
> +/* PEXTP_ANA_LN_RX_REG[3:0] RX impedance selection */
> +#define EFUSE_LN_RX_SEL GENMASK(3, 0)
> +#define EFUSE_LN_RX_VAL(x) (0xf & (x))
> +
> +/* PEXTP_ANA_LN_TX_REG[5:2] TX PMOS impedance selection */
> +#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
> +#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
> +
> +/* PEXTP_ANA_LN_TX_REG[11:8] TX NMOS impedance selection */
> +#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
> +#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
> +
> +/* Efuse data for each lane */
> +struct mtk_pcie_lane_efuse {
> + u32 tx_pmos;
> + u32 tx_nmos;
> + u32 rx_data;
> + bool lane_efuse_supported;
> +};
> +
> +struct mtk_pcie_phy {
> + struct device *dev;
> + struct phy *phy;
> + void __iomem *sif_base;
> +
> + /*
> + * Support software efuse initialization,
> + * currently we only support 2 lane in maximum.
> + */
> + bool sw_efuse_supported;
> + u32 efuse_glb_intr;
> + struct mtk_pcie_lane_efuse efuse[2];
> +};
> +
> +static void mtk_pcie_efuse_set_lane(struct mtk_pcie_phy *pcie_phy,
> + unsigned int lane)
> +{
> + struct mtk_pcie_lane_efuse *data;
> + void __iomem *addr;
> +
> + if (lane >= ARRAY_SIZE(pcie_phy->efuse)) {
> + dev_err(pcie_phy->dev,
> + "Requested lane number %d exceeds maximum %ld\n",
> + lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
> + return;
> + }
> +
> + data = &pcie_phy->efuse[lane];
> + if (!data->lane_efuse_supported)
> + return;
> +
> + addr = pcie_phy->sif_base + PEXTP_ANA_LN0_TRX_REG +
> + lane * PEXTP_ANA_LANE_OFFSET;
> +
> + mtk_phy_update_bits(addr + PEXTP_ANA_TX_OFFSET, EFUSE_LN_TX_PMOS_SEL,
> + EFUSE_LN_TX_PMOS_VAL(data->tx_pmos));
> +
> + mtk_phy_update_bits(addr + PEXTP_ANA_TX_OFFSET, EFUSE_LN_TX_NMOS_SEL,
> + EFUSE_LN_TX_NMOS_VAL(data->tx_nmos));
> +
> + mtk_phy_update_bits(addr + PEXTP_ANA_RX_OFFSET, EFUSE_LN_RX_SEL,
> + EFUSE_LN_RX_VAL(data->rx_data));
> +}
> +
> +/**
> + * mtk_pcie_phy_init() - Initialize the phy
> + * @phy: the phy to be initialized
> + *
> + * Initialize the phy by setting the efuse data.
> + * The hardware settings will be reset during suspend, it should be
> + * reinitialized when the consumer calls phy_init() again on resume.
> + */
> +static int mtk_pcie_phy_init(struct phy *phy)
> +{
> + struct mtk_pcie_phy *pcie_phy = phy_get_drvdata(phy);
> +
> + if (!pcie_phy->sw_efuse_supported)
> + return 0;
> +
> + /* Set global data */
> + mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_GLB_00_REG,
> + EFUSE_GLB_INTR_SEL,
> + EFUSE_GLB_INTR_VAL(pcie_phy->efuse_glb_intr));
> +
> + mtk_pcie_efuse_set_lane(pcie_phy, 0);
> +
> + mtk_pcie_efuse_set_lane(pcie_phy, 1);
> +
> + return 0;
> +}
> +
> +static const struct phy_ops mtk_pcie_phy_ops = {
> + .init = mtk_pcie_phy_init,
> + .owner = THIS_MODULE,
> +};
> +
> +static int mtk_pcie_efuse_read_for_lane(struct mtk_pcie_phy *pcie_phy,
> + unsigned int lane)
> +{
> + struct device *dev = pcie_phy->dev;
> + struct mtk_pcie_lane_efuse *data;
> + char efuse_id[15];
> + int ret;
> +
> + if (lane >= ARRAY_SIZE(pcie_phy->efuse))
> + return dev_err_probe(pcie_phy->dev, -EINVAL,
> + "Requested lane number %d exceeds maximum %ld\n",
> + lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
> +
> + data = &pcie_phy->efuse[lane];
> +
> + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_pmos", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_pmos);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_nmos", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_nmos);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + snprintf(efuse_id, sizeof(efuse_id), "rx_ln%d", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->rx_data);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + if (!(data->tx_pmos || data->tx_nmos || data->rx_data))
> + return dev_err_probe(dev, -EINVAL,
> + "No efuse data found for lane%d, but dts enable it\n",
> + lane);
> +
> + data->lane_efuse_supported = true;
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_read_efuse(struct mtk_pcie_phy *pcie_phy)
> +{
> + struct device *dev = pcie_phy->dev;
> + bool nvmem_enabled;
> + int ret;
> +
> + nvmem_enabled = device_property_read_bool(dev, "nvmem-cells");
> + if (!nvmem_enabled)
> + return -ENODEV;
> +
> + ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr",
> + &pcie_phy->efuse_glb_intr);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read glb_intr\n");
> +
> + pcie_phy->sw_efuse_supported = true;
> +
> + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 0);
> + if (ret)
> + return ret;
> +
> + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 1);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_phy_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct phy_provider *provider;
> + struct mtk_pcie_phy *pcie_phy;
> +
> + pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy), GFP_KERNEL);
> + if (!pcie_phy)
> + return -ENOMEM;
> +
> + pcie_phy->dev = dev;
> +
> + pcie_phy->sif_base = devm_platform_ioremap_resource_byname(pdev, "sif");
> + if (IS_ERR(pcie_phy->sif_base))
> + return dev_err_probe(dev, PTR_ERR(pcie_phy->sif_base),
> + "Failed to map phy-sif base\n");
> +
> + pcie_phy->phy = devm_phy_create(dev, dev->of_node, &mtk_pcie_phy_ops);
> + if (IS_ERR(pcie_phy->phy))
> + return dev_err_probe(dev, PTR_ERR(pcie_phy->phy),
> + "Failed to create PCIe phy\n");
> +
> + /*
> + * Failed to read the efuse data is not a fatal problem,
> + * ignore the failure and keep going.
> + */
> + mtk_pcie_read_efuse(pcie_phy);
You need to check the return value for -EPROBE_DEFER to defer probing.
> +
> + phy_set_drvdata(pcie_phy->phy, pcie_phy);
> +
> + provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> + if (IS_ERR(provider))
> + return dev_err_probe(dev, PTR_ERR(provider),
> + "PCIe phy probe failed\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mtk_pcie_phy_of_match[] = {
> + { .compatible = "mediatek,mt8195-pcie-phy" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, mtk_pcie_phy_of_match);
> +
> +static struct platform_driver mtk_pcie_phy_driver = {
> + .probe = mtk_pcie_phy_probe,
> + .driver = {
> + .name = "mtk-pcie-phy",
> + .of_match_table = mtk_pcie_phy_of_match,
> + },
> +};
> +module_platform_driver(mtk_pcie_phy_driver);
> +
> +MODULE_DESCRIPTION("MediaTek PCIe PHY driver");
> +MODULE_AUTHOR("Jianjun Wang <jianjun.wang@mediatek.com>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.18.0
>
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH v2 2/2] phy: mediatek: Add PCIe PHY driver
@ 2022-03-18 9:59 ` Chen-Yu Tsai
0 siblings, 0 replies; 52+ messages in thread
From: Chen-Yu Tsai @ 2022-03-18 9:59 UTC (permalink / raw)
To: Jianjun Wang
Cc: Chunfeng Yun, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
Matthias Brugger, linux-arm-kernel, linux-mediatek, linux-phy,
devicetree, linux-kernel, rex-bc.chen, Randy.Wu, jieyy.yang,
chuanjia.liu, qizhong.cheng, jian.yang
On Fri, Mar 18, 2022 at 5:54 PM Jianjun Wang <jianjun.wang@mediatek.com> wrote:
>
> Add PCIe GEN3 PHY driver support on MediaTek chipsets.
>
> Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> ---
> drivers/phy/mediatek/Kconfig | 11 ++
> drivers/phy/mediatek/Makefile | 1 +
> drivers/phy/mediatek/phy-mtk-pcie.c | 246 ++++++++++++++++++++++++++++
> 3 files changed, 258 insertions(+)
> create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
>
> diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
> index 55f8e6c048ab..387ed1b3f2cc 100644
> --- a/drivers/phy/mediatek/Kconfig
> +++ b/drivers/phy/mediatek/Kconfig
> @@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
> select GENERIC_PHY
> help
> Support MIPI DSI for Mediatek SoCs.
> +
> +config PHY_MTK_PCIE
> + tristate "MediaTek PCIe-PHY Driver"
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + depends on OF
> + select GENERIC_PHY
> + help
> + Say 'Y' here to add support for MediaTek PCIe PHY driver.
> + This driver create the basic PHY instance and provides initialize
> + callback for PCIe GEN3 port, it supports software efuse
> + initialization.
> diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
> index ace660fbed3a..788c13147f63 100644
> --- a/drivers/phy/mediatek/Makefile
> +++ b/drivers/phy/mediatek/Makefile
> @@ -6,6 +6,7 @@
> obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
> obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
> obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
> +obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
>
> phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
> phy-mtk-hdmi-drv-y += phy-mtk-hdmi-mt2701.o
> diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c b/drivers/phy/mediatek/phy-mtk-pcie.c
> new file mode 100644
> index 000000000000..0f5d7c7e2b7e
> --- /dev/null
> +++ b/drivers/phy/mediatek/phy-mtk-pcie.c
> @@ -0,0 +1,246 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 MediaTek Inc.
> + * Author: Jianjun Wang <jianjun.wang@mediatek.com>
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/compiler_types.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "phy-mtk-io.h"
> +
> +#define PEXTP_ANA_GLB_00_REG 0x9000
> +#define PEXTP_ANA_LN0_TRX_REG 0xa000
> +#define PEXTP_ANA_TX_OFFSET 0x04
> +#define PEXTP_ANA_RX_OFFSET 0x3c
> +#define PEXTP_ANA_LANE_OFFSET 0x100
> +
> +/* PEXTP_GLB_00_REG[28:24] Internal Resistor Selection of TX Bias Current */
> +#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
> +#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
> +
> +/* PEXTP_ANA_LN_RX_REG[3:0] RX impedance selection */
> +#define EFUSE_LN_RX_SEL GENMASK(3, 0)
> +#define EFUSE_LN_RX_VAL(x) (0xf & (x))
> +
> +/* PEXTP_ANA_LN_TX_REG[5:2] TX PMOS impedance selection */
> +#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
> +#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
> +
> +/* PEXTP_ANA_LN_TX_REG[11:8] TX NMOS impedance selection */
> +#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
> +#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
> +
> +/* Efuse data for each lane */
> +struct mtk_pcie_lane_efuse {
> + u32 tx_pmos;
> + u32 tx_nmos;
> + u32 rx_data;
> + bool lane_efuse_supported;
> +};
> +
> +struct mtk_pcie_phy {
> + struct device *dev;
> + struct phy *phy;
> + void __iomem *sif_base;
> +
> + /*
> + * Support software efuse initialization,
> + * currently we only support 2 lane in maximum.
> + */
> + bool sw_efuse_supported;
> + u32 efuse_glb_intr;
> + struct mtk_pcie_lane_efuse efuse[2];
> +};
> +
> +static void mtk_pcie_efuse_set_lane(struct mtk_pcie_phy *pcie_phy,
> + unsigned int lane)
> +{
> + struct mtk_pcie_lane_efuse *data;
> + void __iomem *addr;
> +
> + if (lane >= ARRAY_SIZE(pcie_phy->efuse)) {
> + dev_err(pcie_phy->dev,
> + "Requested lane number %d exceeds maximum %ld\n",
> + lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
> + return;
> + }
> +
> + data = &pcie_phy->efuse[lane];
> + if (!data->lane_efuse_supported)
> + return;
> +
> + addr = pcie_phy->sif_base + PEXTP_ANA_LN0_TRX_REG +
> + lane * PEXTP_ANA_LANE_OFFSET;
> +
> + mtk_phy_update_bits(addr + PEXTP_ANA_TX_OFFSET, EFUSE_LN_TX_PMOS_SEL,
> + EFUSE_LN_TX_PMOS_VAL(data->tx_pmos));
> +
> + mtk_phy_update_bits(addr + PEXTP_ANA_TX_OFFSET, EFUSE_LN_TX_NMOS_SEL,
> + EFUSE_LN_TX_NMOS_VAL(data->tx_nmos));
> +
> + mtk_phy_update_bits(addr + PEXTP_ANA_RX_OFFSET, EFUSE_LN_RX_SEL,
> + EFUSE_LN_RX_VAL(data->rx_data));
> +}
> +
> +/**
> + * mtk_pcie_phy_init() - Initialize the phy
> + * @phy: the phy to be initialized
> + *
> + * Initialize the phy by setting the efuse data.
> + * The hardware settings will be reset during suspend, it should be
> + * reinitialized when the consumer calls phy_init() again on resume.
> + */
> +static int mtk_pcie_phy_init(struct phy *phy)
> +{
> + struct mtk_pcie_phy *pcie_phy = phy_get_drvdata(phy);
> +
> + if (!pcie_phy->sw_efuse_supported)
> + return 0;
> +
> + /* Set global data */
> + mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_GLB_00_REG,
> + EFUSE_GLB_INTR_SEL,
> + EFUSE_GLB_INTR_VAL(pcie_phy->efuse_glb_intr));
> +
> + mtk_pcie_efuse_set_lane(pcie_phy, 0);
> +
> + mtk_pcie_efuse_set_lane(pcie_phy, 1);
> +
> + return 0;
> +}
> +
> +static const struct phy_ops mtk_pcie_phy_ops = {
> + .init = mtk_pcie_phy_init,
> + .owner = THIS_MODULE,
> +};
> +
> +static int mtk_pcie_efuse_read_for_lane(struct mtk_pcie_phy *pcie_phy,
> + unsigned int lane)
> +{
> + struct device *dev = pcie_phy->dev;
> + struct mtk_pcie_lane_efuse *data;
> + char efuse_id[15];
> + int ret;
> +
> + if (lane >= ARRAY_SIZE(pcie_phy->efuse))
> + return dev_err_probe(pcie_phy->dev, -EINVAL,
> + "Requested lane number %d exceeds maximum %ld\n",
> + lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
> +
> + data = &pcie_phy->efuse[lane];
> +
> + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_pmos", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_pmos);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_nmos", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_nmos);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + snprintf(efuse_id, sizeof(efuse_id), "rx_ln%d", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->rx_data);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + if (!(data->tx_pmos || data->tx_nmos || data->rx_data))
> + return dev_err_probe(dev, -EINVAL,
> + "No efuse data found for lane%d, but dts enable it\n",
> + lane);
> +
> + data->lane_efuse_supported = true;
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_read_efuse(struct mtk_pcie_phy *pcie_phy)
> +{
> + struct device *dev = pcie_phy->dev;
> + bool nvmem_enabled;
> + int ret;
> +
> + nvmem_enabled = device_property_read_bool(dev, "nvmem-cells");
> + if (!nvmem_enabled)
> + return -ENODEV;
> +
> + ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr",
> + &pcie_phy->efuse_glb_intr);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read glb_intr\n");
> +
> + pcie_phy->sw_efuse_supported = true;
> +
> + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 0);
> + if (ret)
> + return ret;
> +
> + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 1);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_phy_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct phy_provider *provider;
> + struct mtk_pcie_phy *pcie_phy;
> +
> + pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy), GFP_KERNEL);
> + if (!pcie_phy)
> + return -ENOMEM;
> +
> + pcie_phy->dev = dev;
> +
> + pcie_phy->sif_base = devm_platform_ioremap_resource_byname(pdev, "sif");
> + if (IS_ERR(pcie_phy->sif_base))
> + return dev_err_probe(dev, PTR_ERR(pcie_phy->sif_base),
> + "Failed to map phy-sif base\n");
> +
> + pcie_phy->phy = devm_phy_create(dev, dev->of_node, &mtk_pcie_phy_ops);
> + if (IS_ERR(pcie_phy->phy))
> + return dev_err_probe(dev, PTR_ERR(pcie_phy->phy),
> + "Failed to create PCIe phy\n");
> +
> + /*
> + * Failed to read the efuse data is not a fatal problem,
> + * ignore the failure and keep going.
> + */
> + mtk_pcie_read_efuse(pcie_phy);
You need to check the return value for -EPROBE_DEFER to defer probing.
> +
> + phy_set_drvdata(pcie_phy->phy, pcie_phy);
> +
> + provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> + if (IS_ERR(provider))
> + return dev_err_probe(dev, PTR_ERR(provider),
> + "PCIe phy probe failed\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mtk_pcie_phy_of_match[] = {
> + { .compatible = "mediatek,mt8195-pcie-phy" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, mtk_pcie_phy_of_match);
> +
> +static struct platform_driver mtk_pcie_phy_driver = {
> + .probe = mtk_pcie_phy_probe,
> + .driver = {
> + .name = "mtk-pcie-phy",
> + .of_match_table = mtk_pcie_phy_of_match,
> + },
> +};
> +module_platform_driver(mtk_pcie_phy_driver);
> +
> +MODULE_DESCRIPTION("MediaTek PCIe PHY driver");
> +MODULE_AUTHOR("Jianjun Wang <jianjun.wang@mediatek.com>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.18.0
>
_______________________________________________
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] 52+ messages in thread* Re: [PATCH v2 2/2] phy: mediatek: Add PCIe PHY driver
@ 2022-03-18 9:59 ` Chen-Yu Tsai
0 siblings, 0 replies; 52+ messages in thread
From: Chen-Yu Tsai @ 2022-03-18 9:59 UTC (permalink / raw)
To: Jianjun Wang
Cc: Chunfeng Yun, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
Matthias Brugger, linux-arm-kernel, linux-mediatek, linux-phy,
devicetree, linux-kernel, rex-bc.chen, Randy.Wu, jieyy.yang,
chuanjia.liu, qizhong.cheng, jian.yang
On Fri, Mar 18, 2022 at 5:54 PM Jianjun Wang <jianjun.wang@mediatek.com> wrote:
>
> Add PCIe GEN3 PHY driver support on MediaTek chipsets.
>
> Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> ---
> drivers/phy/mediatek/Kconfig | 11 ++
> drivers/phy/mediatek/Makefile | 1 +
> drivers/phy/mediatek/phy-mtk-pcie.c | 246 ++++++++++++++++++++++++++++
> 3 files changed, 258 insertions(+)
> create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
>
> diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
> index 55f8e6c048ab..387ed1b3f2cc 100644
> --- a/drivers/phy/mediatek/Kconfig
> +++ b/drivers/phy/mediatek/Kconfig
> @@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
> select GENERIC_PHY
> help
> Support MIPI DSI for Mediatek SoCs.
> +
> +config PHY_MTK_PCIE
> + tristate "MediaTek PCIe-PHY Driver"
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + depends on OF
> + select GENERIC_PHY
> + help
> + Say 'Y' here to add support for MediaTek PCIe PHY driver.
> + This driver create the basic PHY instance and provides initialize
> + callback for PCIe GEN3 port, it supports software efuse
> + initialization.
> diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
> index ace660fbed3a..788c13147f63 100644
> --- a/drivers/phy/mediatek/Makefile
> +++ b/drivers/phy/mediatek/Makefile
> @@ -6,6 +6,7 @@
> obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
> obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
> obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
> +obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
>
> phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
> phy-mtk-hdmi-drv-y += phy-mtk-hdmi-mt2701.o
> diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c b/drivers/phy/mediatek/phy-mtk-pcie.c
> new file mode 100644
> index 000000000000..0f5d7c7e2b7e
> --- /dev/null
> +++ b/drivers/phy/mediatek/phy-mtk-pcie.c
> @@ -0,0 +1,246 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 MediaTek Inc.
> + * Author: Jianjun Wang <jianjun.wang@mediatek.com>
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/compiler_types.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "phy-mtk-io.h"
> +
> +#define PEXTP_ANA_GLB_00_REG 0x9000
> +#define PEXTP_ANA_LN0_TRX_REG 0xa000
> +#define PEXTP_ANA_TX_OFFSET 0x04
> +#define PEXTP_ANA_RX_OFFSET 0x3c
> +#define PEXTP_ANA_LANE_OFFSET 0x100
> +
> +/* PEXTP_GLB_00_REG[28:24] Internal Resistor Selection of TX Bias Current */
> +#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
> +#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
> +
> +/* PEXTP_ANA_LN_RX_REG[3:0] RX impedance selection */
> +#define EFUSE_LN_RX_SEL GENMASK(3, 0)
> +#define EFUSE_LN_RX_VAL(x) (0xf & (x))
> +
> +/* PEXTP_ANA_LN_TX_REG[5:2] TX PMOS impedance selection */
> +#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
> +#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
> +
> +/* PEXTP_ANA_LN_TX_REG[11:8] TX NMOS impedance selection */
> +#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
> +#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
> +
> +/* Efuse data for each lane */
> +struct mtk_pcie_lane_efuse {
> + u32 tx_pmos;
> + u32 tx_nmos;
> + u32 rx_data;
> + bool lane_efuse_supported;
> +};
> +
> +struct mtk_pcie_phy {
> + struct device *dev;
> + struct phy *phy;
> + void __iomem *sif_base;
> +
> + /*
> + * Support software efuse initialization,
> + * currently we only support 2 lane in maximum.
> + */
> + bool sw_efuse_supported;
> + u32 efuse_glb_intr;
> + struct mtk_pcie_lane_efuse efuse[2];
> +};
> +
> +static void mtk_pcie_efuse_set_lane(struct mtk_pcie_phy *pcie_phy,
> + unsigned int lane)
> +{
> + struct mtk_pcie_lane_efuse *data;
> + void __iomem *addr;
> +
> + if (lane >= ARRAY_SIZE(pcie_phy->efuse)) {
> + dev_err(pcie_phy->dev,
> + "Requested lane number %d exceeds maximum %ld\n",
> + lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
> + return;
> + }
> +
> + data = &pcie_phy->efuse[lane];
> + if (!data->lane_efuse_supported)
> + return;
> +
> + addr = pcie_phy->sif_base + PEXTP_ANA_LN0_TRX_REG +
> + lane * PEXTP_ANA_LANE_OFFSET;
> +
> + mtk_phy_update_bits(addr + PEXTP_ANA_TX_OFFSET, EFUSE_LN_TX_PMOS_SEL,
> + EFUSE_LN_TX_PMOS_VAL(data->tx_pmos));
> +
> + mtk_phy_update_bits(addr + PEXTP_ANA_TX_OFFSET, EFUSE_LN_TX_NMOS_SEL,
> + EFUSE_LN_TX_NMOS_VAL(data->tx_nmos));
> +
> + mtk_phy_update_bits(addr + PEXTP_ANA_RX_OFFSET, EFUSE_LN_RX_SEL,
> + EFUSE_LN_RX_VAL(data->rx_data));
> +}
> +
> +/**
> + * mtk_pcie_phy_init() - Initialize the phy
> + * @phy: the phy to be initialized
> + *
> + * Initialize the phy by setting the efuse data.
> + * The hardware settings will be reset during suspend, it should be
> + * reinitialized when the consumer calls phy_init() again on resume.
> + */
> +static int mtk_pcie_phy_init(struct phy *phy)
> +{
> + struct mtk_pcie_phy *pcie_phy = phy_get_drvdata(phy);
> +
> + if (!pcie_phy->sw_efuse_supported)
> + return 0;
> +
> + /* Set global data */
> + mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_GLB_00_REG,
> + EFUSE_GLB_INTR_SEL,
> + EFUSE_GLB_INTR_VAL(pcie_phy->efuse_glb_intr));
> +
> + mtk_pcie_efuse_set_lane(pcie_phy, 0);
> +
> + mtk_pcie_efuse_set_lane(pcie_phy, 1);
> +
> + return 0;
> +}
> +
> +static const struct phy_ops mtk_pcie_phy_ops = {
> + .init = mtk_pcie_phy_init,
> + .owner = THIS_MODULE,
> +};
> +
> +static int mtk_pcie_efuse_read_for_lane(struct mtk_pcie_phy *pcie_phy,
> + unsigned int lane)
> +{
> + struct device *dev = pcie_phy->dev;
> + struct mtk_pcie_lane_efuse *data;
> + char efuse_id[15];
> + int ret;
> +
> + if (lane >= ARRAY_SIZE(pcie_phy->efuse))
> + return dev_err_probe(pcie_phy->dev, -EINVAL,
> + "Requested lane number %d exceeds maximum %ld\n",
> + lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
> +
> + data = &pcie_phy->efuse[lane];
> +
> + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_pmos", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_pmos);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_nmos", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_nmos);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + snprintf(efuse_id, sizeof(efuse_id), "rx_ln%d", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->rx_data);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + if (!(data->tx_pmos || data->tx_nmos || data->rx_data))
> + return dev_err_probe(dev, -EINVAL,
> + "No efuse data found for lane%d, but dts enable it\n",
> + lane);
> +
> + data->lane_efuse_supported = true;
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_read_efuse(struct mtk_pcie_phy *pcie_phy)
> +{
> + struct device *dev = pcie_phy->dev;
> + bool nvmem_enabled;
> + int ret;
> +
> + nvmem_enabled = device_property_read_bool(dev, "nvmem-cells");
> + if (!nvmem_enabled)
> + return -ENODEV;
> +
> + ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr",
> + &pcie_phy->efuse_glb_intr);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read glb_intr\n");
> +
> + pcie_phy->sw_efuse_supported = true;
> +
> + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 0);
> + if (ret)
> + return ret;
> +
> + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 1);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_phy_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct phy_provider *provider;
> + struct mtk_pcie_phy *pcie_phy;
> +
> + pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy), GFP_KERNEL);
> + if (!pcie_phy)
> + return -ENOMEM;
> +
> + pcie_phy->dev = dev;
> +
> + pcie_phy->sif_base = devm_platform_ioremap_resource_byname(pdev, "sif");
> + if (IS_ERR(pcie_phy->sif_base))
> + return dev_err_probe(dev, PTR_ERR(pcie_phy->sif_base),
> + "Failed to map phy-sif base\n");
> +
> + pcie_phy->phy = devm_phy_create(dev, dev->of_node, &mtk_pcie_phy_ops);
> + if (IS_ERR(pcie_phy->phy))
> + return dev_err_probe(dev, PTR_ERR(pcie_phy->phy),
> + "Failed to create PCIe phy\n");
> +
> + /*
> + * Failed to read the efuse data is not a fatal problem,
> + * ignore the failure and keep going.
> + */
> + mtk_pcie_read_efuse(pcie_phy);
You need to check the return value for -EPROBE_DEFER to defer probing.
> +
> + phy_set_drvdata(pcie_phy->phy, pcie_phy);
> +
> + provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> + if (IS_ERR(provider))
> + return dev_err_probe(dev, PTR_ERR(provider),
> + "PCIe phy probe failed\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mtk_pcie_phy_of_match[] = {
> + { .compatible = "mediatek,mt8195-pcie-phy" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, mtk_pcie_phy_of_match);
> +
> +static struct platform_driver mtk_pcie_phy_driver = {
> + .probe = mtk_pcie_phy_probe,
> + .driver = {
> + .name = "mtk-pcie-phy",
> + .of_match_table = mtk_pcie_phy_of_match,
> + },
> +};
> +module_platform_driver(mtk_pcie_phy_driver);
> +
> +MODULE_DESCRIPTION("MediaTek PCIe PHY driver");
> +MODULE_AUTHOR("Jianjun Wang <jianjun.wang@mediatek.com>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.18.0
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 2/2] phy: mediatek: Add PCIe PHY driver
2022-03-18 9:54 ` Jianjun Wang
(?)
(?)
@ 2022-03-18 10:57 ` AngeloGioacchino Del Regno
-1 siblings, 0 replies; 52+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-03-18 10:57 UTC (permalink / raw)
To: Jianjun Wang, Chunfeng Yun, Kishon Vijay Abraham I, Vinod Koul,
Rob Herring, Matthias Brugger, Chen-Yu Tsai
Cc: linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
linux-kernel, rex-bc.chen, randy.wu, jieyy.yang, chuanjia.liu,
qizhong.cheng, jian.yang
Il 18/03/22 10:54, Jianjun Wang ha scritto:
> Add PCIe GEN3 PHY driver support on MediaTek chipsets.
>
> Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> ---
> drivers/phy/mediatek/Kconfig | 11 ++
> drivers/phy/mediatek/Makefile | 1 +
> drivers/phy/mediatek/phy-mtk-pcie.c | 246 ++++++++++++++++++++++++++++
> 3 files changed, 258 insertions(+)
> create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
>
> diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
> index 55f8e6c048ab..387ed1b3f2cc 100644
> --- a/drivers/phy/mediatek/Kconfig
> +++ b/drivers/phy/mediatek/Kconfig
> @@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
> select GENERIC_PHY
> help
> Support MIPI DSI for Mediatek SoCs.
> +
> +config PHY_MTK_PCIE
> + tristate "MediaTek PCIe-PHY Driver"
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + depends on OF
> + select GENERIC_PHY
> + help
> + Say 'Y' here to add support for MediaTek PCIe PHY driver.
> + This driver create the basic PHY instance and provides initialize
> + callback for PCIe GEN3 port, it supports software efuse
> + initialization.
> diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
> index ace660fbed3a..788c13147f63 100644
> --- a/drivers/phy/mediatek/Makefile
> +++ b/drivers/phy/mediatek/Makefile
> @@ -6,6 +6,7 @@
> obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
> obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
> obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
> +obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
>
> phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
> phy-mtk-hdmi-drv-y += phy-mtk-hdmi-mt2701.o
> diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c b/drivers/phy/mediatek/phy-mtk-pcie.c
> new file mode 100644
> index 000000000000..0f5d7c7e2b7e
> --- /dev/null
> +++ b/drivers/phy/mediatek/phy-mtk-pcie.c
> @@ -0,0 +1,246 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 MediaTek Inc.
> + * Author: Jianjun Wang <jianjun.wang@mediatek.com>
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/compiler_types.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "phy-mtk-io.h"
> +
> +#define PEXTP_ANA_GLB_00_REG 0x9000
> +#define PEXTP_ANA_LN0_TRX_REG 0xa000
> +#define PEXTP_ANA_TX_OFFSET 0x04
> +#define PEXTP_ANA_RX_OFFSET 0x3c
> +#define PEXTP_ANA_LANE_OFFSET 0x100
> +
> +/* PEXTP_GLB_00_REG[28:24] Internal Resistor Selection of TX Bias Current */
> +#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
> +#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
> +
> +/* PEXTP_ANA_LN_RX_REG[3:0] RX impedance selection */
> +#define EFUSE_LN_RX_SEL GENMASK(3, 0)
> +#define EFUSE_LN_RX_VAL(x) (0xf & (x))
> +
> +/* PEXTP_ANA_LN_TX_REG[5:2] TX PMOS impedance selection */
> +#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
> +#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
> +
> +/* PEXTP_ANA_LN_TX_REG[11:8] TX NMOS impedance selection */
> +#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
> +#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
> +
> +/* Efuse data for each lane */
What about some kerneldoc?
/**
* struct mtk_pcie_lane_efuse - eFuse data for each lane
* @tx_pmos:
......etc :))
> +struct mtk_pcie_lane_efuse {
> + u32 tx_pmos;
> + u32 tx_nmos;
> + u32 rx_data;
> + bool lane_efuse_supported;
> +};
> +
Same here
/**
* struct mtk_pcie_phy - PCIe phy driver main structure
* @dev: ......
> +struct mtk_pcie_phy {
> + struct device *dev;
> + struct phy *phy;
> + void __iomem *sif_base;
> +
> + /*
> + * Support software efuse initialization,
> + * currently we only support 2 lane in maximum.
> + */
Obviously, if you add kerneldoc, this comment would get moved to that kerneldoc.
> + bool sw_efuse_supported;
> + u32 efuse_glb_intr;
> + struct mtk_pcie_lane_efuse efuse[2];
If you dynamically allocate this one, you will be able to support any number
of lanes, futureproofing this driver and giving it more flexibility.
> +};
> +
..snip..
> +
> +static int mtk_pcie_efuse_read_for_lane(struct mtk_pcie_phy *pcie_phy,
> + unsigned int lane)
> +{
> + struct device *dev = pcie_phy->dev;
> + struct mtk_pcie_lane_efuse *data;
> + char efuse_id[15];
> + int ret;
> +
> + if (lane >= ARRAY_SIZE(pcie_phy->efuse))
> + return dev_err_probe(pcie_phy->dev, -EINVAL,
> + "Requested lane number %d exceeds maximum %ld\n",
> + lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
I don't like seeing dev_err_probe() outside of a probe function, but I acknowledge
that the Linux documentation doesn't seem to give any direction about that, so
this is a personal preference, at this point.
> +
> + data = &pcie_phy->efuse[lane];
> +
> + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_pmos", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_pmos);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_nmos", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_nmos);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + snprintf(efuse_id, sizeof(efuse_id), "rx_ln%d", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->rx_data);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + if (!(data->tx_pmos || data->tx_nmos || data->rx_data))
> + return dev_err_probe(dev, -EINVAL,
> + "No efuse data found for lane%d, but dts enable it\n",
> + lane);
> +
> + data->lane_efuse_supported = true;
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_read_efuse(struct mtk_pcie_phy *pcie_phy)
> +{
> + struct device *dev = pcie_phy->dev;
> + bool nvmem_enabled;
> + int ret;
> +
> + nvmem_enabled = device_property_read_bool(dev, "nvmem-cells");
> + if (!nvmem_enabled)
> + return -ENODEV;
> +
> + ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr",
> + &pcie_phy->efuse_glb_intr);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read glb_intr\n");
> +
> + pcie_phy->sw_efuse_supported = true;
> +
> + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 0);
> + if (ret)
> + return ret;
> +
> + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 1);
> + if (ret)
> + return ret;
To give some more future-proofing to this driver, I would instead either add a
u32 devicetree property "num-lanes" or, if the same SoC may not have a different
number of lanes across controller instances, I would add a number of lanes
parameter as data for each of_match.
You'd be at that point using a for loop here like:
for (i = 0; i < pcie_phy->num_lanes, i++) {
ret = mtk_pcie_efuse_read_for_lane(pcie_phy, i);
if (ret)
return ret;
}
Of course, the same logic would apply to mtk_pcie_phy_init(), where you are
instead calling mtk_pcie_efuse_set_lane().
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_phy_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct phy_provider *provider;
> + struct mtk_pcie_phy *pcie_phy;
> +
> + pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy), GFP_KERNEL);
> + if (!pcie_phy)
> + return -ENOMEM;
> +
> + pcie_phy->dev = dev;
> +
> + pcie_phy->sif_base = devm_platform_ioremap_resource_byname(pdev, "sif");
> + if (IS_ERR(pcie_phy->sif_base))
> + return dev_err_probe(dev, PTR_ERR(pcie_phy->sif_base),
> + "Failed to map phy-sif base\n");
> +
> + pcie_phy->phy = devm_phy_create(dev, dev->of_node, &mtk_pcie_phy_ops);
> + if (IS_ERR(pcie_phy->phy))
> + return dev_err_probe(dev, PTR_ERR(pcie_phy->phy),
> + "Failed to create PCIe phy\n");
> +
> + /*
> + * Failed to read the efuse data is not a fatal problem,
> + * ignore the failure and keep going.
> + */
> + mtk_pcie_read_efuse(pcie_phy);
If you get an -EPROBE_DEFER here, you surely want to defer probing this driver,
so, yes you're free to ignore the other failures, but you should fix that corner
case.
Everything else looks good, so, please make sure to add me to the Cc's for the
next version of this series for me to give you a faster review.
Regards,
Angelo
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH v2 2/2] phy: mediatek: Add PCIe PHY driver
@ 2022-03-18 10:57 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 52+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-03-18 10:57 UTC (permalink / raw)
To: Jianjun Wang, Chunfeng Yun, Kishon Vijay Abraham I, Vinod Koul,
Rob Herring, Matthias Brugger, Chen-Yu Tsai
Cc: linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
linux-kernel, rex-bc.chen, randy.wu, jieyy.yang, chuanjia.liu,
qizhong.cheng, jian.yang
Il 18/03/22 10:54, Jianjun Wang ha scritto:
> Add PCIe GEN3 PHY driver support on MediaTek chipsets.
>
> Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> ---
> drivers/phy/mediatek/Kconfig | 11 ++
> drivers/phy/mediatek/Makefile | 1 +
> drivers/phy/mediatek/phy-mtk-pcie.c | 246 ++++++++++++++++++++++++++++
> 3 files changed, 258 insertions(+)
> create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
>
> diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
> index 55f8e6c048ab..387ed1b3f2cc 100644
> --- a/drivers/phy/mediatek/Kconfig
> +++ b/drivers/phy/mediatek/Kconfig
> @@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
> select GENERIC_PHY
> help
> Support MIPI DSI for Mediatek SoCs.
> +
> +config PHY_MTK_PCIE
> + tristate "MediaTek PCIe-PHY Driver"
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + depends on OF
> + select GENERIC_PHY
> + help
> + Say 'Y' here to add support for MediaTek PCIe PHY driver.
> + This driver create the basic PHY instance and provides initialize
> + callback for PCIe GEN3 port, it supports software efuse
> + initialization.
> diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
> index ace660fbed3a..788c13147f63 100644
> --- a/drivers/phy/mediatek/Makefile
> +++ b/drivers/phy/mediatek/Makefile
> @@ -6,6 +6,7 @@
> obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
> obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
> obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
> +obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
>
> phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
> phy-mtk-hdmi-drv-y += phy-mtk-hdmi-mt2701.o
> diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c b/drivers/phy/mediatek/phy-mtk-pcie.c
> new file mode 100644
> index 000000000000..0f5d7c7e2b7e
> --- /dev/null
> +++ b/drivers/phy/mediatek/phy-mtk-pcie.c
> @@ -0,0 +1,246 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 MediaTek Inc.
> + * Author: Jianjun Wang <jianjun.wang@mediatek.com>
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/compiler_types.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "phy-mtk-io.h"
> +
> +#define PEXTP_ANA_GLB_00_REG 0x9000
> +#define PEXTP_ANA_LN0_TRX_REG 0xa000
> +#define PEXTP_ANA_TX_OFFSET 0x04
> +#define PEXTP_ANA_RX_OFFSET 0x3c
> +#define PEXTP_ANA_LANE_OFFSET 0x100
> +
> +/* PEXTP_GLB_00_REG[28:24] Internal Resistor Selection of TX Bias Current */
> +#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
> +#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
> +
> +/* PEXTP_ANA_LN_RX_REG[3:0] RX impedance selection */
> +#define EFUSE_LN_RX_SEL GENMASK(3, 0)
> +#define EFUSE_LN_RX_VAL(x) (0xf & (x))
> +
> +/* PEXTP_ANA_LN_TX_REG[5:2] TX PMOS impedance selection */
> +#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
> +#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
> +
> +/* PEXTP_ANA_LN_TX_REG[11:8] TX NMOS impedance selection */
> +#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
> +#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
> +
> +/* Efuse data for each lane */
What about some kerneldoc?
/**
* struct mtk_pcie_lane_efuse - eFuse data for each lane
* @tx_pmos:
......etc :))
> +struct mtk_pcie_lane_efuse {
> + u32 tx_pmos;
> + u32 tx_nmos;
> + u32 rx_data;
> + bool lane_efuse_supported;
> +};
> +
Same here
/**
* struct mtk_pcie_phy - PCIe phy driver main structure
* @dev: ......
> +struct mtk_pcie_phy {
> + struct device *dev;
> + struct phy *phy;
> + void __iomem *sif_base;
> +
> + /*
> + * Support software efuse initialization,
> + * currently we only support 2 lane in maximum.
> + */
Obviously, if you add kerneldoc, this comment would get moved to that kerneldoc.
> + bool sw_efuse_supported;
> + u32 efuse_glb_intr;
> + struct mtk_pcie_lane_efuse efuse[2];
If you dynamically allocate this one, you will be able to support any number
of lanes, futureproofing this driver and giving it more flexibility.
> +};
> +
..snip..
> +
> +static int mtk_pcie_efuse_read_for_lane(struct mtk_pcie_phy *pcie_phy,
> + unsigned int lane)
> +{
> + struct device *dev = pcie_phy->dev;
> + struct mtk_pcie_lane_efuse *data;
> + char efuse_id[15];
> + int ret;
> +
> + if (lane >= ARRAY_SIZE(pcie_phy->efuse))
> + return dev_err_probe(pcie_phy->dev, -EINVAL,
> + "Requested lane number %d exceeds maximum %ld\n",
> + lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
I don't like seeing dev_err_probe() outside of a probe function, but I acknowledge
that the Linux documentation doesn't seem to give any direction about that, so
this is a personal preference, at this point.
> +
> + data = &pcie_phy->efuse[lane];
> +
> + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_pmos", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_pmos);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_nmos", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_nmos);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + snprintf(efuse_id, sizeof(efuse_id), "rx_ln%d", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->rx_data);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + if (!(data->tx_pmos || data->tx_nmos || data->rx_data))
> + return dev_err_probe(dev, -EINVAL,
> + "No efuse data found for lane%d, but dts enable it\n",
> + lane);
> +
> + data->lane_efuse_supported = true;
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_read_efuse(struct mtk_pcie_phy *pcie_phy)
> +{
> + struct device *dev = pcie_phy->dev;
> + bool nvmem_enabled;
> + int ret;
> +
> + nvmem_enabled = device_property_read_bool(dev, "nvmem-cells");
> + if (!nvmem_enabled)
> + return -ENODEV;
> +
> + ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr",
> + &pcie_phy->efuse_glb_intr);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read glb_intr\n");
> +
> + pcie_phy->sw_efuse_supported = true;
> +
> + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 0);
> + if (ret)
> + return ret;
> +
> + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 1);
> + if (ret)
> + return ret;
To give some more future-proofing to this driver, I would instead either add a
u32 devicetree property "num-lanes" or, if the same SoC may not have a different
number of lanes across controller instances, I would add a number of lanes
parameter as data for each of_match.
You'd be at that point using a for loop here like:
for (i = 0; i < pcie_phy->num_lanes, i++) {
ret = mtk_pcie_efuse_read_for_lane(pcie_phy, i);
if (ret)
return ret;
}
Of course, the same logic would apply to mtk_pcie_phy_init(), where you are
instead calling mtk_pcie_efuse_set_lane().
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_phy_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct phy_provider *provider;
> + struct mtk_pcie_phy *pcie_phy;
> +
> + pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy), GFP_KERNEL);
> + if (!pcie_phy)
> + return -ENOMEM;
> +
> + pcie_phy->dev = dev;
> +
> + pcie_phy->sif_base = devm_platform_ioremap_resource_byname(pdev, "sif");
> + if (IS_ERR(pcie_phy->sif_base))
> + return dev_err_probe(dev, PTR_ERR(pcie_phy->sif_base),
> + "Failed to map phy-sif base\n");
> +
> + pcie_phy->phy = devm_phy_create(dev, dev->of_node, &mtk_pcie_phy_ops);
> + if (IS_ERR(pcie_phy->phy))
> + return dev_err_probe(dev, PTR_ERR(pcie_phy->phy),
> + "Failed to create PCIe phy\n");
> +
> + /*
> + * Failed to read the efuse data is not a fatal problem,
> + * ignore the failure and keep going.
> + */
> + mtk_pcie_read_efuse(pcie_phy);
If you get an -EPROBE_DEFER here, you surely want to defer probing this driver,
so, yes you're free to ignore the other failures, but you should fix that corner
case.
Everything else looks good, so, please make sure to add me to the Cc's for the
next version of this series for me to give you a faster review.
Regards,
Angelo
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH v2 2/2] phy: mediatek: Add PCIe PHY driver
@ 2022-03-18 10:57 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 52+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-03-18 10:57 UTC (permalink / raw)
To: Jianjun Wang, Chunfeng Yun, Kishon Vijay Abraham I, Vinod Koul,
Rob Herring, Matthias Brugger, Chen-Yu Tsai
Cc: linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
linux-kernel, rex-bc.chen, randy.wu, jieyy.yang, chuanjia.liu,
qizhong.cheng, jian.yang
Il 18/03/22 10:54, Jianjun Wang ha scritto:
> Add PCIe GEN3 PHY driver support on MediaTek chipsets.
>
> Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> ---
> drivers/phy/mediatek/Kconfig | 11 ++
> drivers/phy/mediatek/Makefile | 1 +
> drivers/phy/mediatek/phy-mtk-pcie.c | 246 ++++++++++++++++++++++++++++
> 3 files changed, 258 insertions(+)
> create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
>
> diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
> index 55f8e6c048ab..387ed1b3f2cc 100644
> --- a/drivers/phy/mediatek/Kconfig
> +++ b/drivers/phy/mediatek/Kconfig
> @@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
> select GENERIC_PHY
> help
> Support MIPI DSI for Mediatek SoCs.
> +
> +config PHY_MTK_PCIE
> + tristate "MediaTek PCIe-PHY Driver"
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + depends on OF
> + select GENERIC_PHY
> + help
> + Say 'Y' here to add support for MediaTek PCIe PHY driver.
> + This driver create the basic PHY instance and provides initialize
> + callback for PCIe GEN3 port, it supports software efuse
> + initialization.
> diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
> index ace660fbed3a..788c13147f63 100644
> --- a/drivers/phy/mediatek/Makefile
> +++ b/drivers/phy/mediatek/Makefile
> @@ -6,6 +6,7 @@
> obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
> obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
> obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
> +obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
>
> phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
> phy-mtk-hdmi-drv-y += phy-mtk-hdmi-mt2701.o
> diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c b/drivers/phy/mediatek/phy-mtk-pcie.c
> new file mode 100644
> index 000000000000..0f5d7c7e2b7e
> --- /dev/null
> +++ b/drivers/phy/mediatek/phy-mtk-pcie.c
> @@ -0,0 +1,246 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 MediaTek Inc.
> + * Author: Jianjun Wang <jianjun.wang@mediatek.com>
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/compiler_types.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "phy-mtk-io.h"
> +
> +#define PEXTP_ANA_GLB_00_REG 0x9000
> +#define PEXTP_ANA_LN0_TRX_REG 0xa000
> +#define PEXTP_ANA_TX_OFFSET 0x04
> +#define PEXTP_ANA_RX_OFFSET 0x3c
> +#define PEXTP_ANA_LANE_OFFSET 0x100
> +
> +/* PEXTP_GLB_00_REG[28:24] Internal Resistor Selection of TX Bias Current */
> +#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
> +#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
> +
> +/* PEXTP_ANA_LN_RX_REG[3:0] RX impedance selection */
> +#define EFUSE_LN_RX_SEL GENMASK(3, 0)
> +#define EFUSE_LN_RX_VAL(x) (0xf & (x))
> +
> +/* PEXTP_ANA_LN_TX_REG[5:2] TX PMOS impedance selection */
> +#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
> +#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
> +
> +/* PEXTP_ANA_LN_TX_REG[11:8] TX NMOS impedance selection */
> +#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
> +#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
> +
> +/* Efuse data for each lane */
What about some kerneldoc?
/**
* struct mtk_pcie_lane_efuse - eFuse data for each lane
* @tx_pmos:
......etc :))
> +struct mtk_pcie_lane_efuse {
> + u32 tx_pmos;
> + u32 tx_nmos;
> + u32 rx_data;
> + bool lane_efuse_supported;
> +};
> +
Same here
/**
* struct mtk_pcie_phy - PCIe phy driver main structure
* @dev: ......
> +struct mtk_pcie_phy {
> + struct device *dev;
> + struct phy *phy;
> + void __iomem *sif_base;
> +
> + /*
> + * Support software efuse initialization,
> + * currently we only support 2 lane in maximum.
> + */
Obviously, if you add kerneldoc, this comment would get moved to that kerneldoc.
> + bool sw_efuse_supported;
> + u32 efuse_glb_intr;
> + struct mtk_pcie_lane_efuse efuse[2];
If you dynamically allocate this one, you will be able to support any number
of lanes, futureproofing this driver and giving it more flexibility.
> +};
> +
..snip..
> +
> +static int mtk_pcie_efuse_read_for_lane(struct mtk_pcie_phy *pcie_phy,
> + unsigned int lane)
> +{
> + struct device *dev = pcie_phy->dev;
> + struct mtk_pcie_lane_efuse *data;
> + char efuse_id[15];
> + int ret;
> +
> + if (lane >= ARRAY_SIZE(pcie_phy->efuse))
> + return dev_err_probe(pcie_phy->dev, -EINVAL,
> + "Requested lane number %d exceeds maximum %ld\n",
> + lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
I don't like seeing dev_err_probe() outside of a probe function, but I acknowledge
that the Linux documentation doesn't seem to give any direction about that, so
this is a personal preference, at this point.
> +
> + data = &pcie_phy->efuse[lane];
> +
> + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_pmos", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_pmos);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_nmos", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_nmos);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + snprintf(efuse_id, sizeof(efuse_id), "rx_ln%d", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->rx_data);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + if (!(data->tx_pmos || data->tx_nmos || data->rx_data))
> + return dev_err_probe(dev, -EINVAL,
> + "No efuse data found for lane%d, but dts enable it\n",
> + lane);
> +
> + data->lane_efuse_supported = true;
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_read_efuse(struct mtk_pcie_phy *pcie_phy)
> +{
> + struct device *dev = pcie_phy->dev;
> + bool nvmem_enabled;
> + int ret;
> +
> + nvmem_enabled = device_property_read_bool(dev, "nvmem-cells");
> + if (!nvmem_enabled)
> + return -ENODEV;
> +
> + ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr",
> + &pcie_phy->efuse_glb_intr);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read glb_intr\n");
> +
> + pcie_phy->sw_efuse_supported = true;
> +
> + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 0);
> + if (ret)
> + return ret;
> +
> + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 1);
> + if (ret)
> + return ret;
To give some more future-proofing to this driver, I would instead either add a
u32 devicetree property "num-lanes" or, if the same SoC may not have a different
number of lanes across controller instances, I would add a number of lanes
parameter as data for each of_match.
You'd be at that point using a for loop here like:
for (i = 0; i < pcie_phy->num_lanes, i++) {
ret = mtk_pcie_efuse_read_for_lane(pcie_phy, i);
if (ret)
return ret;
}
Of course, the same logic would apply to mtk_pcie_phy_init(), where you are
instead calling mtk_pcie_efuse_set_lane().
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_phy_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct phy_provider *provider;
> + struct mtk_pcie_phy *pcie_phy;
> +
> + pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy), GFP_KERNEL);
> + if (!pcie_phy)
> + return -ENOMEM;
> +
> + pcie_phy->dev = dev;
> +
> + pcie_phy->sif_base = devm_platform_ioremap_resource_byname(pdev, "sif");
> + if (IS_ERR(pcie_phy->sif_base))
> + return dev_err_probe(dev, PTR_ERR(pcie_phy->sif_base),
> + "Failed to map phy-sif base\n");
> +
> + pcie_phy->phy = devm_phy_create(dev, dev->of_node, &mtk_pcie_phy_ops);
> + if (IS_ERR(pcie_phy->phy))
> + return dev_err_probe(dev, PTR_ERR(pcie_phy->phy),
> + "Failed to create PCIe phy\n");
> +
> + /*
> + * Failed to read the efuse data is not a fatal problem,
> + * ignore the failure and keep going.
> + */
> + mtk_pcie_read_efuse(pcie_phy);
If you get an -EPROBE_DEFER here, you surely want to defer probing this driver,
so, yes you're free to ignore the other failures, but you should fix that corner
case.
Everything else looks good, so, please make sure to add me to the Cc's for the
next version of this series for me to give you a faster review.
Regards,
Angelo
_______________________________________________
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] 52+ messages in thread* Re: [PATCH v2 2/2] phy: mediatek: Add PCIe PHY driver
@ 2022-03-18 10:57 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 52+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-03-18 10:57 UTC (permalink / raw)
To: Jianjun Wang, Chunfeng Yun, Kishon Vijay Abraham I, Vinod Koul,
Rob Herring, Matthias Brugger, Chen-Yu Tsai
Cc: linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
linux-kernel, rex-bc.chen, randy.wu, jieyy.yang, chuanjia.liu,
qizhong.cheng, jian.yang
Il 18/03/22 10:54, Jianjun Wang ha scritto:
> Add PCIe GEN3 PHY driver support on MediaTek chipsets.
>
> Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> ---
> drivers/phy/mediatek/Kconfig | 11 ++
> drivers/phy/mediatek/Makefile | 1 +
> drivers/phy/mediatek/phy-mtk-pcie.c | 246 ++++++++++++++++++++++++++++
> 3 files changed, 258 insertions(+)
> create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
>
> diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
> index 55f8e6c048ab..387ed1b3f2cc 100644
> --- a/drivers/phy/mediatek/Kconfig
> +++ b/drivers/phy/mediatek/Kconfig
> @@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
> select GENERIC_PHY
> help
> Support MIPI DSI for Mediatek SoCs.
> +
> +config PHY_MTK_PCIE
> + tristate "MediaTek PCIe-PHY Driver"
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + depends on OF
> + select GENERIC_PHY
> + help
> + Say 'Y' here to add support for MediaTek PCIe PHY driver.
> + This driver create the basic PHY instance and provides initialize
> + callback for PCIe GEN3 port, it supports software efuse
> + initialization.
> diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
> index ace660fbed3a..788c13147f63 100644
> --- a/drivers/phy/mediatek/Makefile
> +++ b/drivers/phy/mediatek/Makefile
> @@ -6,6 +6,7 @@
> obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
> obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
> obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
> +obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
>
> phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
> phy-mtk-hdmi-drv-y += phy-mtk-hdmi-mt2701.o
> diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c b/drivers/phy/mediatek/phy-mtk-pcie.c
> new file mode 100644
> index 000000000000..0f5d7c7e2b7e
> --- /dev/null
> +++ b/drivers/phy/mediatek/phy-mtk-pcie.c
> @@ -0,0 +1,246 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 MediaTek Inc.
> + * Author: Jianjun Wang <jianjun.wang@mediatek.com>
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/compiler_types.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "phy-mtk-io.h"
> +
> +#define PEXTP_ANA_GLB_00_REG 0x9000
> +#define PEXTP_ANA_LN0_TRX_REG 0xa000
> +#define PEXTP_ANA_TX_OFFSET 0x04
> +#define PEXTP_ANA_RX_OFFSET 0x3c
> +#define PEXTP_ANA_LANE_OFFSET 0x100
> +
> +/* PEXTP_GLB_00_REG[28:24] Internal Resistor Selection of TX Bias Current */
> +#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
> +#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
> +
> +/* PEXTP_ANA_LN_RX_REG[3:0] RX impedance selection */
> +#define EFUSE_LN_RX_SEL GENMASK(3, 0)
> +#define EFUSE_LN_RX_VAL(x) (0xf & (x))
> +
> +/* PEXTP_ANA_LN_TX_REG[5:2] TX PMOS impedance selection */
> +#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
> +#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
> +
> +/* PEXTP_ANA_LN_TX_REG[11:8] TX NMOS impedance selection */
> +#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
> +#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
> +
> +/* Efuse data for each lane */
What about some kerneldoc?
/**
* struct mtk_pcie_lane_efuse - eFuse data for each lane
* @tx_pmos:
......etc :))
> +struct mtk_pcie_lane_efuse {
> + u32 tx_pmos;
> + u32 tx_nmos;
> + u32 rx_data;
> + bool lane_efuse_supported;
> +};
> +
Same here
/**
* struct mtk_pcie_phy - PCIe phy driver main structure
* @dev: ......
> +struct mtk_pcie_phy {
> + struct device *dev;
> + struct phy *phy;
> + void __iomem *sif_base;
> +
> + /*
> + * Support software efuse initialization,
> + * currently we only support 2 lane in maximum.
> + */
Obviously, if you add kerneldoc, this comment would get moved to that kerneldoc.
> + bool sw_efuse_supported;
> + u32 efuse_glb_intr;
> + struct mtk_pcie_lane_efuse efuse[2];
If you dynamically allocate this one, you will be able to support any number
of lanes, futureproofing this driver and giving it more flexibility.
> +};
> +
..snip..
> +
> +static int mtk_pcie_efuse_read_for_lane(struct mtk_pcie_phy *pcie_phy,
> + unsigned int lane)
> +{
> + struct device *dev = pcie_phy->dev;
> + struct mtk_pcie_lane_efuse *data;
> + char efuse_id[15];
> + int ret;
> +
> + if (lane >= ARRAY_SIZE(pcie_phy->efuse))
> + return dev_err_probe(pcie_phy->dev, -EINVAL,
> + "Requested lane number %d exceeds maximum %ld\n",
> + lane, ARRAY_SIZE(pcie_phy->efuse) - 1);
I don't like seeing dev_err_probe() outside of a probe function, but I acknowledge
that the Linux documentation doesn't seem to give any direction about that, so
this is a personal preference, at this point.
> +
> + data = &pcie_phy->efuse[lane];
> +
> + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_pmos", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_pmos);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_nmos", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->tx_nmos);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + snprintf(efuse_id, sizeof(efuse_id), "rx_ln%d", lane);
> + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data->rx_data);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read %s\n", efuse_id);
> +
> + if (!(data->tx_pmos || data->tx_nmos || data->rx_data))
> + return dev_err_probe(dev, -EINVAL,
> + "No efuse data found for lane%d, but dts enable it\n",
> + lane);
> +
> + data->lane_efuse_supported = true;
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_read_efuse(struct mtk_pcie_phy *pcie_phy)
> +{
> + struct device *dev = pcie_phy->dev;
> + bool nvmem_enabled;
> + int ret;
> +
> + nvmem_enabled = device_property_read_bool(dev, "nvmem-cells");
> + if (!nvmem_enabled)
> + return -ENODEV;
> +
> + ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr",
> + &pcie_phy->efuse_glb_intr);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read glb_intr\n");
> +
> + pcie_phy->sw_efuse_supported = true;
> +
> + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 0);
> + if (ret)
> + return ret;
> +
> + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 1);
> + if (ret)
> + return ret;
To give some more future-proofing to this driver, I would instead either add a
u32 devicetree property "num-lanes" or, if the same SoC may not have a different
number of lanes across controller instances, I would add a number of lanes
parameter as data for each of_match.
You'd be at that point using a for loop here like:
for (i = 0; i < pcie_phy->num_lanes, i++) {
ret = mtk_pcie_efuse_read_for_lane(pcie_phy, i);
if (ret)
return ret;
}
Of course, the same logic would apply to mtk_pcie_phy_init(), where you are
instead calling mtk_pcie_efuse_set_lane().
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_phy_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct phy_provider *provider;
> + struct mtk_pcie_phy *pcie_phy;
> +
> + pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy), GFP_KERNEL);
> + if (!pcie_phy)
> + return -ENOMEM;
> +
> + pcie_phy->dev = dev;
> +
> + pcie_phy->sif_base = devm_platform_ioremap_resource_byname(pdev, "sif");
> + if (IS_ERR(pcie_phy->sif_base))
> + return dev_err_probe(dev, PTR_ERR(pcie_phy->sif_base),
> + "Failed to map phy-sif base\n");
> +
> + pcie_phy->phy = devm_phy_create(dev, dev->of_node, &mtk_pcie_phy_ops);
> + if (IS_ERR(pcie_phy->phy))
> + return dev_err_probe(dev, PTR_ERR(pcie_phy->phy),
> + "Failed to create PCIe phy\n");
> +
> + /*
> + * Failed to read the efuse data is not a fatal problem,
> + * ignore the failure and keep going.
> + */
> + mtk_pcie_read_efuse(pcie_phy);
If you get an -EPROBE_DEFER here, you surely want to defer probing this driver,
so, yes you're free to ignore the other failures, but you should fix that corner
case.
Everything else looks good, so, please make sure to add me to the Cc's for the
next version of this series for me to give you a faster review.
Regards,
Angelo
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH v2 2/2] phy: mediatek: Add PCIe PHY driver
2022-03-18 10:57 ` AngeloGioacchino Del Regno
(?)
(?)
@ 2022-03-18 11:48 ` Jianjun Wang
-1 siblings, 0 replies; 52+ messages in thread
From: Jianjun Wang @ 2022-03-18 11:48 UTC (permalink / raw)
To: AngeloGioacchino Del Regno, Chunfeng Yun, Kishon Vijay Abraham I,
Vinod Koul, Rob Herring, Matthias Brugger, Chen-Yu Tsai
Cc: linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
linux-kernel, rex-bc.chen, randy.wu, jieyy.yang, chuanjia.liu,
qizhong.cheng, jian.yang
Hi Angelo,
Thanks for all these suggestions, I'll fix them in the next version.
Thanks.
On Fri, 2022-03-18 at 11:57 +0100, AngeloGioacchino Del Regno wrote:
> Il 18/03/22 10:54, Jianjun Wang ha scritto:
> > Add PCIe GEN3 PHY driver support on MediaTek chipsets.
> >
> > Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> > ---
> > drivers/phy/mediatek/Kconfig | 11 ++
> > drivers/phy/mediatek/Makefile | 1 +
> > drivers/phy/mediatek/phy-mtk-pcie.c | 246
> > ++++++++++++++++++++++++++++
> > 3 files changed, 258 insertions(+)
> > create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
> >
> > diff --git a/drivers/phy/mediatek/Kconfig
> > b/drivers/phy/mediatek/Kconfig
> > index 55f8e6c048ab..387ed1b3f2cc 100644
> > --- a/drivers/phy/mediatek/Kconfig
> > +++ b/drivers/phy/mediatek/Kconfig
> > @@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
> > select GENERIC_PHY
> > help
> > Support MIPI DSI for Mediatek SoCs.
> > +
> > +config PHY_MTK_PCIE
> > + tristate "MediaTek PCIe-PHY Driver"
> > + depends on ARCH_MEDIATEK || COMPILE_TEST
> > + depends on OF
> > + select GENERIC_PHY
> > + help
> > + Say 'Y' here to add support for MediaTek PCIe PHY driver.
> > + This driver create the basic PHY instance and provides
> > initialize
> > + callback for PCIe GEN3 port, it supports software efuse
> > + initialization.
> > diff --git a/drivers/phy/mediatek/Makefile
> > b/drivers/phy/mediatek/Makefile
> > index ace660fbed3a..788c13147f63 100644
> > --- a/drivers/phy/mediatek/Makefile
> > +++ b/drivers/phy/mediatek/Makefile
> > @@ -6,6 +6,7 @@
> > obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
> > obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
> > obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
> > +obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
> >
> > phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
> > phy-mtk-hdmi-drv-y += phy-mtk-hdmi-
> > mt2701.o
> > diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c
> > b/drivers/phy/mediatek/phy-mtk-pcie.c
> > new file mode 100644
> > index 000000000000..0f5d7c7e2b7e
> > --- /dev/null
> > +++ b/drivers/phy/mediatek/phy-mtk-pcie.c
> > @@ -0,0 +1,246 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2022 MediaTek Inc.
> > + * Author: Jianjun Wang <jianjun.wang@mediatek.com>
> > + */
> > +
> > +#include <linux/bits.h>
> > +#include <linux/compiler_types.h>
> > +#include <linux/module.h>
> > +#include <linux/nvmem-consumer.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +
> > +#include "phy-mtk-io.h"
> > +
> > +#define PEXTP_ANA_GLB_00_REG 0x9000
> > +#define PEXTP_ANA_LN0_TRX_REG 0xa000
> > +#define PEXTP_ANA_TX_OFFSET 0x04
> > +#define PEXTP_ANA_RX_OFFSET 0x3c
> > +#define PEXTP_ANA_LANE_OFFSET 0x100
> > +
> > +/* PEXTP_GLB_00_REG[28:24] Internal Resistor Selection of TX Bias
> > Current */
> > +#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
> > +#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
> > +
> > +/* PEXTP_ANA_LN_RX_REG[3:0] RX impedance selection */
> > +#define EFUSE_LN_RX_SEL GENMASK(3, 0)
> > +#define EFUSE_LN_RX_VAL(x) (0xf & (x))
> > +
> > +/* PEXTP_ANA_LN_TX_REG[5:2] TX PMOS impedance selection */
> > +#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
> > +#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
> > +
> > +/* PEXTP_ANA_LN_TX_REG[11:8] TX NMOS impedance selection */
> > +#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
> > +#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
> > +
> > +/* Efuse data for each lane */
>
> What about some kerneldoc?
>
> /**
> * struct mtk_pcie_lane_efuse - eFuse data for each lane
> * @tx_pmos:
> ......etc :))
>
> > +struct mtk_pcie_lane_efuse {
> > + u32 tx_pmos;
> > + u32 tx_nmos;
> > + u32 rx_data;
> > + bool lane_efuse_supported;
> > +};
> > +
>
> Same here
>
> /**
> * struct mtk_pcie_phy - PCIe phy driver main structure
> * @dev: ......
>
> > +struct mtk_pcie_phy {
> > + struct device *dev;
> > + struct phy *phy;
> > + void __iomem *sif_base;
> > +
> > + /*
> > + * Support software efuse initialization,
> > + * currently we only support 2 lane in maximum.
> > + */
>
> Obviously, if you add kerneldoc, this comment would get moved to that
> kerneldoc.
>
> > + bool sw_efuse_supported;
> > + u32 efuse_glb_intr;
>
>
> > + struct mtk_pcie_lane_efuse efuse[2];
>
> If you dynamically allocate this one, you will be able to support any
> number
> of lanes, futureproofing this driver and giving it more flexibility.
>
> > +};
> > +
>
> ..snip..
>
> > +
> > +static int mtk_pcie_efuse_read_for_lane(struct mtk_pcie_phy
> > *pcie_phy,
> > + unsigned int lane)
> > +{
> > + struct device *dev = pcie_phy->dev;
> > + struct mtk_pcie_lane_efuse *data;
> > + char efuse_id[15];
> > + int ret;
> > +
> > + if (lane >= ARRAY_SIZE(pcie_phy->efuse))
> > + return dev_err_probe(pcie_phy->dev, -EINVAL,
> > + "Requested lane number %d exceeds
> > maximum %ld\n",
> > + lane, ARRAY_SIZE(pcie_phy->efuse)
> > - 1);
>
> I don't like seeing dev_err_probe() outside of a probe function, but
> I acknowledge
> that the Linux documentation doesn't seem to give any direction about
> that, so
> this is a personal preference, at this point.
>
> > +
> > + data = &pcie_phy->efuse[lane];
> > +
> > + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_pmos", lane);
> > + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data-
> > >tx_pmos);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to read %s\n",
> > efuse_id);
> > +
> > + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_nmos", lane);
> > + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data-
> > >tx_nmos);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to read %s\n",
> > efuse_id);
> > +
> > + snprintf(efuse_id, sizeof(efuse_id), "rx_ln%d", lane);
> > + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data-
> > >rx_data);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to read %s\n",
> > efuse_id);
> > +
> > + if (!(data->tx_pmos || data->tx_nmos || data->rx_data))
> > + return dev_err_probe(dev, -EINVAL,
> > + "No efuse data found for lane%d,
> > but dts enable it\n",
> > + lane);
> > +
> > + data->lane_efuse_supported = true;
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_pcie_read_efuse(struct mtk_pcie_phy *pcie_phy)
> > +{
> > + struct device *dev = pcie_phy->dev;
> > + bool nvmem_enabled;
> > + int ret;
> > +
> > + nvmem_enabled = device_property_read_bool(dev, "nvmem-cells");
> > + if (!nvmem_enabled)
> > + return -ENODEV;
> > +
> > + ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr",
> > + &pcie_phy-
> > >efuse_glb_intr);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to read
> > glb_intr\n");
> > +
> > + pcie_phy->sw_efuse_supported = true;
> > +
> > + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 0);
> > + if (ret)
> > + return ret;
> > +
> > + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 1);
> > + if (ret)
> > + return ret;
>
> To give some more future-proofing to this driver, I would instead
> either add a
> u32 devicetree property "num-lanes" or, if the same SoC may not have
> a different
> number of lanes across controller instances, I would add a number of
> lanes
> parameter as data for each of_match.
>
> You'd be at that point using a for loop here like:
>
> for (i = 0; i < pcie_phy->num_lanes, i++) {
> ret = mtk_pcie_efuse_read_for_lane(pcie_phy, i);
> if (ret)
> return ret;
> }
>
> Of course, the same logic would apply to mtk_pcie_phy_init(), where
> you are
> instead calling mtk_pcie_efuse_set_lane().
>
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_pcie_phy_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct phy_provider *provider;
> > + struct mtk_pcie_phy *pcie_phy;
> > +
> > + pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy), GFP_KERNEL);
> > + if (!pcie_phy)
> > + return -ENOMEM;
> > +
> > + pcie_phy->dev = dev;
> > +
> > + pcie_phy->sif_base =
> > devm_platform_ioremap_resource_byname(pdev, "sif");
> > + if (IS_ERR(pcie_phy->sif_base))
> > + return dev_err_probe(dev, PTR_ERR(pcie_phy->sif_base),
> > + "Failed to map phy-sif base\n");
> > +
> > + pcie_phy->phy = devm_phy_create(dev, dev->of_node,
> > &mtk_pcie_phy_ops);
> > + if (IS_ERR(pcie_phy->phy))
> > + return dev_err_probe(dev, PTR_ERR(pcie_phy->phy),
> > + "Failed to create PCIe phy\n");
> > +
> > + /*
> > + * Failed to read the efuse data is not a fatal problem,
> > + * ignore the failure and keep going.
> > + */
> > + mtk_pcie_read_efuse(pcie_phy);
>
> If you get an -EPROBE_DEFER here, you surely want to defer probing
> this driver,
> so, yes you're free to ignore the other failures, but you should fix
> that corner
> case.
>
> Everything else looks good, so, please make sure to add me to the
> Cc's for the
> next version of this series for me to give you a faster review.
>
> Regards,
> Angelo
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH v2 2/2] phy: mediatek: Add PCIe PHY driver
@ 2022-03-18 11:48 ` Jianjun Wang
0 siblings, 0 replies; 52+ messages in thread
From: Jianjun Wang @ 2022-03-18 11:48 UTC (permalink / raw)
To: AngeloGioacchino Del Regno, Chunfeng Yun, Kishon Vijay Abraham I,
Vinod Koul, Rob Herring, Matthias Brugger, Chen-Yu Tsai
Cc: linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
linux-kernel, rex-bc.chen, randy.wu, jieyy.yang, chuanjia.liu,
qizhong.cheng, jian.yang
Hi Angelo,
Thanks for all these suggestions, I'll fix them in the next version.
Thanks.
On Fri, 2022-03-18 at 11:57 +0100, AngeloGioacchino Del Regno wrote:
> Il 18/03/22 10:54, Jianjun Wang ha scritto:
> > Add PCIe GEN3 PHY driver support on MediaTek chipsets.
> >
> > Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> > ---
> > drivers/phy/mediatek/Kconfig | 11 ++
> > drivers/phy/mediatek/Makefile | 1 +
> > drivers/phy/mediatek/phy-mtk-pcie.c | 246
> > ++++++++++++++++++++++++++++
> > 3 files changed, 258 insertions(+)
> > create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
> >
> > diff --git a/drivers/phy/mediatek/Kconfig
> > b/drivers/phy/mediatek/Kconfig
> > index 55f8e6c048ab..387ed1b3f2cc 100644
> > --- a/drivers/phy/mediatek/Kconfig
> > +++ b/drivers/phy/mediatek/Kconfig
> > @@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
> > select GENERIC_PHY
> > help
> > Support MIPI DSI for Mediatek SoCs.
> > +
> > +config PHY_MTK_PCIE
> > + tristate "MediaTek PCIe-PHY Driver"
> > + depends on ARCH_MEDIATEK || COMPILE_TEST
> > + depends on OF
> > + select GENERIC_PHY
> > + help
> > + Say 'Y' here to add support for MediaTek PCIe PHY driver.
> > + This driver create the basic PHY instance and provides
> > initialize
> > + callback for PCIe GEN3 port, it supports software efuse
> > + initialization.
> > diff --git a/drivers/phy/mediatek/Makefile
> > b/drivers/phy/mediatek/Makefile
> > index ace660fbed3a..788c13147f63 100644
> > --- a/drivers/phy/mediatek/Makefile
> > +++ b/drivers/phy/mediatek/Makefile
> > @@ -6,6 +6,7 @@
> > obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
> > obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
> > obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
> > +obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
> >
> > phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
> > phy-mtk-hdmi-drv-y += phy-mtk-hdmi-
> > mt2701.o
> > diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c
> > b/drivers/phy/mediatek/phy-mtk-pcie.c
> > new file mode 100644
> > index 000000000000..0f5d7c7e2b7e
> > --- /dev/null
> > +++ b/drivers/phy/mediatek/phy-mtk-pcie.c
> > @@ -0,0 +1,246 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2022 MediaTek Inc.
> > + * Author: Jianjun Wang <jianjun.wang@mediatek.com>
> > + */
> > +
> > +#include <linux/bits.h>
> > +#include <linux/compiler_types.h>
> > +#include <linux/module.h>
> > +#include <linux/nvmem-consumer.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +
> > +#include "phy-mtk-io.h"
> > +
> > +#define PEXTP_ANA_GLB_00_REG 0x9000
> > +#define PEXTP_ANA_LN0_TRX_REG 0xa000
> > +#define PEXTP_ANA_TX_OFFSET 0x04
> > +#define PEXTP_ANA_RX_OFFSET 0x3c
> > +#define PEXTP_ANA_LANE_OFFSET 0x100
> > +
> > +/* PEXTP_GLB_00_REG[28:24] Internal Resistor Selection of TX Bias
> > Current */
> > +#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
> > +#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
> > +
> > +/* PEXTP_ANA_LN_RX_REG[3:0] RX impedance selection */
> > +#define EFUSE_LN_RX_SEL GENMASK(3, 0)
> > +#define EFUSE_LN_RX_VAL(x) (0xf & (x))
> > +
> > +/* PEXTP_ANA_LN_TX_REG[5:2] TX PMOS impedance selection */
> > +#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
> > +#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
> > +
> > +/* PEXTP_ANA_LN_TX_REG[11:8] TX NMOS impedance selection */
> > +#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
> > +#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
> > +
> > +/* Efuse data for each lane */
>
> What about some kerneldoc?
>
> /**
> * struct mtk_pcie_lane_efuse - eFuse data for each lane
> * @tx_pmos:
> ......etc :))
>
> > +struct mtk_pcie_lane_efuse {
> > + u32 tx_pmos;
> > + u32 tx_nmos;
> > + u32 rx_data;
> > + bool lane_efuse_supported;
> > +};
> > +
>
> Same here
>
> /**
> * struct mtk_pcie_phy - PCIe phy driver main structure
> * @dev: ......
>
> > +struct mtk_pcie_phy {
> > + struct device *dev;
> > + struct phy *phy;
> > + void __iomem *sif_base;
> > +
> > + /*
> > + * Support software efuse initialization,
> > + * currently we only support 2 lane in maximum.
> > + */
>
> Obviously, if you add kerneldoc, this comment would get moved to that
> kerneldoc.
>
> > + bool sw_efuse_supported;
> > + u32 efuse_glb_intr;
>
>
> > + struct mtk_pcie_lane_efuse efuse[2];
>
> If you dynamically allocate this one, you will be able to support any
> number
> of lanes, futureproofing this driver and giving it more flexibility.
>
> > +};
> > +
>
> ..snip..
>
> > +
> > +static int mtk_pcie_efuse_read_for_lane(struct mtk_pcie_phy
> > *pcie_phy,
> > + unsigned int lane)
> > +{
> > + struct device *dev = pcie_phy->dev;
> > + struct mtk_pcie_lane_efuse *data;
> > + char efuse_id[15];
> > + int ret;
> > +
> > + if (lane >= ARRAY_SIZE(pcie_phy->efuse))
> > + return dev_err_probe(pcie_phy->dev, -EINVAL,
> > + "Requested lane number %d exceeds
> > maximum %ld\n",
> > + lane, ARRAY_SIZE(pcie_phy->efuse)
> > - 1);
>
> I don't like seeing dev_err_probe() outside of a probe function, but
> I acknowledge
> that the Linux documentation doesn't seem to give any direction about
> that, so
> this is a personal preference, at this point.
>
> > +
> > + data = &pcie_phy->efuse[lane];
> > +
> > + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_pmos", lane);
> > + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data-
> > >tx_pmos);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to read %s\n",
> > efuse_id);
> > +
> > + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_nmos", lane);
> > + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data-
> > >tx_nmos);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to read %s\n",
> > efuse_id);
> > +
> > + snprintf(efuse_id, sizeof(efuse_id), "rx_ln%d", lane);
> > + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data-
> > >rx_data);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to read %s\n",
> > efuse_id);
> > +
> > + if (!(data->tx_pmos || data->tx_nmos || data->rx_data))
> > + return dev_err_probe(dev, -EINVAL,
> > + "No efuse data found for lane%d,
> > but dts enable it\n",
> > + lane);
> > +
> > + data->lane_efuse_supported = true;
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_pcie_read_efuse(struct mtk_pcie_phy *pcie_phy)
> > +{
> > + struct device *dev = pcie_phy->dev;
> > + bool nvmem_enabled;
> > + int ret;
> > +
> > + nvmem_enabled = device_property_read_bool(dev, "nvmem-cells");
> > + if (!nvmem_enabled)
> > + return -ENODEV;
> > +
> > + ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr",
> > + &pcie_phy-
> > >efuse_glb_intr);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to read
> > glb_intr\n");
> > +
> > + pcie_phy->sw_efuse_supported = true;
> > +
> > + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 0);
> > + if (ret)
> > + return ret;
> > +
> > + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 1);
> > + if (ret)
> > + return ret;
>
> To give some more future-proofing to this driver, I would instead
> either add a
> u32 devicetree property "num-lanes" or, if the same SoC may not have
> a different
> number of lanes across controller instances, I would add a number of
> lanes
> parameter as data for each of_match.
>
> You'd be at that point using a for loop here like:
>
> for (i = 0; i < pcie_phy->num_lanes, i++) {
> ret = mtk_pcie_efuse_read_for_lane(pcie_phy, i);
> if (ret)
> return ret;
> }
>
> Of course, the same logic would apply to mtk_pcie_phy_init(), where
> you are
> instead calling mtk_pcie_efuse_set_lane().
>
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_pcie_phy_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct phy_provider *provider;
> > + struct mtk_pcie_phy *pcie_phy;
> > +
> > + pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy), GFP_KERNEL);
> > + if (!pcie_phy)
> > + return -ENOMEM;
> > +
> > + pcie_phy->dev = dev;
> > +
> > + pcie_phy->sif_base =
> > devm_platform_ioremap_resource_byname(pdev, "sif");
> > + if (IS_ERR(pcie_phy->sif_base))
> > + return dev_err_probe(dev, PTR_ERR(pcie_phy->sif_base),
> > + "Failed to map phy-sif base\n");
> > +
> > + pcie_phy->phy = devm_phy_create(dev, dev->of_node,
> > &mtk_pcie_phy_ops);
> > + if (IS_ERR(pcie_phy->phy))
> > + return dev_err_probe(dev, PTR_ERR(pcie_phy->phy),
> > + "Failed to create PCIe phy\n");
> > +
> > + /*
> > + * Failed to read the efuse data is not a fatal problem,
> > + * ignore the failure and keep going.
> > + */
> > + mtk_pcie_read_efuse(pcie_phy);
>
> If you get an -EPROBE_DEFER here, you surely want to defer probing
> this driver,
> so, yes you're free to ignore the other failures, but you should fix
> that corner
> case.
>
> Everything else looks good, so, please make sure to add me to the
> Cc's for the
> next version of this series for me to give you a faster review.
>
> Regards,
> Angelo
>
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH v2 2/2] phy: mediatek: Add PCIe PHY driver
@ 2022-03-18 11:48 ` Jianjun Wang
0 siblings, 0 replies; 52+ messages in thread
From: Jianjun Wang @ 2022-03-18 11:48 UTC (permalink / raw)
To: AngeloGioacchino Del Regno, Chunfeng Yun, Kishon Vijay Abraham I,
Vinod Koul, Rob Herring, Matthias Brugger, Chen-Yu Tsai
Cc: linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
linux-kernel, rex-bc.chen, randy.wu, jieyy.yang, chuanjia.liu,
qizhong.cheng, jian.yang
Hi Angelo,
Thanks for all these suggestions, I'll fix them in the next version.
Thanks.
On Fri, 2022-03-18 at 11:57 +0100, AngeloGioacchino Del Regno wrote:
> Il 18/03/22 10:54, Jianjun Wang ha scritto:
> > Add PCIe GEN3 PHY driver support on MediaTek chipsets.
> >
> > Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> > ---
> > drivers/phy/mediatek/Kconfig | 11 ++
> > drivers/phy/mediatek/Makefile | 1 +
> > drivers/phy/mediatek/phy-mtk-pcie.c | 246
> > ++++++++++++++++++++++++++++
> > 3 files changed, 258 insertions(+)
> > create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
> >
> > diff --git a/drivers/phy/mediatek/Kconfig
> > b/drivers/phy/mediatek/Kconfig
> > index 55f8e6c048ab..387ed1b3f2cc 100644
> > --- a/drivers/phy/mediatek/Kconfig
> > +++ b/drivers/phy/mediatek/Kconfig
> > @@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
> > select GENERIC_PHY
> > help
> > Support MIPI DSI for Mediatek SoCs.
> > +
> > +config PHY_MTK_PCIE
> > + tristate "MediaTek PCIe-PHY Driver"
> > + depends on ARCH_MEDIATEK || COMPILE_TEST
> > + depends on OF
> > + select GENERIC_PHY
> > + help
> > + Say 'Y' here to add support for MediaTek PCIe PHY driver.
> > + This driver create the basic PHY instance and provides
> > initialize
> > + callback for PCIe GEN3 port, it supports software efuse
> > + initialization.
> > diff --git a/drivers/phy/mediatek/Makefile
> > b/drivers/phy/mediatek/Makefile
> > index ace660fbed3a..788c13147f63 100644
> > --- a/drivers/phy/mediatek/Makefile
> > +++ b/drivers/phy/mediatek/Makefile
> > @@ -6,6 +6,7 @@
> > obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
> > obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
> > obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
> > +obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
> >
> > phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
> > phy-mtk-hdmi-drv-y += phy-mtk-hdmi-
> > mt2701.o
> > diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c
> > b/drivers/phy/mediatek/phy-mtk-pcie.c
> > new file mode 100644
> > index 000000000000..0f5d7c7e2b7e
> > --- /dev/null
> > +++ b/drivers/phy/mediatek/phy-mtk-pcie.c
> > @@ -0,0 +1,246 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2022 MediaTek Inc.
> > + * Author: Jianjun Wang <jianjun.wang@mediatek.com>
> > + */
> > +
> > +#include <linux/bits.h>
> > +#include <linux/compiler_types.h>
> > +#include <linux/module.h>
> > +#include <linux/nvmem-consumer.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +
> > +#include "phy-mtk-io.h"
> > +
> > +#define PEXTP_ANA_GLB_00_REG 0x9000
> > +#define PEXTP_ANA_LN0_TRX_REG 0xa000
> > +#define PEXTP_ANA_TX_OFFSET 0x04
> > +#define PEXTP_ANA_RX_OFFSET 0x3c
> > +#define PEXTP_ANA_LANE_OFFSET 0x100
> > +
> > +/* PEXTP_GLB_00_REG[28:24] Internal Resistor Selection of TX Bias
> > Current */
> > +#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
> > +#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
> > +
> > +/* PEXTP_ANA_LN_RX_REG[3:0] RX impedance selection */
> > +#define EFUSE_LN_RX_SEL GENMASK(3, 0)
> > +#define EFUSE_LN_RX_VAL(x) (0xf & (x))
> > +
> > +/* PEXTP_ANA_LN_TX_REG[5:2] TX PMOS impedance selection */
> > +#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
> > +#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
> > +
> > +/* PEXTP_ANA_LN_TX_REG[11:8] TX NMOS impedance selection */
> > +#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
> > +#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
> > +
> > +/* Efuse data for each lane */
>
> What about some kerneldoc?
>
> /**
> * struct mtk_pcie_lane_efuse - eFuse data for each lane
> * @tx_pmos:
> ......etc :))
>
> > +struct mtk_pcie_lane_efuse {
> > + u32 tx_pmos;
> > + u32 tx_nmos;
> > + u32 rx_data;
> > + bool lane_efuse_supported;
> > +};
> > +
>
> Same here
>
> /**
> * struct mtk_pcie_phy - PCIe phy driver main structure
> * @dev: ......
>
> > +struct mtk_pcie_phy {
> > + struct device *dev;
> > + struct phy *phy;
> > + void __iomem *sif_base;
> > +
> > + /*
> > + * Support software efuse initialization,
> > + * currently we only support 2 lane in maximum.
> > + */
>
> Obviously, if you add kerneldoc, this comment would get moved to that
> kerneldoc.
>
> > + bool sw_efuse_supported;
> > + u32 efuse_glb_intr;
>
>
> > + struct mtk_pcie_lane_efuse efuse[2];
>
> If you dynamically allocate this one, you will be able to support any
> number
> of lanes, futureproofing this driver and giving it more flexibility.
>
> > +};
> > +
>
> ..snip..
>
> > +
> > +static int mtk_pcie_efuse_read_for_lane(struct mtk_pcie_phy
> > *pcie_phy,
> > + unsigned int lane)
> > +{
> > + struct device *dev = pcie_phy->dev;
> > + struct mtk_pcie_lane_efuse *data;
> > + char efuse_id[15];
> > + int ret;
> > +
> > + if (lane >= ARRAY_SIZE(pcie_phy->efuse))
> > + return dev_err_probe(pcie_phy->dev, -EINVAL,
> > + "Requested lane number %d exceeds
> > maximum %ld\n",
> > + lane, ARRAY_SIZE(pcie_phy->efuse)
> > - 1);
>
> I don't like seeing dev_err_probe() outside of a probe function, but
> I acknowledge
> that the Linux documentation doesn't seem to give any direction about
> that, so
> this is a personal preference, at this point.
>
> > +
> > + data = &pcie_phy->efuse[lane];
> > +
> > + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_pmos", lane);
> > + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data-
> > >tx_pmos);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to read %s\n",
> > efuse_id);
> > +
> > + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_nmos", lane);
> > + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data-
> > >tx_nmos);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to read %s\n",
> > efuse_id);
> > +
> > + snprintf(efuse_id, sizeof(efuse_id), "rx_ln%d", lane);
> > + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data-
> > >rx_data);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to read %s\n",
> > efuse_id);
> > +
> > + if (!(data->tx_pmos || data->tx_nmos || data->rx_data))
> > + return dev_err_probe(dev, -EINVAL,
> > + "No efuse data found for lane%d,
> > but dts enable it\n",
> > + lane);
> > +
> > + data->lane_efuse_supported = true;
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_pcie_read_efuse(struct mtk_pcie_phy *pcie_phy)
> > +{
> > + struct device *dev = pcie_phy->dev;
> > + bool nvmem_enabled;
> > + int ret;
> > +
> > + nvmem_enabled = device_property_read_bool(dev, "nvmem-cells");
> > + if (!nvmem_enabled)
> > + return -ENODEV;
> > +
> > + ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr",
> > + &pcie_phy-
> > >efuse_glb_intr);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to read
> > glb_intr\n");
> > +
> > + pcie_phy->sw_efuse_supported = true;
> > +
> > + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 0);
> > + if (ret)
> > + return ret;
> > +
> > + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 1);
> > + if (ret)
> > + return ret;
>
> To give some more future-proofing to this driver, I would instead
> either add a
> u32 devicetree property "num-lanes" or, if the same SoC may not have
> a different
> number of lanes across controller instances, I would add a number of
> lanes
> parameter as data for each of_match.
>
> You'd be at that point using a for loop here like:
>
> for (i = 0; i < pcie_phy->num_lanes, i++) {
> ret = mtk_pcie_efuse_read_for_lane(pcie_phy, i);
> if (ret)
> return ret;
> }
>
> Of course, the same logic would apply to mtk_pcie_phy_init(), where
> you are
> instead calling mtk_pcie_efuse_set_lane().
>
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_pcie_phy_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct phy_provider *provider;
> > + struct mtk_pcie_phy *pcie_phy;
> > +
> > + pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy), GFP_KERNEL);
> > + if (!pcie_phy)
> > + return -ENOMEM;
> > +
> > + pcie_phy->dev = dev;
> > +
> > + pcie_phy->sif_base =
> > devm_platform_ioremap_resource_byname(pdev, "sif");
> > + if (IS_ERR(pcie_phy->sif_base))
> > + return dev_err_probe(dev, PTR_ERR(pcie_phy->sif_base),
> > + "Failed to map phy-sif base\n");
> > +
> > + pcie_phy->phy = devm_phy_create(dev, dev->of_node,
> > &mtk_pcie_phy_ops);
> > + if (IS_ERR(pcie_phy->phy))
> > + return dev_err_probe(dev, PTR_ERR(pcie_phy->phy),
> > + "Failed to create PCIe phy\n");
> > +
> > + /*
> > + * Failed to read the efuse data is not a fatal problem,
> > + * ignore the failure and keep going.
> > + */
> > + mtk_pcie_read_efuse(pcie_phy);
>
> If you get an -EPROBE_DEFER here, you surely want to defer probing
> this driver,
> so, yes you're free to ignore the other failures, but you should fix
> that corner
> case.
>
> Everything else looks good, so, please make sure to add me to the
> Cc's for the
> next version of this series for me to give you a faster review.
>
> Regards,
> Angelo
>
_______________________________________________
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] 52+ messages in thread* Re: [PATCH v2 2/2] phy: mediatek: Add PCIe PHY driver
@ 2022-03-18 11:48 ` Jianjun Wang
0 siblings, 0 replies; 52+ messages in thread
From: Jianjun Wang @ 2022-03-18 11:48 UTC (permalink / raw)
To: AngeloGioacchino Del Regno, Chunfeng Yun, Kishon Vijay Abraham I,
Vinod Koul, Rob Herring, Matthias Brugger, Chen-Yu Tsai
Cc: linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
linux-kernel, rex-bc.chen, randy.wu, jieyy.yang, chuanjia.liu,
qizhong.cheng, jian.yang
Hi Angelo,
Thanks for all these suggestions, I'll fix them in the next version.
Thanks.
On Fri, 2022-03-18 at 11:57 +0100, AngeloGioacchino Del Regno wrote:
> Il 18/03/22 10:54, Jianjun Wang ha scritto:
> > Add PCIe GEN3 PHY driver support on MediaTek chipsets.
> >
> > Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> > ---
> > drivers/phy/mediatek/Kconfig | 11 ++
> > drivers/phy/mediatek/Makefile | 1 +
> > drivers/phy/mediatek/phy-mtk-pcie.c | 246
> > ++++++++++++++++++++++++++++
> > 3 files changed, 258 insertions(+)
> > create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
> >
> > diff --git a/drivers/phy/mediatek/Kconfig
> > b/drivers/phy/mediatek/Kconfig
> > index 55f8e6c048ab..387ed1b3f2cc 100644
> > --- a/drivers/phy/mediatek/Kconfig
> > +++ b/drivers/phy/mediatek/Kconfig
> > @@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
> > select GENERIC_PHY
> > help
> > Support MIPI DSI for Mediatek SoCs.
> > +
> > +config PHY_MTK_PCIE
> > + tristate "MediaTek PCIe-PHY Driver"
> > + depends on ARCH_MEDIATEK || COMPILE_TEST
> > + depends on OF
> > + select GENERIC_PHY
> > + help
> > + Say 'Y' here to add support for MediaTek PCIe PHY driver.
> > + This driver create the basic PHY instance and provides
> > initialize
> > + callback for PCIe GEN3 port, it supports software efuse
> > + initialization.
> > diff --git a/drivers/phy/mediatek/Makefile
> > b/drivers/phy/mediatek/Makefile
> > index ace660fbed3a..788c13147f63 100644
> > --- a/drivers/phy/mediatek/Makefile
> > +++ b/drivers/phy/mediatek/Makefile
> > @@ -6,6 +6,7 @@
> > obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
> > obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
> > obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
> > +obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
> >
> > phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
> > phy-mtk-hdmi-drv-y += phy-mtk-hdmi-
> > mt2701.o
> > diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c
> > b/drivers/phy/mediatek/phy-mtk-pcie.c
> > new file mode 100644
> > index 000000000000..0f5d7c7e2b7e
> > --- /dev/null
> > +++ b/drivers/phy/mediatek/phy-mtk-pcie.c
> > @@ -0,0 +1,246 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2022 MediaTek Inc.
> > + * Author: Jianjun Wang <jianjun.wang@mediatek.com>
> > + */
> > +
> > +#include <linux/bits.h>
> > +#include <linux/compiler_types.h>
> > +#include <linux/module.h>
> > +#include <linux/nvmem-consumer.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +
> > +#include "phy-mtk-io.h"
> > +
> > +#define PEXTP_ANA_GLB_00_REG 0x9000
> > +#define PEXTP_ANA_LN0_TRX_REG 0xa000
> > +#define PEXTP_ANA_TX_OFFSET 0x04
> > +#define PEXTP_ANA_RX_OFFSET 0x3c
> > +#define PEXTP_ANA_LANE_OFFSET 0x100
> > +
> > +/* PEXTP_GLB_00_REG[28:24] Internal Resistor Selection of TX Bias
> > Current */
> > +#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
> > +#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
> > +
> > +/* PEXTP_ANA_LN_RX_REG[3:0] RX impedance selection */
> > +#define EFUSE_LN_RX_SEL GENMASK(3, 0)
> > +#define EFUSE_LN_RX_VAL(x) (0xf & (x))
> > +
> > +/* PEXTP_ANA_LN_TX_REG[5:2] TX PMOS impedance selection */
> > +#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
> > +#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
> > +
> > +/* PEXTP_ANA_LN_TX_REG[11:8] TX NMOS impedance selection */
> > +#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
> > +#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
> > +
> > +/* Efuse data for each lane */
>
> What about some kerneldoc?
>
> /**
> * struct mtk_pcie_lane_efuse - eFuse data for each lane
> * @tx_pmos:
> ......etc :))
>
> > +struct mtk_pcie_lane_efuse {
> > + u32 tx_pmos;
> > + u32 tx_nmos;
> > + u32 rx_data;
> > + bool lane_efuse_supported;
> > +};
> > +
>
> Same here
>
> /**
> * struct mtk_pcie_phy - PCIe phy driver main structure
> * @dev: ......
>
> > +struct mtk_pcie_phy {
> > + struct device *dev;
> > + struct phy *phy;
> > + void __iomem *sif_base;
> > +
> > + /*
> > + * Support software efuse initialization,
> > + * currently we only support 2 lane in maximum.
> > + */
>
> Obviously, if you add kerneldoc, this comment would get moved to that
> kerneldoc.
>
> > + bool sw_efuse_supported;
> > + u32 efuse_glb_intr;
>
>
> > + struct mtk_pcie_lane_efuse efuse[2];
>
> If you dynamically allocate this one, you will be able to support any
> number
> of lanes, futureproofing this driver and giving it more flexibility.
>
> > +};
> > +
>
> ..snip..
>
> > +
> > +static int mtk_pcie_efuse_read_for_lane(struct mtk_pcie_phy
> > *pcie_phy,
> > + unsigned int lane)
> > +{
> > + struct device *dev = pcie_phy->dev;
> > + struct mtk_pcie_lane_efuse *data;
> > + char efuse_id[15];
> > + int ret;
> > +
> > + if (lane >= ARRAY_SIZE(pcie_phy->efuse))
> > + return dev_err_probe(pcie_phy->dev, -EINVAL,
> > + "Requested lane number %d exceeds
> > maximum %ld\n",
> > + lane, ARRAY_SIZE(pcie_phy->efuse)
> > - 1);
>
> I don't like seeing dev_err_probe() outside of a probe function, but
> I acknowledge
> that the Linux documentation doesn't seem to give any direction about
> that, so
> this is a personal preference, at this point.
>
> > +
> > + data = &pcie_phy->efuse[lane];
> > +
> > + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_pmos", lane);
> > + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data-
> > >tx_pmos);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to read %s\n",
> > efuse_id);
> > +
> > + snprintf(efuse_id, sizeof(efuse_id), "tx_ln%d_nmos", lane);
> > + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data-
> > >tx_nmos);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to read %s\n",
> > efuse_id);
> > +
> > + snprintf(efuse_id, sizeof(efuse_id), "rx_ln%d", lane);
> > + ret = nvmem_cell_read_variable_le_u32(dev, efuse_id, &data-
> > >rx_data);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to read %s\n",
> > efuse_id);
> > +
> > + if (!(data->tx_pmos || data->tx_nmos || data->rx_data))
> > + return dev_err_probe(dev, -EINVAL,
> > + "No efuse data found for lane%d,
> > but dts enable it\n",
> > + lane);
> > +
> > + data->lane_efuse_supported = true;
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_pcie_read_efuse(struct mtk_pcie_phy *pcie_phy)
> > +{
> > + struct device *dev = pcie_phy->dev;
> > + bool nvmem_enabled;
> > + int ret;
> > +
> > + nvmem_enabled = device_property_read_bool(dev, "nvmem-cells");
> > + if (!nvmem_enabled)
> > + return -ENODEV;
> > +
> > + ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr",
> > + &pcie_phy-
> > >efuse_glb_intr);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to read
> > glb_intr\n");
> > +
> > + pcie_phy->sw_efuse_supported = true;
> > +
> > + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 0);
> > + if (ret)
> > + return ret;
> > +
> > + ret = mtk_pcie_efuse_read_for_lane(pcie_phy, 1);
> > + if (ret)
> > + return ret;
>
> To give some more future-proofing to this driver, I would instead
> either add a
> u32 devicetree property "num-lanes" or, if the same SoC may not have
> a different
> number of lanes across controller instances, I would add a number of
> lanes
> parameter as data for each of_match.
>
> You'd be at that point using a for loop here like:
>
> for (i = 0; i < pcie_phy->num_lanes, i++) {
> ret = mtk_pcie_efuse_read_for_lane(pcie_phy, i);
> if (ret)
> return ret;
> }
>
> Of course, the same logic would apply to mtk_pcie_phy_init(), where
> you are
> instead calling mtk_pcie_efuse_set_lane().
>
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_pcie_phy_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct phy_provider *provider;
> > + struct mtk_pcie_phy *pcie_phy;
> > +
> > + pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy), GFP_KERNEL);
> > + if (!pcie_phy)
> > + return -ENOMEM;
> > +
> > + pcie_phy->dev = dev;
> > +
> > + pcie_phy->sif_base =
> > devm_platform_ioremap_resource_byname(pdev, "sif");
> > + if (IS_ERR(pcie_phy->sif_base))
> > + return dev_err_probe(dev, PTR_ERR(pcie_phy->sif_base),
> > + "Failed to map phy-sif base\n");
> > +
> > + pcie_phy->phy = devm_phy_create(dev, dev->of_node,
> > &mtk_pcie_phy_ops);
> > + if (IS_ERR(pcie_phy->phy))
> > + return dev_err_probe(dev, PTR_ERR(pcie_phy->phy),
> > + "Failed to create PCIe phy\n");
> > +
> > + /*
> > + * Failed to read the efuse data is not a fatal problem,
> > + * ignore the failure and keep going.
> > + */
> > + mtk_pcie_read_efuse(pcie_phy);
>
> If you get an -EPROBE_DEFER here, you surely want to defer probing
> this driver,
> so, yes you're free to ignore the other failures, but you should fix
> that corner
> case.
>
> Everything else looks good, so, please make sure to add me to the
> Cc's for the
> next version of this series for me to give you a faster review.
>
> Regards,
> Angelo
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 52+ messages in thread