2023-02-23 02:00:11

by Changhuang Liang

[permalink] [raw]
Subject: [PATCH v2 0/3] Add JH7110 MIPI DPHY RX support

This patchset adds mipi dphy rx driver for the StarFive JH7110 SoC.
It is used to transfer CSI camera data. The series has been tested on
the VisionFive 2 board.

This patchset should be applied after the patchset [1] and patch [2]:
[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/

changes since v1:
- Rebased on tag v6.2.
- Dropped patch 1, it will be added by the patch [2].

patch 1:
- Changed the node name 'dphy' to 'phy'.
- Changed the "starfive,aon-syscon" description.
- Changed the MIPI DPHY RX IP description.
- Add description to resets.
- Update devicetree binding examples.

patch 2:
- Changed the commit message.

patch 3:
- Changed the commit message.
- Changed the node name 'dphy' to 'phy'.
- Sorted the node by address.

v1: https://lore.kernel.org/all/[email protected]/

Changhuang Liang (3):
dt-bindings: phy: Add starfive,jh7110-dphy-rx
phy: starfive: Add mipi dphy rx support
riscv: dts: starfive: Add dphy rx node

.../bindings/phy/starfive,jh7110-dphy-rx.yaml | 74 ++++
MAINTAINERS | 7 +
arch/riscv/boot/dts/starfive/jh7110.dtsi | 13 +
drivers/phy/Kconfig | 1 +
drivers/phy/Makefile | 1 +
drivers/phy/starfive/Kconfig | 13 +
drivers/phy/starfive/Makefile | 2 +
drivers/phy/starfive/phy-starfive-dphy-rx.c | 362 ++++++++++++++++++
8 files changed, 473 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-rx.yaml
create mode 100644 drivers/phy/starfive/Kconfig
create mode 100644 drivers/phy/starfive/Makefile
create mode 100644 drivers/phy/starfive/phy-starfive-dphy-rx.c


base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c
prerequisite-patch-id: 4dc515731ce237184553c1606ffb3afaeb51c3d8
prerequisite-patch-id: ac150a8c622e858e088df8121093d448df49c245
prerequisite-patch-id: a4255724d4698f1238663443024de56de38d717b
prerequisite-patch-id: a798370d170dc2bcc79ed86f741c21c1e6d87c78
prerequisite-patch-id: 203d2500cadc112bd20fefc56eabf1470d3d2d2d
prerequisite-patch-id: 315303931e4b6499de7127a88113763f86e97e16
prerequisite-patch-id: 40cb8212ddb024c20593f73d8b87d9894877e172
prerequisite-patch-id: a1673a9e9f19d6fab5a51abb721e54e36636f067
prerequisite-patch-id: 94860423c7acc9025249d4bb36652a585bd0a797
prerequisite-patch-id: b5084253283929d9a6d0e66c350400c7c85d034d
prerequisite-patch-id: a428ed7a2aa45abab86923dc467e1e6b08427e85
prerequisite-patch-id: d4f80829fca7ce370a6fad766593cdcb502fa245
prerequisite-patch-id: e3490e19e089fe284334db300ee189b619a61628
prerequisite-patch-id: 34298e3882261bc2d72955b1570cc9612ab7d662
prerequisite-patch-id: 377c5c282a0776feee9acd10b565adbd5275a67e
prerequisite-patch-id: 3ccee718de0750adbf8d0b77d553a2778a344f64
prerequisite-patch-id: 4710f2ac22dca0bdd9ff5d744d2c37cab3c74515
prerequisite-patch-id: 65f2aed865d88e6fa468d2923527b523d4313857
prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f
prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9
prerequisite-patch-id: e3b986b9c60b2b93b7812ec174c9e1b4cfb14c97
prerequisite-patch-id: a2b3a9cff8a683422eb0ccf3a0850091401812d4
prerequisite-patch-id: dbb0c0151b8bdf093e6ce79fd2fe3f60791a6e0b
prerequisite-patch-id: ea9a6d0313dd3936c8de0239dc2072c3360a2f6b
prerequisite-patch-id: d57e95d31686772abc4c4d5aa1cadc344dc293cd
prerequisite-patch-id: 29aab7148bf56a20acddcb8a11f290705fcc97f6
prerequisite-patch-id: eeecb5367bea99627210b661986ca5b49e2a9d63
prerequisite-patch-id: c02658f677990683d4c4957e06c19ed3a3a0cfab
prerequisite-patch-id: 2ddada18ab6ea5cd1da14212aaf59632f5203d40
prerequisite-patch-id: 55b80c069faccc7dede15748d888bb13c7120a30
prerequisite-patch-id: 7acbc9c924e802712d3574dd74a6b3576089f78c
prerequisite-patch-id: 35b9e27cc439a35eebc18cbe375ea65da94653be
prerequisite-patch-id: d426704849c21687b4ef32c7d399081706e34c51
prerequisite-patch-id: 9f71c539a241baf1e73c7e7dfde5b0b04c66a502
prerequisite-patch-id: 5acc7f9a7547a301014071f446685e21779cb5f7
prerequisite-patch-id: 9fddcb61301b615ad6f0102f2235265a01ccc2a2
prerequisite-patch-id: 0c04762f1d20f09cd2a1356334a86e520907d111
prerequisite-patch-id: 04d1167facdccfd6c737bf64d83cfd6ed1d74099
prerequisite-patch-id: ff5bca2dbaafaa14248f5f9a6ddc5d26d827cf50
prerequisite-patch-id: 2bc43b375b470f7e8bbe937b78678ba3856e3b8f
--
2.25.1


2023-02-23 02:00:13

by Changhuang Liang

[permalink] [raw]
Subject: [PATCH v2 1/3] dt-bindings: phy: Add starfive,jh7110-dphy-rx

Starfive SoCs like the jh7110 use a MIPI D-PHY RX controller based on
a M31 IP. Add a binding for it.

Signed-off-by: Changhuang Liang <[email protected]>
---
.../bindings/phy/starfive,jh7110-dphy-rx.yaml | 74 +++++++++++++++++++
1 file changed, 74 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-rx.yaml

diff --git a/Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-rx.yaml b/Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-rx.yaml
new file mode 100644
index 000000000000..a67ca57a6f21
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-rx.yaml
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/starfive,jh7110-dphy-rx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Starfive SoC MIPI D-PHY Rx Controller
+
+maintainers:
+ - Jack Zhu <[email protected]>
+ - Changhuang Liang <[email protected]>
+
+description:
+ The Starfive SoC uses the MIPI CSI D-PHY based on M31 IP to transfer
+ CSI camera data.
+
+properties:
+ compatible:
+ const: starfive,jh7110-dphy-rx
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 3
+
+ clock-names:
+ items:
+ - const: cfg
+ - const: ref
+ - const: tx
+
+ resets:
+ items:
+ - description: DPHY_HW reset
+ - description: DPHY_B09_ALWAYS_ON reset
+
+ starfive,aon-syscon:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ items:
+ items:
+ - description: phandle of AON SYSCON
+ - description: register offset
+ description: The power of dphy rx is configured by AON SYSCON
+ in this property.
+
+ "#phy-cells":
+ const: 0
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - resets
+ - starfive,aon-syscon
+ - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ phy@19820000 {
+ compatible = "starfive,jh7110-dphy-rx";
+ reg = <0x19820000 0x10000>;
+ clocks = <&ispcrg 3>,
+ <&ispcrg 4>,
+ <&ispcrg 5>;
+ clock-names = "cfg", "ref", "tx";
+ resets = <&ispcrg 2>,
+ <&ispcrg 3>;
+ starfive,aon-syscon = <&aon_syscon 0x00>;
+ #phy-cells = <0>;
+ };
--
2.25.1


2023-02-23 02:00:16

by Changhuang Liang

[permalink] [raw]
Subject: [PATCH v2 3/3] riscv: dts: starfive: Add dphy rx node

Add dphy rx node for the Starfive JH7110 SoC. It is used to transfer CSI
camera data.

Signed-off-by: Changhuang Liang <[email protected]>
---
arch/riscv/boot/dts/starfive/jh7110.dtsi | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index 697ab59191a1..074423d17786 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -563,6 +563,19 @@ ispcrg: clock-controller@19810000 {
power-domains = <&pwrc JH7110_PD_ISP>;
};

+ csi_phy: phy@19820000 {
+ compatible = "starfive,jh7110-dphy-rx";
+ reg = <0x0 0x19820000 0x0 0x10000>;
+ clocks = <&ispcrg JH7110_ISPCLK_M31DPHY_CFGCLK_IN>,
+ <&ispcrg JH7110_ISPCLK_M31DPHY_REFCLK_IN>,
+ <&ispcrg JH7110_ISPCLK_M31DPHY_TXCLKESC_LAN0>;
+ clock-names = "cfg", "ref", "tx";
+ resets = <&ispcrg JH7110_ISPRST_M31DPHY_HW>,
+ <&ispcrg JH7110_ISPRST_M31DPHY_B09_ALWAYS_ON>;
+ starfive,aon-syscon = <&aon_syscon 0x00>;
+ #phy-cells = <0>;
+ };
+
voutcrg: clock-controller@295C0000 {
compatible = "starfive,jh7110-voutcrg";
reg = <0x0 0x295C0000 0x0 0x10000>;
--
2.25.1


2023-02-23 02:00:18

by Changhuang Liang

[permalink] [raw]
Subject: [PATCH v2 2/3] phy: starfive: Add mipi dphy rx support

Add mipi dphy rx support for the Starfive JH7110 SoC. It is used to
transfer CSI camera data.

Signed-off-by: Changhuang Liang <[email protected]>
---
MAINTAINERS | 7 +
drivers/phy/Kconfig | 1 +
drivers/phy/Makefile | 1 +
drivers/phy/starfive/Kconfig | 13 +
drivers/phy/starfive/Makefile | 2 +
drivers/phy/starfive/phy-starfive-dphy-rx.c | 362 ++++++++++++++++++++
6 files changed, 386 insertions(+)
create mode 100644 drivers/phy/starfive/Kconfig
create mode 100644 drivers/phy/starfive/Makefile
create mode 100644 drivers/phy/starfive/phy-starfive-dphy-rx.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 2e70c9f21989..8ddef8669efb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19952,6 +19952,13 @@ F: Documentation/devicetree/bindings/power/starfive*
F: drivers/soc/starfive/jh71xx_pmu.c
F: include/dt-bindings/power/starfive,jh7110-pmu.h

+STARFIVE JH7110 DPHY RX DRIVER
+M: Jack Zhu <[email protected]>
+M: Changhuang Liang <[email protected]>
+S: Supported
+F: Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-rx.yaml
+F: drivers/phy/starfive/phy-starfive-dphy-rx.c
+
STATIC BRANCH/CALL
M: Peter Zijlstra <[email protected]>
M: Josh Poimboeuf <[email protected]>
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 7bd00a11d074..c4b2a86e2afb 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -91,6 +91,7 @@ source "drivers/phy/rockchip/Kconfig"
source "drivers/phy/samsung/Kconfig"
source "drivers/phy/socionext/Kconfig"
source "drivers/phy/st/Kconfig"
+source "drivers/phy/starfive/Kconfig"
source "drivers/phy/sunplus/Kconfig"
source "drivers/phy/tegra/Kconfig"
source "drivers/phy/ti/Kconfig"
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 54f312c10a40..fb3dc9de6111 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -31,6 +31,7 @@ obj-y += allwinner/ \
samsung/ \
socionext/ \
st/ \
+ starfive/ \
sunplus/ \
tegra/ \
ti/ \
diff --git a/drivers/phy/starfive/Kconfig b/drivers/phy/starfive/Kconfig
new file mode 100644
index 000000000000..e449a662acf5
--- /dev/null
+++ b/drivers/phy/starfive/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Phy drivers for Starfive platforms
+#
+
+config PHY_STARFIVE_DPHY_RX
+ tristate "Starfive D-PHY RX Support"
+ select GENERIC_PHY
+ select GENERIC_PHY_MIPI_DPHY
+ help
+ Choose this option if you have a Starfive D-PHY in your
+ system. If M is selected, the module will be called
+ phy-starfive-dphy-rx.
diff --git a/drivers/phy/starfive/Makefile b/drivers/phy/starfive/Makefile
new file mode 100644
index 000000000000..7ec576cb30ae
--- /dev/null
+++ b/drivers/phy/starfive/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_PHY_STARFIVE_DPHY_RX) += phy-starfive-dphy-rx.o
diff --git a/drivers/phy/starfive/phy-starfive-dphy-rx.c b/drivers/phy/starfive/phy-starfive-dphy-rx.c
new file mode 100644
index 000000000000..2a7c5453f4ce
--- /dev/null
+++ b/drivers/phy/starfive/phy-starfive-dphy-rx.c
@@ -0,0 +1,362 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * DPHY driver for the StarFive JH7110 SoC
+ *
+ * Copyright (C) 2023 StarFive Technology Co., Ltd.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+
+#define STF_DPHY_APBCFGSAIF__SYSCFG(x) (x)
+
+#define STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_EN BIT(6)
+#define STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_IN GENMASK(12, 7)
+#define STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_EN BIT(19)
+#define STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_IN GENMASK(25, 20)
+
+#define STF_DPHY_DATA_BUS16_8 BIT(8)
+#define STF_DPHY_DEBUG_MODE_SEL GENMASK(15, 9)
+
+#define STF_DPHY_ENABLE_CLK BIT(6)
+#define STF_DPHY_ENABLE_CLK1 BIT(7)
+#define STF_DPHY_ENABLE_LAN0 BIT(8)
+#define STF_DPHY_ENABLE_LAN1 BIT(9)
+#define STF_DPHY_ENABLE_LAN2 BIT(10)
+#define STF_DPHY_ENABLE_LAN3 BIT(11)
+#define STF_DPHY_GPI_EN GENMASK(17, 12)
+#define STF_DPHY_HS_FREQ_CHANGE_CLK BIT(18)
+#define STF_DPHY_HS_FREQ_CHANGE_CLK1 BIT(19)
+#define STF_DPHY_LANE_SWAP_CLK GENMASK(22, 20)
+#define STF_DPHY_LANE_SWAP_CLK1 GENMASK(25, 23)
+#define STF_DPHY_LANE_SWAP_LAN0 GENMASK(28, 26)
+#define STF_DPHY_LANE_SWAP_LAN1 GENMASK(31, 29)
+
+#define STF_DPHY_LANE_SWAP_LAN2 GENMASK(2, 0)
+#define STF_DPHY_LANE_SWAP_LAN3 GENMASK(5, 3)
+#define STF_DPHY_MP_TEST_EN BIT(6)
+#define STF_DPHY_MP_TEST_MODE_SEL GENMASK(11, 7)
+#define STF_DPHY_PLL_CLK_SEL GENMASK(21, 12)
+#define STF_DPHY_PRECOUNTER_IN_CLK GENMASK(29, 22)
+
+#define STF_DPHY_PRECOUNTER_IN_CLK1 GENMASK(7, 0)
+#define STF_DPHY_PRECOUNTER_IN_LAN0 GENMASK(15, 8)
+#define STF_DPHY_PRECOUNTER_IN_LAN1 GENMASK(23, 16)
+#define STF_DPHY_PRECOUNTER_IN_LAN2 GENMASK(31, 24)
+
+#define STF_DPHY_PRECOUNTER_IN_LAN3 GENMASK(7, 0)
+#define STF_DPHY_RX_1C2C_SEL BIT(8)
+
+struct regval_t {
+ u32 addr;
+ u32 val;
+};
+
+struct stf_dphy {
+ struct device *dev;
+ void __iomem *regs;
+ struct clk *cfg_clk;
+ struct clk *ref_clk;
+ struct clk *tx_clk;
+ struct reset_control *rstc;
+ struct regulator *mipi_0p9;
+ struct phy *phy;
+ struct regmap *stf_aon_syscon;
+ unsigned int aon_gp_reg;
+};
+
+struct stf_dphy_info {
+ bool external_support;
+ int (*external_get)(struct stf_dphy *dphy);
+ void (*external_init)(struct stf_dphy *dphy);
+ void (*external_exit)(struct stf_dphy *dphy);
+};
+
+static const struct stf_dphy_info *stf_dphy_info;
+
+static const struct regval_t stf_dphy_init_list[] = {
+ { STF_DPHY_APBCFGSAIF__SYSCFG(4), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(8), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(12), 0x0000fff0 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(16), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(20), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(24), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(28), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(32), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(36), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(40), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(40), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(48), 0x24000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(52), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(56), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(60), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(64), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(68), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(72), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(76), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(80), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(84), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(88), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(92), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(96), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(100), 0x02000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(104), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(108), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(112), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(116), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(120), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(124), 0x0000000c },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(128), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(132), 0xcc500000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(136), 0x000000cc },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(140), 0x00000000 },
+ { STF_DPHY_APBCFGSAIF__SYSCFG(144), 0x00000000 },
+};
+
+static int stf_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
+{
+ struct stf_dphy *dphy = phy_get_drvdata(phy);
+ int map[6] = {4, 0, 1, 2, 3, 5};
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(stf_dphy_init_list); i++)
+ writel(stf_dphy_init_list[i].val,
+ dphy->regs + stf_dphy_init_list[i].addr);
+
+ writel(FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_EN, 1) |
+ FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_IN, 0x1b) |
+ FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_EN, 1) |
+ FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_IN, 0x1b),
+ dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(0));
+
+ writel(FIELD_PREP(STF_DPHY_DATA_BUS16_8, 0) |
+ FIELD_PREP(STF_DPHY_DEBUG_MODE_SEL, 0x5a),
+ dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(184));
+
+ writel(FIELD_PREP(STF_DPHY_ENABLE_CLK, 1) |
+ FIELD_PREP(STF_DPHY_ENABLE_CLK1, 1) |
+ FIELD_PREP(STF_DPHY_ENABLE_LAN0, 1) |
+ FIELD_PREP(STF_DPHY_ENABLE_LAN1, 1) |
+ FIELD_PREP(STF_DPHY_ENABLE_LAN2, 1) |
+ FIELD_PREP(STF_DPHY_ENABLE_LAN3, 1) |
+ FIELD_PREP(STF_DPHY_GPI_EN, 0) |
+ FIELD_PREP(STF_DPHY_HS_FREQ_CHANGE_CLK, 0) |
+ FIELD_PREP(STF_DPHY_HS_FREQ_CHANGE_CLK1, 0) |
+ FIELD_PREP(STF_DPHY_LANE_SWAP_CLK, map[0]) |
+ FIELD_PREP(STF_DPHY_LANE_SWAP_CLK1, map[5]) |
+ FIELD_PREP(STF_DPHY_LANE_SWAP_LAN0, map[1]) |
+ FIELD_PREP(STF_DPHY_LANE_SWAP_LAN1, map[2]),
+ dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(188));
+
+ writel(FIELD_PREP(STF_DPHY_LANE_SWAP_LAN2, map[3]) |
+ FIELD_PREP(STF_DPHY_LANE_SWAP_LAN3, map[4]) |
+ FIELD_PREP(STF_DPHY_MP_TEST_EN, 0) |
+ FIELD_PREP(STF_DPHY_MP_TEST_MODE_SEL, 0) |
+ FIELD_PREP(STF_DPHY_PLL_CLK_SEL, 0x37c) |
+ FIELD_PREP(STF_DPHY_PRECOUNTER_IN_CLK, 8),
+ dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(192));
+
+ writel(FIELD_PREP(STF_DPHY_PRECOUNTER_IN_CLK1, 8) |
+ FIELD_PREP(STF_DPHY_PRECOUNTER_IN_LAN0, 7) |
+ FIELD_PREP(STF_DPHY_PRECOUNTER_IN_LAN1, 7) |
+ FIELD_PREP(STF_DPHY_PRECOUNTER_IN_LAN2, 7),
+ dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(196));
+
+ writel(FIELD_PREP(STF_DPHY_PRECOUNTER_IN_LAN3, 7) |
+ FIELD_PREP(STF_DPHY_RX_1C2C_SEL, 0),
+ dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(200));
+
+ return 0;
+}
+
+static int stf_dphy_init(struct phy *phy)
+{
+ struct stf_dphy *dphy = phy_get_drvdata(phy);
+ int ret;
+
+ ret = regulator_enable(dphy->mipi_0p9);
+ if (ret)
+ goto err_0p9;
+
+ if (stf_dphy_info->external_support && stf_dphy_info->external_init)
+ stf_dphy_info->external_init(dphy);
+
+ return 0;
+
+err_0p9:
+ return ret;
+}
+
+static int stf_dphy_exit(struct phy *phy)
+{
+ struct stf_dphy *dphy = phy_get_drvdata(phy);
+
+ if (stf_dphy_info->external_support && stf_dphy_info->external_exit)
+ stf_dphy_info->external_exit(dphy);
+
+ regulator_disable(dphy->mipi_0p9);
+
+ return 0;
+}
+
+static int stf_dphy_power_on(struct phy *phy)
+{
+ struct stf_dphy *dphy = phy_get_drvdata(phy);
+
+ clk_set_rate(dphy->cfg_clk, 99000000);
+ clk_set_rate(dphy->ref_clk, 49500000);
+ clk_set_rate(dphy->tx_clk, 19800000);
+ reset_control_deassert(dphy->rstc);
+
+ return 0;
+}
+
+static int stf_dphy_power_off(struct phy *phy)
+{
+ struct stf_dphy *dphy = phy_get_drvdata(phy);
+
+ reset_control_assert(dphy->rstc);
+
+ return 0;
+}
+
+static const struct phy_ops stf_dphy_ops = {
+ .init = stf_dphy_init,
+ .exit = stf_dphy_exit,
+ .configure = stf_dphy_configure,
+ .power_on = stf_dphy_power_on,
+ .power_off = stf_dphy_power_off,
+};
+
+static int stf_dphy_probe(struct platform_device *pdev)
+{
+ struct phy_provider *phy_provider;
+ struct stf_dphy *dphy;
+ int ret;
+
+ dphy = devm_kzalloc(&pdev->dev, sizeof(*dphy), GFP_KERNEL);
+ if (!dphy)
+ return -ENOMEM;
+ stf_dphy_info = of_device_get_match_data(&pdev->dev);
+ dev_set_drvdata(&pdev->dev, dphy);
+ dphy->dev = &pdev->dev;
+
+ dphy->regs = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(dphy->regs))
+ return PTR_ERR(dphy->regs);
+
+ dphy->cfg_clk = devm_clk_get(&pdev->dev, "cfg");
+ if (IS_ERR(dphy->cfg_clk))
+ return PTR_ERR(dphy->cfg_clk);
+
+ dphy->ref_clk = devm_clk_get(&pdev->dev, "ref");
+ if (IS_ERR(dphy->ref_clk))
+ return PTR_ERR(dphy->ref_clk);
+
+ dphy->tx_clk = devm_clk_get(&pdev->dev, "tx");
+ if (IS_ERR(dphy->tx_clk))
+ return PTR_ERR(dphy->tx_clk);
+
+ dphy->rstc = devm_reset_control_array_get_exclusive(&pdev->dev);
+ if (IS_ERR(dphy->rstc))
+ return PTR_ERR(dphy->rstc);
+
+ dphy->mipi_0p9 = devm_regulator_get(&pdev->dev, "mipi_0p9");
+ if (IS_ERR(dphy->mipi_0p9))
+ return PTR_ERR(dphy->mipi_0p9);
+
+ if (stf_dphy_info->external_support && stf_dphy_info->external_get) {
+ ret = stf_dphy_info->external_get(dphy);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to get PHY external info\n");
+ return ret;
+ }
+ }
+
+ dphy->phy = devm_phy_create(&pdev->dev, NULL, &stf_dphy_ops);
+ if (IS_ERR(dphy->phy)) {
+ dev_err(&pdev->dev, "Failed to create PHY\n");
+ return PTR_ERR(dphy->phy);
+ }
+
+ phy_set_drvdata(dphy->phy, dphy);
+ phy_provider = devm_of_phy_provider_register(&pdev->dev,
+ of_phy_simple_xlate);
+
+ return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static int stf_external_get(struct stf_dphy *dphy)
+{
+ struct of_phandle_args args;
+ int ret;
+
+ ret = of_parse_phandle_with_fixed_args(dphy->dev->of_node,
+ "starfive,aon-syscon",
+ 1, 0, &args);
+ if (ret < 0) {
+ dev_err(dphy->dev, "Failed to parse starfive,aon-syscon\n");
+ return -EINVAL;
+ }
+
+ dphy->stf_aon_syscon = syscon_node_to_regmap(args.np);
+ of_node_put(args.np);
+ if (IS_ERR(dphy->stf_aon_syscon))
+ return PTR_ERR(dphy->stf_aon_syscon);
+
+ dphy->aon_gp_reg = args.args[0];
+
+ return 0;
+}
+
+static void stf_external_init(struct stf_dphy *dphy)
+{
+ regmap_update_bits(dphy->stf_aon_syscon, dphy->aon_gp_reg,
+ BIT(31), BIT(31));
+}
+
+static void stf_external_exit(struct stf_dphy *dphy)
+{
+ regmap_update_bits(dphy->stf_aon_syscon, dphy->aon_gp_reg,
+ BIT(31), 0);
+}
+
+static const struct stf_dphy_info starfive_dphy_info = {
+ .external_support = true,
+ .external_get = stf_external_get,
+ .external_init = stf_external_init,
+ .external_exit = stf_external_exit,
+};
+
+static const struct of_device_id stf_dphy_dt_ids[] = {
+ {
+ .compatible = "starfive,jh7110-dphy-rx",
+ .data = &starfive_dphy_info,
+ },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, stf_dphy_dt_ids);
+
+static struct platform_driver stf_dphy_driver = {
+ .probe = stf_dphy_probe,
+ .driver = {
+ .name = "starfive-dphy-rx",
+ .of_match_table = stf_dphy_dt_ids,
+ },
+};
+module_platform_driver(stf_dphy_driver);
+
+MODULE_AUTHOR("Jack Zhu <[email protected]>");
+MODULE_AUTHOR("Changhuang Liang <[email protected]>");
+MODULE_DESCRIPTION("Starfive DPHY RX driver");
+MODULE_LICENSE("GPL");
--
2.25.1


2023-02-27 18:35:01

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: phy: Add starfive,jh7110-dphy-rx

On Wed, Feb 22, 2023 at 05:59:50PM -0800, Changhuang Liang wrote:
> Starfive SoCs like the jh7110 use a MIPI D-PHY RX controller based on
> a M31 IP. Add a binding for it.
>
> Signed-off-by: Changhuang Liang <[email protected]>
> ---
> .../bindings/phy/starfive,jh7110-dphy-rx.yaml | 74 +++++++++++++++++++
> 1 file changed, 74 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-rx.yaml
>
> diff --git a/Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-rx.yaml b/Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-rx.yaml
> new file mode 100644
> index 000000000000..a67ca57a6f21
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-rx.yaml
> @@ -0,0 +1,74 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/starfive,jh7110-dphy-rx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Starfive SoC MIPI D-PHY Rx Controller
> +
> +maintainers:
> + - Jack Zhu <[email protected]>
> + - Changhuang Liang <[email protected]>
> +
> +description:
> + The Starfive SoC uses the MIPI CSI D-PHY based on M31 IP to transfer
> + CSI camera data.
> +
> +properties:
> + compatible:
> + const: starfive,jh7110-dphy-rx
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 3
> +
> + clock-names:
> + items:
> + - const: cfg
> + - const: ref
> + - const: tx

Should be 'rx' given this is the 'rx' block? A description of each clock
in 'clocks' would be good.

> +
> + resets:
> + items:
> + - description: DPHY_HW reset
> + - description: DPHY_B09_ALWAYS_ON reset
> +
> + starfive,aon-syscon:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + items:
> + items:

- items: ?

Otherwise, multiple 2 cell entries are allowed. Is that intended?

> + - description: phandle of AON SYSCON
> + - description: register offset
> + description: The power of dphy rx is configured by AON SYSCON
> + in this property.


> +
> + "#phy-cells":
> + const: 0
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - resets
> + - starfive,aon-syscon
> + - "#phy-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + phy@19820000 {
> + compatible = "starfive,jh7110-dphy-rx";
> + reg = <0x19820000 0x10000>;
> + clocks = <&ispcrg 3>,
> + <&ispcrg 4>,
> + <&ispcrg 5>;
> + clock-names = "cfg", "ref", "tx";
> + resets = <&ispcrg 2>,
> + <&ispcrg 3>;
> + starfive,aon-syscon = <&aon_syscon 0x00>;
> + #phy-cells = <0>;
> + };
> --
> 2.25.1
>

2023-02-28 08:58:06

by Changhuang Liang

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: phy: Add starfive,jh7110-dphy-rx



On 2023/2/28 2:34, Rob Herring wrote:
> On Wed, Feb 22, 2023 at 05:59:50PM -0800, Changhuang Liang wrote:
[...]
>> +
>> + clocks:
>> + maxItems: 3
>> +
>> + clock-names:
>> + items:
>> + - const: cfg
>> + - const: ref
>> + - const: tx
>
> Should be 'rx' given this is the 'rx' block? A description of each clock
> in 'clocks' would be good.
>

'tx': This clock is directly used to generate transmit escape sequences,
will add description of each clock in 'clocks'.

>> +
>> + resets:
>> + items:
>> + - description: DPHY_HW reset
>> + - description: DPHY_B09_ALWAYS_ON reset
>> +
>> + starfive,aon-syscon:
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> + items:
>> + items:
>
> - items: ?
>
> Otherwise, multiple 2 cell entries are allowed. Is that intended?
>

Will change to:
items:
- items:

>> + - description: phandle of AON SYSCON
>> + - description: register offset
>> + description: The power of dphy rx is configured by AON SYSCON
>> + in this property.
>
>
>> +
>> + "#phy-cells":
>> + const: 0
>> +
>> +required:
>> + - compatible
>> + - reg
[...]
>> --
>> 2.25.1
>>

2023-03-03 06:05:18

by Changhuang Liang

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] Add JH7110 MIPI DPHY RX support



On 2023/2/23 9:59, Changhuang Liang wrote:
> This patchset adds mipi dphy rx driver for the StarFive JH7110 SoC.
> It is used to transfer CSI camera data. The series has been tested on
> the VisionFive 2 board.
>
> This patchset should be applied after the patchset [1] and patch [2]:
> [1] https://lore.kernel.org/all/[email protected]/
> [2] https://lore.kernel.org/all/[email protected]/
>
Hi, Vinod and Kishon

Could you please help to review and give me some suggestions
for this patch series? Thank you for your time.

Best regards,
Changhuang

2023-03-20 12:38:01

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] phy: starfive: Add mipi dphy rx support

On 22-02-23, 17:59, Changhuang Liang wrote:
> Add mipi dphy rx support for the Starfive JH7110 SoC. It is used to
> transfer CSI camera data.
>
> Signed-off-by: Changhuang Liang <[email protected]>
> ---
> MAINTAINERS | 7 +
> drivers/phy/Kconfig | 1 +
> drivers/phy/Makefile | 1 +
> drivers/phy/starfive/Kconfig | 13 +
> drivers/phy/starfive/Makefile | 2 +
> drivers/phy/starfive/phy-starfive-dphy-rx.c | 362 ++++++++++++++++++++
> 6 files changed, 386 insertions(+)
> create mode 100644 drivers/phy/starfive/Kconfig
> create mode 100644 drivers/phy/starfive/Makefile
> create mode 100644 drivers/phy/starfive/phy-starfive-dphy-rx.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2e70c9f21989..8ddef8669efb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19952,6 +19952,13 @@ F: Documentation/devicetree/bindings/power/starfive*
> F: drivers/soc/starfive/jh71xx_pmu.c
> F: include/dt-bindings/power/starfive,jh7110-pmu.h
>
> +STARFIVE JH7110 DPHY RX DRIVER
> +M: Jack Zhu <[email protected]>
> +M: Changhuang Liang <[email protected]>
> +S: Supported
> +F: Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-rx.yaml
> +F: drivers/phy/starfive/phy-starfive-dphy-rx.c
> +
> STATIC BRANCH/CALL
> M: Peter Zijlstra <[email protected]>
> M: Josh Poimboeuf <[email protected]>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 7bd00a11d074..c4b2a86e2afb 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -91,6 +91,7 @@ source "drivers/phy/rockchip/Kconfig"
> source "drivers/phy/samsung/Kconfig"
> source "drivers/phy/socionext/Kconfig"
> source "drivers/phy/st/Kconfig"
> +source "drivers/phy/starfive/Kconfig"
> source "drivers/phy/sunplus/Kconfig"
> source "drivers/phy/tegra/Kconfig"
> source "drivers/phy/ti/Kconfig"
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 54f312c10a40..fb3dc9de6111 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -31,6 +31,7 @@ obj-y += allwinner/ \
> samsung/ \
> socionext/ \
> st/ \
> + starfive/ \
> sunplus/ \
> tegra/ \
> ti/ \
> diff --git a/drivers/phy/starfive/Kconfig b/drivers/phy/starfive/Kconfig
> new file mode 100644
> index 000000000000..e449a662acf5
> --- /dev/null
> +++ b/drivers/phy/starfive/Kconfig
> @@ -0,0 +1,13 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Phy drivers for Starfive platforms
> +#
> +
> +config PHY_STARFIVE_DPHY_RX
> + tristate "Starfive D-PHY RX Support"
> + select GENERIC_PHY
> + select GENERIC_PHY_MIPI_DPHY
> + help
> + Choose this option if you have a Starfive D-PHY in your
> + system. If M is selected, the module will be called
> + phy-starfive-dphy-rx.
> diff --git a/drivers/phy/starfive/Makefile b/drivers/phy/starfive/Makefile
> new file mode 100644
> index 000000000000..7ec576cb30ae
> --- /dev/null
> +++ b/drivers/phy/starfive/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_PHY_STARFIVE_DPHY_RX) += phy-starfive-dphy-rx.o
> diff --git a/drivers/phy/starfive/phy-starfive-dphy-rx.c b/drivers/phy/starfive/phy-starfive-dphy-rx.c
> new file mode 100644
> index 000000000000..2a7c5453f4ce
> --- /dev/null
> +++ b/drivers/phy/starfive/phy-starfive-dphy-rx.c
> @@ -0,0 +1,362 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * DPHY driver for the StarFive JH7110 SoC
> + *
> + * Copyright (C) 2023 StarFive Technology Co., Ltd.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +
> +#define STF_DPHY_APBCFGSAIF__SYSCFG(x) (x)

What is the purpose of this? also whats with __ ?

> +
> +#define STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_EN BIT(6)
> +#define STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_IN GENMASK(12, 7)
> +#define STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_EN BIT(19)
> +#define STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_IN GENMASK(25, 20)
> +
> +#define STF_DPHY_DATA_BUS16_8 BIT(8)
> +#define STF_DPHY_DEBUG_MODE_SEL GENMASK(15, 9)
> +
> +#define STF_DPHY_ENABLE_CLK BIT(6)
> +#define STF_DPHY_ENABLE_CLK1 BIT(7)
> +#define STF_DPHY_ENABLE_LAN0 BIT(8)
> +#define STF_DPHY_ENABLE_LAN1 BIT(9)
> +#define STF_DPHY_ENABLE_LAN2 BIT(10)
> +#define STF_DPHY_ENABLE_LAN3 BIT(11)
> +#define STF_DPHY_GPI_EN GENMASK(17, 12)
> +#define STF_DPHY_HS_FREQ_CHANGE_CLK BIT(18)
> +#define STF_DPHY_HS_FREQ_CHANGE_CLK1 BIT(19)
> +#define STF_DPHY_LANE_SWAP_CLK GENMASK(22, 20)
> +#define STF_DPHY_LANE_SWAP_CLK1 GENMASK(25, 23)
> +#define STF_DPHY_LANE_SWAP_LAN0 GENMASK(28, 26)
> +#define STF_DPHY_LANE_SWAP_LAN1 GENMASK(31, 29)
> +
> +#define STF_DPHY_LANE_SWAP_LAN2 GENMASK(2, 0)
> +#define STF_DPHY_LANE_SWAP_LAN3 GENMASK(5, 3)
> +#define STF_DPHY_MP_TEST_EN BIT(6)
> +#define STF_DPHY_MP_TEST_MODE_SEL GENMASK(11, 7)
> +#define STF_DPHY_PLL_CLK_SEL GENMASK(21, 12)
> +#define STF_DPHY_PRECOUNTER_IN_CLK GENMASK(29, 22)
> +
> +#define STF_DPHY_PRECOUNTER_IN_CLK1 GENMASK(7, 0)
> +#define STF_DPHY_PRECOUNTER_IN_LAN0 GENMASK(15, 8)
> +#define STF_DPHY_PRECOUNTER_IN_LAN1 GENMASK(23, 16)
> +#define STF_DPHY_PRECOUNTER_IN_LAN2 GENMASK(31, 24)
> +
> +#define STF_DPHY_PRECOUNTER_IN_LAN3 GENMASK(7, 0)
> +#define STF_DPHY_RX_1C2C_SEL BIT(8)
> +
> +struct regval_t {

regval should be okay, no?

> + u32 addr;
> + u32 val;
> +};
> +
> +struct stf_dphy {
> + struct device *dev;
> + void __iomem *regs;
> + struct clk *cfg_clk;
> + struct clk *ref_clk;
> + struct clk *tx_clk;
> + struct reset_control *rstc;
> + struct regulator *mipi_0p9;
> + struct phy *phy;
> + struct regmap *stf_aon_syscon;
> + unsigned int aon_gp_reg;
> +};
> +
> +struct stf_dphy_info {
> + bool external_support;
> + int (*external_get)(struct stf_dphy *dphy);
> + void (*external_init)(struct stf_dphy *dphy);
> + void (*external_exit)(struct stf_dphy *dphy);
> +};
> +
> +static const struct stf_dphy_info *stf_dphy_info;
> +
> +static const struct regval_t stf_dphy_init_list[] = {
> + { STF_DPHY_APBCFGSAIF__SYSCFG(4), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(8), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(12), 0x0000fff0 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(16), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(20), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(24), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(28), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(32), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(36), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(40), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(40), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(48), 0x24000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(52), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(56), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(60), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(64), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(68), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(72), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(76), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(80), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(84), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(88), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(92), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(96), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(100), 0x02000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(104), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(108), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(112), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(116), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(120), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(124), 0x0000000c },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(128), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(132), 0xcc500000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(136), 0x000000cc },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(140), 0x00000000 },
> + { STF_DPHY_APBCFGSAIF__SYSCFG(144), 0x00000000 },
> +};
> +
> +static int stf_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> +{
> + struct stf_dphy *dphy = phy_get_drvdata(phy);
> + int map[6] = {4, 0, 1, 2, 3, 5};

what does this mean?

> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(stf_dphy_init_list); i++)
> + writel(stf_dphy_init_list[i].val,
> + dphy->regs + stf_dphy_init_list[i].addr);
> +
> + writel(FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_EN, 1) |
> + FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_IN, 0x1b) |
> + FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_EN, 1) |
> + FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_IN, 0x1b),
> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(0));
> +
> + writel(FIELD_PREP(STF_DPHY_DATA_BUS16_8, 0) |
> + FIELD_PREP(STF_DPHY_DEBUG_MODE_SEL, 0x5a),
> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(184));
> +
> + writel(FIELD_PREP(STF_DPHY_ENABLE_CLK, 1) |
> + FIELD_PREP(STF_DPHY_ENABLE_CLK1, 1) |
> + FIELD_PREP(STF_DPHY_ENABLE_LAN0, 1) |
> + FIELD_PREP(STF_DPHY_ENABLE_LAN1, 1) |
> + FIELD_PREP(STF_DPHY_ENABLE_LAN2, 1) |
> + FIELD_PREP(STF_DPHY_ENABLE_LAN3, 1) |
> + FIELD_PREP(STF_DPHY_GPI_EN, 0) |
> + FIELD_PREP(STF_DPHY_HS_FREQ_CHANGE_CLK, 0) |
> + FIELD_PREP(STF_DPHY_HS_FREQ_CHANGE_CLK1, 0) |
> + FIELD_PREP(STF_DPHY_LANE_SWAP_CLK, map[0]) |
> + FIELD_PREP(STF_DPHY_LANE_SWAP_CLK1, map[5]) |
> + FIELD_PREP(STF_DPHY_LANE_SWAP_LAN0, map[1]) |
> + FIELD_PREP(STF_DPHY_LANE_SWAP_LAN1, map[2]),
> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(188));
> +
> + writel(FIELD_PREP(STF_DPHY_LANE_SWAP_LAN2, map[3]) |
> + FIELD_PREP(STF_DPHY_LANE_SWAP_LAN3, map[4]) |
> + FIELD_PREP(STF_DPHY_MP_TEST_EN, 0) |
> + FIELD_PREP(STF_DPHY_MP_TEST_MODE_SEL, 0) |
> + FIELD_PREP(STF_DPHY_PLL_CLK_SEL, 0x37c) |
> + FIELD_PREP(STF_DPHY_PRECOUNTER_IN_CLK, 8),
> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(192));
> +
> + writel(FIELD_PREP(STF_DPHY_PRECOUNTER_IN_CLK1, 8) |
> + FIELD_PREP(STF_DPHY_PRECOUNTER_IN_LAN0, 7) |
> + FIELD_PREP(STF_DPHY_PRECOUNTER_IN_LAN1, 7) |
> + FIELD_PREP(STF_DPHY_PRECOUNTER_IN_LAN2, 7),
> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(196));
> +
> + writel(FIELD_PREP(STF_DPHY_PRECOUNTER_IN_LAN3, 7) |
> + FIELD_PREP(STF_DPHY_RX_1C2C_SEL, 0),
> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(200));
> +
> + return 0;
> +}
> +
> +static int stf_dphy_init(struct phy *phy)
> +{
> + struct stf_dphy *dphy = phy_get_drvdata(phy);
> + int ret;
> +
> + ret = regulator_enable(dphy->mipi_0p9);
> + if (ret)
> + goto err_0p9;
> +
> + if (stf_dphy_info->external_support && stf_dphy_info->external_init)
> + stf_dphy_info->external_init(dphy);
> +
> + return 0;
> +
> +err_0p9:
> + return ret;
> +}
> +
> +static int stf_dphy_exit(struct phy *phy)
> +{
> + struct stf_dphy *dphy = phy_get_drvdata(phy);
> +
> + if (stf_dphy_info->external_support && stf_dphy_info->external_exit)
> + stf_dphy_info->external_exit(dphy);
> +
> + regulator_disable(dphy->mipi_0p9);
> +
> + return 0;
> +}
> +
> +static int stf_dphy_power_on(struct phy *phy)
> +{
> + struct stf_dphy *dphy = phy_get_drvdata(phy);
> +
> + clk_set_rate(dphy->cfg_clk, 99000000);
> + clk_set_rate(dphy->ref_clk, 49500000);
> + clk_set_rate(dphy->tx_clk, 19800000);
> + reset_control_deassert(dphy->rstc);
> +
> + return 0;
> +}
> +
> +static int stf_dphy_power_off(struct phy *phy)
> +{
> + struct stf_dphy *dphy = phy_get_drvdata(phy);
> +
> + reset_control_assert(dphy->rstc);
> +
> + return 0;
> +}
> +
> +static const struct phy_ops stf_dphy_ops = {
> + .init = stf_dphy_init,
> + .exit = stf_dphy_exit,
> + .configure = stf_dphy_configure,
> + .power_on = stf_dphy_power_on,
> + .power_off = stf_dphy_power_off,
> +};
> +
> +static int stf_dphy_probe(struct platform_device *pdev)
> +{
> + struct phy_provider *phy_provider;
> + struct stf_dphy *dphy;
> + int ret;
> +
> + dphy = devm_kzalloc(&pdev->dev, sizeof(*dphy), GFP_KERNEL);
> + if (!dphy)
> + return -ENOMEM;
> + stf_dphy_info = of_device_get_match_data(&pdev->dev);
> + dev_set_drvdata(&pdev->dev, dphy);
> + dphy->dev = &pdev->dev;
> +
> + dphy->regs = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(dphy->regs))
> + return PTR_ERR(dphy->regs);
> +
> + dphy->cfg_clk = devm_clk_get(&pdev->dev, "cfg");
> + if (IS_ERR(dphy->cfg_clk))
> + return PTR_ERR(dphy->cfg_clk);
> +
> + dphy->ref_clk = devm_clk_get(&pdev->dev, "ref");
> + if (IS_ERR(dphy->ref_clk))
> + return PTR_ERR(dphy->ref_clk);
> +
> + dphy->tx_clk = devm_clk_get(&pdev->dev, "tx");
> + if (IS_ERR(dphy->tx_clk))
> + return PTR_ERR(dphy->tx_clk);
> +
> + dphy->rstc = devm_reset_control_array_get_exclusive(&pdev->dev);
> + if (IS_ERR(dphy->rstc))
> + return PTR_ERR(dphy->rstc);
> +
> + dphy->mipi_0p9 = devm_regulator_get(&pdev->dev, "mipi_0p9");
> + if (IS_ERR(dphy->mipi_0p9))
> + return PTR_ERR(dphy->mipi_0p9);
> +
> + if (stf_dphy_info->external_support && stf_dphy_info->external_get) {
> + ret = stf_dphy_info->external_get(dphy);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Failed to get PHY external info\n");
> + return ret;
> + }
> + }
> +
> + dphy->phy = devm_phy_create(&pdev->dev, NULL, &stf_dphy_ops);
> + if (IS_ERR(dphy->phy)) {
> + dev_err(&pdev->dev, "Failed to create PHY\n");
> + return PTR_ERR(dphy->phy);
> + }
> +
> + phy_set_drvdata(dphy->phy, dphy);
> + phy_provider = devm_of_phy_provider_register(&pdev->dev,
> + of_phy_simple_xlate);
> +
> + return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static int stf_external_get(struct stf_dphy *dphy)
> +{
> + struct of_phandle_args args;
> + int ret;
> +
> + ret = of_parse_phandle_with_fixed_args(dphy->dev->of_node,
> + "starfive,aon-syscon",
> + 1, 0, &args);
> + if (ret < 0) {
> + dev_err(dphy->dev, "Failed to parse starfive,aon-syscon\n");
> + return -EINVAL;
> + }
> +
> + dphy->stf_aon_syscon = syscon_node_to_regmap(args.np);
> + of_node_put(args.np);
> + if (IS_ERR(dphy->stf_aon_syscon))
> + return PTR_ERR(dphy->stf_aon_syscon);
> +
> + dphy->aon_gp_reg = args.args[0];
> +
> + return 0;
> +}
> +
> +static void stf_external_init(struct stf_dphy *dphy)
> +{
> + regmap_update_bits(dphy->stf_aon_syscon, dphy->aon_gp_reg,
> + BIT(31), BIT(31));
> +}
> +
> +static void stf_external_exit(struct stf_dphy *dphy)
> +{
> + regmap_update_bits(dphy->stf_aon_syscon, dphy->aon_gp_reg,
> + BIT(31), 0);
> +}
> +
> +static const struct stf_dphy_info starfive_dphy_info = {
> + .external_support = true,
> + .external_get = stf_external_get,
> + .external_init = stf_external_init,
> + .external_exit = stf_external_exit,
> +};
> +
> +static const struct of_device_id stf_dphy_dt_ids[] = {
> + {
> + .compatible = "starfive,jh7110-dphy-rx",
> + .data = &starfive_dphy_info,

is there a plan to support multiple versions which need different
get/init/exit methods?

> + },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, stf_dphy_dt_ids);
> +
> +static struct platform_driver stf_dphy_driver = {
> + .probe = stf_dphy_probe,
> + .driver = {
> + .name = "starfive-dphy-rx",
> + .of_match_table = stf_dphy_dt_ids,
> + },
> +};
> +module_platform_driver(stf_dphy_driver);
> +
> +MODULE_AUTHOR("Jack Zhu <[email protected]>");
> +MODULE_AUTHOR("Changhuang Liang <[email protected]>");
> +MODULE_DESCRIPTION("Starfive DPHY RX driver");
> +MODULE_LICENSE("GPL");
> --
> 2.25.1

--
~Vinod

2023-03-20 12:39:24

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] Add JH7110 MIPI DPHY RX support

On 03-03-23, 14:05, Changhuang Liang wrote:
>
>
> On 2023/2/23 9:59, Changhuang Liang wrote:
> > This patchset adds mipi dphy rx driver for the StarFive JH7110 SoC.
> > It is used to transfer CSI camera data. The series has been tested on
> > the VisionFive 2 board.
> >
> > This patchset should be applied after the patchset [1] and patch [2]:
> > [1] https://lore.kernel.org/all/[email protected]/
> > [2] https://lore.kernel.org/all/[email protected]/
> >
> Hi, Vinod and Kishon
>
> Could you please help to review and give me some suggestions
> for this patch series? Thank you for your time.

There are already comments on dt-binding, please address them!

>
> Best regards,
> Changhuang

--
~Vinod

2023-03-21 02:10:08

by Changhuang Liang

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] Add JH7110 MIPI DPHY RX support



On 2023/3/20 20:39, Vinod Koul wrote:
> On 03-03-23, 14:05, Changhuang Liang wrote:
>>
>>
>> On 2023/2/23 9:59, Changhuang Liang wrote:
>>> This patchset adds mipi dphy rx driver for the StarFive JH7110 SoC.
>>> It is used to transfer CSI camera data. The series has been tested on
>>> the VisionFive 2 board.
>>>
>>> This patchset should be applied after the patchset [1] and patch [2]:
>>> [1] https://lore.kernel.org/all/[email protected]/
>>> [2] https://lore.kernel.org/all/[email protected]/
>>>
>> Hi, Vinod and Kishon
>>
>> Could you please help to review and give me some suggestions
>> for this patch series? Thank you for your time.
>
> There are already comments on dt-binding, please address them!
>

I have address them on v3, and I am really looking forward to your comments.
Thank you so much for wasting your time.

>>
>> Best regards,
>> Changhuang
>

2023-03-21 06:09:08

by Changhuang Liang

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] phy: starfive: Add mipi dphy rx support



On 2023/3/20 20:37, Vinod Koul wrote:
> On 22-02-23, 17:59, Changhuang Liang wrote:
>> [...]
>> +++ b/drivers/phy/starfive/phy-starfive-dphy-rx.c
>> @@ -0,0 +1,362 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * DPHY driver for the StarFive JH7110 SoC
>> + *
>> + * Copyright (C) 2023 StarFive Technology Co., Ltd.
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/bitops.h>
>> +#include <linux/clk.h>
>> +#include <linux/io.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_device.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/reset.h>
>> +
>> +#define STF_DPHY_APBCFGSAIF__SYSCFG(x) (x)
>
> What is the purpose of this? also whats with __ ?
>

In starfive jh7110 SoC, Dphy rx module register's name is called such as
STF_DPHY_APBCFGSAIF__SYSCFG_4, STF_DPHY_APBCFGSAIF__SYSCFG_8, STF_DPHY_APBCFGSAIF__SYSCFG_12...
so I merge them to use a marco STF_DPHY_APBCFGSAIF__SYSCFG(x).

Should I use one "_" is enoughed?

>> +
>> +#define STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_EN BIT(6)
>> +#define STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_IN GENMASK(12, 7)
>> +#define STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_EN BIT(19)
>> +#define STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_IN GENMASK(25, 20)
>> +
>> +#define STF_DPHY_DATA_BUS16_8 BIT(8)
>> +#define STF_DPHY_DEBUG_MODE_SEL GENMASK(15, 9)
>> +
>> +#define STF_DPHY_ENABLE_CLK BIT(6)
>> +#define STF_DPHY_ENABLE_CLK1 BIT(7)
>> +#define STF_DPHY_ENABLE_LAN0 BIT(8)
>> +#define STF_DPHY_ENABLE_LAN1 BIT(9)
>> +#define STF_DPHY_ENABLE_LAN2 BIT(10)
>> +#define STF_DPHY_ENABLE_LAN3 BIT(11)
>> +#define STF_DPHY_GPI_EN GENMASK(17, 12)
>> +#define STF_DPHY_HS_FREQ_CHANGE_CLK BIT(18)
>> +#define STF_DPHY_HS_FREQ_CHANGE_CLK1 BIT(19)
>> +#define STF_DPHY_LANE_SWAP_CLK GENMASK(22, 20)
>> +#define STF_DPHY_LANE_SWAP_CLK1 GENMASK(25, 23)
>> +#define STF_DPHY_LANE_SWAP_LAN0 GENMASK(28, 26)
>> +#define STF_DPHY_LANE_SWAP_LAN1 GENMASK(31, 29)
>> +
>> +#define STF_DPHY_LANE_SWAP_LAN2 GENMASK(2, 0)
>> +#define STF_DPHY_LANE_SWAP_LAN3 GENMASK(5, 3)
>> +#define STF_DPHY_MP_TEST_EN BIT(6)
>> +#define STF_DPHY_MP_TEST_MODE_SEL GENMASK(11, 7)
>> +#define STF_DPHY_PLL_CLK_SEL GENMASK(21, 12)
>> +#define STF_DPHY_PRECOUNTER_IN_CLK GENMASK(29, 22)
>> +
>> +#define STF_DPHY_PRECOUNTER_IN_CLK1 GENMASK(7, 0)
>> +#define STF_DPHY_PRECOUNTER_IN_LAN0 GENMASK(15, 8)
>> +#define STF_DPHY_PRECOUNTER_IN_LAN1 GENMASK(23, 16)
>> +#define STF_DPHY_PRECOUNTER_IN_LAN2 GENMASK(31, 24)
>> +
>> +#define STF_DPHY_PRECOUNTER_IN_LAN3 GENMASK(7, 0)
>> +#define STF_DPHY_RX_1C2C_SEL BIT(8)
>> +
>> +struct regval_t {
>
> regval should be okay, no?
>

OK, will change to regval

>> + u32 addr;
>> + u32 val;
>> +};
>> +
>> +struct stf_dphy {
>> + struct device *dev;
>> + void __iomem *regs;
>> + struct clk *cfg_clk;
>> + struct clk *ref_clk;
>> + struct clk *tx_clk;
>> + struct reset_control *rstc;
>> + struct regulator *mipi_0p9;
>> + struct phy *phy;
>> + struct regmap *stf_aon_syscon;
>> + unsigned int aon_gp_reg;
>> +};
>> +
>> +struct stf_dphy_info {
>> + bool external_support;
>> + int (*external_get)(struct stf_dphy *dphy);
>> + void (*external_init)(struct stf_dphy *dphy);
>> + void (*external_exit)(struct stf_dphy *dphy);
>> +};
>> +
>> +static const struct stf_dphy_info *stf_dphy_info;
>> +
>> +static const struct regval_t stf_dphy_init_list[] = {
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(4), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(8), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(12), 0x0000fff0 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(16), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(20), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(24), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(28), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(32), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(36), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(40), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(40), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(48), 0x24000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(52), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(56), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(60), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(64), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(68), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(72), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(76), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(80), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(84), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(88), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(92), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(96), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(100), 0x02000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(104), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(108), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(112), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(116), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(120), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(124), 0x0000000c },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(128), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(132), 0xcc500000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(136), 0x000000cc },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(140), 0x00000000 },
>> + { STF_DPHY_APBCFGSAIF__SYSCFG(144), 0x00000000 },
>> +};
>> +
>> +static int stf_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
>> +{
>> + struct stf_dphy *dphy = phy_get_drvdata(phy);
>> + int map[6] = {4, 0, 1, 2, 3, 5};
>
> what does this mean?
>

This is the physical lane and logical lane mapping table, should I add a note for it?

>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(stf_dphy_init_list); i++)
>> + writel(stf_dphy_init_list[i].val,
>> + dphy->regs + stf_dphy_init_list[i].addr);
>> +
>> + writel(FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_EN, 1) |
>> + FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_IN, 0x1b) |
>> + FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_EN, 1) |
>> + FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_IN, 0x1b),
>> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(0));
>> +
>> + writel(FIELD_PREP(STF_DPHY_DATA_BUS16_8, 0) |
>> + FIELD_PREP(STF_DPHY_DEBUG_MODE_SEL, 0x5a),
>> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(184));
>> +
>> + writel(FIELD_PREP(STF_DPHY_ENABLE_CLK, 1) |
>> + FIELD_PREP(STF_DPHY_ENABLE_CLK1, 1) |
>> + FIELD_PREP(STF_DPHY_ENABLE_LAN0, 1) |
>> + FIELD_PREP(STF_DPHY_ENABLE_LAN1, 1) |
>> + FIELD_PREP(STF_DPHY_ENABLE_LAN2, 1) |
>> + FIELD_PREP(STF_DPHY_ENABLE_LAN3, 1) |
>> + FIELD_PREP(STF_DPHY_GPI_EN, 0) |
>> + FIELD_PREP(STF_DPHY_HS_FREQ_CHANGE_CLK, 0) |
>> + FIELD_PREP(STF_DPHY_HS_FREQ_CHANGE_CLK1, 0) |
>> + FIELD_PREP(STF_DPHY_LANE_SWAP_CLK, map[0]) |
>> + FIELD_PREP(STF_DPHY_LANE_SWAP_CLK1, map[5]) |
>> + FIELD_PREP(STF_DPHY_LANE_SWAP_LAN0, map[1]) |
>> + FIELD_PREP(STF_DPHY_LANE_SWAP_LAN1, map[2]),
>> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(188));
>> +
>> + writel(FIELD_PREP(STF_DPHY_LANE_SWAP_LAN2, map[3]) |
>> + FIELD_PREP(STF_DPHY_LANE_SWAP_LAN3, map[4]) |
>> + FIELD_PREP(STF_DPHY_MP_TEST_EN, 0) |
>> + FIELD_PREP(STF_DPHY_MP_TEST_MODE_SEL, 0) |
>> + FIELD_PREP(STF_DPHY_PLL_CLK_SEL, 0x37c) |
>> + FIELD_PREP(STF_DPHY_PRECOUNTER_IN_CLK, 8),
>> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(192));
>> +
>> + writel(FIELD_PREP(STF_DPHY_PRECOUNTER_IN_CLK1, 8) |
>> + FIELD_PREP(STF_DPHY_PRECOUNTER_IN_LAN0, 7) |
>> + FIELD_PREP(STF_DPHY_PRECOUNTER_IN_LAN1, 7) |
>> + FIELD_PREP(STF_DPHY_PRECOUNTER_IN_LAN2, 7),
>> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(196));
>> +
>> + writel(FIELD_PREP(STF_DPHY_PRECOUNTER_IN_LAN3, 7) |
>> + FIELD_PREP(STF_DPHY_RX_1C2C_SEL, 0),
>> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(200));
>> +
>> + return 0;
>> +}
>> +
>> +static int stf_dphy_init(struct phy *phy)
>> +{
>> + struct stf_dphy *dphy = phy_get_drvdata(phy);
>> + int ret;
>> +
>> + ret = regulator_enable(dphy->mipi_0p9);
>> + if (ret)
>> + goto err_0p9;
>> +
>> + if (stf_dphy_info->external_support && stf_dphy_info->external_init)
>> + stf_dphy_info->external_init(dphy);
>> +
>> + return 0;
>> +
>> +err_0p9:
>> + return ret;
>> +}
>> +
>> +static int stf_dphy_exit(struct phy *phy)
>> +{
>> + struct stf_dphy *dphy = phy_get_drvdata(phy);
>> +
>> + if (stf_dphy_info->external_support && stf_dphy_info->external_exit)
>> + stf_dphy_info->external_exit(dphy);
>> +
>> + regulator_disable(dphy->mipi_0p9);
>> +
>> + return 0;
>> +}
>> +
>> +static int stf_dphy_power_on(struct phy *phy)
>> +{
>> + struct stf_dphy *dphy = phy_get_drvdata(phy);
>> +
>> + clk_set_rate(dphy->cfg_clk, 99000000);
>> + clk_set_rate(dphy->ref_clk, 49500000);
>> + clk_set_rate(dphy->tx_clk, 19800000);
>> + reset_control_deassert(dphy->rstc);
>> +
>> + return 0;
>> +}
>> +
>> +static int stf_dphy_power_off(struct phy *phy)
>> +{
>> + struct stf_dphy *dphy = phy_get_drvdata(phy);
>> +
>> + reset_control_assert(dphy->rstc);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct phy_ops stf_dphy_ops = {
>> + .init = stf_dphy_init,
>> + .exit = stf_dphy_exit,
>> + .configure = stf_dphy_configure,
>> + .power_on = stf_dphy_power_on,
>> + .power_off = stf_dphy_power_off,
>> +};
>> +
>> +static int stf_dphy_probe(struct platform_device *pdev)
>> +{
>> + struct phy_provider *phy_provider;
>> + struct stf_dphy *dphy;
>> + int ret;
>> +
>> + dphy = devm_kzalloc(&pdev->dev, sizeof(*dphy), GFP_KERNEL);
>> + if (!dphy)
>> + return -ENOMEM;
>> + stf_dphy_info = of_device_get_match_data(&pdev->dev);
>> + dev_set_drvdata(&pdev->dev, dphy);
>> + dphy->dev = &pdev->dev;
>> +
>> + dphy->regs = devm_platform_ioremap_resource(pdev, 0);
>> + if (IS_ERR(dphy->regs))
>> + return PTR_ERR(dphy->regs);
>> +
>> + dphy->cfg_clk = devm_clk_get(&pdev->dev, "cfg");
>> + if (IS_ERR(dphy->cfg_clk))
>> + return PTR_ERR(dphy->cfg_clk);
>> +
>> + dphy->ref_clk = devm_clk_get(&pdev->dev, "ref");
>> + if (IS_ERR(dphy->ref_clk))
>> + return PTR_ERR(dphy->ref_clk);
>> +
>> + dphy->tx_clk = devm_clk_get(&pdev->dev, "tx");
>> + if (IS_ERR(dphy->tx_clk))
>> + return PTR_ERR(dphy->tx_clk);
>> +
>> + dphy->rstc = devm_reset_control_array_get_exclusive(&pdev->dev);
>> + if (IS_ERR(dphy->rstc))
>> + return PTR_ERR(dphy->rstc);
>> +
>> + dphy->mipi_0p9 = devm_regulator_get(&pdev->dev, "mipi_0p9");
>> + if (IS_ERR(dphy->mipi_0p9))
>> + return PTR_ERR(dphy->mipi_0p9);
>> +
>> + if (stf_dphy_info->external_support && stf_dphy_info->external_get) {
>> + ret = stf_dphy_info->external_get(dphy);
>> + if (ret < 0) {
>> + dev_err(&pdev->dev, "Failed to get PHY external info\n");
>> + return ret;
>> + }
>> + }
>> +
>> + dphy->phy = devm_phy_create(&pdev->dev, NULL, &stf_dphy_ops);
>> + if (IS_ERR(dphy->phy)) {
>> + dev_err(&pdev->dev, "Failed to create PHY\n");
>> + return PTR_ERR(dphy->phy);
>> + }
>> +
>> + phy_set_drvdata(dphy->phy, dphy);
>> + phy_provider = devm_of_phy_provider_register(&pdev->dev,
>> + of_phy_simple_xlate);
>> +
>> + return PTR_ERR_OR_ZERO(phy_provider);
>> +}
>> +
>> +static int stf_external_get(struct stf_dphy *dphy)
>> +{
>> + struct of_phandle_args args;
>> + int ret;
>> +
>> + ret = of_parse_phandle_with_fixed_args(dphy->dev->of_node,
>> + "starfive,aon-syscon",
>> + 1, 0, &args);
>> + if (ret < 0) {
>> + dev_err(dphy->dev, "Failed to parse starfive,aon-syscon\n");
>> + return -EINVAL;
>> + }
>> +
>> + dphy->stf_aon_syscon = syscon_node_to_regmap(args.np);
>> + of_node_put(args.np);
>> + if (IS_ERR(dphy->stf_aon_syscon))
>> + return PTR_ERR(dphy->stf_aon_syscon);
>> +
>> + dphy->aon_gp_reg = args.args[0];
>> +
>> + return 0;
>> +}
>> +
>> +static void stf_external_init(struct stf_dphy *dphy)
>> +{
>> + regmap_update_bits(dphy->stf_aon_syscon, dphy->aon_gp_reg,
>> + BIT(31), BIT(31));
>> +}
>> +
>> +static void stf_external_exit(struct stf_dphy *dphy)
>> +{
>> + regmap_update_bits(dphy->stf_aon_syscon, dphy->aon_gp_reg,
>> + BIT(31), 0);
>> +}
>> +
>> +static const struct stf_dphy_info starfive_dphy_info = {
>> + .external_support = true,
>> + .external_get = stf_external_get,
>> + .external_init = stf_external_init,
>> + .external_exit = stf_external_exit,
>> +};
>> +
>> +static const struct of_device_id stf_dphy_dt_ids[] = {
>> + {
>> + .compatible = "starfive,jh7110-dphy-rx",
>> + .data = &starfive_dphy_info,
>
> is there a plan to support multiple versions which need different
> get/init/exit methods?
>

There is no plan to support multiple versions. So this different methods
interface can cancel?

>> + },
>> + { /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, stf_dphy_dt_ids);
>> +
>> +static struct platform_driver stf_dphy_driver = {
>> + .probe = stf_dphy_probe,
>> + .driver = {
>> + .name = "starfive-dphy-rx",
>> + .of_match_table = stf_dphy_dt_ids,
>> + },
>> +};
>> +module_platform_driver(stf_dphy_driver);
>> +
>> +MODULE_AUTHOR("Jack Zhu <[email protected]>");
>> +MODULE_AUTHOR("Changhuang Liang <[email protected]>");
>> +MODULE_DESCRIPTION("Starfive DPHY RX driver");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.25.1
>

2023-03-31 13:56:26

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] phy: starfive: Add mipi dphy rx support

On 21-03-23, 14:08, Changhuang Liang wrote:
>
>
> On 2023/3/20 20:37, Vinod Koul wrote:
> > On 22-02-23, 17:59, Changhuang Liang wrote:
> >> [...]
> >> +++ b/drivers/phy/starfive/phy-starfive-dphy-rx.c
> >> @@ -0,0 +1,362 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +/*
> >> + * DPHY driver for the StarFive JH7110 SoC
> >> + *
> >> + * Copyright (C) 2023 StarFive Technology Co., Ltd.
> >> + */
> >> +
> >> +#include <linux/bitfield.h>
> >> +#include <linux/bitops.h>
> >> +#include <linux/clk.h>
> >> +#include <linux/io.h>
> >> +#include <linux/mfd/syscon.h>
> >> +#include <linux/module.h>
> >> +#include <linux/of_address.h>
> >> +#include <linux/of_device.h>
> >> +#include <linux/phy/phy.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/regmap.h>
> >> +#include <linux/reset.h>
> >> +
> >> +#define STF_DPHY_APBCFGSAIF__SYSCFG(x) (x)
> >
> > What is the purpose of this? also whats with __ ?
> >
>
> In starfive jh7110 SoC, Dphy rx module register's name is called such as
> STF_DPHY_APBCFGSAIF__SYSCFG_4, STF_DPHY_APBCFGSAIF__SYSCFG_8, STF_DPHY_APBCFGSAIF__SYSCFG_12...
> so I merge them to use a marco STF_DPHY_APBCFGSAIF__SYSCFG(x).
>
> Should I use one "_" is enoughed?

Yes, it should be enough


>
> >> +
> >> +#define STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_EN BIT(6)
> >> +#define STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_IN GENMASK(12, 7)
> >> +#define STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_EN BIT(19)
> >> +#define STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_IN GENMASK(25, 20)
> >> +
> >> +#define STF_DPHY_DATA_BUS16_8 BIT(8)
> >> +#define STF_DPHY_DEBUG_MODE_SEL GENMASK(15, 9)
> >> +
> >> +#define STF_DPHY_ENABLE_CLK BIT(6)
> >> +#define STF_DPHY_ENABLE_CLK1 BIT(7)
> >> +#define STF_DPHY_ENABLE_LAN0 BIT(8)
> >> +#define STF_DPHY_ENABLE_LAN1 BIT(9)
> >> +#define STF_DPHY_ENABLE_LAN2 BIT(10)
> >> +#define STF_DPHY_ENABLE_LAN3 BIT(11)
> >> +#define STF_DPHY_GPI_EN GENMASK(17, 12)
> >> +#define STF_DPHY_HS_FREQ_CHANGE_CLK BIT(18)
> >> +#define STF_DPHY_HS_FREQ_CHANGE_CLK1 BIT(19)
> >> +#define STF_DPHY_LANE_SWAP_CLK GENMASK(22, 20)
> >> +#define STF_DPHY_LANE_SWAP_CLK1 GENMASK(25, 23)
> >> +#define STF_DPHY_LANE_SWAP_LAN0 GENMASK(28, 26)
> >> +#define STF_DPHY_LANE_SWAP_LAN1 GENMASK(31, 29)
> >> +
> >> +#define STF_DPHY_LANE_SWAP_LAN2 GENMASK(2, 0)
> >> +#define STF_DPHY_LANE_SWAP_LAN3 GENMASK(5, 3)
> >> +#define STF_DPHY_MP_TEST_EN BIT(6)
> >> +#define STF_DPHY_MP_TEST_MODE_SEL GENMASK(11, 7)
> >> +#define STF_DPHY_PLL_CLK_SEL GENMASK(21, 12)
> >> +#define STF_DPHY_PRECOUNTER_IN_CLK GENMASK(29, 22)
> >> +
> >> +#define STF_DPHY_PRECOUNTER_IN_CLK1 GENMASK(7, 0)
> >> +#define STF_DPHY_PRECOUNTER_IN_LAN0 GENMASK(15, 8)
> >> +#define STF_DPHY_PRECOUNTER_IN_LAN1 GENMASK(23, 16)
> >> +#define STF_DPHY_PRECOUNTER_IN_LAN2 GENMASK(31, 24)
> >> +
> >> +#define STF_DPHY_PRECOUNTER_IN_LAN3 GENMASK(7, 0)
> >> +#define STF_DPHY_RX_1C2C_SEL BIT(8)
> >> +
> >> +struct regval_t {
> >
> > regval should be okay, no?
> >
>
> OK, will change to regval
>
> >> + u32 addr;
> >> + u32 val;
> >> +};
> >> +
> >> +struct stf_dphy {
> >> + struct device *dev;
> >> + void __iomem *regs;
> >> + struct clk *cfg_clk;
> >> + struct clk *ref_clk;
> >> + struct clk *tx_clk;
> >> + struct reset_control *rstc;
> >> + struct regulator *mipi_0p9;
> >> + struct phy *phy;
> >> + struct regmap *stf_aon_syscon;
> >> + unsigned int aon_gp_reg;
> >> +};
> >> +
> >> +struct stf_dphy_info {
> >> + bool external_support;
> >> + int (*external_get)(struct stf_dphy *dphy);
> >> + void (*external_init)(struct stf_dphy *dphy);
> >> + void (*external_exit)(struct stf_dphy *dphy);
> >> +};
> >> +
> >> +static const struct stf_dphy_info *stf_dphy_info;
> >> +
> >> +static const struct regval_t stf_dphy_init_list[] = {
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(4), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(8), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(12), 0x0000fff0 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(16), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(20), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(24), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(28), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(32), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(36), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(40), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(40), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(48), 0x24000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(52), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(56), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(60), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(64), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(68), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(72), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(76), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(80), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(84), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(88), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(92), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(96), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(100), 0x02000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(104), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(108), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(112), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(116), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(120), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(124), 0x0000000c },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(128), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(132), 0xcc500000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(136), 0x000000cc },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(140), 0x00000000 },
> >> + { STF_DPHY_APBCFGSAIF__SYSCFG(144), 0x00000000 },
> >> +};
> >> +
> >> +static int stf_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> >> +{
> >> + struct stf_dphy *dphy = phy_get_drvdata(phy);
> >> + int map[6] = {4, 0, 1, 2, 3, 5};
> >
> > what does this mean?
>
> This is the physical lane and logical lane mapping table, should I add a note for it?

Yes please. Also will the mapping be always static or ever change?


>
> >> + int i;
> >> +
> >> + for (i = 0; i < ARRAY_SIZE(stf_dphy_init_list); i++)
> >> + writel(stf_dphy_init_list[i].val,
> >> + dphy->regs + stf_dphy_init_list[i].addr);
> >> +
> >> + writel(FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_EN, 1) |
> >> + FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_IN, 0x1b) |
> >> + FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_EN, 1) |
> >> + FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_IN, 0x1b),
> >> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(0));
> >> +
> >> + writel(FIELD_PREP(STF_DPHY_DATA_BUS16_8, 0) |
> >> + FIELD_PREP(STF_DPHY_DEBUG_MODE_SEL, 0x5a),
> >> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(184));
> >> +
> >> + writel(FIELD_PREP(STF_DPHY_ENABLE_CLK, 1) |
> >> + FIELD_PREP(STF_DPHY_ENABLE_CLK1, 1) |
> >> + FIELD_PREP(STF_DPHY_ENABLE_LAN0, 1) |
> >> + FIELD_PREP(STF_DPHY_ENABLE_LAN1, 1) |
> >> + FIELD_PREP(STF_DPHY_ENABLE_LAN2, 1) |
> >> + FIELD_PREP(STF_DPHY_ENABLE_LAN3, 1) |
> >> + FIELD_PREP(STF_DPHY_GPI_EN, 0) |
> >> + FIELD_PREP(STF_DPHY_HS_FREQ_CHANGE_CLK, 0) |
> >> + FIELD_PREP(STF_DPHY_HS_FREQ_CHANGE_CLK1, 0) |
> >> + FIELD_PREP(STF_DPHY_LANE_SWAP_CLK, map[0]) |
> >> + FIELD_PREP(STF_DPHY_LANE_SWAP_CLK1, map[5]) |
> >> + FIELD_PREP(STF_DPHY_LANE_SWAP_LAN0, map[1]) |
> >> + FIELD_PREP(STF_DPHY_LANE_SWAP_LAN1, map[2]),
> >> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(188));
> >> +
> >> + writel(FIELD_PREP(STF_DPHY_LANE_SWAP_LAN2, map[3]) |
> >> + FIELD_PREP(STF_DPHY_LANE_SWAP_LAN3, map[4]) |
> >> + FIELD_PREP(STF_DPHY_MP_TEST_EN, 0) |
> >> + FIELD_PREP(STF_DPHY_MP_TEST_MODE_SEL, 0) |
> >> + FIELD_PREP(STF_DPHY_PLL_CLK_SEL, 0x37c) |
> >> + FIELD_PREP(STF_DPHY_PRECOUNTER_IN_CLK, 8),
> >> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(192));
> >> +
> >> + writel(FIELD_PREP(STF_DPHY_PRECOUNTER_IN_CLK1, 8) |
> >> + FIELD_PREP(STF_DPHY_PRECOUNTER_IN_LAN0, 7) |
> >> + FIELD_PREP(STF_DPHY_PRECOUNTER_IN_LAN1, 7) |
> >> + FIELD_PREP(STF_DPHY_PRECOUNTER_IN_LAN2, 7),
> >> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(196));
> >> +
> >> + writel(FIELD_PREP(STF_DPHY_PRECOUNTER_IN_LAN3, 7) |
> >> + FIELD_PREP(STF_DPHY_RX_1C2C_SEL, 0),
> >> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(200));
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int stf_dphy_init(struct phy *phy)
> >> +{
> >> + struct stf_dphy *dphy = phy_get_drvdata(phy);
> >> + int ret;
> >> +
> >> + ret = regulator_enable(dphy->mipi_0p9);
> >> + if (ret)
> >> + goto err_0p9;
> >> +
> >> + if (stf_dphy_info->external_support && stf_dphy_info->external_init)
> >> + stf_dphy_info->external_init(dphy);
> >> +
> >> + return 0;
> >> +
> >> +err_0p9:
> >> + return ret;
> >> +}
> >> +
> >> +static int stf_dphy_exit(struct phy *phy)
> >> +{
> >> + struct stf_dphy *dphy = phy_get_drvdata(phy);
> >> +
> >> + if (stf_dphy_info->external_support && stf_dphy_info->external_exit)
> >> + stf_dphy_info->external_exit(dphy);
> >> +
> >> + regulator_disable(dphy->mipi_0p9);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int stf_dphy_power_on(struct phy *phy)
> >> +{
> >> + struct stf_dphy *dphy = phy_get_drvdata(phy);
> >> +
> >> + clk_set_rate(dphy->cfg_clk, 99000000);
> >> + clk_set_rate(dphy->ref_clk, 49500000);
> >> + clk_set_rate(dphy->tx_clk, 19800000);
> >> + reset_control_deassert(dphy->rstc);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int stf_dphy_power_off(struct phy *phy)
> >> +{
> >> + struct stf_dphy *dphy = phy_get_drvdata(phy);
> >> +
> >> + reset_control_assert(dphy->rstc);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static const struct phy_ops stf_dphy_ops = {
> >> + .init = stf_dphy_init,
> >> + .exit = stf_dphy_exit,
> >> + .configure = stf_dphy_configure,
> >> + .power_on = stf_dphy_power_on,
> >> + .power_off = stf_dphy_power_off,
> >> +};
> >> +
> >> +static int stf_dphy_probe(struct platform_device *pdev)
> >> +{
> >> + struct phy_provider *phy_provider;
> >> + struct stf_dphy *dphy;
> >> + int ret;
> >> +
> >> + dphy = devm_kzalloc(&pdev->dev, sizeof(*dphy), GFP_KERNEL);
> >> + if (!dphy)
> >> + return -ENOMEM;
> >> + stf_dphy_info = of_device_get_match_data(&pdev->dev);
> >> + dev_set_drvdata(&pdev->dev, dphy);
> >> + dphy->dev = &pdev->dev;
> >> +
> >> + dphy->regs = devm_platform_ioremap_resource(pdev, 0);
> >> + if (IS_ERR(dphy->regs))
> >> + return PTR_ERR(dphy->regs);
> >> +
> >> + dphy->cfg_clk = devm_clk_get(&pdev->dev, "cfg");
> >> + if (IS_ERR(dphy->cfg_clk))
> >> + return PTR_ERR(dphy->cfg_clk);
> >> +
> >> + dphy->ref_clk = devm_clk_get(&pdev->dev, "ref");
> >> + if (IS_ERR(dphy->ref_clk))
> >> + return PTR_ERR(dphy->ref_clk);
> >> +
> >> + dphy->tx_clk = devm_clk_get(&pdev->dev, "tx");
> >> + if (IS_ERR(dphy->tx_clk))
> >> + return PTR_ERR(dphy->tx_clk);
> >> +
> >> + dphy->rstc = devm_reset_control_array_get_exclusive(&pdev->dev);
> >> + if (IS_ERR(dphy->rstc))
> >> + return PTR_ERR(dphy->rstc);
> >> +
> >> + dphy->mipi_0p9 = devm_regulator_get(&pdev->dev, "mipi_0p9");
> >> + if (IS_ERR(dphy->mipi_0p9))
> >> + return PTR_ERR(dphy->mipi_0p9);
> >> +
> >> + if (stf_dphy_info->external_support && stf_dphy_info->external_get) {
> >> + ret = stf_dphy_info->external_get(dphy);
> >> + if (ret < 0) {
> >> + dev_err(&pdev->dev, "Failed to get PHY external info\n");
> >> + return ret;
> >> + }
> >> + }
> >> +
> >> + dphy->phy = devm_phy_create(&pdev->dev, NULL, &stf_dphy_ops);
> >> + if (IS_ERR(dphy->phy)) {
> >> + dev_err(&pdev->dev, "Failed to create PHY\n");
> >> + return PTR_ERR(dphy->phy);
> >> + }
> >> +
> >> + phy_set_drvdata(dphy->phy, dphy);
> >> + phy_provider = devm_of_phy_provider_register(&pdev->dev,
> >> + of_phy_simple_xlate);
> >> +
> >> + return PTR_ERR_OR_ZERO(phy_provider);
> >> +}
> >> +
> >> +static int stf_external_get(struct stf_dphy *dphy)
> >> +{
> >> + struct of_phandle_args args;
> >> + int ret;
> >> +
> >> + ret = of_parse_phandle_with_fixed_args(dphy->dev->of_node,
> >> + "starfive,aon-syscon",
> >> + 1, 0, &args);
> >> + if (ret < 0) {
> >> + dev_err(dphy->dev, "Failed to parse starfive,aon-syscon\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + dphy->stf_aon_syscon = syscon_node_to_regmap(args.np);
> >> + of_node_put(args.np);
> >> + if (IS_ERR(dphy->stf_aon_syscon))
> >> + return PTR_ERR(dphy->stf_aon_syscon);
> >> +
> >> + dphy->aon_gp_reg = args.args[0];
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static void stf_external_init(struct stf_dphy *dphy)
> >> +{
> >> + regmap_update_bits(dphy->stf_aon_syscon, dphy->aon_gp_reg,
> >> + BIT(31), BIT(31));
> >> +}
> >> +
> >> +static void stf_external_exit(struct stf_dphy *dphy)
> >> +{
> >> + regmap_update_bits(dphy->stf_aon_syscon, dphy->aon_gp_reg,
> >> + BIT(31), 0);
> >> +}
> >> +
> >> +static const struct stf_dphy_info starfive_dphy_info = {
> >> + .external_support = true,
> >> + .external_get = stf_external_get,
> >> + .external_init = stf_external_init,
> >> + .external_exit = stf_external_exit,
> >> +};
> >> +
> >> +static const struct of_device_id stf_dphy_dt_ids[] = {
> >> + {
> >> + .compatible = "starfive,jh7110-dphy-rx",
> >> + .data = &starfive_dphy_info,
> >
> > is there a plan to support multiple versions which need different
> > get/init/exit methods?
> >
>
> There is no plan to support multiple versions. So this different methods
> interface can cancel?

yes

>
> >> + },
> >> + { /* sentinel */ },
> >> +};
> >> +MODULE_DEVICE_TABLE(of, stf_dphy_dt_ids);
> >> +
> >> +static struct platform_driver stf_dphy_driver = {
> >> + .probe = stf_dphy_probe,
> >> + .driver = {
> >> + .name = "starfive-dphy-rx",
> >> + .of_match_table = stf_dphy_dt_ids,
> >> + },
> >> +};
> >> +module_platform_driver(stf_dphy_driver);
> >> +
> >> +MODULE_AUTHOR("Jack Zhu <[email protected]>");
> >> +MODULE_AUTHOR("Changhuang Liang <[email protected]>");
> >> +MODULE_DESCRIPTION("Starfive DPHY RX driver");
> >> +MODULE_LICENSE("GPL");
> >> --
> >> 2.25.1
> >

--
~Vinod

2023-04-03 01:59:22

by Changhuang Liang

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] phy: starfive: Add mipi dphy rx support



On 2023/3/31 21:52, Vinod Koul wrote:
> On 21-03-23, 14:08, Changhuang Liang wrote:
>>
>>
>> On 2023/3/20 20:37, Vinod Koul wrote:
>>> On 22-02-23, 17:59, Changhuang Liang wrote:
>>>> [...]
>>>> +static const struct regval_t stf_dphy_init_list[] = {
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(4), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(8), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(12), 0x0000fff0 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(16), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(20), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(24), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(28), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(32), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(36), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(40), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(40), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(48), 0x24000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(52), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(56), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(60), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(64), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(68), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(72), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(76), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(80), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(84), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(88), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(92), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(96), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(100), 0x02000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(104), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(108), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(112), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(116), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(120), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(124), 0x0000000c },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(128), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(132), 0xcc500000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(136), 0x000000cc },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(140), 0x00000000 },
>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(144), 0x00000000 },
>>>> +};
>>>> +
>>>> +static int stf_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
>>>> +{
>>>> + struct stf_dphy *dphy = phy_get_drvdata(phy);
>>>> + int map[6] = {4, 0, 1, 2, 3, 5};
>>>
>>> what does this mean?
>>
>> This is the physical lane and logical lane mapping table, should I add a note for it?
>
> Yes please. Also will the mapping be always static or ever change?
>

The mapping is always static on the visionfive2 single board computer.
Thanks for your comments.

>>
>>>> + int i;
>>>> +
>>>> + for (i = 0; i < ARRAY_SIZE(stf_dphy_init_list); i++)
>>>> + writel(stf_dphy_init_list[i].val,
>>>> + dphy->regs + stf_dphy_init_list[i].addr);
>>>> +
>>>> + writel(FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_EN, 1) |
>>>> + FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL0_2D1C_EFUSE_IN, 0x1b) |
>>>> + FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_EN, 1) |
>>>> + FIELD_PREP(STF_DPHY_DA_CDPHY_R100_CTRL1_2D1C_EFUSE_IN, 0x1b),
>>>> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(0));
>>>> +
>>>> + writel(FIELD_PREP(STF_DPHY_DATA_BUS16_8, 0) |
>>>> + FIELD_PREP(STF_DPHY_DEBUG_MODE_SEL, 0x5a),
>>>> + dphy->regs + STF_DPHY_APBCFGSAIF__SYSCFG(184));
>>>> [...]
>>>> +module_platform_driver(stf_dphy_driver);
>>>> +
>>>> +MODULE_AUTHOR("Jack Zhu <[email protected]>");
>>>> +MODULE_AUTHOR("Changhuang Liang <[email protected]>");
>>>> +MODULE_DESCRIPTION("Starfive DPHY RX driver");
>>>> +MODULE_LICENSE("GPL");
>>>> --
>>>> 2.25.1
>>>
>

2023-04-03 06:27:34

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] phy: starfive: Add mipi dphy rx support

On Mon, Apr 03, 2023 at 09:39:00AM +0800, Changhuang Liang wrote:
> On 2023/3/31 21:52, Vinod Koul wrote:
> > On 21-03-23, 14:08, Changhuang Liang wrote:
> >> On 2023/3/20 20:37, Vinod Koul wrote:
> >>> On 22-02-23, 17:59, Changhuang Liang wrote:
> >>>> [...]
> >>>> +static const struct regval_t stf_dphy_init_list[] = {
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(4), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(8), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(12), 0x0000fff0 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(16), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(20), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(24), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(28), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(32), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(36), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(40), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(40), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(48), 0x24000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(52), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(56), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(60), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(64), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(68), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(72), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(76), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(80), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(84), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(88), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(92), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(96), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(100), 0x02000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(104), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(108), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(112), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(116), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(120), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(124), 0x0000000c },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(128), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(132), 0xcc500000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(136), 0x000000cc },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(140), 0x00000000 },
> >>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(144), 0x00000000 },
> >>>> +};
> >>>> +
> >>>> +static int stf_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> >>>> +{
> >>>> + struct stf_dphy *dphy = phy_get_drvdata(phy);
> >>>> + int map[6] = {4, 0, 1, 2, 3, 5};
> >>>
> >>> what does this mean?
> >>
> >> This is the physical lane and logical lane mapping table, should I add a note for it?
> >
> > Yes please. Also will the mapping be always static or ever change?
> >
>
> The mapping is always static on the visionfive2 single board computer.
> Thanks for your comments.

What about other boards featuring a JH7110?

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

2023-04-03 06:40:17

by Changhuang Liang

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] phy: starfive: Add mipi dphy rx support



On 2023/4/3 14:24, Conor Dooley wrote:
> On Mon, Apr 03, 2023 at 09:39:00AM +0800, Changhuang Liang wrote:
>> On 2023/3/31 21:52, Vinod Koul wrote:
>>> On 21-03-23, 14:08, Changhuang Liang wrote:
>>>> On 2023/3/20 20:37, Vinod Koul wrote:
>>>>> On 22-02-23, 17:59, Changhuang Liang wrote:
>>>>>> [...]
>>>>>> +static const struct regval_t stf_dphy_init_list[] = {
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(4), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(8), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(12), 0x0000fff0 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(16), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(20), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(24), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(28), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(32), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(36), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(40), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(40), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(48), 0x24000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(52), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(56), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(60), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(64), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(68), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(72), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(76), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(80), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(84), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(88), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(92), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(96), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(100), 0x02000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(104), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(108), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(112), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(116), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(120), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(124), 0x0000000c },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(128), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(132), 0xcc500000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(136), 0x000000cc },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(140), 0x00000000 },
>>>>>> + { STF_DPHY_APBCFGSAIF__SYSCFG(144), 0x00000000 },
>>>>>> +};
>>>>>> +
>>>>>> +static int stf_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
>>>>>> +{
>>>>>> + struct stf_dphy *dphy = phy_get_drvdata(phy);
>>>>>> + int map[6] = {4, 0, 1, 2, 3, 5};
>>>>>
>>>>> what does this mean?
>>>>
>>>> This is the physical lane and logical lane mapping table, should I add a note for it?
>>>
>>> Yes please. Also will the mapping be always static or ever change?
>>>
>>
>> The mapping is always static on the visionfive2 single board computer.
>> Thanks for your comments.
>
> What about other boards featuring a JH7110?

maybe add this mapping in the device tree, we just need to parse the device tree is better.

2023-04-03 06:42:18

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] phy: starfive: Add mipi dphy rx support

On 03-04-23, 14:31, Changhuang Liang wrote:
>
>
> On 2023/4/3 14:24, Conor Dooley wrote:
> > On Mon, Apr 03, 2023 at 09:39:00AM +0800, Changhuang Liang wrote:
> >> On 2023/3/31 21:52, Vinod Koul wrote:
> >>> On 21-03-23, 14:08, Changhuang Liang wrote:
> >>>> On 2023/3/20 20:37, Vinod Koul wrote:
> >>>>> On 22-02-23, 17:59, Changhuang Liang wrote:

> >>>>>> +static int stf_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> >>>>>> +{
> >>>>>> + struct stf_dphy *dphy = phy_get_drvdata(phy);
> >>>>>> + int map[6] = {4, 0, 1, 2, 3, 5};
> >>>>>
> >>>>> what does this mean?
> >>>>
> >>>> This is the physical lane and logical lane mapping table, should I add a note for it?
> >>>
> >>> Yes please. Also will the mapping be always static or ever change?
> >>>
> >>
> >> The mapping is always static on the visionfive2 single board computer.
> >> Thanks for your comments.
> >
> > What about other boards featuring a JH7110?
>
> maybe add this mapping in the device tree, we just need to parse the device tree is better.

If the mapping is hw description then yes it makes sense

--
~Vinod

2023-04-03 06:52:28

by Changhuang Liang

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] phy: starfive: Add mipi dphy rx support



On 2023/4/3 14:34, Vinod Koul wrote:
> On 03-04-23, 14:31, Changhuang Liang wrote:
>>
>>
>> On 2023/4/3 14:24, Conor Dooley wrote:
>>> On Mon, Apr 03, 2023 at 09:39:00AM +0800, Changhuang Liang wrote:
>>>> On 2023/3/31 21:52, Vinod Koul wrote:
>>>>> On 21-03-23, 14:08, Changhuang Liang wrote:
>>>>>> On 2023/3/20 20:37, Vinod Koul wrote:
>>>>>>> On 22-02-23, 17:59, Changhuang Liang wrote:
>
>>>>>>>> +static int stf_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
>>>>>>>> +{
>>>>>>>> + struct stf_dphy *dphy = phy_get_drvdata(phy);
>>>>>>>> + int map[6] = {4, 0, 1, 2, 3, 5};
>>>>>>>
>>>>>>> what does this mean?
>>>>>>
>>>>>> This is the physical lane and logical lane mapping table, should I add a note for it?
>>>>>
>>>>> Yes please. Also will the mapping be always static or ever change?
>>>>>
>>>>
>>>> The mapping is always static on the visionfive2 single board computer.
>>>> Thanks for your comments.
>>>
>>> What about other boards featuring a JH7110?
>>
>> maybe add this mapping in the device tree, we just need to parse the device tree is better.
>
> If the mapping is hw description then yes it makes sense
>

Yes, different hardware may be described differently, I will change it in next version.