v3 changes:
Refer to Conor's comments.
- Let filename match a compatible
- Refine description of the fsl,refclk-pad-mode.
- Remove power-domains description.
- Keep clock ording for two devices.
- Drop the unused label and status.
Refer to Rob's comments.
- Use standard phy mode defines.
- Correct the spell mistakes in the binding document.
v2:https://patchwork.kernel.org/project/linux-phy/cover/[email protected]/
v2 changes:
- Place the dt-bindings header file changes as the first one
in the patch-set, make the annotation more clear, and add
Frank's Reviewed-by tag.
v1:https://patchwork.kernel.org/project/linux-phy/cover/[email protected]/
[PATCH v3 1/3] dt-bindings: phy: phy-imx8-pcie: Add header file for
[PATCH v3 2/3] dt-bindings: phy: Add i.MX8Q HSIO SerDes PHY binding
[PATCH v3 3/3] phy: freescale: imx8qm-hsio: Add i.MX8QM HSIO PHY
Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml | 146 ++++++++++++++++++++
drivers/phy/freescale/Kconfig | 8 ++
drivers/phy/freescale/Makefile | 1 +
drivers/phy/freescale/phy-fsl-imx8qm-hsio.c | 607 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
include/dt-bindings/phy/phy-imx8-pcie.h | 62 +++++++++
5 files changed, 824 insertions(+)
Add lane index and HSIO configuration definitions of the i.MX8Q HSIO
SerDes PHY into header file.
Signed-off-by: Richard Zhu <[email protected]>
Reviewed-by: Frank Li <[email protected]>
---
include/dt-bindings/phy/phy-imx8-pcie.h | 62 +++++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/include/dt-bindings/phy/phy-imx8-pcie.h b/include/dt-bindings/phy/phy-imx8-pcie.h
index 8bbe2d6538d8..60447b95a952 100644
--- a/include/dt-bindings/phy/phy-imx8-pcie.h
+++ b/include/dt-bindings/phy/phy-imx8-pcie.h
@@ -11,4 +11,66 @@
#define IMX8_PCIE_REFCLK_PAD_INPUT 1
#define IMX8_PCIE_REFCLK_PAD_OUTPUT 2
+/*
+ * i.MX8QM HSIO subsystem has three lane PHYs and three controllers:
+ * PCIEA(2 lanes capable PCIe controller), PCIEB (only support one
+ * lane) and SATA.
+ *
+ * Meanwhile, i.MX8QXP HSIO subsystem has one lane PHY and PCIEB(only
+ * support one lane) controller.
+ *
+ * In the different use cases. PCIEA can be bound to PHY lane0, lane1
+ * or Lane0 and lane1. PCIEB can be bound to lane1 or lane2 PHY. SATA
+ * can only be bound to last lane2 PHY.
+ *
+ * +-------------------------------+------------------+
+ * | i.MX8QM | i.MX8QXP |
+ * |-------------------------------|------------------|
+ * | | PCIEA | PCIEB | SATA | | PCIEB |
+ * |-------------------------------|-------|----------|
+ * | LAN 0 | X | | | LAN 0 | * |
+ * |-------------------------------|-------|----------|
+ * | LAN 1 | X | * | | | |
+ * |-------------------------------|-------|----------|
+ * | LAN 2 | | * | * | | |
+ * +-------------------------------+------------------+
+ * NOTE:
+ * *: Choose one only.
+ * X: Choose any of these.
+ *
+ * Define i.MX8Q HSIO PHY lane index here to specify the lane mask.
+ */
+#define IMX8Q_HSIO_LANE0 0x1
+#define IMX8Q_HSIO_LANE1 0x2
+#define IMX8Q_HSIO_LANE2 0x4
+
+/*
+ * Regarding the design of i.MX8QM HSIO subsystem, HSIO module can be
+ * confiured as following three use cases.
+ *
+ * Define different configurations refer to the use cases, since it is
+ * mandatory required in the initialization.
+ *
+ * On i.MX8QXP, HSIO module only has PCIEB and one lane PHY.
+ * Define "IMX8Q_HSIO_CFG_PCIEB" for i.MX8QXP platforms.
+ *
+ * +----------------------------------------------------+----------+
+ * | | i.MX8QM | i.MX8QXP |
+ * |-------------------------------|--------------------|----------|
+ * | | LAN0 | LAN1 | LAN2 | LAN0 |
+ * |-------------------------------|------|------|------|----------|
+ * | IMX8Q_HSIO_CFG_PCIEAX2SATA | PCIEA| PCIEA| SATA | |
+ * |-------------------------------|------|------|------|----------|
+ * | IMX8Q_HSIO_CFG_PCIEAX2PCIEB | PCIEA| PCIEA| PCIEB| |
+ * |-------------------------------|------|------|------|----------|
+ * | IMX8Q_HSIO_CFG_PCIEAPCIEBSATA | PCIEA| PCIEB| SATA | |
+ * |-------------------------------|------|------|------|----------|
+ * | IMX8Q_HSIO_CFG_PCIEB | - | - | - | PCIEB |
+ * +----------------------------------------------------+----------+
+ */
+#define IMX8Q_HSIO_CFG_PCIEAX2SATA 0x1
+#define IMX8Q_HSIO_CFG_PCIEAX2PCIEB 0x2
+#define IMX8Q_HSIO_CFG_PCIEAPCIEBSATA (IMX8Q_HSIO_CFG_PCIEAX2SATA | IMX8Q_HSIO_CFG_PCIEAX2PCIEB)
+#define IMX8Q_HSIO_CFG_PCIEB IMX8Q_HSIO_CFG_PCIEAX2PCIEB
+
#endif /* _DT_BINDINGS_IMX8_PCIE_H */
--
2.37.1
Add i.MX8QM HSIO PHY driver support.
Signed-off-by: Richard Zhu <[email protected]>
---
drivers/phy/freescale/Kconfig | 8 +
drivers/phy/freescale/Makefile | 1 +
drivers/phy/freescale/phy-fsl-imx8qm-hsio.c | 607 ++++++++++++++++++++
3 files changed, 616 insertions(+)
create mode 100644 drivers/phy/freescale/phy-fsl-imx8qm-hsio.c
diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig
index 853958fb2c06..c9ee48aeea9e 100644
--- a/drivers/phy/freescale/Kconfig
+++ b/drivers/phy/freescale/Kconfig
@@ -35,6 +35,14 @@ config PHY_FSL_IMX8M_PCIE
Enable this to add support for the PCIE PHY as found on
i.MX8M family of SOCs.
+config PHY_FSL_IMX8QM_HSIO
+ tristate "Freescale i.MX8QM HSIO PHY"
+ depends on OF && HAS_IOMEM
+ select GENERIC_PHY
+ help
+ Enable this to add support for the HSIO PHY as found on
+ i.MX8QM family of SOCs.
+
endif
config PHY_FSL_LYNX_28G
diff --git a/drivers/phy/freescale/Makefile b/drivers/phy/freescale/Makefile
index cedb328bc4d2..b56b4d5c18ea 100644
--- a/drivers/phy/freescale/Makefile
+++ b/drivers/phy/freescale/Makefile
@@ -3,4 +3,5 @@ obj-$(CONFIG_PHY_FSL_IMX8MQ_USB) += phy-fsl-imx8mq-usb.o
obj-$(CONFIG_PHY_MIXEL_LVDS_PHY) += phy-fsl-imx8qm-lvds-phy.o
obj-$(CONFIG_PHY_MIXEL_MIPI_DPHY) += phy-fsl-imx8-mipi-dphy.o
obj-$(CONFIG_PHY_FSL_IMX8M_PCIE) += phy-fsl-imx8m-pcie.o
+obj-$(CONFIG_PHY_FSL_IMX8QM_HSIO) += phy-fsl-imx8qm-hsio.o
obj-$(CONFIG_PHY_FSL_LYNX_28G) += phy-fsl-lynx-28g.o
diff --git a/drivers/phy/freescale/phy-fsl-imx8qm-hsio.c b/drivers/phy/freescale/phy-fsl-imx8qm-hsio.c
new file mode 100644
index 000000000000..b3e17163e859
--- /dev/null
+++ b/drivers/phy/freescale/phy-fsl-imx8qm-hsio.c
@@ -0,0 +1,607 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2024 NXP
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/pci_regs.h>
+#include <linux/phy/phy.h>
+#include <linux/phy/pcie.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/phy/phy.h>
+#include <dt-bindings/phy/phy-imx8-pcie.h>
+
+#define MAX_NUM_PHY_MIXS 4
+#define PHY_MIX_MAX_NUM_LANES 2
+#define LANE_NUM_CLKS 5
+
+/* Parameters for the waiting for PCIe PHY PLL to lock */
+#define PHY_INIT_WAIT_USLEEP_MAX 10
+#define PHY_INIT_WAIT_TIMEOUT (1000 * PHY_INIT_WAIT_USLEEP_MAX)
+
+/* i.MX8Q HSIO registers */
+#define HSIO_CTRL0 0x0
+#define HSIO_APB_RSTN_0 BIT(0)
+#define HSIO_APB_RSTN_1 BIT(1)
+#define HSIO_PIPE_RSTN_0_MASK GENMASK(25, 24)
+#define HSIO_PIPE_RSTN_1_MASK GENMASK(27, 26)
+#define HSIO_MODE_MASK GENMASK(20, 17)
+#define HSIO_MODE_PCIE 0x0
+#define HSIO_MODE_SATA 0x4
+#define HSIO_DEVICE_TYPE_MASK GENMASK(27, 24)
+#define HSIO_EPCS_TXDEEMP BIT(5)
+#define HSIO_EPCS_TXDEEMP_SEL BIT(6)
+#define HSIO_EPCS_PHYRESET_N BIT(7)
+#define HSIO_RESET_N BIT(12)
+
+#define HSIO_IOB_RXENA BIT(0)
+#define HSIO_IOB_TXENA BIT(1)
+#define HSIO_IOB_A_0_TXOE BIT(2)
+#define HSIO_IOB_A_0_M1M0_2 BIT(4)
+#define HSIO_IOB_A_0_M1M0_MASK GENMASK(4, 3)
+#define HSIO_PHYX1_EPCS_SEL BIT(12)
+#define HSIO_PCIE_AB_SELECT BIT(13)
+
+#define HSIO_PHY_STS0 0x4
+#define HSIO_LANE0_TX_PLL_LOCK BIT(4)
+#define HSIO_LANE1_TX_PLL_LOCK BIT(12)
+
+#define HSIO_CTRL2 0x8
+#define HSIO_LTSSM_ENABLE BIT(4)
+#define HSIO_BUTTON_RST_N BIT(21)
+#define HSIO_PERST_N BIT(22)
+#define HSIO_POWER_UP_RST_N BIT(23)
+
+#define HSIO_PCIE_STS0 0xc
+#define HSIO_PM_REQ_CORE_RST BIT(19)
+
+#define HSIO_REG48_PMA_STATUS 0x30
+#define HSIO_REG48_PMA_RDY BIT(7)
+
+/*
+ * There are three lanes PHY in i.MX8QM HSIO, and can be made up the
+ * following PHY modes in different use cases.
+ * +------------------------------------+
+ * | index | LAN0 | LAN1 | LAN2 |
+ * |------------------------------------|
+ * | 0 | PCIEA | | |
+ * |------------------------------------|
+ * | 1 | | PCIEB | |
+ * |------------------------------------|
+ * | 2 | PCIEA | PCIEA | |
+ * |------------------------------------|
+ * | 3 | | | PCIEB/SATA |
+ * +------------------------------------+
+ */
+enum phy_mode_index {
+ IMX8Q_HSIO_LANE0_PCIE_PHY,
+ IMX8Q_HSIO_LANE1_PCIE_PHY,
+ IMX8Q_HSIO_LANE0_1_PCIE_PHY,
+ IMX8Q_HSIO_LANE2_PHY
+};
+
+struct imx_hsio_drvdata {
+ int phy_mix_num;
+};
+
+struct imx_hsio_phy_lane {
+ const char * const *clk_names;
+ struct clk_bulk_data clks[LANE_NUM_CLKS];
+};
+
+struct imx_hsio_phy_mix {
+ u32 ctrl_index;
+ u32 ctrl_off;
+ u32 phy_off;
+ u32 phy_type;
+ struct imx_hsio_phy_lane lane[PHY_MIX_MAX_NUM_LANES];
+ struct imx_hsio_priv *priv;
+ struct phy *phy;
+ enum phy_mode pmix_mode;
+ enum phy_mode_index idx;
+};
+
+struct imx_hsio_priv {
+ void __iomem *base;
+ struct device *dev;
+ u32 refclk_pad;
+ u32 hsio_cfg;
+ struct regmap *phy;
+ struct regmap *ctrl;
+ struct regmap *misc;
+ const struct imx_hsio_drvdata *drvdata;
+ struct imx_hsio_phy_mix pmix[MAX_NUM_PHY_MIXS];
+};
+
+static const char * const lan0_pcie_clks[] = {"apb_pclk0", "pclk0", "ctl0_crr",
+ "phy0_crr", "misc_crr"};
+static const char * const lan1_pciea_clks[] = {"apb_pclk1", "pclk1", "ctl0_crr",
+ "phy0_crr", "misc_crr"};
+static const char * const lan1_pcieb_clks[] = {"apb_pclk1", "pclk1", "ctl1_crr",
+ "phy0_crr", "misc_crr"};
+static const char * const lan2_pcieb_clks[] = {"apb_pclk2", "pclk2", "ctl1_crr",
+ "phy1_crr", "misc_crr"};
+static const char * const lan2_sata_clks[] = {"pclk2", "epcs_tx", "epcs_rx",
+ "phy1_crr", "misc_crr"};
+
+static const struct regmap_config regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+};
+
+static int imx_hsio_get_idx(int phy_type, int ctrl_index, int lane_mask)
+{
+ int index;
+
+ switch (phy_type) {
+ case PHY_TYPE_PCIE:
+ if (ctrl_index) { /* PCIEB */
+ if (lane_mask == IMX8Q_HSIO_LANE0) /* i.MX8QXP */
+ index = IMX8Q_HSIO_LANE0_PCIE_PHY;
+ else if (lane_mask == IMX8Q_HSIO_LANE1) /* i.MX8QM */
+ index = IMX8Q_HSIO_LANE1_PCIE_PHY;
+ else if (lane_mask == IMX8Q_HSIO_LANE2) /* i.MX8QM */
+ index = IMX8Q_HSIO_LANE2_PHY;
+ else
+ return -EINVAL;
+ } else { /* PCIEA */
+ if (lane_mask == (IMX8Q_HSIO_LANE0 | IMX8Q_HSIO_LANE1))
+ index = IMX8Q_HSIO_LANE0_1_PCIE_PHY;
+ else if (lane_mask == IMX8Q_HSIO_LANE0)
+ index = IMX8Q_HSIO_LANE0_PCIE_PHY;
+ else
+ return -EINVAL;
+ }
+ break;
+ case PHY_TYPE_SATA:
+ index = IMX8Q_HSIO_LANE2_PHY;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return index;
+}
+
+static int imx_hsio_init(struct phy *phy)
+{
+ int ret, i;
+ struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = pmix->priv;
+ struct device *dev = priv->dev;
+
+ /* Assign clocks refer to different modes */
+ switch (pmix->phy_type) {
+ case PHY_TYPE_PCIE:
+ if (pmix->ctrl_index == 0) { /* PCIEA */
+ pmix->pmix_mode = PHY_MODE_PCIE;
+ pmix->ctrl_off = 0;
+ pmix->phy_off = 0;
+
+ for (i = 0; i < LANE_NUM_CLKS; i++) {
+ if (pmix->idx == IMX8Q_HSIO_LANE0_PCIE_PHY) {
+ pmix->lane[0].clks[i].id = lan0_pcie_clks[i];
+ } else { /* 2 lanes are bound to PCIEA */
+ pmix->lane[0].clks[i].id = lan0_pcie_clks[i];
+ pmix->lane[1].clks[i].id = lan1_pciea_clks[i];
+ }
+ }
+ } else { /* PCIEB */
+ pmix->pmix_mode = PHY_MODE_PCIE;
+ if (pmix->idx == IMX8Q_HSIO_LANE0_PCIE_PHY) {
+ /* i.MX8QXP */
+ pmix->ctrl_off = 0;
+ pmix->phy_off = 0;
+ } else {
+ /*
+ * On i.MX8QM, only second or third lane can be
+ * bound to PCIEB.
+ */
+ pmix->ctrl_off = SZ_64K;
+ if (pmix->idx == IMX8Q_HSIO_LANE1_PCIE_PHY)
+ pmix->phy_off = 0;
+ else /* the third lane is bound to PCIEB */
+ pmix->phy_off = SZ_64K;
+ }
+
+ for (i = 0; i < LANE_NUM_CLKS; i++) {
+ if (pmix->idx == IMX8Q_HSIO_LANE1_PCIE_PHY)
+ pmix->lane[0].clks[i].id = lan1_pcieb_clks[i];
+ else if (pmix->idx == IMX8Q_HSIO_LANE2_PHY)
+ pmix->lane[0].clks[i].id = lan2_pcieb_clks[i];
+ else /* i.MX8QXP only has PCIEB, idx is 0 */
+ pmix->lane[0].clks[i].id = lan0_pcie_clks[i];
+ }
+ }
+ break;
+ case PHY_TYPE_SATA:
+ /* On i.MX8QM, only the third lane can be bound to SATA */
+ pmix->ctrl_off = SZ_128K;
+ pmix->pmix_mode = PHY_MODE_SATA;
+ pmix->phy_off = SZ_64K;
+
+ for (i = 0; i < LANE_NUM_CLKS; i++)
+ pmix->lane[0].clks[i].id = lan2_sata_clks[i];
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* Fetch clocks and enable them */
+ ret = devm_clk_bulk_get(dev, LANE_NUM_CLKS, pmix->lane[0].clks);
+ if (ret)
+ return ret;
+ if (pmix->idx == IMX8Q_HSIO_LANE0_1_PCIE_PHY) {
+ ret = devm_clk_bulk_get(dev, LANE_NUM_CLKS, pmix->lane[1].clks);
+ if (ret)
+ return ret;
+ }
+ ret = clk_bulk_prepare_enable(LANE_NUM_CLKS, pmix->lane[0].clks);
+ if (ret)
+ return ret;
+ if (pmix->idx == IMX8Q_HSIO_LANE0_1_PCIE_PHY) {
+ /* Enable the second lane's clocks */
+ ret = clk_bulk_prepare_enable(LANE_NUM_CLKS,
+ pmix->lane[1].clks);
+ if (ret) {
+ clk_bulk_disable_unprepare(LANE_NUM_CLKS,
+ pmix->lane[0].clks);
+ return ret;
+ }
+ }
+
+ /* allow the clocks to stabilize */
+ usleep_range(200, 500);
+ return 0;
+}
+
+static int imx_hsio_exit(struct phy *phy)
+{
+ struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
+
+ clk_bulk_disable_unprepare(LANE_NUM_CLKS, pmix->lane[0].clks);
+ if (pmix->idx == IMX8Q_HSIO_LANE0_1_PCIE_PHY)
+ /* Disable the clocks used by sencond lane of the PHY */
+ clk_bulk_disable_unprepare(LANE_NUM_CLKS, pmix->lane[1].clks);
+
+ return 0;
+}
+
+static void imx_hsio_pcie_phy_resets(struct phy *phy)
+{
+ struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = pmix->priv;
+
+ regmap_clear_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
+ HSIO_BUTTON_RST_N);
+ regmap_clear_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
+ HSIO_PERST_N);
+ regmap_clear_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
+ HSIO_POWER_UP_RST_N);
+ regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
+ HSIO_BUTTON_RST_N);
+ regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
+ HSIO_PERST_N);
+ regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
+ HSIO_POWER_UP_RST_N);
+
+ if (pmix->idx == IMX8Q_HSIO_LANE0_1_PCIE_PHY) {
+ regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
+ HSIO_APB_RSTN_0);
+ regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
+ HSIO_PIPE_RSTN_0_MASK);
+ regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
+ HSIO_APB_RSTN_1);
+ regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
+ HSIO_PIPE_RSTN_1_MASK);
+ } else if (pmix->idx == IMX8Q_HSIO_LANE1_PCIE_PHY) {
+ /* The second pmix */
+ regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
+ HSIO_APB_RSTN_1);
+ regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
+ HSIO_PIPE_RSTN_1_MASK);
+ } else {
+ regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
+ HSIO_APB_RSTN_0);
+ regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
+ HSIO_PIPE_RSTN_0_MASK);
+ }
+}
+
+static void imx_hsio_sata_phy_resets(struct phy *phy)
+{
+ struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = pmix->priv;
+
+ /* clear PHY RST, then set it */
+ regmap_clear_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
+ HSIO_EPCS_PHYRESET_N);
+ regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
+ HSIO_EPCS_PHYRESET_N);
+
+ /* CTRL RST: SET -> delay 1 us -> CLEAR -> SET */
+ regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0, HSIO_RESET_N);
+ udelay(1);
+ regmap_clear_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
+ HSIO_RESET_N);
+ regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0, HSIO_RESET_N);
+}
+
+static void imx_hsio_configure_clk_pad(struct phy *phy)
+{
+ bool pll = false;
+ u32 pad_mode;
+ struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = pmix->priv;
+
+ pad_mode = priv->refclk_pad;
+ if (pad_mode == IMX8_PCIE_REFCLK_PAD_OUTPUT) {
+ pll = true;
+ regmap_update_bits(priv->misc, HSIO_CTRL0,
+ HSIO_IOB_A_0_TXOE | HSIO_IOB_A_0_M1M0_MASK,
+ HSIO_IOB_A_0_TXOE | HSIO_IOB_A_0_M1M0_2);
+ }
+
+ regmap_update_bits(priv->misc, HSIO_CTRL0, HSIO_IOB_RXENA,
+ pll ? 0 : HSIO_IOB_RXENA);
+ regmap_update_bits(priv->misc, HSIO_CTRL0, HSIO_IOB_TXENA,
+ pll ? HSIO_IOB_TXENA : 0);
+}
+
+static int imx_hsio_power_on(struct phy *phy)
+{
+ int ret;
+ u32 val, addr, cond;
+ struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = pmix->priv;
+
+ if (pmix->pmix_mode == PHY_MODE_PCIE)
+ imx_hsio_pcie_phy_resets(phy);
+ else /* SATA */
+ regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
+ HSIO_APB_RSTN_0);
+
+ if (priv->hsio_cfg & IMX8Q_HSIO_CFG_PCIEAX2PCIEB)
+ regmap_set_bits(priv->misc, HSIO_CTRL0, HSIO_PCIE_AB_SELECT);
+ if (priv->hsio_cfg & IMX8Q_HSIO_CFG_PCIEAX2SATA)
+ regmap_set_bits(priv->misc, HSIO_CTRL0, HSIO_PHYX1_EPCS_SEL);
+
+ imx_hsio_configure_clk_pad(phy);
+
+ if (pmix->pmix_mode == PHY_MODE_SATA) {
+ regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
+ HSIO_EPCS_TXDEEMP);
+ regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
+ HSIO_EPCS_TXDEEMP_SEL);
+
+ imx_hsio_sata_phy_resets(phy);
+ } else {
+ /* Toggle apb_pclk to make sure PM_REQ_CORE_RST is cleared. */
+ clk_disable_unprepare(pmix->lane[0].clks[0].clk);
+ mdelay(1);
+ ret = clk_prepare_enable(pmix->lane[0].clks[0].clk);
+ if (ret) {
+ dev_err(priv->dev, "unable to enable phy apb_pclk\n");
+ return ret;
+ }
+
+ addr = pmix->ctrl_off + HSIO_PCIE_STS0;
+ cond = HSIO_PM_REQ_CORE_RST;
+ ret = regmap_read_poll_timeout(priv->ctrl, addr, val,
+ (val & cond) == 0,
+ PHY_INIT_WAIT_USLEEP_MAX,
+ PHY_INIT_WAIT_TIMEOUT);
+ if (ret) {
+ dev_err(priv->dev, "HSIO_PM_REQ_CORE_RST is set\n");
+ return ret;
+ }
+ }
+
+ /* Polling to check the PHY is ready or not. */
+ if (pmix->idx == IMX8Q_HSIO_LANE1_PCIE_PHY)
+ cond = HSIO_LANE1_TX_PLL_LOCK;
+ else
+ cond = HSIO_LANE0_TX_PLL_LOCK;
+
+ ret = regmap_read_poll_timeout(priv->phy, pmix->phy_off + HSIO_PHY_STS0,
+ val, ((val & cond) == cond),
+ PHY_INIT_WAIT_USLEEP_MAX,
+ PHY_INIT_WAIT_TIMEOUT);
+ if (ret) {
+ dev_err(priv->dev, "IMX8Q PHY%d PLL lock timeout\n", pmix->idx);
+ return ret;
+ }
+ dev_info(priv->dev, "IMX8Q PHY%d PLL is locked\n", pmix->idx);
+
+ if (pmix->pmix_mode == PHY_MODE_SATA) {
+ cond = HSIO_REG48_PMA_RDY;
+ ret = read_poll_timeout(readb, val, ((val & cond) == cond),
+ PHY_INIT_WAIT_USLEEP_MAX,
+ PHY_INIT_WAIT_TIMEOUT, false,
+ priv->base + HSIO_REG48_PMA_STATUS);
+ if (ret)
+ dev_err(priv->dev, "PHY calibration is timeout\n");
+ else
+ dev_info(priv->dev, "PHY calibration is done\n");
+ }
+
+ return ret;
+}
+
+static int imx_hsio_set_mode(struct phy *phy, enum phy_mode mode,
+ int submode)
+{
+ u32 val;
+ struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = pmix->priv;
+
+ if (pmix->pmix_mode != mode)
+ return -EINVAL;
+
+ val = (mode == PHY_MODE_PCIE) ? HSIO_MODE_PCIE : HSIO_MODE_SATA;
+ val = FIELD_PREP(HSIO_MODE_MASK, val);
+ regmap_update_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
+ HSIO_MODE_MASK, val);
+
+ switch (submode) {
+ case PHY_MODE_PCIE_RC:
+ val = FIELD_PREP(HSIO_DEVICE_TYPE_MASK, PCI_EXP_TYPE_ROOT_PORT);
+ break;
+ case PHY_MODE_PCIE_EP:
+ val = FIELD_PREP(HSIO_DEVICE_TYPE_MASK, PCI_EXP_TYPE_ENDPOINT);
+ break;
+ default: /* Support only PCIe EP and RC now. */
+ return 0;
+ }
+ if (submode)
+ regmap_update_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
+ HSIO_DEVICE_TYPE_MASK, val);
+
+ return 0;
+}
+
+static int imx_hsio_set_speed(struct phy *phy, int speed)
+{
+ struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = pmix->priv;
+
+ regmap_update_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
+ HSIO_LTSSM_ENABLE,
+ speed ? HSIO_LTSSM_ENABLE : 0);
+ return 0;
+}
+
+static const struct phy_ops imx_hsio_ops = {
+ .init = imx_hsio_init,
+ .exit = imx_hsio_exit,
+ .power_on = imx_hsio_power_on,
+ .set_mode = imx_hsio_set_mode,
+ .set_speed = imx_hsio_set_speed,
+ .owner = THIS_MODULE,
+};
+
+static const struct imx_hsio_drvdata imx8qxp_hsio_drvdata = {
+ .phy_mix_num = 0x1,
+};
+
+static const struct imx_hsio_drvdata imx_hsio_drvdata = {
+ .phy_mix_num = 0x4,
+};
+
+static const struct of_device_id imx_hsio_of_match[] = {
+ {.compatible = "fsl,imx8qm-hsio", .data = &imx_hsio_drvdata},
+ {.compatible = "fsl,imx8qxp-hsio", .data = &imx8qxp_hsio_drvdata},
+ { },
+};
+MODULE_DEVICE_TABLE(of, imx_hsio_of_match);
+
+static struct phy *imx_hsio_xlate(struct device *dev,
+ const struct of_phandle_args *args)
+{
+ struct imx_hsio_priv *priv = dev_get_drvdata(dev);
+ int phy_type = args->args[0];
+ int ctrl_index = args->args[1];
+ int idx, lane_mask = args->args[2];
+
+ idx = imx_hsio_get_idx(phy_type, ctrl_index, lane_mask);
+ if (idx < 0 || idx >= priv->drvdata->phy_mix_num)
+ return ERR_PTR(-EINVAL);
+ priv->pmix[idx].phy_type = phy_type;
+ priv->pmix[idx].ctrl_index = ctrl_index;
+ priv->pmix[idx].idx = idx;
+
+ return priv->pmix[idx].phy;
+}
+
+static int imx_hsio_probe(struct platform_device *pdev)
+{
+ int i;
+ void __iomem *off;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ const struct of_device_id *of_id;
+ struct imx_hsio_priv *priv;
+ struct phy_provider *provider;
+
+ of_id = of_match_device(imx_hsio_of_match, dev);
+ if (!of_id)
+ return -EINVAL;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+ priv->dev = &pdev->dev;
+ priv->drvdata = of_device_get_match_data(dev);
+
+ /* Get HSIO configuration mode */
+ of_property_read_u32(np, "fsl,hsio-cfg", &priv->hsio_cfg);
+ /* Get PHY refclk pad mode */
+ if (of_property_read_u32(np, "fsl,refclk-pad-mode", &priv->refclk_pad))
+ priv->refclk_pad = IMX8_PCIE_REFCLK_PAD_OUTPUT;
+
+ priv->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
+
+ off = devm_platform_ioremap_resource_byname(pdev, "phy");
+ priv->phy = devm_regmap_init_mmio(dev, off, ®map_config);
+ if (IS_ERR(priv->phy))
+ return dev_err_probe(dev, PTR_ERR(priv->phy),
+ "unable to find phy csr registers\n");
+
+ off = devm_platform_ioremap_resource_byname(pdev, "ctrl");
+ priv->ctrl = devm_regmap_init_mmio(dev, off, ®map_config);
+ if (IS_ERR(priv->ctrl))
+ return dev_err_probe(dev, PTR_ERR(priv->ctrl),
+ "unable to find ctrl csr registers\n");
+
+ off = devm_platform_ioremap_resource_byname(pdev, "misc");
+ priv->misc = devm_regmap_init_mmio(dev, off, ®map_config);
+ if (IS_ERR(priv->misc))
+ return dev_err_probe(dev, PTR_ERR(priv->misc),
+ "unable to find misc csr registers\n");
+
+ for (i = 0; i < priv->drvdata->phy_mix_num; i++) {
+ struct imx_hsio_phy_mix *pmix = &priv->pmix[i];
+ struct phy *phy;
+
+ memset(pmix, 0, sizeof(*pmix));
+
+ phy = devm_phy_create(&pdev->dev, NULL, &imx_hsio_ops);
+ if (IS_ERR(phy))
+ return PTR_ERR(phy);
+
+ pmix->priv = priv;
+ pmix->phy = phy;
+ pmix->idx = i;
+ phy_set_drvdata(phy, pmix);
+ }
+
+ dev_set_drvdata(dev, priv);
+ dev_set_drvdata(&pdev->dev, priv);
+
+ provider = devm_of_phy_provider_register(&pdev->dev, imx_hsio_xlate);
+
+ return PTR_ERR_OR_ZERO(provider);
+}
+
+static struct platform_driver imx_hsio_driver = {
+ .probe = imx_hsio_probe,
+ .driver = {
+ .name = "imx8qm-hsio-phy",
+ .of_match_table = imx_hsio_of_match,
+ }
+};
+module_platform_driver(imx_hsio_driver);
+
+MODULE_DESCRIPTION("FSL IMX8QM HSIO SERDES PHY driver");
+MODULE_LICENSE("GPL");
--
2.37.1
On Wed, Apr 24, 2024 at 02:21:23PM +0800, Richard Zhu wrote:
> Add i.MX8QM HSIO PHY driver support.
>
> Signed-off-by: Richard Zhu <[email protected]>
> ---
> drivers/phy/freescale/Kconfig | 8 +
> drivers/phy/freescale/Makefile | 1 +
> drivers/phy/freescale/phy-fsl-imx8qm-hsio.c | 607 ++++++++++++++++++++
> 3 files changed, 616 insertions(+)
> create mode 100644 drivers/phy/freescale/phy-fsl-imx8qm-hsio.c
>
> diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig
> index 853958fb2c06..c9ee48aeea9e 100644
> --- a/drivers/phy/freescale/Kconfig
> +++ b/drivers/phy/freescale/Kconfig
> @@ -35,6 +35,14 @@ config PHY_FSL_IMX8M_PCIE
> Enable this to add support for the PCIE PHY as found on
> i.MX8M family of SOCs.
>
> +config PHY_FSL_IMX8QM_HSIO
> + tristate "Freescale i.MX8QM HSIO PHY"
> + depends on OF && HAS_IOMEM
> + select GENERIC_PHY
> + help
> + Enable this to add support for the HSIO PHY as found on
> + i.MX8QM family of SOCs.
> +
> endif
>
> config PHY_FSL_LYNX_28G
> diff --git a/drivers/phy/freescale/Makefile b/drivers/phy/freescale/Makefile
> index cedb328bc4d2..b56b4d5c18ea 100644
> --- a/drivers/phy/freescale/Makefile
> +++ b/drivers/phy/freescale/Makefile
> @@ -3,4 +3,5 @@ obj-$(CONFIG_PHY_FSL_IMX8MQ_USB) += phy-fsl-imx8mq-usb.o
> obj-$(CONFIG_PHY_MIXEL_LVDS_PHY) += phy-fsl-imx8qm-lvds-phy.o
> obj-$(CONFIG_PHY_MIXEL_MIPI_DPHY) += phy-fsl-imx8-mipi-dphy.o
> obj-$(CONFIG_PHY_FSL_IMX8M_PCIE) += phy-fsl-imx8m-pcie.o
> +obj-$(CONFIG_PHY_FSL_IMX8QM_HSIO) += phy-fsl-imx8qm-hsio.o
> obj-$(CONFIG_PHY_FSL_LYNX_28G) += phy-fsl-lynx-28g.o
> diff --git a/drivers/phy/freescale/phy-fsl-imx8qm-hsio.c b/drivers/phy/freescale/phy-fsl-imx8qm-hsio.c
> new file mode 100644
> index 000000000000..b3e17163e859
> --- /dev/null
> +++ b/drivers/phy/freescale/phy-fsl-imx8qm-hsio.c
> @@ -0,0 +1,607 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2024 NXP
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
Check that you are including the right DT includes.
> +#include <linux/pci_regs.h>
> +#include <linux/phy/phy.h>
> +#include <linux/phy/pcie.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include <dt-bindings/phy/phy.h>
> +#include <dt-bindings/phy/phy-imx8-pcie.h>
> +
> +#define MAX_NUM_PHY_MIXS 4
> +#define PHY_MIX_MAX_NUM_LANES 2
> +#define LANE_NUM_CLKS 5
> +
> +/* Parameters for the waiting for PCIe PHY PLL to lock */
> +#define PHY_INIT_WAIT_USLEEP_MAX 10
> +#define PHY_INIT_WAIT_TIMEOUT (1000 * PHY_INIT_WAIT_USLEEP_MAX)
> +
> +/* i.MX8Q HSIO registers */
> +#define HSIO_CTRL0 0x0
> +#define HSIO_APB_RSTN_0 BIT(0)
> +#define HSIO_APB_RSTN_1 BIT(1)
> +#define HSIO_PIPE_RSTN_0_MASK GENMASK(25, 24)
> +#define HSIO_PIPE_RSTN_1_MASK GENMASK(27, 26)
> +#define HSIO_MODE_MASK GENMASK(20, 17)
> +#define HSIO_MODE_PCIE 0x0
> +#define HSIO_MODE_SATA 0x4
> +#define HSIO_DEVICE_TYPE_MASK GENMASK(27, 24)
> +#define HSIO_EPCS_TXDEEMP BIT(5)
> +#define HSIO_EPCS_TXDEEMP_SEL BIT(6)
> +#define HSIO_EPCS_PHYRESET_N BIT(7)
> +#define HSIO_RESET_N BIT(12)
> +
> +#define HSIO_IOB_RXENA BIT(0)
> +#define HSIO_IOB_TXENA BIT(1)
> +#define HSIO_IOB_A_0_TXOE BIT(2)
> +#define HSIO_IOB_A_0_M1M0_2 BIT(4)
> +#define HSIO_IOB_A_0_M1M0_MASK GENMASK(4, 3)
> +#define HSIO_PHYX1_EPCS_SEL BIT(12)
> +#define HSIO_PCIE_AB_SELECT BIT(13)
> +
> +#define HSIO_PHY_STS0 0x4
> +#define HSIO_LANE0_TX_PLL_LOCK BIT(4)
> +#define HSIO_LANE1_TX_PLL_LOCK BIT(12)
> +
> +#define HSIO_CTRL2 0x8
> +#define HSIO_LTSSM_ENABLE BIT(4)
> +#define HSIO_BUTTON_RST_N BIT(21)
> +#define HSIO_PERST_N BIT(22)
> +#define HSIO_POWER_UP_RST_N BIT(23)
> +
> +#define HSIO_PCIE_STS0 0xc
> +#define HSIO_PM_REQ_CORE_RST BIT(19)
> +
> +#define HSIO_REG48_PMA_STATUS 0x30
> +#define HSIO_REG48_PMA_RDY BIT(7)
> +
> +/*
> + * There are three lanes PHY in i.MX8QM HSIO, and can be made up the
> + * following PHY modes in different use cases.
> + * +------------------------------------+
> + * | index | LAN0 | LAN1 | LAN2 |
> + * |------------------------------------|
> + * | 0 | PCIEA | | |
> + * |------------------------------------|
> + * | 1 | | PCIEB | |
> + * |------------------------------------|
> + * | 2 | PCIEA | PCIEA | |
> + * |------------------------------------|
> + * | 3 | | | PCIEB/SATA |
> + * +------------------------------------+
> + */
> +enum phy_mode_index {
> + IMX8Q_HSIO_LANE0_PCIE_PHY,
> + IMX8Q_HSIO_LANE1_PCIE_PHY,
> + IMX8Q_HSIO_LANE0_1_PCIE_PHY,
> + IMX8Q_HSIO_LANE2_PHY
> +};
> +
> +struct imx_hsio_drvdata {
> + int phy_mix_num;
> +};
> +
> +struct imx_hsio_phy_lane {
> + const char * const *clk_names;
> + struct clk_bulk_data clks[LANE_NUM_CLKS];
> +};
> +
> +struct imx_hsio_phy_mix {
> + u32 ctrl_index;
> + u32 ctrl_off;
> + u32 phy_off;
> + u32 phy_type;
> + struct imx_hsio_phy_lane lane[PHY_MIX_MAX_NUM_LANES];
> + struct imx_hsio_priv *priv;
> + struct phy *phy;
> + enum phy_mode pmix_mode;
> + enum phy_mode_index idx;
> +};
> +
> +struct imx_hsio_priv {
> + void __iomem *base;
> + struct device *dev;
> + u32 refclk_pad;
> + u32 hsio_cfg;
> + struct regmap *phy;
> + struct regmap *ctrl;
> + struct regmap *misc;
> + const struct imx_hsio_drvdata *drvdata;
> + struct imx_hsio_phy_mix pmix[MAX_NUM_PHY_MIXS];
> +};
> +
> +static const char * const lan0_pcie_clks[] = {"apb_pclk0", "pclk0", "ctl0_crr",
> + "phy0_crr", "misc_crr"};
> +static const char * const lan1_pciea_clks[] = {"apb_pclk1", "pclk1", "ctl0_crr",
> + "phy0_crr", "misc_crr"};
> +static const char * const lan1_pcieb_clks[] = {"apb_pclk1", "pclk1", "ctl1_crr",
> + "phy0_crr", "misc_crr"};
> +static const char * const lan2_pcieb_clks[] = {"apb_pclk2", "pclk2", "ctl1_crr",
> + "phy1_crr", "misc_crr"};
> +static const char * const lan2_sata_clks[] = {"pclk2", "epcs_tx", "epcs_rx",
> + "phy1_crr", "misc_crr"};
> +
> +static const struct regmap_config regmap_config = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> +};
> +
> +static int imx_hsio_get_idx(int phy_type, int ctrl_index, int lane_mask)
> +{
> + int index;
> +
> + switch (phy_type) {
> + case PHY_TYPE_PCIE:
> + if (ctrl_index) { /* PCIEB */
> + if (lane_mask == IMX8Q_HSIO_LANE0) /* i.MX8QXP */
> + index = IMX8Q_HSIO_LANE0_PCIE_PHY;
> + else if (lane_mask == IMX8Q_HSIO_LANE1) /* i.MX8QM */
> + index = IMX8Q_HSIO_LANE1_PCIE_PHY;
> + else if (lane_mask == IMX8Q_HSIO_LANE2) /* i.MX8QM */
> + index = IMX8Q_HSIO_LANE2_PHY;
> + else
> + return -EINVAL;
> + } else { /* PCIEA */
> + if (lane_mask == (IMX8Q_HSIO_LANE0 | IMX8Q_HSIO_LANE1))
> + index = IMX8Q_HSIO_LANE0_1_PCIE_PHY;
> + else if (lane_mask == IMX8Q_HSIO_LANE0)
> + index = IMX8Q_HSIO_LANE0_PCIE_PHY;
> + else
> + return -EINVAL;
> + }
> + break;
> + case PHY_TYPE_SATA:
> + index = IMX8Q_HSIO_LANE2_PHY;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return index;
> +}
> +
> +static int imx_hsio_init(struct phy *phy)
> +{
> + int ret, i;
> + struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = pmix->priv;
> + struct device *dev = priv->dev;
> +
> + /* Assign clocks refer to different modes */
> + switch (pmix->phy_type) {
> + case PHY_TYPE_PCIE:
> + if (pmix->ctrl_index == 0) { /* PCIEA */
> + pmix->pmix_mode = PHY_MODE_PCIE;
> + pmix->ctrl_off = 0;
> + pmix->phy_off = 0;
> +
> + for (i = 0; i < LANE_NUM_CLKS; i++) {
> + if (pmix->idx == IMX8Q_HSIO_LANE0_PCIE_PHY) {
> + pmix->lane[0].clks[i].id = lan0_pcie_clks[i];
> + } else { /* 2 lanes are bound to PCIEA */
> + pmix->lane[0].clks[i].id = lan0_pcie_clks[i];
> + pmix->lane[1].clks[i].id = lan1_pciea_clks[i];
> + }
> + }
> + } else { /* PCIEB */
> + pmix->pmix_mode = PHY_MODE_PCIE;
> + if (pmix->idx == IMX8Q_HSIO_LANE0_PCIE_PHY) {
> + /* i.MX8QXP */
> + pmix->ctrl_off = 0;
> + pmix->phy_off = 0;
> + } else {
> + /*
> + * On i.MX8QM, only second or third lane can be
> + * bound to PCIEB.
> + */
> + pmix->ctrl_off = SZ_64K;
> + if (pmix->idx == IMX8Q_HSIO_LANE1_PCIE_PHY)
> + pmix->phy_off = 0;
> + else /* the third lane is bound to PCIEB */
> + pmix->phy_off = SZ_64K;
> + }
> +
> + for (i = 0; i < LANE_NUM_CLKS; i++) {
> + if (pmix->idx == IMX8Q_HSIO_LANE1_PCIE_PHY)
> + pmix->lane[0].clks[i].id = lan1_pcieb_clks[i];
> + else if (pmix->idx == IMX8Q_HSIO_LANE2_PHY)
> + pmix->lane[0].clks[i].id = lan2_pcieb_clks[i];
> + else /* i.MX8QXP only has PCIEB, idx is 0 */
> + pmix->lane[0].clks[i].id = lan0_pcie_clks[i];
> + }
> + }
> + break;
> + case PHY_TYPE_SATA:
> + /* On i.MX8QM, only the third lane can be bound to SATA */
> + pmix->ctrl_off = SZ_128K;
> + pmix->pmix_mode = PHY_MODE_SATA;
> + pmix->phy_off = SZ_64K;
> +
> + for (i = 0; i < LANE_NUM_CLKS; i++)
> + pmix->lane[0].clks[i].id = lan2_sata_clks[i];
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + /* Fetch clocks and enable them */
> + ret = devm_clk_bulk_get(dev, LANE_NUM_CLKS, pmix->lane[0].clks);
> + if (ret)
> + return ret;
> + if (pmix->idx == IMX8Q_HSIO_LANE0_1_PCIE_PHY) {
> + ret = devm_clk_bulk_get(dev, LANE_NUM_CLKS, pmix->lane[1].clks);
> + if (ret)
> + return ret;
> + }
> + ret = clk_bulk_prepare_enable(LANE_NUM_CLKS, pmix->lane[0].clks);
> + if (ret)
> + return ret;
> + if (pmix->idx == IMX8Q_HSIO_LANE0_1_PCIE_PHY) {
> + /* Enable the second lane's clocks */
> + ret = clk_bulk_prepare_enable(LANE_NUM_CLKS,
> + pmix->lane[1].clks);
> + if (ret) {
> + clk_bulk_disable_unprepare(LANE_NUM_CLKS,
> + pmix->lane[0].clks);
> + return ret;
> + }
> + }
> +
> + /* allow the clocks to stabilize */
> + usleep_range(200, 500);
> + return 0;
> +}
> +
> +static int imx_hsio_exit(struct phy *phy)
> +{
> + struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
> +
> + clk_bulk_disable_unprepare(LANE_NUM_CLKS, pmix->lane[0].clks);
> + if (pmix->idx == IMX8Q_HSIO_LANE0_1_PCIE_PHY)
> + /* Disable the clocks used by sencond lane of the PHY */
> + clk_bulk_disable_unprepare(LANE_NUM_CLKS, pmix->lane[1].clks);
> +
> + return 0;
> +}
> +
> +static void imx_hsio_pcie_phy_resets(struct phy *phy)
> +{
> + struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = pmix->priv;
> +
> + regmap_clear_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
> + HSIO_BUTTON_RST_N);
> + regmap_clear_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
> + HSIO_PERST_N);
> + regmap_clear_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
> + HSIO_POWER_UP_RST_N);
> + regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
> + HSIO_BUTTON_RST_N);
> + regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
> + HSIO_PERST_N);
> + regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
> + HSIO_POWER_UP_RST_N);
> +
> + if (pmix->idx == IMX8Q_HSIO_LANE0_1_PCIE_PHY) {
> + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> + HSIO_APB_RSTN_0);
> + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> + HSIO_PIPE_RSTN_0_MASK);
> + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> + HSIO_APB_RSTN_1);
> + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> + HSIO_PIPE_RSTN_1_MASK);
> + } else if (pmix->idx == IMX8Q_HSIO_LANE1_PCIE_PHY) {
> + /* The second pmix */
> + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> + HSIO_APB_RSTN_1);
> + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> + HSIO_PIPE_RSTN_1_MASK);
> + } else {
> + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> + HSIO_APB_RSTN_0);
> + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> + HSIO_PIPE_RSTN_0_MASK);
> + }
> +}
> +
> +static void imx_hsio_sata_phy_resets(struct phy *phy)
> +{
> + struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = pmix->priv;
> +
> + /* clear PHY RST, then set it */
> + regmap_clear_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
> + HSIO_EPCS_PHYRESET_N);
> + regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
> + HSIO_EPCS_PHYRESET_N);
> +
> + /* CTRL RST: SET -> delay 1 us -> CLEAR -> SET */
> + regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0, HSIO_RESET_N);
> + udelay(1);
> + regmap_clear_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
> + HSIO_RESET_N);
> + regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0, HSIO_RESET_N);
> +}
> +
> +static void imx_hsio_configure_clk_pad(struct phy *phy)
> +{
> + bool pll = false;
> + u32 pad_mode;
> + struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = pmix->priv;
> +
> + pad_mode = priv->refclk_pad;
> + if (pad_mode == IMX8_PCIE_REFCLK_PAD_OUTPUT) {
> + pll = true;
> + regmap_update_bits(priv->misc, HSIO_CTRL0,
> + HSIO_IOB_A_0_TXOE | HSIO_IOB_A_0_M1M0_MASK,
> + HSIO_IOB_A_0_TXOE | HSIO_IOB_A_0_M1M0_2);
> + }
> +
> + regmap_update_bits(priv->misc, HSIO_CTRL0, HSIO_IOB_RXENA,
> + pll ? 0 : HSIO_IOB_RXENA);
> + regmap_update_bits(priv->misc, HSIO_CTRL0, HSIO_IOB_TXENA,
> + pll ? HSIO_IOB_TXENA : 0);
> +}
> +
> +static int imx_hsio_power_on(struct phy *phy)
> +{
> + int ret;
> + u32 val, addr, cond;
> + struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = pmix->priv;
> +
> + if (pmix->pmix_mode == PHY_MODE_PCIE)
> + imx_hsio_pcie_phy_resets(phy);
> + else /* SATA */
> + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> + HSIO_APB_RSTN_0);
> +
> + if (priv->hsio_cfg & IMX8Q_HSIO_CFG_PCIEAX2PCIEB)
Can't this just be based on lane==2 and mode==pcie?
> + regmap_set_bits(priv->misc, HSIO_CTRL0, HSIO_PCIE_AB_SELECT);
> + if (priv->hsio_cfg & IMX8Q_HSIO_CFG_PCIEAX2SATA)
And this on mode==sata?
> + regmap_set_bits(priv->misc, HSIO_CTRL0, HSIO_PHYX1_EPCS_SEL);
> +
> + imx_hsio_configure_clk_pad(phy);
power_on is called per phy, but this appears to be some global state.
> +
> + if (pmix->pmix_mode == PHY_MODE_SATA) {
> + regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
> + HSIO_EPCS_TXDEEMP);
> + regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
> + HSIO_EPCS_TXDEEMP_SEL);
> +
> + imx_hsio_sata_phy_resets(phy);
> + } else {
> + /* Toggle apb_pclk to make sure PM_REQ_CORE_RST is cleared. */
> + clk_disable_unprepare(pmix->lane[0].clks[0].clk);
> + mdelay(1);
> + ret = clk_prepare_enable(pmix->lane[0].clks[0].clk);
> + if (ret) {
> + dev_err(priv->dev, "unable to enable phy apb_pclk\n");
> + return ret;
> + }
> +
> + addr = pmix->ctrl_off + HSIO_PCIE_STS0;
> + cond = HSIO_PM_REQ_CORE_RST;
> + ret = regmap_read_poll_timeout(priv->ctrl, addr, val,
> + (val & cond) == 0,
> + PHY_INIT_WAIT_USLEEP_MAX,
> + PHY_INIT_WAIT_TIMEOUT);
> + if (ret) {
> + dev_err(priv->dev, "HSIO_PM_REQ_CORE_RST is set\n");
> + return ret;
> + }
> + }
> +
> + /* Polling to check the PHY is ready or not. */
> + if (pmix->idx == IMX8Q_HSIO_LANE1_PCIE_PHY)
> + cond = HSIO_LANE1_TX_PLL_LOCK;
> + else
> + cond = HSIO_LANE0_TX_PLL_LOCK;
In SATA mode, don't you need LANE2? Or you should have exited above?
> +
> + ret = regmap_read_poll_timeout(priv->phy, pmix->phy_off + HSIO_PHY_STS0,
> + val, ((val & cond) == cond),
> + PHY_INIT_WAIT_USLEEP_MAX,
> + PHY_INIT_WAIT_TIMEOUT);
> + if (ret) {
> + dev_err(priv->dev, "IMX8Q PHY%d PLL lock timeout\n", pmix->idx);
> + return ret;
> + }
> + dev_info(priv->dev, "IMX8Q PHY%d PLL is locked\n", pmix->idx);
These info level prints should be debug level IMO.
> +
> + if (pmix->pmix_mode == PHY_MODE_SATA) {
> + cond = HSIO_REG48_PMA_RDY;
> + ret = read_poll_timeout(readb, val, ((val & cond) == cond),
> + PHY_INIT_WAIT_USLEEP_MAX,
> + PHY_INIT_WAIT_TIMEOUT, false,
> + priv->base + HSIO_REG48_PMA_STATUS);
> + if (ret)
> + dev_err(priv->dev, "PHY calibration is timeout\n");
> + else
> + dev_info(priv->dev, "PHY calibration is done\n");
> + }
This function is a bunch of 'if SATA' or 'if PCIE' blocks which is hard
to follow. I think it would be easier to follow if you had a specific
power_on function for each mode which can then call any common helpers
(like polling for PLL lock).
And consider if mode specific stuff can go into set_mode() hook instead.
> +
> + return ret;
> +}
> +
> +static int imx_hsio_set_mode(struct phy *phy, enum phy_mode mode,
> + int submode)
> +{
> + u32 val;
> + struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = pmix->priv;
> +
> + if (pmix->pmix_mode != mode)
> + return -EINVAL;
> +
> + val = (mode == PHY_MODE_PCIE) ? HSIO_MODE_PCIE : HSIO_MODE_SATA;
> + val = FIELD_PREP(HSIO_MODE_MASK, val);
> + regmap_update_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> + HSIO_MODE_MASK, val);
> +
> + switch (submode) {
> + case PHY_MODE_PCIE_RC:
> + val = FIELD_PREP(HSIO_DEVICE_TYPE_MASK, PCI_EXP_TYPE_ROOT_PORT);
> + break;
> + case PHY_MODE_PCIE_EP:
> + val = FIELD_PREP(HSIO_DEVICE_TYPE_MASK, PCI_EXP_TYPE_ENDPOINT);
> + break;
> + default: /* Support only PCIe EP and RC now. */
> + return 0;
> + }
> + if (submode)
> + regmap_update_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
> + HSIO_DEVICE_TYPE_MASK, val);
> +
> + return 0;
> +}
> +
> +static int imx_hsio_set_speed(struct phy *phy, int speed)
> +{
> + struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = pmix->priv;
> +
> + regmap_update_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
> + HSIO_LTSSM_ENABLE,
> + speed ? HSIO_LTSSM_ENABLE : 0);
> + return 0;
> +}
> +
> +static const struct phy_ops imx_hsio_ops = {
> + .init = imx_hsio_init,
> + .exit = imx_hsio_exit,
> + .power_on = imx_hsio_power_on,
> + .set_mode = imx_hsio_set_mode,
> + .set_speed = imx_hsio_set_speed,
> + .owner = THIS_MODULE,
> +};
> +
> +static const struct imx_hsio_drvdata imx8qxp_hsio_drvdata = {
> + .phy_mix_num = 0x1,
> +};
> +
> +static const struct imx_hsio_drvdata imx_hsio_drvdata = {
> + .phy_mix_num = 0x4,
> +};
> +
> +static const struct of_device_id imx_hsio_of_match[] = {
> + {.compatible = "fsl,imx8qm-hsio", .data = &imx_hsio_drvdata},
> + {.compatible = "fsl,imx8qxp-hsio", .data = &imx8qxp_hsio_drvdata},
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, imx_hsio_of_match);
> +
> +static struct phy *imx_hsio_xlate(struct device *dev,
> + const struct of_phandle_args *args)
> +{
> + struct imx_hsio_priv *priv = dev_get_drvdata(dev);
> + int phy_type = args->args[0];
> + int ctrl_index = args->args[1];
> + int idx, lane_mask = args->args[2];
> +
> + idx = imx_hsio_get_idx(phy_type, ctrl_index, lane_mask);
> + if (idx < 0 || idx >= priv->drvdata->phy_mix_num)
> + return ERR_PTR(-EINVAL);
> + priv->pmix[idx].phy_type = phy_type;
> + priv->pmix[idx].ctrl_index = ctrl_index;
> + priv->pmix[idx].idx = idx;
> +
> + return priv->pmix[idx].phy;
> +}
> +
> +static int imx_hsio_probe(struct platform_device *pdev)
> +{
> + int i;
> + void __iomem *off;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + const struct of_device_id *of_id;
> + struct imx_hsio_priv *priv;
> + struct phy_provider *provider;
> +
> + of_id = of_match_device(imx_hsio_of_match, dev);
> + if (!of_id)
> + return -EINVAL;
This driver only works with DT. We've entered probe because matching
happened. How could this fail?
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> + priv->dev = &pdev->dev;
> + priv->drvdata = of_device_get_match_data(dev);
> +
> + /* Get HSIO configuration mode */
> + of_property_read_u32(np, "fsl,hsio-cfg", &priv->hsio_cfg);
> + /* Get PHY refclk pad mode */
> + if (of_property_read_u32(np, "fsl,refclk-pad-mode", &priv->refclk_pad))
> + priv->refclk_pad = IMX8_PCIE_REFCLK_PAD_OUTPUT;
> +
> + priv->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(priv->base))
> + return PTR_ERR(priv->base);
> +
> + off = devm_platform_ioremap_resource_byname(pdev, "phy");
> + priv->phy = devm_regmap_init_mmio(dev, off, ®map_config);
> + if (IS_ERR(priv->phy))
> + return dev_err_probe(dev, PTR_ERR(priv->phy),
> + "unable to find phy csr registers\n");
> +
> + off = devm_platform_ioremap_resource_byname(pdev, "ctrl");
> + priv->ctrl = devm_regmap_init_mmio(dev, off, ®map_config);
> + if (IS_ERR(priv->ctrl))
> + return dev_err_probe(dev, PTR_ERR(priv->ctrl),
> + "unable to find ctrl csr registers\n");
> +
> + off = devm_platform_ioremap_resource_byname(pdev, "misc");
> + priv->misc = devm_regmap_init_mmio(dev, off, ®map_config);
> + if (IS_ERR(priv->misc))
> + return dev_err_probe(dev, PTR_ERR(priv->misc),
> + "unable to find misc csr registers\n");
> +
> + for (i = 0; i < priv->drvdata->phy_mix_num; i++) {
> + struct imx_hsio_phy_mix *pmix = &priv->pmix[i];
> + struct phy *phy;
> +
> + memset(pmix, 0, sizeof(*pmix));
> +
> + phy = devm_phy_create(&pdev->dev, NULL, &imx_hsio_ops);
You have up to 3 phys, why do you register 4?
> + if (IS_ERR(phy))
> + return PTR_ERR(phy);
> +
> + pmix->priv = priv;
> + pmix->phy = phy;
> + pmix->idx = i;
> + phy_set_drvdata(phy, pmix);
> + }
> +
> + dev_set_drvdata(dev, priv);
> + dev_set_drvdata(&pdev->dev, priv);
> +
> + provider = devm_of_phy_provider_register(&pdev->dev, imx_hsio_xlate);
> +
> + return PTR_ERR_OR_ZERO(provider);
> +}
> +
> +static struct platform_driver imx_hsio_driver = {
> + .probe = imx_hsio_probe,
> + .driver = {
> + .name = "imx8qm-hsio-phy",
> + .of_match_table = imx_hsio_of_match,
> + }
> +};
> +module_platform_driver(imx_hsio_driver);
> +
> +MODULE_DESCRIPTION("FSL IMX8QM HSIO SERDES PHY driver");
> +MODULE_LICENSE("GPL");
> --
> 2.37.1
>
On Wed, Apr 24, 2024 at 02:21:21PM +0800, Richard Zhu wrote:
> Add lane index and HSIO configuration definitions of the i.MX8Q HSIO
> SerDes PHY into header file.
This belongs in the binding patch. It is part of the binding.
> Signed-off-by: Richard Zhu <[email protected]>
> Reviewed-by: Frank Li <[email protected]>
> ---
> include/dt-bindings/phy/phy-imx8-pcie.h | 62 +++++++++++++++++++++++++
> 1 file changed, 62 insertions(+)
>
> diff --git a/include/dt-bindings/phy/phy-imx8-pcie.h b/include/dt-bindings/phy/phy-imx8-pcie.h
> index 8bbe2d6538d8..60447b95a952 100644
> --- a/include/dt-bindings/phy/phy-imx8-pcie.h
> +++ b/include/dt-bindings/phy/phy-imx8-pcie.h
> @@ -11,4 +11,66 @@
> #define IMX8_PCIE_REFCLK_PAD_INPUT 1
> #define IMX8_PCIE_REFCLK_PAD_OUTPUT 2
>
> +/*
> + * i.MX8QM HSIO subsystem has three lane PHYs and three controllers:
> + * PCIEA(2 lanes capable PCIe controller), PCIEB (only support one
> + * lane) and SATA.
> + *
> + * Meanwhile, i.MX8QXP HSIO subsystem has one lane PHY and PCIEB(only
> + * support one lane) controller.
> + *
> + * In the different use cases. PCIEA can be bound to PHY lane0, lane1
> + * or Lane0 and lane1. PCIEB can be bound to lane1 or lane2 PHY. SATA
> + * can only be bound to last lane2 PHY.
> + *
> + * +-------------------------------+------------------+
> + * | i.MX8QM | i.MX8QXP |
> + * |-------------------------------|------------------|
> + * | | PCIEA | PCIEB | SATA | | PCIEB |
> + * |-------------------------------|-------|----------|
> + * | LAN 0 | X | | | LAN 0 | * |
LAN? Local Area Network? Just use 'Lane'.
Don't need this column ^^^^^^^
> + * |-------------------------------|-------|----------|
> + * | LAN 1 | X | * | | | |
> + * |-------------------------------|-------|----------|
> + * | LAN 2 | | * | * | | |
> + * +-------------------------------+------------------+
> + * NOTE:
> + * *: Choose one only.
> + * X: Choose any of these.
> + *
> + * Define i.MX8Q HSIO PHY lane index here to specify the lane mask.
> + */
> +#define IMX8Q_HSIO_LANE0 0x1
> +#define IMX8Q_HSIO_LANE1 0x2
> +#define IMX8Q_HSIO_LANE2 0x4
Thinking about this some more, in most cases of the phy binding where
individual lanes can be assigned, each lane is a phys entry.
PCIEA:
phys = <&hsio_phy 0 PHY_MODE_PCIE>;
or:
phys = <&hsio_phy 0 PHY_MODE_PCIE>, <&hsio_phy 1 PHY_MODE_PCIE>;
PCIEB:
phys = <&hsio_phy 1 PHY_MODE_PCIE>;
or:
phys = <&hsio_phy 2 PHY_MODE_PCIE>;
SATA:
phys = <&hsio_phy 2 PHY_MODE_SATA>;
> +
> +/*
> + * Regarding the design of i.MX8QM HSIO subsystem, HSIO module can be
> + * confiured as following three use cases.
> + *
> + * Define different configurations refer to the use cases, since it is
> + * mandatory required in the initialization.
> + *
> + * On i.MX8QXP, HSIO module only has PCIEB and one lane PHY.
> + * Define "IMX8Q_HSIO_CFG_PCIEB" for i.MX8QXP platforms.
> + *
> + * +----------------------------------------------------+----------+
> + * | | i.MX8QM | i.MX8QXP |
> + * |-------------------------------|--------------------|----------|
> + * | | LAN0 | LAN1 | LAN2 | LAN0 |
s/LAN/Lane/
> + * |-------------------------------|------|------|------|----------|
> + * | IMX8Q_HSIO_CFG_PCIEAX2SATA | PCIEA| PCIEA| SATA | |
> + * |-------------------------------|------|------|------|----------|
> + * | IMX8Q_HSIO_CFG_PCIEAX2PCIEB | PCIEA| PCIEA| PCIEB| |
> + * |-------------------------------|------|------|------|----------|
> + * | IMX8Q_HSIO_CFG_PCIEAPCIEBSATA | PCIEA| PCIEB| SATA | |
> + * |-------------------------------|------|------|------|----------|
> + * | IMX8Q_HSIO_CFG_PCIEB | - | - | - | PCIEB |
> + * +----------------------------------------------------+----------+
> + */
> +#define IMX8Q_HSIO_CFG_PCIEAX2SATA 0x1
> +#define IMX8Q_HSIO_CFG_PCIEAX2PCIEB 0x2
> +#define IMX8Q_HSIO_CFG_PCIEAPCIEBSATA (IMX8Q_HSIO_CFG_PCIEAX2SATA | IMX8Q_HSIO_CFG_PCIEAX2PCIEB)
> +#define IMX8Q_HSIO_CFG_PCIEB IMX8Q_HSIO_CFG_PCIEAX2PCIEB
Again, I don't see why you need all this. You now have mode and lanes,
and per SoC data in the driver, so you should be able to figure out what
you need from this.
Rob
> -----Original Message-----
> From: Rob Herring <[email protected]>
> Sent: 2024??4??25?? 5:58
> To: Hongxing Zhu <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; Frank Li <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v3 1/3] dt-bindings: phy: phy-imx8-pcie: Add header file for
> i.MX8Q HSIO SerDes PHY
>
> On Wed, Apr 24, 2024 at 02:21:21PM +0800, Richard Zhu wrote:
> > Add lane index and HSIO configuration definitions of the i.MX8Q HSIO
> > SerDes PHY into header file.
>
> This belongs in the binding patch. It is part of the binding.
Should I merge this patch and the second into one binding patch?
>
> > Signed-off-by: Richard Zhu <[email protected]>
> > Reviewed-by: Frank Li <[email protected]>
> > ---
> > include/dt-bindings/phy/phy-imx8-pcie.h | 62
> > +++++++++++++++++++++++++
> > 1 file changed, 62 insertions(+)
> >
> > diff --git a/include/dt-bindings/phy/phy-imx8-pcie.h
> > b/include/dt-bindings/phy/phy-imx8-pcie.h
> > index 8bbe2d6538d8..60447b95a952 100644
> > --- a/include/dt-bindings/phy/phy-imx8-pcie.h
> > +++ b/include/dt-bindings/phy/phy-imx8-pcie.h
> > @@ -11,4 +11,66 @@
> > #define IMX8_PCIE_REFCLK_PAD_INPUT 1
> > #define IMX8_PCIE_REFCLK_PAD_OUTPUT 2
> >
> > +/*
> > + * i.MX8QM HSIO subsystem has three lane PHYs and three controllers:
> > + * PCIEA(2 lanes capable PCIe controller), PCIEB (only support one
> > + * lane) and SATA.
> > + *
> > + * Meanwhile, i.MX8QXP HSIO subsystem has one lane PHY and PCIEB(only
> > + * support one lane) controller.
> > + *
> > + * In the different use cases. PCIEA can be bound to PHY lane0, lane1
> > + * or Lane0 and lane1. PCIEB can be bound to lane1 or lane2 PHY. SATA
> > + * can only be bound to last lane2 PHY.
> > + *
> > + * +-------------------------------+------------------+
> > + * | i.MX8QM | i.MX8QXP |
> > + * |-------------------------------|------------------|
> > + * | | PCIEA | PCIEB | SATA | | PCIEB |
> > + * |-------------------------------|-------|----------|
> > + * | LAN 0 | X | | | LAN 0 | * |
>
> LAN? Local Area Network? Just use 'Lane'.
>
> Don't need this column ^^^^^^^
>
> > + * |-------------------------------|-------|----------|
> > + * | LAN 1 | X | * | | | |
> > + * |-------------------------------|-------|----------|
> > + * | LAN 2 | | * | * | | |
> > + * +-------------------------------+------------------+
> > + * NOTE:
> > + * *: Choose one only.
> > + * X: Choose any of these.
> > + *
> > + * Define i.MX8Q HSIO PHY lane index here to specify the lane mask.
> > + */
> > +#define IMX8Q_HSIO_LANE0 0x1
> > +#define IMX8Q_HSIO_LANE1 0x2
> > +#define IMX8Q_HSIO_LANE2 0x4
>
> Thinking about this some more, in most cases of the phy binding where individual
> lanes can be assigned, each lane is a phys entry.
>
> PCIEA:
> phys = <&hsio_phy 0 PHY_MODE_PCIE>;
> or:
> phys = <&hsio_phy 0 PHY_MODE_PCIE>, <&hsio_phy 1 PHY_MODE_PCIE>;
>
> PCIEB:
> phys = <&hsio_phy 1 PHY_MODE_PCIE>;
> or:
> phys = <&hsio_phy 2 PHY_MODE_PCIE>;
>
> SATA:
> phys = <&hsio_phy 2 PHY_MODE_SATA>;
>
> > +
> > +/*
> > + * Regarding the design of i.MX8QM HSIO subsystem, HSIO module can be
> > + * confiured as following three use cases.
> > + *
> > + * Define different configurations refer to the use cases, since it
> > +is
> > + * mandatory required in the initialization.
> > + *
> > + * On i.MX8QXP, HSIO module only has PCIEB and one lane PHY.
> > + * Define "IMX8Q_HSIO_CFG_PCIEB" for i.MX8QXP platforms.
> > + *
> > + * +----------------------------------------------------+----------+
> > + * | | i.MX8QM | i.MX8QXP |
> > + * |-------------------------------|--------------------|----------|
> > + * | | LAN0 | LAN1 | LAN2 | LAN0 |
>
> s/LAN/Lane/
>
> > + * |-------------------------------|------|------|------|----------|
> > + * | IMX8Q_HSIO_CFG_PCIEAX2SATA | PCIEA| PCIEA| SATA | |
> > + * |-------------------------------|------|------|------|----------|
> > + * | IMX8Q_HSIO_CFG_PCIEAX2PCIEB | PCIEA| PCIEA| PCIEB| |
> > + * |-------------------------------|------|------|------|----------|
> > + * | IMX8Q_HSIO_CFG_PCIEAPCIEBSATA | PCIEA| PCIEB| SATA | |
> > + * |-------------------------------|------|------|------|----------|
> > + * | IMX8Q_HSIO_CFG_PCIEB | - | - | - | PCIEB |
> > + * +----------------------------------------------------+----------+
> > + */
> > +#define IMX8Q_HSIO_CFG_PCIEAX2SATA 0x1
> > +#define IMX8Q_HSIO_CFG_PCIEAX2PCIEB 0x2
> > +#define IMX8Q_HSIO_CFG_PCIEAPCIEBSATA
> (IMX8Q_HSIO_CFG_PCIEAX2SATA | IMX8Q_HSIO_CFG_PCIEAX2PCIEB)
> > +#define IMX8Q_HSIO_CFG_PCIEB IMX8Q_HSIO_CFG_PCIEAX2PCIEB
>
> Again, I don't see why you need all this. You now have mode and lanes, and per
> SoC data in the driver, so you should be able to figure out what you need from
> this.
Thanks for your comments.
It's my fault that I didn't describe it clear.
The HSIO configuration (fsl,hsio-cfg) is one global state too refer to the
design document.
It should be known and used to set pcie_ab_select and phy_x1_epcs_sel at the
begin of HSIO initialization like this listed below.
+-------------------------------------------------------------+
|CRR(SYS.CSR) register|PCIAx2 and|PCIEAx1, PCIEBx1|PCIEAx2 and|
| |SATA |SATA |PCIEBx1 |
|---------------------|----------|----------------|-----------|
|PCIE_AB_SELECT | 0 | 1 | 1 |
|---------------------|----------|----------------|-----------|
|PHY_X1_EPCS_SEL | 1 | 1 | 0 |
+-------------------------------------------------------------+
For example, in the PCIEAx2 and SATA use case. The PHY_X1_EPCS_SEL should be
1b'1 and PCIE_AB_SELECT should be 1b'0 at the begin of the initialization of
the PCIEA stance.
And in PCIEAx2 and PCIEBx1 use case. The PCIE_AB_SELECT should be 1b'1 and
PHYX1_EPCS_SEL should be 1b'0.
Otherwise, the PCIEA instance wouldn't be functional in the end.
Unfortunately, based on lane index and phy mode of PCIEA instance, I
don't know how to set PCIE_AB_SELECT and PHY_X1_EPCC_SEL.
So, one property named "fsl,hsio-cfg" has to be introduced here to specify the
setting of the PCIE_AB_SELECT and PHY_X1_EPCS_SEL of HSIO.
This is the reason that these IMX8Q_HSIO_CFG_# macros are defined here.
Best Regards
Richard Zhu
>
> Rob
> -----Original Message-----
> From: Rob Herring <[email protected]>
> Sent: 2024??4??25?? 5:51
> To: Hongxing Zhu <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; Frank Li <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v3 3/3] phy: freescale: imx8qm-hsio: Add i.MX8QM HSIO
> PHY driver support
>
> On Wed, Apr 24, 2024 at 02:21:23PM +0800, Richard Zhu wrote:
> > Add i.MX8QM HSIO PHY driver support.
> >
> > Signed-off-by: Richard Zhu <[email protected]>
> > ---
> > drivers/phy/freescale/Kconfig | 8 +
> > drivers/phy/freescale/Makefile | 1 +
> > drivers/phy/freescale/phy-fsl-imx8qm-hsio.c | 607
> > ++++++++++++++++++++
> > 3 files changed, 616 insertions(+)
> > create mode 100644 drivers/phy/freescale/phy-fsl-imx8qm-hsio.c
> >
> > diff --git a/drivers/phy/freescale/Kconfig
> > b/drivers/phy/freescale/Kconfig index 853958fb2c06..c9ee48aeea9e
> > 100644
> > --- a/drivers/phy/freescale/Kconfig
> > +++ b/drivers/phy/freescale/Kconfig
> > @@ -35,6 +35,14 @@ config PHY_FSL_IMX8M_PCIE
> > Enable this to add support for the PCIE PHY as found on
> > i.MX8M family of SOCs.
> >
> > +config PHY_FSL_IMX8QM_HSIO
> > + tristate "Freescale i.MX8QM HSIO PHY"
> > + depends on OF && HAS_IOMEM
> > + select GENERIC_PHY
> > + help
> > + Enable this to add support for the HSIO PHY as found on
> > + i.MX8QM family of SOCs.
> > +
> > endif
> >
> > config PHY_FSL_LYNX_28G
> > diff --git a/drivers/phy/freescale/Makefile
> > b/drivers/phy/freescale/Makefile index cedb328bc4d2..b56b4d5c18ea
> > 100644
> > --- a/drivers/phy/freescale/Makefile
> > +++ b/drivers/phy/freescale/Makefile
> > @@ -3,4 +3,5 @@ obj-$(CONFIG_PHY_FSL_IMX8MQ_USB) +=
> phy-fsl-imx8mq-usb.o
> > obj-$(CONFIG_PHY_MIXEL_LVDS_PHY) += phy-fsl-imx8qm-lvds-phy.o
> > obj-$(CONFIG_PHY_MIXEL_MIPI_DPHY) += phy-fsl-imx8-mipi-dphy.o
> > obj-$(CONFIG_PHY_FSL_IMX8M_PCIE) += phy-fsl-imx8m-pcie.o
> > +obj-$(CONFIG_PHY_FSL_IMX8QM_HSIO) += phy-fsl-imx8qm-hsio.o
> > obj-$(CONFIG_PHY_FSL_LYNX_28G) += phy-fsl-lynx-28g.o
> > diff --git a/drivers/phy/freescale/phy-fsl-imx8qm-hsio.c
> > b/drivers/phy/freescale/phy-fsl-imx8qm-hsio.c
> > new file mode 100644
> > index 000000000000..b3e17163e859
> > --- /dev/null
> > +++ b/drivers/phy/freescale/phy-fsl-imx8qm-hsio.c
> > @@ -0,0 +1,607 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2024 NXP
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/io.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/module.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_device.h>
>
> Check that you are including the right DT includes.
>
> > +#include <linux/pci_regs.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/phy/pcie.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +
> > +#include <dt-bindings/phy/phy.h>
> > +#include <dt-bindings/phy/phy-imx8-pcie.h>
> > +
> > +#define MAX_NUM_PHY_MIXS 4
> > +#define PHY_MIX_MAX_NUM_LANES 2
> > +#define LANE_NUM_CLKS 5
> > +
> > +/* Parameters for the waiting for PCIe PHY PLL to lock */
> > +#define PHY_INIT_WAIT_USLEEP_MAX 10
> > +#define PHY_INIT_WAIT_TIMEOUT (1000 *
> PHY_INIT_WAIT_USLEEP_MAX)
> > +
> > +/* i.MX8Q HSIO registers */
> > +#define HSIO_CTRL0 0x0
> > +#define HSIO_APB_RSTN_0 BIT(0)
> > +#define HSIO_APB_RSTN_1 BIT(1)
> > +#define HSIO_PIPE_RSTN_0_MASK GENMASK(25, 24)
> > +#define HSIO_PIPE_RSTN_1_MASK GENMASK(27, 26)
> > +#define HSIO_MODE_MASK GENMASK(20, 17)
> > +#define HSIO_MODE_PCIE 0x0
> > +#define HSIO_MODE_SATA 0x4
> > +#define HSIO_DEVICE_TYPE_MASK GENMASK(27, 24)
> > +#define HSIO_EPCS_TXDEEMP BIT(5)
> > +#define HSIO_EPCS_TXDEEMP_SEL BIT(6)
> > +#define HSIO_EPCS_PHYRESET_N BIT(7)
> > +#define HSIO_RESET_N BIT(12)
> > +
> > +#define HSIO_IOB_RXENA BIT(0)
> > +#define HSIO_IOB_TXENA BIT(1)
> > +#define HSIO_IOB_A_0_TXOE BIT(2)
> > +#define HSIO_IOB_A_0_M1M0_2 BIT(4)
> > +#define HSIO_IOB_A_0_M1M0_MASK GENMASK(4, 3)
> > +#define HSIO_PHYX1_EPCS_SEL BIT(12)
> > +#define HSIO_PCIE_AB_SELECT BIT(13)
> > +
> > +#define HSIO_PHY_STS0 0x4
> > +#define HSIO_LANE0_TX_PLL_LOCK BIT(4)
> > +#define HSIO_LANE1_TX_PLL_LOCK BIT(12)
> > +
> > +#define HSIO_CTRL2 0x8
> > +#define HSIO_LTSSM_ENABLE BIT(4)
> > +#define HSIO_BUTTON_RST_N BIT(21)
> > +#define HSIO_PERST_N BIT(22)
> > +#define HSIO_POWER_UP_RST_N BIT(23)
> > +
> > +#define HSIO_PCIE_STS0 0xc
> > +#define HSIO_PM_REQ_CORE_RST BIT(19)
> > +
> > +#define HSIO_REG48_PMA_STATUS 0x30
> > +#define HSIO_REG48_PMA_RDY BIT(7)
> > +
> > +/*
> > + * There are three lanes PHY in i.MX8QM HSIO, and can be made up the
> > + * following PHY modes in different use cases.
> > + * +------------------------------------+
> > + * | index | LAN0 | LAN1 | LAN2 |
> > + * |------------------------------------|
> > + * | 0 | PCIEA | | |
> > + * |------------------------------------|
> > + * | 1 | | PCIEB | |
> > + * |------------------------------------|
> > + * | 2 | PCIEA | PCIEA | |
> > + * |------------------------------------|
> > + * | 3 | | | PCIEB/SATA |
> > + * +------------------------------------+
> > + */
> > +enum phy_mode_index {
> > + IMX8Q_HSIO_LANE0_PCIE_PHY,
> > + IMX8Q_HSIO_LANE1_PCIE_PHY,
> > + IMX8Q_HSIO_LANE0_1_PCIE_PHY,
> > + IMX8Q_HSIO_LANE2_PHY
> > +};
> > +
> > +struct imx_hsio_drvdata {
> > + int phy_mix_num;
> > +};
> > +
> > +struct imx_hsio_phy_lane {
> > + const char * const *clk_names;
> > + struct clk_bulk_data clks[LANE_NUM_CLKS]; };
> > +
> > +struct imx_hsio_phy_mix {
> > + u32 ctrl_index;
> > + u32 ctrl_off;
> > + u32 phy_off;
> > + u32 phy_type;
> > + struct imx_hsio_phy_lane lane[PHY_MIX_MAX_NUM_LANES];
> > + struct imx_hsio_priv *priv;
> > + struct phy *phy;
> > + enum phy_mode pmix_mode;
> > + enum phy_mode_index idx;
> > +};
> > +
> > +struct imx_hsio_priv {
> > + void __iomem *base;
> > + struct device *dev;
> > + u32 refclk_pad;
> > + u32 hsio_cfg;
> > + struct regmap *phy;
> > + struct regmap *ctrl;
> > + struct regmap *misc;
> > + const struct imx_hsio_drvdata *drvdata;
> > + struct imx_hsio_phy_mix pmix[MAX_NUM_PHY_MIXS]; };
> > +
> > +static const char * const lan0_pcie_clks[] = {"apb_pclk0", "pclk0", "ctl0_crr",
> > + "phy0_crr", "misc_crr"};
> > +static const char * const lan1_pciea_clks[] = {"apb_pclk1", "pclk1", "ctl0_crr",
> > + "phy0_crr", "misc_crr"};
> > +static const char * const lan1_pcieb_clks[] = {"apb_pclk1", "pclk1", "ctl1_crr",
> > + "phy0_crr", "misc_crr"};
> > +static const char * const lan2_pcieb_clks[] = {"apb_pclk2", "pclk2", "ctl1_crr",
> > + "phy1_crr", "misc_crr"};
> > +static const char * const lan2_sata_clks[] = {"pclk2", "epcs_tx", "epcs_rx",
> > + "phy1_crr", "misc_crr"};
> > +
> > +static const struct regmap_config regmap_config = {
> > + .reg_bits = 32,
> > + .val_bits = 32,
> > + .reg_stride = 4,
> > +};
> > +
> > +static int imx_hsio_get_idx(int phy_type, int ctrl_index, int
> > +lane_mask) {
> > + int index;
> > +
> > + switch (phy_type) {
> > + case PHY_TYPE_PCIE:
> > + if (ctrl_index) { /* PCIEB */
> > + if (lane_mask == IMX8Q_HSIO_LANE0) /* i.MX8QXP */
> > + index = IMX8Q_HSIO_LANE0_PCIE_PHY;
> > + else if (lane_mask == IMX8Q_HSIO_LANE1) /* i.MX8QM */
> > + index = IMX8Q_HSIO_LANE1_PCIE_PHY;
> > + else if (lane_mask == IMX8Q_HSIO_LANE2) /* i.MX8QM */
> > + index = IMX8Q_HSIO_LANE2_PHY;
> > + else
> > + return -EINVAL;
> > + } else { /* PCIEA */
> > + if (lane_mask == (IMX8Q_HSIO_LANE0 | IMX8Q_HSIO_LANE1))
> > + index = IMX8Q_HSIO_LANE0_1_PCIE_PHY;
> > + else if (lane_mask == IMX8Q_HSIO_LANE0)
> > + index = IMX8Q_HSIO_LANE0_PCIE_PHY;
> > + else
> > + return -EINVAL;
> > + }
> > + break;
> > + case PHY_TYPE_SATA:
> > + index = IMX8Q_HSIO_LANE2_PHY;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return index;
> > +}
> > +
> > +static int imx_hsio_init(struct phy *phy) {
> > + int ret, i;
> > + struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
> > + struct imx_hsio_priv *priv = pmix->priv;
> > + struct device *dev = priv->dev;
> > +
> > + /* Assign clocks refer to different modes */
> > + switch (pmix->phy_type) {
> > + case PHY_TYPE_PCIE:
> > + if (pmix->ctrl_index == 0) { /* PCIEA */
> > + pmix->pmix_mode = PHY_MODE_PCIE;
> > + pmix->ctrl_off = 0;
> > + pmix->phy_off = 0;
> > +
> > + for (i = 0; i < LANE_NUM_CLKS; i++) {
> > + if (pmix->idx == IMX8Q_HSIO_LANE0_PCIE_PHY) {
> > + pmix->lane[0].clks[i].id = lan0_pcie_clks[i];
> > + } else { /* 2 lanes are bound to PCIEA */
> > + pmix->lane[0].clks[i].id = lan0_pcie_clks[i];
> > + pmix->lane[1].clks[i].id = lan1_pciea_clks[i];
> > + }
> > + }
> > + } else { /* PCIEB */
> > + pmix->pmix_mode = PHY_MODE_PCIE;
> > + if (pmix->idx == IMX8Q_HSIO_LANE0_PCIE_PHY) {
> > + /* i.MX8QXP */
> > + pmix->ctrl_off = 0;
> > + pmix->phy_off = 0;
> > + } else {
> > + /*
> > + * On i.MX8QM, only second or third lane can be
> > + * bound to PCIEB.
> > + */
> > + pmix->ctrl_off = SZ_64K;
> > + if (pmix->idx == IMX8Q_HSIO_LANE1_PCIE_PHY)
> > + pmix->phy_off = 0;
> > + else /* the third lane is bound to PCIEB */
> > + pmix->phy_off = SZ_64K;
> > + }
> > +
> > + for (i = 0; i < LANE_NUM_CLKS; i++) {
> > + if (pmix->idx == IMX8Q_HSIO_LANE1_PCIE_PHY)
> > + pmix->lane[0].clks[i].id = lan1_pcieb_clks[i];
> > + else if (pmix->idx == IMX8Q_HSIO_LANE2_PHY)
> > + pmix->lane[0].clks[i].id = lan2_pcieb_clks[i];
> > + else /* i.MX8QXP only has PCIEB, idx is 0 */
> > + pmix->lane[0].clks[i].id = lan0_pcie_clks[i];
> > + }
> > + }
> > + break;
> > + case PHY_TYPE_SATA:
> > + /* On i.MX8QM, only the third lane can be bound to SATA */
> > + pmix->ctrl_off = SZ_128K;
> > + pmix->pmix_mode = PHY_MODE_SATA;
> > + pmix->phy_off = SZ_64K;
> > +
> > + for (i = 0; i < LANE_NUM_CLKS; i++)
> > + pmix->lane[0].clks[i].id = lan2_sata_clks[i];
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + /* Fetch clocks and enable them */
> > + ret = devm_clk_bulk_get(dev, LANE_NUM_CLKS, pmix->lane[0].clks);
> > + if (ret)
> > + return ret;
> > + if (pmix->idx == IMX8Q_HSIO_LANE0_1_PCIE_PHY) {
> > + ret = devm_clk_bulk_get(dev, LANE_NUM_CLKS, pmix->lane[1].clks);
> > + if (ret)
> > + return ret;
> > + }
> > + ret = clk_bulk_prepare_enable(LANE_NUM_CLKS, pmix->lane[0].clks);
> > + if (ret)
> > + return ret;
> > + if (pmix->idx == IMX8Q_HSIO_LANE0_1_PCIE_PHY) {
> > + /* Enable the second lane's clocks */
> > + ret = clk_bulk_prepare_enable(LANE_NUM_CLKS,
> > + pmix->lane[1].clks);
> > + if (ret) {
> > + clk_bulk_disable_unprepare(LANE_NUM_CLKS,
> > + pmix->lane[0].clks);
> > + return ret;
> > + }
> > + }
> > +
> > + /* allow the clocks to stabilize */
> > + usleep_range(200, 500);
> > + return 0;
> > +}
> > +
> > +static int imx_hsio_exit(struct phy *phy) {
> > + struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
> > +
> > + clk_bulk_disable_unprepare(LANE_NUM_CLKS, pmix->lane[0].clks);
> > + if (pmix->idx == IMX8Q_HSIO_LANE0_1_PCIE_PHY)
> > + /* Disable the clocks used by sencond lane of the PHY */
> > + clk_bulk_disable_unprepare(LANE_NUM_CLKS, pmix->lane[1].clks);
> > +
> > + return 0;
> > +}
> > +
> > +static void imx_hsio_pcie_phy_resets(struct phy *phy) {
> > + struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
> > + struct imx_hsio_priv *priv = pmix->priv;
> > +
> > + regmap_clear_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
> > + HSIO_BUTTON_RST_N);
> > + regmap_clear_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
> > + HSIO_PERST_N);
> > + regmap_clear_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
> > + HSIO_POWER_UP_RST_N);
> > + regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
> > + HSIO_BUTTON_RST_N);
> > + regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
> > + HSIO_PERST_N);
> > + regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
> > + HSIO_POWER_UP_RST_N);
> > +
> > + if (pmix->idx == IMX8Q_HSIO_LANE0_1_PCIE_PHY) {
> > + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> > + HSIO_APB_RSTN_0);
> > + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> > + HSIO_PIPE_RSTN_0_MASK);
> > + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> > + HSIO_APB_RSTN_1);
> > + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> > + HSIO_PIPE_RSTN_1_MASK);
> > + } else if (pmix->idx == IMX8Q_HSIO_LANE1_PCIE_PHY) {
> > + /* The second pmix */
> > + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> > + HSIO_APB_RSTN_1);
> > + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> > + HSIO_PIPE_RSTN_1_MASK);
> > + } else {
> > + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> > + HSIO_APB_RSTN_0);
> > + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> > + HSIO_PIPE_RSTN_0_MASK);
> > + }
> > +}
> > +
> > +static void imx_hsio_sata_phy_resets(struct phy *phy) {
> > + struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
> > + struct imx_hsio_priv *priv = pmix->priv;
> > +
> > + /* clear PHY RST, then set it */
> > + regmap_clear_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
> > + HSIO_EPCS_PHYRESET_N);
> > + regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
> > + HSIO_EPCS_PHYRESET_N);
> > +
> > + /* CTRL RST: SET -> delay 1 us -> CLEAR -> SET */
> > + regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0, HSIO_RESET_N);
> > + udelay(1);
> > + regmap_clear_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
> > + HSIO_RESET_N);
> > + regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
> > +HSIO_RESET_N); }
> > +
> > +static void imx_hsio_configure_clk_pad(struct phy *phy) {
> > + bool pll = false;
> > + u32 pad_mode;
> > + struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
> > + struct imx_hsio_priv *priv = pmix->priv;
> > +
> > + pad_mode = priv->refclk_pad;
> > + if (pad_mode == IMX8_PCIE_REFCLK_PAD_OUTPUT) {
> > + pll = true;
> > + regmap_update_bits(priv->misc, HSIO_CTRL0,
> > + HSIO_IOB_A_0_TXOE | HSIO_IOB_A_0_M1M0_MASK,
> > + HSIO_IOB_A_0_TXOE | HSIO_IOB_A_0_M1M0_2);
> > + }
> > +
> > + regmap_update_bits(priv->misc, HSIO_CTRL0, HSIO_IOB_RXENA,
> > + pll ? 0 : HSIO_IOB_RXENA);
> > + regmap_update_bits(priv->misc, HSIO_CTRL0, HSIO_IOB_TXENA,
> > + pll ? HSIO_IOB_TXENA : 0);
> > +}
> > +
> > +static int imx_hsio_power_on(struct phy *phy) {
> > + int ret;
> > + u32 val, addr, cond;
> > + struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
> > + struct imx_hsio_priv *priv = pmix->priv;
> > +
> > + if (pmix->pmix_mode == PHY_MODE_PCIE)
> > + imx_hsio_pcie_phy_resets(phy);
> > + else /* SATA */
> > + regmap_set_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> > + HSIO_APB_RSTN_0);
> > +
> > + if (priv->hsio_cfg & IMX8Q_HSIO_CFG_PCIEAX2PCIEB)
>
> Can't this just be based on lane==2 and mode==pcie?
>
The IMX8Q_HSIO_CFG_# are bit-map definitions.
IMX8Q_HSIO_CFG_PCIEAX2PCIEB means that not only PCIEA is enabled but also
the PCIEB is enabled too.
Thus, the PCIE_AB_SELECT should be asserted to be 1b'1.
> > + regmap_set_bits(priv->misc, HSIO_CTRL0, HSIO_PCIE_AB_SELECT);
> > + if (priv->hsio_cfg & IMX8Q_HSIO_CFG_PCIEAX2SATA)
>
> And this on mode==sata?
IMX8Q_HSIO_CFG_PCIEAX2SATA means that not only PCIEA is enabled but also
the SATA is enabled too.
So, the PHYX1_EPCS_SEL should be asserted to be 1b'1 here.
>
> > + regmap_set_bits(priv->misc, HSIO_CTRL0, HSIO_PHYX1_EPCS_SEL);
> > +
> > + imx_hsio_configure_clk_pad(phy);
>
> power_on is called per phy, but this appears to be some global state.
>
You're right. The pad setting is global and should be called once.
BTW, the setting of the PCIE_AB_SELECT and PHYX1_EPCS_SEL are global too.
> > +
> > + if (pmix->pmix_mode == PHY_MODE_SATA) {
> > + regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
> > + HSIO_EPCS_TXDEEMP);
> > + regmap_set_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
> > + HSIO_EPCS_TXDEEMP_SEL);
> > +
> > + imx_hsio_sata_phy_resets(phy);
> > + } else {
> > + /* Toggle apb_pclk to make sure PM_REQ_CORE_RST is cleared. */
> > + clk_disable_unprepare(pmix->lane[0].clks[0].clk);
> > + mdelay(1);
> > + ret = clk_prepare_enable(pmix->lane[0].clks[0].clk);
> > + if (ret) {
> > + dev_err(priv->dev, "unable to enable phy apb_pclk\n");
> > + return ret;
> > + }
> > +
> > + addr = pmix->ctrl_off + HSIO_PCIE_STS0;
> > + cond = HSIO_PM_REQ_CORE_RST;
> > + ret = regmap_read_poll_timeout(priv->ctrl, addr, val,
> > + (val & cond) == 0,
> > + PHY_INIT_WAIT_USLEEP_MAX,
> > + PHY_INIT_WAIT_TIMEOUT);
> > + if (ret) {
> > + dev_err(priv->dev, "HSIO_PM_REQ_CORE_RST is set\n");
> > + return ret;
> > + }
> > + }
> > +
> > + /* Polling to check the PHY is ready or not. */
> > + if (pmix->idx == IMX8Q_HSIO_LANE1_PCIE_PHY)
> > + cond = HSIO_LANE1_TX_PLL_LOCK;
> > + else
> > + cond = HSIO_LANE0_TX_PLL_LOCK;
>
> In SATA mode, don't you need LANE2? Or you should have exited above?
>
Except the pmix->phy_off of lane2, the others are same to the setting of lane0.
So, the PLL_LOCK check of lane2 is merged into the check of lane0 here.
>
> > +
> > + ret = regmap_read_poll_timeout(priv->phy, pmix->phy_off +
> HSIO_PHY_STS0,
> > + val, ((val & cond) == cond),
> > + PHY_INIT_WAIT_USLEEP_MAX,
> > + PHY_INIT_WAIT_TIMEOUT);
> > + if (ret) {
> > + dev_err(priv->dev, "IMX8Q PHY%d PLL lock timeout\n", pmix->idx);
> > + return ret;
> > + }
> > + dev_info(priv->dev, "IMX8Q PHY%d PLL is locked\n", pmix->idx);
>
> These info level prints should be debug level IMO.
>
> > +
> > + if (pmix->pmix_mode == PHY_MODE_SATA) {
> > + cond = HSIO_REG48_PMA_RDY;
> > + ret = read_poll_timeout(readb, val, ((val & cond) == cond),
> > + PHY_INIT_WAIT_USLEEP_MAX,
> > + PHY_INIT_WAIT_TIMEOUT, false,
> > + priv->base + HSIO_REG48_PMA_STATUS);
> > + if (ret)
> > + dev_err(priv->dev, "PHY calibration is timeout\n");
> > + else
> > + dev_info(priv->dev, "PHY calibration is done\n");
> > + }
>
> This function is a bunch of 'if SATA' or 'if PCIE' blocks which is hard to follow. I
> think it would be easier to follow if you had a specific power_on function for each
> mode which can then call any common helpers (like polling for PLL lock).
>
> And consider if mode specific stuff can go into set_mode() hook instead.
Good idea.
Thanks.
>
> > +
> > + return ret;
> > +}
> > +
> > +static int imx_hsio_set_mode(struct phy *phy, enum phy_mode mode,
> > + int submode)
> > +{
> > + u32 val;
> > + struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
> > + struct imx_hsio_priv *priv = pmix->priv;
> > +
> > + if (pmix->pmix_mode != mode)
> > + return -EINVAL;
> > +
> > + val = (mode == PHY_MODE_PCIE) ? HSIO_MODE_PCIE : HSIO_MODE_SATA;
> > + val = FIELD_PREP(HSIO_MODE_MASK, val);
> > + regmap_update_bits(priv->phy, pmix->phy_off + HSIO_CTRL0,
> > + HSIO_MODE_MASK, val);
> > +
> > + switch (submode) {
> > + case PHY_MODE_PCIE_RC:
> > + val = FIELD_PREP(HSIO_DEVICE_TYPE_MASK,
> PCI_EXP_TYPE_ROOT_PORT);
> > + break;
> > + case PHY_MODE_PCIE_EP:
> > + val = FIELD_PREP(HSIO_DEVICE_TYPE_MASK,
> PCI_EXP_TYPE_ENDPOINT);
> > + break;
> > + default: /* Support only PCIe EP and RC now. */
> > + return 0;
> > + }
> > + if (submode)
> > + regmap_update_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL0,
> > + HSIO_DEVICE_TYPE_MASK, val);
> > +
> > + return 0;
> > +}
> > +
> > +static int imx_hsio_set_speed(struct phy *phy, int speed) {
> > + struct imx_hsio_phy_mix *pmix = phy_get_drvdata(phy);
> > + struct imx_hsio_priv *priv = pmix->priv;
> > +
> > + regmap_update_bits(priv->ctrl, pmix->ctrl_off + HSIO_CTRL2,
> > + HSIO_LTSSM_ENABLE,
> > + speed ? HSIO_LTSSM_ENABLE : 0);
> > + return 0;
> > +}
> > +
> > +static const struct phy_ops imx_hsio_ops = {
> > + .init = imx_hsio_init,
> > + .exit = imx_hsio_exit,
> > + .power_on = imx_hsio_power_on,
> > + .set_mode = imx_hsio_set_mode,
> > + .set_speed = imx_hsio_set_speed,
> > + .owner = THIS_MODULE,
> > +};
> > +
> > +static const struct imx_hsio_drvdata imx8qxp_hsio_drvdata = {
> > + .phy_mix_num = 0x1,
> > +};
> > +
> > +static const struct imx_hsio_drvdata imx_hsio_drvdata = {
> > + .phy_mix_num = 0x4,
> > +};
> > +
> > +static const struct of_device_id imx_hsio_of_match[] = {
> > + {.compatible = "fsl,imx8qm-hsio", .data = &imx_hsio_drvdata},
> > + {.compatible = "fsl,imx8qxp-hsio", .data = &imx8qxp_hsio_drvdata},
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, imx_hsio_of_match);
> > +
> > +static struct phy *imx_hsio_xlate(struct device *dev,
> > + const struct of_phandle_args *args) {
> > + struct imx_hsio_priv *priv = dev_get_drvdata(dev);
> > + int phy_type = args->args[0];
> > + int ctrl_index = args->args[1];
> > + int idx, lane_mask = args->args[2];
> > +
> > + idx = imx_hsio_get_idx(phy_type, ctrl_index, lane_mask);
> > + if (idx < 0 || idx >= priv->drvdata->phy_mix_num)
> > + return ERR_PTR(-EINVAL);
> > + priv->pmix[idx].phy_type = phy_type;
> > + priv->pmix[idx].ctrl_index = ctrl_index;
> > + priv->pmix[idx].idx = idx;
> > +
> > + return priv->pmix[idx].phy;
> > +}
> > +
> > +static int imx_hsio_probe(struct platform_device *pdev) {
> > + int i;
> > + void __iomem *off;
> > + struct device *dev = &pdev->dev;
> > + struct device_node *np = dev->of_node;
> > + const struct of_device_id *of_id;
> > + struct imx_hsio_priv *priv;
> > + struct phy_provider *provider;
> > +
> > + of_id = of_match_device(imx_hsio_of_match, dev);
> > + if (!of_id)
> > + return -EINVAL;
>
> This driver only works with DT. We've entered probe because matching happened.
> How could this fail?
>
You're right.
This check can be removed totally. Thanks.
> > +
> > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > + priv->dev = &pdev->dev;
> > + priv->drvdata = of_device_get_match_data(dev);
> > +
> > + /* Get HSIO configuration mode */
> > + of_property_read_u32(np, "fsl,hsio-cfg", &priv->hsio_cfg);
> > + /* Get PHY refclk pad mode */
> > + if (of_property_read_u32(np, "fsl,refclk-pad-mode", &priv->refclk_pad))
> > + priv->refclk_pad = IMX8_PCIE_REFCLK_PAD_OUTPUT;
> > +
> > + priv->base = devm_platform_ioremap_resource(pdev, 0);
> > + if (IS_ERR(priv->base))
> > + return PTR_ERR(priv->base);
> > +
> > + off = devm_platform_ioremap_resource_byname(pdev, "phy");
> > + priv->phy = devm_regmap_init_mmio(dev, off, ®map_config);
> > + if (IS_ERR(priv->phy))
> > + return dev_err_probe(dev, PTR_ERR(priv->phy),
> > + "unable to find phy csr registers\n");
> > +
> > + off = devm_platform_ioremap_resource_byname(pdev, "ctrl");
> > + priv->ctrl = devm_regmap_init_mmio(dev, off, ®map_config);
> > + if (IS_ERR(priv->ctrl))
> > + return dev_err_probe(dev, PTR_ERR(priv->ctrl),
> > + "unable to find ctrl csr registers\n");
> > +
> > + off = devm_platform_ioremap_resource_byname(pdev, "misc");
> > + priv->misc = devm_regmap_init_mmio(dev, off, ®map_config);
> > + if (IS_ERR(priv->misc))
> > + return dev_err_probe(dev, PTR_ERR(priv->misc),
> > + "unable to find misc csr registers\n");
> > +
> > + for (i = 0; i < priv->drvdata->phy_mix_num; i++) {
> > + struct imx_hsio_phy_mix *pmix = &priv->pmix[i];
> > + struct phy *phy;
> > +
> > + memset(pmix, 0, sizeof(*pmix));
> > +
> > + phy = devm_phy_create(&pdev->dev, NULL, &imx_hsio_ops);
>
> You have up to 3 phys, why do you register 4?
Based on the 3 hardware lane phy instances, there are four phy combination modes
defined above.
So, 4 abstract phys are register here according to the different phy modes.
Anyway, I consider to drop this method, and following your suggestions
listed below.
PCIEA:
phys = <&hsio_phy 0 PHY_MODE_PCIE>;
or:
phys = <&hsio_phy 0 PHY_MODE_PCIE>, <&hsio_phy 1 PHY_MODE_PCIE>;
PCIEB:
phys = <&hsio_phy 1 PHY_MODE_PCIE>;
or:
phys = <&hsio_phy 2 PHY_MODE_PCIE>;
SATA:
phys = <&hsio_phy 2 PHY_MODE_SATA>;
Richard
Best Regards
>
>
> > + if (IS_ERR(phy))
> > + return PTR_ERR(phy);
> > +
> > + pmix->priv = priv;
> > + pmix->phy = phy;
> > + pmix->idx = i;
> > + phy_set_drvdata(phy, pmix);
> > + }
> > +
> > + dev_set_drvdata(dev, priv);
> > + dev_set_drvdata(&pdev->dev, priv);
> > +
> > + provider = devm_of_phy_provider_register(&pdev->dev,
> > +imx_hsio_xlate);
> > +
> > + return PTR_ERR_OR_ZERO(provider);
> > +}
> > +
> > +static struct platform_driver imx_hsio_driver = {
> > + .probe = imx_hsio_probe,
> > + .driver = {
> > + .name = "imx8qm-hsio-phy",
> > + .of_match_table = imx_hsio_of_match,
> > + }
> > +};
> > +module_platform_driver(imx_hsio_driver);
> > +
> > +MODULE_DESCRIPTION("FSL IMX8QM HSIO SERDES PHY driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.37.1
> >
> -----Original Message-----
> From: Hongxing Zhu
> Sent: 2024??4??25?? 16:35
> To: 'Rob Herring' <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; Frank Li <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: RE: [PATCH v3 1/3] dt-bindings: phy: phy-imx8-pcie: Add header file for
> i.MX8Q HSIO SerDes PHY
>
> > -----Original Message-----
> > From: Rob Herring <[email protected]>
> > Sent: 2024??4??25?? 5:58
> > To: Hongxing Zhu <[email protected]>
> > Cc: [email protected]; [email protected]; [email protected];
> > [email protected]; Frank Li <[email protected]>;
> > [email protected]; [email protected];
> > [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]
> > Subject: Re: [PATCH v3 1/3] dt-bindings: phy: phy-imx8-pcie: Add
> > header file for i.MX8Q HSIO SerDes PHY
> >
> > On Wed, Apr 24, 2024 at 02:21:21PM +0800, Richard Zhu wrote:
> > > Add lane index and HSIO configuration definitions of the i.MX8Q HSIO
> > > SerDes PHY into header file.
> >
> > This belongs in the binding patch. It is part of the binding.
> Should I merge this patch and the second into one binding patch?
>
> >
> > > Signed-off-by: Richard Zhu <[email protected]>
> > > Reviewed-by: Frank Li <[email protected]>
> > > ---
> > > include/dt-bindings/phy/phy-imx8-pcie.h | 62
> > > +++++++++++++++++++++++++
> > > 1 file changed, 62 insertions(+)
> > >
> > > diff --git a/include/dt-bindings/phy/phy-imx8-pcie.h
> > > b/include/dt-bindings/phy/phy-imx8-pcie.h
> > > index 8bbe2d6538d8..60447b95a952 100644
> > > --- a/include/dt-bindings/phy/phy-imx8-pcie.h
> > > +++ b/include/dt-bindings/phy/phy-imx8-pcie.h
> > > @@ -11,4 +11,66 @@
> > > #define IMX8_PCIE_REFCLK_PAD_INPUT 1
> > > #define IMX8_PCIE_REFCLK_PAD_OUTPUT 2
> > >
> > > +/*
> > > + * i.MX8QM HSIO subsystem has three lane PHYs and three controllers:
> > > + * PCIEA(2 lanes capable PCIe controller), PCIEB (only support one
> > > + * lane) and SATA.
> > > + *
> > > + * Meanwhile, i.MX8QXP HSIO subsystem has one lane PHY and
> > > +PCIEB(only
> > > + * support one lane) controller.
> > > + *
> > > + * In the different use cases. PCIEA can be bound to PHY lane0,
> > > +lane1
> > > + * or Lane0 and lane1. PCIEB can be bound to lane1 or lane2 PHY.
> > > +SATA
> > > + * can only be bound to last lane2 PHY.
> > > + *
> > > + * +-------------------------------+------------------+
> > > + * | i.MX8QM | i.MX8QXP |
> > > + * |-------------------------------|------------------|
> > > + * | | PCIEA | PCIEB | SATA | | PCIEB |
> > > + * |-------------------------------|-------|----------|
> > > + * | LAN 0 | X | | | LAN 0 | * |
> >
> > LAN? Local Area Network? Just use 'Lane'.
> >
> > Don't need this column ^^^^^^^
> >
> > > + * |-------------------------------|-------|----------|
> > > + * | LAN 1 | X | * | | | |
> > > + * |-------------------------------|-------|----------|
> > > + * | LAN 2 | | * | * | | |
> > > + * +-------------------------------+------------------+
> > > + * NOTE:
> > > + * *: Choose one only.
> > > + * X: Choose any of these.
> > > + *
> > > + * Define i.MX8Q HSIO PHY lane index here to specify the lane mask.
> > > + */
> > > +#define IMX8Q_HSIO_LANE0 0x1
> > > +#define IMX8Q_HSIO_LANE1 0x2
> > > +#define IMX8Q_HSIO_LANE2 0x4
> >
> > Thinking about this some more, in most cases of the phy binding where
> > individual lanes can be assigned, each lane is a phys entry.
> >
> > PCIEA:
> > phys = <&hsio_phy 0 PHY_MODE_PCIE>;
> > or:
> > phys = <&hsio_phy 0 PHY_MODE_PCIE>, <&hsio_phy 1 PHY_MODE_PCIE>;
> >
> > PCIEB:
> > phys = <&hsio_phy 1 PHY_MODE_PCIE>;
> > or:
> > phys = <&hsio_phy 2 PHY_MODE_PCIE>;
> >
> > SATA:
> > phys = <&hsio_phy 2 PHY_MODE_SATA>;
> >
> > > +
> > > +/*
> > > + * Regarding the design of i.MX8QM HSIO subsystem, HSIO module can
> > > +be
> > > + * confiured as following three use cases.
> > > + *
> > > + * Define different configurations refer to the use cases, since it
> > > +is
> > > + * mandatory required in the initialization.
> > > + *
> > > + * On i.MX8QXP, HSIO module only has PCIEB and one lane PHY.
> > > + * Define "IMX8Q_HSIO_CFG_PCIEB" for i.MX8QXP platforms.
> > > + *
> > > + * +----------------------------------------------------+----------+
> > > + * | | i.MX8QM |
> i.MX8QXP |
> > > + * |-------------------------------|--------------------|----------|
> > > + * | | LAN0 | LAN1 | LAN2 | LAN0
> |
> >
> > s/LAN/Lane/
> >
> > > + * |-------------------------------|------|------|------|----------|
> > > + * | IMX8Q_HSIO_CFG_PCIEAX2SATA | PCIEA| PCIEA| SATA |
> |
> > > + * |-------------------------------|------|------|------|----------|
> > > + * | IMX8Q_HSIO_CFG_PCIEAX2PCIEB | PCIEA| PCIEA| PCIEB|
> |
> > > + * |-------------------------------|------|------|------|----------|
> > > + * | IMX8Q_HSIO_CFG_PCIEAPCIEBSATA | PCIEA| PCIEB| SATA |
> |
> > > + * |-------------------------------|------|------|------|----------|
> > > + * | IMX8Q_HSIO_CFG_PCIEB | - | - | - | PCIEB |
> > > + *
> > > ++----------------------------------------------------+----------+
> > > + */
> > > +#define IMX8Q_HSIO_CFG_PCIEAX2SATA 0x1
> > > +#define IMX8Q_HSIO_CFG_PCIEAX2PCIEB 0x2
> > > +#define IMX8Q_HSIO_CFG_PCIEAPCIEBSATA
> > (IMX8Q_HSIO_CFG_PCIEAX2SATA | IMX8Q_HSIO_CFG_PCIEAX2PCIEB)
> > > +#define IMX8Q_HSIO_CFG_PCIEB IMX8Q_HSIO_CFG_PCIEAX2PCIEB
> >
> > Again, I don't see why you need all this. You now have mode and lanes,
> > and per SoC data in the driver, so you should be able to figure out
> > what you need from this.
> Thanks for your comments.
> It's my fault that I didn't describe it clear.
>
> The HSIO configuration (fsl,hsio-cfg) is one global state too refer to the design
> document.
> It should be known and used to set pcie_ab_select and phy_x1_epcs_sel at the
> begin of HSIO initialization like this listed below.
>
> +-------------------------------------------------------------+
> |CRR(SYS.CSR) register|PCIAx2 and|PCIEAx1, PCIEBx1|PCIEAx2 and|
> | |SATA |SATA |PCIEBx1 |
> |---------------------|----------|----------------|-----------|
> |PCIE_AB_SELECT | 0 | 1 | 1 |
> |---------------------|----------|----------------|-----------|
> |PHY_X1_EPCS_SEL | 1 | 1 | 0 |
> +-------------------------------------------------------------+
> For example, in the PCIEAx2 and SATA use case. The PHY_X1_EPCS_SEL should be
> 1b'1 and PCIE_AB_SELECT should be 1b'0 at the begin of the initialization of the
> PCIEA stance.
> And in PCIEAx2 and PCIEBx1 use case. The PCIE_AB_SELECT should be 1b'1 and
> PHYX1_EPCS_SEL should be 1b'0.
>
> Otherwise, the PCIEA instance wouldn't be functional in the end.
>
> Unfortunately, based on lane index and phy mode of PCIEA instance, I don't
> know how to set PCIE_AB_SELECT and PHY_X1_EPCC_SEL.
>
In another words, phy driver can't get a global view of the HSIO use case
and doesn't know how to set PCIE_AB_SELECT and PHYX1_EPCS_SEL when first
instance is probed, during the initialization of multi HSIO instances.
To set the global states properly, the "fsl,hsio-cfg" is new added here.
> So, one property named "fsl,hsio-cfg" has to be introduced here to specify the
> setting of the PCIE_AB_SELECT and PHY_X1_EPCS_SEL of HSIO.
>
> This is the reason that these IMX8Q_HSIO_CFG_# macros are defined here.
>
> Best Regards
> Richard Zhu
> >
> > Rob