Add the device tree bindings for the Realtek DHC(Digital Home Center)
RTD SoCs PCIe PHY.
Signed-off-by: Tzuyi Chang <[email protected]>
---
.../bindings/phy/realtek,rtd-pcie-phy.yaml | 61 +++++++++++++++++++
1 file changed, 61 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/realtek,rtd-pcie-phy.yaml
diff --git a/Documentation/devicetree/bindings/phy/realtek,rtd-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/realtek,rtd-pcie-phy.yaml
new file mode 100644
index 000000000000..44ff23f698e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/realtek,rtd-pcie-phy.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2023 Realtek Semiconductor Corporation
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/realtek,rtd-pcie-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Realtek DHC PCIe PHY
+
+maintainers:
+ - Tzuyi Chang <[email protected]>
+
+description:
+ Realtek PCIe PHY supports the DHC(Digital Home Center) RTD series SoCs.
+ The PCIe PHY driver is designed to support physical layer functionality
+ of the PCIe controller.
+
+properties:
+ compatible:
+ enum:
+ - realtek,rtd1319-pcie0-phy
+ - realtek,rtd1319-pcie1-phy
+ - realtek,rtd1319-pcie2-phy
+ - realtek,rtd1619b-pcie1-phy
+ - realtek,rtd1619b-pcie2-phy
+ - realtek,rtd1319d-pcie1-phy
+ - realtek,rtd1315e-pcie1-phy
+
+ "#phy-cells":
+ const: 0
+
+ nvmem-cells:
+ maxItems: 1
+ description:
+ Phandle to nvmem cell that contains 'Tx swing trim'
+ tuning parameter value for PCIe phy.
+
+ nvmem-cell-names:
+ items:
+ - const: tx_swing_trim
+
+ realtek,pcie-syscon:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: phandle of syscon used to control PCIe MDIO register.
+
+required:
+ - compatible
+ - realtek,pcie-syscon
+ - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ pcie1_phy {
+ compatible = "realtek,rtd1319d-pcie1-phy";
+ realtek,pcie-syscon = <&pcie1>;
+ #phy-cells = <0>;
+ nvmem-cells = <&otp_pcie_tx_swing_trim>;
+ nvmem-cell-names = "tx_swing_trim";
+ };
--
2.43.0
On Fri, Dec 01, 2023 at 06:52:06PM +0800, Tzuyi Chang wrote:
> Add the device tree bindings for the Realtek DHC(Digital Home Center)
> RTD SoCs PCIe PHY.
>
> Signed-off-by: Tzuyi Chang <[email protected]>
> ---
> .../bindings/phy/realtek,rtd-pcie-phy.yaml | 61 +++++++++++++++++++
> 1 file changed, 61 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/realtek,rtd-pcie-phy.yaml
>
> diff --git a/Documentation/devicetree/bindings/phy/realtek,rtd-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/realtek,rtd-pcie-phy.yaml
> new file mode 100644
> index 000000000000..44ff23f698e6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/realtek,rtd-pcie-phy.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2023 Realtek Semiconductor Corporation
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/realtek,rtd-pcie-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Realtek DHC PCIe PHY
> +
> +maintainers:
> + - Tzuyi Chang <[email protected]>
> +
> +description:
> + Realtek PCIe PHY supports the DHC(Digital Home Center) RTD series SoCs.
> + The PCIe PHY driver is designed to support physical layer functionality
> + of the PCIe controller.
> +
> +properties:
> + compatible:
> + enum:
> + - realtek,rtd1319-pcie0-phy
> + - realtek,rtd1319-pcie1-phy
> + - realtek,rtd1319-pcie2-phy
> + - realtek,rtd1619b-pcie1-phy
> + - realtek,rtd1619b-pcie2-phy
Please explain why different PHYs on the same SoC need different
compatibles.
> + - realtek,rtd1319d-pcie1-phy
> + - realtek,rtd1315e-pcie1-phy
And why bother with the 1 here given there is no 0 or 2?
This looks suspiciously like abuse of the compatible - especially since
most of the ops are the same despite the differing compatibles. The case
where that does not apply, it looks like the issue is down to the
portion of the nvmem cell corresponding to the PHY, which has nothing to
do with the programming model of the PHY itself IMO.
Cheers,
Conor.
> +
> + "#phy-cells":
> + const: 0
> +
> + nvmem-cells:
> + maxItems: 1
> + description:
> + Phandle to nvmem cell that contains 'Tx swing trim'
> + tuning parameter value for PCIe phy.
> +
> + nvmem-cell-names:
> + items:
> + - const: tx_swing_trim
> +
> + realtek,pcie-syscon:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description: phandle of syscon used to control PCIe MDIO register.
> +
> +required:
> + - compatible
> + - realtek,pcie-syscon
> + - "#phy-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + pcie1_phy {
> + compatible = "realtek,rtd1319d-pcie1-phy";
> + realtek,pcie-syscon = <&pcie1>;
> + #phy-cells = <0>;
> + nvmem-cells = <&otp_pcie_tx_swing_trim>;
> + nvmem-cell-names = "tx_swing_trim";
> + };
> --
> 2.43.0
>
On 01/12/2023 11:52, Tzuyi Chang wrote:
> + "#phy-cells":
> + const: 0
> +
> + nvmem-cells:
> + maxItems: 1
> + description:
> + Phandle to nvmem cell that contains 'Tx swing trim'
> + tuning parameter value for PCIe phy.
> +
> + nvmem-cell-names:
> + items:
> + - const: tx_swing_trim
> +
> + realtek,pcie-syscon:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description: phandle of syscon used to control PCIe MDIO register.
Why this does not have reg property but syscon? This looks hacky.
Where is the DTS of your platform so we can verify the bindings? In the
past Realtek bindings and DTS were sent without testing.
> +
> +required:
> + - compatible
> + - realtek,pcie-syscon
> + - "#phy-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + pcie1_phy {
phy {
Best regards,
Krzysztof
Hi Conor,
Thank you for the review.
>> +properties:
>> + compatible:
>> + enum:
>
>> + - realtek,rtd1319-pcie0-phy
>> + - realtek,rtd1319-pcie1-phy
>> + - realtek,rtd1319-pcie2-phy
>> + - realtek,rtd1619b-pcie1-phy
>> + - realtek,rtd1619b-pcie2-phy
>
>Please explain why different PHYs on the same SoC need different compatibles.
>
I hadn't thought this clearly. I added the compatible for each PCIe ports. However,
only one compatible is needed for the PHY driver on each SoC.
I will revise it in the next version.
There are multiple ports for PCIe on different SoCs. RTD1319 has three PCIe ports (port 0, port1, port2).
RTD1619B has two PCIe ports. Both RTD1319D and RTD1315E have one PCIe port.
>> + - realtek,rtd1319d-pcie1-phy
>> + - realtek,rtd1315e-pcie1-phy
>
>And why bother with the 1 here given there is no 0 or 2?
>
I'm sorry for the confusion caused by the naming. The PCIe controller register address on
RTD1319D and RTD1315E is the same as RTD1319's PCIe port1, so I named it as pcie1.
I'll refrain from using such naming in the future.
>This looks suspiciously like abuse of the compatible - especially since most of
>the ops are the same despite the differing compatibles. The case where that
>does not apply, it looks like the issue is down to the portion of the nvmem cell
>corresponding to the PHY, which has nothing to do with the programming model
>of the PHY itself IMO.
Thanks,
Tzuyi Chang
Hi Krzysztof,
Thank you for the review.
>On 01/12/2023 11:52, Tzuyi Chang wrote:
>> + "#phy-cells":
>> + const: 0
>> +
>> + nvmem-cells:
>> + maxItems: 1
>> + description:
>> + Phandle to nvmem cell that contains 'Tx swing trim'
>> + tuning parameter value for PCIe phy.
>> +
>> + nvmem-cell-names:
>> + items:
>> + - const: tx_swing_trim
>> +
>> + realtek,pcie-syscon:
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> + description: phandle of syscon used to control PCIe MDIO register.
>
>Why this does not have reg property but syscon? This looks hacky.
>
Our PCIe PHY driver needs to access two registers:
1. PCIe MDIO register: Utilized for configuring the PCIe PHY.
2. PCIe MAC Link Control and Link Status Register: Use to get the current
link speed for calibration purposes.
Both these registers reside within the PCIe controller registers. The PCIe
driver has mapped these register address region, so I use regmap to access
these registers.
>Where is the DTS of your platform so we can verify the bindings? In the past
>Realtek bindings and DTS were sent without testing.
The bindings and DTS for our platform are continuously being adjusted for the upstream.
Therefore, I only modified and tested the DTS node of the binding documentations I submitted.
The DTS node is the same as the examples in the binding documentation. I tested it using the
command "make dtbs_check DT_SCHEMA_FILES=..." without encountering any errors.
>> +
>> +required:
>> + - compatible
>> + - realtek,pcie-syscon
>> + - "#phy-cells"
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + pcie1_phy {
>
>phy {
>
I will fix it in the next version.
Thanks,
Tzuyi Chang
On 07/12/2023 11:10, TY_Chang[張子逸] wrote:
> Hi Krzysztof,
>
> Thank you for the review.
>
>> On 01/12/2023 11:52, Tzuyi Chang wrote:
>>> + "#phy-cells":
>>> + const: 0
>>> +
>>> + nvmem-cells:
>>> + maxItems: 1
>>> + description:
>>> + Phandle to nvmem cell that contains 'Tx swing trim'
>>> + tuning parameter value for PCIe phy.
>>> +
>>> + nvmem-cell-names:
>>> + items:
>>> + - const: tx_swing_trim
>>> +
>>> + realtek,pcie-syscon:
>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>> + description: phandle of syscon used to control PCIe MDIO register.
>>
>> Why this does not have reg property but syscon? This looks hacky.
>>
>
> Our PCIe PHY driver needs to access two registers:
> 1. PCIe MDIO register: Utilized for configuring the PCIe PHY.
> 2. PCIe MAC Link Control and Link Status Register: Use to get the current
> link speed for calibration purposes.
>
> Both these registers reside within the PCIe controller registers. The PCIe
> driver has mapped these register address region, so I use regmap to access
> these registers.
Hm, isn't in such case PCIe PHY a child of the PCIe controller? How is
it with resources, like power domains or regulators?
Best regards,
Krzysztof
Hi Krzysztof,
>>> On 01/12/2023 11:52, Tzuyi Chang wrote:
>>>> + "#phy-cells":
>>>> + const: 0
>>>> +
>>>> + nvmem-cells:
>>>> + maxItems: 1
>>>> + description:
>>>> + Phandle to nvmem cell that contains 'Tx swing trim'
>>>> + tuning parameter value for PCIe phy.
>>>> +
>>>> + nvmem-cell-names:
>>>> + items:
>>>> + - const: tx_swing_trim
>>>> +
>>>> + realtek,pcie-syscon:
>>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>>> + description: phandle of syscon used to control PCIe MDIO register.
>>>
>>> Why this does not have reg property but syscon? This looks hacky.
>>>
>>
>> Our PCIe PHY driver needs to access two registers:
>> 1. PCIe MDIO register: Utilized for configuring the PCIe PHY.
>> 2. PCIe MAC Link Control and Link Status Register: Use to get the current
>> link speed for calibration purposes.
>>
>> Both these registers reside within the PCIe controller registers. The
>> PCIe driver has mapped these register address region, so I use regmap
>> to access these registers.
>
>Hm, isn't in such case PCIe PHY a child of the PCIe controller? How is it with
>resources, like power domains or regulators?
In fact, I positioned the PCIe PHY node outside the PCIe controller node.
It would be more appropriate for the PCIe PHY as the child node of the PCIe
controller. I will revise to this structure.
I will also remove the "realtek,pcie-syscon" property and use dev->parent->of_node
to get the syscon of the PCIe controller.
Since the MDIO register is located within the PCIe controller registers, it can
only be accessed after enabling the clock and asserting the resets of the PCIe controller.
Therefore, the PCIe PHY driver only registers the callback functions of phy_ops (.init and .calibrate).
After the PCIe controller driver sets the clock and resets, it will execute PHY framework API to
configure the PHY.
Thanks,
Tzuyi Chang