[PATCH v4 0/6] initial usbdrd phy support for Exynosautov920 soc

[PATCH v4 0/6] initial usbdrd phy support for Exynosautov920 soc

[Pritam Manohar Sutar]

This SoC has a single USB 3.1 DRD combo phy and three USB2.0 only
DRD phy controllers as mentined below

  * Combo phy supports USB3.1 SSP+(10Gbps) protocol and is backwards
    compatible to the USB3.0 SS(5Gbps). 'Add-on USB2.0' phy is added
    to support USB2.0 HS(480Mbps), FS(12Mbps) and LS(1.5Mbps) data rates.
    These two phys are combined to form a combo phy as mentioned below.


   USB30DRD_0 port

 +------------------------------------------------------------+
 |                                                            |
 |                (combo) USB phy controller                  |
 |      +----------------------------------------------+      |
 |      |                  USB HSPHY                   |      |
 |      |  (samsung,exynosautov920-usbdrd-combo-hsphy) |      |
 |      +----------------------------------------------+      |
 |                                                            |
 |    +--------------------------------------------------+    |
 |    |                   USB SSPHY                      |    |
 |    |   (samsung,exynosautov920-usb31drd-combo-ssphy)  |    |
 |    +--------------------------------------------------+-   |
 |                                                            |
 +------------------------------------------------------------+
 |                                                            |
 |                     USBDRD30 Link                          |
 |                       Controller                           |
 +------------------------------------------------------------+

  * USB2.0 phy supports only UTMI+ interface. USB2.0DRD phy
    is very similar to the existing Exynos850 support in this driver.

    USB20DRD_0/1/2 ports

      +---------------------------------------------------+
      |                                                   |
      |                USB PHY controller                 |
      |    +-----------------------------------------+    |
      |    |              USB HSPHY                  |    |
      |    |  (samsung,exynosautov920-usbdrd-phy)    |    |
      |    +-----------------------------------------+    |
      |                                                   |
      +---------------------------------------------------+
      |                                                   |
      |             USBDRD20_* Link                       |
      |                Controller                         |
      |                                                   |
      +---------------------------------------------------+

The "USB20 phy output isolation" is shared across the USB20 phys.
We have to bypass isolation when any one of the USBs is configured
and enable it when all are turned off. The "USB31 phy isolation"
is seperate for USB31 phy.

This patchset only supports device mode and same is verified with
as NCM device with below configfs commands

changelog
----------
Changes in v4:
- addressed comments from v3 patchset
  - removed dts related patches, to be posted in new patchset.
  - added regulator, pmu and power sequences.
  - phy isol is shared across USBs, added usage counter to bypass or
    enable phy isolation.
  - modified schemas with hs and combo phy compatible names
    (used "combo" to denote combo phy) and regulators
- modified code to work with binding and unbinding devices/drivers
- added "Reviewed-by" tag.
  link for v3: https://lore.kernel.org/linux-phy/20250613055613.866909-1-pritam.sutar@samsung.com/

Changes in v3:
- Updated dt-bindings for USB2.0 only.
- Added dt-bindings for combo phy.
- Added implementation for combo phy (SS and HS phy).
- Added added DTS nodes for all the phys
  link for v2: https://lore.kernel.org/linux-phy/20250516102650.2144487-1-pritam.sutar@samsung.com/

Changes in v2:
- Used standard GENMASK() and FIELD_GET() to get the major version
  from controller version register.
  link for v1: https://lore.kernel.org/linux-phy/20250514134813.380807-1-pritam.sutar@samsung.com/

Pritam Manohar Sutar (6):
  dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 HS phy
    compatible
  phy: exynos5-usbdrd: support HS phy for ExynosAutov920
  dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 combo HS
    phy
  phy: exynos5-usbdrd: support HS combo phy for ExynosAutov920
  dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 combo SS
    phy
  phy: exynos5-usbdrd: support SS combo phy for ExynosAutov920

 .../bindings/phy/samsung,usb3-drd-phy.yaml    |  41 ++
 drivers/phy/samsung/phy-exynos5-usbdrd.c      | 662 ++++++++++++++++++
 include/linux/soc/samsung/exynos-regs-pmu.h   |   3 +
 3 files changed, 706 insertions(+)

-- 
2.34.1

[PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 HS phy compatible

[Pritam Manohar Sutar]

Add a dedicated compatible string for USB HS phy found in this SoC.
The SoC requires two clocks, named "phy" and "ref" (same as clocks
required by Exynos850).

It also requires various power supplies (regulators) for the internal
circuitry to work. The required voltages are:
* avdd075_usb - 0.75v
* avdd18_usb20 - 1.8v
* avdd33_usb20 - 3.3v

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Pritam Manohar Sutar <pritam.sutar@samsung.com>
---
 .../bindings/phy/samsung,usb3-drd-phy.yaml    | 37 +++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
index e906403208c0..2e29ff749bba 100644
--- a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
@@ -34,6 +34,7 @@ properties:
       - samsung,exynos7870-usbdrd-phy
       - samsung,exynos850-usbdrd-phy
       - samsung,exynos990-usbdrd-phy
+      - samsung,exynosautov920-usbdrd-phy
 
   clocks:
     minItems: 1
@@ -110,6 +111,15 @@ properties:
   vddh-usbdp-supply:
     description: VDDh power supply for the USB DP phy.
 
+  avdd075_usb-supply:
+    description: 0.75V power supply for USB phy
+
+  avdd18_usb20-supply:
+    description: 1.8V power supply for USB phy
+
+  avdd33_usb20-supply:
+    description: 3.3V power supply for USB phy
+
 required:
   - compatible
   - clocks
@@ -235,6 +245,33 @@ allOf:
 
         reg-names:
           maxItems: 1
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - samsung,exynosautov920-usbdrd-phy
+    then:
+      properties:
+        clocks:
+          minItems: 2
+          maxItems: 2
+
+        clock-names:
+          items:
+            - const: phy
+            - const: ref
+
+        reg:
+          maxItems: 1
+
+        reg-names:
+          maxItems: 1
+
+      required:
+        - avdd075_usb-supply
+        - avdd18_usb20-supply
+        - avdd33_usb20-supply
 
 unevaluatedProperties: false
 
-- 
2.34.1

[PATCH v4 2/6] phy: exynos5-usbdrd: support HS phy for ExynosAutov920

[Pritam Manohar Sutar]

This SoC has a single USB 3.1 DRD combo phy that supports both
UTMI+ (HS) and PIPE3 (SS) and three USB2.0 DRD HS phy controllers
those only support the UTMI+ (HS) interface.

Support only UTMI+ port for this SoC which is very similar to what
the existing Exynos850 supports.

This SoC shares phy isol between USBs. Bypass PHY isol when first USB
is powered on and enable it when all of then are powered off. Add
required change in phy driver to support HS phy for this SoC.

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Pritam Manohar Sutar <pritam.sutar@samsung.com>
---
 drivers/phy/samsung/phy-exynos5-usbdrd.c    | 131 ++++++++++++++++++++
 include/linux/soc/samsung/exynos-regs-pmu.h |   2 +
 2 files changed, 133 insertions(+)

diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
index dd660ebe8045..64f3316f6ad4 100644
--- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
+++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
@@ -480,6 +480,8 @@ struct exynos5_usbdrd_phy {
 	enum typec_orientation orientation;
 };
 
+static atomic_t usage_count = ATOMIC_INIT(0);
+
 static inline
 struct exynos5_usbdrd_phy *to_usbdrd_phy(struct phy_usb_instance *inst)
 {
@@ -2054,6 +2056,132 @@ static const struct exynos5_usbdrd_phy_drvdata exynos990_usbdrd_phy = {
 	.n_regulators		= ARRAY_SIZE(exynos5_regulator_names),
 };
 
+static int exynosautov920_usbdrd_phy_init(struct phy *phy)
+{
+	struct phy_usb_instance *inst = phy_get_drvdata(phy);
+	struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
+	int ret;
+
+	ret = clk_bulk_prepare_enable(phy_drd->drv_data->n_clks, phy_drd->clks);
+	if (ret)
+		return ret;
+
+	if (inst->phy_cfg->id == EXYNOS5_DRDPHY_UTMI) {
+		/* Bypass PHY isol when first USB is powered on */
+		if ((atomic_inc_return(&usage_count) == 1))
+			inst->phy_cfg->phy_isol(inst, false);
+	}
+
+	/* UTMI or PIPE3 specific init */
+	inst->phy_cfg->phy_init(phy_drd);
+
+	clk_bulk_disable_unprepare(phy_drd->drv_data->n_clks, phy_drd->clks);
+
+	return 0;
+}
+
+static int exynosautov920_usbdrd_phy_exit(struct phy *phy)
+{
+	struct phy_usb_instance *inst = phy_get_drvdata(phy);
+	struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
+	int ret = 0;
+
+	ret = clk_bulk_prepare_enable(phy_drd->drv_data->n_clks, phy_drd->clks);
+	if (ret)
+		return ret;
+
+	if (inst->phy_cfg->id == EXYNOS5_DRDPHY_UTMI) {
+		exynos850_usbdrd_phy_exit(phy);
+
+		/* enable PHY isol when all USBs are powered off */
+		if (atomic_dec_and_test(&usage_count))
+			inst->phy_cfg->phy_isol(inst, true);
+	}
+
+	clk_bulk_disable_unprepare(phy_drd->drv_data->n_clks, phy_drd->clks);
+
+	return 0;
+}
+
+static int exynosautov920_usbdrd_phy_power_on(struct phy *phy)
+{
+	int ret;
+	struct phy_usb_instance *inst = phy_get_drvdata(phy);
+	struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
+
+	dev_dbg(phy_drd->dev, "Request to power_on usbdrd_phy phy\n");
+
+	ret = clk_bulk_prepare_enable(phy_drd->drv_data->n_core_clks,
+				      phy_drd->core_clks);
+	if (ret)
+		return ret;
+
+	/* Enable supply */
+	ret = regulator_bulk_enable(phy_drd->drv_data->n_regulators,
+				    phy_drd->regulators);
+	if (ret) {
+		dev_err(phy_drd->dev, "Failed to enable PHY regulator(s)\n");
+		goto fail_supply;
+	}
+
+	return 0;
+
+fail_supply:
+	clk_bulk_disable_unprepare(phy_drd->drv_data->n_core_clks,
+				   phy_drd->core_clks);
+
+	return ret;
+}
+
+static int exynosautov920_usbdrd_phy_power_off(struct phy *phy)
+{
+	struct phy_usb_instance *inst = phy_get_drvdata(phy);
+	struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
+
+	dev_dbg(phy_drd->dev, "Request to power_off usbdrd_phy phy\n");
+
+	/* Disable supply */
+	regulator_bulk_disable(phy_drd->drv_data->n_regulators,
+			       phy_drd->regulators);
+
+	clk_bulk_disable_unprepare(phy_drd->drv_data->n_core_clks,
+				   phy_drd->core_clks);
+
+	return 0;
+}
+
+static const char * const exynosautov920_regulator_names[] = {
+	"avdd075_usb", "avdd18_usb20", "avdd33_usb20",
+};
+
+static const struct phy_ops exynosautov920_usbdrd_phy_ops = {
+	.init		= exynosautov920_usbdrd_phy_init,
+	.exit		= exynosautov920_usbdrd_phy_exit,
+	.power_on       = exynosautov920_usbdrd_phy_power_on,
+	.power_off      = exynosautov920_usbdrd_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static const struct exynos5_usbdrd_phy_config phy_cfg_exynosautov920[] = {
+	{
+		.id		= EXYNOS5_DRDPHY_UTMI,
+		.phy_isol	= exynos5_usbdrd_phy_isol,
+		.phy_init	= exynos850_usbdrd_utmi_init,
+	},
+};
+
+static const struct exynos5_usbdrd_phy_drvdata exynosautov920_usbdrd_phy = {
+	.phy_cfg		= phy_cfg_exynosautov920,
+	.phy_ops		= &exynosautov920_usbdrd_phy_ops,
+	.pmu_offset_usbdrd0_phy	= EXYNOSAUTOV920_PHY_CTRL_USB20,
+	.clk_names		= exynos5_clk_names,
+	.n_clks			= ARRAY_SIZE(exynos5_clk_names),
+	.core_clk_names		= exynos5_core_clk_names,
+	.n_core_clks		= ARRAY_SIZE(exynos5_core_clk_names),
+	.regulator_names	= exynosautov920_regulator_names,
+	.n_regulators		= ARRAY_SIZE(exynosautov920_regulator_names),
+};
+
 static const struct exynos5_usbdrd_phy_config phy_cfg_gs101[] = {
 	{
 		.id		= EXYNOS5_DRDPHY_UTMI,
@@ -2260,6 +2388,9 @@ static const struct of_device_id exynos5_usbdrd_phy_of_match[] = {
 	}, {
 		.compatible = "samsung,exynos990-usbdrd-phy",
 		.data = &exynos990_usbdrd_phy
+	}, {
+		.compatible = "samsung,exynosautov920-usbdrd-phy",
+		.data = &exynosautov920_usbdrd_phy
 	},
 	{ },
 };
diff --git a/include/linux/soc/samsung/exynos-regs-pmu.h b/include/linux/soc/samsung/exynos-regs-pmu.h
index 71e0c09a49eb..4923f9be3d1f 100644
--- a/include/linux/soc/samsung/exynos-regs-pmu.h
+++ b/include/linux/soc/samsung/exynos-regs-pmu.h
@@ -688,4 +688,6 @@
 #define GS101_GRP2_INTR_BID_UPEND				(0x0208)
 #define GS101_GRP2_INTR_BID_CLEAR				(0x020c)
 
+/* exynosautov920 */
+#define EXYNOSAUTOV920_PHY_CTRL_USB20				(0x0710)
 #endif /* __LINUX_SOC_EXYNOS_REGS_PMU_H */
-- 
2.34.1

[PATCH v4 3/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 combo HS phy

[Pritam Manohar Sutar]

This phy supports USB3.1 SSP+(10Gbps) protocol and is backwards
compatible to the USB3.0 SS(5Gbps). 'Add-on USB2.0' phy is added
to support USB2.0 HS(480Mbps), FS(12Mbps) and LS(1.5Mbps) data rates.
These two phys are combined to form a combo phy.

Add a dedicated compatible string for USB combo HS phy found in this
SoC. The SoC requires two clocks, named "phy" and "ref" and various
power supplies (regulators) for the internal circuitry to work.
The required voltages are:
* avdd075_usb - 0.75v
* avdd18_usb20 - 1.8v
* avdd33_usb20 - 3.3v

Add schema only for 'Add-on USB2.0' HS phy in this combo phy.

Signed-off-by: Pritam Manohar Sutar <pritam.sutar@samsung.com>
---
 Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
index 2e29ff749bba..b024339b5acc 100644
--- a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
@@ -34,6 +34,7 @@ properties:
       - samsung,exynos7870-usbdrd-phy
       - samsung,exynos850-usbdrd-phy
       - samsung,exynos990-usbdrd-phy
+      - samsung,exynosautov920-usbdrd-combo-hsphy
       - samsung,exynosautov920-usbdrd-phy
 
   clocks:
@@ -250,6 +251,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - samsung,exynosautov920-usbdrd-combo-hsphy
               - samsung,exynosautov920-usbdrd-phy
     then:
       properties:
-- 
2.34.1

[PATCH v4 4/6] phy: exynos5-usbdrd: support HS combo phy for ExynosAutov920

[Pritam Manohar Sutar]

This SoC has a single USB 3.1 DRD combo phy that supports both
UTMI+ (HS) and PIPE3 (SS) and three USB2.0 DRD HS phy controllers
those only support the UTMI+ (HS) interface.

Support UTMI+ combo phy for this SoC which is somewhat simmilar to
what the existing Exynos850 support does. The difference is that
some register offsets and bit fields are defferent from Exynos850.

Add required change in phy driver to support combo HS phy for this SoC.

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Pritam Manohar Sutar <pritam.sutar@samsung.com>
---
 drivers/phy/samsung/phy-exynos5-usbdrd.c | 212 +++++++++++++++++++++++
 1 file changed, 212 insertions(+)

diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
index 64f3316f6ad4..8a1cd63b29ce 100644
--- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
+++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
@@ -41,6 +41,13 @@
 #define EXYNOS2200_CLKRST_LINK_PCLK_SEL		BIT(1)
 
 #define EXYNOS2200_DRD_UTMI			0x10
+
+/* ExynosAutov920 bits */
+#define UTMICTL_FORCE_UTMI_SUSPEND		BIT(13)
+#define UTMICTL_FORCE_UTMI_SLEEP		BIT(12)
+#define UTMICTL_FORCE_DPPULLDOWN		BIT(9)
+#define UTMICTL_FORCE_DMPULLDOWN		BIT(8)
+
 #define EXYNOS2200_UTMI_FORCE_VBUSVALID		BIT(1)
 #define EXYNOS2200_UTMI_FORCE_BVALID		BIT(0)
 
@@ -250,6 +257,22 @@
 #define EXYNOS850_DRD_HSP_TEST			0x5c
 #define HSP_TEST_SIDDQ				BIT(24)
 
+#define EXYNOSAUTOV920_DRD_HSP_CLKRST		0x100
+#define HSPCLKRST_PHY20_SW_PORTRESET		BIT(3)
+#define HSPCLKRST_PHY20_SW_POR			BIT(1)
+#define HSPCLKRST_PHY20_SW_POR_SEL		BIT(0)
+
+#define EXYNOSAUTOV920_DRD_HSPCTL		0x104
+#define HSPCTRL_VBUSVLDEXTSEL			BIT(13)
+#define HSPCTRL_VBUSVLDEXT			BIT(12)
+#define HSPCTRL_EN_UTMISUSPEND			BIT(9)
+#define HSPCTRL_COMMONONN			BIT(8)
+
+#define EXYNOSAUTOV920_DRD_HSP_TEST		0x10c
+
+#define EXYNOSAUTOV920_DRD_HSPPLLTUNE		0x110
+#define HSPPLLTUNE_FSEL				GENMASK(18, 16)
+
 /* Exynos9 - GS101 */
 #define EXYNOS850_DRD_SECPMACTL			0x48
 #define SECPMACTL_PMA_ROPLL_REF_CLK_SEL		GENMASK(13, 12)
@@ -2056,6 +2079,139 @@ static const struct exynos5_usbdrd_phy_drvdata exynos990_usbdrd_phy = {
 	.n_regulators		= ARRAY_SIZE(exynos5_regulator_names),
 };
 
+static void
+exynosautov920_usbdrd_utmi_init(struct exynos5_usbdrd_phy *phy_drd)
+{
+	void __iomem *reg_phy = phy_drd->reg_phy;
+	u32 reg;
+
+	/*
+	 * Disable HWACG (hardware auto clock gating control). This
+	 * forces QACTIVE signal in Q-Channel interface to HIGH level,
+	 * to make sure the PHY clock is not gated by the hardware.
+	 */
+	reg = readl(reg_phy + EXYNOS850_DRD_LINKCTRL);
+	reg |= LINKCTRL_FORCE_QACT;
+	writel(reg, reg_phy + EXYNOS850_DRD_LINKCTRL);
+
+	/* De-assert link reset */
+	reg = readl(reg_phy + EXYNOS2200_DRD_CLKRST);
+	reg &= ~CLKRST_LINK_SW_RST;
+	writel(reg, reg_phy + EXYNOS2200_DRD_CLKRST);
+
+	/* Set PHY POR High */
+	reg = readl(reg_phy + EXYNOSAUTOV920_DRD_HSP_CLKRST);
+	reg |= HSPCLKRST_PHY20_SW_POR | HSPCLKRST_PHY20_SW_POR_SEL;
+	writel(reg, reg_phy + EXYNOSAUTOV920_DRD_HSP_CLKRST);
+
+	/* Enable UTMI+ */
+	reg = readl(reg_phy + EXYNOS2200_DRD_UTMI);
+	reg &= ~(UTMICTL_FORCE_UTMI_SUSPEND | UTMICTL_FORCE_UTMI_SLEEP |
+		UTMICTL_FORCE_DPPULLDOWN | UTMICTL_FORCE_DMPULLDOWN);
+	writel(reg, reg_phy + EXYNOS2200_DRD_UTMI);
+
+	/* set phy clock & control HS phy */
+	reg = readl(reg_phy + EXYNOSAUTOV920_DRD_HSPCTL);
+	reg |= HSPCTRL_EN_UTMISUSPEND | HSPCTRL_COMMONONN;
+	writel(reg, reg_phy + EXYNOSAUTOV920_DRD_HSPCTL);
+
+	fsleep(100);
+
+	/* Set VBUS Valid and DP-Pull up control by VBUS pad usage */
+	reg = readl(reg_phy + EXYNOS850_DRD_LINKCTRL);
+	reg |= FIELD_PREP_CONST(LINKCTRL_BUS_FILTER_BYPASS, 0xf);
+	writel(reg, reg_phy + EXYNOS850_DRD_LINKCTRL);
+
+	reg = readl(reg_phy + EXYNOS2200_DRD_UTMI);
+	reg |= EXYNOS2200_UTMI_FORCE_VBUSVALID | EXYNOS2200_UTMI_FORCE_BVALID;
+	writel(reg, reg_phy + EXYNOS2200_DRD_UTMI);
+
+	reg = readl(reg_phy + EXYNOSAUTOV920_DRD_HSPCTL);
+	reg |= HSPCTRL_VBUSVLDEXTSEL | HSPCTRL_VBUSVLDEXT;
+	writel(reg, reg_phy + EXYNOSAUTOV920_DRD_HSPCTL);
+
+	/* Setting FSEL for refference clock */
+	reg = readl(reg_phy + EXYNOSAUTOV920_DRD_HSPPLLTUNE);
+	reg &= ~HSPPLLTUNE_FSEL;
+	switch (phy_drd->extrefclk) {
+	case EXYNOS5_FSEL_50MHZ:
+		reg |= FIELD_PREP(HSPPLLTUNE_FSEL, 7);
+		break;
+	case EXYNOS5_FSEL_26MHZ:
+		reg |= FIELD_PREP(HSPPLLTUNE_FSEL, 6);
+		break;
+	case EXYNOS5_FSEL_24MHZ:
+		reg |= FIELD_PREP(HSPPLLTUNE_FSEL, 2);
+		break;
+	case EXYNOS5_FSEL_20MHZ:
+		reg |= FIELD_PREP(HSPPLLTUNE_FSEL, 1);
+		break;
+	case EXYNOS5_FSEL_19MHZ2:
+		reg |= FIELD_PREP(HSPPLLTUNE_FSEL, 0);
+		break;
+	default:
+		dev_warn(phy_drd->dev, "unsupported ref clk: %#.2x\n",
+			 phy_drd->extrefclk);
+		break;
+	}
+	writel(reg, reg_phy + EXYNOSAUTOV920_DRD_HSPPLLTUNE);
+
+	/* Enable PHY Power Mode */
+	reg = readl(reg_phy + EXYNOSAUTOV920_DRD_HSP_TEST);
+	reg &= ~HSP_TEST_SIDDQ;
+	writel(reg, reg_phy + EXYNOSAUTOV920_DRD_HSP_TEST);
+
+	/* before POR low, 10us delay is needed to Finish PHY reset */
+	fsleep(10);
+
+	/* Set PHY POR Low */
+	reg = readl(reg_phy + EXYNOSAUTOV920_DRD_HSP_CLKRST);
+	reg |= HSPCLKRST_PHY20_SW_POR_SEL;
+	reg &= ~(HSPCLKRST_PHY20_SW_POR | HSPCLKRST_PHY20_SW_PORTRESET);
+	writel(reg, reg_phy + EXYNOSAUTOV920_DRD_HSP_CLKRST);
+
+	/* after POR low and delay 75us, PHYCLOCK is guaranteed. */
+	fsleep(75);
+
+	/* force pipe3 signal for link */
+	reg = readl(reg_phy + EXYNOS850_DRD_LINKCTRL);
+	reg |= LINKCTRL_FORCE_PIPE_EN;
+	reg &= ~LINKCTRL_FORCE_PHYSTATUS;
+	reg |= LINKCTRL_FORCE_RXELECIDLE;
+	writel(reg, reg_phy + EXYNOS850_DRD_LINKCTRL);
+}
+
+static void
+exynosautov920_usbdrd_hsphy_disable(struct exynos5_usbdrd_phy *phy_drd)
+{
+	u32 reg;
+	void __iomem *reg_phy = phy_drd->reg_phy;
+
+	/* set phy clock & control HS phy */
+	reg = readl(reg_phy + EXYNOS2200_DRD_UTMI);
+	reg |= UTMICTL_FORCE_UTMI_SUSPEND | UTMICTL_FORCE_UTMI_SLEEP;
+	reg &= ~(UTMICTL_FORCE_DPPULLDOWN | UTMICTL_FORCE_DMPULLDOWN);
+	writel(reg, reg_phy + EXYNOS2200_DRD_UTMI);
+
+	/* Disable PHY Power Mode */
+	reg = readl(reg_phy + EXYNOSAUTOV920_DRD_HSP_TEST);
+	reg |= HSP_TEST_SIDDQ;
+	writel(reg, reg_phy + EXYNOSAUTOV920_DRD_HSP_TEST);
+
+	/* clear force q-channel */
+	reg = readl(reg_phy + EXYNOS850_DRD_LINKCTRL);
+	reg &= ~LINKCTRL_FORCE_QACT;
+	writel(reg, reg_phy + EXYNOS850_DRD_LINKCTRL);
+
+	/* link sw reset is need for USB_DP/DM high-z in host mode */
+	reg = readl(reg_phy + EXYNOS2200_DRD_CLKRST);
+	reg |= CLKRST_LINK_SW_RST;
+	writel(reg, reg_phy + EXYNOS2200_DRD_CLKRST);
+	fsleep(10);
+	reg &= ~CLKRST_LINK_SW_RST;
+	writel(reg, reg_phy + EXYNOS2200_DRD_CLKRST);
+}
+
 static int exynosautov920_usbdrd_phy_init(struct phy *phy)
 {
 	struct phy_usb_instance *inst = phy_get_drvdata(phy);
@@ -2103,6 +2259,29 @@ static int exynosautov920_usbdrd_phy_exit(struct phy *phy)
 	return 0;
 }
 
+static int exynosautov920_usbdrd_combo_phy_exit(struct phy *phy)
+{
+	struct phy_usb_instance *inst = phy_get_drvdata(phy);
+	struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
+	int ret = 0;
+
+	ret = clk_bulk_prepare_enable(phy_drd->drv_data->n_clks, phy_drd->clks);
+	if (ret)
+		return ret;
+
+	if (inst->phy_cfg->id == EXYNOS5_DRDPHY_UTMI) {
+		exynosautov920_usbdrd_hsphy_disable(phy_drd);
+
+		/* enable PHY isol when all USBs are powered off */
+		if (atomic_dec_and_test(&usage_count))
+			inst->phy_cfg->phy_isol(inst, true);
+	}
+
+	clk_bulk_disable_unprepare(phy_drd->drv_data->n_clks, phy_drd->clks);
+
+	return 0;
+}
+
 static int exynosautov920_usbdrd_phy_power_on(struct phy *phy)
 {
 	int ret;
@@ -2154,6 +2333,36 @@ static const char * const exynosautov920_regulator_names[] = {
 	"avdd075_usb", "avdd18_usb20", "avdd33_usb20",
 };
 
+static const struct phy_ops exynosautov920_usbdrd_combo_hsphy_ops = {
+	.init		= exynosautov920_usbdrd_phy_init,
+	.exit		= exynosautov920_usbdrd_combo_phy_exit,
+	.power_on	= exynosautov920_usbdrd_phy_power_on,
+	.power_off	= exynosautov920_usbdrd_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static const struct
+exynos5_usbdrd_phy_config usbdrd_hsphy_cfg_exynosautov920[] = {
+	{
+		.id		= EXYNOS5_DRDPHY_UTMI,
+		.phy_isol	= exynos5_usbdrd_phy_isol,
+		.phy_init	= exynosautov920_usbdrd_utmi_init,
+	},
+};
+
+static const
+struct exynos5_usbdrd_phy_drvdata exynosautov920_usbdrd_combo_hsphy = {
+	.phy_cfg		= usbdrd_hsphy_cfg_exynosautov920,
+	.phy_ops		= &exynosautov920_usbdrd_combo_hsphy_ops,
+	.pmu_offset_usbdrd0_phy	= EXYNOSAUTOV920_PHY_CTRL_USB20,
+	.clk_names		= exynos5_clk_names,
+	.n_clks			= ARRAY_SIZE(exynos5_clk_names),
+	.core_clk_names		= exynos5_core_clk_names,
+	.n_core_clks		= ARRAY_SIZE(exynos5_core_clk_names),
+	.regulator_names	= exynosautov920_regulator_names,
+	.n_regulators		= ARRAY_SIZE(exynosautov920_regulator_names),
+};
+
 static const struct phy_ops exynosautov920_usbdrd_phy_ops = {
 	.init		= exynosautov920_usbdrd_phy_init,
 	.exit		= exynosautov920_usbdrd_phy_exit,
@@ -2388,6 +2597,9 @@ static const struct of_device_id exynos5_usbdrd_phy_of_match[] = {
 	}, {
 		.compatible = "samsung,exynos990-usbdrd-phy",
 		.data = &exynos990_usbdrd_phy
+	}, {
+		.compatible = "samsung,exynosautov920-usbdrd-combo-hsphy",
+		.data = &exynosautov920_usbdrd_combo_hsphy
 	}, {
 		.compatible = "samsung,exynosautov920-usbdrd-phy",
 		.data = &exynosautov920_usbdrd_phy
-- 
2.34.1

[PATCH v4 5/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 combo SS phy

[Pritam Manohar Sutar]

This phy supports USB3.1 SSP+(10Gbps) protocol and is backwards
compatible to the USB3.0 SS(5Gbps). 'Add-on USB2.0' phy is required
to support USB2.0 HS(480Mbps), FS(12Mbps) and LS(1.5Mbps) data rates.
These two phys are combined to form a combo phy.

Add a dedicated compatible string for USB combo SS phy found in this
SoC. The SoC requires two clocks, named "phy" and "ref" and various
power supplies (regulators) for the internal circuitry to work.
The required voltages are:
* avdd075_usb - 0.75v
* avdd18_usb20 - 1.8v
* avdd33_usb20 - 3.3v

Add schema only for 'USB3.1 SSP+' SS phy in this commit.

Signed-off-by: Pritam Manohar Sutar <pritam.sutar@samsung.com>
---
 Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
index b024339b5acc..b43b2ecbc132 100644
--- a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
@@ -34,6 +34,7 @@ properties:
       - samsung,exynos7870-usbdrd-phy
       - samsung,exynos850-usbdrd-phy
       - samsung,exynos990-usbdrd-phy
+      - samsung,exynosautov920-usb31drd-combo-ssphy
       - samsung,exynosautov920-usbdrd-combo-hsphy
       - samsung,exynosautov920-usbdrd-phy
 
@@ -251,6 +252,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - samsung,exynosautov920-usb31drd-combo-ssphy
               - samsung,exynosautov920-usbdrd-combo-hsphy
               - samsung,exynosautov920-usbdrd-phy
     then:
-- 
2.34.1

[PATCH v4 6/6] phy: exynos5-usbdrd: support SS combo phy for ExynosAutov920

[Pritam Manohar Sutar]

This SoC has a DWC3 compatible link controller and single USB 3.1 DRD
combo phy that supports both UTMI+ (HS) and PIPE3 (SS) and three USB2.0
DRD HS phy controllers those only support the UTMI+ (HS) interface.

Combo phy is combination of two phys. Among these phys, one supports
USB3.1 SSP+(10Gbps) protocol and is backwards compatible to the
USB3.0 SS(5Gbps). 'Add-on USB2.0' phy is required to support
USB2.0 HS(480Mbps), FS(12Mbps) and LS(1.5Mbps) data rates.

Add required change in phy driver to support combo SS phy for this SoC.

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Pritam Manohar Sutar <pritam.sutar@samsung.com>
---
 drivers/phy/samsung/phy-exynos5-usbdrd.c    | 327 +++++++++++++++++++-
 include/linux/soc/samsung/exynos-regs-pmu.h |   1 +
 2 files changed, 324 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
index 8a1cd63b29ce..9e70bc815937 100644
--- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
+++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
@@ -273,6 +273,36 @@
 #define EXYNOSAUTOV920_DRD_HSPPLLTUNE		0x110
 #define HSPPLLTUNE_FSEL				GENMASK(18, 16)
 
+/* ExynosAutov920 phy usb31drd port reg */
+#define EXYNOSAUTOV920_USB31DRD_PHY_RST_CTRL	0x000
+#define PHY_RST_CTRL_PIPE_LANE0_RESET_N_OVRD_EN	BIT(5)
+#define PHY_RST_CTRL_PIPE_LANE0_RESET_N		BIT(4)
+#define PHY_RST_CTRL_PHY_RESET_OVRD_EN		BIT(1)
+#define PHY_RST_CTRL_PHY_RESET			BIT(0)
+
+#define EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0	0x0004
+#define PHY_CR_PARA_CON0_PHY0_CR_PARA_ADDR		GENMASK(31, 16)
+#define PHY_CR_PARA_CON0_PHY0_CR_PARA_CLK		BIT(8)
+#define PHY_CR_PARA_CON0_PHY0_CR_PARA_ACK		BIT(4)
+#define PHY_CR_PARA_CON0_PHY0_CR_PARA_SEL		BIT(0)
+
+#define EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON1	0x0008
+
+#define EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON2	0x000c
+#define PHY_CR_PARA_CON2_PHY0_CR_PARA_WR_EN		BIT(0)
+#define PHY_CR_PARA_CON2_PHY0_CR_PARA_WR_DATA		GENMASK(31, 16)
+
+#define EXYNOSAUTOV920_USB31DRD_PHY_CONFIG0	0x100
+#define PHY_CONFIG0_PHY0_PMA_PWR_STABLE		BIT(14)
+#define PHY_CONFIG0_PHY0_PCS_PWR_STABLE		BIT(13)
+#define PHY_CONFIG0_PHY0_ANA_PWR_EN		BIT(1)
+
+#define EXYNOSAUTOV920_USB31DRD_PHY_CONFIG7	0x11c
+#define PHY_CONFIG7_PHY_TEST_POWERDOWN		BIT(24)
+
+#define EXYNOSAUTOV920_USB31DRD_PHY_CONFIG4	0x110
+#define PHY_CONFIG4_PIPE_RX0_SRIS_MODE_EN	BIT(2)
+
 /* Exynos9 - GS101 */
 #define EXYNOS850_DRD_SECPMACTL			0x48
 #define SECPMACTL_PMA_ROPLL_REF_CLK_SEL		GENMASK(13, 12)
@@ -2079,6 +2109,253 @@ static const struct exynos5_usbdrd_phy_drvdata exynos990_usbdrd_phy = {
 	.n_regulators		= ARRAY_SIZE(exynos5_regulator_names),
 };
 
+static void
+exynosautov920_usb31drd_cr_clk(struct exynos5_usbdrd_phy *phy_drd, bool high)
+{
+	void __iomem *reg_phy = phy_drd->reg_phy;
+	u32 reg = 0;
+
+	reg = readl(reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
+	if (high)
+		reg |= PHY_CR_PARA_CON0_PHY0_CR_PARA_CLK;
+	else
+		reg &= ~PHY_CR_PARA_CON0_PHY0_CR_PARA_CLK;
+
+	writel(reg, reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
+	fsleep(1);
+}
+
+static void
+exynosautov920_usb31drd_port_phy_ready(struct exynos5_usbdrd_phy *phy_drd)
+{
+	struct device *dev = phy_drd->dev;
+	void __iomem *reg_phy = phy_drd->reg_phy;
+	static const unsigned int timeout_us = 20000;
+	static const unsigned int sleep_us = 40;
+	u32 reg = 0;
+	int err;
+
+	/* Clear cr_para_con */
+	reg &= ~(PHY_CR_PARA_CON0_PHY0_CR_PARA_CLK |
+			PHY_CR_PARA_CON0_PHY0_CR_PARA_ADDR);
+	reg |= PHY_CR_PARA_CON0_PHY0_CR_PARA_SEL;
+	writel(reg, reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
+	writel(0x0, reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON1);
+	writel(0x0, reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON2);
+
+	exynosautov920_usb31drd_cr_clk(phy_drd, true);
+	exynosautov920_usb31drd_cr_clk(phy_drd, false);
+
+	/*
+	 * The maximum time from phy reset de-assertion to de-assertion of
+	 * tx/rx_ack can be as high as 5ms in fast simulation mode.
+	 * Time to phy ready is < 20ms
+	 */
+	err = readl_poll_timeout(reg_phy +
+				EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0,
+			reg, !(reg & PHY_CR_PARA_CON0_PHY0_CR_PARA_ACK),
+			sleep_us, timeout_us);
+	if (err)
+		dev_err(dev, "timed out waiting for rx/tx_ack: %#.8x\n", reg);
+
+	reg &= ~PHY_CR_PARA_CON0_PHY0_CR_PARA_CLK;
+	writel(reg, reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
+}
+
+static void
+exynosautov920_usb31drd_cr_write(struct exynos5_usbdrd_phy *phy_drd,
+				 u16 addr, u16 data)
+{
+	struct device *dev = phy_drd->dev;
+	void __iomem *reg_phy = phy_drd->reg_phy;
+	u32 cnt = 0;
+	u32 reg = 0;
+
+	/* Pre Clocking */
+	reg = readl(reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
+	reg |= PHY_CR_PARA_CON0_PHY0_CR_PARA_SEL;
+	writel(reg, reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
+
+	/*
+	 * tx clks must be available prior to assertion of tx req.
+	 * tx pstate p2 to p0 transition directly is not permitted.
+	 * tx clk ready must be asserted synchronously on tx clk prior
+	 * to internal transmit clk alignment sequence in the phy
+	 * when entering from p2 to p1 to p0.
+	 */
+	do {
+		exynosautov920_usb31drd_cr_clk(phy_drd, true);
+		exynosautov920_usb31drd_cr_clk(phy_drd, false);
+		cnt++;
+	} while (cnt < 15);
+
+	reg &= ~PHY_CR_PARA_CON0_PHY0_CR_PARA_SEL;
+	writel(reg, reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
+
+	/*
+	 * tx data path is active when tx lane is in p0 state
+	 * and tx data en asserted. enable cr_para_wr_en.
+	 */
+	reg = readl(reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON2);
+	reg &= ~PHY_CR_PARA_CON2_PHY0_CR_PARA_WR_DATA;
+	reg |= FIELD_PREP(PHY_CR_PARA_CON2_PHY0_CR_PARA_WR_DATA, data) |
+		PHY_CR_PARA_CON2_PHY0_CR_PARA_WR_EN;
+	writel(reg, reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON2);
+
+	/* write addr */
+	reg = readl(reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
+	reg &= ~PHY_CR_PARA_CON0_PHY0_CR_PARA_ADDR;
+	reg |= FIELD_PREP(PHY_CR_PARA_CON0_PHY0_CR_PARA_ADDR, addr) |
+		PHY_CR_PARA_CON0_PHY0_CR_PARA_CLK |
+		PHY_CR_PARA_CON0_PHY0_CR_PARA_SEL;
+	writel(reg, reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
+
+	/* check cr_para_ack*/
+	cnt = 0;
+	do {
+		/*
+		 * data symbols are captured by phy on rising edge of the
+		 * tx_clk when tx data enabled.
+		 * completion of the write cycle is acknowledged by assertion
+		 * of the cr_para_ack.
+		 */
+		exynosautov920_usb31drd_cr_clk(phy_drd, true);
+		reg = readl(reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
+		if ((reg & PHY_CR_PARA_CON0_PHY0_CR_PARA_ACK))
+			break;
+
+		exynosautov920_usb31drd_cr_clk(phy_drd, false);
+
+		/*
+		 * wait for minimum of 10 cr_para_clk cycles after phy reset
+		 * is negated, before accessing control regs to allow for
+		 * internal resets.
+		 */
+		cnt++;
+	} while (cnt < 10);
+
+	if (cnt == 10)
+		dev_dbg(dev, "CR write failed to 0x%04x\n", addr);
+	else
+		exynosautov920_usb31drd_cr_clk(phy_drd, false);
+}
+
+static void
+exynosautov920_usb31drd_phy_reset(struct exynos5_usbdrd_phy *phy_drd, int val)
+{
+	void __iomem *reg_phy = phy_drd->reg_phy;
+	u32 reg;
+
+	reg = readl(reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_RST_CTRL);
+	reg &= ~PHY_RST_CTRL_PHY_RESET_OVRD_EN;
+	writel(reg, reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_RST_CTRL);
+
+	reg = readl(reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_RST_CTRL);
+	if (val)
+		reg |= PHY_RST_CTRL_PHY_RESET;
+	else
+		reg &= ~PHY_RST_CTRL_PHY_RESET;
+
+	reg |= PHY_RST_CTRL_PHY_RESET_OVRD_EN;
+	writel(reg, reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_RST_CTRL);
+}
+
+static void
+exynosautov920_usb31drd_lane0_reset(struct exynos5_usbdrd_phy *phy_drd, int val)
+{
+	void __iomem *reg_phy = phy_drd->reg_phy;
+	u32 reg;
+
+	reg = readl(reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_RST_CTRL);
+	reg |= PHY_RST_CTRL_PIPE_LANE0_RESET_N_OVRD_EN;
+	writel(reg, reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_RST_CTRL);
+
+	reg = readl(reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_RST_CTRL);
+	if (val)
+		reg &= ~PHY_RST_CTRL_PIPE_LANE0_RESET_N;
+	else
+		reg |= PHY_RST_CTRL_PIPE_LANE0_RESET_N;
+
+	reg &= ~PHY_RST_CTRL_PIPE_LANE0_RESET_N_OVRD_EN;
+	writel(reg, reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_RST_CTRL);
+}
+
+static void
+exynosautov920_usb31drd_pipe3_init(struct exynos5_usbdrd_phy *phy_drd)
+{
+	void __iomem *reg_phy = phy_drd->reg_phy;
+	u32 reg;
+
+	/*
+	 * Phy and Pipe Lane reset assert.
+	 * assert reset (phy_reset = 1).
+	 * The lane-ack outputs are asserted during reset (tx_ack = rx_ack = 1)
+	 */
+	exynosautov920_usb31drd_phy_reset(phy_drd, 1);
+	exynosautov920_usb31drd_lane0_reset(phy_drd, 1);
+
+	/*
+	 * ANA Power En, PCS & PMA PWR Stable Set
+	 * ramp-up power suppiles
+	 */
+	reg = readl(reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CONFIG0);
+	reg |= PHY_CONFIG0_PHY0_ANA_PWR_EN | PHY_CONFIG0_PHY0_PCS_PWR_STABLE |
+		PHY_CONFIG0_PHY0_PMA_PWR_STABLE;
+	writel(reg, reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CONFIG0);
+
+	fsleep(10);
+
+	/*
+	 * phy is not functional in test_powerdown mode, test_powerdown to be
+	 * de-asserted for normal operation
+	 */
+	reg = readl(reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CONFIG7);
+	reg &= ~PHY_CONFIG7_PHY_TEST_POWERDOWN;
+	writel(reg, reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CONFIG7);
+
+	/*
+	 * phy reset signal be asserted for minimum 10us after power
+	 * supplies are ramped-up
+	 */
+	fsleep(10);
+
+	/*
+	 * Phy and Pipe Lane reset assert de-assert
+	 */
+	exynosautov920_usb31drd_phy_reset(phy_drd, 0);
+	exynosautov920_usb31drd_lane0_reset(phy_drd, 0);
+
+	/* Pipe_rx0_sris_mode_en  = 1 */
+	reg = readl(reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CONFIG4);
+	reg |= PHY_CONFIG4_PIPE_RX0_SRIS_MODE_EN;
+	writel(reg, reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CONFIG4);
+
+	/*
+	 * wait for lane ack outputs to de-assert (tx_ack = rx_ack = 0)
+	 * Exit from the reset state is indicated by de-assertion of *_ack
+	 */
+	exynosautov920_usb31drd_port_phy_ready(phy_drd);
+
+	/* override values for level settings */
+	exynosautov920_usb31drd_cr_write(phy_drd, 0x22, 0x00F5);
+}
+
+static void
+exynosautov920_usb31drd_ssphy_disable(struct exynos5_usbdrd_phy *phy_drd)
+{
+	void __iomem *reg_phy = phy_drd->reg_phy;
+	u32 reg;
+
+	/* 1. Assert reset (phy_reset = 1) */
+	exynosautov920_usb31drd_lane0_reset(phy_drd, 1);
+	exynosautov920_usb31drd_phy_reset(phy_drd, 1);
+
+	/* phy test power down */
+	reg = readl(reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CONFIG7);
+	reg |= PHY_CONFIG7_PHY_TEST_POWERDOWN;
+	writel(reg, reg_phy + EXYNOSAUTOV920_USB31DRD_PHY_CONFIG7);
+}
+
 static void
 exynosautov920_usbdrd_utmi_init(struct exynos5_usbdrd_phy *phy_drd)
 {
@@ -2173,12 +2450,15 @@ exynosautov920_usbdrd_utmi_init(struct exynos5_usbdrd_phy *phy_drd)
 	/* after POR low and delay 75us, PHYCLOCK is guaranteed. */
 	fsleep(75);
 
-	/* force pipe3 signal for link */
+	/* Disable forcing pipe interface */
 	reg = readl(reg_phy + EXYNOS850_DRD_LINKCTRL);
-	reg |= LINKCTRL_FORCE_PIPE_EN;
-	reg &= ~LINKCTRL_FORCE_PHYSTATUS;
-	reg |= LINKCTRL_FORCE_RXELECIDLE;
+	reg &= ~LINKCTRL_FORCE_PIPE_EN;
 	writel(reg, reg_phy + EXYNOS850_DRD_LINKCTRL);
+
+	/* Pclk to pipe_clk */
+	reg = readl(reg_phy + EXYNOS2200_DRD_CLKRST);
+	reg |= EXYNOS2200_CLKRST_LINK_PCLK_SEL;
+	writel(reg, reg_phy + EXYNOS2200_DRD_CLKRST);
 }
 
 static void
@@ -2226,6 +2506,8 @@ static int exynosautov920_usbdrd_phy_init(struct phy *phy)
 		/* Bypass PHY isol when first USB is powered on */
 		if ((atomic_inc_return(&usage_count) == 1))
 			inst->phy_cfg->phy_isol(inst, false);
+	} else if (inst->phy_cfg->id == EXYNOS5_DRDPHY_PIPE3) {
+		inst->phy_cfg->phy_isol(inst, false);
 	}
 
 	/* UTMI or PIPE3 specific init */
@@ -2275,6 +2557,10 @@ static int exynosautov920_usbdrd_combo_phy_exit(struct phy *phy)
 		/* enable PHY isol when all USBs are powered off */
 		if (atomic_dec_and_test(&usage_count))
 			inst->phy_cfg->phy_isol(inst, true);
+	} else if (inst->phy_cfg->id == EXYNOS5_DRDPHY_PIPE3) {
+		exynosautov920_usb31drd_ssphy_disable(phy_drd);
+
+		inst->phy_cfg->phy_isol(inst, true);
 	}
 
 	clk_bulk_disable_unprepare(phy_drd->drv_data->n_clks, phy_drd->clks);
@@ -2333,6 +2619,36 @@ static const char * const exynosautov920_regulator_names[] = {
 	"avdd075_usb", "avdd18_usb20", "avdd33_usb20",
 };
 
+static const struct
+exynos5_usbdrd_phy_config usb31drd_phy_cfg_exynosautov920[] = {
+	{
+		.id		= EXYNOS5_DRDPHY_PIPE3,
+		.phy_isol	= exynos5_usbdrd_phy_isol,
+		.phy_init	= exynosautov920_usb31drd_pipe3_init,
+	},
+};
+
+static const struct phy_ops exynosautov920_usb31drd_combo_ssphy_ops = {
+	.init		= exynosautov920_usbdrd_phy_init,
+	.exit		= exynosautov920_usbdrd_combo_phy_exit,
+	.power_on	= exynosautov920_usbdrd_phy_power_on,
+	.power_off	= exynosautov920_usbdrd_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static const
+struct exynos5_usbdrd_phy_drvdata exynosautov920_usb31drd_combo_ssphy = {
+	.phy_cfg		= usb31drd_phy_cfg_exynosautov920,
+	.phy_ops		= &exynosautov920_usb31drd_combo_ssphy_ops,
+	.pmu_offset_usbdrd0_phy	= EXYNOSAUTOV920_PHY_CTRL_USB31,
+	.clk_names		= exynos5_clk_names,
+	.n_clks			= ARRAY_SIZE(exynos5_clk_names),
+	.core_clk_names		= exynos5_core_clk_names,
+	.n_core_clks		= ARRAY_SIZE(exynos5_core_clk_names),
+	.regulator_names	= exynosautov920_regulator_names,
+	.n_regulators		= ARRAY_SIZE(exynosautov920_regulator_names),
+};
+
 static const struct phy_ops exynosautov920_usbdrd_combo_hsphy_ops = {
 	.init		= exynosautov920_usbdrd_phy_init,
 	.exit		= exynosautov920_usbdrd_combo_phy_exit,
@@ -2597,6 +2913,9 @@ static const struct of_device_id exynos5_usbdrd_phy_of_match[] = {
 	}, {
 		.compatible = "samsung,exynos990-usbdrd-phy",
 		.data = &exynos990_usbdrd_phy
+	}, {
+		.compatible = "samsung,exynosautov920-usb31drd-combo-ssphy",
+		.data = &exynosautov920_usb31drd_combo_ssphy
 	}, {
 		.compatible = "samsung,exynosautov920-usbdrd-combo-hsphy",
 		.data = &exynosautov920_usbdrd_combo_hsphy
diff --git a/include/linux/soc/samsung/exynos-regs-pmu.h b/include/linux/soc/samsung/exynos-regs-pmu.h
index 4923f9be3d1f..f96c773b85c9 100644
--- a/include/linux/soc/samsung/exynos-regs-pmu.h
+++ b/include/linux/soc/samsung/exynos-regs-pmu.h
@@ -690,4 +690,5 @@
 
 /* exynosautov920 */
 #define EXYNOSAUTOV920_PHY_CTRL_USB20				(0x0710)
+#define EXYNOSAUTOV920_PHY_CTRL_USB31				(0x0714)
 #endif /* __LINUX_SOC_EXYNOS_REGS_PMU_H */
-- 
2.34.1

Re: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 HS phy compatible

[Krzysztof Kozlowski]

On Tue, Jul 01, 2025 at 05:37:01PM +0530, Pritam Manohar Sutar wrote:
> Add a dedicated compatible string for USB HS phy found in this SoC.
> The SoC requires two clocks, named "phy" and "ref" (same as clocks
> required by Exynos850).
> 
> It also requires various power supplies (regulators) for the internal
> circuitry to work. The required voltages are:
> * avdd075_usb - 0.75v
> * avdd18_usb20 - 1.8v
> * avdd33_usb20 - 3.3v
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

No, really. Look:

> Signed-off-by: Pritam Manohar Sutar <pritam.sutar@samsung.com>
> ---
>  .../bindings/phy/samsung,usb3-drd-phy.yaml    | 37 +++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
> index e906403208c0..2e29ff749bba 100644
> --- a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
> @@ -34,6 +34,7 @@ properties:
>        - samsung,exynos7870-usbdrd-phy
>        - samsung,exynos850-usbdrd-phy
>        - samsung,exynos990-usbdrd-phy
> +      - samsung,exynosautov920-usbdrd-phy
>  
>    clocks:
>      minItems: 1
> @@ -110,6 +111,15 @@ properties:
>    vddh-usbdp-supply:
>      description: VDDh power supply for the USB DP phy.
>  
> +  avdd075_usb-supply:
> +    description: 0.75V power supply for USB phy
> +
> +  avdd18_usb20-supply:
> +    description: 1.8V power supply for USB phy
> +
> +  avdd33_usb20-supply:
> +    description: 3.3V power supply for USB phy
> +

None of these were here. Follow DTS coding style... but why are you
adding completely new supplies?


>  required:
>    - compatible
>    - clocks
> @@ -235,6 +245,33 @@ allOf:
>  
>          reg-names:
>            maxItems: 1
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - samsung,exynosautov920-usbdrd-phy
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 2
> +          maxItems: 2
> +
> +        clock-names:
> +          items:
> +            - const: phy
> +            - const: ref
> +
> +        reg:
> +          maxItems: 1
> +
> +        reg-names:
> +          maxItems: 1
> +
> +      required:
> +        - avdd075_usb-supply
> +        - avdd18_usb20-supply
> +        - avdd33_usb20-supply

Neither was this entire diff hunk here.

This was part of other block for a reason.

NAK

Best regards,
Krzysztof

Re: [PATCH v4 3/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 combo HS phy

[Krzysztof Kozlowski]

On Tue, Jul 01, 2025 at 05:37:03PM +0530, Pritam Manohar Sutar wrote:
> This phy supports USB3.1 SSP+(10Gbps) protocol and is backwards

What is "this"? You add here HS PHY, so HS is 3.1?

If this is the same phy, why are you adding another compatible?

Best regards,
Krzysztof

Re: [PATCH v4 5/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 combo SS phy

[Krzysztof Kozlowski]

On Tue, Jul 01, 2025 at 05:37:05PM +0530, Pritam Manohar Sutar wrote:
> This phy supports USB3.1 SSP+(10Gbps) protocol and is backwards

Agian, this?

> compatible to the USB3.0 SS(5Gbps). 'Add-on USB2.0' phy is required
> to support USB2.0 HS(480Mbps), FS(12Mbps) and LS(1.5Mbps) data rates.
> These two phys are combined to form a combo phy.
> 
> Add a dedicated compatible string for USB combo SS phy found in this
> SoC. The SoC requires two clocks, named "phy" and "ref" and various
> power supplies (regulators) for the internal circuitry to work.
> The required voltages are:
> * avdd075_usb - 0.75v
> * avdd18_usb20 - 1.8v
> * avdd33_usb20 - 3.3v

One more commitm message completely copy-pasted and completely
uninforming. The voltages are irrelevant. Explain the architecture. This
should be just one patch with proper full description.

> 
> Add schema only for 'USB3.1 SSP+' SS phy in this commit.

Why only? Add everything, describe everything, but not what voltages you
have there but the architecture of the PHY.

Best regards,
Krzysztof

RE: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 HS phy compatible

[Pritam Manohar Sutar]

Hi Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: 06 July 2025 03:11 PM
> To: Pritam Manohar Sutar <pritam.sutar@samsung.com>
> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; alim.akhtar@samsung.com;
> andre.draszik@linaro.org; peter.griffin@linaro.org; neil.armstrong@linaro.org;
> kauschluss@disroot.org; ivo.ivanov.ivanov1@gmail.com;
> m.szyprowski@samsung.com; s.nawrocki@samsung.com; linux-
> phy@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-
> soc@vger.kernel.org; rosa.pila@samsung.com; dev.tailor@samsung.com;
> faraz.ata@samsung.com; muhammed.ali@samsung.com;
> selvarasu.g@samsung.com
> Subject: Re: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add
> ExynosAutov920 HS phy compatible
> 
> On Tue, Jul 01, 2025 at 05:37:01PM +0530, Pritam Manohar Sutar wrote:
> > Add a dedicated compatible string for USB HS phy found in this SoC.
> > The SoC requires two clocks, named "phy" and "ref" (same as clocks
> > required by Exynos850).
> >
> > It also requires various power supplies (regulators) for the internal
> > circuitry to work. The required voltages are:
> > * avdd075_usb - 0.75v
> > * avdd18_usb20 - 1.8v
> > * avdd33_usb20 - 3.3v
> >
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> No, really. Look:
> 
> > Signed-off-by: Pritam Manohar Sutar <pritam.sutar@samsung.com>
> > ---
> >  .../bindings/phy/samsung,usb3-drd-phy.yaml    | 37 +++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
> > b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
> > index e906403208c0..2e29ff749bba 100644
> > --- a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
> > @@ -34,6 +34,7 @@ properties:
> >        - samsung,exynos7870-usbdrd-phy
> >        - samsung,exynos850-usbdrd-phy
> >        - samsung,exynos990-usbdrd-phy
> > +      - samsung,exynosautov920-usbdrd-phy
> >
> >    clocks:
> >      minItems: 1
> > @@ -110,6 +111,15 @@ properties:
> >    vddh-usbdp-supply:
> >      description: VDDh power supply for the USB DP phy.
> >
> > +  avdd075_usb-supply:
> > +    description: 0.75V power supply for USB phy
> > +
> > +  avdd18_usb20-supply:
> > +    description: 1.8V power supply for USB phy
> > +
> > +  avdd33_usb20-supply:
> > +    description: 3.3V power supply for USB phy
> > +
> 
> None of these were here. Follow DTS coding style... but why are you adding
> completely new supplies?

Digital supplies were here. Need Analog supplies for exynosautov920, hence added new Analog supplies; 'a'vdd075_usb, 'a'vdd18_usb20, 'a'vdd33_usb20

Will add "full stops" for DTS coding style in description.

> 
> 
> >  required:
> >    - compatible
> >    - clocks
> > @@ -235,6 +245,33 @@ allOf:
> >
> >          reg-names:
> >            maxItems: 1
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - samsung,exynosautov920-usbdrd-phy
> > +    then:
> > +      properties:
> > +        clocks:
> > +          minItems: 2
> > +          maxItems: 2
> > +
> > +        clock-names:
> > +          items:
> > +            - const: phy
> > +            - const: ref
> > +
> > +        reg:
> > +          maxItems: 1
> > +
> > +        reg-names:
> > +          maxItems: 1
> > +
> > +      required:
> > +        - avdd075_usb-supply
> > +        - avdd18_usb20-supply
> > +        - avdd33_usb20-supply
> 
> Neither was this entire diff hunk here.
> 
> This was part of other block for a reason.

Added regulators in driver. This block is added for regulators (other block does not have "required" field for power supplies)
if excluded this block,  
"make ARCH=arm64 dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml" will fail as mentioned below 
		
/home/pritam.sutar/workspace/auto/kitt2/projects/upstream/src/kernel/github/linux-next/arch/arm64/boot/dts/exynos/exynosautov920-sadk.dtb: phy@16480000: Unevaluated properties are not allowed ('avdd075_usb-supply', 'avdd18_usb20-supply', 'avdd33_usb20-supply' were unexpected)
	from schema $id: http://devicetree.org/schemas/phy/samsung,usb3-drd-phy.yaml#
/home/pritam.sutar/workspace/auto/kitt2/projects/upstream/src/kernel/github/linux-next/arch/arm64/boot/dts/exynos/exynosautov920-sadk.dtb: phy@16490000: Unevaluated properties are not allowed ('avdd075_usb-supply', 'avdd18_usb20-supply', 'avdd33_usb20-supply' were unexpected)
		from schema $id: http://devicetree.org/schemas/phy/samsung,usb3-drd-phy.yaml#
/home/pritam.sutar/workspace/auto/kitt2/projects/upstream/src/kernel/github/linux-next/arch/arm64/boot/dts/exynos/exynosautov920-sadk.dtb: phy@16500000: Unevaluated properties are not allowed ('avdd075_usb-supply', 'avdd18_usb20-supply', 'avdd33_usb20-supply' were unexpected)
		from schema $id: http://devicetree.org/schemas/phy/samsung,usb3-drd-phy.yaml#
/home/pritam.sutar/workspace/auto/kitt2/projects/upstream/src/kernel/github/linux-next/arch/arm64/boot/dts/exynos/exynosautov920-sadk.dtb: phy@16510000: Unevaluated properties are not allowed ('avdd075_usb-supply', 'avdd18_usb20-supply', 'avdd33_usb20-supply' were unexpected)
		from schema $id: http://devicetree.org/schemas/phy/samsung,usb3-drd-phy.yaml#
/home/pritam.sutar/workspace/auto/kitt2/projects/upstream/src/kernel/github/linux-next/arch/arm64/boot/dts/exynos/exynosautov920-sadk.dtb: phy@16520000: Unevaluated properties are not allowed ('avdd075_usb-supply', 'avdd18_usb20-supply', 'avdd33_usb20-supply' were unexpected)
		from schema $id: http://devicetree.org/schemas/phy/samsung,usb3-drd-phy.yaml#

Note: These patches are being validated by enabling DTS (However, DTS patches are planned in next phase).

If you have any idea to re-use existing block by differentiating required power supplies, please suggest.

> 
> NAK
> 
> Best regards,
> Krzysztof

RE: [PATCH v4 3/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 combo HS phy

[Pritam Manohar Sutar]

Hi Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 06 July 2025 03:12 PM
> To: Pritam Manohar Sutar <pritam.sutar@samsung.com>
> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; alim.akhtar@samsung.com;
> andre.draszik@linaro.org; peter.griffin@linaro.org; neil.armstrong@linaro.org;
> kauschluss@disroot.org; ivo.ivanov.ivanov1@gmail.com;
> m.szyprowski@samsung.com; s.nawrocki@samsung.com; linux-
> phy@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-
> soc@vger.kernel.org; rosa.pila@samsung.com; dev.tailor@samsung.com;
> faraz.ata@samsung.com; muhammed.ali@samsung.com;
> selvarasu.g@samsung.com
> Subject: Re: [PATCH v4 3/6] dt-bindings: phy: samsung,usb3-drd-phy: add
> ExynosAutov920 combo HS phy
> 
> On Tue, Jul 01, 2025 at 05:37:03PM +0530, Pritam Manohar Sutar wrote:
> > This phy supports USB3.1 SSP+(10Gbps) protocol and is backwards
> 
> What is "this"? You add here HS PHY, so HS is 3.1?
> 

"this" means "combo phy". Combo phy is combination of 2 types of the phys. 
1. one supports only USB3.1 SSP+ and denoted as "samsung,exynosautov920-usb31drd-combo-ssphy" as in patch no 5 (combo SS phy)
2. another supports only USB2.0 HS and termed as "samsung,exynosautov920-usbdrd-combo-hsphy" as in this patch (combo HS phy)

> If this is the same phy, why are you adding another compatible?

As explained earlier (even in cover letter), there are 3 types of the phys in this SoC.
1. one is simmilar with exynos850 and termed as "samsung,exynosautov920-usbdrd-phy" as mentioned in patch no.1
2. two phys are in combo phys as explained above (patch no 3 [combo HS phy] and 5[combo SS phy])
			
NOTE: hs phy in combo phy in "NOT" same as phy (patch no. 1 which is similar with exynos850). 
			
These three phys (usbdrd-phy, combo-hsphy, combo-ssphy) are totally deferent "NOT" same, hence added 3 three compatible for three phys.

> 
> Best regards,
> Krzysztof

RE: [PATCH v4 5/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 combo SS phy

[Pritam Manohar Sutar]

Hi Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 06 July 2025 03:14 PM
> To: Pritam Manohar Sutar <pritam.sutar@samsung.com>
> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; alim.akhtar@samsung.com;
> andre.draszik@linaro.org; peter.griffin@linaro.org; neil.armstrong@linaro.org;
> kauschluss@disroot.org; ivo.ivanov.ivanov1@gmail.com;
> m.szyprowski@samsung.com; s.nawrocki@samsung.com; linux-
> phy@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-
> soc@vger.kernel.org; rosa.pila@samsung.com; dev.tailor@samsung.com;
> faraz.ata@samsung.com; muhammed.ali@samsung.com;
> selvarasu.g@samsung.com
> Subject: Re: [PATCH v4 5/6] dt-bindings: phy: samsung,usb3-drd-phy: add
> ExynosAutov920 combo SS phy
> 
> On Tue, Jul 01, 2025 at 05:37:05PM +0530, Pritam Manohar Sutar wrote:
> > This phy supports USB3.1 SSP+(10Gbps) protocol and is backwards
> 
> Agian, this?
> 
> > compatible to the USB3.0 SS(5Gbps). 'Add-on USB2.0' phy is required to
> > support USB2.0 HS(480Mbps), FS(12Mbps) and LS(1.5Mbps) data rates.
> > These two phys are combined to form a combo phy.
> >
> > Add a dedicated compatible string for USB combo SS phy found in this
> > SoC. The SoC requires two clocks, named "phy" and "ref" and various
> > power supplies (regulators) for the internal circuitry to work.
> > The required voltages are:
> > * avdd075_usb - 0.75v
> > * avdd18_usb20 - 1.8v
> > * avdd33_usb20 - 3.3v
> 
> One more commitm message completely copy-pasted and completely
> uninforming. The voltages are irrelevant. Explain the architecture. This should be
> just one patch with proper full description.
> 
> >
> > Add schema only for 'USB3.1 SSP+' SS phy in this commit.
> 
> Why only? Add everything, describe everything, but not what voltages you have
> there but the architecture of the PHY.
> 

Will combine patch 3 (combo HS phy) & 5(combo SS phy) to describe combo phy and even will add some details as mentioned in cover letter.

> Best regards,
> Krzysztof

RE: [PATCH v4 2/6] phy: exynos5-usbdrd: support HS phy for ExynosAutov920

[Pritam Manohar Sutar]

Hi Neil,

> -----Original Message-----
> From: Pritam Manohar Sutar <pritam.sutar@samsung.com>
> Sent: 01 July 2025 05:37 PM
> To: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; alim.akhtar@samsung.com;
> andre.draszik@linaro.org; peter.griffin@linaro.org; neil.armstrong@linaro.org;
> kauschluss@disroot.org; ivo.ivanov.ivanov1@gmail.com;
> m.szyprowski@samsung.com; s.nawrocki@samsung.com;
> pritam.sutar@samsung.com
> Cc: linux-phy@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-
> soc@vger.kernel.org; rosa.pila@samsung.com; dev.tailor@samsung.com;
> faraz.ata@samsung.com; muhammed.ali@samsung.com;
> selvarasu.g@samsung.com
> Subject: [PATCH v4 2/6] phy: exynos5-usbdrd: support HS phy for
> ExynosAutov920
> 
> This SoC has a single USB 3.1 DRD combo phy that supports both
> UTMI+ (HS) and PIPE3 (SS) and three USB2.0 DRD HS phy controllers
> those only support the UTMI+ (HS) interface.
> 
> Support only UTMI+ port for this SoC which is very similar to what the existing
> Exynos850 supports.
> 
> This SoC shares phy isol between USBs. Bypass PHY isol when first USB is
> powered on and enable it when all of then are powered off. Add required
> change in phy driver to support HS phy for this SoC.
> 
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
> Signed-off-by: Pritam Manohar Sutar <pritam.sutar@samsung.com>
> ---
>  drivers/phy/samsung/phy-exynos5-usbdrd.c    | 131 ++++++++++++++++++++
>  include/linux/soc/samsung/exynos-regs-pmu.h |   2 +
>  2 files changed, 133 insertions(+)
> 
> diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> index dd660ebe8045..64f3316f6ad4 100644
> --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> @@ -480,6 +480,8 @@ struct exynos5_usbdrd_phy {
>  	enum typec_orientation orientation;
>  };
> 
> +static atomic_t usage_count = ATOMIC_INIT(0);
> +

Added phy isolation for exynosautov920 support as per comments on v3 patch-set.
Since phy isol is shared across usb ports, added usage counter to bypass and enable it. 

Can you please review the code? 

However, added "Reviewed-by" tag as per comments on v3 patch-set.

>  static inline
>  struct exynos5_usbdrd_phy *to_usbdrd_phy(struct phy_usb_instance *inst)  {
> @@ -2054,6 +2056,132 @@ static const struct exynos5_usbdrd_phy_drvdata
> exynos990_usbdrd_phy = {
>  	.n_regulators		= ARRAY_SIZE(exynos5_regulator_names),
>  };
> 
> +static int exynosautov920_usbdrd_phy_init(struct phy *phy) {
> +	struct phy_usb_instance *inst = phy_get_drvdata(phy);
> +	struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
> +	int ret;
> +
> +	ret = clk_bulk_prepare_enable(phy_drd->drv_data->n_clks, phy_drd-
> >clks);
> +	if (ret)
> +		return ret;
> +
> +	if (inst->phy_cfg->id == EXYNOS5_DRDPHY_UTMI) {
> +		/* Bypass PHY isol when first USB is powered on */
> +		if ((atomic_inc_return(&usage_count) == 1))
> +			inst->phy_cfg->phy_isol(inst, false);
> +	}
> +
> +	/* UTMI or PIPE3 specific init */
> +	inst->phy_cfg->phy_init(phy_drd);
> +
> +	clk_bulk_disable_unprepare(phy_drd->drv_data->n_clks, phy_drd->clks);
> +
> +	return 0;
> +}
> +
> +static int exynosautov920_usbdrd_phy_exit(struct phy *phy) {
> +	struct phy_usb_instance *inst = phy_get_drvdata(phy);
> +	struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
> +	int ret = 0;
> +
> +	ret = clk_bulk_prepare_enable(phy_drd->drv_data->n_clks, phy_drd-
> >clks);
> +	if (ret)
> +		return ret;
> +
> +	if (inst->phy_cfg->id == EXYNOS5_DRDPHY_UTMI) {
> +		exynos850_usbdrd_phy_exit(phy);
> +
> +		/* enable PHY isol when all USBs are powered off */
> +		if (atomic_dec_and_test(&usage_count))
> +			inst->phy_cfg->phy_isol(inst, true);
> +	}
> +
> +	clk_bulk_disable_unprepare(phy_drd->drv_data->n_clks, phy_drd->clks);
> +
> +	return 0;
> +}
> +
> +static int exynosautov920_usbdrd_phy_power_on(struct phy *phy) {
> +	int ret;
> +	struct phy_usb_instance *inst = phy_get_drvdata(phy);
> +	struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
> +
> +	dev_dbg(phy_drd->dev, "Request to power_on usbdrd_phy phy\n");
> +
> +	ret = clk_bulk_prepare_enable(phy_drd->drv_data->n_core_clks,
> +				      phy_drd->core_clks);
> +	if (ret)
> +		return ret;
> +
> +	/* Enable supply */
> +	ret = regulator_bulk_enable(phy_drd->drv_data->n_regulators,
> +				    phy_drd->regulators);
> +	if (ret) {
> +		dev_err(phy_drd->dev, "Failed to enable PHY regulator(s)\n");
> +		goto fail_supply;
> +	}
> +
> +	return 0;
> +
> +fail_supply:
> +	clk_bulk_disable_unprepare(phy_drd->drv_data->n_core_clks,
> +				   phy_drd->core_clks);
> +
> +	return ret;
> +}
> +
> +static int exynosautov920_usbdrd_phy_power_off(struct phy *phy) {
> +	struct phy_usb_instance *inst = phy_get_drvdata(phy);
> +	struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
> +
> +	dev_dbg(phy_drd->dev, "Request to power_off usbdrd_phy phy\n");
> +
> +	/* Disable supply */
> +	regulator_bulk_disable(phy_drd->drv_data->n_regulators,
> +			       phy_drd->regulators);
> +
> +	clk_bulk_disable_unprepare(phy_drd->drv_data->n_core_clks,
> +				   phy_drd->core_clks);
> +
> +	return 0;
> +}
> +
> +static const char * const exynosautov920_regulator_names[] = {
> +	"avdd075_usb", "avdd18_usb20", "avdd33_usb20", };
> +
> +static const struct phy_ops exynosautov920_usbdrd_phy_ops = {
> +	.init		= exynosautov920_usbdrd_phy_init,
> +	.exit		= exynosautov920_usbdrd_phy_exit,
> +	.power_on       = exynosautov920_usbdrd_phy_power_on,
> +	.power_off      = exynosautov920_usbdrd_phy_power_off,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static const struct exynos5_usbdrd_phy_config phy_cfg_exynosautov920[] = {
> +	{
> +		.id		= EXYNOS5_DRDPHY_UTMI,
> +		.phy_isol	= exynos5_usbdrd_phy_isol,
> +		.phy_init	= exynos850_usbdrd_utmi_init,
> +	},
> +};
> +
> +static const struct exynos5_usbdrd_phy_drvdata exynosautov920_usbdrd_phy
> = {
> +	.phy_cfg		= phy_cfg_exynosautov920,
> +	.phy_ops		= &exynosautov920_usbdrd_phy_ops,
> +	.pmu_offset_usbdrd0_phy	=
> EXYNOSAUTOV920_PHY_CTRL_USB20,
> +	.clk_names		= exynos5_clk_names,
> +	.n_clks			= ARRAY_SIZE(exynos5_clk_names),
> +	.core_clk_names		= exynos5_core_clk_names,
> +	.n_core_clks		= ARRAY_SIZE(exynos5_core_clk_names),
> +	.regulator_names	= exynosautov920_regulator_names,
> +	.n_regulators		=
> ARRAY_SIZE(exynosautov920_regulator_names),
> +};
> +
>  static const struct exynos5_usbdrd_phy_config phy_cfg_gs101[] = {
>  	{
>  		.id		= EXYNOS5_DRDPHY_UTMI,
> @@ -2260,6 +2388,9 @@ static const struct of_device_id
> exynos5_usbdrd_phy_of_match[] = {
>  	}, {
>  		.compatible = "samsung,exynos990-usbdrd-phy",
>  		.data = &exynos990_usbdrd_phy
> +	}, {
> +		.compatible = "samsung,exynosautov920-usbdrd-phy",
> +		.data = &exynosautov920_usbdrd_phy
>  	},
>  	{ },
>  };
> diff --git a/include/linux/soc/samsung/exynos-regs-pmu.h
> b/include/linux/soc/samsung/exynos-regs-pmu.h
> index 71e0c09a49eb..4923f9be3d1f 100644
> --- a/include/linux/soc/samsung/exynos-regs-pmu.h
> +++ b/include/linux/soc/samsung/exynos-regs-pmu.h
> @@ -688,4 +688,6 @@
>  #define GS101_GRP2_INTR_BID_UPEND
> 	(0x0208)
>  #define GS101_GRP2_INTR_BID_CLEAR				(0x020c)
> 
> +/* exynosautov920 */
> +#define EXYNOSAUTOV920_PHY_CTRL_USB20
> 	(0x0710)
>  #endif /* __LINUX_SOC_EXYNOS_REGS_PMU_H */
> --
> 2.34.1

Thank you.

Regards,
Pritam

Re: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 HS phy compatible

[Krzysztof Kozlowski]

On 09/07/2025 10:46, Pritam Manohar Sutar wrote:
> Hi Krzysztof,
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: 06 July 2025 03:11 PM
>> To: Pritam Manohar Sutar <pritam.sutar@samsung.com>
>> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
>> krzk+dt@kernel.org; conor+dt@kernel.org; alim.akhtar@samsung.com;
>> andre.draszik@linaro.org; peter.griffin@linaro.org; neil.armstrong@linaro.org;
>> kauschluss@disroot.org; ivo.ivanov.ivanov1@gmail.com;
>> m.szyprowski@samsung.com; s.nawrocki@samsung.com; linux-
>> phy@lists.infradead.org; devicetree@vger.kernel.org; linux-
>> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-
>> soc@vger.kernel.org; rosa.pila@samsung.com; dev.tailor@samsung.com;
>> faraz.ata@samsung.com; muhammed.ali@samsung.com;
>> selvarasu.g@samsung.com
>> Subject: Re: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add
>> ExynosAutov920 HS phy compatible
>>
>> On Tue, Jul 01, 2025 at 05:37:01PM +0530, Pritam Manohar Sutar wrote:
>>> Add a dedicated compatible string for USB HS phy found in this SoC.
>>> The SoC requires two clocks, named "phy" and "ref" (same as clocks
>>> required by Exynos850).
>>>
>>> It also requires various power supplies (regulators) for the internal
>>> circuitry to work. The required voltages are:
>>> * avdd075_usb - 0.75v
>>> * avdd18_usb20 - 1.8v
>>> * avdd33_usb20 - 3.3v
>>>
>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>
>> No, really. Look:
>>
>>> Signed-off-by: Pritam Manohar Sutar <pritam.sutar@samsung.com>
>>> ---
>>>  .../bindings/phy/samsung,usb3-drd-phy.yaml    | 37 +++++++++++++++++++
>>>  1 file changed, 37 insertions(+)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
>>> b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
>>> index e906403208c0..2e29ff749bba 100644
>>> --- a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
>>> +++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
>>> @@ -34,6 +34,7 @@ properties:
>>>        - samsung,exynos7870-usbdrd-phy
>>>        - samsung,exynos850-usbdrd-phy
>>>        - samsung,exynos990-usbdrd-phy
>>> +      - samsung,exynosautov920-usbdrd-phy
>>>
>>>    clocks:
>>>      minItems: 1
>>> @@ -110,6 +111,15 @@ properties:
>>>    vddh-usbdp-supply:
>>>      description: VDDh power supply for the USB DP phy.
>>>
>>> +  avdd075_usb-supply:
>>> +    description: 0.75V power supply for USB phy
>>> +
>>> +  avdd18_usb20-supply:
>>> +    description: 1.8V power supply for USB phy
>>> +
>>> +  avdd33_usb20-supply:
>>> +    description: 3.3V power supply for USB phy
>>> +
>>
>> None of these were here. Follow DTS coding style... but why are you adding
>> completely new supplies?
> 
> Digital supplies were here. Need Analog supplies for exynosautov920, hence added new Analog supplies; 'a'vdd075_usb, 'a'vdd18_usb20, 'a'vdd33_usb20
> 
> Will add "full stops" for DTS coding style in description.

You cannot grow one line change into 50 line change and retain the review.
> 
>>
>>
>>>  required:
>>>    - compatible
>>>    - clocks
>>> @@ -235,6 +245,33 @@ allOf:
>>>
>>>          reg-names:
>>>            maxItems: 1
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - samsung,exynosautov920-usbdrd-phy
>>> +    then:
>>> +      properties:
>>> +        clocks:
>>> +          minItems: 2
>>> +          maxItems: 2
>>> +
>>> +        clock-names:
>>> +          items:
>>> +            - const: phy
>>> +            - const: ref
>>> +
>>> +        reg:
>>> +          maxItems: 1
>>> +
>>> +        reg-names:
>>> +          maxItems: 1
>>> +
>>> +      required:
>>> +        - avdd075_usb-supply
>>> +        - avdd18_usb20-supply
>>> +        - avdd33_usb20-supply
>>
>> Neither was this entire diff hunk here.
>>
>> This was part of other block for a reason.
> 
> Added regulators in driver. This block is added for regulators (other block does not have "required" field for power supplies)
> if excluded this block,  
> "make ARCH=arm64 dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml" will fail as mentioned below 


Nothing is explained in changelog/cover letter. You claim you only added
Rb tag. This is an entirely silent change while keeping the review.
Combined with not even following DTS style!

It's not acceptable.

Best regards,
Krzysztof

Re: [PATCH v4 2/6] phy: exynos5-usbdrd: support HS phy for ExynosAutov920

[Krzysztof Kozlowski]

On 01/07/2025 14:07, Pritam Manohar Sutar wrote:
> This SoC has a single USB 3.1 DRD combo phy that supports both
> UTMI+ (HS) and PIPE3 (SS) and three USB2.0 DRD HS phy controllers
> those only support the UTMI+ (HS) interface.
> 
> Support only UTMI+ port for this SoC which is very similar to what
> the existing Exynos850 supports.
> 
> This SoC shares phy isol between USBs. Bypass PHY isol when first USB
> is powered on and enable it when all of then are powered off. Add
> required change in phy driver to support HS phy for this SoC.
> 
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Drop

You again added significant changes and claimed they were reviewed.

> Signed-off-by: Pritam Manohar Sutar <pritam.sutar@samsung.com>
> ---
>  drivers/phy/samsung/phy-exynos5-usbdrd.c    | 131 ++++++++++++++++++++
>  include/linux/soc/samsung/exynos-regs-pmu.h |   2 +
>  2 files changed, 133 insertions(+)
> 
> diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> index dd660ebe8045..64f3316f6ad4 100644
> --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> @@ -480,6 +480,8 @@ struct exynos5_usbdrd_phy {
>  	enum typec_orientation orientation;
>  };
>  
> +static atomic_t usage_count = ATOMIC_INIT(0);

No, you cannot have singletons. How are you going to handle two
independent phys?

Best regards,
Krzysztof

RE: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 HS phy compatible

[Pritam Manohar Sutar]

Hi Krzysztof, 

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 12 July 2025 01:44 PM
> To: Pritam Manohar Sutar <pritam.sutar@samsung.com>; 'Krzysztof Kozlowski'
> <krzysztof.kozlowski@linaro.org>
> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; alim.akhtar@samsung.com;
> andre.draszik@linaro.org; peter.griffin@linaro.org; neil.armstrong@linaro.org;
> kauschluss@disroot.org; ivo.ivanov.ivanov1@gmail.com;
> m.szyprowski@samsung.com; s.nawrocki@samsung.com; linux-
> phy@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-
> soc@vger.kernel.org; rosa.pila@samsung.com; dev.tailor@samsung.com;
> faraz.ata@samsung.com; muhammed.ali@samsung.com;
> selvarasu.g@samsung.com
> Subject: Re: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add
> ExynosAutov920 HS phy compatible
> 
> On 09/07/2025 10:46, Pritam Manohar Sutar wrote:
> > Hi Krzysztof,
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> Sent: 06 July 2025 03:11 PM
> >> To: Pritam Manohar Sutar <pritam.sutar@samsung.com>
> >> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> >> krzk+dt@kernel.org; conor+dt@kernel.org; alim.akhtar@samsung.com;
> >> andre.draszik@linaro.org; peter.griffin@linaro.org;
> >> neil.armstrong@linaro.org; kauschluss@disroot.org;
> >> ivo.ivanov.ivanov1@gmail.com; m.szyprowski@samsung.com;
> >> s.nawrocki@samsung.com; linux- phy@lists.infradead.org;
> >> devicetree@vger.kernel.org; linux- kernel@vger.kernel.org;
> >> linux-arm-kernel@lists.infradead.org; linux-samsung-
> >> soc@vger.kernel.org; rosa.pila@samsung.com; dev.tailor@samsung.com;
> >> faraz.ata@samsung.com; muhammed.ali@samsung.com;
> >> selvarasu.g@samsung.com
> >> Subject: Re: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy:
> >> add
> >> ExynosAutov920 HS phy compatible
> >>
> >> On Tue, Jul 01, 2025 at 05:37:01PM +0530, Pritam Manohar Sutar wrote:
> >>> Add a dedicated compatible string for USB HS phy found in this SoC.
> >>> The SoC requires two clocks, named "phy" and "ref" (same as clocks
> >>> required by Exynos850).
> >>>
> >>> It also requires various power supplies (regulators) for the
> >>> internal circuitry to work. The required voltages are:
> >>> * avdd075_usb - 0.75v
> >>> * avdd18_usb20 - 1.8v
> >>> * avdd33_usb20 - 3.3v
> >>>
> >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>
> >> No, really. Look:
> >>
> >>> Signed-off-by: Pritam Manohar Sutar <pritam.sutar@samsung.com>
> >>> ---
> >>>  .../bindings/phy/samsung,usb3-drd-phy.yaml    | 37 +++++++++++++++++++
> >>>  1 file changed, 37 insertions(+)
> >>>
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
> >>> b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
> >>> index e906403208c0..2e29ff749bba 100644
> >>> ---
> >>> a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
> >>> +++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yam
> >>> +++ l
> >>> @@ -34,6 +34,7 @@ properties:
> >>>        - samsung,exynos7870-usbdrd-phy
> >>>        - samsung,exynos850-usbdrd-phy
> >>>        - samsung,exynos990-usbdrd-phy
> >>> +      - samsung,exynosautov920-usbdrd-phy
> >>>
> >>>    clocks:
> >>>      minItems: 1
> >>> @@ -110,6 +111,15 @@ properties:
> >>>    vddh-usbdp-supply:
> >>>      description: VDDh power supply for the USB DP phy.
> >>>
> >>> +  avdd075_usb-supply:
> >>> +    description: 0.75V power supply for USB phy
> >>> +
> >>> +  avdd18_usb20-supply:
> >>> +    description: 1.8V power supply for USB phy
> >>> +
> >>> +  avdd33_usb20-supply:
> >>> +    description: 3.3V power supply for USB phy
> >>> +
> >>
> >> None of these were here. Follow DTS coding style... but why are you
> >> adding completely new supplies?
> >
> > Digital supplies were here. Need Analog supplies for exynosautov920,
> > hence added new Analog supplies; 'a'vdd075_usb, 'a'vdd18_usb20,
> > 'a'vdd33_usb20
> >
> > Will add "full stops" for DTS coding style in description.
> 
> You cannot grow one line change into 50 line change and retain the review.

Yes agreed. Will remove "Reviewed-by" tag and requesting you to review the 
patches again and provide your valuable comments.

> >
> >>
> >>
> >>>  required:
> >>>    - compatible
> >>>    - clocks
> >>> @@ -235,6 +245,33 @@ allOf:
> >>>
> >>>          reg-names:
> >>>            maxItems: 1
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          contains:
> >>> +            enum:
> >>> +              - samsung,exynosautov920-usbdrd-phy
> >>> +    then:
> >>> +      properties:
> >>> +        clocks:
> >>> +          minItems: 2
> >>> +          maxItems: 2
> >>> +
> >>> +        clock-names:
> >>> +          items:
> >>> +            - const: phy
> >>> +            - const: ref
> >>> +
> >>> +        reg:
> >>> +          maxItems: 1
> >>> +
> >>> +        reg-names:
> >>> +          maxItems: 1
> >>> +
> >>> +      required:
> >>> +        - avdd075_usb-supply
> >>> +        - avdd18_usb20-supply
> >>> +        - avdd33_usb20-supply
> >>
> >> Neither was this entire diff hunk here.
> >>
> >> This was part of other block for a reason.
> >
> > Added regulators in driver. This block is added for regulators (other
> > block does not have "required" field for power supplies) if excluded
> > this block, "make ARCH=arm64 dtbs_check
> > DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/samsung,usb3-
> drd
> > -phy.yaml" will fail as mentioned below
> 
> 
> Nothing is explained in changelog/cover letter. You claim you only added Rb tag.
> This is an entirely silent change while keeping the review.

Will add more explanations in cover letter/changelog why this block is added.

> Combined with not even following DTS style!

Ok got it. Will change supplies name as below 
avdd075_usb => avdd075-usb
avdd18_usb20 => avdd18-usb20
avdd33_usb20 => avdd33-usb20
   
Confirm the above change that is meant in terms of DTS style.

> 
> It's not acceptable.
> 
> Best regards,
> Krzysztof

Thank you.

Regards,
Pritam

RE: [PATCH v4 2/6] phy: exynos5-usbdrd: support HS phy for ExynosAutov920

[Pritam Manohar Sutar]

Hi Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 12 July 2025 01:46 PM
> To: Pritam Manohar Sutar <pritam.sutar@samsung.com>; vkoul@kernel.org;
> kishon@kernel.org; robh@kernel.org; krzk+dt@kernel.org;
> conor+dt@kernel.org; alim.akhtar@samsung.com; andre.draszik@linaro.org;
> peter.griffin@linaro.org; neil.armstrong@linaro.org; kauschluss@disroot.org;
> ivo.ivanov.ivanov1@gmail.com; m.szyprowski@samsung.com;
> s.nawrocki@samsung.com
> Cc: linux-phy@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-
> soc@vger.kernel.org; rosa.pila@samsung.com; dev.tailor@samsung.com;
> faraz.ata@samsung.com; muhammed.ali@samsung.com;
> selvarasu.g@samsung.com
> Subject: Re: [PATCH v4 2/6] phy: exynos5-usbdrd: support HS phy for
> ExynosAutov920
> 
> On 01/07/2025 14:07, Pritam Manohar Sutar wrote:
> > This SoC has a single USB 3.1 DRD combo phy that supports both
> > UTMI+ (HS) and PIPE3 (SS) and three USB2.0 DRD HS phy controllers
> > those only support the UTMI+ (HS) interface.
> >
> > Support only UTMI+ port for this SoC which is very similar to what the
> > existing Exynos850 supports.
> >
> > This SoC shares phy isol between USBs. Bypass PHY isol when first USB
> > is powered on and enable it when all of then are powered off. Add
> > required change in phy driver to support HS phy for this SoC.
> >
> > Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
> 
> Drop

Sure.

> 
> You again added significant changes and claimed they were reviewed.
> 
> > Signed-off-by: Pritam Manohar Sutar <pritam.sutar@samsung.com>
> > ---
> >  drivers/phy/samsung/phy-exynos5-usbdrd.c    | 131 ++++++++++++++++++++
> >  include/linux/soc/samsung/exynos-regs-pmu.h |   2 +
> >  2 files changed, 133 insertions(+)
> >
> > diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> > b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> > index dd660ebe8045..64f3316f6ad4 100644
> > --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> > +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> > @@ -480,6 +480,8 @@ struct exynos5_usbdrd_phy {
> >  	enum typec_orientation orientation;
> >  };
> >
> > +static atomic_t usage_count = ATOMIC_INIT(0);

Presently, existed SoC in driver, supports only one USB port and 
it does not have any complications to handle shared isol. However, 
phy isols are shared across USB20s in ExynosAutov920 
(here ExynosAutov920 has 4 ports with shared phy isols). phy isol 
should be enabled when all ports are disabled else bypassed if any 
one of usbs is in use. phy isol is handled at bootloader in Downstream code. 
USB20 ports won't work if isol is not handled in any bootloader. Hence, 
added usage_count for the purpose.
   
But still, it depends on phy isol architecture, if SoC shares phy isol with 
USBs or it can have separate phy isols for each usbs.

> 
> No, you cannot have singletons. How are you going to handle two independent
> phys?

Agreed with your point and have to handle shared and separate isols in same driver.  
We will take this later in subsequent patch-sets.
   
For now, will remove this usage_count and bring up only basic support for ExynosAutov920 phy.

> 
> Best regards,
> Krzysztof

Thank you.

Regards,
Pritam

Re: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 HS phy compatible

[Krzysztof Kozlowski]

On 17/07/2025 13:13, Pritam Manohar Sutar wrote:
>>
>>
>> Nothing is explained in changelog/cover letter. You claim you only added Rb tag.
>> This is an entirely silent change while keeping the review.
> 
> Will add more explanations in cover letter/changelog why this block is added.
> 
>> Combined with not even following DTS style!
> 
> Ok got it. Will change supplies name as below 
> avdd075_usb => avdd075-usb
> avdd18_usb20 => avdd18-usb20
> avdd33_usb20 => avdd33-usb20
>    
> Confirm the above change that is meant in terms of DTS style.
Yes. I have doubts that actual supplies have suffix usb20. Are there
more than one avdd18 for this block?

Best regards,
Krzysztof

RE: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 HS phy compatible

[Pritam Manohar Sutar]

Hi Krzysztof, 

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 17 July 2025 04:59 PM
> To: Pritam Manohar Sutar <pritam.sutar@samsung.com>; 'Krzysztof Kozlowski'
> <krzysztof.kozlowski@linaro.org>
> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; alim.akhtar@samsung.com;
> andre.draszik@linaro.org; peter.griffin@linaro.org; neil.armstrong@linaro.org;
> kauschluss@disroot.org; ivo.ivanov.ivanov1@gmail.com;
> m.szyprowski@samsung.com; s.nawrocki@samsung.com; linux-
> phy@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-
> soc@vger.kernel.org; rosa.pila@samsung.com; dev.tailor@samsung.com;
> faraz.ata@samsung.com; muhammed.ali@samsung.com;
> selvarasu.g@samsung.com
> Subject: Re: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add
> ExynosAutov920 HS phy compatible
> 
> On 17/07/2025 13:13, Pritam Manohar Sutar wrote:
> >>
> >>
> >> Nothing is explained in changelog/cover letter. You claim you only added Rb
> tag.
> >> This is an entirely silent change while keeping the review.
> >
> > Will add more explanations in cover letter/changelog why this block is added.
> >
> >> Combined with not even following DTS style!
> >
> > Ok got it. Will change supplies name as below avdd075_usb =>
> > avdd075-usb
> > avdd18_usb20 => avdd18-usb20
> > avdd33_usb20 => avdd33-usb20
> >
> > Confirm the above change that is meant in terms of DTS style.
> Yes. I have doubts that actual supplies have suffix usb20. Are there more than
> one avdd18 for this block?
> 

Yes, there are more than one vdd18 supplies for this block. 

Re-analysed your comment on adding new supplies. 
Going to re-use existing supplies as mentioned below, rather than 
introducing new supplies

  dvdd-usb20-supply   => for 0.75v
  vddh-usb20-supply   => for 1.8v
  vdd33-usb20-supply => for 3.3v

> Best regards,
> Krzysztof


Thank you.

Regards,
Pritam

Re: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 HS phy compatible

[Krzysztof Kozlowski]

On 22/07/2025 06:34, Pritam Manohar Sutar wrote:
>>>> Nothing is explained in changelog/cover letter. You claim you only added Rb
>> tag.
>>>> This is an entirely silent change while keeping the review.
>>>
>>> Will add more explanations in cover letter/changelog why this block is added.
>>>
>>>> Combined with not even following DTS style!
>>>
>>> Ok got it. Will change supplies name as below avdd075_usb =>
>>> avdd075-usb
>>> avdd18_usb20 => avdd18-usb20
>>> avdd33_usb20 => avdd33-usb20
>>>
>>> Confirm the above change that is meant in terms of DTS style.
>> Yes. I have doubts that actual supplies have suffix usb20. Are there more than
>> one avdd18 for this block?
>>
> 
> Yes, there are more than one vdd18 supplies for this block. 

And their names are?

> 
> Re-analysed your comment on adding new supplies. 
> Going to re-use existing supplies as mentioned below, rather than 
> introducing new supplies
> 
>   dvdd-usb20-supply   => for 0.75v
>   vddh-usb20-supply   => for 1.8v
>   vdd33-usb20-supply => for 3.3v


You just expect us to guess whether this is correct...

Best regards,
Krzysztof

RE: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 HS phy compatible

[Pritam Manohar Sutar]

Hi Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 22 July 2025 11:37 AM
> To: Pritam Manohar Sutar <pritam.sutar@samsung.com>; 'Krzysztof Kozlowski'
> <krzysztof.kozlowski@linaro.org>
> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; alim.akhtar@samsung.com;
> andre.draszik@linaro.org; peter.griffin@linaro.org; neil.armstrong@linaro.org;
> kauschluss@disroot.org; ivo.ivanov.ivanov1@gmail.com;
> m.szyprowski@samsung.com; s.nawrocki@samsung.com; linux-
> phy@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-
> soc@vger.kernel.org; rosa.pila@samsung.com; dev.tailor@samsung.com;
> faraz.ata@samsung.com; muhammed.ali@samsung.com;
> selvarasu.g@samsung.com
> Subject: Re: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add
> ExynosAutov920 HS phy compatible
> 
> On 22/07/2025 06:34, Pritam Manohar Sutar wrote:
> >>>> Nothing is explained in changelog/cover letter. You claim you only
> >>>> added Rb
> >> tag.
> >>>> This is an entirely silent change while keeping the review.
> >>>
> >>> Will add more explanations in cover letter/changelog why this block is
> added.
> >>>
> >>>> Combined with not even following DTS style!
> >>>
> >>> Ok got it. Will change supplies name as below avdd075_usb =>
> >>> avdd075-usb
> >>> avdd18_usb20 => avdd18-usb20
> >>> avdd33_usb20 => avdd33-usb20
> >>>
> >>> Confirm the above change that is meant in terms of DTS style.
> >> Yes. I have doubts that actual supplies have suffix usb20. Are there
> >> more than one avdd18 for this block?
> >>
> >
> > Yes, there are more than one vdd18 supplies for this block.
> 
> And their names are?
> 
> >
> > Re-analysed your comment on adding new supplies.
> > Going to re-use existing supplies as mentioned below, rather than
> > introducing new supplies
> >
> >   dvdd-usb20-supply   => for 0.75v
> >   vddh-usb20-supply   => for 1.8v
> >   vdd33-usb20-supply => for 3.3v
> 
> 
> You just expect us to guess whether this is correct...

Sorry about not being clear so far. 

V920 needs three supplies, 0.75v, 1.8v and 3.3v for USB PHY
The naming convention used in the schematic are
avdd075-usb, 
avdd18_usb20, 
avdd33_usb20.

However, PHY's user manual just mentions DVDD, VDD33 and VDD18.
Since GS101 binding already using supply names similar to what is mentioned in the PHY user manual.
I thought of using the same instead of earlier naming conventions (which was as per v920 schematic).

Let me know if this make sense or we should be just using as per schematic?

> 
> Best regards,
> Krzysztof

Regards,
Pritam

Re: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 HS phy compatible

[Krzysztof Kozlowski]

On 23/07/2025 07:11, Pritam Manohar Sutar wrote:
>> On 22/07/2025 06:34, Pritam Manohar Sutar wrote:
>>>>>> Nothing is explained in changelog/cover letter. You claim you only
>>>>>> added Rb
>>>> tag.
>>>>>> This is an entirely silent change while keeping the review.
>>>>>
>>>>> Will add more explanations in cover letter/changelog why this block is
>> added.
>>>>>
>>>>>> Combined with not even following DTS style!
>>>>>
>>>>> Ok got it. Will change supplies name as below avdd075_usb =>
>>>>> avdd075-usb
>>>>> avdd18_usb20 => avdd18-usb20
>>>>> avdd33_usb20 => avdd33-usb20
>>>>>
>>>>> Confirm the above change that is meant in terms of DTS style.
>>>> Yes. I have doubts that actual supplies have suffix usb20. Are there
>>>> more than one avdd18 for this block?
>>>>
>>>
>>> Yes, there are more than one vdd18 supplies for this block.
>>
>> And their names are?
>>
>>>
>>> Re-analysed your comment on adding new supplies.
>>> Going to re-use existing supplies as mentioned below, rather than
>>> introducing new supplies
>>>
>>>   dvdd-usb20-supply   => for 0.75v
>>>   vddh-usb20-supply   => for 1.8v
>>>   vdd33-usb20-supply => for 3.3v
>>
>>
>> You just expect us to guess whether this is correct...
> 
> Sorry about not being clear so far. 
> 
> V920 needs three supplies, 0.75v, 1.8v and 3.3v for USB PHY
> The naming convention used in the schematic are
> avdd075-usb, 
> avdd18_usb20, 
> avdd33_usb20.
> 
> However, PHY's user manual just mentions DVDD, VDD33 and VDD18.


Then dvdd, vdd33 and vdd18.

> Since GS101 binding already using supply names similar to what is mentioned in the PHY user manual.


GS101 has USB 2.0 and DP, thus the suffix made some sense. I think you
have only USB 2.0, that's why I question the suffix.


Best regards,
Krzysztof

RE: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 HS phy compatible

[Alim Akhtar]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Wednesday, July 23, 2025 2:13 PM
> To: Pritam Manohar Sutar <pritam.sutar@samsung.com>; 'Krzysztof
> Kozlowski' <krzysztof.kozlowski@linaro.org>
> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
[snip]
> >>>>> Ok got it. Will change supplies name as below avdd075_usb =>
> >>>>> avdd075-usb
> >>>>> avdd18_usb20 => avdd18-usb20
> >>>>> avdd33_usb20 => avdd33-usb20
> >>>>>
> >>>>> Confirm the above change that is meant in terms of DTS style.
> >>>> Yes. I have doubts that actual supplies have suffix usb20. Are
> >>>> there more than one avdd18 for this block?
> >>>>
> >>>
> >>> Yes, there are more than one vdd18 supplies for this block.
> >>
> >> And their names are?
> >>
> >>>
> >>> Re-analysed your comment on adding new supplies.
> >>> Going to re-use existing supplies as mentioned below, rather than
> >>> introducing new supplies
> >>>
> >>>   dvdd-usb20-supply   => for 0.75v
> >>>   vddh-usb20-supply   => for 1.8v
> >>>   vdd33-usb20-supply => for 3.3v
> >>
> >>
> >> You just expect us to guess whether this is correct...
> >
> > Sorry about not being clear so far.
> >
> > V920 needs three supplies, 0.75v, 1.8v and 3.3v for USB PHY The naming
> > convention used in the schematic are avdd075-usb, avdd18_usb20,
> > avdd33_usb20.
> >
> > However, PHY's user manual just mentions DVDD, VDD33 and VDD18.
> 
> 
> Then dvdd, vdd33 and vdd18.
> 
> > Since GS101 binding already using supply names similar to what is
> mentioned in the PHY user manual.
> 
> 
> GS101 has USB 2.0 and DP, thus the suffix made some sense. I think you have
> only USB 2.0, that's why I question the suffix.
> 
I cross checked the schematic of v920 SADK, this is a combo PHY which support USB-3.0 as well. 
@ Pritam
Schema should capture all the supplies including USB-3.0, similar to GS101 (which has USB2.0 and DP combo).
So that would be as below:
dvdd075-usb20-supply
vdd18-usb20-supply
vdd33-usb20-supply
dvdd075-usb30-supply
vdd18-usb30-supply
please cross check the supply at your end and do the needful. 

> 
> Best regards,
> Krzysztof

RE: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 HS phy compatible

[Pritam Manohar Sutar]

Hi, 

> -----Original Message-----
> From: Alim Akhtar <alim.akhtar@samsung.com>
> Sent: 23 July 2025 06:15 PM
> To: 'Krzysztof Kozlowski' <krzk@kernel.org>; 'Pritam Manohar Sutar'
> <pritam.sutar@samsung.com>; 'Krzysztof Kozlowski'
> <krzysztof.kozlowski@linaro.org>
> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; andre.draszik@linaro.org;
> peter.griffin@linaro.org; neil.armstrong@linaro.org; kauschluss@disroot.org;
> ivo.ivanov.ivanov1@gmail.com; m.szyprowski@samsung.com;
> s.nawrocki@samsung.com; linux-phy@lists.infradead.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org;
> rosa.pila@samsung.com; dev.tailor@samsung.com;
> faraz.ata@samsung.com; muhammed.ali@samsung.com;
> selvarasu.g@samsung.com
> Subject: RE: [PATCH v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add
> ExynosAutov920 HS phy compatible
> 
> 
> 
> > -----Original Message-----
> > From: Krzysztof Kozlowski <krzk@kernel.org>
> > Sent: Wednesday, July 23, 2025 2:13 PM
> > To: Pritam Manohar Sutar <pritam.sutar@samsung.com>; 'Krzysztof
> > Kozlowski' <krzysztof.kozlowski@linaro.org>
> > Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> [snip]
> > >>>>> Ok got it. Will change supplies name as below avdd075_usb =>
> > >>>>> avdd075-usb
> > >>>>> avdd18_usb20 => avdd18-usb20
> > >>>>> avdd33_usb20 => avdd33-usb20
> > >>>>>
> > >>>>> Confirm the above change that is meant in terms of DTS style.
> > >>>> Yes. I have doubts that actual supplies have suffix usb20. Are
> > >>>> there more than one avdd18 for this block?
> > >>>>
> > >>>
> > >>> Yes, there are more than one vdd18 supplies for this block.
> > >>
> > >> And their names are?
> > >>
> > >>>
> > >>> Re-analysed your comment on adding new supplies.
> > >>> Going to re-use existing supplies as mentioned below, rather than
> > >>> introducing new supplies
> > >>>
> > >>>   dvdd-usb20-supply   => for 0.75v
> > >>>   vddh-usb20-supply   => for 1.8v
> > >>>   vdd33-usb20-supply => for 3.3v
> > >>
> > >>
> > >> You just expect us to guess whether this is correct...
> > >
> > > Sorry about not being clear so far.
> > >
> > > V920 needs three supplies, 0.75v, 1.8v and 3.3v for USB PHY The
> > > naming convention used in the schematic are avdd075-usb,
> > > avdd18_usb20, avdd33_usb20.
> > >
> > > However, PHY's user manual just mentions DVDD, VDD33 and VDD18.
> >
> >
> > Then dvdd, vdd33 and vdd18.
> >
> > > Since GS101 binding already using supply names similar to what is
> > mentioned in the PHY user manual.
> >
> >
> > GS101 has USB 2.0 and DP, thus the suffix made some sense. I think you
> > have only USB 2.0, that's why I question the suffix.
> >
> I cross checked the schematic of v920 SADK, this is a combo PHY which
> support USB-3.0 as well.
> @ Pritam
> Schema should capture all the supplies including USB-3.0, similar to GS101
> (which has USB2.0 and DP combo).
> So that would be as below:
> dvdd075-usb20-supply
> vdd18-usb20-supply
> vdd33-usb20-supply
> dvdd075-usb30-supply
> vdd18-usb30-supply
> please cross check the supply at your end and do the needful.
> 

Yes, we have a combo PHY that supports USB20 and USB30 and 
need these supplies. Will update the binding accordingly.

> >
> > Best regards,
> > Krzysztof

Thank you.

Regards,
Pritam