[PATCH v3 0/2] phy: ti: ti-pipe3: allow configuring ACSPCIe TX/RX mode

[PATCH v3 0/2] phy: ti: ti-pipe3: allow configuring ACSPCIe TX/RX mode

[David Oberhollenzer]

This patch set adds a device tree setting to the PIPE3 PHY for
the reference clock selection of the ACSPCIe clock buffer stage.

The clock buffer can operate in either RX mode (clock pins are input,
recover the clock from the bus) or in TX mode (generate the reference
clock for the bus).

This was tested on a board using a phyCORE-AM57x SoM.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>

---
Changes in v3:
 - Cleanup device tree property schema

---
Changes in v2:
 - Add ti vendor prefix to property name
 - Reorder patch set to add the device tree documentation first
 - Add a property description to the device tree schema
 - Add an example to the device tree documentation
 - Make the property exclusive to the PCIe PHY in the device tree schema

---
Sashiko:
 - syscon_regmap_lookup_by_phandle_args() does not return -EPROBE_DEFER,
   there is no broken probe deferral here
 - logging but ignoring a failure of ti_pipe3_acspcie_tx_rx_mode() is
   intentional, following implementations of related properties,
   e.g. dra7xx_pcie_unaligned_memaccess()

---

David Oberhollenzer (2):
  dt-bindings: phy: ti,phy-usb3: Add aspcie reference clock setting
  phy: ti: ti-pipe3: allow configuring ACSPCIe TX/RX mode

 .../devicetree/bindings/phy/ti,phy-usb3.yaml  | 38 +++++++++++++++++++
 drivers/phy/ti/phy-ti-pipe3.c                 | 27 +++++++++++++
 2 files changed, 65 insertions(+)

-- 
2.55.0

[PATCH v3 1/2] dt-bindings: phy: ti,phy-usb3: Add aspcie reference clock setting

[David Oberhollenzer]

The PHY control logic in the OMAP PCIe PHY subsystem has a
configurable reference clock selection. It can either use
the ljcb_clkp/ljcb_clkn pin as inputs, recovering the clock
from the bus (RX mode), or generate the reference clock
internally, driving the bus pins as outputs (TX mode).

The setting is exposed through a register in a system controller.
A device tree property is added to specify the syscon node,
register offset and mode value.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 .../devicetree/bindings/phy/ti,phy-usb3.yaml  | 38 +++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/ti,phy-usb3.yaml b/Documentation/devicetree/bindings/phy/ti,phy-usb3.yaml
index 84f538aa587c..41e7bb42cbbe 100644
--- a/Documentation/devicetree/bindings/phy/ti,phy-usb3.yaml
+++ b/Documentation/devicetree/bindings/phy/ti,phy-usb3.yaml
@@ -52,6 +52,20 @@ properties:
       enum: [wkupclk, sysclk, refclk, dpll_ref,
              dpll_ref_m2, phy-div, div-clk]
 
+  ti,syscon-acspcie-tx-rx:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      Syscon register and setting to configure the clock buffer to run
+      in RX mode (clock pins are inputs, the clock is recovered from
+      the bus), or in TX mode (pins are outputs, internal reference
+      clock drives the bus).
+    maxItems: 1
+    items:
+      items:
+        - description: Phandle to the system control module
+        - description: Register offset of CTRL_CORE_SMA_SW_6
+        - description: Setting for PCIE_TX_RX_CONTROL
+
   syscon-phy-power:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     maxItems: 1
@@ -94,6 +108,15 @@ allOf:
     else:
       properties:
         syscon-pllreset: false
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              const: ti,phy-pipe3-pcie
+    then:
+      properties:
+        ti,syscon-acspcie-tx-rx: false
 
 required:
   - reg
@@ -135,4 +158,19 @@ examples:
         syscon-pllreset = <&scm_conf 0x3fc>;
         #phy-cells = <0>;
     };
+
+  - |
+    /* TI PIPE3 PCIE PHY */
+    pcie-phy@4a0a4000 {
+        compatible = "ti,phy-pipe3-pcie";
+        reg = <0x4a0a4000 0x80>,  /* phy_rx */
+              <0x4a0a4400 0x64>;  /* phy_tx */
+        reg-names = "phy_rx", "phy_tx";
+        clocks = <&dpll_pcie_ref_ck>, <&dpll_pcie_ref_m2ldo_ck>;
+        clock-names = "dpll_ref", "dpll_ref_m2";
+        #phy-cells = <0>;
+        syscon-phy-power = <&scm_conf_pcie 0x1c>;
+        syscon-pcs = <&scm_conf_pcie 0x10>;
+        ti,syscon-acspcie-tx-rx = <&scm_conf1 0x10 0x2>;
+    };
 ...
-- 
2.55.0

Re: [PATCH v3 1/2] dt-bindings: phy: ti,phy-usb3: Add aspcie reference clock setting

[Krzysztof Kozlowski]

On Sat, Sep 05, 2026 at 11:15:54PM +0200, David Oberhollenzer wrote:
> +      not:
> +        properties:
> +          compatible:
> +            contains:
> +              const: ti,phy-pipe3-pcie
> +    then:
> +      properties:
> +        ti,syscon-acspcie-tx-rx: false
>  
>  required:
>    - reg
> @@ -135,4 +158,19 @@ examples:
>          syscon-pllreset = <&scm_conf 0x3fc>;
>          #phy-cells = <0>;
>      };
> +
> +  - |
> +    /* TI PIPE3 PCIE PHY */
> +    pcie-phy@4a0a4000 {

There are already two examples, so we usually do not need a third one
for difference in one property.

Drop, with this change:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

[PATCH v3 2/2] phy: ti: ti-pipe3: allow configuring ACSPCIe TX/RX mode

[David Oberhollenzer]

For the PCIe PHYs, read the property from the device tree and
update the RX/TX mode selection bits in the specified syscon
register.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 drivers/phy/ti/phy-ti-pipe3.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
index b5543b5c674c..bc02010dadb7 100644
--- a/drivers/phy/ti/phy-ti-pipe3.c
+++ b/drivers/phy/ti/phy-ti-pipe3.c
@@ -115,6 +115,9 @@
 #define MEM_CDR_LOS_SOURCE_MASK		GENMASK(10, 9)
 #define MEM_CDR_LOS_SOURCE_SHIFT	9
 
+#define PCIE_TX_RX_CTRL_MASK		GENMASK(17, 16)
+#define PCIE_TX_RX_CTRL_SHIFT		16
+
 /*
  * This is an Empirical value that works, need to confirm the actual
  * value required for the PIPE3PHY_PLL_CONFIGURATION2.PLL_IDLE status
@@ -785,6 +788,24 @@ static int ti_pipe3_get_pll_base(struct ti_pipe3 *phy)
 	return PTR_ERR_OR_ZERO(phy->pll_ctrl_base);
 }
 
+static int ti_pipe3_acspcie_tx_rx_mode(struct ti_pipe3 *phy)
+{
+	struct device_node *np = phy->dev->of_node;
+	struct regmap *regmap;
+	unsigned int args[2];
+
+	regmap = syscon_regmap_lookup_by_phandle_args(np,
+						      "ti,syscon-acspcie-tx-rx",
+						      2, args);
+	if (IS_ERR(regmap)) {
+		dev_warn(phy->dev, "can't get ti,syscon-acspcie-tx-rx\n");
+		return -EINVAL;
+	}
+
+	return regmap_update_bits(regmap, args[0], PCIE_TX_RX_CTRL_MASK,
+				  args[1] << PCIE_TX_RX_CTRL_SHIFT);
+}
+
 static int ti_pipe3_probe(struct platform_device *pdev)
 {
 	struct ti_pipe3 *phy;
@@ -807,6 +828,12 @@ static int ti_pipe3_probe(struct platform_device *pdev)
 	phy->dpll_map = data->dpll_map;
 	phy->settings = data->settings;
 
+	if (phy->mode == PIPE3_MODE_PCIE) {
+		ret = ti_pipe3_acspcie_tx_rx_mode(phy);
+		if (ret)
+			dev_warn(dev, "failed to set ACSPCIe TX/RX mode\n");
+	}
+
 	ret = ti_pipe3_get_pll_base(phy);
 	if (ret)
 		return ret;
-- 
2.55.0

Re: [PATCH v3 2/2] phy: ti: ti-pipe3: allow configuring ACSPCIe TX/RX mode

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver improperly handles errors from `syscon_regmap_lookup_by_phandle_args()`, breaking probe deferral and masking failure states.
--

commit 222aba3021868be395b0b2808f4e70dc057987a7
Author: David Oberhollenzer <david.oberhollenzer@sigma-star.at>

phy: ti: ti-pipe3: allow configuring ACSPCIe TX/RX mode

For the PCIe PHYs, read the property from the device tree and
update the RX/TX mode selection bits in the specified syscon
register.

> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
> index b5543b5c674c8..bc02010dadb74 100644
> --- a/drivers/phy/ti/phy-ti-pipe3.c
> +++ b/drivers/phy/ti/phy-ti-pipe3.c

[ ... ]

> @@ -785,6 +788,24 @@ static int ti_pipe3_get_pll_base(struct ti_pipe3 *phy)
>  	return PTR_ERR_OR_ZERO(phy->pll_ctrl_base);
>  }
>  
> +static int ti_pipe3_acspcie_tx_rx_mode(struct ti_pipe3 *phy)
> +{
> +	struct device_node *np = phy->dev->of_node;
> +	struct regmap *regmap;
> +	unsigned int args[2];
> +
> +	regmap = syscon_regmap_lookup_by_phandle_args(np,
> +						      "ti,syscon-acspcie-tx-rx",
> +						      2, args);
> +	if (IS_ERR(regmap)) {
> +		dev_warn(phy->dev, "can't get ti,syscon-acspcie-tx-rx\n");
> +		return -EINVAL;

[Severity: High]
Does this mask the original error code (such as -EPROBE_DEFER or -ENOENT)
from syscon_regmap_lookup_by_phandle_args() by returning a hardcoded
-EINVAL? Unconditionally logging a warning and returning -EINVAL could
break probe deferral if the syscon device probes later.

> +	}
> +
> +	return regmap_update_bits(regmap, args[0], PCIE_TX_RX_CTRL_MASK,
> +				  args[1] << PCIE_TX_RX_CTRL_SHIFT);
> +}
> +
>  static int ti_pipe3_probe(struct platform_device *pdev)
>  {
>  	struct ti_pipe3 *phy;
> @@ -807,6 +828,12 @@ static int ti_pipe3_probe(struct platform_device *pdev)
>  	phy->dpll_map = data->dpll_map;
>  	phy->settings = data->settings;
>  
> +	if (phy->mode == PIPE3_MODE_PCIE) {
> +		ret = ti_pipe3_acspcie_tx_rx_mode(phy);
> +		if (ret)
> +			dev_warn(dev, "failed to set ACSPCIe TX/RX mode\n");
> +	}
> +
>  	ret = ti_pipe3_get_pll_base(phy);

[Severity: High]
Does this overwrite the returned error code in ret with the result of
ti_pipe3_get_pll_base()? If ti_pipe3_acspcie_tx_rx_mode() failed or
should have been deferred, ignoring the error here causes the probe
function to falsely succeed without configuring the PHY.

>  	if (ret)
>  		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905211555.15944-1-david.oberhollenzer@sigma-star.at?part=2

Re: [PATCH v3 2/2] phy: ti: ti-pipe3: allow configuring ACSPCIe TX/RX mode

[Krzysztof Kozlowski]

On Sat, Sep 05, 2026 at 11:15:55PM +0200, David Oberhollenzer wrote:
> For the PCIe PHYs, read the property from the device tree and
> update the RX/TX mode selection bits in the specified syscon
> register.
> 
> Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
> ---
>  drivers/phy/ti/phy-ti-pipe3.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
> index b5543b5c674c..bc02010dadb7 100644
> --- a/drivers/phy/ti/phy-ti-pipe3.c
> +++ b/drivers/phy/ti/phy-ti-pipe3.c
> @@ -115,6 +115,9 @@
>  #define MEM_CDR_LOS_SOURCE_MASK		GENMASK(10, 9)
>  #define MEM_CDR_LOS_SOURCE_SHIFT	9
>  
> +#define PCIE_TX_RX_CTRL_MASK		GENMASK(17, 16)
> +#define PCIE_TX_RX_CTRL_SHIFT		16
> +
>  /*
>   * This is an Empirical value that works, need to confirm the actual
>   * value required for the PIPE3PHY_PLL_CONFIGURATION2.PLL_IDLE status
> @@ -785,6 +788,24 @@ static int ti_pipe3_get_pll_base(struct ti_pipe3 *phy)
>  	return PTR_ERR_OR_ZERO(phy->pll_ctrl_base);
>  }
>  
> +static int ti_pipe3_acspcie_tx_rx_mode(struct ti_pipe3 *phy)
> +{
> +	struct device_node *np = phy->dev->of_node;
> +	struct regmap *regmap;
> +	unsigned int args[2];
> +
> +	regmap = syscon_regmap_lookup_by_phandle_args(np,
> +						      "ti,syscon-acspcie-tx-rx",
> +						      2, args);
> +	if (IS_ERR(regmap)) {
> +		dev_warn(phy->dev, "can't get ti,syscon-acspcie-tx-rx\n");
> +		return -EINVAL;
> +	}
> +
> +	return regmap_update_bits(regmap, args[0], PCIE_TX_RX_CTRL_MASK,
> +				  args[1] << PCIE_TX_RX_CTRL_SHIFT);
> +}
> +
>  static int ti_pipe3_probe(struct platform_device *pdev)
>  {
>  	struct ti_pipe3 *phy;
> @@ -807,6 +828,12 @@ static int ti_pipe3_probe(struct platform_device *pdev)
>  	phy->dpll_map = data->dpll_map;
>  	phy->settings = data->settings;
>  
> +	if (phy->mode == PIPE3_MODE_PCIE) {
> +		ret = ti_pipe3_acspcie_tx_rx_mode(phy);
> +		if (ret)
> +			dev_warn(dev, "failed to set ACSPCIe TX/RX mode\n");

This looks like new warning for existing ABI without any explanation why
is this a warning. Was it working? If yes, why existing boards should
warn now?

Best regards,
Krzysztof