[PATCH v1 0/4] phy: samsung: add Exynos2200 SNPS eUSB2 driver

[PATCH v1 0/4] phy: samsung: add Exynos2200 SNPS eUSB2 driver

[Ivaylo Ivanov]

Hey folks,

In the vendor kernel, everything is handled in a single phy driver,
with helpers for functions outside it. Clocks and regulators are
specified and enabled in one node, which makes it difficult to
separate what clocks and regulators go where without access to
schematics or TRMs. The following gates are defined for USB:

CLK_BLK_HSI0_UID_USB32DRD_IPCLKPORT_I_USBSUBCTL_APB_PCLK
CLK_BLK_HSI0_UID_USB32DRD_IPCLKPORT_I_USBDPPHY_CTRL_PCLK
CLK_BLK_HSI0_UID_USB32DRD_IPCLKPORT_I_USBDPPHY_TCA_APB_CLK

CLK_BLK_HSI0_UID_USB32DRD_IPCLKPORT_I_USBLINK_ACLK
CLK_BLK_HSI0_UID_USB32DRD_IPCLKPORT_I_USB32DRD_REF_CLK_40

CLK_BLK_HSI0_UID_USB32DRD_IPCLKPORT_I_EUSB_CTRL_PCLK
CLK_BLK_HSI0_UID_USB32DRD_IPCLKPORT_I_EUSB_APB_CLK
CLK_BLK_HSI0_UID_AS_APB_EUSBPHY_HSI0_IPCLKPORT_PCLKM
CLK_BLK_HSI0_UID_RSTNSYNC_CLK_HSI0_EUSB_IPCLKPORT_CLK

From what I've managed to understand, this SoC has 3 PHYs for USB:
 - a synopsys eusb2 phy
 - a synposys combophy for usbdp and superspeed
 - a usbcon phy, which can detach/attach pipe3 and has registers for
   linkctrl

The approach taken here is to introduce 2 new separate drivers: one for
the eusb2 and one for the usbcon. The eusb2 driver is modelled to take
a phandle to the usbcon and configure/unconfigure it for the eUSB2
to become accessible and not cause a hang whenever its registers are
touched.

A new USBDP driver will be added later on, so that pipe3 and super-speed
can be configured.

Best regards,
Ivaylo

Ivaylo Ivanov (4):
  dt-bindings: phy: add samsung,exynos2200-snps-eusb2-phy schema file
  dt-bindings: phy: add samsung,exynos2200-usbcon-phy schema file
  phy: samsung: add Exynos2200 SNPS eUSB2 driver
  phy: samsung: add Exynos2200 usb phy controller

 .../samsung,exynos2200-snps-eusb2-phy.yaml    |  75 ++++
 .../phy/samsung,exynos2200-usbcon-phy.yaml    |  70 ++++
 drivers/phy/samsung/Kconfig                   |  26 ++
 drivers/phy/samsung/Makefile                  |   2 +
 .../phy/samsung/phy-exynos2200-snps-eusb2.c   | 351 ++++++++++++++++++
 drivers/phy/samsung/phy-exynos2200-usbcon.c   | 241 ++++++++++++
 include/linux/soc/samsung/exynos-regs-pmu.h   |   3 +
 7 files changed, 768 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-snps-eusb2-phy.yaml
 create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
 create mode 100644 drivers/phy/samsung/phy-exynos2200-snps-eusb2.c
 create mode 100644 drivers/phy/samsung/phy-exynos2200-usbcon.c

-- 
2.43.0

[PATCH v1 1/4] dt-bindings: phy: add samsung,exynos2200-snps-eusb2-phy schema file

[Ivaylo Ivanov]

The Exynos2200 SoC uses Synopsis eUSB2 PHY. Add a dt-binding schema
for the new driver.

Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
---
 .../samsung,exynos2200-snps-eusb2-phy.yaml    | 75 +++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-snps-eusb2-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos2200-snps-eusb2-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos2200-snps-eusb2-phy.yaml
new file mode 100644
index 000000000..d69a10f00
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/samsung,exynos2200-snps-eusb2-phy.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/samsung,exynos2200-snps-eusb2-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung SNPS eUSB2 phy controller
+
+maintainers:
+  - Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
+
+description:
+  eUSB2 controller supports LS/FS/HS usb connectivity on Exynos chipsets.
+
+properties:
+  compatible:
+    enum:
+      - samsung,exynos2200-snps-eusb2-phy
+
+  reg:
+    maxItems: 1
+
+  "#phy-cells":
+    const: 0
+
+  clocks:
+    items:
+      - description: Reference clock
+      - description: APB clock
+      - description: Control PCLK
+
+  clock-names:
+    items:
+      - const: ref
+      - const: apb
+      - const: ctrl
+
+  phys:
+    maxItems: 1
+    description:
+      Phandle to USBCON phy
+
+  vdd-supply:
+    description:
+      Phandle to 0.88V regulator supply
+
+  vdda12-supply:
+    description:
+      Phandle to 1.2V regulator supply
+
+required:
+  - compatible
+  - reg
+  - "#phy-cells"
+  - clocks
+  - clock-names
+  - vdd-supply
+  - vdda12-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/samsung,exynos2200.h>
+
+    usb_hsphy: phy@10ab0000 {
+        compatible = "samsung,exynos2200-snps-eusb2-phy";
+        reg = <0 0x10ab0000 0 0x10000>;
+        clocks = <&cmu_hsi0 CLK_MOUT_HSI0_USB32DRD>,
+                 <&cmu_hsi0 CLK_MOUT_HSI0_NOC>,
+                 <&cmu_hsi0 CLK_DOUT_DIV_CLK_HSI0_EUSB>;
+        clock-names = "ref", "apb", "ctrl";
+        #phy-cells = <0>;
+        phys = <&usbcon_phy>;
+    };
-- 
2.43.0

[PATCH v1 2/4] dt-bindings: phy: add samsung,exynos2200-usbcon-phy schema file

[Ivaylo Ivanov]

The Exynos2200 SoC has a USB phy controller block that controls the
usage of USB phys. Add a dt-binding schema for the new driver.

Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
---
 .../phy/samsung,exynos2200-usbcon-phy.yaml    | 70 +++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
new file mode 100644
index 000000000..468dc1560
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/samsung,exynos2200-usbcon-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Exynos2200 USB controller phy
+
+maintainers:
+  - Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
+
+description:
+  USB controller manages phy selecton for UTMI and PIPE3
+
+properties:
+  compatible:
+    enum:
+      - samsung,exynos2200-usbcon-phy
+
+  reg:
+    maxItems: 1
+
+  "#phy-cells":
+    const: 0
+
+  clocks:
+    items:
+      - description: APB clock
+
+  clock-names:
+    items:
+      - const: apb
+
+  samsung,pmu-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: phandle for PMU system controller interface, used to
+                 control PMU registers bits for USBCON PHY
+
+  vdd1p9-supply:
+    description:
+      Phandle to 1.9V regulator supply
+
+  vdd3-supply:
+    description:
+      Phandle to 3V regulator supply
+
+required:
+  - compatible
+  - reg
+  - "#phy-cells"
+  - clocks
+  - clock-names
+  - samsung,pmu-syscon
+  - vdd1p9-supply
+  - vdd3-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/samsung,exynos2200.h>
+
+    usb_con_phy: phy@10aa0000 {
+        compatible = "samsung,exynos2200-snps-eusb2-phy";
+        reg = <0 0x10aa0000 0 0x10000>;
+        clocks = <&cmu_hsi0 CLK_MOUT_HSI0_NOC>;
+        clock-names = "apb";
+        #phy-cells = <0>;
+        samsung,pmu-syscon = <&pmu_system_controller>;
+    };
-- 
2.43.0

[PATCH v1 3/4] phy: samsung: add Exynos2200 SNPS eUSB2 driver

[Ivaylo Ivanov]

The Exynos2200 SoC uses Synopsis eUSB2 PHY for USB 2.0. Add a new
driver for it.

eUSB2 on Exynos SoCs is usually paired alongside a USB PHY controller.
Currently the driver is modelled to take and enable/disable the usb phy
controller when needed.

The driver is based on information from downstream drivers.

Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
---
 drivers/phy/samsung/Kconfig                   |  13 +
 drivers/phy/samsung/Makefile                  |   1 +
 .../phy/samsung/phy-exynos2200-snps-eusb2.c   | 351 ++++++++++++++++++
 3 files changed, 365 insertions(+)
 create mode 100644 drivers/phy/samsung/phy-exynos2200-snps-eusb2.c

diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig
index e2330b089..f62285254 100644
--- a/drivers/phy/samsung/Kconfig
+++ b/drivers/phy/samsung/Kconfig
@@ -77,6 +77,19 @@ config PHY_S5PV210_USB2
 	  particular SoC is compiled in the driver. In case of S5PV210 two phys
 	  are available - device and host.
 
+config PHY_EXYNOS2200_SNPS_EUSB2
+	tristate "Exynos2200 eUSB 2.0 PHY driver"
+	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
+	depends on HAS_IOMEM
+	depends on USB_DWC3_EXYNOS
+	select GENERIC_PHY
+	select MFD_SYSCON
+	default y
+	help
+	  Enable USBCON PHY support for Exynos2200 SoC.
+	  This driver provides PHY interface for eUSB 2.0 controller
+	  present on Exynos5 SoC series.
+
 config PHY_EXYNOS5_USBDRD
 	tristate "Exynos5 SoC series USB DRD PHY driver"
 	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile
index fea1f96d0..90b84c7fc 100644
--- a/drivers/phy/samsung/Makefile
+++ b/drivers/phy/samsung/Makefile
@@ -14,5 +14,6 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2)	+= phy-exynos4210-usb2.o
 phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
 phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
 phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
+obj-$(CONFIG_PHY_EXYNOS2200_SNPS_EUSB2)	+= phy-exynos2200-snps-eusb2.o
 obj-$(CONFIG_PHY_EXYNOS5_USBDRD)	+= phy-exynos5-usbdrd.o
 obj-$(CONFIG_PHY_EXYNOS5250_SATA)	+= phy-exynos5250-sata.o
diff --git a/drivers/phy/samsung/phy-exynos2200-snps-eusb2.c b/drivers/phy/samsung/phy-exynos2200-snps-eusb2.c
new file mode 100644
index 000000000..ee6d96411
--- /dev/null
+++ b/drivers/phy/samsung/phy-exynos2200-snps-eusb2.c
@@ -0,0 +1,351 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/iopoll.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mod_devicetable.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+
+#define EXYNOS2200_EUSB2_RST_CTRL		0x0
+#define EXYNOS2200_UTMI_PORT_RESET_MASK		GENMASK(5, 4)
+#define EXYNOS2200_EUSB_PHY_RESET_MASK		GENMASK(1, 0)
+
+#define EXYNOS2200_EUSB2_CMN_CTRL		0x4
+#define EXYNOS2200_PHY_CFG_RPTR_MODE		BIT(10)
+#define EXYNOS2200_REF_FREQ_SEL			GENMASK(6, 4)
+#define EXYNOS2200_PHY_ENABLE			BIT(0)
+
+#define EXYNOS2200_EUSB2_PLLCFG0		0x8
+#define EXYNOS2200_PLL_FB_DIV_MASK		GENMASK(19, 8)
+
+#define EXYNOS2200_EUSB2_PLLCFG1		0xc
+#define EXYNOS2200_PLL_REF_DIV			GENMASK(11, 8)
+
+#define EXYNOS2200_EUSB2_TESTSE			0x20
+#define EXYNOS2200_TEST_IDDQ			BIT(6)
+
+struct exynos2200_snps_eusb2_phy_drvdata {
+	const char * const *clk_names;
+	int n_clks;
+	const char * const *regulator_names;
+	int n_regulators;
+};
+
+struct exynos2200_snps_eusb2_phy {
+	struct phy *phy;
+	void __iomem *base;
+
+	struct clk *ref_clk;
+	struct clk_bulk_data *clks;
+	const struct exynos2200_snps_eusb2_phy_drvdata *drv_data;
+	struct reset_control *phy_reset;
+
+	struct regulator_bulk_data *vregs;
+
+	enum phy_mode mode;
+
+	struct phy *repeater;
+	struct phy *usbcon;
+};
+
+static void exynos2200_snps_eusb2_phy_write_mask(void __iomem *base, u32 offset,
+						 u32 mask, u32 val)
+{
+	u32 reg;
+
+	reg = readl_relaxed(base + offset);
+	reg &= ~mask;
+	reg |= val & mask;
+	writel_relaxed(reg, base + offset);
+
+	/* ensure above write is completed */
+	readl_relaxed(base + offset);
+}
+
+static int exynos2200_snps_eusb2_ref_clk_init(struct exynos2200_snps_eusb2_phy *phy)
+{
+	unsigned long ref_clk_freq = clk_get_rate(phy->ref_clk);
+
+	switch (ref_clk_freq) {
+	case 19200000:
+		exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_CMN_CTRL,
+						     EXYNOS2200_REF_FREQ_SEL,
+						     0);
+
+		exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG0,
+						     EXYNOS2200_PLL_FB_DIV_MASK,
+						     FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 368));
+
+		exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG1,
+						     EXYNOS2200_PLL_REF_DIV,
+						     FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 0));
+		break;
+
+	case 20000000:
+		exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_CMN_CTRL,
+						     EXYNOS2200_REF_FREQ_SEL,
+						     FIELD_PREP(EXYNOS2200_REF_FREQ_SEL, 1));
+
+		exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG0,
+						     EXYNOS2200_PLL_FB_DIV_MASK,
+						     FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 352));
+
+		exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG1,
+						     EXYNOS2200_PLL_REF_DIV,
+						     FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 0));
+		break;
+
+	case 24000000:
+		exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_CMN_CTRL,
+						     EXYNOS2200_REF_FREQ_SEL,
+						     FIELD_PREP(EXYNOS2200_REF_FREQ_SEL, 2));
+
+		exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG0,
+						     EXYNOS2200_PLL_FB_DIV_MASK,
+						     FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 288));
+
+		exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG1,
+						     EXYNOS2200_PLL_REF_DIV,
+						     FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 0));
+		break;
+
+	case 26000000:
+		exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_CMN_CTRL,
+						     EXYNOS2200_REF_FREQ_SEL,
+						     FIELD_PREP(EXYNOS2200_REF_FREQ_SEL, 3));
+
+		exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG0,
+						     EXYNOS2200_PLL_FB_DIV_MASK,
+						     FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 263));
+
+		exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG1,
+						     EXYNOS2200_PLL_REF_DIV,
+						     FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 0));
+		break;
+
+	case 48000000:
+		exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_CMN_CTRL,
+						     EXYNOS2200_REF_FREQ_SEL,
+						     FIELD_PREP(EXYNOS2200_REF_FREQ_SEL, 2));
+
+		exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG0,
+						     EXYNOS2200_PLL_FB_DIV_MASK,
+						     FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 288));
+
+		exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG1,
+						     EXYNOS2200_PLL_REF_DIV,
+						     FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 1));
+		break;
+
+	default:
+		dev_err(&phy->phy->dev, "unsupported ref_clk_freq:%lu\n", ref_clk_freq);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int exynos2200_snps_eusb2_phy_init(struct phy *p)
+{
+	struct exynos2200_snps_eusb2_phy *phy = phy_get_drvdata(p);
+	int ret;
+
+	ret = regulator_bulk_enable(phy->drv_data->n_regulators, phy->vregs);
+	if (ret)
+		return ret;
+
+	ret = clk_bulk_prepare_enable(phy->drv_data->n_clks, phy->clks);
+	if (ret) {
+		dev_err(&p->dev, "failed to enable clocks, %d\n", ret);
+		goto disable_vreg;
+	}
+
+	ret = phy_init(phy->usbcon);
+	if (ret) {
+		dev_err(&p->dev, "usbcon init failed. %d\n", ret);
+		goto disable_ref_clk;
+	}
+
+	exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_RST_CTRL,
+					     EXYNOS2200_UTMI_PORT_RESET_MASK |
+					     EXYNOS2200_EUSB_PHY_RESET_MASK,
+					     FIELD_PREP(EXYNOS2200_UTMI_PORT_RESET_MASK, 0x1) |
+					     FIELD_PREP(EXYNOS2200_EUSB_PHY_RESET_MASK, 0x1));
+	fsleep(50); /* required after holding phy in reset */
+
+	exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_CMN_CTRL,
+					     EXYNOS2200_PHY_CFG_RPTR_MODE,
+					     EXYNOS2200_PHY_CFG_RPTR_MODE);
+
+	/* update ref_clk related registers */
+	ret = exynos2200_snps_eusb2_ref_clk_init(phy);
+	if (ret)
+		return ret;
+
+	exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_TESTSE,
+					     EXYNOS2200_TEST_IDDQ,
+					     0);
+	fsleep(10); /* required after releasing test_iddq */
+
+	exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_RST_CTRL,
+					     EXYNOS2200_EUSB_PHY_RESET_MASK,
+					     0);
+
+	exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_CMN_CTRL,
+					     EXYNOS2200_PHY_ENABLE,
+					     EXYNOS2200_PHY_ENABLE);
+
+	exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_RST_CTRL,
+					     EXYNOS2200_UTMI_PORT_RESET_MASK,
+					     0);
+	return 0;
+
+disable_ref_clk:
+	clk_disable_unprepare(phy->ref_clk);
+
+disable_vreg:
+	regulator_bulk_disable(phy->drv_data->n_regulators, phy->vregs);
+
+	return ret;
+}
+
+static int exynos2200_snps_eusb2_phy_exit(struct phy *p)
+{
+	struct exynos2200_snps_eusb2_phy *phy = phy_get_drvdata(p);
+
+	phy_exit(phy->usbcon);
+
+	clk_disable_unprepare(phy->ref_clk);
+	regulator_bulk_disable(phy->drv_data->n_regulators, phy->vregs);
+
+	return 0;
+}
+
+static const struct phy_ops exynos2200_snps_eusb2_phy_ops = {
+	.init		= exynos2200_snps_eusb2_phy_init,
+	.exit		= exynos2200_snps_eusb2_phy_exit,
+	.owner		= THIS_MODULE,
+};
+
+static int exynos2200_snps_eusb2_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct exynos2200_snps_eusb2_phy *phy;
+	const struct exynos2200_snps_eusb2_phy_drvdata *drv_data;
+	struct phy_provider *phy_provider;
+	struct phy *generic_phy;
+	int ret;
+
+	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	drv_data = of_device_get_match_data(dev);
+	if (!drv_data)
+		return -EINVAL;
+	phy->drv_data = drv_data;
+
+	phy->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(phy->base))
+		return PTR_ERR(phy->base);
+
+	phy->clks = devm_kcalloc(dev, drv_data->n_clks,
+				 sizeof(*phy->clks), GFP_KERNEL);
+	if (!phy->clks)
+		return -ENOMEM;
+
+	for (int i = 0; i < drv_data->n_clks; ++i)
+		phy->clks[i].id = drv_data->clk_names[i];
+
+	ret = devm_clk_bulk_get(dev, drv_data->n_clks,
+				phy->clks);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to get phy clock(s)\n");
+
+	for (int i = 0; i < phy->drv_data->n_clks; ++i) {
+		if (!strcmp(phy->clks[i].id, "ref")) {
+			phy->ref_clk = phy->clks[i].clk;
+			break;
+		}
+	}
+
+	phy->vregs = devm_kcalloc(dev, drv_data->n_regulators,
+				  sizeof(*phy->vregs), GFP_KERNEL);
+	if (!phy->vregs)
+		return -ENOMEM;
+	regulator_bulk_set_supply_names(phy->vregs,
+					drv_data->regulator_names,
+					drv_data->n_regulators);
+	ret = devm_regulator_bulk_get(dev, drv_data->n_regulators,
+				      phy->vregs);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to get regulators\n");
+
+	/* we treat the usblink controller phy as a separate phy */
+	phy->usbcon = devm_of_phy_get_by_index(dev, np, 0);
+	if (IS_ERR(phy->usbcon))
+		return dev_err_probe(dev, PTR_ERR(phy->usbcon),
+				     "failed to get usbcon\n");
+
+	generic_phy = devm_phy_create(dev, NULL, &exynos2200_snps_eusb2_phy_ops);
+	if (IS_ERR(generic_phy)) {
+		dev_err(dev, "failed to create phy %d\n", ret);
+		return PTR_ERR(generic_phy);
+	}
+
+	dev_set_drvdata(dev, phy);
+	phy_set_drvdata(generic_phy, phy);
+
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(phy_provider)) {
+		dev_err(dev, "failed to register phy provider\n");
+		return PTR_ERR(phy_provider);
+	};
+
+	return 0;
+}
+
+static const char * const exynos2200_clk_names[] = {
+	"ref", "apb", "ctrl",
+};
+
+static const char * const exynos2200_regulator_names[] = {
+	"vdd", "vdda12",
+};
+
+static const struct exynos2200_snps_eusb2_phy_drvdata exynos2200_snps_eusb2_phy = {
+	.clk_names		= exynos2200_clk_names,
+	.n_clks			= ARRAY_SIZE(exynos2200_clk_names),
+	.regulator_names	= exynos2200_regulator_names,
+	.n_regulators		= ARRAY_SIZE(exynos2200_regulator_names),
+};
+
+static const struct of_device_id exynos2200_snps_eusb2_phy_of_match_table[] = {
+	{
+		.compatible = "samsung,exynos2200-snps-eusb2-phy",
+		.data = &exynos2200_snps_eusb2_phy,
+	}, { },
+};
+MODULE_DEVICE_TABLE(of, exynos2200_snps_eusb2_phy_of_match_table);
+
+static struct platform_driver exynos2200_snps_eusb2_phy_driver = {
+	.probe		= exynos2200_snps_eusb2_phy_probe,
+	.driver = {
+		.name	= "exynos2200-snps-eusb2-hsphy",
+		.of_match_table = exynos2200_snps_eusb2_phy_of_match_table,
+	},
+};
+
+module_platform_driver(exynos2200_snps_eusb2_phy_driver);
+MODULE_DESCRIPTION("Exynos2200 SNPS eUSB2 PHY driver");
+MODULE_LICENSE("GPL");
-- 
2.43.0

[PATCH v1 4/4] phy: samsung: add Exynos2200 usb phy controller

[Ivaylo Ivanov]

The Exynos2200 SoC comes with 3 PHYs - snps eUSB2, snps USBDP combophy
and a cut-off phy that origins from exynos5-usbdrd. The latter is used
for link control, as well as pipe3 attachment and detachment.

Add a new driver for it.

Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
---
 drivers/phy/samsung/Kconfig                 |  13 ++
 drivers/phy/samsung/Makefile                |   1 +
 drivers/phy/samsung/phy-exynos2200-usbcon.c | 241 ++++++++++++++++++++
 include/linux/soc/samsung/exynos-regs-pmu.h |   3 +
 4 files changed, 258 insertions(+)
 create mode 100644 drivers/phy/samsung/phy-exynos2200-usbcon.c

diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig
index f62285254..47e9b9926 100644
--- a/drivers/phy/samsung/Kconfig
+++ b/drivers/phy/samsung/Kconfig
@@ -90,6 +90,19 @@ config PHY_EXYNOS2200_SNPS_EUSB2
 	  This driver provides PHY interface for eUSB 2.0 controller
 	  present on Exynos5 SoC series.
 
+config PHY_EXYNOS2200_USBCON
+	tristate "Exynos2200 USBCON PHY driver"
+	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
+	depends on HAS_IOMEM
+	depends on USB_DWC3_EXYNOS
+	select GENERIC_PHY
+	select MFD_SYSCON
+	default y
+	help
+	  Enable USBCON PHY support for Exynos2200 SoC.
+	  This driver provides PHY interface for USB controller present
+	  on Exynos2200 SoC.
+
 config PHY_EXYNOS5_USBDRD
 	tristate "Exynos5 SoC series USB DRD PHY driver"
 	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile
index 90b84c7fc..f70e12ddf 100644
--- a/drivers/phy/samsung/Makefile
+++ b/drivers/phy/samsung/Makefile
@@ -15,5 +15,6 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
 phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
 phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
 obj-$(CONFIG_PHY_EXYNOS2200_SNPS_EUSB2)	+= phy-exynos2200-snps-eusb2.o
+obj-$(CONFIG_PHY_EXYNOS2200_USBCON)	+= phy-exynos2200-usbcon.o
 obj-$(CONFIG_PHY_EXYNOS5_USBDRD)	+= phy-exynos5-usbdrd.o
 obj-$(CONFIG_PHY_EXYNOS5250_SATA)	+= phy-exynos5250-sata.o
diff --git a/drivers/phy/samsung/phy-exynos2200-usbcon.c b/drivers/phy/samsung/phy-exynos2200-usbcon.c
new file mode 100644
index 000000000..7968c9792
--- /dev/null
+++ b/drivers/phy/samsung/phy-exynos2200-usbcon.c
@@ -0,0 +1,241 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/iopoll.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mod_devicetable.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <linux/soc/samsung/exynos-regs-pmu.h>
+
+#define EXYNOS2200_USBCON_LINKCTRL		0x4
+#define LINKCTRL_FORCE_QACT			BIT(8)
+
+#define EXYNOS2200_USBCON_UTMI_CTRL		0x10
+#define UTMI_CTRL_FORCE_VBUSVALID		BIT(1)
+#define UTMI_CTRL_FORCE_BVALID			BIT(0)
+
+#define EXYNOS2200_USBCON_LINK_CLKRST		0xc
+#define LINK_CLKRST_SW_RST			BIT(0)
+
+struct exynos2200_usbcon_phy_drvdata {
+	const char * const *clk_names;
+	int n_clks;
+	const char * const *regulator_names;
+	int n_regulators;
+	u32 pmu_offset_usbcon_phy;
+};
+
+struct exynos2200_usbcon_phy {
+	struct phy *phy;
+	void __iomem *base;
+	struct regmap *reg_pmu;
+	struct clk_bulk_data *clks;
+	const struct exynos2200_usbcon_phy_drvdata *drv_data;
+	u32 pmu_offset;
+	struct regulator_bulk_data *vregs;
+};
+
+static void exynos2200_usbcon_phy_isol(struct exynos2200_usbcon_phy *inst,
+				       bool isolate)
+{
+	unsigned int val;
+
+	if (!inst->reg_pmu)
+		return;
+
+	val = isolate ? 0 : EXYNOS4_PHY_ENABLE;
+
+	regmap_update_bits(inst->reg_pmu, inst->pmu_offset,
+			   EXYNOS4_PHY_ENABLE, val);
+}
+
+static void exynos2200_usbcon_phy_write_mask(void __iomem *base, u32 offset,
+					     u32 mask, u32 val)
+{
+	u32 reg;
+
+	reg = readl_relaxed(base + offset);
+	reg &= ~mask;
+	reg |= val & mask;
+	writel_relaxed(reg, base + offset);
+
+	/* Ensure above write is completed */
+	readl_relaxed(base + offset);
+}
+
+static int exynos2200_usbcon_phy_init(struct phy *p)
+{
+	struct exynos2200_usbcon_phy *phy = phy_get_drvdata(p);
+	int ret;
+
+	ret = regulator_bulk_enable(phy->drv_data->n_regulators, phy->vregs);
+	if (ret)
+		return ret;
+
+	exynos2200_usbcon_phy_isol(phy, false);
+
+	/*
+	 * Disable HWACG (hardware auto clock gating control). This will force
+	 * QACTIVE signal in Q-Channel interface to HIGH level, to make sure
+	 * the PHY clock is not gated by the hardware.
+	 */
+	exynos2200_usbcon_phy_write_mask(phy->base, EXYNOS2200_USBCON_LINKCTRL,
+					 LINKCTRL_FORCE_QACT,
+					 LINKCTRL_FORCE_QACT);
+
+	/* Reset Link */
+	exynos2200_usbcon_phy_write_mask(phy->base,
+					 EXYNOS2200_USBCON_LINK_CLKRST,
+					 LINK_CLKRST_SW_RST,
+					 LINK_CLKRST_SW_RST);
+
+	fsleep(10); /* required after POR high */
+	exynos2200_usbcon_phy_write_mask(phy->base,
+					 EXYNOS2200_USBCON_LINK_CLKRST,
+					 LINK_CLKRST_SW_RST, 0);
+
+	exynos2200_usbcon_phy_write_mask(phy->base,
+					 EXYNOS2200_USBCON_UTMI_CTRL,
+					 UTMI_CTRL_FORCE_BVALID |
+					 UTMI_CTRL_FORCE_VBUSVALID,
+					 UTMI_CTRL_FORCE_BVALID |
+					 UTMI_CTRL_FORCE_VBUSVALID);
+
+	return 0;
+}
+
+static int exynos2200_usbcon_phy_exit(struct phy *p)
+{
+	struct exynos2200_usbcon_phy *phy = phy_get_drvdata(p);
+
+	exynos2200_usbcon_phy_isol(phy, true);
+
+	regulator_bulk_disable(phy->drv_data->n_regulators, phy->vregs);
+
+	return 0;
+}
+
+static const struct phy_ops exynos2200_usbcon_phy_ops = {
+	.init		= exynos2200_usbcon_phy_init,
+	.exit		= exynos2200_usbcon_phy_exit,
+	.owner		= THIS_MODULE,
+};
+
+static int exynos2200_usbcon_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct exynos2200_usbcon_phy *phy;
+	const struct exynos2200_usbcon_phy_drvdata *drv_data;
+	struct phy_provider *phy_provider;
+	struct phy *generic_phy;
+	int ret;
+
+	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	drv_data = of_device_get_match_data(dev);
+	if (!drv_data)
+		return -EINVAL;
+	phy->drv_data = drv_data;
+
+	phy->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(phy->base))
+		return PTR_ERR(phy->base);
+
+	phy->clks = devm_kcalloc(dev, drv_data->n_clks,
+				 sizeof(*phy->clks), GFP_KERNEL);
+	if (!phy->clks)
+		return -ENOMEM;
+
+	for (int i = 0; i < drv_data->n_clks; ++i)
+		phy->clks[i].id = drv_data->clk_names[i];
+
+	ret = devm_clk_bulk_get(dev, phy->drv_data->n_clks,
+				phy->clks);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to get phy clock(s)\n");
+
+	phy->reg_pmu = syscon_regmap_lookup_by_phandle(dev->of_node,
+						       "samsung,pmu-syscon");
+	if (IS_ERR(phy->reg_pmu)) {
+		dev_err(dev, "Failed to lookup PMU regmap\n");
+		return PTR_ERR(phy->reg_pmu);
+	}
+
+	phy->pmu_offset = drv_data->pmu_offset_usbcon_phy;
+	phy->vregs = devm_kcalloc(dev, drv_data->n_regulators,
+				  sizeof(*phy->vregs), GFP_KERNEL);
+	if (!phy->vregs)
+		return -ENOMEM;
+	regulator_bulk_set_supply_names(phy->vregs,
+					drv_data->regulator_names,
+					drv_data->n_regulators);
+	ret = devm_regulator_bulk_get(dev, drv_data->n_regulators,
+				      phy->vregs);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to get regulators\n");
+
+	generic_phy = devm_phy_create(dev, NULL, &exynos2200_usbcon_phy_ops);
+	if (IS_ERR(generic_phy)) {
+		dev_err(dev, "failed to create phy %d\n", ret);
+		return PTR_ERR(generic_phy);
+	}
+
+	dev_set_drvdata(dev, phy);
+	phy_set_drvdata(generic_phy, phy);
+
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(phy_provider)) {
+		dev_err(dev, "failed to register phy provider\n");
+		return PTR_ERR(phy_provider);
+	};
+
+	return 0;
+}
+
+static const char * const exynos2200_clk_names[] = {
+	"apb",
+};
+
+static const char * const exynos2200_regulator_names[] = {
+	"vdd1p9", "vdd3",
+};
+
+static const struct exynos2200_usbcon_phy_drvdata exynos2200_usbcon_phy = {
+	.clk_names		= exynos2200_clk_names,
+	.n_clks			= ARRAY_SIZE(exynos2200_clk_names),
+	.pmu_offset_usbcon_phy	= EXYNOS2200_PHY_CTRL_USB20,
+	.regulator_names	= exynos2200_regulator_names,
+	.n_regulators		= ARRAY_SIZE(exynos2200_regulator_names),
+};
+
+static const struct of_device_id exynos2200_usbcon_phy_of_match_table[] = {
+	{
+		.compatible = "samsung,exynos2200-usbcon-phy",
+		.data = &exynos2200_usbcon_phy,
+	}, { },
+};
+MODULE_DEVICE_TABLE(of, exynos2200_usbcon_phy_of_match_table);
+
+static struct platform_driver exynos2200_usbcon_phy_driver = {
+	.probe		= exynos2200_usbcon_phy_probe,
+	.driver = {
+		.name	= "exynos2200-usbcon-phy",
+		.of_match_table = exynos2200_usbcon_phy_of_match_table,
+	},
+};
+
+module_platform_driver(exynos2200_usbcon_phy_driver);
+MODULE_DESCRIPTION("Exynos2200 USBCON PHY driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/soc/samsung/exynos-regs-pmu.h b/include/linux/soc/samsung/exynos-regs-pmu.h
index ce1a3790d..b77187ba5 100644
--- a/include/linux/soc/samsung/exynos-regs-pmu.h
+++ b/include/linux/soc/samsung/exynos-regs-pmu.h
@@ -185,6 +185,9 @@
 /* Only for S5Pv210 */
 #define S5PV210_EINT_WAKEUP_MASK	0xC004
 
+/* Only for Exynos2200 */
+#define EXYNOS2200_PHY_CTRL_USB20	0x72C
+
 /* Only for Exynos4210 */
 #define S5P_CMU_CLKSTOP_LCD1_LOWPWR	0x1154
 #define S5P_CMU_RESET_LCD1_LOWPWR	0x1174
-- 
2.43.0

Re: [PATCH v1 1/4] dt-bindings: phy: add samsung,exynos2200-snps-eusb2-phy schema file

[Rob Herring (Arm)]

On Sat, 15 Feb 2025 14:24:06 +0200, Ivaylo Ivanov wrote:
> The Exynos2200 SoC uses Synopsis eUSB2 PHY. Add a dt-binding schema
> for the new driver.
> 
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
>  .../samsung,exynos2200-snps-eusb2-phy.yaml    | 75 +++++++++++++++++++
>  1 file changed, 75 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-snps-eusb2-phy.yaml
> 

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

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/phy/samsung,exynos2200-snps-eusb2-phy.example.dts:18:18: fatal error: dt-bindings/clock/samsung,exynos2200.h: No such file or directory
   18 |         #include <dt-bindings/clock/samsung,exynos2200.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [scripts/Makefile.dtbs:131: Documentation/devicetree/bindings/phy/samsung,exynos2200-snps-eusb2-phy.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1511: dt_binding_check] Error 2
make: *** [Makefile:251: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250215122409.162810-2-ivo.ivanov.ivanov1@gmail.com

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.

Re: [PATCH v1 2/4] dt-bindings: phy: add samsung,exynos2200-usbcon-phy schema file

[Rob Herring (Arm)]

On Sat, 15 Feb 2025 14:24:07 +0200, Ivaylo Ivanov wrote:
> The Exynos2200 SoC has a USB phy controller block that controls the
> usage of USB phys. Add a dt-binding schema for the new driver.
> 
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
>  .../phy/samsung,exynos2200-usbcon-phy.yaml    | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
> 

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

yamllint warnings/errors:

dtschema/dtc warnings/errors:


doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250215122409.162810-3-ivo.ivanov.ivanov1@gmail.com

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.

Re: [PATCH v1 1/4] dt-bindings: phy: add samsung,exynos2200-snps-eusb2-phy schema file

[Diederik de Haas]

[-- Attachment #1: Type: text/plain, Size: 2775 bytes --]

On Sat Feb 15, 2025 at 1:24 PM CET, Ivaylo Ivanov wrote:
> The Exynos2200 SoC uses Synopsis eUSB2 PHY. Add a dt-binding schema
> for the new driver.
>
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
>  .../samsung,exynos2200-snps-eusb2-phy.yaml    | 75 +++++++++++++++++++
>  1 file changed, 75 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-snps-eusb2-phy.yaml
>
> diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos2200-snps-eusb2-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos2200-snps-eusb2-phy.yaml
> new file mode 100644
> index 000000000..d69a10f00
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/samsung,exynos2200-snps-eusb2-phy.yaml
> @@ -0,0 +1,75 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/samsung,exynos2200-snps-eusb2-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung SNPS eUSB2 phy controller
> +
> +maintainers:
> +  - Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> +
> +description:
> +  eUSB2 controller supports LS/FS/HS usb connectivity on Exynos chipsets.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - samsung,exynos2200-snps-eusb2-phy
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#phy-cells":
> +    const: 0
> +
> +  clocks:
> +    items:
> +      - description: Reference clock
> +      - description: APB clock
> +      - description: Control PCLK
> +
> +  clock-names:
> +    items:
> +      - const: ref
> +      - const: apb
> +      - const: ctrl
> +
> +  phys:
> +    maxItems: 1
> +    description:
> +      Phandle to USBCON phy
> +
> +  vdd-supply:
> +    description:
> +      Phandle to 0.88V regulator supply
> +
> +  vdda12-supply:
> +    description:
> +      Phandle to 1.2V regulator supply
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#phy-cells"
> +  - clocks
> +  - clock-names
> +  - vdd-supply
> +  - vdda12-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/samsung,exynos2200.h>
> +
> +    usb_hsphy: phy@10ab0000 {
> +        compatible = "samsung,exynos2200-snps-eusb2-phy";
> +        reg = <0 0x10ab0000 0 0x10000>;
> +        clocks = <&cmu_hsi0 CLK_MOUT_HSI0_USB32DRD>,
> +                 <&cmu_hsi0 CLK_MOUT_HSI0_NOC>,
> +                 <&cmu_hsi0 CLK_DOUT_DIV_CLK_HSI0_EUSB>;
> +        clock-names = "ref", "apb", "ctrl";
> +        #phy-cells = <0>;
> +        phys = <&usbcon_phy>;
> +    };

Shouldn't the example have at least all the *required* properties?
Same for patch 2 of this series.

Cheers,
  Diederik


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

Re: [PATCH v1 1/4] dt-bindings: phy: add samsung,exynos2200-snps-eusb2-phy schema file

[Krzysztof Kozlowski]

On 16/02/2025 10:14, Diederik de Haas wrote:
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/samsung,exynos2200.h>
>> +
>> +    usb_hsphy: phy@10ab0000 {
>> +        compatible = "samsung,exynos2200-snps-eusb2-phy";
>> +        reg = <0 0x10ab0000 0 0x10000>;
>> +        clocks = <&cmu_hsi0 CLK_MOUT_HSI0_USB32DRD>,
>> +                 <&cmu_hsi0 CLK_MOUT_HSI0_NOC>,
>> +                 <&cmu_hsi0 CLK_DOUT_DIV_CLK_HSI0_EUSB>;
>> +        clock-names = "ref", "apb", "ctrl";
>> +        #phy-cells = <0>;
>> +        phys = <&usbcon_phy>;
>> +    };
> 
> Shouldn't the example have at least all the *required* properties?
> Same for patch 2 of this series.


Yeah, this wasn't ever tested.

Best regards,
Krzysztof

Re: [PATCH v1 3/4] phy: samsung: add Exynos2200 SNPS eUSB2 driver

[Krzysztof Kozlowski]

On 15/02/2025 13:24, Ivaylo Ivanov wrote:
> The Exynos2200 SoC uses Synopsis eUSB2 PHY for USB 2.0. Add a new
> driver for it.
> 
> eUSB2 on Exynos SoCs is usually paired alongside a USB PHY controller.
> Currently the driver is modelled to take and enable/disable the usb phy
> controller when needed.
> 
> The driver is based on information from downstream drivers.
> 
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
>  drivers/phy/samsung/Kconfig                   |  13 +
>  drivers/phy/samsung/Makefile                  |   1 +
>  .../phy/samsung/phy-exynos2200-snps-eusb2.c   | 351 ++++++++++++++++++
>  3 files changed, 365 insertions(+)
>  create mode 100644 drivers/phy/samsung/phy-exynos2200-snps-eusb2.c
> 
> diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig
> index e2330b089..f62285254 100644
> --- a/drivers/phy/samsung/Kconfig
> +++ b/drivers/phy/samsung/Kconfig
> @@ -77,6 +77,19 @@ config PHY_S5PV210_USB2
>  	  particular SoC is compiled in the driver. In case of S5PV210 two phys
>  	  are available - device and host.
>  
> +config PHY_EXYNOS2200_SNPS_EUSB2
> +	tristate "Exynos2200 eUSB 2.0 PHY driver"
> +	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
> +	depends on HAS_IOMEM
> +	depends on USB_DWC3_EXYNOS


How does it depend? What are you using from DWC3?

> +	select GENERIC_PHY
> +	select MFD_SYSCON

Where do you use it?

> +	default y
> +	help
> +	  Enable USBCON PHY support for Exynos2200 SoC.
> +	  This driver provides PHY interface for eUSB 2.0 controller
> +	  present on Exynos5 SoC series.
> +
>  config PHY_EXYNOS5_USBDRD
>  	tristate "Exynos5 SoC series USB DRD PHY driver"
>  	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
> diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile
> index fea1f96d0..90b84c7fc 100644
> --- a/drivers/phy/samsung/Makefile
> +++ b/drivers/phy/samsung/Makefile
> @@ -14,5 +14,6 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2)	+= phy-exynos4210-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
> +obj-$(CONFIG_PHY_EXYNOS2200_SNPS_EUSB2)	+= phy-exynos2200-snps-eusb2.o

Entire driver looks like repeating existing qcom-snps-eusb2. You need to
integrate the changes, not create duplicated driver.

...

> +
> +	ret = devm_clk_bulk_get(dev, drv_data->n_clks,
> +				phy->clks);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to get phy clock(s)\n");
> +
> +	for (int i = 0; i < phy->drv_data->n_clks; ++i) {
> +		if (!strcmp(phy->clks[i].id, "ref")) {
> +			phy->ref_clk = phy->clks[i].clk;
> +			break;
> +		}
> +	}
> +
> +	phy->vregs = devm_kcalloc(dev, drv_data->n_regulators,
> +				  sizeof(*phy->vregs), GFP_KERNEL);
> +	if (!phy->vregs)
> +		return -ENOMEM;
> +	regulator_bulk_set_supply_names(phy->vregs,
> +					drv_data->regulator_names,
> +					drv_data->n_regulators);
> +	ret = devm_regulator_bulk_get(dev, drv_data->n_regulators,
> +				      phy->vregs);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to get regulators\n");
> +
> +	/* we treat the usblink controller phy as a separate phy */
> +	phy->usbcon = devm_of_phy_get_by_index(dev, np, 0);
> +	if (IS_ERR(phy->usbcon))
> +		return dev_err_probe(dev, PTR_ERR(phy->usbcon),
> +				     "failed to get usbcon\n");
> +
> +	generic_phy = devm_phy_create(dev, NULL, &exynos2200_snps_eusb2_phy_ops);
> +	if (IS_ERR(generic_phy)) {
> +		dev_err(dev, "failed to create phy %d\n", ret);


Syntax is return dev_err_probe

> +		return PTR_ERR(generic_phy);
> +	}
> +
> +	dev_set_drvdata(dev, phy);
> +	phy_set_drvdata(generic_phy, phy);
> +
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	if (IS_ERR(phy_provider)) {
> +		dev_err(dev, "failed to register phy provider\n");


Syntax is return dev_err_probe

> +		return PTR_ERR(phy_provider);
> +	};
> +
> +	return 0;
> +}
Best regards,
Krzysztof

Re: [PATCH v1 1/4] dt-bindings: phy: add samsung,exynos2200-snps-eusb2-phy schema file

[Ivaylo Ivanov]

On 2/16/25 11:22, Krzysztof Kozlowski wrote:
> On 16/02/2025 10:14, Diederik de Haas wrote:
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/clock/samsung,exynos2200.h>
>>> +
>>> +    usb_hsphy: phy@10ab0000 {
>>> +        compatible = "samsung,exynos2200-snps-eusb2-phy";
>>> +        reg = <0 0x10ab0000 0 0x10000>;
>>> +        clocks = <&cmu_hsi0 CLK_MOUT_HSI0_USB32DRD>,
>>> +                 <&cmu_hsi0 CLK_MOUT_HSI0_NOC>,
>>> +                 <&cmu_hsi0 CLK_DOUT_DIV_CLK_HSI0_EUSB>;
>>> +        clock-names = "ref", "apb", "ctrl";
>>> +        #phy-cells = <0>;
>>> +        phys = <&usbcon_phy>;
>>> +    };
>> Shouldn't the example have at least all the *required* properties?
>> Same for patch 2 of this series.
>
> Yeah, this wasn't ever tested.

Device trees were tested with dtbs_check W=1 but I overlooked testing bindings
with dt_bindings_check. Anyways this is rather a small problem, will be fixed
in a v2.

Best regards,
Ivaylo

>
> Best regards,
> Krzysztof

Re: [PATCH v1 4/4] phy: samsung: add Exynos2200 usb phy controller

[Krzysztof Kozlowski]

On 15/02/2025 13:24, Ivaylo Ivanov wrote:
> The Exynos2200 SoC comes with 3 PHYs - snps eUSB2, snps USBDP combophy
> and a cut-off phy that origins from exynos5-usbdrd. The latter is used
> for link control, as well as pipe3 attachment and detachment.
> 
> Add a new driver for it.
> 
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
>  drivers/phy/samsung/Kconfig                 |  13 ++
>  drivers/phy/samsung/Makefile                |   1 +
>  drivers/phy/samsung/phy-exynos2200-usbcon.c | 241 ++++++++++++++++++++
>  include/linux/soc/samsung/exynos-regs-pmu.h |   3 +
>  4 files changed, 258 insertions(+)
>  create mode 100644 drivers/phy/samsung/phy-exynos2200-usbcon.c
> 
> diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig
> index f62285254..47e9b9926 100644
> --- a/drivers/phy/samsung/Kconfig
> +++ b/drivers/phy/samsung/Kconfig
> @@ -90,6 +90,19 @@ config PHY_EXYNOS2200_SNPS_EUSB2
>  	  This driver provides PHY interface for eUSB 2.0 controller
>  	  present on Exynos5 SoC series.
>  
> +config PHY_EXYNOS2200_USBCON
> +	tristate "Exynos2200 USBCON PHY driver"
> +	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
> +	depends on HAS_IOMEM
> +	depends on USB_DWC3_EXYNOS

How? What are you using from DWC3?

> +	select GENERIC_PHY
> +	select MFD_SYSCON
> +	default y
> +	help
> +	  Enable USBCON PHY support for Exynos2200 SoC.
> +	  This driver provides PHY interface for USB controller present
> +	  on Exynos2200 SoC.
> +
>  config PHY_EXYNOS5_USBDRD
>  	tristate "Exynos5 SoC series USB DRD PHY driver"
>  	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
> diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile
> index 90b84c7fc..f70e12ddf 100644
> --- a/drivers/phy/samsung/Makefile
> +++ b/drivers/phy/samsung/Makefile
> @@ -15,5 +15,6 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
>  obj-$(CONFIG_PHY_EXYNOS2200_SNPS_EUSB2)	+= phy-exynos2200-snps-eusb2.o
> +obj-$(CONFIG_PHY_EXYNOS2200_USBCON)	+= phy-exynos2200-usbcon.o
>  obj-$(CONFIG_PHY_EXYNOS5_USBDRD)	+= phy-exynos5-usbdrd.o
>  obj-$(CONFIG_PHY_EXYNOS5250_SATA)	+= phy-exynos5250-sata.o
> diff --git a/drivers/phy/samsung/phy-exynos2200-usbcon.c b/drivers/phy/samsung/phy-exynos2200-usbcon.c
> new file mode 100644
> index 000000000..7968c9792
> --- /dev/null
> +++ b/drivers/phy/samsung/phy-exynos2200-usbcon.c
> @@ -0,0 +1,241 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025, Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> + */
> +
> +#include <linux/bitfield.h>

Are you using this header?

> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/iopoll.h>

And rhis?

> +#include <linux/mfd/syscon.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>

And this?

> +#include <linux/soc/samsung/exynos-regs-pmu.h>
> +
> +#define EXYNOS2200_USBCON_LINKCTRL		0x4
> +#define LINKCTRL_FORCE_QACT			BIT(8)
> +
> +#define EXYNOS2200_USBCON_UTMI_CTRL		0x10
> +#define UTMI_CTRL_FORCE_VBUSVALID		BIT(1)
> +#define UTMI_CTRL_FORCE_BVALID			BIT(0)
> +
> +#define EXYNOS2200_USBCON_LINK_CLKRST		0xc
> +#define LINK_CLKRST_SW_RST			BIT(0)
> +
> +struct exynos2200_usbcon_phy_drvdata {
> +	const char * const *clk_names;
> +	int n_clks;
> +	const char * const *regulator_names;
> +	int n_regulators;
> +	u32 pmu_offset_usbcon_phy;
> +};
> +
> +struct exynos2200_usbcon_phy {
> +	struct phy *phy;
> +	void __iomem *base;
> +	struct regmap *reg_pmu;
> +	struct clk_bulk_data *clks;
> +	const struct exynos2200_usbcon_phy_drvdata *drv_data;
> +	u32 pmu_offset;
> +	struct regulator_bulk_data *vregs;
> +};
> +
> +static void exynos2200_usbcon_phy_isol(struct exynos2200_usbcon_phy *inst,
> +				       bool isolate)
> +{
> +	unsigned int val;
> +
> +	if (!inst->reg_pmu)
> +		return;
> +
> +	val = isolate ? 0 : EXYNOS4_PHY_ENABLE;
> +
> +	regmap_update_bits(inst->reg_pmu, inst->pmu_offset,
> +			   EXYNOS4_PHY_ENABLE, val);
> +}
> +
> +static void exynos2200_usbcon_phy_write_mask(void __iomem *base, u32 offset,
> +					     u32 mask, u32 val)
> +{
> +	u32 reg;
> +
> +	reg = readl_relaxed(base + offset);
> +	reg &= ~mask;
> +	reg |= val & mask;
> +	writel_relaxed(reg, base + offset);
> +
> +	/* Ensure above write is completed */
> +	readl_relaxed(base + offset);

None of these should be relaxed.

> +}
> +
> +static int exynos2200_usbcon_phy_init(struct phy *p)
> +{
> +	struct exynos2200_usbcon_phy *phy = phy_get_drvdata(p);
> +	int ret;
> +
> +	ret = regulator_bulk_enable(phy->drv_data->n_regulators, phy->vregs);
> +	if (ret)
> +		return ret;
> +
> +	exynos2200_usbcon_phy_isol(phy, false);
> +
> +	/*
> +	 * Disable HWACG (hardware auto clock gating control). This will force
> +	 * QACTIVE signal in Q-Channel interface to HIGH level, to make sure
> +	 * the PHY clock is not gated by the hardware.
> +	 */
> +	exynos2200_usbcon_phy_write_mask(phy->base, EXYNOS2200_USBCON_LINKCTRL,
> +					 LINKCTRL_FORCE_QACT,
> +					 LINKCTRL_FORCE_QACT);
> +
> +	/* Reset Link */
> +	exynos2200_usbcon_phy_write_mask(phy->base,
> +					 EXYNOS2200_USBCON_LINK_CLKRST,
> +					 LINK_CLKRST_SW_RST,
> +					 LINK_CLKRST_SW_RST);
> +
> +	fsleep(10); /* required after POR high */
> +	exynos2200_usbcon_phy_write_mask(phy->base,
> +					 EXYNOS2200_USBCON_LINK_CLKRST,
> +					 LINK_CLKRST_SW_RST, 0);
> +
> +	exynos2200_usbcon_phy_write_mask(phy->base,
> +					 EXYNOS2200_USBCON_UTMI_CTRL,
> +					 UTMI_CTRL_FORCE_BVALID |
> +					 UTMI_CTRL_FORCE_VBUSVALID,
> +					 UTMI_CTRL_FORCE_BVALID |
> +					 UTMI_CTRL_FORCE_VBUSVALID);
> +
> +	return 0;
> +}
> +
> +static int exynos2200_usbcon_phy_exit(struct phy *p)
> +{
> +	struct exynos2200_usbcon_phy *phy = phy_get_drvdata(p);
> +
> +	exynos2200_usbcon_phy_isol(phy, true);
> +
> +	regulator_bulk_disable(phy->drv_data->n_regulators, phy->vregs);


This looks like power off callback, not exit.

> +
> +	return 0;
> +}
> +
> +static const struct phy_ops exynos2200_usbcon_phy_ops = {
> +	.init		= exynos2200_usbcon_phy_init,
> +	.exit		= exynos2200_usbcon_phy_exit,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static int exynos2200_usbcon_phy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct exynos2200_usbcon_phy *phy;
> +	const struct exynos2200_usbcon_phy_drvdata *drv_data;
> +	struct phy_provider *phy_provider;
> +	struct phy *generic_phy;
> +	int ret;
> +
> +	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
> +	if (!phy)
> +		return -ENOMEM;
> +
> +	drv_data = of_device_get_match_data(dev);
> +	if (!drv_data)
> +		return -EINVAL;
> +	phy->drv_data = drv_data;
> +
> +	phy->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(phy->base))
> +		return PTR_ERR(phy->base);
> +
> +	phy->clks = devm_kcalloc(dev, drv_data->n_clks,
> +				 sizeof(*phy->clks), GFP_KERNEL);
> +	if (!phy->clks)
> +		return -ENOMEM;
> +
> +	for (int i = 0; i < drv_data->n_clks; ++i)
> +		phy->clks[i].id = drv_data->clk_names[i];
> +
> +	ret = devm_clk_bulk_get(dev, phy->drv_data->n_clks,
> +				phy->clks);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to get phy clock(s)\n");
> +
> +	phy->reg_pmu = syscon_regmap_lookup_by_phandle(dev->of_node,
> +						       "samsung,pmu-syscon");

syscon_regmap_lookup_by_phandle_args

> +	if (IS_ERR(phy->reg_pmu)) {
> +		dev_err(dev, "Failed to lookup PMU regmap\n");
> +		return PTR_ERR(phy->reg_pmu);
> +	}
> +
> +	phy->pmu_offset = drv_data->pmu_offset_usbcon_phy;
> +	phy->vregs = devm_kcalloc(dev, drv_data->n_regulators,
> +				  sizeof(*phy->vregs), GFP_KERNEL);
> +	if (!phy->vregs)
> +		return -ENOMEM;
> +	regulator_bulk_set_supply_names(phy->vregs,
> +					drv_data->regulator_names,
> +					drv_data->n_regulators);
> +	ret = devm_regulator_bulk_get(dev, drv_data->n_regulators,
> +				      phy->vregs);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to get regulators\n");
> +
> +	generic_phy = devm_phy_create(dev, NULL, &exynos2200_usbcon_phy_ops);
> +	if (IS_ERR(generic_phy)) {
> +		dev_err(dev, "failed to create phy %d\n", ret);
> +		return PTR_ERR(generic_phy);
> +	}
> +
> +	dev_set_drvdata(dev, phy);
> +	phy_set_drvdata(generic_phy, phy);
> +
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	if (IS_ERR(phy_provider)) {
> +		dev_err(dev, "failed to register phy provider\n");
> +		return PTR_ERR(phy_provider);


Same comments as on previous patch.



Best regards,
Krzysztof

Re: [PATCH v1 1/4] dt-bindings: phy: add samsung,exynos2200-snps-eusb2-phy schema file

[Krzysztof Kozlowski]

On 16/02/2025 10:27, Ivaylo Ivanov wrote:
> On 2/16/25 11:22, Krzysztof Kozlowski wrote:
>> On 16/02/2025 10:14, Diederik de Haas wrote:
>>>> +examples:
>>>> +  - |
>>>> +    #include <dt-bindings/clock/samsung,exynos2200.h>
>>>> +
>>>> +    usb_hsphy: phy@10ab0000 {
>>>> +        compatible = "samsung,exynos2200-snps-eusb2-phy";
>>>> +        reg = <0 0x10ab0000 0 0x10000>;
>>>> +        clocks = <&cmu_hsi0 CLK_MOUT_HSI0_USB32DRD>,
>>>> +                 <&cmu_hsi0 CLK_MOUT_HSI0_NOC>,
>>>> +                 <&cmu_hsi0 CLK_DOUT_DIV_CLK_HSI0_EUSB>;
>>>> +        clock-names = "ref", "apb", "ctrl";
>>>> +        #phy-cells = <0>;
>>>> +        phys = <&usbcon_phy>;
>>>> +    };
>>> Shouldn't the example have at least all the *required* properties?
>>> Same for patch 2 of this series.
>>
>> Yeah, this wasn't ever tested.
> 
> Device trees were tested with dtbs_check W=1 but I overlooked testing bindings
> with dt_bindings_check. Anyways this is rather a small problem, will be fixed
> in a v2.
> 

This makes the bindings unreviewable and they will be marked as "changes
requested".

Best regards,
Krzysztof

Re: [PATCH v1 3/4] phy: samsung: add Exynos2200 SNPS eUSB2 driver

[Ivaylo Ivanov]

On 2/16/25 11:26, Krzysztof Kozlowski wrote:
> On 15/02/2025 13:24, Ivaylo Ivanov wrote:
>> The Exynos2200 SoC uses Synopsis eUSB2 PHY for USB 2.0. Add a new
>> driver for it.
>>
>> eUSB2 on Exynos SoCs is usually paired alongside a USB PHY controller.
>> Currently the driver is modelled to take and enable/disable the usb phy
>> controller when needed.
>>
>> The driver is based on information from downstream drivers.
>>
>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>> ---
>>  drivers/phy/samsung/Kconfig                   |  13 +
>>  drivers/phy/samsung/Makefile                  |   1 +
>>  .../phy/samsung/phy-exynos2200-snps-eusb2.c   | 351 ++++++++++++++++++
>>  3 files changed, 365 insertions(+)
>>  create mode 100644 drivers/phy/samsung/phy-exynos2200-snps-eusb2.c
>>
>> diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig
>> index e2330b089..f62285254 100644
>> --- a/drivers/phy/samsung/Kconfig
>> +++ b/drivers/phy/samsung/Kconfig
>> @@ -77,6 +77,19 @@ config PHY_S5PV210_USB2
>>  	  particular SoC is compiled in the driver. In case of S5PV210 two phys
>>  	  are available - device and host.
>>  
>> +config PHY_EXYNOS2200_SNPS_EUSB2
>> +	tristate "Exynos2200 eUSB 2.0 PHY driver"
>> +	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
>> +	depends on HAS_IOMEM
>> +	depends on USB_DWC3_EXYNOS
>
> How does it depend? What are you using from DWC3?

Can drop, I guess.

>
>> +	select GENERIC_PHY
>> +	select MFD_SYSCON
> Where do you use it?

Remained from USBCON driver.

>
>> +	default y
>> +	help
>> +	  Enable USBCON PHY support for Exynos2200 SoC.
>> +	  This driver provides PHY interface for eUSB 2.0 controller
>> +	  present on Exynos5 SoC series.
>> +
>>  config PHY_EXYNOS5_USBDRD
>>  	tristate "Exynos5 SoC series USB DRD PHY driver"
>>  	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
>> diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile
>> index fea1f96d0..90b84c7fc 100644
>> --- a/drivers/phy/samsung/Makefile
>> +++ b/drivers/phy/samsung/Makefile
>> @@ -14,5 +14,6 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2)	+= phy-exynos4210-usb2.o
>>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
>>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
>>  phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
>> +obj-$(CONFIG_PHY_EXYNOS2200_SNPS_EUSB2)	+= phy-exynos2200-snps-eusb2.o
> Entire driver looks like repeating existing qcom-snps-eusb2.

It's the same IP, but implemented differently on a different platform. At
the very least, the register layout is different.

>  You need to
> integrate the changes, not create duplicated driver.

I can do that, but it would be come a bit cluttered, won't it? Depends on
if we want to follow the current oem-provided initialization sequence, or
try and fully reuse what we have in there.

Best regards,
Ivaylo

>
> ...
>
>> +
>> +	ret = devm_clk_bulk_get(dev, drv_data->n_clks,
>> +				phy->clks);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret,
>> +				     "failed to get phy clock(s)\n");
>> +
>> +	for (int i = 0; i < phy->drv_data->n_clks; ++i) {
>> +		if (!strcmp(phy->clks[i].id, "ref")) {
>> +			phy->ref_clk = phy->clks[i].clk;
>> +			break;
>> +		}
>> +	}
>> +
>> +	phy->vregs = devm_kcalloc(dev, drv_data->n_regulators,
>> +				  sizeof(*phy->vregs), GFP_KERNEL);
>> +	if (!phy->vregs)
>> +		return -ENOMEM;
>> +	regulator_bulk_set_supply_names(phy->vregs,
>> +					drv_data->regulator_names,
>> +					drv_data->n_regulators);
>> +	ret = devm_regulator_bulk_get(dev, drv_data->n_regulators,
>> +				      phy->vregs);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "failed to get regulators\n");
>> +
>> +	/* we treat the usblink controller phy as a separate phy */
>> +	phy->usbcon = devm_of_phy_get_by_index(dev, np, 0);
>> +	if (IS_ERR(phy->usbcon))
>> +		return dev_err_probe(dev, PTR_ERR(phy->usbcon),
>> +				     "failed to get usbcon\n");
>> +
>> +	generic_phy = devm_phy_create(dev, NULL, &exynos2200_snps_eusb2_phy_ops);
>> +	if (IS_ERR(generic_phy)) {
>> +		dev_err(dev, "failed to create phy %d\n", ret);
>
> Syntax is return dev_err_probe
>
>> +		return PTR_ERR(generic_phy);
>> +	}
>> +
>> +	dev_set_drvdata(dev, phy);
>> +	phy_set_drvdata(generic_phy, phy);
>> +
>> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>> +	if (IS_ERR(phy_provider)) {
>> +		dev_err(dev, "failed to register phy provider\n");
>
> Syntax is return dev_err_probe
>
>> +		return PTR_ERR(phy_provider);
>> +	};
>> +
>> +	return 0;
>> +}
> Best regards,
> Krzysztof

Re: [PATCH v1 3/4] phy: samsung: add Exynos2200 SNPS eUSB2 driver

[Krzysztof Kozlowski]

On 16/02/2025 10:41, Ivaylo Ivanov wrote:
> On 2/16/25 11:26, Krzysztof Kozlowski wrote:
>> On 15/02/2025 13:24, Ivaylo Ivanov wrote:
>>> The Exynos2200 SoC uses Synopsis eUSB2 PHY for USB 2.0. Add a new
>>> driver for it.
>>>
>>> eUSB2 on Exynos SoCs is usually paired alongside a USB PHY controller.
>>> Currently the driver is modelled to take and enable/disable the usb phy
>>> controller when needed.
>>>
>>> The driver is based on information from downstream drivers.
>>>
>>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>> ---
>>>  drivers/phy/samsung/Kconfig                   |  13 +
>>>  drivers/phy/samsung/Makefile                  |   1 +
>>>  .../phy/samsung/phy-exynos2200-snps-eusb2.c   | 351 ++++++++++++++++++
>>>  3 files changed, 365 insertions(+)
>>>  create mode 100644 drivers/phy/samsung/phy-exynos2200-snps-eusb2.c
>>>
>>> diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig
>>> index e2330b089..f62285254 100644
>>> --- a/drivers/phy/samsung/Kconfig
>>> +++ b/drivers/phy/samsung/Kconfig
>>> @@ -77,6 +77,19 @@ config PHY_S5PV210_USB2
>>>  	  particular SoC is compiled in the driver. In case of S5PV210 two phys
>>>  	  are available - device and host.
>>>  
>>> +config PHY_EXYNOS2200_SNPS_EUSB2
>>> +	tristate "Exynos2200 eUSB 2.0 PHY driver"
>>> +	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
>>> +	depends on HAS_IOMEM
>>> +	depends on USB_DWC3_EXYNOS
>>
>> How does it depend? What are you using from DWC3?
> 
> Can drop, I guess.
> 
>>
>>> +	select GENERIC_PHY
>>> +	select MFD_SYSCON
>> Where do you use it?
> 
> Remained from USBCON driver.
> 
>>
>>> +	default y
>>> +	help
>>> +	  Enable USBCON PHY support for Exynos2200 SoC.
>>> +	  This driver provides PHY interface for eUSB 2.0 controller
>>> +	  present on Exynos5 SoC series.
>>> +
>>>  config PHY_EXYNOS5_USBDRD
>>>  	tristate "Exynos5 SoC series USB DRD PHY driver"
>>>  	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
>>> diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile
>>> index fea1f96d0..90b84c7fc 100644
>>> --- a/drivers/phy/samsung/Makefile
>>> +++ b/drivers/phy/samsung/Makefile
>>> @@ -14,5 +14,6 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2)	+= phy-exynos4210-usb2.o
>>>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
>>>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
>>>  phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
>>> +obj-$(CONFIG_PHY_EXYNOS2200_SNPS_EUSB2)	+= phy-exynos2200-snps-eusb2.o
>> Entire driver looks like repeating existing qcom-snps-eusb2.
> 
> It's the same IP, but implemented differently on a different platform. At
> the very least, the register layout is different.


I checked few registers, looked very the same. Same blocks from synopsys
have the common register layouts.

> 
>>  You need to
>> integrate the changes, not create duplicated driver.
> 
> I can do that, but it would be come a bit cluttered, won't it? Depends on
> if we want to follow the current oem-provided initialization sequence, or
> try and fully reuse what we have in there.


I think it duplicates a lot, so it won't be clutter. We have many
drivers having common code and per-variant ops.

Best regards,
Krzysztof

Re: [PATCH v1 3/4] phy: samsung: add Exynos2200 SNPS eUSB2 driver

[Ivaylo Ivanov]

On 2/16/25 11:44, Krzysztof Kozlowski wrote:
> On 16/02/2025 10:41, Ivaylo Ivanov wrote:
>> On 2/16/25 11:26, Krzysztof Kozlowski wrote:
>>> On 15/02/2025 13:24, Ivaylo Ivanov wrote:
>>>> The Exynos2200 SoC uses Synopsis eUSB2 PHY for USB 2.0. Add a new
>>>> driver for it.
>>>>
>>>> eUSB2 on Exynos SoCs is usually paired alongside a USB PHY controller.
>>>> Currently the driver is modelled to take and enable/disable the usb phy
>>>> controller when needed.
>>>>
>>>> The driver is based on information from downstream drivers.
>>>>
>>>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>>> ---
>>>>  drivers/phy/samsung/Kconfig                   |  13 +
>>>>  drivers/phy/samsung/Makefile                  |   1 +
>>>>  .../phy/samsung/phy-exynos2200-snps-eusb2.c   | 351 ++++++++++++++++++
>>>>  3 files changed, 365 insertions(+)
>>>>  create mode 100644 drivers/phy/samsung/phy-exynos2200-snps-eusb2.c
>>>>
>>>> diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig
>>>> index e2330b089..f62285254 100644
>>>> --- a/drivers/phy/samsung/Kconfig
>>>> +++ b/drivers/phy/samsung/Kconfig
>>>> @@ -77,6 +77,19 @@ config PHY_S5PV210_USB2
>>>>  	  particular SoC is compiled in the driver. In case of S5PV210 two phys
>>>>  	  are available - device and host.
>>>>  
>>>> +config PHY_EXYNOS2200_SNPS_EUSB2
>>>> +	tristate "Exynos2200 eUSB 2.0 PHY driver"
>>>> +	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
>>>> +	depends on HAS_IOMEM
>>>> +	depends on USB_DWC3_EXYNOS
>>> How does it depend? What are you using from DWC3?
>> Can drop, I guess.
>>
>>>> +	select GENERIC_PHY
>>>> +	select MFD_SYSCON
>>> Where do you use it?
>> Remained from USBCON driver.
>>
>>>> +	default y
>>>> +	help
>>>> +	  Enable USBCON PHY support for Exynos2200 SoC.
>>>> +	  This driver provides PHY interface for eUSB 2.0 controller
>>>> +	  present on Exynos5 SoC series.
>>>> +
>>>>  config PHY_EXYNOS5_USBDRD
>>>>  	tristate "Exynos5 SoC series USB DRD PHY driver"
>>>>  	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
>>>> diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile
>>>> index fea1f96d0..90b84c7fc 100644
>>>> --- a/drivers/phy/samsung/Makefile
>>>> +++ b/drivers/phy/samsung/Makefile
>>>> @@ -14,5 +14,6 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2)	+= phy-exynos4210-usb2.o
>>>>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
>>>>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
>>>>  phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
>>>> +obj-$(CONFIG_PHY_EXYNOS2200_SNPS_EUSB2)	+= phy-exynos2200-snps-eusb2.o
>>> Entire driver looks like repeating existing qcom-snps-eusb2.
>> It's the same IP, but implemented differently on a different platform. At
>> the very least, the register layout is different.
>
> I checked few registers, looked very the same. Same blocks from synopsys
> have the common register layouts.

I see.

>
>>>  You need to
>>> integrate the changes, not create duplicated driver.
>> I can do that, but it would be come a bit cluttered, won't it? Depends on
>> if we want to follow the current oem-provided initialization sequence, or
>> try and fully reuse what we have in there.
>
> I think it duplicates a lot, so it won't be clutter. We have many
> drivers having common code and per-variant ops.

So the approach to take here is to make a common driver?

What about the current modelling scheme, as-in taking the phandle to
the usbcon phy and handling it?

Best regards,
Ivaylo

>
> Best regards,
> Krzysztof

Re: [PATCH v1 4/4] phy: samsung: add Exynos2200 usb phy controller

[Ivaylo Ivanov]

On 2/16/25 11:36, Krzysztof Kozlowski wrote:
> On 15/02/2025 13:24, Ivaylo Ivanov wrote:
>> The Exynos2200 SoC comes with 3 PHYs - snps eUSB2, snps USBDP combophy
>> and a cut-off phy that origins from exynos5-usbdrd. The latter is used
>> for link control, as well as pipe3 attachment and detachment.
>>
>> Add a new driver for it.
>>
>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>> ---
>>  drivers/phy/samsung/Kconfig                 |  13 ++
>>  drivers/phy/samsung/Makefile                |   1 +
>>  drivers/phy/samsung/phy-exynos2200-usbcon.c | 241 ++++++++++++++++++++
>>  include/linux/soc/samsung/exynos-regs-pmu.h |   3 +
>>  4 files changed, 258 insertions(+)
>>  create mode 100644 drivers/phy/samsung/phy-exynos2200-usbcon.c
>>
>> diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig
>> index f62285254..47e9b9926 100644
>> --- a/drivers/phy/samsung/Kconfig
>> +++ b/drivers/phy/samsung/Kconfig
>> @@ -90,6 +90,19 @@ config PHY_EXYNOS2200_SNPS_EUSB2
>>  	  This driver provides PHY interface for eUSB 2.0 controller
>>  	  present on Exynos5 SoC series.
>>  
>> +config PHY_EXYNOS2200_USBCON
>> +	tristate "Exynos2200 USBCON PHY driver"
>> +	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
>> +	depends on HAS_IOMEM
>> +	depends on USB_DWC3_EXYNOS
> How? What are you using from DWC3?

Will drop.

>
>> +	select GENERIC_PHY
>> +	select MFD_SYSCON
>> +	default y
>> +	help
>> +	  Enable USBCON PHY support for Exynos2200 SoC.
>> +	  This driver provides PHY interface for USB controller present
>> +	  on Exynos2200 SoC.
>> +
>>  config PHY_EXYNOS5_USBDRD
>>  	tristate "Exynos5 SoC series USB DRD PHY driver"
>>  	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
>> diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile
>> index 90b84c7fc..f70e12ddf 100644
>> --- a/drivers/phy/samsung/Makefile
>> +++ b/drivers/phy/samsung/Makefile
>> @@ -15,5 +15,6 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
>>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
>>  phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
>>  obj-$(CONFIG_PHY_EXYNOS2200_SNPS_EUSB2)	+= phy-exynos2200-snps-eusb2.o
>> +obj-$(CONFIG_PHY_EXYNOS2200_USBCON)	+= phy-exynos2200-usbcon.o
>>  obj-$(CONFIG_PHY_EXYNOS5_USBDRD)	+= phy-exynos5-usbdrd.o
>>  obj-$(CONFIG_PHY_EXYNOS5250_SATA)	+= phy-exynos5250-sata.o
>> diff --git a/drivers/phy/samsung/phy-exynos2200-usbcon.c b/drivers/phy/samsung/phy-exynos2200-usbcon.c
>> new file mode 100644
>> index 000000000..7968c9792
>> --- /dev/null
>> +++ b/drivers/phy/samsung/phy-exynos2200-usbcon.c
>> @@ -0,0 +1,241 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2025, Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>> + */
>> +
>> +#include <linux/bitfield.h>
> Are you using this header?
>
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/iopoll.h>
> And rhis?
>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/reset.h>
> And this?

Nope, it's a leftover I forgot to drop. Same with the above

...
> Same comments as on previous patch.

Alright. Thanks for the feedback!

Best regards,
Ivaylo

>
>
>
> Best regards,
> Krzysztof

Re: [PATCH v1 3/4] phy: samsung: add Exynos2200 SNPS eUSB2 driver

[Krzysztof Kozlowski]

On 16/02/2025 10:51, Ivaylo Ivanov wrote:
>>
>>>>  You need to
>>>> integrate the changes, not create duplicated driver.
>>> I can do that, but it would be come a bit cluttered, won't it? Depends on
>>> if we want to follow the current oem-provided initialization sequence, or
>>> try and fully reuse what we have in there.
>>
>> I think it duplicates a lot, so it won't be clutter. We have many
>> drivers having common code and per-variant ops.
> 
> So the approach to take here is to make a common driver?

For example: one common module and two modules per each soc, because I
assume some per-soc stuff might be needed. But maybe even these two
modules are not necessary and everything could be in one driver.


> 
> What about the current modelling scheme, as-in taking the phandle to
> the usbcon phy and handling it?

What about it? Did you look at the bindings of qcom snps eusb2? Are you
saying you do not have here repeater? If so, then this phy phandle might
not be correct.



Best regards,
Krzysztof

Re: [PATCH v1 3/4] phy: samsung: add Exynos2200 SNPS eUSB2 driver

[Ivaylo Ivanov]

On 2/16/25 15:19, Krzysztof Kozlowski wrote:
> On 16/02/2025 10:51, Ivaylo Ivanov wrote:
>>>>>  You need to
>>>>> integrate the changes, not create duplicated driver.
>>>> I can do that, but it would be come a bit cluttered, won't it? Depends on
>>>> if we want to follow the current oem-provided initialization sequence, or
>>>> try and fully reuse what we have in there.
>>> I think it duplicates a lot, so it won't be clutter. We have many
>>> drivers having common code and per-variant ops.
>> So the approach to take here is to make a common driver?
> For example: one common module and two modules per each soc, because I
> assume some per-soc stuff might be needed. But maybe even these two
> modules are not necessary and everything could be in one driver.

The issue here is that, while both QCOM and SAMSUNG use that IP, a lot of
the registers are not mapped for Exynoses.

For example with QCOM:
``
#define USB_PHY_UTMI_CTRL0 (0x3c)
#define PHY_UTMI_CTRL5 (0x50)
``

Exynoses:
``
/* nothing before this */
#define EXYNOS2200_EUSB2_RST_CTRL 0x0
``

Here EXYNOS2200_EUSB2_RST_CTRL seems to be the same register as
QCOM_PHY_UTMI_CTRL5. Another instance is:
``
#define UTMI_PHY_CMN_CTRL0 (0x98)
#define TESTBURNIN BIT(6)
``

Exynoses:
``
#define EXYNOS2200_EUSB2_TESTSE (0x20)
#define EXYNOS2200_TEST_IDDQ BIT(6)
``

But at the same time there are some.. inconsistencies between them.
Looking at the register layout for the exynos implementation for TX tuning
the following register offset and bits are described:
``
/*
 * Offset : 0x0014
 * Description: tune register of tx driver
 */
typedef union {
  u32 data;
  struct {
   // bit[0] :
   unsigned fsls_slew_rate : 1;
   // bit[2:1] :
   unsigned fsls_vref_tune : 2;
   // bit[3] :
   unsigned fsls_vreg_bypass : 1;
   // bit[6:4]
   unsigned hs_vref_tune : 3;
   // bit [8:7]
   unsigned hs_xv : 2;
   // bit [11:9]
   unsigned preemp : 3;
   // bit [13:12]
   unsigned res : 2;
   // bit [15:14]
   unsigned rise : 2;
   // bit[31:16]
   unsigned RSVD31_16 : 16;
  } b;
} EUSBCON_REG_TXTUNE_o, *EUSBPHY_REG_TXTUNE_p;
``

And for QCOM the latter functionality is split into two separate register
offsets:
``
#define USB_PHY_CFG_CTRL_8 (0x78)
#define PHY_CFG_TX_FSLS_VREF_TUNE_MASK GENMASK(1, 0)
#define PHY_CFG_TX_FSLS_VREG_BYPASS BIT(2)
#define PHY_CFG_TX_HS_VREF_TUNE_MASK GENMASK(5, 3)
#define PHY_CFG_TX_HS_XV_TUNE_MASK GENMASK(7, 6)

#define USB_PHY_CFG_CTRL_9 (0x7c)
#define PHY_CFG_TX_PREEMP_TUNE_MASK GENMASK(2, 0)
#define PHY_CFG_TX_RES_TUNE_MASK GENMASK(4, 3)
#define PHY_CFG_TX_RISE_TUNE_MASK GENMASK(6, 5)
#define PHY_CFG_RCAL_BYPASS BIT(7)
``

So, Exynos2200 has a much simpler eusb initialization sequence than what
is present in mainline for QCOMs. I still don't really think the drivers
should be merged, as we aren't really duplicating code per-say.

I've already started working on merging them, and my current idea is to
not redefine the registers once again for 2200, but rather make an enum
that defines if the SoC is a QCOM or EXYNOS, and select the register
offsets dynamically - similarly as how I did with USIv1. If a register
offset is not present, it'd just not do the write. My guess is that this
will make it work with the qualcomm init sequence as well, so it'd result
in even less redundant code (apart from the eUSB tuning, which can be
omitted for now).

>
>> What about the current modelling scheme, as-in taking the phandle to
>> the usbcon phy and handling it?
> What about it? 

As I said in the commit description, I'm passing the USBCON phy as a
phandle to the eusb2 node and enabling/disabling it when needed. I'm
not 100% sure it would be adequate to include that in a common snps EUSB
driver, as it seems to more of a quirk with the exynoses. But then how
can I model it so that it's correctly described according to how the
hardware works (as-in usbcon "muxing" between child phys, in this case
eUSB and snps USBDP combophy)

Regarding repeaters, I still don't have the TI repeater implemented.

Best regards,
Ivaylo

> Did you look at the bindings of qcom snps eusb2? Are you
> saying you do not have here repeater? If so, then this phy phandle might
> not be correct.
>
>
>
> Best regards,
> Krzysztof

Re: [PATCH v1 3/4] phy: samsung: add Exynos2200 SNPS eUSB2 driver

[Ivaylo Ivanov]

On 2/16/25 15:57, Ivaylo Ivanov wrote:
> On 2/16/25 15:19, Krzysztof Kozlowski wrote:
>> On 16/02/2025 10:51, Ivaylo Ivanov wrote:
>>>>>>  You need to
>>>>>> integrate the changes, not create duplicated driver.
>>>>> I can do that, but it would be come a bit cluttered, won't it? Depends on
>>>>> if we want to follow the current oem-provided initialization sequence, or
>>>>> try and fully reuse what we have in there.
>>>> I think it duplicates a lot, so it won't be clutter. We have many
>>>> drivers having common code and per-variant ops.
>>> So the approach to take here is to make a common driver?
>> For example: one common module and two modules per each soc, because I
>> assume some per-soc stuff might be needed. But maybe even these two
>> modules are not necessary and everything could be in one driver.
...
>
> So, Exynos2200 has a much simpler eusb initialization sequence than what
> is present in mainline for QCOMs. I still don't really think the drivers
> should be merged, as we aren't really duplicating code per-say.
>
> I've already started working on merging them, and my current idea is to
> not redefine the registers once again for 2200, but rather make an enum
> that defines if the SoC is a QCOM or EXYNOS, and select the register
> offsets dynamically

Never mind. That's a bad idea - after more digging way too much bits differ
not just the register layout. I'll implement the init/exit sequence in the
qcom driver separately. Sadly I can't reuse much code.

Best regards,
Ivaylo

>  - similarly as how I did with USIv1. If a register
> offset is not present, it'd just not do the write. My guess is that this
> will make it work with the qualcomm init sequence as well, so it'd result
> in even less redundant code (apart from the eUSB tuning, which can be
> omitted for now).
>
>>> What about the current modelling scheme, as-in taking the phandle to
>>> the usbcon phy and handling it?
>> What about it? 
> As I said in the commit description, I'm passing the USBCON phy as a
> phandle to the eusb2 node and enabling/disabling it when needed. I'm
> not 100% sure it would be adequate to include that in a common snps EUSB
> driver, as it seems to more of a quirk with the exynoses. But then how
> can I model it so that it's correctly described according to how the
> hardware works (as-in usbcon "muxing" between child phys, in this case
> eUSB and snps USBDP combophy)
>
> Regarding repeaters, I still don't have the TI repeater implemented.
>
> Best regards,
> Ivaylo
>
>> Did you look at the bindings of qcom snps eusb2? Are you
>> saying you do not have here repeater? If so, then this phy phandle might
>> not be correct.
>>
>>
>>
>> Best regards,
>> Krzysztof

Re: [PATCH v1 3/4] phy: samsung: add Exynos2200 SNPS eUSB2 driver

[Philipp Zabel]

On Sa, 2025-02-15 at 14:24 +0200, Ivaylo Ivanov wrote:
> The Exynos2200 SoC uses Synopsis eUSB2 PHY for USB 2.0. Add a new
> driver for it.
> 
> eUSB2 on Exynos SoCs is usually paired alongside a USB PHY controller.
> Currently the driver is modelled to take and enable/disable the usb phy
> controller when needed.
> 
> The driver is based on information from downstream drivers.
> 
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
>  drivers/phy/samsung/Kconfig                   |  13 +
>  drivers/phy/samsung/Makefile                  |   1 +
>  .../phy/samsung/phy-exynos2200-snps-eusb2.c   | 351 ++++++++++++++++++
>  3 files changed, 365 insertions(+)
>  create mode 100644 drivers/phy/samsung/phy-exynos2200-snps-eusb2.c
> 
[...]
> diff --git a/drivers/phy/samsung/phy-exynos2200-snps-eusb2.c b/drivers/phy/samsung/phy-exynos2200-snps-eusb2.c
> new file mode 100644
> index 000000000..ee6d96411
> --- /dev/null
> +++ b/drivers/phy/samsung/phy-exynos2200-snps-eusb2.c
> @@ -0,0 +1,351 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025, Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/iopoll.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>

Drop this ...

[...]
> +struct exynos2200_snps_eusb2_phy {
> +	struct phy *phy;
> +	void __iomem *base;
> +
> +	struct clk *ref_clk;
> +	struct clk_bulk_data *clks;
> +	const struct exynos2200_snps_eusb2_phy_drvdata *drv_data;
> +	struct reset_control *phy_reset;

... and this. It's never used.


regards
Philipp