Skip to contentSkip to conversation

Add driver support for Eswin EIC7700 SoC SATA PHY

PATCH Superseded Yulin Lu 2 participants 4 messages Jan 23

Subsystems (2)

v8 of 9 Superseded Latest is v9 →

lore
Share & tools
lore

Yulin Lu

author
#0 lore raw

v8 cover Add driver support for Eswin EIC7700 SoC SATA PHY

Updates:
v8 -> v7:
- eswin,eic7700-sata-phy.yaml
- Add "eswin,tx-amplitude-tuning" and "eswin,tx-preemph-tuning" properties, because these parameters may vary across different
circuit boards.
- Delete reviewed-by tag of Krzysztof Kozlowski, because the tuning properties are introduced.
- phy-eic7700-sata.c
- Try to get SATA PHY transmitter amplitude and pre-emphasis signal
eye diagram tuning parameters from dts instead of hardcoded values
in the code. Because, these parameters may vary across different
circuit boards. Define default tuning parameters and use it when
these properties are not declared in dts.
- Add a comment to explain the reason for mapping I/O resources with platform_get_resource and devm_ioremap instead of using the devm_platform_ioremap_resource API.
- Link to v7: https://lore.kernel.org/lkml/[email protected]/

v7 -> v6:
- phy-eic7700-sata.c
- Rename PHY_READY_TIMEOUT to PLL_LOCK_TIMEOUT_US with value 1000.
- Add macro PLL_LOCK_SLEEP_US set to 10.
- Add "goto disable_clk" in the eic7700_sata_phy_init function.
- Modify Copyright year from 2024 to 2026.
- Link to v6: https://lore.kernel.org/lkml/[email protected]/

v6 -> v5:
- eswin,eic7700-ahci.yaml
- Delete this file and it has already been applied in reply[1].
- eswin,eic7700-sata-phy.yaml
- Add clock and reset related properties.
- phy-eic7700-sata.c
- Map the io resource with platform_get_resource and devm_ioremap
instead of devm_platform_ioremap_resource API. Because the address region of sata-phy falls into the region of hsp clock&reset which
has been got by hsp clock&reset driver.
- Use regmap_read_poll_timeout in wait_for_phy_ready to replace the
while loop check.
- Use devm_regmap_init_mmio and regmap_write to replace writel.
- Adapt to the clock and reset driver framework, replacing the
original readl and writel.
Because we are implementing the HSP layer clock and reset drivers,
the corresponding clock and reset registers can be registered into
the driver framework. And I have tested on the Sifive HiFive
Premier P550 board.
- Link to v5: https://lore.kernel.org/lkml/[email protected]/

v5 -> v4:
- eswin,eic7700-ahci.yaml
- Add "dt-bindings: ata:" prefix to the subject.
- Wrap at 80 characters in the YAML description field.
- Link to v4: https://lore.kernel.org/lkml/[email protected]/

v4 -> v3:
- eswin,eic7700-ahci.yaml
- Fix grammatical errors in patch subject and commit message
- Add an explanation in the commit message of patch 1 for retaining
the "ports-implemented" field, which Rob Herring suggested to
remove in the review comments on v2.
Link to Rob Herring's review:
https://lore.kernel.org/lkml/CAL_JsqKFotNLZZXwiy7S6K8qXLdGRAnsa-1zvZRDQBE39Gf5kg@mail.gmail.com/
Link to my question and Niklas Cassel's reply:
https://lore.kernel.org/lkml/[email protected]/
In this reply, Niklas Cassel mentioned his view:
If the ports-implemented register gets reset from ahci_platform_assert_rsts(), then it seems acceptable to
retain the ports-implemented property in the device tree.
This aligns with our design.
Link to my reply:
https://lore.kernel.org/lkml/[email protected]/
Link to Niklas Cassel's question and my further explanation:
https://lore.kernel.org/lkml/aLlYkZWBaI5Yz6fo@ryzen/
https://lore.kernel.org/lkml/[email protected]/
- eswin,eic7700-sata-phy.yaml
- Fix grammatical errors in patch subject and commit message
- Adjust the position of reg in the properties and required arrays
- Add reviewed-by tag of Krzysztof Kozlowski
- phy-eic7700-sata.c
- Correct the loop condition in wait_for_phy_ready() to use the
current jiffies instead of the fixed start time.
- Change the return value from -EFAULT to -ETIMEDOUT to correctly
indicate a timeout condition.
- Remove redundant clock disable handling in probe error path, as SATA_SYS_CLK_EN is managed in phy_init() and phy_exit().
- Use dev_err_probe return in probe.
- Reorder local variables to follow reverse Xmas tree order.
- Wrap each line in the extended comments to 80 columns before
splitting lines.
- Adjust the position of #include <linux/io.h> for proper ordering.
- Link to v3: https://lore.kernel.org/lkml/[email protected]/

v2 -> v3:
- Use full name in "From" and "Signed-off-by" fields information
- eswin,eic7700-ahci.yaml
- Remove the introduction to the reg, interrupts, phys, and
phy-names fields.
- Modify the usage of the clocks field in the examples.
- Correct the order of dt properties.
- phy-eic7700-sata.c
- Register operations use the GENMASK macro and FIELD_PREP instead
of the original bit offset method, and add
"#include <linux/bitfield.h>".
- Modify some macro definition names.
- Remove the redundant initialization assignments for "ret" and
"val".
- Delete ".suppress_bind_attrs = true".
- Modify the driver name.
- Add "#include <linux/io.h>" to fix the robot test issue.
- Link to v2: https://lore.kernel.org/lkml/[email protected]/

v2 -> 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/[email protected]/

Yulin Lu (2):
dt-bindings: phy: eswin: Document the EIC7700 SoC SATA PHY
phy: eswin: Create eswin directory and add EIC7700 SATA PHY driver

 .../bindings/phy/eswin,eic7700-sata-phy.yaml  |  96 ++++++
 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          | 273 ++++++++++++++++++
 6 files changed, 387 insertions(+)
 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
Signature · 7 lines
2.25.1


-- 
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy
author
#1 lore raw

PATCH v8 1/2 dt-bindings: phy: eswin: Document the EIC7700 SoC SATA PHY

Document the SATA PHY on the EIC7700 SoC platform, describing its usage.

---
 .../bindings/phy/eswin,eic7700-sata-phy.yaml  | 96 +++++++++++++++++++
 1 file changed, 96 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/eswin,eic7700-sata-phy.yaml
Documentation/devicetree/bindings/phy/eswin,eic7700-sata-phy.yaml new file +96 -0
new file mode 100644
index 000000000000..4bb2d7a53761
@@ -0,0 +1,96 @@
+# 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 <[email protected]>
+  - Huan He <[email protected]>
+
+properties:
+  compatible:
+    const: eswin,eic7700-sata-phy
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: phy
+
+  resets:
+    maxItems: 2
+
+  reset-names:
+    items:
+      - const: port
+      - const: phy
+
+  eswin,tx-amplitude-tuning:
+    description: This adjusts the transmitter amplitude signal, and its value
+      is derived from eye diagram tuning. The three values correspond to Gen1,
+      Gen2, and Gen3 parameters respectively.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    items:
+      - description: Gen1 parameter.
+        minimum: 0
+        maximum: 0x7f
+        default: 0
+      - description: Gen2 parameter.
+        minimum: 0
+        maximum: 0x7f
+        default: 0
+      - description: Gen3 parameter.
+        minimum: 0
+        maximum: 0x7f
+        default: 0
+
+  eswin,tx-preemph-tuning:
+    description: This adjusts the transmitter de-emphasis signal, and its value
+      is derived from eye diagram tuning. The three values correspond to Gen1,
+      Gen2, and Gen3 parameters respectively.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    items:
+      - description: Gen1 parameter.
+        minimum: 0
+        maximum: 0x3f
+        default: 0
+      - description: Gen2 parameter.
+        minimum: 0
+        maximum: 0x3f
+        default: 0
+      - description: Gen3 parameter.
+        minimum: 0
+        maximum: 0x3f
+        default: 0
+
+  "#phy-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    sata-phy@50440300 {
+        compatible = "eswin,eic7700-sata-phy";
+        reg = <0x50440300 0x40>;
+        clocks = <&hspcrg 17>;
+        clock-names = "phy";
+        resets = <&hspcrg 0>, <&hspcrg 1>;
+        reset-names = "port", "phy";
+        #phy-cells = <0>;
+    };
Signature · 1 line
2.25.1
#2 lore raw

Re: [PATCH v8 1/2] dt-bindings: phy: eswin: Document the EIC7700 SoC SATA PHY

Quoted 8 lines · Yulin Lu
Documentation/devicetree/bindings/phy/eswin,eic7700-sata-phy.yaml new file +8 -0
+  eswin,tx-amplitude-tuning:
+    description: This adjusts the transmitter amplitude signal, and its value
+      is derived from eye diagram tuning. The three values correspond to Gen1,
+      Gen2, and Gen3 parameters respectively.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    items:
+      - description: Gen1 parameter.
+        minimum: 0

Documentation/devicetree/bindings/phy/eswin,eic7700-sata-phy.yaml line 51

Quoted 10 lines
Documentation/devicetree/bindings/phy/eswin,eic7700-sata-phy.yaml new file +10 -0
+        maximum: 0x7f
+        default: 0
+      - description: Gen2 parameter.
+        minimum: 0
+        maximum: 0x7f
+        default: 0
+      - description: Gen3 parameter.
+        minimum: 0
+        maximum: 0x7f
+        default: 0

I think defaults should be specified like this:
https://elixir.bootlin.com/linux/v6.19-rc5/source/Documentation/devicetree/bindings/sound/adi,adau7118.yaml#L54

Quoted 9 lines
Documentation/devicetree/bindings/phy/eswin,eic7700-sata-phy.yaml new file +9 -0
+
+  eswin,tx-preemph-tuning:
+    description: This adjusts the transmitter de-emphasis signal, and its value
+      is derived from eye diagram tuning. The three values correspond to Gen1,
+      Gen2, and Gen3 parameters respectively.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    items:
+      - description: Gen1 parameter.
+        minimum: 0

Documentation/devicetree/bindings/phy/eswin,eic7700-sata-phy.yaml line 70

Quoted 10 lines
Documentation/devicetree/bindings/phy/eswin,eic7700-sata-phy.yaml new file +10 -0
+        maximum: 0x3f
+        default: 0
+      - description: Gen2 parameter.
+        minimum: 0
+        maximum: 0x3f
+        default: 0
+      - description: Gen3 parameter.
+        minimum: 0
+        maximum: 0x3f
+        default: 0

Same here

Best regards,
Krzysztof

author
#3 lore raw

PATCH v8 2/2 phy: eswin: Create eswin directory and add EIC7700 SATA PHY driver

In reply to Yulin Lu

Create the eswin phy driver directory and add support for the SATA PHY driver on the EIC7700 SoC platform.

---
 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 | 273 +++++++++++++++++++++++++++
 5 files changed, 291 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
drivers/phy/Kconfig +1 -0
index 678dd0452f0a..6d50704917f0 100644
@@ -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"
drivers/phy/Makefile +1 -0
index bfb27fb5a494..482a143d3417 100644
@@ -17,6 +17,7 @@ obj-y					+= allwinner/	\
 					   amlogic/	\
 					   broadcom/	\
 					   cadence/	\
+					   eswin/	\
 					   freescale/	\
 					   hisilicon/	\
 					   ingenic/	\
drivers/phy/eswin/Kconfig new file +14 -0
new file mode 100644
index 000000000000..37447cc3af63
@@ -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.
drivers/phy/eswin/Makefile new file +2 -0
new file mode 100644
index 000000000000..db08c66be812
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_PHY_EIC7700_SATA)	+= phy-eic7700-sata.o
drivers/phy/eswin/phy-eic7700-sata.c new file +273 -0

273 lines not on this page · raw message

Signature · 1 line
2.25.1
Signature · 3 lines
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy