linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] ESWIN EIC7700 sata phy driver and yaml,
@ 2025-08-19 13:47 Yulin Lu
  2025-08-19 13:54 ` [PATCH v2 1/3] dt-bindings: ata: eswin: Document for EIC7700 SoC ahci Yulin Lu
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Yulin Lu @ 2025-08-19 13:47 UTC (permalink / raw)
  To: dlemoal, cassel, robh, krzk+dt, conor+dt, linux-ide, devicetree,
	linux-kernel, vkoul, kishon, linux-phy
  Cc: ningyu, zhengyu, linmin, huangyifeng, fenglin, lianghujun,
	luyulin

From: luyulin <luyulin@eswincomputing.com>

  Implements support for the Eswin EIC7700 SoC sata phy.
  Implements the calling sequence to interface with dwc-ahci,
  ensuring correct hardware execution order.
  Integration with the Linux phy subsystem for consistency and
  scalability.
  Add documentation for ahci and sata phy on the ESWIN EIC7700
  SoC platform.

  Supported chips:
    Eswin EIC7700 SoC.

  Test:
    Tested this patch on the Sifive HiFive Premier P550 (which uses
    the EIC7700 SoC). Based on this driver, the SATA device read/write
    operations are functioning normally, supporting SATA 1.5 Gb/s,
    3.0 Gb/s, and 6.0 Gb/s speeds, so this verifies that this sata
    driver patch is working properly.

  This series depends on the vendor prefix patch [1] and config option patch [2].
  [1] https://lore.kernel.org/all/20250616112316.3833343-4-pinkesh.vaghela@einfochips.com/
  [2] https://lore.kernel.org/all/20250616112316.3833343-3-pinkesh.vaghela@einfochips.com/

Updates:

  Changes since V1:
    - Delete the original controller driver and use ahci_dwc.c instead.
    - Add eswin,eic7700-ahci.yaml
      - Correct the descriptions of reset, interrupt and other
        hardware resources for the sata controller on EIC7700 SoC.
      - The clocks for both sata controller and sata PHY are controlled
        via a register bit in the HSP bus and are not registered in the
        clock tree. Clock are managed within the PHY driver, therefore
        it is not described in this document.
      - Add $ref: snps,dwc-ahci-common.yaml#.
    - Add eswin,eic7700-sata-phy.yaml
      - Add this file to include the description of the PHY on EIC7700 SoC.
    - Add an eswin directory under the PHY driver path, and include the SATA
      PHY driver code for EIC7700 SoC.
    - Link to v1: https://lore.kernel.org/all/20250515085114.1692-1-hehuan1@eswincomputing.com/

luyulin (3):
  dt-bindings: ata: eswin: Document for EIC7700 SoC ahci
  dt-bindings: phy: eswin: Document for EIC7700 SoC SATA
  phy: eswin: Create eswin directory and add EIC7700 SATA PHY driver

 .../bindings/ata/eswin,eic7700-ahci.yaml      |  92 ++++++++
 .../bindings/phy/eswin,eic7700-sata-phy.yaml  |  36 ++++
 drivers/phy/Kconfig                           |   1 +
 drivers/phy/Makefile                          |   1 +
 drivers/phy/eswin/Kconfig                     |  14 ++
 drivers/phy/eswin/Makefile                    |   2 +
 drivers/phy/eswin/phy-eic7700-sata.c          | 197 ++++++++++++++++++
 7 files changed, 343 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml
 create mode 100644 Documentation/devicetree/bindings/phy/eswin,eic7700-sata-phy.yaml
 create mode 100644 drivers/phy/eswin/Kconfig
 create mode 100644 drivers/phy/eswin/Makefile
 create mode 100644 drivers/phy/eswin/phy-eic7700-sata.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/3] dt-bindings: ata: eswin: Document for EIC7700 SoC ahci
  2025-08-19 13:47 [PATCH v2 0/3] ESWIN EIC7700 sata phy driver and yaml, Yulin Lu
@ 2025-08-19 13:54 ` Yulin Lu
  2025-08-19 14:25   ` Rob Herring
  2025-08-19 14:00 ` [PATCH v2 3/3] phy: eswin: Create eswin directory and add EIC7700 SATA PHY driver Yulin Lu
  2025-08-20  9:27 ` [PATCH v2 2/3] dt-bindings: phy: eswin: Document for EIC7700 SoC SATA PHY Yulin Lu
  2 siblings, 1 reply; 13+ messages in thread
From: Yulin Lu @ 2025-08-19 13:54 UTC (permalink / raw)
  To: dlemoal, cassel, robh, krzk+dt, conor+dt, linux-ide, devicetree,
	linux-kernel, vkoul, kishon, linux-phy
  Cc: ningyu, zhengyu, linmin, huangyifeng, fenglin, lianghujun,
	luyulin

From: luyulin <luyulin@eswincomputing.com>

Add document for the SATA AHCI controller on the EIC7700 SoC platform,
including descriptions of its hardware configurations.

Signed-off-by: luyulin <luyulin@eswincomputing.com>
---
 .../bindings/ata/eswin,eic7700-ahci.yaml      | 92 +++++++++++++++++++
 1 file changed, 92 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml

diff --git a/Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml b/Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml
new file mode 100644
index 000000000000..9ef58c9c2f28
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml
@@ -0,0 +1,92 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/ata/eswin,eic7700-ahci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Eswin EIC7700 SoC SATA Controller
+
+maintainers:
+  - Yulin Lu <luyulin@eswincomputing.com>
+  - Huan He <hehuan1@eswincomputing.com>
+
+description:
+  This document defines device tree bindings for the Synopsys DWC
+  implementation of the AHCI SATA controller found in Eswin's
+  Eic7700 SoC platform.
+
+select:
+  properties:
+    compatible:
+      const: eswin,eic7700-ahci
+  required:
+    - compatible
+
+allOf:
+  - $ref: snps,dwc-ahci-common.yaml#
+
+properties:
+  compatible:
+    items:
+      - const: eswin,eic7700-ahci
+      - const: snps,dwc-ahci
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  ports-implemented:
+    const: 1
+
+  clocks:
+    minItems: 2
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: pclk
+      - const: aclk
+
+  resets:
+    maxItems: 1
+
+  reset-names:
+    const: arst
+
+  phys:
+    maxItems: 1
+
+  phy-names:
+    const: sata-phy
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - phys
+  - phy-names
+  - ports-implemented
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    sata@50420000 {
+        compatible = "eswin,eic7700-ahci", "snps,dwc-ahci";
+        reg = <0x50420000 0x10000>;
+        interrupt-parent = <&plic>;
+        interrupts = <58>;
+        ports-implemented = <0x1>;
+        clocks = <&gate_clk_hsp_cfgclk>, <&gate_clk_hsp_aclk>;
+        clock-names = "pclk", "aclk";
+        resets = <&reset 96>;
+        reset-names = "arst";
+        phys = <&sata_phy>;
+        phy-names = "sata-phy";
+    };
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 3/3] phy: eswin: Create eswin directory and add EIC7700 SATA PHY driver
  2025-08-19 13:47 [PATCH v2 0/3] ESWIN EIC7700 sata phy driver and yaml, Yulin Lu
  2025-08-19 13:54 ` [PATCH v2 1/3] dt-bindings: ata: eswin: Document for EIC7700 SoC ahci Yulin Lu
@ 2025-08-19 14:00 ` Yulin Lu
  2025-08-20 16:07   ` Vinod Koul
  2025-08-21  8:41   ` kernel test robot
  2025-08-20  9:27 ` [PATCH v2 2/3] dt-bindings: phy: eswin: Document for EIC7700 SoC SATA PHY Yulin Lu
  2 siblings, 2 replies; 13+ messages in thread
From: Yulin Lu @ 2025-08-19 14:00 UTC (permalink / raw)
  To: dlemoal, cassel, robh, krzk+dt, conor+dt, linux-ide, devicetree,
	linux-kernel, vkoul, kishon, linux-phy
  Cc: ningyu, zhengyu, linmin, huangyifeng, fenglin, lianghujun,
	luyulin

From: luyulin <luyulin@eswincomputing.com>

Created the eswin phy driver directory and added support for
the SATA phy driver on the EIC7700 SoC platform.

Signed-off-by: luyulin <luyulin@eswincomputing.com>
---
 drivers/phy/Kconfig                  |   1 +
 drivers/phy/Makefile                 |   1 +
 drivers/phy/eswin/Kconfig            |  14 ++
 drivers/phy/eswin/Makefile           |   2 +
 drivers/phy/eswin/phy-eic7700-sata.c | 197 +++++++++++++++++++++++++++
 5 files changed, 215 insertions(+)
 create mode 100644 drivers/phy/eswin/Kconfig
 create mode 100644 drivers/phy/eswin/Makefile
 create mode 100644 drivers/phy/eswin/phy-eic7700-sata.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 58c911e1b2d2..e82ebcfe534a 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -105,6 +105,7 @@ source "drivers/phy/allwinner/Kconfig"
 source "drivers/phy/amlogic/Kconfig"
 source "drivers/phy/broadcom/Kconfig"
 source "drivers/phy/cadence/Kconfig"
+source "drivers/phy/eswin/Kconfig"
 source "drivers/phy/freescale/Kconfig"
 source "drivers/phy/hisilicon/Kconfig"
 source "drivers/phy/ingenic/Kconfig"
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index c670a8dac468..ed7444949259 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -17,6 +17,7 @@ obj-y					+= allwinner/	\
 					   amlogic/	\
 					   broadcom/	\
 					   cadence/	\
+					   eswin/	\
 					   freescale/	\
 					   hisilicon/	\
 					   ingenic/	\
diff --git a/drivers/phy/eswin/Kconfig b/drivers/phy/eswin/Kconfig
new file mode 100644
index 000000000000..3fcd76582c3b
--- /dev/null
+++ b/drivers/phy/eswin/Kconfig
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Phy drivers for Eswin platforms
+#
+config PHY_EIC7700_SATA
+	tristate "eic7700 Sata SerDes/PHY driver"
+	depends on ARCH_ESWIN || COMPILE_TEST
+	depends on HAS_IOMEM
+	select GENERIC_PHY
+	help
+	  Enable this to support SerDes/Phy found on ESWIN's
+	  EIC7700 SoC.This Phy supports SATA 1.5 Gb/s,
+	  SATA 3.0 Gb/s, SATA 6.0 Gb/s speeds.
+	  It supports one SATA host port to accept one SATA device.
diff --git a/drivers/phy/eswin/Makefile b/drivers/phy/eswin/Makefile
new file mode 100644
index 000000000000..db08c66be812
--- /dev/null
+++ b/drivers/phy/eswin/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_PHY_EIC7700_SATA)	+= phy-eic7700-sata.o
diff --git a/drivers/phy/eswin/phy-eic7700-sata.c b/drivers/phy/eswin/phy-eic7700-sata.c
new file mode 100644
index 000000000000..8a757839e868
--- /dev/null
+++ b/drivers/phy/eswin/phy-eic7700-sata.c
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ESWIN SATA PHY driver
+ *
+ * Copyright 2024, Beijing ESWIN Computing Technology Co., Ltd..
+ * All rights reserved.
+ *
+ * Authors: Yulin Lu <luyulin@eswincomputing.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+#define SATA_CLK_CTRL			0x0
+#define SATA_AXI_LP_CTRL		0x08
+#define SATA_MPLL_CTRL			0x20
+#define SATA_PHY_CTRL0			0x28
+#define SATA_PHY_CTRL1			0x2c
+#define SATA_REF_CTRL1			0x38
+#define SATA_REG_CTRL			0x34
+#define SATA_LOS_IDEN			0x3c
+#define SATA_RESET_CTRL			0x40
+#define SATA_CLK_RST_SOURCE_PHY		BIT(0)
+#define SATA_SYS_CLK_EN			BIT(28)
+#define SATA_PHY_RESET			BIT(0)
+#define SATA_PORT_RESET			BIT(1)
+#define SATA_LOS_LEVEL			0x9
+#define SATA_LOS_BIAS			(0x02 << 16)
+#define SATA_REF_REPEATCLK_EN		BIT(0)
+#define SATA_REF_USE_PAD		BIT(20)
+#define SATA_P0_AMPLITUDE_GEN1		0x42
+#define SATA_P0_AMPLITUDE_GEN2		(0x46 << 8)
+#define SATA_P0_AMPLITUDE_GEN3		(0x73 << 16)
+#define SATA_P0_PHY_TX_PREEMPH_GEN1	0x05
+#define SATA_P0_PHY_TX_PREEMPH_GEN2	(0x05 << 8)
+#define SATA_P0_PHY_TX_PREEMPH_GEN3	(0x08 << 16)
+#define SATA_MPLL_MULTIPLIER		(0x3c << 16)
+#define SATA_M_CSYSREQ			BIT(0)
+#define SATA_S_CSYSREQ			BIT(16)
+#define SATA_P0_PHY_STAT		0x24
+#define SATA_P0_PHY_READY		BIT(0)
+
+#define PHY_READY_TIMEOUT		(usecs_to_jiffies(4000))
+
+struct eic7700_sata_phy {
+	struct phy *phy;
+	void __iomem *regs;
+};
+
+static int wait_for_phy_ready(void __iomem *base, u32 reg, u32 checkbit,
+			      u32 status)
+{
+	unsigned long start = jiffies;
+	unsigned long timeout = start + PHY_READY_TIMEOUT;
+
+	while (time_before(start, timeout)) {
+		if ((readl(base + reg) & checkbit) == status)
+			return 0;
+		usleep_range(50, 70);
+	}
+
+	return -EFAULT;
+}
+
+static int eic7700_sata_phy_init(struct phy *phy)
+{
+	struct eic7700_sata_phy *sata_phy = phy_get_drvdata(phy);
+	u32 val = 0;
+	int ret = 0;
+
+	/*
+	 * The SATA_CLK_CTRL register offset controls the pmalive, rxoob,
+	 * and rbc clocks gate provided by the PHY through the HSP bus,
+	 * and it is not registered in the clock tree.
+	 */
+	val = readl(sata_phy->regs + SATA_CLK_CTRL);
+	val |= SATA_SYS_CLK_EN;
+	writel(val, sata_phy->regs + SATA_CLK_CTRL);
+
+	writel(SATA_CLK_RST_SOURCE_PHY, sata_phy->regs + SATA_REF_CTRL1);
+	writel(SATA_P0_AMPLITUDE_GEN1 | SATA_P0_AMPLITUDE_GEN2 |
+	       SATA_P0_AMPLITUDE_GEN3, sata_phy->regs + SATA_PHY_CTRL0);
+	writel(SATA_P0_PHY_TX_PREEMPH_GEN1 | SATA_P0_PHY_TX_PREEMPH_GEN2 |
+	       SATA_P0_PHY_TX_PREEMPH_GEN3, sata_phy->regs + SATA_PHY_CTRL1);
+	writel(SATA_LOS_LEVEL | SATA_LOS_BIAS,
+	       sata_phy->regs + SATA_LOS_IDEN);
+	writel(SATA_M_CSYSREQ | SATA_S_CSYSREQ,
+	       sata_phy->regs + SATA_AXI_LP_CTRL);
+	writel(SATA_REF_REPEATCLK_EN | SATA_REF_USE_PAD,
+	       sata_phy->regs + SATA_REG_CTRL);
+	writel(SATA_MPLL_MULTIPLIER, sata_phy->regs + SATA_MPLL_CTRL);
+	usleep_range(15, 20);
+
+	/*
+	 * The SATA_RESET_CTRL register offset controls reset/deassert
+	 * for both the port and the PHY through the HSP bus,
+	 * and it is not registered in the reset tree.
+	 */
+	val = readl(sata_phy->regs + SATA_RESET_CTRL);
+	val &= ~(SATA_PHY_RESET | SATA_PORT_RESET);
+	writel(val, sata_phy->regs + SATA_RESET_CTRL);
+
+	ret = wait_for_phy_ready(sata_phy->regs, SATA_P0_PHY_STAT,
+				 SATA_P0_PHY_READY, 1);
+	if (ret < 0)
+		dev_err(&sata_phy->phy->dev,
+			"PHY READY check failed\n");
+	return ret;
+}
+
+static int eic7700_sata_phy_exit(struct phy *phy)
+{
+	struct eic7700_sata_phy *sata_phy = phy_get_drvdata(phy);
+	u32 val = 0;
+
+	val = readl(sata_phy->regs + SATA_RESET_CTRL);
+	val |= SATA_PHY_RESET | SATA_PORT_RESET;
+	writel(val, sata_phy->regs + SATA_RESET_CTRL);
+
+	val = readl(sata_phy->regs + SATA_CLK_CTRL);
+	val &= ~SATA_SYS_CLK_EN;
+	writel(val, sata_phy->regs + SATA_CLK_CTRL);
+
+	return 0;
+}
+
+static const struct phy_ops eic7700_sata_phy_ops = {
+	.init		= eic7700_sata_phy_init,
+	.exit		= eic7700_sata_phy_exit,
+	.owner		= THIS_MODULE,
+};
+
+static int eic7700_sata_phy_probe(struct platform_device *pdev)
+{
+	struct eic7700_sata_phy *sata_phy;
+	struct device *dev = &pdev->dev;
+	struct phy_provider *phy_provider;
+	u32 val = 0;
+	int ret = 0;
+
+	sata_phy = devm_kzalloc(dev, sizeof(*sata_phy), GFP_KERNEL);
+	if (!sata_phy)
+		return -ENOMEM;
+
+	sata_phy->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(sata_phy->regs))
+		return PTR_ERR(sata_phy->regs);
+
+	dev_set_drvdata(dev, sata_phy);
+
+	sata_phy->phy = devm_phy_create(dev, NULL, &eic7700_sata_phy_ops);
+	if (IS_ERR(sata_phy->phy)) {
+		dev_err(dev, "failed to create PHY\n");
+		ret = PTR_ERR(sata_phy->phy);
+		goto clk_disable;
+	}
+
+	phy_set_drvdata(sata_phy->phy, sata_phy);
+
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(phy_provider)) {
+		ret = PTR_ERR(phy_provider);
+		goto clk_disable;
+	}
+
+	return 0;
+
+clk_disable:
+	val = readl(sata_phy->regs + SATA_CLK_CTRL);
+	val &= ~SATA_SYS_CLK_EN;
+	writel(val, sata_phy->regs + SATA_CLK_CTRL);
+
+	return ret;
+}
+
+static const struct of_device_id eic7700_sata_phy_of_match[] = {
+	{ .compatible = "eswin,eic7700-sata-phy" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, eic7700_sata_phy_of_match);
+
+static struct platform_driver eic7700_sata_phy_driver = {
+	.probe	= eic7700_sata_phy_probe,
+	.driver = {
+		.of_match_table	= eic7700_sata_phy_of_match,
+		.name  = "eswin,sata-phy",
+		.suppress_bind_attrs = true,
+	}
+};
+module_platform_driver(eic7700_sata_phy_driver);
+
+MODULE_DESCRIPTION("SATA PHY driver for the ESWIN EIC7700 SoC");
+MODULE_AUTHOR("Yulin Lu <luyulin@eswincomputing.com>");
+MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: ata: eswin: Document for EIC7700 SoC ahci
  2025-08-19 13:54 ` [PATCH v2 1/3] dt-bindings: ata: eswin: Document for EIC7700 SoC ahci Yulin Lu
@ 2025-08-19 14:25   ` Rob Herring
  2025-08-25  8:19     ` luyulin
  2025-08-28 10:22     ` luyulin
  0 siblings, 2 replies; 13+ messages in thread
From: Rob Herring @ 2025-08-19 14:25 UTC (permalink / raw)
  To: Yulin Lu
  Cc: dlemoal, cassel, krzk+dt, conor+dt, linux-ide, devicetree,
	linux-kernel, vkoul, kishon, linux-phy, ningyu, zhengyu, linmin,
	huangyifeng, fenglin, lianghujun

On Tue, Aug 19, 2025 at 8:54 AM Yulin Lu <luyulin@eswincomputing.com> wrote:
>
> From: luyulin <luyulin@eswincomputing.com>

Please fix your name.

>
> Add document for the SATA AHCI controller on the EIC7700 SoC platform,
> including descriptions of its hardware configurations.
>
> Signed-off-by: luyulin <luyulin@eswincomputing.com>

And here.

> ---
>  .../bindings/ata/eswin,eic7700-ahci.yaml      | 92 +++++++++++++++++++
>  1 file changed, 92 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml
>
> diff --git a/Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml b/Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml
> new file mode 100644
> index 000000000000..9ef58c9c2f28
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml
> @@ -0,0 +1,92 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/ata/eswin,eic7700-ahci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Eswin EIC7700 SoC SATA Controller
> +
> +maintainers:
> +  - Yulin Lu <luyulin@eswincomputing.com>
> +  - Huan He <hehuan1@eswincomputing.com>
> +
> +description:
> +  This document defines device tree bindings for the Synopsys DWC
> +  implementation of the AHCI SATA controller found in Eswin's
> +  Eic7700 SoC platform.
> +
> +select:
> +  properties:
> +    compatible:
> +      const: eswin,eic7700-ahci
> +  required:
> +    - compatible
> +
> +allOf:
> +  - $ref: snps,dwc-ahci-common.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: eswin,eic7700-ahci
> +      - const: snps,dwc-ahci
> +
> +  reg:
> +    maxItems: 1

Drop. snps,dwc-ahci-common.yaml already defines this.

> +
> +  interrupts:
> +    maxItems: 1

Drop. snps,dwc-ahci-common.yaml already defines this.

> +
> +  ports-implemented:
> +    const: 1

Really, your firmware should initialize the DWC specific register that
sets this and is discoverable via a standard AHCI register.

> +
> +  clocks:
> +    minItems: 2
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: pclk
> +      - const: aclk
> +
> +  resets:
> +    maxItems: 1
> +
> +  reset-names:
> +    const: arst
> +
> +  phys:
> +    maxItems: 1

Drop. ahci-common.yaml already defines this.

> +
> +  phy-names:
> +    const: sata-phy

Drop. ahci-common.yaml already defines this.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +  - phys
> +  - phy-names
> +  - ports-implemented
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    sata@50420000 {
> +        compatible = "eswin,eic7700-ahci", "snps,dwc-ahci";
> +        reg = <0x50420000 0x10000>;
> +        interrupt-parent = <&plic>;
> +        interrupts = <58>;
> +        ports-implemented = <0x1>;
> +        clocks = <&gate_clk_hsp_cfgclk>, <&gate_clk_hsp_aclk>;
> +        clock-names = "pclk", "aclk";
> +        resets = <&reset 96>;
> +        reset-names = "arst";
> +        phys = <&sata_phy>;
> +        phy-names = "sata-phy";
> +    };
> --
> 2.25.1
>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 2/3] dt-bindings: phy: eswin: Document for EIC7700 SoC SATA PHY
  2025-08-19 13:47 [PATCH v2 0/3] ESWIN EIC7700 sata phy driver and yaml, Yulin Lu
  2025-08-19 13:54 ` [PATCH v2 1/3] dt-bindings: ata: eswin: Document for EIC7700 SoC ahci Yulin Lu
  2025-08-19 14:00 ` [PATCH v2 3/3] phy: eswin: Create eswin directory and add EIC7700 SATA PHY driver Yulin Lu
@ 2025-08-20  9:27 ` Yulin Lu
  2025-08-21  7:55   ` Krzysztof Kozlowski
  2 siblings, 1 reply; 13+ messages in thread
From: Yulin Lu @ 2025-08-20  9:27 UTC (permalink / raw)
  To: dlemoal, cassel, robh, krzk+dt, conor+dt, linux-ide, devicetree,
	linux-kernel, vkoul, kishon, linux-phy
  Cc: ningyu, zhengyu, linmin, huangyifeng, fenglin, lianghujun,
	Yulin Lu

Add document for the SATA phy on the EIC7700 SoC platform,
describing its usage.

Signed-off-by: Yulin Lu <luyulin@eswincomputing.com>
---
 .../bindings/phy/eswin,eic7700-sata-phy.yaml  | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/eswin,eic7700-sata-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/eswin,eic7700-sata-phy.yaml b/Documentation/devicetree/bindings/phy/eswin,eic7700-sata-phy.yaml
new file mode 100644
index 000000000000..d914cb4402d8
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/eswin,eic7700-sata-phy.yaml
@@ -0,0 +1,36 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/eswin,eic7700-sata-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Eswin EIC7700 SoC SATA PHY
+
+maintainers:
+  - Yulin Lu <luyulin@eswincomputing.com>
+  - Huan He <hehuan1@eswincomputing.com>
+
+properties:
+  compatible:
+    const: eswin,eic7700-sata-phy
+
+  "#phy-cells":
+    const: 0
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - "#phy-cells"
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    sata-phy@50440300 {
+        compatible = "eswin,eic7700-sata-phy";
+        reg = <0x50440300 0x40>;
+        #phy-cells = <0>;
+    };
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] phy: eswin: Create eswin directory and add EIC7700 SATA PHY driver
  2025-08-19 14:00 ` [PATCH v2 3/3] phy: eswin: Create eswin directory and add EIC7700 SATA PHY driver Yulin Lu
@ 2025-08-20 16:07   ` Vinod Koul
  2025-08-21  8:41   ` kernel test robot
  1 sibling, 0 replies; 13+ messages in thread
From: Vinod Koul @ 2025-08-20 16:07 UTC (permalink / raw)
  To: Yulin Lu
  Cc: dlemoal, cassel, robh, krzk+dt, conor+dt, linux-ide, devicetree,
	linux-kernel, kishon, linux-phy, ningyu, zhengyu, linmin,
	huangyifeng, fenglin, lianghujun

On 19-08-25, 22:00, Yulin Lu wrote:
> From: luyulin <luyulin@eswincomputing.com>
> 
> Created the eswin phy driver directory and added support for
> the SATA phy driver on the EIC7700 SoC platform.
> 
> Signed-off-by: luyulin <luyulin@eswincomputing.com>

Please use full name as you have used in the copyright notices


> +#define SATA_P0_PHY_TX_PREEMPH_GEN2	(0x05 << 8)
> +#define SATA_P0_PHY_TX_PREEMPH_GEN3	(0x08 << 16)
> +#define SATA_MPLL_MULTIPLIER		(0x3c << 16)

Use GENMASK for these

> +static int eic7700_sata_phy_init(struct phy *phy)
> +{
> +	struct eic7700_sata_phy *sata_phy = phy_get_drvdata(phy);
> +	u32 val = 0;
> +	int ret = 0;

both initializations are superfluous

> +static int eic7700_sata_phy_exit(struct phy *phy)
> +{
> +	struct eic7700_sata_phy *sata_phy = phy_get_drvdata(phy);
> +	u32 val = 0;

here and other places

> +static struct platform_driver eic7700_sata_phy_driver = {
> +	.probe	= eic7700_sata_phy_probe,
> +	.driver = {
> +		.of_match_table	= eic7700_sata_phy_of_match,
> +		.name  = "eswin,sata-phy",
> +		.suppress_bind_attrs = true,

why?

-- 
~Vinod

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/3] dt-bindings: phy: eswin: Document for EIC7700 SoC SATA PHY
  2025-08-20  9:27 ` [PATCH v2 2/3] dt-bindings: phy: eswin: Document for EIC7700 SoC SATA PHY Yulin Lu
@ 2025-08-21  7:55   ` Krzysztof Kozlowski
  2025-08-26  3:23     ` luyulin
  0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-21  7:55 UTC (permalink / raw)
  To: Yulin Lu
  Cc: dlemoal, cassel, robh, krzk+dt, conor+dt, linux-ide, devicetree,
	linux-kernel, vkoul, kishon, linux-phy, ningyu, zhengyu, linmin,
	huangyifeng, fenglin, lianghujun

On Wed, Aug 20, 2025 at 05:27:58PM +0800, Yulin Lu wrote:
> Add document for the SATA phy on the EIC7700 SoC platform,
> describing its usage.
> 
> Signed-off-by: Yulin Lu <luyulin@eswincomputing.com>

You already sent this patch separately (!!!) and received review.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] phy: eswin: Create eswin directory and add EIC7700 SATA PHY driver
  2025-08-19 14:00 ` [PATCH v2 3/3] phy: eswin: Create eswin directory and add EIC7700 SATA PHY driver Yulin Lu
  2025-08-20 16:07   ` Vinod Koul
@ 2025-08-21  8:41   ` kernel test robot
  1 sibling, 0 replies; 13+ messages in thread
From: kernel test robot @ 2025-08-21  8:41 UTC (permalink / raw)
  To: Yulin Lu, dlemoal, cassel, robh, krzk+dt, conor+dt, linux-ide,
	devicetree, linux-kernel, vkoul, kishon, linux-phy
  Cc: llvm, oe-kbuild-all, ningyu, zhengyu, linmin, huangyifeng,
	fenglin, lianghujun, luyulin

Hi Yulin,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v6.17-rc2 next-20250820]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yulin-Lu/dt-bindings-ata-eswin-Document-for-EIC7700-SoC-ahci/20250820-213411
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20250819140043.1862-1-luyulin%40eswincomputing.com
patch subject: [PATCH v2 3/3] phy: eswin: Create eswin directory and add EIC7700 SATA PHY driver
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20250821/202508211623.d8Spdqn7-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250821/202508211623.d8Spdqn7-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508211623.d8Spdqn7-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/phy/eswin/phy-eic7700-sata.c:60:8: error: call to undeclared function 'readl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
      60 |                 if ((readl(base + reg) & checkbit) == status)
         |                      ^
   drivers/phy/eswin/phy-eic7700-sata.c:79:8: error: call to undeclared function 'readl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
      79 |         val = readl(sata_phy->regs + SATA_CLK_CTRL);
         |               ^
>> drivers/phy/eswin/phy-eic7700-sata.c:81:2: error: call to undeclared function 'writel'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
      81 |         writel(val, sata_phy->regs + SATA_CLK_CTRL);
         |         ^
   drivers/phy/eswin/phy-eic7700-sata.c:119:8: error: call to undeclared function 'readl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     119 |         val = readl(sata_phy->regs + SATA_RESET_CTRL);
         |               ^
   drivers/phy/eswin/phy-eic7700-sata.c:121:2: error: call to undeclared function 'writel'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     121 |         writel(val, sata_phy->regs + SATA_RESET_CTRL);
         |         ^
   drivers/phy/eswin/phy-eic7700-sata.c:172:8: error: call to undeclared function 'readl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     172 |         val = readl(sata_phy->regs + SATA_CLK_CTRL);
         |               ^
   drivers/phy/eswin/phy-eic7700-sata.c:174:2: error: call to undeclared function 'writel'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     174 |         writel(val, sata_phy->regs + SATA_CLK_CTRL);
         |         ^
   7 errors generated.


vim +/readl +60 drivers/phy/eswin/phy-eic7700-sata.c

    52	
    53	static int wait_for_phy_ready(void __iomem *base, u32 reg, u32 checkbit,
    54				      u32 status)
    55	{
    56		unsigned long start = jiffies;
    57		unsigned long timeout = start + PHY_READY_TIMEOUT;
    58	
    59		while (time_before(start, timeout)) {
  > 60			if ((readl(base + reg) & checkbit) == status)
    61				return 0;
    62			usleep_range(50, 70);
    63		}
    64	
    65		return -EFAULT;
    66	}
    67	
    68	static int eic7700_sata_phy_init(struct phy *phy)
    69	{
    70		struct eic7700_sata_phy *sata_phy = phy_get_drvdata(phy);
    71		u32 val = 0;
    72		int ret = 0;
    73	
    74		/*
    75		 * The SATA_CLK_CTRL register offset controls the pmalive, rxoob,
    76		 * and rbc clocks gate provided by the PHY through the HSP bus,
    77		 * and it is not registered in the clock tree.
    78		 */
    79		val = readl(sata_phy->regs + SATA_CLK_CTRL);
    80		val |= SATA_SYS_CLK_EN;
  > 81		writel(val, sata_phy->regs + SATA_CLK_CTRL);
    82	
    83		writel(SATA_CLK_RST_SOURCE_PHY, sata_phy->regs + SATA_REF_CTRL1);
    84		writel(SATA_P0_AMPLITUDE_GEN1 | SATA_P0_AMPLITUDE_GEN2 |
    85		       SATA_P0_AMPLITUDE_GEN3, sata_phy->regs + SATA_PHY_CTRL0);
    86		writel(SATA_P0_PHY_TX_PREEMPH_GEN1 | SATA_P0_PHY_TX_PREEMPH_GEN2 |
    87		       SATA_P0_PHY_TX_PREEMPH_GEN3, sata_phy->regs + SATA_PHY_CTRL1);
    88		writel(SATA_LOS_LEVEL | SATA_LOS_BIAS,
    89		       sata_phy->regs + SATA_LOS_IDEN);
    90		writel(SATA_M_CSYSREQ | SATA_S_CSYSREQ,
    91		       sata_phy->regs + SATA_AXI_LP_CTRL);
    92		writel(SATA_REF_REPEATCLK_EN | SATA_REF_USE_PAD,
    93		       sata_phy->regs + SATA_REG_CTRL);
    94		writel(SATA_MPLL_MULTIPLIER, sata_phy->regs + SATA_MPLL_CTRL);
    95		usleep_range(15, 20);
    96	
    97		/*
    98		 * The SATA_RESET_CTRL register offset controls reset/deassert
    99		 * for both the port and the PHY through the HSP bus,
   100		 * and it is not registered in the reset tree.
   101		 */
   102		val = readl(sata_phy->regs + SATA_RESET_CTRL);
   103		val &= ~(SATA_PHY_RESET | SATA_PORT_RESET);
   104		writel(val, sata_phy->regs + SATA_RESET_CTRL);
   105	
   106		ret = wait_for_phy_ready(sata_phy->regs, SATA_P0_PHY_STAT,
   107					 SATA_P0_PHY_READY, 1);
   108		if (ret < 0)
   109			dev_err(&sata_phy->phy->dev,
   110				"PHY READY check failed\n");
   111		return ret;
   112	}
   113	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Re: [PATCH v2 1/3] dt-bindings: ata: eswin: Document for EIC7700 SoC ahci
  2025-08-19 14:25   ` Rob Herring
@ 2025-08-25  8:19     ` luyulin
  2025-08-28 10:22     ` luyulin
  1 sibling, 0 replies; 13+ messages in thread
From: luyulin @ 2025-08-25  8:19 UTC (permalink / raw)
  To: Rob Herring
  Cc: dlemoal, cassel, krzk+dt, conor+dt, linux-ide, devicetree,
	linux-kernel, vkoul, kishon, linux-phy, ningyu, zhengyu, linmin,
	huangyifeng, fenglin, lianghujun

Hi, Rob
Thank you very much for your professional response and suggestions.
Among the suggestions you mentioned, 
one point is not entirely clear, and I would like to seek your advice on it.
> 
> On Tue, Aug 19, 2025 at 8:54 AM Yulin Lu <luyulin@eswincomputing.com> wrote:
> >
> > From: luyulin <luyulin@eswincomputing.com>
> 
> Please fix your name.
> 
> >
> > Add document for the SATA AHCI controller on the EIC7700 SoC platform,
> > including descriptions of its hardware configurations.
> >
> > Signed-off-by: luyulin <luyulin@eswincomputing.com>
> 
> And here.
> 
> > ---
> >  .../bindings/ata/eswin,eic7700-ahci.yaml      | 92 +++++++++++++++++++
> >  1 file changed, 92 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml b/Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml
> > new file mode 100644
> > index 000000000000..9ef58c9c2f28
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml
> > @@ -0,0 +1,92 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/ata/eswin,eic7700-ahci.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Eswin EIC7700 SoC SATA Controller
> > +
> > +maintainers:
> > +  - Yulin Lu <luyulin@eswincomputing.com>
> > +  - Huan He <hehuan1@eswincomputing.com>
> > +
> > +description:
> > +  This document defines device tree bindings for the Synopsys DWC
> > +  implementation of the AHCI SATA controller found in Eswin's
> > +  Eic7700 SoC platform.
> > +
> > +select:
> > +  properties:
> > +    compatible:
> > +      const: eswin,eic7700-ahci
> > +  required:
> > +    - compatible
> > +
> > +allOf:
> > +  - $ref: snps,dwc-ahci-common.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: eswin,eic7700-ahci
> > +      - const: snps,dwc-ahci
> > +
> > +  reg:
> > +    maxItems: 1
> 
> Drop. snps,dwc-ahci-common.yaml already defines this.
> 
> > +
> > +  interrupts:
> > +    maxItems: 1
> 
> Drop. snps,dwc-ahci-common.yaml already defines this.
> 
> > +
> > +  ports-implemented:
> > +    const: 1
> 
> Really, your firmware should initialize the DWC specific register that
> sets this and is discoverable via a standard AHCI register.
> 
Yes, I initialized the relevant registers during the uboot stage,
and they worked correctly after entering the Linux system.
However, after unloading and then reloading the ko driver,
a reset operation causes these registers to be initialized to 0,
leading to initialization errors when reloading the ko driver.
If I want to implement it this way, I need to modify the ahci_dwc.c driver
and add program handling tailored to our SoC design.

I searched the kernel for "snps,dwc-ahci" and found that
in manufacturers' dts files, such as rk3588-base.dtsi, dra7-l4.dtsi,
exynos5250.dtsi, etc., all include the ports-implemented field in their sata nodes. 
only the sata node in amd-seattle-soc.dtsi lacks the ports-implemented field
and does not have reset resources.
Additionally, in the YAML files under the /Documentation/ata directory that
integrate the DWC AHCI controller, the ports-implemented field has been implemented,
as seen in examples such as rockchip,dwc-ahci.yaml and baikal,bt1-ahci.yaml.

Therefore, I have questions I would like to confirm with you:
1. In your previous feedback, were you suggesting that I remove the ports-implemented field?
And what is the reason for doing so?
Based on your professional advice and considering our SoC design, do you think
it is acceptable to retain the ports-implemented field in the dts instead of
removing it and modifying the ahci_dwc.c driver?
2. I noticed that in the arch/ directory, chips like dra7-l4.dtsi and
exynos5250.dtsi integrate the DWC AHCI controller and include corresponding
hardware resource designs such as clocks and resets.
Why do they not have independent yaml files implemented in the kernel?

Best regards,
Yulin Lu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Re: [PATCH v2 2/3] dt-bindings: phy: eswin: Document for EIC7700 SoC SATA PHY
  2025-08-21  7:55   ` Krzysztof Kozlowski
@ 2025-08-26  3:23     ` luyulin
  0 siblings, 0 replies; 13+ messages in thread
From: luyulin @ 2025-08-26  3:23 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: dlemoal, cassel, robh, krzk+dt, conor+dt, linux-ide, devicetree,
	linux-kernel, vkoul, kishon, linux-phy, ningyu, zhengyu, linmin,
	huangyifeng, fenglin, lianghujun

Hi,
Krzysztof

I apologize for incorrectly filling in the "In-Reply-To" field
when I first sent this patch, which caused it to lose association
with the other patches.
As a result, I have resent this patch.
Could you please help review the other patches in this patch series again?
The link to the cover letter for this patch series is:
https://lore.kernel.org/lkml/20250819134722.220-1-luyulin@eswincomputing.com/
Thank you very much for your review suggestions!
> 
> On Wed, Aug 20, 2025 at 05:27:58PM +0800, Yulin Lu wrote:
> > Add document for the SATA phy on the EIC7700 SoC platform,
> > describing its usage.
> > 
> > Signed-off-by: Yulin Lu <luyulin@eswincomputing.com>
> 
> You already sent this patch separately (!!!) and received review.
> 
> Best regards,
> Krzysztof

Best regards,
Yulin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Re: [PATCH v2 1/3] dt-bindings: ata: eswin: Document for EIC7700 SoC ahci
  2025-08-19 14:25   ` Rob Herring
  2025-08-25  8:19     ` luyulin
@ 2025-08-28 10:22     ` luyulin
  2025-08-28 13:05       ` Niklas Cassel
  1 sibling, 1 reply; 13+ messages in thread
From: luyulin @ 2025-08-28 10:22 UTC (permalink / raw)
  To: Rob Herring
  Cc: dlemoal, cassel, krzk+dt, conor+dt, linux-ide, devicetree,
	linux-kernel, vkoul, kishon, linux-phy, ningyu, zhengyu, linmin,
	huangyifeng, fenglin, lianghujun

Hello, Rob

Thank you very much for your reply.
I have a question that I would like to seek your advice on and clarify.

> 
> On Tue, Aug 19, 2025 at 8:54 AM Yulin Lu <luyulin@eswincomputing.com> wrote:
> >
> > From: luyulin <luyulin@eswincomputing.com>
> 
> Please fix your name.
> 
> >
> > Add document for the SATA AHCI controller on the EIC7700 SoC platform,
> > including descriptions of its hardware configurations.
> >
> > Signed-off-by: luyulin <luyulin@eswincomputing.com>
> 
> And here.
> 
> > ---
> >  .../bindings/ata/eswin,eic7700-ahci.yaml      | 92 +++++++++++++++++++
> >  1 file changed, 92 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml b/Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml
> > new file mode 100644
> > index 000000000000..9ef58c9c2f28
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml
> > @@ -0,0 +1,92 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/ata/eswin,eic7700-ahci.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Eswin EIC7700 SoC SATA Controller
> > +
> > +maintainers:
> > +  - Yulin Lu <luyulin@eswincomputing.com>
> > +  - Huan He <hehuan1@eswincomputing.com>
> > +
> > +description:
> > +  This document defines device tree bindings for the Synopsys DWC
> > +  implementation of the AHCI SATA controller found in Eswin's
> > +  Eic7700 SoC platform.
> > +
> > +select:
> > +  properties:
> > +    compatible:
> > +      const: eswin,eic7700-ahci
> > +  required:
> > +    - compatible
> > +
> > +allOf:
> > +  - $ref: snps,dwc-ahci-common.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: eswin,eic7700-ahci
> > +      - const: snps,dwc-ahci
> > +
> > +  reg:
> > +    maxItems: 1
> 
> Drop. snps,dwc-ahci-common.yaml already defines this.
> 
> > +
> > +  interrupts:
> > +    maxItems: 1
> 
> Drop. snps,dwc-ahci-common.yaml already defines this.
> 
> > +
> > +  ports-implemented:
> > +    const: 1
> 
> Really, your firmware should initialize the DWC specific register that
> sets this and is discoverable via a standard AHCI register.
> 

Accord to my understanding, if ports-implemented is configured in the dts,
this register will be set by the platform driver in libahci_platform.c.

Do you mean that ports-implemented should be removed from the dts,
and the corresponding register should be configured by the firmware
(which is U-Boot on the HiFive Premier P550 board)? Is this understanding correct?
If so, when the driver is removed, a reset will be triggered,
causing the configuration of this register to be lost,
which will result in an error when insmod the driver again.

Best regards,
Yulin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Re: [PATCH v2 1/3] dt-bindings: ata: eswin: Document for EIC7700 SoC ahci
  2025-08-28 10:22     ` luyulin
@ 2025-08-28 13:05       ` Niklas Cassel
  2025-08-29  6:22         ` luyulin
  0 siblings, 1 reply; 13+ messages in thread
From: Niklas Cassel @ 2025-08-28 13:05 UTC (permalink / raw)
  To: luyulin
  Cc: Rob Herring, dlemoal, krzk+dt, conor+dt, linux-ide, devicetree,
	linux-kernel, vkoul, kishon, linux-phy, ningyu, zhengyu, linmin,
	huangyifeng, fenglin, lianghujun

On Thu, Aug 28, 2025 at 06:22:40PM +0800, luyulin@eswincomputing.com wrote:
> 
> Do you mean that ports-implemented should be removed from the dts,
> and the corresponding register should be configured by the firmware
> (which is U-Boot on the HiFive Premier P550 board)? Is this understanding correct?
> If so, when the driver is removed, a reset will be triggered,
> causing the configuration of this register to be lost,
> which will result in an error when insmod the driver again.

My 50 cents,

if the ports implemented register gets reset from the reset_control_reset()
in ahci_platform_assert_rsts(), then it seems like having ports-implemented
in device tree is acceptable.

There are a bunch of device trees that have this already:
arch/arm/boot/dts/qcom/qcom-apq8064.dtsi:                       ports-implemented = <0x1>;
arch/arm/boot/dts/qcom/qcom-ipq8064-v1.0.dtsi:                  ports-implemented = <0x1>;
arch/arm/boot/dts/qcom/qcom-ipq8064-v2.0.dtsi:  ports-implemented = <0x1>;
arch/arm/boot/dts/samsung/exynos5250.dtsi:                      ports-implemented = <0x1>;
arch/arm/boot/dts/socionext/uniphier-pro4.dtsi:                 ports-implemented = <1>;
arch/arm/boot/dts/socionext/uniphier-pro4.dtsi:                 ports-implemented = <1>;
arch/arm/boot/dts/socionext/uniphier-pxs2.dtsi:                 ports-implemented = <1>;
arch/arm/boot/dts/st/stih407-family.dtsi:                       ports-implemented = <0x1>;
arch/arm/boot/dts/st/stih407-family.dtsi:                       ports-implemented = <0x1>;
arch/arm/boot/dts/ti/omap/dra7-l4.dtsi:                         ports-implemented = <0x1>;
arch/arm/boot/dts/ti/omap/omap5-l4.dtsi:                                ports-implemented = <0x1>;
arch/arm64/boot/dts/mediatek/mt7622.dtsi:               ports-implemented = <0x1>;
arch/arm64/boot/dts/rockchip/rk3568.dtsi:               ports-implemented = <0x1>;
arch/arm64/boot/dts/rockchip/rk356x-base.dtsi:          ports-implemented = <0x1>;
arch/arm64/boot/dts/rockchip/rk356x-base.dtsi:          ports-implemented = <0x1>;
arch/arm64/boot/dts/rockchip/rk3576.dtsi:                       ports-implemented = <0x1>;
arch/arm64/boot/dts/rockchip/rk3576.dtsi:                       ports-implemented = <0x1>;
arch/arm64/boot/dts/rockchip/rk3588-base.dtsi:          ports-implemented = <0x1>;
arch/arm64/boot/dts/rockchip/rk3588-base.dtsi:          ports-implemented = <0x1>;
arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi:         ports-implemented = <0x1>;
arch/arm64/boot/dts/socionext/uniphier-pxs3.dtsi:                       ports-implemented = <1>;
arch/arm64/boot/dts/socionext/uniphier-pxs3.dtsi:                       ports-implemented = <1>;


Sure, if the ports implemented register was sticky (kept its value after a
reset), then I think Rob's suggestion would make sense.



Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Re: Re: [PATCH v2 1/3] dt-bindings: ata: eswin: Document for EIC7700 SoC ahci
  2025-08-28 13:05       ` Niklas Cassel
@ 2025-08-29  6:22         ` luyulin
  0 siblings, 0 replies; 13+ messages in thread
From: luyulin @ 2025-08-29  6:22 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Rob Herring, dlemoal, krzk+dt, conor+dt, linux-ide, devicetree,
	linux-kernel, vkoul, kishon, linux-phy, ningyu, zhengyu, linmin,
	huangyifeng, fenglin, lianghujun


> On Thu, Aug 28, 2025 at 06:22:40PM +0800, luyulin@eswincomputing.com wrote:
> > 
> > Do you mean that ports-implemented should be removed from the dts,
> > and the corresponding register should be configured by the firmware
> > (which is U-Boot on the HiFive Premier P550 board)? Is this understanding correct?
> > If so, when the driver is removed, a reset will be triggered,
> > causing the configuration of this register to be lost,
> > which will result in an error when insmod the driver again.
> 
> My 50 cents,
> 
> if the ports implemented register gets reset from the reset_control_reset()
> in ahci_platform_assert_rsts(), then it seems like having ports-implemented
> in device tree is acceptable.
> 

In our design, the ports implemented register gets reset from the ahci_platform_assert_rsts().

> There are a bunch of device trees that have this already:
> arch/arm/boot/dts/qcom/qcom-apq8064.dtsi:                       ports-implemented = <0x1>;
> arch/arm/boot/dts/qcom/qcom-ipq8064-v1.0.dtsi:                  ports-implemented = <0x1>;
> arch/arm/boot/dts/qcom/qcom-ipq8064-v2.0.dtsi:  ports-implemented = <0x1>;
> arch/arm/boot/dts/samsung/exynos5250.dtsi:                      ports-implemented = <0x1>;
> arch/arm/boot/dts/socionext/uniphier-pro4.dtsi:                 ports-implemented = <1>;
> arch/arm/boot/dts/socionext/uniphier-pro4.dtsi:                 ports-implemented = <1>;
> arch/arm/boot/dts/socionext/uniphier-pxs2.dtsi:                 ports-implemented = <1>;
> arch/arm/boot/dts/st/stih407-family.dtsi:                       ports-implemented = <0x1>;
> arch/arm/boot/dts/st/stih407-family.dtsi:                       ports-implemented = <0x1>;
> arch/arm/boot/dts/ti/omap/dra7-l4.dtsi:                         ports-implemented = <0x1>;
> arch/arm/boot/dts/ti/omap/omap5-l4.dtsi:                                ports-implemented = <0x1>;
> arch/arm64/boot/dts/mediatek/mt7622.dtsi:               ports-implemented = <0x1>;
> arch/arm64/boot/dts/rockchip/rk3568.dtsi:               ports-implemented = <0x1>;
> arch/arm64/boot/dts/rockchip/rk356x-base.dtsi:          ports-implemented = <0x1>;
> arch/arm64/boot/dts/rockchip/rk356x-base.dtsi:          ports-implemented = <0x1>;
> arch/arm64/boot/dts/rockchip/rk3576.dtsi:                       ports-implemented = <0x1>;
> arch/arm64/boot/dts/rockchip/rk3576.dtsi:                       ports-implemented = <0x1>;
> arch/arm64/boot/dts/rockchip/rk3588-base.dtsi:          ports-implemented = <0x1>;
> arch/arm64/boot/dts/rockchip/rk3588-base.dtsi:          ports-implemented = <0x1>;
> arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi:         ports-implemented = <0x1>;
> arch/arm64/boot/dts/socionext/uniphier-pxs3.dtsi:                       ports-implemented = <1>;
> arch/arm64/boot/dts/socionext/uniphier-pxs3.dtsi:                       ports-implemented = <1>;
> 
> 
> Sure, if the ports implemented register was sticky (kept its value after a
> reset), then I think Rob's suggestion would make sense.
> 
> 

Thank you very much for the clarification.

> 
> Kind regards,
> Niklas

Best regards,
Yulin

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-08-29  6:22 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-19 13:47 [PATCH v2 0/3] ESWIN EIC7700 sata phy driver and yaml, Yulin Lu
2025-08-19 13:54 ` [PATCH v2 1/3] dt-bindings: ata: eswin: Document for EIC7700 SoC ahci Yulin Lu
2025-08-19 14:25   ` Rob Herring
2025-08-25  8:19     ` luyulin
2025-08-28 10:22     ` luyulin
2025-08-28 13:05       ` Niklas Cassel
2025-08-29  6:22         ` luyulin
2025-08-19 14:00 ` [PATCH v2 3/3] phy: eswin: Create eswin directory and add EIC7700 SATA PHY driver Yulin Lu
2025-08-20 16:07   ` Vinod Koul
2025-08-21  8:41   ` kernel test robot
2025-08-20  9:27 ` [PATCH v2 2/3] dt-bindings: phy: eswin: Document for EIC7700 SoC SATA PHY Yulin Lu
2025-08-21  7:55   ` Krzysztof Kozlowski
2025-08-26  3:23     ` luyulin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).