Re: [PATCH v3 net-next 01/15] dt-bindings: ptp: add NETC Timer PTP clock

Frank Li
2025-08-15T18:11:12Z

On Fri, Aug 15, 2025 at 08:05:12AM +0200, Krzysztof Kozlowski wrote:
> On 14/08/2025 20:50, Frank Li wrote:
> > On Thu, Aug 14, 2025 at 10:25:14AM +0200, Krzysztof Kozlowski wrote:
> >> On Tue, Aug 12, 2025 at 05:46:20PM +0800, Wei Fang wrote:
> >>> NXP NETC (Ethernet Controller) is a multi-function PCIe Root Complex
> >>> Integrated Endpoint (RCiEP), the Timer is one of its functions which
> >>> provides current time with nanosecond resolution, precise periodic
> >>> pulse, pulse on timeout (alarm), and time capture on external pulse
> >>> support. And also supports time synchronization as required for IEEE
> >>> 1588 and IEEE 802.1AS-2020. So add device tree binding doc for the
> >>> PTP clock based on NETC Timer.
> >>>
> >>> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> >>>
> >>> ---
> >>> v2 changes:
> >>> 1. Refine the subject and the commit message
> >>> 2. Remove "nxp,pps-channel"
> >>> 3. Add description to "clocks" and "clock-names"
> >>> v3 changes:
> >>> 1. Remove the "system" clock from clock-names
> >>> ---
> >>> .../devicetree/bindings/ptp/nxp,ptp-netc.yaml | 63 +++++++++++++++++++
> >>> 1 file changed, 63 insertions(+)
> >>> create mode 100644 Documentation/devicetree/bindings/ptp/nxp,ptp-netc.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/ptp/nxp,ptp-netc.yaml b/Documentation/devicetree/bindings/ptp/nxp,ptp-netc.yaml
> >>> new file mode 100644
> >>> index 000000000000..60fb2513fd76
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/ptp/nxp,ptp-netc.yaml
> >>> @@ -0,0 +1,63 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/ptp/nxp,ptp-netc.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: NXP NETC V4 Timer PTP clock
> >>> +
> >>> +description:
> >>> + NETC V4 Timer provides current time with nanosecond resolution, precise
> >>> + periodic pulse, pulse on timeout (alarm), and time capture on external
> >>> + pulse support. And it supports time synchronization as required for
> >>> + IEEE 1588 and IEEE 802.1AS-2020.
> >>> +
> >>> +maintainers:
> >>> + - Wei Fang <wei.fang@nxp.com>
> >>> + - Clark Wang <xiaoning.wang@nxp.com>
> >>> +
> >>> +properties:
> >>> + compatible:
> >>> + enum:
> >>> + - pci1131,ee02
> >>> +
> >>> + reg:
> >>> + maxItems: 1
> >>> +
> >>> + clocks:
> >>> + maxItems: 1
> >>> + description:
> >>> + The reference clock of NETC Timer, if not present, indicates that
> >>> + the system clock of NETC IP is selected as the reference clock.
> >>> +
> >>> + clock-names:
> >>> + description:
> >>> + The "ccm_timer" means the reference clock comes from CCM of SoC.
> >>> + The "ext_1588" means the reference clock comes from external IO
> >>> + pins.
> >>> + enum:
> >>> + - ccm_timer
> >>
> >> You should name here how the input pin is called, not the source. Pin is
> >> "ref"?
> >>
> >>> + - ext_1588
> >>
> >> This should be just "ext"? We probably talked about this, but this feels
> >> like you describe one input in different ways.
> >>
> >> You will get the same questions in the future, if commit msg does not
> >> reflect previous talks.
> >>
> >>> +
> >>> +required:
> >>> + - compatible
> >>> + - reg
> >>> +
> >>> +allOf:
> >>> + - $ref: /schemas/pci/pci-device.yaml
> >>> +
> >>> +unevaluatedProperties: false
> >>> +
> >>> +examples:
> >>> + - |
> >>> + pcie {
> >>> + #address-cells = <3>;
> >>> + #size-cells = <2>;
> >>> +
> >>> + ethernet@18,0 {
> >>
> >> That's rather timer or ptp-timer or your binding is incorrect. Please
> >> describe COMPLETE device in your binding.
> >
> > Krzysztof:
> >
> > I have question about "COMPLETE" here. For some MFD/syscon, I know
> > need descript all children nodes to make MFD/syscon complete.
> >
> > But here it is PCIe device.
> >
> > pcie_4ca00000: pcie@4ca00000 {
> > compatible = "pci-host-ecam-generic";
> > ...
> >
> > enetc_port0: ethernet@0,0 {
> > compatible = "fsl,imx95-enetc", "...";
> > ...
> >
> > ptp-timer@18,0 {
> > compatible = "pci1131,ee02";
> > }
> > };
> >
> > parent "pci-host-ecam-generic" is common pci binding, each children
> > is indepentant part.
> >
> > I am not sure how to decript COMPLETE device for PCI devices.
>
> I don't know what is missing here, but naming it ethernet suggested
> there are other functions not being described in the binding.

name 'ethernet@18,0' is wrong, which should be fixed as 'ptp-timer@18,0'.

which is independence device. This file descript its's bindings.

Other embedded pcie devices located in ENETC submodule is descript by other
binding files.

Such as MIDO: Documentation/devicetree/bindings/net/fsl,enetc-mdio.yaml
NIC: Documentation/devicetree/bindings/net/fsl,enetc.yaml

For ptp-timer, I think it should be complete enough. Do we need descirpt
its's consumer? An example

https://lore.kernel.org/imx/20250812094634.489901-4-wei.fang@nxp.com/

+ pcie {
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ ethernet@0,0 {
+ compatible = "pci1131,e101";
+ reg = <0x000000 0 0 0 0>;
+ clocks = <&scmi_clk 102>;
+ clock-names = "ref";
+ phy-handle = <&ethphy0>;
+ phy-mode = "rgmii-id";
+ ptp-timer = <&netc_timer>;

This phandle point to ptp-timer@18,0

+ };
+ };

Frank
>
>
>
> Best regards,
> Krzysztof