2024-05-09 06:16:31

by Richard Zhu

[permalink] [raw]
Subject: [PATCH v4 0/3] Add i.MX8Q HSIO PHY support

v4 changes:
- Re-format the "phy-cells" as <&hsio_phy lane_id phy_mode controller_id>
- Use each lane as a phys entry, suggested by Rob.
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>;

- Add a new propery "fsl,hsio-cfg".
The HSIO configuration (fsl,hsio-cfg) is one global state.
It should be known and used to set global setting: 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 |
+-------------------------------------------------------------+
When first PHY instance is probed, PHY driver can't get a global view of the
HSIO use case and doesn't know how to set global setting: PCIE_AB_SELECT and
PHYX1_EPCS_SEL.
Because first PHY instance doesn't know followed PHY instance use mode.

So, one property named "fsl,hsio-cfg" has to be introduced here to specify the
setting of the global setting: PCIE_AB_SELECT and PHY_X1_EPCS_SEL.

Here is the discussion about this.
https://lkml.org/lkml/2024/4/26/231

- Address Conor's comments about the "fsl,refclk-pad-mode".
fsl,refclk-pad-mode:
description:
...
enum: ["input", "output"].

v3:https://patchwork.kernel.org/project/linux-phy/cover/[email protected]/

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 v4 1/3] dt-bindings: phy: phy-imx8-pcie: Add header file for
[PATCH v4 2/3] dt-bindings: phy: Add i.MX8Q HSIO SerDes PHY binding
[PATCH v4 3/3] phy: freescale: imx8qm-hsio: Add i.MX8QM HSIO PHY

Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml | 142 ++++++++++++++++++++
drivers/phy/freescale/Kconfig | 8 ++
drivers/phy/freescale/Makefile | 1 +
drivers/phy/freescale/phy-fsl-imx8qm-hsio.c | 598 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
include/dt-bindings/phy/phy-imx8-pcie.h | 29 +++++
5 files changed, 778 insertions(+)


2024-05-09 06:16:32

by Richard Zhu

[permalink] [raw]
Subject: [PATCH v4 1/3] dt-bindings: phy: phy-imx8-pcie: Add header file for i.MX8Q HSIO SerDes PHY

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 | 29 +++++++++++++++++++++++++
1 file changed, 29 insertions(+)

diff --git a/include/dt-bindings/phy/phy-imx8-pcie.h b/include/dt-bindings/phy/phy-imx8-pcie.h
index 8bbe2d6538d8..8f65a77fca09 100644
--- a/include/dt-bindings/phy/phy-imx8-pcie.h
+++ b/include/dt-bindings/phy/phy-imx8-pcie.h
@@ -11,4 +11,33 @@
#define IMX8_PCIE_REFCLK_PAD_INPUT 1
#define IMX8_PCIE_REFCLK_PAD_OUTPUT 2

+/*
+ * 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 |
+ * |-------------------------------|--------------------|----------|
+ * | | Lane0| Lane1| Lane2| Lane0 |
+ * |-------------------------------|------|------|------|----------|
+ * | 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


2024-05-09 06:16:37

by Richard Zhu

[permalink] [raw]
Subject: [PATCH v4 3/3] phy: freescale: imx8qm-hsio: Add i.MX8QM HSIO PHY driver support

Add i.MX8QM HSIO PHY driver support.

i.MX8QM HSIO has three lane PHY instances, and can be bound to the
following controllers in the different use cases listed in below table.
- two lanes capable PCIEA controller.
- one lane PCIEB controller.
- AHCI SATA controller.

i.MX8QM HSIO PHYs support the following use cases.
+----------------------------------------------------+
| | Lane0| Lane1| Lane2|
|-------------------------------|------|------|------|
| use case 1: PCIEAX2SATA | PCIEA| PCIEA| SATA |
|-------------------------------|------|------|------|
| use case 2: PCIEAX2PCIEB | PCIEA| PCIEA| PCIEB|
|-------------------------------|------|------|------|
| use case 3: PCIEAPCIEBSATA | PCIEA| PCIEB| SATA |
+----------------------------------------------------+

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 | 598 ++++++++++++++++++++
3 files changed, 607 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..90b57a6893c9
--- /dev/null
+++ b/drivers/phy/freescale/phy-fsl-imx8qm-hsio.c
@@ -0,0 +1,598 @@
+// 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.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_LANE 3
+#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)
+
+struct imx_hsio_drvdata {
+ int lane_num;
+};
+
+struct imx_hsio_lane {
+ u32 ctrl_index;
+ u32 ctrl_off;
+ u32 idx;
+ u32 phy_off;
+ u32 phy_type;
+ const char * const *clk_names;
+ struct clk_bulk_data clks[LANE_NUM_CLKS];
+ struct imx_hsio_priv *priv;
+ struct phy *phy;
+ enum phy_mode phy_mode;
+};
+
+struct imx_hsio_priv {
+ void __iomem *base;
+ struct device *dev;
+ atomic_t open_cnt;
+ const char *refclk_pad;
+ u32 hsio_cfg;
+ struct regmap *phy;
+ struct regmap *ctrl;
+ struct regmap *misc;
+ const struct imx_hsio_drvdata *drvdata;
+ struct imx_hsio_lane lane[MAX_NUM_LANE];
+};
+
+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_init(struct phy *phy)
+{
+ int ret, i;
+ struct imx_hsio_lane *lane = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = lane->priv;
+ struct device *dev = priv->dev;
+
+ /* Assign clocks refer to different modes */
+ switch (lane->phy_type) {
+ case PHY_TYPE_PCIE:
+ lane->phy_mode = PHY_MODE_PCIE;
+ if (lane->ctrl_index == 0) { /* PCIEA */
+ lane->ctrl_off = 0;
+ lane->phy_off = 0;
+
+ for (i = 0; i < LANE_NUM_CLKS; i++) {
+ if (lane->idx == 0)
+ lane->clks[i].id = lan0_pcie_clks[i];
+ else
+ lane->clks[i].id = lan1_pciea_clks[i];
+ }
+ } else { /* PCIEB */
+ if (lane->idx == 0) { /* i.MX8QXP */
+ lane->ctrl_off = 0;
+ lane->phy_off = 0;
+ } else {
+ /*
+ * On i.MX8QM, only second or third lane can be
+ * bound to PCIEB.
+ */
+ lane->ctrl_off = SZ_64K;
+ if (lane->idx == 1)
+ lane->phy_off = 0;
+ else /* the third lane is bound to PCIEB */
+ lane->phy_off = SZ_64K;
+ }
+
+ for (i = 0; i < LANE_NUM_CLKS; i++) {
+ if (lane->idx == 1)
+ lane->clks[i].id = lan1_pcieb_clks[i];
+ else if (lane->idx == 2)
+ lane->clks[i].id = lan2_pcieb_clks[i];
+ else /* i.MX8QXP only has PCIEB, idx is 0 */
+ lane->clks[i].id = lan0_pcie_clks[i];
+ }
+ }
+ break;
+ case PHY_TYPE_SATA:
+ /* On i.MX8QM, only the third lane can be bound to SATA */
+ lane->phy_mode = PHY_MODE_SATA;
+ lane->ctrl_off = SZ_128K;
+ lane->phy_off = SZ_64K;
+
+ for (i = 0; i < LANE_NUM_CLKS; i++)
+ lane->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, lane->clks);
+ if (ret)
+ return ret;
+ ret = clk_bulk_prepare_enable(LANE_NUM_CLKS, lane->clks);
+ if (ret)
+ return ret;
+
+ /* allow the clocks to stabilize */
+ usleep_range(200, 500);
+ return 0;
+}
+
+static int imx_hsio_exit(struct phy *phy)
+{
+ struct imx_hsio_lane *lane = phy_get_drvdata(phy);
+
+ clk_bulk_disable_unprepare(LANE_NUM_CLKS, lane->clks);
+
+ return 0;
+}
+
+static void imx_hsio_pcie_phy_resets(struct phy *phy)
+{
+ struct imx_hsio_lane *lane = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = lane->priv;
+
+ regmap_clear_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL2,
+ HSIO_BUTTON_RST_N);
+ regmap_clear_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL2,
+ HSIO_PERST_N);
+ regmap_clear_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL2,
+ HSIO_POWER_UP_RST_N);
+ regmap_set_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL2,
+ HSIO_BUTTON_RST_N);
+ regmap_set_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL2,
+ HSIO_PERST_N);
+ regmap_set_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL2,
+ HSIO_POWER_UP_RST_N);
+
+ if (lane->idx == 1) {
+ regmap_set_bits(priv->phy, lane->phy_off + HSIO_CTRL0,
+ HSIO_APB_RSTN_1);
+ regmap_set_bits(priv->phy, lane->phy_off + HSIO_CTRL0,
+ HSIO_PIPE_RSTN_1_MASK);
+ } else {
+ regmap_set_bits(priv->phy, lane->phy_off + HSIO_CTRL0,
+ HSIO_APB_RSTN_0);
+ regmap_set_bits(priv->phy, lane->phy_off + HSIO_CTRL0,
+ HSIO_PIPE_RSTN_0_MASK);
+ }
+}
+
+static void imx_hsio_sata_phy_resets(struct phy *phy)
+{
+ struct imx_hsio_lane *lane = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = lane->priv;
+
+ /* clear PHY RST, then set it */
+ regmap_clear_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL0,
+ HSIO_EPCS_PHYRESET_N);
+ regmap_set_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL0,
+ HSIO_EPCS_PHYRESET_N);
+
+ /* CTRL RST: SET -> delay 1 us -> CLEAR -> SET */
+ regmap_set_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL0, HSIO_RESET_N);
+ udelay(1);
+ regmap_clear_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL0,
+ HSIO_RESET_N);
+ regmap_set_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL0, HSIO_RESET_N);
+}
+
+static void imx_hsio_configure_clk_pad(struct phy *phy)
+{
+ bool pll = false;
+ struct imx_hsio_lane *lane = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = lane->priv;
+
+ if (strncmp(priv->refclk_pad, "output", 6) == 0) {
+ 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);
+ } else {
+ regmap_update_bits(priv->misc, HSIO_CTRL0,
+ HSIO_IOB_A_0_TXOE | HSIO_IOB_A_0_M1M0_MASK,
+ 0);
+ }
+
+ 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 void imx_hsio_pre_set(struct phy *phy)
+{
+ struct imx_hsio_lane *lane = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = lane->priv;
+
+ atomic_set(&priv->open_cnt, 1);
+ 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);
+}
+
+static int imx_hsio_pcie_power_on(struct phy *phy)
+{
+ int ret;
+ u32 val, addr, cond;
+ struct imx_hsio_lane *lane = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = lane->priv;
+
+ imx_hsio_pcie_phy_resets(phy);
+
+ /* Toggle apb_pclk to make sure PM_REQ_CORE_RST is cleared. */
+ clk_disable_unprepare(lane->clks[0].clk);
+ mdelay(1);
+ ret = clk_prepare_enable(lane->clks[0].clk);
+ if (ret) {
+ dev_err(priv->dev, "unable to enable phy apb_pclk\n");
+ return ret;
+ }
+
+ addr = lane->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;
+}
+
+static int imx_hsio_sata_power_on(struct phy *phy)
+{
+ int ret;
+ u32 val, cond;
+ struct imx_hsio_lane *lane = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = lane->priv;
+
+ regmap_set_bits(priv->phy, lane->phy_off + HSIO_CTRL0, HSIO_APB_RSTN_0);
+ regmap_set_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL0,
+ HSIO_EPCS_TXDEEMP);
+ regmap_set_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL0,
+ HSIO_EPCS_TXDEEMP_SEL);
+
+ imx_hsio_sata_phy_resets(phy);
+
+ 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_dbg(priv->dev, "PHY calibration is done\n");
+
+ return ret;
+}
+
+static int imx_hsio_power_on(struct phy *phy)
+{
+ int ret;
+ u32 val, cond;
+ struct imx_hsio_lane *lane = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = lane->priv;
+
+ if (atomic_inc_and_test(&priv->open_cnt))
+ imx_hsio_pre_set(phy);
+
+ if (lane->phy_mode == PHY_MODE_PCIE)
+ ret = imx_hsio_pcie_power_on(phy);
+ else /* SATA */
+ ret = imx_hsio_sata_power_on(phy);
+ if (ret)
+ return ret;
+
+ /* Polling to check the PHY is ready or not. */
+ if (lane->idx == 1)
+ cond = HSIO_LANE1_TX_PLL_LOCK;
+ else
+ /*
+ * Except the phy_off, the bit-offset of lane2 is same to lane0.
+ * Merge the lane0 and lane2 bit-operations together.
+ */
+ cond = HSIO_LANE0_TX_PLL_LOCK;
+
+ ret = regmap_read_poll_timeout(priv->phy, lane->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", lane->idx);
+ return ret;
+ }
+ dev_dbg(priv->dev, "IMX8Q PHY%d PLL is locked\n", lane->idx);
+
+ return ret;
+}
+
+static int imx_hsio_power_off(struct phy *phy)
+{
+ struct imx_hsio_lane *lane = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = lane->priv;
+
+ if (atomic_dec_and_test(&priv->open_cnt)) {
+ atomic_set(&priv->open_cnt, -1);
+ regmap_clear_bits(priv->misc, HSIO_CTRL0, HSIO_PCIE_AB_SELECT);
+ regmap_clear_bits(priv->misc, HSIO_CTRL0, HSIO_PHYX1_EPCS_SEL);
+
+ if (lane->phy_mode == PHY_MODE_PCIE) {
+ regmap_clear_bits(priv->ctrl,
+ lane->ctrl_off + HSIO_CTRL2,
+ HSIO_BUTTON_RST_N);
+ regmap_clear_bits(priv->ctrl,
+ lane->ctrl_off + HSIO_CTRL2,
+ HSIO_PERST_N);
+ regmap_clear_bits(priv->ctrl,
+ lane->ctrl_off + HSIO_CTRL2,
+ HSIO_POWER_UP_RST_N);
+ } else {
+ regmap_clear_bits(priv->ctrl,
+ lane->ctrl_off + HSIO_CTRL0,
+ HSIO_EPCS_TXDEEMP);
+ regmap_clear_bits(priv->ctrl,
+ lane->ctrl_off + HSIO_CTRL0,
+ HSIO_EPCS_TXDEEMP_SEL);
+ regmap_clear_bits(priv->ctrl,
+ lane->ctrl_off + HSIO_CTRL0,
+ HSIO_RESET_N);
+ }
+
+ if (lane->idx == 1) {
+ regmap_clear_bits(priv->phy, lane->phy_off + HSIO_CTRL0,
+ HSIO_APB_RSTN_1);
+ regmap_clear_bits(priv->phy, lane->phy_off + HSIO_CTRL0,
+ HSIO_PIPE_RSTN_1_MASK);
+ } else {
+ /*
+ * Except the phy_off, the bit-offset of lane2 is same
+ * to lane0. Merge the lane0 and lane2 bit-operations
+ * together.
+ */
+ regmap_clear_bits(priv->phy, lane->phy_off + HSIO_CTRL0,
+ HSIO_APB_RSTN_0);
+ regmap_clear_bits(priv->phy, lane->phy_off + HSIO_CTRL0,
+ HSIO_PIPE_RSTN_0_MASK);
+ }
+ }
+
+ return 0;
+}
+
+static int imx_hsio_set_mode(struct phy *phy, enum phy_mode mode,
+ int submode)
+{
+ u32 val;
+ struct imx_hsio_lane *lane = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = lane->priv;
+
+ if (lane->phy_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, lane->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, lane->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_lane *lane = phy_get_drvdata(phy);
+ struct imx_hsio_priv *priv = lane->priv;
+
+ regmap_update_bits(priv->ctrl, lane->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,
+ .power_off = imx_hsio_power_off,
+ .set_mode = imx_hsio_set_mode,
+ .set_speed = imx_hsio_set_speed,
+ .owner = THIS_MODULE,
+};
+
+static const struct imx_hsio_drvdata imx8qxp_hsio_drvdata = {
+ .lane_num = 0x1,
+};
+
+static const struct imx_hsio_drvdata imx_hsio_drvdata = {
+ .lane_num = 0x3,
+};
+
+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 idx = args->args[0];
+ int phy_type = args->args[1];
+ int ctrl_index = args->args[2];
+
+ if (idx < 0 || idx >= priv->drvdata->lane_num)
+ return ERR_PTR(-EINVAL);
+ priv->lane[idx].idx = idx;
+ priv->lane[idx].phy_type = phy_type;
+ priv->lane[idx].ctrl_index = ctrl_index;
+
+ return priv->lane[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;
+ struct imx_hsio_priv *priv;
+ struct phy_provider *provider;
+
+ 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);
+ atomic_set(&priv->open_cnt, -1);
+
+ /* Get HSIO configuration mode */
+ if (of_property_read_u32(np, "fsl,hsio-cfg", &priv->hsio_cfg))
+ priv->hsio_cfg = 0;
+ /* Get PHY refclk pad mode */
+ if (of_property_read_string(np, "fsl,refclk-pad-mode",
+ &priv->refclk_pad))
+ priv->refclk_pad = NULL;
+
+ 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, &regmap_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, &regmap_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, &regmap_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->lane_num; i++) {
+ struct imx_hsio_lane *lane = &priv->lane[i];
+ struct phy *phy;
+
+ memset(lane, 0, sizeof(*lane));
+
+ phy = devm_phy_create(&pdev->dev, NULL, &imx_hsio_ops);
+ if (IS_ERR(phy))
+ return PTR_ERR(phy);
+
+ lane->priv = priv;
+ lane->phy = phy;
+ lane->idx = i;
+ phy_set_drvdata(phy, lane);
+ }
+
+ 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


2024-05-09 06:16:50

by Richard Zhu

[permalink] [raw]
Subject: [PATCH v4 2/3] dt-bindings: phy: Add i.MX8Q HSIO SerDes PHY binding

Add i.MX8QM and i.MX8QXP HSIO SerDes PHY binding.
Introduce one HSIO configuration 'fsl,hsio-cfg', which need be set at
initialization according to board design.

Signed-off-by: Richard Zhu <[email protected]>
---
.../bindings/phy/fsl,imx8qm-hsio.yaml | 142 ++++++++++++++++++
1 file changed, 142 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml

diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml
new file mode 100644
index 000000000000..e8648cd9fea6
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml
@@ -0,0 +1,142 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/fsl,imx8qm-hsio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale i.MX8QM SoC series HSIO SERDES PHY
+
+maintainers:
+ - Richard Zhu <[email protected]>
+
+properties:
+ compatible:
+ enum:
+ - fsl,imx8qm-hsio
+ - fsl,imx8qxp-hsio
+ reg:
+ minItems: 4
+ maxItems: 4
+
+ "#phy-cells":
+ const: 3
+ description:
+ The first defines lane index.
+ The second defines the type of the PHY refer to the include phy.h.
+ The third defines the controller index, indicated which controller
+ is bound to the lane.
+
+ reg-names:
+ items:
+ - const: reg
+ - const: phy
+ - const: ctrl
+ - const: misc
+
+ clocks:
+ minItems: 5
+ maxItems: 14
+
+ clock-names:
+ minItems: 5
+ maxItems: 14
+
+ fsl,hsio-cfg:
+ description: Refer macro HSIO_CFG* include/dt-bindings/phy/phy-imx8-pcie.h.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ fsl,refclk-pad-mode:
+ description:
+ Specifies the mode of the refclk pad used. INPUT(PHY refclock is
+ provided externally via the refclk pad) or OUTPUT(PHY refclock is
+ derived from SoC internal source and provided on the refclk pad).
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [ "input", "output" ]
+
+ power-domains:
+ minItems: 1
+ maxItems: 2
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - "#phy-cells"
+ - clocks
+ - clock-names
+ - fsl,hsio-cfg
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - fsl,imx8qxp-hsio
+ then:
+ properties:
+ clock-names:
+ items:
+ - const: pclk0
+ - const: apb_pclk0
+ - const: phy0_crr
+ - const: ctl0_crr
+ - const: misc_crr
+ power-domains:
+ minItems: 1
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - fsl,imx8qm-hsio
+ then:
+ properties:
+ clock-names:
+ items:
+ - const: pclk0
+ - const: pclk1
+ - const: apb_pclk0
+ - const: apb_pclk1
+ - const: pclk2
+ - const: epcs_tx
+ - const: epcs_rx
+ - const: apb_pclk2
+ - const: phy0_crr
+ - const: phy1_crr
+ - const: ctl0_crr
+ - const: ctl1_crr
+ - const: ctl2_crr
+ - const: misc_crr
+ power-domains:
+ minItems: 2
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/imx8-clock.h>
+ #include <dt-bindings/clock/imx8-lpcg.h>
+ #include <dt-bindings/firmware/imx/rsrc.h>
+ #include <dt-bindings/phy/phy-imx8-pcie.h>
+
+ hsio_phy@5f1a0000 {
+ compatible = "fsl,imx8qxp-hsio";
+ reg = <0x5f1a0000 0x10000>,
+ <0x5f120000 0x10000>,
+ <0x5f140000 0x10000>,
+ <0x5f160000 0x10000>;
+ reg-names = "reg", "phy", "ctrl", "misc";
+ clocks = <&phyx1_lpcg IMX_LPCG_CLK_0>,
+ <&phyx1_lpcg IMX_LPCG_CLK_4>,
+ <&phyx1_crr1_lpcg IMX_LPCG_CLK_4>,
+ <&pcieb_crr3_lpcg IMX_LPCG_CLK_4>,
+ <&misc_crr5_lpcg IMX_LPCG_CLK_4>;
+ clock-names = "pclk0", "apb_pclk0", "phy0_crr", "ctl0_crr", "misc_crr";
+ power-domains = <&pd IMX_SC_R_SERDES_1>;
+ #phy-cells = <3>;
+ fsl,hsio-cfg = <IMX8Q_HSIO_CFG_PCIEB>;
+ fsl,refclk-pad-mode = "input";
+ };
+...
--
2.37.1


2024-05-09 07:37:25

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] dt-bindings: phy: Add i.MX8Q HSIO SerDes PHY binding


On Thu, 09 May 2024 13:56:20 +0800, Richard Zhu wrote:
> Add i.MX8QM and i.MX8QXP HSIO SerDes PHY binding.
> Introduce one HSIO configuration 'fsl,hsio-cfg', which need be set at
> initialization according to board design.
>
> Signed-off-by: Richard Zhu <[email protected]>
> ---
> .../bindings/phy/fsl,imx8qm-hsio.yaml | 142 ++++++++++++++++++
> 1 file changed, 142 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml
>

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
/Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml:54:13: [error] string value is redundantly quoted with any quotes (quoted-strings)
/Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml:54:22: [error] string value is redundantly quoted with any quotes (quoted-strings)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


2024-05-09 14:44:33

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] phy: freescale: imx8qm-hsio: Add i.MX8QM HSIO PHY driver support

On Thu, May 09, 2024 at 01:56:21PM +0800, Richard Zhu wrote:
> Add i.MX8QM HSIO PHY driver support.
>
> i.MX8QM HSIO has three lane PHY instances, and can be bound to the
> following controllers in the different use cases listed in below table.
> - two lanes capable PCIEA controller.
> - one lane PCIEB controller.
> - AHCI SATA controller.
>
> i.MX8QM HSIO PHYs support the following use cases.
> +----------------------------------------------------+
> | | Lane0| Lane1| Lane2|
> |-------------------------------|------|------|------|
> | use case 1: PCIEAX2SATA | PCIEA| PCIEA| SATA |
> |-------------------------------|------|------|------|
> | use case 2: PCIEAX2PCIEB | PCIEA| PCIEA| PCIEB|
> |-------------------------------|------|------|------|
> | use case 3: PCIEAPCIEBSATA | PCIEA| PCIEB| SATA |
> +----------------------------------------------------+
>
> 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 | 598 ++++++++++++++++++++
> 3 files changed, 607 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..90b57a6893c9
> --- /dev/null
> +++ b/drivers/phy/freescale/phy-fsl-imx8qm-hsio.c
> @@ -0,0 +1,598 @@
> +// 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.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_LANE 3
> +#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)
> +
> +struct imx_hsio_drvdata {
> + int lane_num;
> +};
> +
> +struct imx_hsio_lane {
> + u32 ctrl_index;
> + u32 ctrl_off;
> + u32 idx;
> + u32 phy_off;
> + u32 phy_type;
> + const char * const *clk_names;
> + struct clk_bulk_data clks[LANE_NUM_CLKS];
> + struct imx_hsio_priv *priv;
> + struct phy *phy;
> + enum phy_mode phy_mode;
> +};
> +
> +struct imx_hsio_priv {
> + void __iomem *base;
> + struct device *dev;
> + atomic_t open_cnt;
> + const char *refclk_pad;
> + u32 hsio_cfg;
> + struct regmap *phy;
> + struct regmap *ctrl;
> + struct regmap *misc;
> + const struct imx_hsio_drvdata *drvdata;
> + struct imx_hsio_lane lane[MAX_NUM_LANE];
> +};
> +
> +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_init(struct phy *phy)
> +{
> + int ret, i;
> + struct imx_hsio_lane *lane = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = lane->priv;
> + struct device *dev = priv->dev;
> +
> + /* Assign clocks refer to different modes */
> + switch (lane->phy_type) {
> + case PHY_TYPE_PCIE:
> + lane->phy_mode = PHY_MODE_PCIE;
> + if (lane->ctrl_index == 0) { /* PCIEA */
> + lane->ctrl_off = 0;
> + lane->phy_off = 0;
> +
> + for (i = 0; i < LANE_NUM_CLKS; i++) {
> + if (lane->idx == 0)
> + lane->clks[i].id = lan0_pcie_clks[i];
> + else
> + lane->clks[i].id = lan1_pciea_clks[i];
> + }
> + } else { /* PCIEB */
> + if (lane->idx == 0) { /* i.MX8QXP */
> + lane->ctrl_off = 0;
> + lane->phy_off = 0;
> + } else {
> + /*
> + * On i.MX8QM, only second or third lane can be
> + * bound to PCIEB.
> + */
> + lane->ctrl_off = SZ_64K;
> + if (lane->idx == 1)
> + lane->phy_off = 0;
> + else /* the third lane is bound to PCIEB */
> + lane->phy_off = SZ_64K;
> + }
> +
> + for (i = 0; i < LANE_NUM_CLKS; i++) {
> + if (lane->idx == 1)
> + lane->clks[i].id = lan1_pcieb_clks[i];
> + else if (lane->idx == 2)
> + lane->clks[i].id = lan2_pcieb_clks[i];
> + else /* i.MX8QXP only has PCIEB, idx is 0 */
> + lane->clks[i].id = lan0_pcie_clks[i];
> + }
> + }
> + break;
> + case PHY_TYPE_SATA:
> + /* On i.MX8QM, only the third lane can be bound to SATA */
> + lane->phy_mode = PHY_MODE_SATA;
> + lane->ctrl_off = SZ_128K;
> + lane->phy_off = SZ_64K;
> +
> + for (i = 0; i < LANE_NUM_CLKS; i++)
> + lane->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, lane->clks);
> + if (ret)
> + return ret;
> + ret = clk_bulk_prepare_enable(LANE_NUM_CLKS, lane->clks);
> + if (ret)
> + return ret;
> +
> + /* allow the clocks to stabilize */
> + usleep_range(200, 500);
> + return 0;
> +}
> +
> +static int imx_hsio_exit(struct phy *phy)
> +{
> + struct imx_hsio_lane *lane = phy_get_drvdata(phy);
> +
> + clk_bulk_disable_unprepare(LANE_NUM_CLKS, lane->clks);
> +
> + return 0;
> +}
> +
> +static void imx_hsio_pcie_phy_resets(struct phy *phy)
> +{
> + struct imx_hsio_lane *lane = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = lane->priv;
> +
> + regmap_clear_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL2,
> + HSIO_BUTTON_RST_N);
> + regmap_clear_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL2,
> + HSIO_PERST_N);
> + regmap_clear_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL2,
> + HSIO_POWER_UP_RST_N);
> + regmap_set_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL2,
> + HSIO_BUTTON_RST_N);
> + regmap_set_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL2,
> + HSIO_PERST_N);
> + regmap_set_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL2,
> + HSIO_POWER_UP_RST_N);
> +
> + if (lane->idx == 1) {
> + regmap_set_bits(priv->phy, lane->phy_off + HSIO_CTRL0,
> + HSIO_APB_RSTN_1);
> + regmap_set_bits(priv->phy, lane->phy_off + HSIO_CTRL0,
> + HSIO_PIPE_RSTN_1_MASK);
> + } else {
> + regmap_set_bits(priv->phy, lane->phy_off + HSIO_CTRL0,
> + HSIO_APB_RSTN_0);
> + regmap_set_bits(priv->phy, lane->phy_off + HSIO_CTRL0,
> + HSIO_PIPE_RSTN_0_MASK);
> + }
> +}
> +
> +static void imx_hsio_sata_phy_resets(struct phy *phy)
> +{
> + struct imx_hsio_lane *lane = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = lane->priv;
> +
> + /* clear PHY RST, then set it */
> + regmap_clear_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL0,
> + HSIO_EPCS_PHYRESET_N);
> + regmap_set_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL0,
> + HSIO_EPCS_PHYRESET_N);
> +
> + /* CTRL RST: SET -> delay 1 us -> CLEAR -> SET */
> + regmap_set_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL0, HSIO_RESET_N);
> + udelay(1);
> + regmap_clear_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL0,
> + HSIO_RESET_N);
> + regmap_set_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL0, HSIO_RESET_N);
> +}
> +
> +static void imx_hsio_configure_clk_pad(struct phy *phy)
> +{
> + bool pll = false;
> + struct imx_hsio_lane *lane = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = lane->priv;
> +
> + if (strncmp(priv->refclk_pad, "output", 6) == 0) {
> + 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);
> + } else {
> + regmap_update_bits(priv->misc, HSIO_CTRL0,
> + HSIO_IOB_A_0_TXOE | HSIO_IOB_A_0_M1M0_MASK,
> + 0);
> + }
> +
> + 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 void imx_hsio_pre_set(struct phy *phy)
> +{
> + struct imx_hsio_lane *lane = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = lane->priv;
> +
> + atomic_set(&priv->open_cnt, 1);
> + 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);
> +}
> +
> +static int imx_hsio_pcie_power_on(struct phy *phy)
> +{
> + int ret;
> + u32 val, addr, cond;
> + struct imx_hsio_lane *lane = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = lane->priv;
> +
> + imx_hsio_pcie_phy_resets(phy);
> +
> + /* Toggle apb_pclk to make sure PM_REQ_CORE_RST is cleared. */
> + clk_disable_unprepare(lane->clks[0].clk);
> + mdelay(1);
> + ret = clk_prepare_enable(lane->clks[0].clk);
> + if (ret) {
> + dev_err(priv->dev, "unable to enable phy apb_pclk\n");
> + return ret;
> + }
> +
> + addr = lane->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;
> +}
> +
> +static int imx_hsio_sata_power_on(struct phy *phy)
> +{
> + int ret;
> + u32 val, cond;
> + struct imx_hsio_lane *lane = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = lane->priv;
> +
> + regmap_set_bits(priv->phy, lane->phy_off + HSIO_CTRL0, HSIO_APB_RSTN_0);
> + regmap_set_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL0,
> + HSIO_EPCS_TXDEEMP);
> + regmap_set_bits(priv->ctrl, lane->ctrl_off + HSIO_CTRL0,
> + HSIO_EPCS_TXDEEMP_SEL);
> +
> + imx_hsio_sata_phy_resets(phy);
> +
> + 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_dbg(priv->dev, "PHY calibration is done\n");
> +
> + return ret;
> +}
> +
> +static int imx_hsio_power_on(struct phy *phy)
> +{
> + int ret;
> + u32 val, cond;
> + struct imx_hsio_lane *lane = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = lane->priv;
> +
> + if (atomic_inc_and_test(&priv->open_cnt))
> + imx_hsio_pre_set(phy);

when multi client try power_on at the same time, it need wait first
imx_hsio_pre_set(phy) finish. So atomic is not enough here.

scope_guard(mutex, &priv->lock) {
if (!priv->open_cnt)
imx_hsio_pre_set(phy);
priv->open_cnt ++
}

in power_off function
scope_gard(mutex, &priv->lock) {
priv->open_cnt --;
}

> +
> + if (lane->phy_mode == PHY_MODE_PCIE)
> + ret = imx_hsio_pcie_power_on(phy);
> + else /* SATA */
> + ret = imx_hsio_sata_power_on(phy);
> + if (ret)
> + return ret;
> +
> + /* Polling to check the PHY is ready or not. */
> + if (lane->idx == 1)
> + cond = HSIO_LANE1_TX_PLL_LOCK;
> + else
> + /*
> + * Except the phy_off, the bit-offset of lane2 is same to lane0.
> + * Merge the lane0 and lane2 bit-operations together.
> + */
> + cond = HSIO_LANE0_TX_PLL_LOCK;
> +
> + ret = regmap_read_poll_timeout(priv->phy, lane->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", lane->idx);
> + return ret;
> + }
> + dev_dbg(priv->dev, "IMX8Q PHY%d PLL is locked\n", lane->idx);
> +
> + return ret;
> +}
> +
> +static int imx_hsio_power_off(struct phy *phy)
> +{
> + struct imx_hsio_lane *lane = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = lane->priv;
> +
> + if (atomic_dec_and_test(&priv->open_cnt)) {
> + atomic_set(&priv->open_cnt, -1);
> + regmap_clear_bits(priv->misc, HSIO_CTRL0, HSIO_PCIE_AB_SELECT);
> + regmap_clear_bits(priv->misc, HSIO_CTRL0, HSIO_PHYX1_EPCS_SEL);
> +
> + if (lane->phy_mode == PHY_MODE_PCIE) {
> + regmap_clear_bits(priv->ctrl,
> + lane->ctrl_off + HSIO_CTRL2,
> + HSIO_BUTTON_RST_N);
> + regmap_clear_bits(priv->ctrl,
> + lane->ctrl_off + HSIO_CTRL2,
> + HSIO_PERST_N);
> + regmap_clear_bits(priv->ctrl,
> + lane->ctrl_off + HSIO_CTRL2,
> + HSIO_POWER_UP_RST_N);
> + } else {
> + regmap_clear_bits(priv->ctrl,
> + lane->ctrl_off + HSIO_CTRL0,
> + HSIO_EPCS_TXDEEMP);
> + regmap_clear_bits(priv->ctrl,
> + lane->ctrl_off + HSIO_CTRL0,
> + HSIO_EPCS_TXDEEMP_SEL);
> + regmap_clear_bits(priv->ctrl,
> + lane->ctrl_off + HSIO_CTRL0,
> + HSIO_RESET_N);
> + }
> +
> + if (lane->idx == 1) {
> + regmap_clear_bits(priv->phy, lane->phy_off + HSIO_CTRL0,
> + HSIO_APB_RSTN_1);
> + regmap_clear_bits(priv->phy, lane->phy_off + HSIO_CTRL0,
> + HSIO_PIPE_RSTN_1_MASK);
> + } else {
> + /*
> + * Except the phy_off, the bit-offset of lane2 is same
> + * to lane0. Merge the lane0 and lane2 bit-operations
> + * together.
> + */
> + regmap_clear_bits(priv->phy, lane->phy_off + HSIO_CTRL0,
> + HSIO_APB_RSTN_0);
> + regmap_clear_bits(priv->phy, lane->phy_off + HSIO_CTRL0,
> + HSIO_PIPE_RSTN_0_MASK);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int imx_hsio_set_mode(struct phy *phy, enum phy_mode mode,
> + int submode)
> +{
> + u32 val;
> + struct imx_hsio_lane *lane = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = lane->priv;
> +
> + if (lane->phy_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, lane->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, lane->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_lane *lane = phy_get_drvdata(phy);
> + struct imx_hsio_priv *priv = lane->priv;
> +
> + regmap_update_bits(priv->ctrl, lane->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,
> + .power_off = imx_hsio_power_off,
> + .set_mode = imx_hsio_set_mode,
> + .set_speed = imx_hsio_set_speed,
> + .owner = THIS_MODULE,
> +};
> +
> +static const struct imx_hsio_drvdata imx8qxp_hsio_drvdata = {
> + .lane_num = 0x1,
> +};
> +
> +static const struct imx_hsio_drvdata imx_hsio_drvdata = {
> + .lane_num = 0x3,
> +};
> +
> +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 idx = args->args[0];
> + int phy_type = args->args[1];
> + int ctrl_index = args->args[2];
> +
> + if (idx < 0 || idx >= priv->drvdata->lane_num)
> + return ERR_PTR(-EINVAL);
> + priv->lane[idx].idx = idx;
> + priv->lane[idx].phy_type = phy_type;
> + priv->lane[idx].ctrl_index = ctrl_index;
> +
> + return priv->lane[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;
> + struct imx_hsio_priv *priv;
> + struct phy_provider *provider;
> +
> + 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);
> + atomic_set(&priv->open_cnt, -1);
> +
> + /* Get HSIO configuration mode */
> + if (of_property_read_u32(np, "fsl,hsio-cfg", &priv->hsio_cfg))
> + priv->hsio_cfg = 0;
> + /* Get PHY refclk pad mode */
> + if (of_property_read_string(np, "fsl,refclk-pad-mode",
> + &priv->refclk_pad))
> + priv->refclk_pad = NULL;
> +
> + 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, &regmap_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, &regmap_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, &regmap_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->lane_num; i++) {
> + struct imx_hsio_lane *lane = &priv->lane[i];
> + struct phy *phy;
> +
> + memset(lane, 0, sizeof(*lane));
> +
> + phy = devm_phy_create(&pdev->dev, NULL, &imx_hsio_ops);
> + if (IS_ERR(phy))
> + return PTR_ERR(phy);
> +
> + lane->priv = priv;
> + lane->phy = phy;
> + lane->idx = i;
> + phy_set_drvdata(phy, lane);
> + }
> +
> + 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
>

2024-05-10 15:39:01

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] dt-bindings: phy: Add i.MX8Q HSIO SerDes PHY binding

On Thu, May 09, 2024 at 01:56:20PM +0800, Richard Zhu wrote:
> Add i.MX8QM and i.MX8QXP HSIO SerDes PHY binding.
> Introduce one HSIO configuration 'fsl,hsio-cfg', which need be set at
> initialization according to board design.
>
> Signed-off-by: Richard Zhu <[email protected]>
> ---
> .../bindings/phy/fsl,imx8qm-hsio.yaml | 142 ++++++++++++++++++
> 1 file changed, 142 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml
>
> diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml
> new file mode 100644
> index 000000000000..e8648cd9fea6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml
> @@ -0,0 +1,142 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/fsl,imx8qm-hsio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale i.MX8QM SoC series HSIO SERDES PHY
> +
> +maintainers:
> + - Richard Zhu <[email protected]>
> +
> +properties:
> + compatible:
> + enum:
> + - fsl,imx8qm-hsio
> + - fsl,imx8qxp-hsio
> + reg:
> + minItems: 4
> + maxItems: 4

Need to define what is each entry:

items:
- description: ...
- description: ...
- description: ...
- description: ...


> +
> + "#phy-cells":
> + const: 3
> + description:
> + The first defines lane index.
> + The second defines the type of the PHY refer to the include phy.h.
> + The third defines the controller index, indicated which controller
> + is bound to the lane.
> +
> + reg-names:
> + items:
> + - const: reg
> + - const: phy
> + - const: ctrl
> + - const: misc
> +
> + clocks:
> + minItems: 5
> + maxItems: 14
> +
> + clock-names:
> + minItems: 5
> + maxItems: 14
> +
> + fsl,hsio-cfg:
> + description: Refer macro HSIO_CFG* include/dt-bindings/phy/phy-imx8-pcie.h.

I can't, it's not in this patch. But it should be.

Please say something about what this is for, not just refer to header.

> + $ref: /schemas/types.yaml#/definitions/uint32

maximum: 3

> +
> + fsl,refclk-pad-mode:
> + description:
> + Specifies the mode of the refclk pad used. INPUT(PHY refclock is
> + provided externally via the refclk pad) or OUTPUT(PHY refclock is
> + derived from SoC internal source and provided on the refclk pad).
> + $ref: /schemas/types.yaml#/definitions/string
> + enum: [ "input", "output" ]

default?

Really, this could just be a boolean for the non-default mode.

> +
> + power-domains:
> + minItems: 1
> + maxItems: 2
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - "#phy-cells"
> + - clocks
> + - clock-names
> + - fsl,hsio-cfg
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - fsl,imx8qxp-hsio
> + then:
> + properties:
> + clock-names:
> + items:
> + - const: pclk0
> + - const: apb_pclk0
> + - const: phy0_crr
> + - const: ctl0_crr
> + - const: misc_crr
> + power-domains:
> + minItems: 1

Should be maxItems? min is already 1.

> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - fsl,imx8qm-hsio
> + then:
> + properties:
> + clock-names:
> + items:
> + - const: pclk0
> + - const: pclk1
> + - const: apb_pclk0
> + - const: apb_pclk1
> + - const: pclk2
> + - const: epcs_tx
> + - const: epcs_rx
> + - const: apb_pclk2
> + - const: phy0_crr
> + - const: phy1_crr
> + - const: ctl0_crr
> + - const: ctl1_crr
> + - const: ctl2_crr
> + - const: misc_crr
> + power-domains:
> + minItems: 2
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/imx8-clock.h>
> + #include <dt-bindings/clock/imx8-lpcg.h>
> + #include <dt-bindings/firmware/imx/rsrc.h>
> + #include <dt-bindings/phy/phy-imx8-pcie.h>
> +
> + hsio_phy@5f1a0000 {

phy@...

> + compatible = "fsl,imx8qxp-hsio";
> + reg = <0x5f1a0000 0x10000>,
> + <0x5f120000 0x10000>,
> + <0x5f140000 0x10000>,
> + <0x5f160000 0x10000>;
> + reg-names = "reg", "phy", "ctrl", "misc";
> + clocks = <&phyx1_lpcg IMX_LPCG_CLK_0>,
> + <&phyx1_lpcg IMX_LPCG_CLK_4>,
> + <&phyx1_crr1_lpcg IMX_LPCG_CLK_4>,
> + <&pcieb_crr3_lpcg IMX_LPCG_CLK_4>,
> + <&misc_crr5_lpcg IMX_LPCG_CLK_4>;
> + clock-names = "pclk0", "apb_pclk0", "phy0_crr", "ctl0_crr", "misc_crr";
> + power-domains = <&pd IMX_SC_R_SERDES_1>;
> + #phy-cells = <3>;
> + fsl,hsio-cfg = <IMX8Q_HSIO_CFG_PCIEB>;
> + fsl,refclk-pad-mode = "input";
> + };
> +...
> --
> 2.37.1
>

2024-05-10 15:52:34

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] dt-bindings: phy: Add i.MX8Q HSIO SerDes PHY binding

On Fri, May 10, 2024 at 10:38:49AM -0500, Rob Herring wrote:
> On Thu, May 09, 2024 at 01:56:20PM +0800, Richard Zhu wrote:
> > + fsl,refclk-pad-mode:
> > + description:
> > + Specifies the mode of the refclk pad used. INPUT(PHY refclock is
> > + provided externally via the refclk pad) or OUTPUT(PHY refclock is
> > + derived from SoC internal source and provided on the refclk pad).
> > + $ref: /schemas/types.yaml#/definitions/string
> > + enum: [ "input", "output" ]
>
> default?
>
> Really, this could just be a boolean for the non-default mode.

There's actually a third option, or at least there was in v1, unused.
The description in v1 was:

It can be UNUSED(PHY
refclock is derived from SoC internal source), INPUT(PHY refclock
is provided externally via the refclk pad) or OUTPUT(PHY refclock
is derived from SoC internal source and provided on the refclk pad).

I suggested that there should be 3 strings and not having the property
would mean unused. But all mention of unused seems to have vanished :/


Attachments:
(No filename) (1.06 kB)
signature.asc (235.00 B)
Download all attachments

2024-05-10 16:10:42

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] dt-bindings: phy: Add i.MX8Q HSIO SerDes PHY binding

On Fri, May 10, 2024 at 04:52:22PM +0100, Conor Dooley wrote:
> On Fri, May 10, 2024 at 10:38:49AM -0500, Rob Herring wrote:
> > On Thu, May 09, 2024 at 01:56:20PM +0800, Richard Zhu wrote:
> > > + fsl,refclk-pad-mode:
> > > + description:
> > > + Specifies the mode of the refclk pad used. INPUT(PHY refclock is
> > > + provided externally via the refclk pad) or OUTPUT(PHY refclock is
> > > + derived from SoC internal source and provided on the refclk pad).
> > > + $ref: /schemas/types.yaml#/definitions/string
> > > + enum: [ "input", "output" ]
> >
> > default?
> >
> > Really, this could just be a boolean for the non-default mode.
>
> There's actually a third option, or at least there was in v1, unused.
> The description in v1 was:
>
> It can be UNUSED(PHY
> refclock is derived from SoC internal source), INPUT(PHY refclock
> is provided externally via the refclk pad) or OUTPUT(PHY refclock
> is derived from SoC internal source and provided on the refclk pad).
>
> I suggested that there should be 3 strings and not having the property
> would mean unused. But all mention of unused seems to have vanished :/

Yes, missed mention about unused.

Is it okay about "fsl,refclk-pad-mode not exist means unused ?

Of course, need mention in description.


Frank


2024-05-11 03:21:30

by Richard Zhu

[permalink] [raw]
Subject: RE: [PATCH v4 2/3] dt-bindings: phy: Add i.MX8Q HSIO SerDes PHY binding

> -----Original Message-----
> From: Rob Herring <[email protected]>
> Sent: 2024??5??10?? 23:39
> 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 v4 2/3] dt-bindings: phy: Add i.MX8Q HSIO SerDes PHY
> binding
>
> On Thu, May 09, 2024 at 01:56:20PM +0800, Richard Zhu wrote:
> > Add i.MX8QM and i.MX8QXP HSIO SerDes PHY binding.
> > Introduce one HSIO configuration 'fsl,hsio-cfg', which need be set at
> > initialization according to board design.
> >
> > Signed-off-by: Richard Zhu <[email protected]>
> > ---
> > .../bindings/phy/fsl,imx8qm-hsio.yaml | 142
> ++++++++++++++++++
> > 1 file changed, 142 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml
> > b/Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml
> > new file mode 100644
> > index 000000000000..e8648cd9fea6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml
> > @@ -0,0 +1,142 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fschemas%2Fphy%2Ffsl%2Cimx8qm-hsio.yaml%23&data=05%7C
> 02%7
> >
> +Chongxing.zhu%40nxp.com%7C2a3e55cc15c4465304eb08dc71074c2f%7C68
> 6ea1d3
> >
> +bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638509523353991074%7CUnkn
> own%7CTWF
> >
> +pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVC
> I6
> >
> +Mn0%3D%7C0%7C%7C%7C&sdata=fBXfU7NNPj57mnI3VXR3vR250oJ7kJrLNjY
> 5W6mL0sk
> > +%3D&reserved=0
> > +$schema:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C02%7Chongxing.z
> hu%
> >
> +40nxp.com%7C2a3e55cc15c4465304eb08dc71074c2f%7C686ea1d3bc2b4c6f
> a92cd9
> >
> +9c5c301635%7C0%7C0%7C638509523353999176%7CUnknown%7CTWFpbG
> Zsb3d8eyJWI
> >
> +joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0
> %7C%
> >
> +7C%7C&sdata=n01dVLoIaHimlyaQTU1hUwIUNvmno7YUVnrjkTZK5TQ%3D&res
> erved=0
> > +
> > +title: Freescale i.MX8QM SoC series HSIO SERDES PHY
> > +
> > +maintainers:
> > + - Richard Zhu <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - fsl,imx8qm-hsio
> > + - fsl,imx8qxp-hsio
> > + reg:
> > + minItems: 4
> > + maxItems: 4
>
> Need to define what is each entry:
>
> items:
> - description: ...
> - description: ...
> - description: ...
> - description: ...
>
>
> > +
> > + "#phy-cells":
> > + const: 3
> > + description:
> > + The first defines lane index.
> > + The second defines the type of the PHY refer to the include phy.h.
> > + The third defines the controller index, indicated which controller
> > + is bound to the lane.
> > +
> > + reg-names:
> > + items:
> > + - const: reg
> > + - const: phy
> > + - const: ctrl
> > + - const: misc
> > +
> > + clocks:
> > + minItems: 5
> > + maxItems: 14
> > +
> > + clock-names:
> > + minItems: 5
> > + maxItems: 14
> > +
> > + fsl,hsio-cfg:
> > + description: Refer macro HSIO_CFG*
> include/dt-bindings/phy/phy-imx8-pcie.h.
>
> I can't, it's not in this patch. But it should be.
>
Hi Rob:
Thanks for your comments.
I would merge the first two of the patch-set into one patch later.

Best Regards
Richard Zhu

> Please say something about what this is for, not just refer to header.
>
> > + $ref: /schemas/types.yaml#/definitions/uint32
>
> maximum: 3
>
> > +
> > + fsl,refclk-pad-mode:
> > + description:
> > + Specifies the mode of the refclk pad used. INPUT(PHY refclock is
> > + provided externally via the refclk pad) or OUTPUT(PHY refclock is
> > + derived from SoC internal source and provided on the refclk pad).
> > + $ref: /schemas/types.yaml#/definitions/string
> > + enum: [ "input", "output" ]
>
> default?
>
> Really, this could just be a boolean for the non-default mode.
>
> > +
> > + power-domains:
> > + minItems: 1
> > + maxItems: 2
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - reg-names
> > + - "#phy-cells"
> > + - clocks
> > + - clock-names
> > + - fsl,hsio-cfg
> > +
> > +allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - fsl,imx8qxp-hsio
> > + then:
> > + properties:
> > + clock-names:
> > + items:
> > + - const: pclk0
> > + - const: apb_pclk0
> > + - const: phy0_crr
> > + - const: ctl0_crr
> > + - const: misc_crr
> > + power-domains:
> > + minItems: 1
>
> Should be maxItems? min is already 1.
>
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - fsl,imx8qm-hsio
> > + then:
> > + properties:
> > + clock-names:
> > + items:
> > + - const: pclk0
> > + - const: pclk1
> > + - const: apb_pclk0
> > + - const: apb_pclk1
> > + - const: pclk2
> > + - const: epcs_tx
> > + - const: epcs_rx
> > + - const: apb_pclk2
> > + - const: phy0_crr
> > + - const: phy1_crr
> > + - const: ctl0_crr
> > + - const: ctl1_crr
> > + - const: ctl2_crr
> > + - const: misc_crr
> > + power-domains:
> > + minItems: 2
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/imx8-clock.h>
> > + #include <dt-bindings/clock/imx8-lpcg.h>
> > + #include <dt-bindings/firmware/imx/rsrc.h>
> > + #include <dt-bindings/phy/phy-imx8-pcie.h>
> > +
> > + hsio_phy@5f1a0000 {
>
> phy@...
>
> > + compatible = "fsl,imx8qxp-hsio";
> > + reg = <0x5f1a0000 0x10000>,
> > + <0x5f120000 0x10000>,
> > + <0x5f140000 0x10000>,
> > + <0x5f160000 0x10000>;
> > + reg-names = "reg", "phy", "ctrl", "misc";
> > + clocks = <&phyx1_lpcg IMX_LPCG_CLK_0>,
> > + <&phyx1_lpcg IMX_LPCG_CLK_4>,
> > + <&phyx1_crr1_lpcg IMX_LPCG_CLK_4>,
> > + <&pcieb_crr3_lpcg IMX_LPCG_CLK_4>,
> > + <&misc_crr5_lpcg IMX_LPCG_CLK_4>;
> > + clock-names = "pclk0", "apb_pclk0", "phy0_crr", "ctl0_crr",
> "misc_crr";
> > + power-domains = <&pd IMX_SC_R_SERDES_1>;
> > + #phy-cells = <3>;
> > + fsl,hsio-cfg = <IMX8Q_HSIO_CFG_PCIEB>;
> > + fsl,refclk-pad-mode = "input";
> > + };
> > +...
> > --
> > 2.37.1
> >