[PATCH net-next 00/12] Add NETC Timer PTP driver and add PTP support for ENETC v4

[PATCH net-next 00/12] Add NETC Timer PTP driver and add PTP support for ENETC v4

[Wei Fang]

This series adds NETC Timer PTP clock driver, which supports precise
periodic pulse, time capture on external pulse and PTP synchronization.
It also adds PTP support to the enetc v4 driver, and also optimizes the
PTP-related code in the enetc driver.

F.S. Peng (1):
  ptp: netc: add external trigger stamp support

Wei Fang (11):
  dt-bindings: ptp: add bindings for NETC Timer
  ptp: netc: add NETC Timer PTP driver support
  ptp: netc: add PPS support
  ptp: netc: add periodic pulse output support
  ptp: netc: add debugfs support to loop back pulse signal
  MAINTAINERS: add NETC Timer PTP clock driver section
  net: enetc: save the parsed information of PTP packet to skb->cb
  net: enetc: Add enetc_update_ptp_sync_msg() to process PTP sync packet
  net: enetc: remove unnecessary CONFIG_FSL_ENETC_PTP_CLOCK check
  net: enetc: add PTP synchronization support for ENETC v4
  net: enetc: don't update sync packet checksum if checksum offload is
    used

 .../devicetree/bindings/ptp/nxp,ptp-netc.yaml |   67 +
 MAINTAINERS                                   |    9 +
 drivers/net/ethernet/freescale/enetc/enetc.c  |  209 +--
 drivers/net/ethernet/freescale/enetc/enetc.h  |   21 +-
 .../net/ethernet/freescale/enetc/enetc4_hw.h  |    6 +
 .../net/ethernet/freescale/enetc/enetc4_pf.c  |    3 +
 .../ethernet/freescale/enetc/enetc_ethtool.c  |   69 +-
 .../net/ethernet/freescale/enetc/enetc_hw.h   |    1 +
 drivers/ptp/Kconfig                           |   11 +
 drivers/ptp/Makefile                          |    1 +
 drivers/ptp/ptp_netc.c                        | 1209 +++++++++++++++++
 include/linux/fsl/netc_global.h               |   12 +-
 12 files changed, 1516 insertions(+), 102 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/ptp/nxp,ptp-netc.yaml
 create mode 100644 drivers/ptp/ptp_netc.c

-- 
2.34.1

[PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Wei Fang]

Add device tree binding doc for the PTP clock based on NETC Timer.

Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 .../devicetree/bindings/ptp/nxp,ptp-netc.yaml | 67 +++++++++++++++++++
 1 file changed, 67 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..b6b2e881a3c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/ptp/nxp,ptp-netc.yaml
@@ -0,0 +1,67 @@
+# 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 Timer PTP clock
+
+description:
+  NETC 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
+
+  clock-names:
+    oneOf:
+      - enum:
+          - system
+          - ccm_timer
+          - ext_1588
+
+  nxp,pps-channel:
+    $ref: /schemas/types.yaml#/definitions/uint8
+    default: 0
+    description:
+      Specifies to which fixed interval period pulse generator is
+      used to generate PPS signal.
+    enum: [0, 1, 2]
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: /schemas/pci/pci-device.yaml
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    pcie {
+        #address-cells = <3>;
+        #size-cells = <2>;
+
+        ethernet@18,0 {
+            compatible = "pci1131,ee02";
+            reg = <0x00c000 0 0 0 0>;
+            clocks = <&scmi_clk 18>;
+            clock-names = "ccm_timer";
+            nxp,pps-channel =  /bits/ 8 <1>;
+        };
+    };
-- 
2.34.1

Re: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Andrew Lunn]

> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    oneOf:
> +      - enum:
> +          - system
> +          - ccm_timer
> +          - ext_1588
> +
> +  nxp,pps-channel:
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    default: 0
> +    description:
> +      Specifies to which fixed interval period pulse generator is
> +      used to generate PPS signal.
> +    enum: [0, 1, 2]
> +
> +required:
> +  - compatible
> +  - reg

Given that clocks and clock-names are not required, please document
what happens if they are not present.

	Andrew

RE: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Wei Fang]

> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    oneOf:
> > +      - enum:
> > +          - system
> > +          - ccm_timer
> > +          - ext_1588
> > +
> > +  nxp,pps-channel:
> > +    $ref: /schemas/types.yaml#/definitions/uint8
> > +    default: 0
> > +    description:
> > +      Specifies to which fixed interval period pulse generator is
> > +      used to generate PPS signal.
> > +    enum: [0, 1, 2]
> > +
> > +required:
> > +  - compatible
> > +  - reg
> 
> Given that clocks and clock-names are not required, please document
> what happens if they are not present.
> 

Sure, will add the description.

Re: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Krzysztof Kozlowski]

On 11/07/2025 08:57, Wei Fang wrote:
> Add device tree binding doc for the PTP clock based on NETC Timer.


A nit, subject: drop second/last, redundant "bindings for". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> 
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
>  .../devicetree/bindings/ptp/nxp,ptp-netc.yaml | 67 +++++++++++++++++++
>  1 file changed, 67 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..b6b2e881a3c0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ptp/nxp,ptp-netc.yaml
> @@ -0,0 +1,67 @@
> +# 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 Timer PTP clock

What is NETC?

> +
> +description:
> +  NETC 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
> +
> +  clock-names:
> +    oneOf:

Why oneOf? Drop

> +      - enum:
> +          - system
> +          - ccm_timer
> +          - ext_1588

Why is this flexible?

> +
> +  nxp,pps-channel:
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    default: 0
> +    description:
> +      Specifies to which fixed interval period pulse generator is
> +      used to generate PPS signal.
> +    enum: [0, 1, 2]

Cell phandle tells that. Drop property.


Best regards,
Krzysztof

RE: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Wei Fang]

> On 11/07/2025 08:57, Wei Fang wrote:
> > Add device tree binding doc for the PTP clock based on NETC Timer.
> 
> 
> A nit, subject: drop second/last, redundant "bindings for". The "dt-bindings"
> prefix is already stating that these are bindings.

Okay, I will fix it, thanks

> > +
> > +title: NXP NETC Timer PTP clock
> 
> What is NETC?
> 

NETC means Ethernet Controller, it is a multi-function PCIe Root Complex
Integrated Endpoint (RCiEP), Timer is one of its PCIe functions.

> > +
> > +description:
> > +  NETC 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
> > +
> > +  clock-names:
> > +    oneOf:
> 
> Why oneOf? Drop
> 
> > +      - enum:
> > +          - system
> > +          - ccm_timer
> > +          - ext_1588
> 
> Why is this flexible?
> 

The NETC Timer has 3 reference clock sources, we need to select one
of them as the reference clock. Set TMR_CTRL[CK_SEL] by parsing the
clock name to tell the hardware which clock is currently being used.
Otherwise, we need to add another property to select the clock source.

> > +
> > +  nxp,pps-channel:
> > +    $ref: /schemas/types.yaml#/definitions/uint8
> > +    default: 0
> > +    description:
> > +      Specifies to which fixed interval period pulse generator is
> > +      used to generate PPS signal.
> > +    enum: [0, 1, 2]
> 
> Cell phandle tells that. Drop property.

Sorry, I do not understand what you mean, could you explain it in more
detail?

This property is similar to the "fsl,ptp-channel" which has been added
to fsl,fec.yaml
https://elixir.bootlin.com/linux/v6.16-rc5/source/Documentation/devicetree/bindings/net/fsl,fec.yaml#L186

The NETC Timer has three 3 PPS generators, and each corresponding
to an output pin, but these pins are multiplexed with other devices,
so the pin of the PPS output depends on the design of the board. This
property is used to tell the driver which PPS generator/pin the board
uses.

Re: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Krzysztof Kozlowski]

On 14/07/2025 09:32, Wei Fang wrote:
>> On 11/07/2025 08:57, Wei Fang wrote:
>>> Add device tree binding doc for the PTP clock based on NETC Timer.
>>
>>
>> A nit, subject: drop second/last, redundant "bindings for". The "dt-bindings"
>> prefix is already stating that these are bindings.
> 
> Okay, I will fix it, thanks
> 
>>> +
>>> +title: NXP NETC Timer PTP clock
>>
>> What is NETC?
>>
> 
> NETC means Ethernet Controller, it is a multi-function PCIe Root Complex
> Integrated Endpoint (RCiEP), Timer is one of its PCIe functions.

This must be explained in description, not here.

> 
>>> +
>>> +description:
>>> +  NETC 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
>>> +
>>> +  clock-names:
>>> +    oneOf:
>>
>> Why oneOf? Drop
>>
>>> +      - enum:
>>> +          - system
>>> +          - ccm_timer
>>> +          - ext_1588
>>
>> Why is this flexible?
>>
> 
> The NETC Timer has 3 reference clock sources, we need to select one
> of them as the reference clock. Set TMR_CTRL[CK_SEL] by parsing the
> clock name to tell the hardware which clock is currently being used.
> Otherwise, we need to add another property to select the clock source.
> 
>>> +
>>> +  nxp,pps-channel:
>>> +    $ref: /schemas/types.yaml#/definitions/uint8
>>> +    default: 0
>>> +    description:
>>> +      Specifies to which fixed interval period pulse generator is
>>> +      used to generate PPS signal.
>>> +    enum: [0, 1, 2]
>>
>> Cell phandle tells that. Drop property.
> 
> Sorry, I do not understand what you mean, could you explain it in more
> detail?

Use phandle cells for that - look at other PTP bindings.

> 
> This property is similar to the "fsl,ptp-channel" which has been added
> to fsl,fec.yaml
> https://elixir.bootlin.com/linux/v6.16-rc5/source/Documentation/devicetree/bindings/net/fsl,fec.yaml#L186

This is not FEC. And if you have existing property, you don't add
entirely different one!

> 
> The NETC Timer has three 3 PPS generators, and each corresponding
> to an output pin, but these pins are multiplexed with other devices,
> so the pin of the PPS output depends on the design of the board. This
> property is used to tell the driver which PPS generator/pin the board
> uses.


Again, phandle cells tells you that. And driver does not use it in the
first place.


Best regards,
Krzysztof

RE: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Wei Fang]

> >>> +  nxp,pps-channel:
> >>> +    $ref: /schemas/types.yaml#/definitions/uint8
> >>> +    default: 0
> >>> +    description:
> >>> +      Specifies to which fixed interval period pulse generator is
> >>> +      used to generate PPS signal.
> >>> +    enum: [0, 1, 2]
> >>
> >> Cell phandle tells that. Drop property.
> >
> > Sorry, I do not understand what you mean, could you explain it in more
> > detail?
> 
> Use phandle cells for that - look at other PTP bindings.

Sorry, I did not find a reference in other PTP bindings. If I understand
correctly, you mean I should add a specific cells property to this doc
as follows, correct?

"#nxp,pps-channel-cells":
    description: |
      Specifies to which fixed interval period pulse generator is
      used to generate PPS signal.
    $ref: /schemas/types.yaml#/definitions/uint32
    const: 1

Re: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Krzysztof Kozlowski]

On 14/07/2025 11:11, Wei Fang wrote:
>>>>> +  nxp,pps-channel:
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint8
>>>>> +    default: 0
>>>>> +    description:
>>>>> +      Specifies to which fixed interval period pulse generator is
>>>>> +      used to generate PPS signal.
>>>>> +    enum: [0, 1, 2]
>>>>
>>>> Cell phandle tells that. Drop property.
>>>
>>> Sorry, I do not understand what you mean, could you explain it in more
>>> detail?
>>
>> Use phandle cells for that - look at other PTP bindings.
> 
> Sorry, I did not find a reference in other PTP bindings. If I understand
> correctly, you mean I should add a specific cells property to this doc
> as follows, correct?
> 
> "#nxp,pps-channel-cells":
>     description: |
>       Specifies to which fixed interval period pulse generator is
>       used to generate PPS signal.
>     $ref: /schemas/types.yaml#/definitions/uint32
>     const: 1
> 


No. Is this a timestamper device? You really did not find a TXT document
describing this, in the same directory? Well, maybe it is not a
timestamper device, how do I know, your description should be clear here.

How does the other consumer - ethernet - reference this one here? Paste
complete DTS of this and users, otherwise it is just ping-pong
discussion where you put just a little effort to bounce back my question.


Best regards,
Krzysztof

RE: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Wei Fang]

> On 14/07/2025 11:11, Wei Fang wrote:
> >>>>> +  nxp,pps-channel:
> >>>>> +    $ref: /schemas/types.yaml#/definitions/uint8
> >>>>> +    default: 0
> >>>>> +    description:
> >>>>> +      Specifies to which fixed interval period pulse generator is
> >>>>> +      used to generate PPS signal.
> >>>>> +    enum: [0, 1, 2]
> >>>>
> >>>> Cell phandle tells that. Drop property.
> >>>
> >>> Sorry, I do not understand what you mean, could you explain it in more
> >>> detail?
> >>
> >> Use phandle cells for that - look at other PTP bindings.
> >
> > Sorry, I did not find a reference in other PTP bindings. If I understand
> > correctly, you mean I should add a specific cells property to this doc
> > as follows, correct?
> >
> > "#nxp,pps-channel-cells":
> >     description: |
> >       Specifies to which fixed interval period pulse generator is
> >       used to generate PPS signal.
> >     $ref: /schemas/types.yaml#/definitions/uint32
> >     const: 1
> >
> 
> 
> No. Is this a timestamper device? You really did not find a TXT document
> describing this, in the same directory? Well, maybe it is not a
> timestamper device, how do I know, your description should be clear here.

I thought I should find the reference from YAML docs. So I did not look at
the TXT docs.

> 
> How does the other consumer - ethernet - reference this one here? Paste
> complete DTS of this and users, otherwise it is just ping-pong
> discussion where you put just a little effort to bounce back my question.

Below is the DTS node of enetc (ethernet device) and timer node.

enetc_port0: ethernet@0,0 {
	compatible = "pci1131,e101";
	reg = <0x000000 0 0 0 0>;
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_enetc0>;
	phy-handle = <&ethphy0>;
	phy-mode = "rgmii-id";
	status = "okay";
};

netc_timer: ethernet@18,0 {
	compatible = "pci1131,ee02";
	reg = <0x00c000 0 0 0 0>;
	clocks = <&netc_system333m>;
	clock-names = "system";
};

Currently, the enetc driver uses the PCIe device number and function number
of the Timer to obtain the Timer device, so there is no related binding in DTS.
In the future, we plan to add phandle to the enetc document to bind enetc
and Timer, because there will be multiple Timer instances on subsequent
platforms.

But what I want to say is that "nxp,pps-channel" is used to specify which
channel/pin the NETC Timer device uses to output the Pulse Per Second
(PPS) signal. This function is independent of enetc (Ethernet device), so it
it is not used by enetc.

Re: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Krzysztof Kozlowski]

On 14/07/2025 11:56, Wei Fang wrote:
> 
>>
>> How does the other consumer - ethernet - reference this one here? Paste
>> complete DTS of this and users, otherwise it is just ping-pong
>> discussion where you put just a little effort to bounce back my question.
> 
> Below is the DTS node of enetc (ethernet device) and timer node.
> 
> enetc_port0: ethernet@0,0 {
> 	compatible = "pci1131,e101";
> 	reg = <0x000000 0 0 0 0>;
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&pinctrl_enetc0>;
> 	phy-handle = <&ethphy0>;
> 	phy-mode = "rgmii-id";
> 	status = "okay";

How do you use netc_timer in such case?

> };
> 
> netc_timer: ethernet@18,0 {
> 	compatible = "pci1131,ee02";
> 	reg = <0x00c000 0 0 0 0>;
> 	clocks = <&netc_system333m>;
> 	clock-names = "system";
> };
> 
> Currently, the enetc driver uses the PCIe device number and function number
> of the Timer to obtain the Timer device, so there is no related binding in DTS.

So you just tightly coupled these devices. Looks poor design for me, but
your choice. Anyway, then use that channel as information to pass the
pin/timer/channel number. You do not get a new property for that.

> In the future, we plan to add phandle to the enetc document to bind enetc
> and Timer, because there will be multiple Timer instances on subsequent
> platforms.

Bindings must be complete, not "in the future" but now. Start sending
complete work, so we won't have to guess it.

> 
> But what I want to say is that "nxp,pps-channel" is used to specify which

There is no user in your DTS of nxp,pps-channel.

Best regards,
Krzysztof

Re: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Krzysztof Kozlowski]

On 14/07/2025 12:09, Krzysztof Kozlowski wrote:
> On 14/07/2025 11:56, Wei Fang wrote:
>>
>>>
>>> How does the other consumer - ethernet - reference this one here? Paste
>>> complete DTS of this and users, otherwise it is just ping-pong
>>> discussion where you put just a little effort to bounce back my question.
>>
>> Below is the DTS node of enetc (ethernet device) and timer node.
>>
>> enetc_port0: ethernet@0,0 {
>> 	compatible = "pci1131,e101";
>> 	reg = <0x000000 0 0 0 0>;
>> 	pinctrl-names = "default";
>> 	pinctrl-0 = <&pinctrl_enetc0>;
>> 	phy-handle = <&ethphy0>;
>> 	phy-mode = "rgmii-id";
>> 	status = "okay";
> 
> How do you use netc_timer in such case?
> 
>> };
>>
>> netc_timer: ethernet@18,0 {
>> 	compatible = "pci1131,ee02";
>> 	reg = <0x00c000 0 0 0 0>;
>> 	clocks = <&netc_system333m>;
>> 	clock-names = "system";
>> };
>>
>> Currently, the enetc driver uses the PCIe device number and function number
>> of the Timer to obtain the Timer device, so there is no related binding in DTS.
> 
> So you just tightly coupled these devices. Looks poor design for me, but
> your choice. Anyway, then use that channel as information to pass the
> pin/timer/channel number. You do not get a new property for that.
> 
>> In the future, we plan to add phandle to the enetc document to bind enetc
>> and Timer, because there will be multiple Timer instances on subsequent
>> platforms.
> 
> Bindings must be complete, not "in the future" but now. Start sending
> complete work, so we won't have to guess it.

BTW, above DTS is not a patch. Post complete upstream DTS as patch for
review, evaluation and merging. Otherwise you will be posting
incomplete, unvalidated code which in fact does not work and shortly
after you will be bringing fix-up patches for bindings.

No. This is not how the process is working for upstreaming SoC
components from major vendor.

Best regards,
Krzysztof

RE: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Wei Fang]

> On 14/07/2025 12:09, Krzysztof Kozlowski wrote:
> > On 14/07/2025 11:56, Wei Fang wrote:
> >>
> >>>
> >>> How does the other consumer - ethernet - reference this one here? Paste
> >>> complete DTS of this and users, otherwise it is just ping-pong
> >>> discussion where you put just a little effort to bounce back my question.
> >>
> >> Below is the DTS node of enetc (ethernet device) and timer node.
> >>
> >> enetc_port0: ethernet@0,0 {
> >> 	compatible = "pci1131,e101";
> >> 	reg = <0x000000 0 0 0 0>;
> >> 	pinctrl-names = "default";
> >> 	pinctrl-0 = <&pinctrl_enetc0>;
> >> 	phy-handle = <&ethphy0>;
> >> 	phy-mode = "rgmii-id";
> >> 	status = "okay";
> >
> > How do you use netc_timer in such case?
> >
> >> };
> >>
> >> netc_timer: ethernet@18,0 {
> >> 	compatible = "pci1131,ee02";
> >> 	reg = <0x00c000 0 0 0 0>;
> >> 	clocks = <&netc_system333m>;
> >> 	clock-names = "system";
> >> };
> >>
> >> Currently, the enetc driver uses the PCIe device number and function number
> >> of the Timer to obtain the Timer device, so there is no related binding in DTS.
> >
> > So you just tightly coupled these devices. Looks poor design for me, but
> > your choice. Anyway, then use that channel as information to pass the
> > pin/timer/channel number. You do not get a new property for that.
> >
> >> In the future, we plan to add phandle to the enetc document to bind enetc
> >> and Timer, because there will be multiple Timer instances on subsequent
> >> platforms.
> >
> > Bindings must be complete, not "in the future" but now. Start sending
> > complete work, so we won't have to guess it.
> 

Okay, I will update the enetc.yaml

> BTW, above DTS is not a patch. Post complete upstream DTS as patch for
> review, evaluation and merging. Otherwise you will be posting
> incomplete, unvalidated code which in fact does not work and shortly
> after you will be bringing fix-up patches for bindings.
> 
> No. This is not how the process is working for upstreaming SoC
> components from major vendor.
> 

RE: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Wei Fang]

> >> How does the other consumer - ethernet - reference this one here? Paste
> >> complete DTS of this and users, otherwise it is just ping-pong
> >> discussion where you put just a little effort to bounce back my question.
> >
> > Below is the DTS node of enetc (ethernet device) and timer node.
> >
> > enetc_port0: ethernet@0,0 {
> > 	compatible = "pci1131,e101";
> > 	reg = <0x000000 0 0 0 0>;
> > 	pinctrl-names = "default";
> > 	pinctrl-0 = <&pinctrl_enetc0>;
> > 	phy-handle = <&ethphy0>;
> > 	phy-mode = "rgmii-id";
> > 	status = "okay";
> 
> How do you use netc_timer in such case?
> 
> > };
> >
> > netc_timer: ethernet@18,0 {
> > 	compatible = "pci1131,ee02";
> > 	reg = <0x00c000 0 0 0 0>;
> > 	clocks = <&netc_system333m>;
> > 	clock-names = "system";
> > };
> >
> > Currently, the enetc driver uses the PCIe device number and function number
> > of the Timer to obtain the Timer device, so there is no related binding in DTS.
> 
> So you just tightly coupled these devices. Looks poor design for me, but
> your choice. Anyway, then use that channel as information to pass the
> pin/timer/channel number. You do not get a new property for that.
> 

I do not understand, the property is to indicate which pin the board is
used to out PPS signal, as I said earlier, these pins are multiplexed with
other devices, so different board design may use different pins to out
this PPS signal.

The PPS interface (echo x > /sys/class/ptp/ptp0/pps_enable) provided
by the current PTP framework only supports enabling or disabling the
PPS signal. This is obviously limited for PTP devices with multiple channels.

> > In the future, we plan to add phandle to the enetc document to bind enetc
> > and Timer, because there will be multiple Timer instances on subsequent
> > platforms.
> 
> Bindings must be complete, not "in the future" but now. Start sending
> complete work, so we won't have to guess it.
> 
> >
> > But what I want to say is that "nxp,pps-channel" is used to specify which
> 
> There is no user in your DTS of nxp,pps-channel.
> 
The user is the PTP device itself.

Re: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Vladimir Oltean]

On Mon, Jul 14, 2025 at 01:28:04PM +0300, Wei Fang wrote:
> I do not understand, the property is to indicate which pin the board is
> used to out PPS signal, as I said earlier, these pins are multiplexed with
> other devices, so different board design may use different pins to out
> this PPS signal.

Did you look at the 'pins' API in ptp, as used by other drivers, to set
a function per pin?

> The PPS interface (echo x > /sys/class/ptp/ptp0/pps_enable) provided
> by the current PTP framework only supports enabling or disabling the
> PPS signal. This is obviously limited for PTP devices with multiple channels.

For what we call "PPS" I think you should be looking at the periodic
output (perout) function. "PPS" is to emit events towards the local
system.

RE: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Wei Fang]

> On Mon, Jul 14, 2025 at 01:28:04PM +0300, Wei Fang wrote:
> > I do not understand, the property is to indicate which pin the board is
> > used to out PPS signal, as I said earlier, these pins are multiplexed with
> > other devices, so different board design may use different pins to out
> > this PPS signal.
> 
> Did you look at the 'pins' API in ptp, as used by other drivers, to set
> a function per pin?

ptp_set_pinfunc()?

> 
> > The PPS interface (echo x > /sys/class/ptp/ptp0/pps_enable) provided
> > by the current PTP framework only supports enabling or disabling the
> > PPS signal. This is obviously limited for PTP devices with multiple channels.
> 
> For what we call "PPS" I think you should be looking at the periodic
> output (perout) function. "PPS" is to emit events towards the local
> system.

The driver supports both PPS and PEROUT.

Re: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Vladimir Oltean]

On Mon, Jul 14, 2025 at 01:43:49PM +0300, Wei Fang wrote:
> > On Mon, Jul 14, 2025 at 01:28:04PM +0300, Wei Fang wrote:
> > > I do not understand, the property is to indicate which pin the board is
> > > used to out PPS signal, as I said earlier, these pins are multiplexed with
> > > other devices, so different board design may use different pins to out
> > > this PPS signal.
> > 
> > Did you look at the 'pins' API in ptp, as used by other drivers, to set
> > a function per pin?
> 
> ptp_set_pinfunc()?

You're in the right area, but ptp_set_pinfunc() is an internal function.
I was specifically referring to struct ptp_clock_info :: pin_config, the
verify() function, etc.

> > > The PPS interface (echo x > /sys/class/ptp/ptp0/pps_enable) provided
> > > by the current PTP framework only supports enabling or disabling the
> > > PPS signal. This is obviously limited for PTP devices with multiple channels.
> > 
> > For what we call "PPS" I think you should be looking at the periodic
> > output (perout) function. "PPS" is to emit events towards the local
> > system.
> 
> The driver supports both PPS and PEROUT.

Ok, I noticed patch 3 but missed patch 4. Anyway, the role of
PTP_CLK_REQ_PPS is to emit events which can be monitored on the
/dev/ppsN char device. It shouldn't have anything to do with external
pins.

RE: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Wei Fang]

> On Mon, Jul 14, 2025 at 01:43:49PM +0300, Wei Fang wrote:
> > > On Mon, Jul 14, 2025 at 01:28:04PM +0300, Wei Fang wrote:
> > > > I do not understand, the property is to indicate which pin the board is
> > > > used to out PPS signal, as I said earlier, these pins are multiplexed with
> > > > other devices, so different board design may use different pins to out
> > > > this PPS signal.
> > >
> > > Did you look at the 'pins' API in ptp, as used by other drivers, to set
> > > a function per pin?
> >
> > ptp_set_pinfunc()?
> 
> You're in the right area, but ptp_set_pinfunc() is an internal function.
> I was specifically referring to struct ptp_clock_info :: pin_config, the
> verify() function, etc.

I don't think these can meet customer's requirement, the PPS pin depends
on the board design. If I understand correctly, these can only indicate
whether the specified pin index is in range, or whether the pin is already
occupied by another PTP function.

However, these pins are multiplexed with other devices, such as FLEXIO,
CAN, etc. If the board is designed to assign this pin to other devices, then
this pin cannot output the PPS signal. For for this use case, we need to
specify a PPS pin which can output PPS signal.

> 
> > > > The PPS interface (echo x > /sys/class/ptp/ptp0/pps_enable) provided
> > > > by the current PTP framework only supports enabling or disabling the
> > > > PPS signal. This is obviously limited for PTP devices with multiple
> channels.
> > >
> > > For what we call "PPS" I think you should be looking at the periodic
> > > output (perout) function. "PPS" is to emit events towards the local
> > > system.
> >
> > The driver supports both PPS and PEROUT.
> 
> Ok, I noticed patch 3 but missed patch 4. Anyway, the role of
> PTP_CLK_REQ_PPS is to emit events which can be monitored on the
> /dev/ppsN char device. It shouldn't have anything to do with external
> pins.

Is there a doc to stating that PTP_CLK_REQ_PPS is not used for external
pins? As far as I know, some customers/users use PTP_CLK_REQ_PPS to
output PPS signal to sync with other timer devices, for example, a similar
property "fsl,pps-channel" was added to fec.yaml by others before.

Re: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Vladimir Oltean]

On Mon, Jul 14, 2025 at 04:22:51PM +0300, Wei Fang wrote:
> > On Mon, Jul 14, 2025 at 01:43:49PM +0300, Wei Fang wrote:
> > > > On Mon, Jul 14, 2025 at 01:28:04PM +0300, Wei Fang wrote:
> > > > > I do not understand, the property is to indicate which pin the board is
> > > > > used to out PPS signal, as I said earlier, these pins are multiplexed with
> > > > > other devices, so different board design may use different pins to out
> > > > > this PPS signal.
> > > >
> > > > Did you look at the 'pins' API in ptp, as used by other drivers, to set
> > > > a function per pin?
> > >
> > > ptp_set_pinfunc()?
> > 
> > You're in the right area, but ptp_set_pinfunc() is an internal function.
> > I was specifically referring to struct ptp_clock_info :: pin_config, the
> > verify() function, etc.
> 
> I don't think these can meet customer's requirement, the PPS pin depends
> on the board design. If I understand correctly, these can only indicate
> whether the specified pin index is in range, or whether the pin is already
> occupied by another PTP function.
> 
> However, these pins are multiplexed with other devices, such as FLEXIO,
> CAN, etc. If the board is designed to assign this pin to other devices, then
> this pin cannot output the PPS signal. For for this use case, we need to
> specify a PPS pin which can output PPS signal.

Ok, apologies if I misunderstood the purpose of this device tree property
as affecting the function of the NETC 1588 timer IP pins. You gave me
this impression because I followed the code and I saw that "nxp,pps-channel"
is used to select in the PTP driver which FIPER block gets configured to
emit PPS. And I commented that maybe you don't need "nxp,pps-channel" at all,
because:
- PTP_CLK_REQ_PPS doesn't do what you think it does
- PTP_CLK_REQ_PEROUT does use the pin API to describe that one of the
  1588 timer block's pins can be used for the periodic output function

You seem to imply that the "nxp,pps-channel" property affects the
function of the SoC pads, which may be connected to the NETC 1588 timer
block or to some other IP. Nothing in the code I saw suggested this
would be the case, and I still don't see how this is the case - but
anyway, my bad.

In this case, echoing Krzysztof's comments: How come it isn't the system
pinmux driver the one concerned with connecting the SoC pads to the NETC
1588 timer or to FlexIO, CAN etc? The pinmux driver controls the pads,
the NETC timer controls its block's pins, regardless of how they are
routed further.

(reductio ad absurdum) Among the other devices which are muxed to these
SoC pads, why is the NETC 1588 timer responsible of controlling the
muxing, and not FlexIO or CAN? Something is illogical.

> > > > > The PPS interface (echo x > /sys/class/ptp/ptp0/pps_enable) provided
> > > > > by the current PTP framework only supports enabling or disabling the
> > > > > PPS signal. This is obviously limited for PTP devices with multiple
> > channels.
> > > >
> > > > For what we call "PPS" I think you should be looking at the periodic
> > > > output (perout) function. "PPS" is to emit events towards the local
> > > > system.
> > >
> > > The driver supports both PPS and PEROUT.
> > 
> > Ok, I noticed patch 3 but missed patch 4. Anyway, the role of
> > PTP_CLK_REQ_PPS is to emit events which can be monitored on the
> > /dev/ppsN char device. It shouldn't have anything to do with external
> > pins.
> 
> Is there a doc to stating that PTP_CLK_REQ_PPS is not used for external
> pins? As far as I know, some customers/users use PTP_CLK_REQ_PPS to
> output PPS signal to sync with other timer devices, for example, a similar
> property "fsl,pps-channel" was added to fec.yaml by others before.

I'm not sure that what you're asking for is realistic, i.e. an explicit
mention in the documentation that PTP_CLK_REQ_PPS does not affect the
state of external pins. There's an infinity of things that PTP_CLK_REQ_PPS
doesn't affect, it doesn't mean they will all be explicitly documented.

That being said, here's what Documentation/ABI/testing/sysfs-ptp says,
hence my interpretation that they're unrelated:

 97 What:»  »       /sys/class/ptp/ptp<N>/pps_available
 98 Date:»  »       September 2010
 99 Contact:»       Richard Cochran <richardcochran@gmail.com>
100 Description:
101 »       »       This file indicates whether the PTP hardware clock
102 »       »       supports a Pulse Per Second to the host CPU. Reading
103 »       »       "1" means that the PPS is supported, while "0" means
104 »       »       not supported.

135 What:»  »       /sys/class/ptp/ptp<N>/pps_enable
136 Date:»  »       September 2010
137 Contact:»       Richard Cochran <richardcochran@gmail.com>
138 Description:
139 »       »       This write-only file enables or disables delivery of
140 »       »       PPS events to the Linux PPS subsystem. To enable PPS
141 »       »       events, write a "1" into the file. To disable events,
142 »       »       write a "0" into the file.

Search for which user space programs interact with /dev/ppsN and you'll
see chrony, ntpd, even phc2sys. None of those consciously affects the
state of external pins when they use the Linux PPS API
(https://docs.kernel.org/driver-api/pps.html).

Maybe the PTP maintainer can give you an explicit ACK that PPS is not
supposed to affect pin states, if that helps.

As usual, bad practice in old drivers does not justify bad practice in
new drivers.

Re: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Vladimir Oltean]

On Mon, Jul 14, 2025 at 04:56:41PM +0300, Vladimir Oltean wrote:
> On Mon, Jul 14, 2025 at 04:22:51PM +0300, Wei Fang wrote:
> > > On Mon, Jul 14, 2025 at 01:43:49PM +0300, Wei Fang wrote:
> > > > > On Mon, Jul 14, 2025 at 01:28:04PM +0300, Wei Fang wrote:
> > > > > > I do not understand, the property is to indicate which pin the board is
> > > > > > used to out PPS signal, as I said earlier, these pins are multiplexed with
> > > > > > other devices, so different board design may use different pins to out
> > > > > > this PPS signal.
> > > > >
> > > > > Did you look at the 'pins' API in ptp, as used by other drivers, to set
> > > > > a function per pin?
> > > >
> > > > ptp_set_pinfunc()?
> > > 
> > > You're in the right area, but ptp_set_pinfunc() is an internal function.
> > > I was specifically referring to struct ptp_clock_info :: pin_config, the
> > > verify() function, etc.
> > 
> > I don't think these can meet customer's requirement, the PPS pin depends
> > on the board design. If I understand correctly, these can only indicate
> > whether the specified pin index is in range, or whether the pin is already
> > occupied by another PTP function.
> > 
> > However, these pins are multiplexed with other devices, such as FLEXIO,
> > CAN, etc. If the board is designed to assign this pin to other devices, then
> > this pin cannot output the PPS signal. For for this use case, we need to
> > specify a PPS pin which can output PPS signal.
> 
> Ok, apologies if I misunderstood the purpose of this device tree property
> as affecting the function of the NETC 1588 timer IP pins. You gave me
> this impression because I followed the code and I saw that "nxp,pps-channel"
> is used to select in the PTP driver which FIPER block gets configured to
> emit PPS. And I commented that maybe you don't need "nxp,pps-channel" at all,
> because:
> - PTP_CLK_REQ_PPS doesn't do what you think it does
> - PTP_CLK_REQ_PEROUT does use the pin API to describe that one of the
>   1588 timer block's pins can be used for the periodic output function

Just to expand on the last point, maybe it's not clear enough.

This is how the Felix/Ocelot switch presents itself on LS1028A. There
are four pins with unspecified functions at boot time.

$ ls -la /sys/class/ptp/ptp1/pins/
total 0
drwxr-xr-x 2 root root    0 Mar  6 16:56 .
drwxr-xr-x 4 root root    0 Mar  6 16:56 ..
-rw-r--r-- 1 root root 4096 Mar  6 16:56 switch_1588_dat0
-rw-r--r-- 1 root root 4096 Mar  6 16:56 switch_1588_dat1
-rw-r--r-- 1 root root 4096 Mar  6 16:56 switch_1588_dat2
-rw-r--r-- 1 root root 4096 Mar  6 16:56 switch_1588_dat3

And this is how we configure one of the pins (here, 1588_dat0) for
periodic output:

# enum ptp_pin_function
PTP_PF_NONE=0
PTP_PF_EXTTS=1
PTP_PF_PEROUT=2
PTP_PF_PHYSYNC=2

PTP1_CHANNEL0=0
PTP1_PEROUT_START="0 0"
PTP1_PEROUT_PERIOD="1 1"

PTP2_CHANNEL0=0

PTP3_CHANNEL0=0

echo "$PTP_PF_PEROUT $PTP1_CHANNEL0" > /sys/class/ptp/ptp1/pins/switch_1588_dat0
echo "$PTP1_CHANNEL0 $PTP1_PEROUT_START $PTP1_PEROUT_PERIOD" > /sys/class/ptp/ptp1/period

In the case above, it still depends upon system pinmuxing (on LS1028A,
that is handled by the RCW) how/whether switch_1588_dat0 is connected to
an SoC pad or not.

I was just saying that _if_ you were to eliminate the use of PTP_CLK_REQ_PPS
from your driver, _then_ the pins API should be sufficiently flexible
for the user to select which pin is configured for periodic output.

RE: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Wei Fang]

> > I don't think these can meet customer's requirement, the PPS pin depends
> > on the board design. If I understand correctly, these can only indicate
> > whether the specified pin index is in range, or whether the pin is already
> > occupied by another PTP function.
> >
> > However, these pins are multiplexed with other devices, such as FLEXIO,
> > CAN, etc. If the board is designed to assign this pin to other devices, then
> > this pin cannot output the PPS signal. For for this use case, we need to
> > specify a PPS pin which can output PPS signal.
> 
> Ok, apologies if I misunderstood the purpose of this device tree property
> as affecting the function of the NETC 1588 timer IP pins. You gave me
> this impression because I followed the code and I saw that "nxp,pps-channel"
> is used to select in the PTP driver which FIPER block gets configured to
> emit PPS. And I commented that maybe you don't need "nxp,pps-channel" at all,
> because:
> - PTP_CLK_REQ_PPS doesn't do what you think it does
> - PTP_CLK_REQ_PEROUT does use the pin API to describe that one of the
>   1588 timer block's pins can be used for the periodic output function
> 

Yes, you are right, PTP_CLK_REQ_PPS is input into the kernel PPS subsystem,
is not used to out PPS signal on external pins. I have found the kernel doc.
I thought PTP_CLK_REQ_PPS can be used to output PPS signal, but now it
seems that many people use it wrongly.

commit d04a53b1c48766665806eb75b73137734abdaa95
Author: Ahmad Fatoum <a.fatoum@pengutronix.de>
Date:   Tue Nov 17 22:38:26 2020 +0100

ptp: document struct ptp_clock_request members

It's arguable most people interested in configuring a PPS signal
want it as external output, not as kernel input. PTP_CLK_REQ_PPS
is for input though. Add documentation to nudge readers into
the correct direction.

> You seem to imply that the "nxp,pps-channel" property affects the
> function of the SoC pads, which may be connected to the NETC 1588 timer
> block or to some other IP. Nothing in the code I saw suggested this
> would be the case, and I still don't see how this is the case - but
> anyway, my bad.
> 
> In this case, echoing Krzysztof's comments: How come it isn't the system
> pinmux driver the one concerned with connecting the SoC pads to the NETC
> 1588 timer or to FlexIO, CAN etc? The pinmux driver controls the pads,
> the NETC timer controls its block's pins, regardless of how they are
> routed further.
> 

pinmux can select which device to use this pin for, but this is the
configuration inside SoC. For the outside, depending on the design
of the board, for example, pin0 is connected to a CAN-related device,
then in fact this pin can only be used by CAN.

> (reductio ad absurdum) Among the other devices which are muxed to these
> SoC pads, why is the NETC 1588 timer responsible of controlling the
> muxing, and not FlexIO or CAN? Something is illogical.

Re: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Vladimir Oltean]

On Tue, Jul 15, 2025 at 05:52:33AM +0300, Wei Fang wrote:
> > You seem to imply that the "nxp,pps-channel" property affects the
> > function of the SoC pads, which may be connected to the NETC 1588 timer
> > block or to some other IP. Nothing in the code I saw suggested this
> > would be the case, and I still don't see how this is the case - but
> > anyway, my bad.
> > 
> > In this case, echoing Krzysztof's comments: How come it isn't the system
> > pinmux driver the one concerned with connecting the SoC pads to the NETC
> > 1588 timer or to FlexIO, CAN etc? The pinmux driver controls the pads,
> > the NETC timer controls its block's pins, regardless of how they are
> > routed further.
> 
> pinmux can select which device to use this pin for, but this is the
> configuration inside SoC. For the outside, depending on the design
> of the board, for example, pin0 is connected to a CAN-related device,
> then in fact this pin can only be used by CAN.

Ok, but I fail to see the relevance here? Do you just mean to say 'there
are multiple FIPER outputs from the 1588 timer block, and they may not
all be pinmuxed to SoC pads, so I intended "nxp,pps-channel" as a way
for the device tree writer to select one of the FIPER outputs which is
multiplexed to an SoC pad, rather than arbitrarily choosing one of the
FIPER channels in the driver'? If so, I believe the case is settled and
this property will disappear in v2.

RE: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Wei Fang]

> On Tue, Jul 15, 2025 at 05:52:33AM +0300, Wei Fang wrote:
> > > You seem to imply that the "nxp,pps-channel" property affects the
> > > function of the SoC pads, which may be connected to the NETC 1588 timer
> > > block or to some other IP. Nothing in the code I saw suggested this
> > > would be the case, and I still don't see how this is the case - but
> > > anyway, my bad.
> > >
> > > In this case, echoing Krzysztof's comments: How come it isn't the system
> > > pinmux driver the one concerned with connecting the SoC pads to the NETC
> > > 1588 timer or to FlexIO, CAN etc? The pinmux driver controls the pads,
> > > the NETC timer controls its block's pins, regardless of how they are
> > > routed further.
> >
> > pinmux can select which device to use this pin for, but this is the
> > configuration inside SoC. For the outside, depending on the design
> > of the board, for example, pin0 is connected to a CAN-related device,
> > then in fact this pin can only be used by CAN.
> 
> Ok, but I fail to see the relevance here? Do you just mean to say 'there
> are multiple FIPER outputs from the 1588 timer block, and they may not
> all be pinmuxed to SoC pads, so I intended "nxp,pps-channel" as a way
> for the device tree writer to select one of the FIPER outputs which is
> multiplexed to an SoC pad, rather than arbitrarily choosing one of the
> FIPER channels in the driver'? If so, I believe the case is settled and
> this property will disappear in v2.

Yeah, now I'm clear that PTP_CLK_REQ_PPS is not used to output PPS
to external pins, so this property will be dropped.

Re: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Krzysztof Kozlowski]

On 14/07/2025 12:28, Wei Fang wrote:
>>>
>>> Currently, the enetc driver uses the PCIe device number and function number
>>> of the Timer to obtain the Timer device, so there is no related binding in DTS.
>>
>> So you just tightly coupled these devices. Looks poor design for me, but
>> your choice. Anyway, then use that channel as information to pass the
>> pin/timer/channel number. You do not get a new property for that.
>>
> 
> I do not understand, the property is to indicate which pin the board is
> used to out PPS signal, as I said earlier, these pins are multiplexed with

Sure, I get it and my argument for phandle cells stays. If you decide
not to use it, you do not get a new property.

> other devices, so different board design may use different pins to out
> this PPS signal.

multiplexing is task for pinctrl, so this only brings confusion.


Best regards,
Krzysztof

RE: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Wei Fang]

> On 14/07/2025 12:28, Wei Fang wrote:
> >>>
> >>> Currently, the enetc driver uses the PCIe device number and function
> number
> >>> of the Timer to obtain the Timer device, so there is no related binding in
> DTS.
> >>
> >> So you just tightly coupled these devices. Looks poor design for me, but
> >> your choice. Anyway, then use that channel as information to pass the
> >> pin/timer/channel number. You do not get a new property for that.
> >>
> >
> > I do not understand, the property is to indicate which pin the board is
> > used to out PPS signal, as I said earlier, these pins are multiplexed with
> 
> Sure, I get it and my argument for phandle cells stays. If you decide
> not to use it, you do not get a new property.

I do not know how to add the phandle cells, a phandle value is a way to
reference another node in the DTS. But for the NETC Timer, what node
does it need to reference? To be honest, I have no idea.

Re: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC Timer

[Krzysztof Kozlowski]

On 14/07/2025 15:43, Wei Fang wrote:
>> On 14/07/2025 12:28, Wei Fang wrote:
>>>>>
>>>>> Currently, the enetc driver uses the PCIe device number and function
>> number
>>>>> of the Timer to obtain the Timer device, so there is no related binding in
>> DTS.
>>>>
>>>> So you just tightly coupled these devices. Looks poor design for me, but
>>>> your choice. Anyway, then use that channel as information to pass the
>>>> pin/timer/channel number. You do not get a new property for that.
>>>>
>>>
>>> I do not understand, the property is to indicate which pin the board is
>>> used to out PPS signal, as I said earlier, these pins are multiplexed with
>>
>> Sure, I get it and my argument for phandle cells stays. If you decide
>> not to use it, you do not get a new property.
> 
> I do not know how to add the phandle cells, a phandle value is a way to
> reference another node in the DTS. But for the NETC Timer, what node
> does it need to reference? To be honest, I have no idea.


I don't think you tried hard enough... I asked to read other bindings.
If you did that, you would notice timestamper property and argument.


Best regards,
Krzysztof

[PATCH net-next 02/12] ptp: netc: add NETC Timer PTP driver support

[Wei Fang]

NETC 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. The enetc v4 driver can implement PTP
synchronization through the relevant interfaces provided by the driver.
Note that the current driver does not support PEROUT, PPS and EXTTS yet,
and support will be added one by one in subsequent patches.

Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/ptp/Kconfig             |  11 +
 drivers/ptp/Makefile            |   1 +
 drivers/ptp/ptp_netc.c          | 565 ++++++++++++++++++++++++++++++++
 include/linux/fsl/netc_global.h |  12 +-
 4 files changed, 588 insertions(+), 1 deletion(-)
 create mode 100644 drivers/ptp/ptp_netc.c

diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index 204278eb215e..92eb2ff41180 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -252,4 +252,15 @@ config PTP_S390
 	  driver provides the raw clock value without the delta to
 	  userspace. That way userspace programs like chrony could steer
 	  the kernel clock.
+
+config PTP_1588_CLOCK_NETC
+	bool "NXP NETC Timer PTP Driver"
+	depends on PTP_1588_CLOCK
+	depends on PCI_MSI
+	help
+	  This driver adds support for using the NXP NETC Timer as a PTP
+	  clock. This clock is used by ENETC MAC or NETC Switch for PTP
+	  synchronization. It also supports periodic output signal (e.g.
+	  PPS) and external trigger timestamping.
+
 endmenu
diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
index 25f846fe48c9..d48fe4009fa4 100644
--- a/drivers/ptp/Makefile
+++ b/drivers/ptp/Makefile
@@ -23,3 +23,4 @@ obj-$(CONFIG_PTP_1588_CLOCK_VMW)	+= ptp_vmw.o
 obj-$(CONFIG_PTP_1588_CLOCK_OCP)	+= ptp_ocp.o
 obj-$(CONFIG_PTP_DFL_TOD)		+= ptp_dfl_tod.o
 obj-$(CONFIG_PTP_S390)			+= ptp_s390.o
+obj-$(CONFIG_PTP_1588_CLOCK_NETC)	+= ptp_netc.o
diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
new file mode 100644
index 000000000000..87d456fcadfd
--- /dev/null
+++ b/drivers/ptp/ptp_netc.c
@@ -0,0 +1,565 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
+/*
+ * NXP NETC Timer driver
+ * Copyright 2025 NXP
+ */
+
+#include <linux/clk.h>
+#include <linux/fsl/netc_global.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/ptp_clock_kernel.h>
+
+#define NETC_TMR_PCI_VENDOR		0x1131
+#define NETC_TMR_PCI_DEVID		0xee02
+
+#define NETC_TMR_CTRL			0x0080
+#define  TMR_CTRL_CK_SEL		GENMASK(1, 0)
+#define  TMR_CTRL_TE			BIT(2)
+#define  TMR_COMP_MODE			BIT(15)
+#define  TMR_CTRL_TCLK_PERIOD		GENMASK(25, 16)
+#define  TMR_CTRL_FS			BIT(28)
+#define  TMR_ALARM1P			BIT(31)
+
+#define NETC_TMR_TEVENT			0x0084
+#define  TMR_TEVENT_ALM1EN		BIT(16)
+#define  TMR_TEVENT_ALM2EN		BIT(17)
+
+#define NETC_TMR_TEMASK			0x0088
+#define NETC_TMR_CNT_L			0x0098
+#define NETC_TMR_CNT_H			0x009c
+#define NETC_TMR_ADD			0x00a0
+#define NETC_TMR_PRSC			0x00a8
+#define NETC_TMR_OFF_L			0x00b0
+#define NETC_TMR_OFF_H			0x00b4
+
+/* i = 0, 1, i indicates the index of TMR_ALARM */
+#define NETC_TMR_ALARM_L(i)		(0x00b8 + (i) * 8)
+#define NETC_TMR_ALARM_H(i)		(0x00bc + (i) * 8)
+
+#define NETC_TMR_FIPER_CTRL		0x00dc
+#define  FIPER_CTRL_DIS(i)		(BIT(7) << (i) * 8)
+#define  FIPER_CTRL_PG(i)		(BIT(6) << (i) * 8)
+
+#define NETC_TMR_CUR_TIME_L		0x00f0
+#define NETC_TMR_CUR_TIME_H		0x00f4
+
+#define NETC_TMR_REGS_BAR		0
+
+#define NETC_TMR_FIPER_NUM		3
+#define NETC_TMR_DEFAULT_PRSC		2
+#define NETC_TMR_DEFAULT_ALARM		GENMASK_ULL(63, 0)
+
+/* 1588 timer reference clock source select */
+#define NETC_TMR_CCM_TIMER1		0 /* enet_timer1_clk_root, from CCM */
+#define NETC_TMR_SYSTEM_CLK		1 /* enet_clk_root/2, from CCM */
+#define NETC_TMR_EXT_OSC		2 /* tmr_1588_clk, from IO pins */
+
+#define NETC_TMR_SYSCLK_333M		333333333U
+
+struct netc_timer {
+	void __iomem *base;
+	struct pci_dev *pdev;
+	spinlock_t lock; /* Prevent concurrent access to registers */
+
+	struct clk *src_clk;
+	struct ptp_clock *clock;
+	struct ptp_clock_info caps;
+	int phc_index;
+	u32 clk_select;
+	u32 clk_freq;
+	u32 oclk_prsc;
+	/* High 32-bit is integer part, low 32-bit is fractional part */
+	u64 period;
+
+	int irq;
+};
+
+#define netc_timer_rd(p, o)		netc_read((p)->base + (o))
+#define netc_timer_wr(p, o, v)		netc_write((p)->base + (o), v)
+#define ptp_to_netc_timer(ptp)		container_of((ptp), struct netc_timer, caps)
+
+static u64 netc_timer_cnt_read(struct netc_timer *priv)
+{
+	u32 tmr_cnt_l, tmr_cnt_h;
+	u64 ns;
+
+	/* The user must read the TMR_CNC_L register first to get
+	 * correct 64-bit TMR_CNT_H/L counter values.
+	 */
+	tmr_cnt_l = netc_timer_rd(priv, NETC_TMR_CNT_L);
+	tmr_cnt_h = netc_timer_rd(priv, NETC_TMR_CNT_H);
+	ns = (((u64)tmr_cnt_h) << 32) | tmr_cnt_l;
+
+	return ns;
+}
+
+static void netc_timer_cnt_write(struct netc_timer *priv, u64 ns)
+{
+	u32 tmr_cnt_h = upper_32_bits(ns);
+	u32 tmr_cnt_l = lower_32_bits(ns);
+
+	/* The user must write to TMR_CNT_L register first. */
+	netc_timer_wr(priv, NETC_TMR_CNT_L, tmr_cnt_l);
+	netc_timer_wr(priv, NETC_TMR_CNT_H, tmr_cnt_h);
+}
+
+static u64 netc_timer_offset_read(struct netc_timer *priv)
+{
+	u32 tmr_off_l, tmr_off_h;
+	u64 offset;
+
+	tmr_off_l = netc_timer_rd(priv, NETC_TMR_OFF_L);
+	tmr_off_h = netc_timer_rd(priv, NETC_TMR_OFF_H);
+	offset = (((u64)tmr_off_h) << 32) | tmr_off_l;
+
+	return offset;
+}
+
+static void netc_timer_offset_write(struct netc_timer *priv, u64 offset)
+{
+	u32 tmr_off_h = upper_32_bits(offset);
+	u32 tmr_off_l = lower_32_bits(offset);
+
+	netc_timer_wr(priv, NETC_TMR_OFF_L, tmr_off_l);
+	netc_timer_wr(priv, NETC_TMR_OFF_H, tmr_off_h);
+}
+
+static u64 netc_timer_cur_time_read(struct netc_timer *priv)
+{
+	u32 time_h, time_l;
+	u64 ns;
+
+	time_l = netc_timer_rd(priv, NETC_TMR_CUR_TIME_L);
+	time_h = netc_timer_rd(priv, NETC_TMR_CUR_TIME_H);
+	ns = (u64)time_h << 32 | time_l;
+
+	return ns;
+}
+
+static void netc_timer_alarm_write(struct netc_timer *priv,
+				   u64 alarm, int index)
+{
+	u32 alarm_h = upper_32_bits(alarm);
+	u32 alarm_l = lower_32_bits(alarm);
+
+	netc_timer_wr(priv, NETC_TMR_ALARM_L(index), alarm_l);
+	netc_timer_wr(priv, NETC_TMR_ALARM_H(index), alarm_h);
+}
+
+static void netc_timer_adjust_period(struct netc_timer *priv, u64 period)
+{
+	u32 fractional_period = lower_32_bits(period);
+	u32 integral_period = upper_32_bits(period);
+	u32 tmr_ctrl, old_tmr_ctrl;
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	old_tmr_ctrl = netc_timer_rd(priv, NETC_TMR_CTRL);
+	tmr_ctrl = u32_replace_bits(old_tmr_ctrl, integral_period,
+				    TMR_CTRL_TCLK_PERIOD);
+	if (tmr_ctrl != old_tmr_ctrl)
+		netc_timer_wr(priv, NETC_TMR_CTRL, tmr_ctrl);
+
+	netc_timer_wr(priv, NETC_TMR_ADD, fractional_period);
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+}
+
+static int netc_timer_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
+{
+	struct netc_timer *priv = ptp_to_netc_timer(ptp);
+	u64 new_period;
+
+	if (!scaled_ppm)
+		return 0;
+
+	new_period = adjust_by_scaled_ppm(priv->period, scaled_ppm);
+	netc_timer_adjust_period(priv, new_period);
+
+	return 0;
+}
+
+static int netc_timer_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+	struct netc_timer *priv = ptp_to_netc_timer(ptp);
+	u64 tmr_cnt, tmr_off;
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	tmr_off = netc_timer_offset_read(priv);
+	if (delta < 0 && tmr_off < abs(delta)) {
+		delta += tmr_off;
+		if (!tmr_off)
+			netc_timer_offset_write(priv, 0);
+
+		tmr_cnt = netc_timer_cnt_read(priv);
+		tmr_cnt += delta;
+		netc_timer_cnt_write(priv, tmr_cnt);
+	} else {
+		tmr_off += delta;
+		netc_timer_offset_write(priv, tmr_off);
+	}
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+static int netc_timer_gettimex64(struct ptp_clock_info *ptp,
+				 struct timespec64 *ts,
+				 struct ptp_system_timestamp *sts)
+{
+	struct netc_timer *priv = ptp_to_netc_timer(ptp);
+	unsigned long flags;
+	u64 ns;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	ptp_read_system_prets(sts);
+	ns = netc_timer_cur_time_read(priv);
+	ptp_read_system_postts(sts);
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	*ts = ns_to_timespec64(ns);
+
+	return 0;
+}
+
+static int netc_timer_settime64(struct ptp_clock_info *ptp,
+				const struct timespec64 *ts)
+{
+	struct netc_timer *priv = ptp_to_netc_timer(ptp);
+	u64 ns = timespec64_to_ns(ts);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	netc_timer_offset_write(priv, 0);
+	netc_timer_cnt_write(priv, ns);
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+int netc_timer_get_phc_index(struct pci_dev *timer_pdev)
+{
+	struct netc_timer *priv;
+
+	if (!timer_pdev)
+		return -ENODEV;
+
+	priv = pci_get_drvdata(timer_pdev);
+	if (!priv)
+		return -EINVAL;
+
+	return priv->phc_index;
+}
+EXPORT_SYMBOL_GPL(netc_timer_get_phc_index);
+
+static const struct ptp_clock_info netc_timer_ptp_caps = {
+	.owner		= THIS_MODULE,
+	.name		= "NETC Timer PTP clock",
+	.max_adj	= 500000000,
+	.n_alarm	= 2,
+	.n_pins		= 0,
+	.adjfine	= netc_timer_adjfine,
+	.adjtime	= netc_timer_adjtime,
+	.gettimex64	= netc_timer_gettimex64,
+	.settime64	= netc_timer_settime64,
+};
+
+static void netc_timer_init(struct netc_timer *priv)
+{
+	u32 tmr_emask = TMR_TEVENT_ALM1EN | TMR_TEVENT_ALM2EN;
+	u32 fractional_period = lower_32_bits(priv->period);
+	u32 integral_period = upper_32_bits(priv->period);
+	u32 tmr_ctrl, fiper_ctrl;
+	struct timespec64 now;
+	u64 ns;
+	int i;
+
+	/* Software must enable timer first and the clock selected must be
+	 * active, otherwise, the registers which are in the timer clock
+	 * domain are not accessible.
+	 */
+	tmr_ctrl = (priv->clk_select & TMR_CTRL_CK_SEL) | TMR_CTRL_TE;
+	netc_timer_wr(priv, NETC_TMR_CTRL, tmr_ctrl);
+	netc_timer_wr(priv, NETC_TMR_PRSC, priv->oclk_prsc);
+
+	/* Disable FIPER by default */
+	fiper_ctrl = netc_timer_rd(priv, NETC_TMR_FIPER_CTRL);
+	for (i = 0; i < NETC_TMR_FIPER_NUM; i++) {
+		fiper_ctrl |= FIPER_CTRL_DIS(i);
+		fiper_ctrl &= ~FIPER_CTRL_PG(i);
+	}
+	netc_timer_wr(priv, NETC_TMR_FIPER_CTRL, fiper_ctrl);
+
+	ktime_get_real_ts64(&now);
+	ns = timespec64_to_ns(&now);
+	netc_timer_cnt_write(priv, ns);
+
+	/* Allow atomic writes to TCLK_PERIOD and TMR_ADD, An update to
+	 * TCLK_PERIOD does not take effect until TMR_ADD is written.
+	 */
+	tmr_ctrl |= ((integral_period << 16) & TMR_CTRL_TCLK_PERIOD) |
+		     TMR_COMP_MODE | TMR_CTRL_FS;
+	netc_timer_wr(priv, NETC_TMR_CTRL, tmr_ctrl);
+	netc_timer_wr(priv, NETC_TMR_ADD, fractional_period);
+	netc_timer_wr(priv, NETC_TMR_TEMASK, tmr_emask);
+}
+
+static int netc_timer_pci_probe(struct pci_dev *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct netc_timer *priv;
+	int err, len;
+
+	pcie_flr(pdev);
+	err = pci_enable_device_mem(pdev);
+	if (err)
+		return dev_err_probe(dev, err, "Failed to enable device\n");
+
+	err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+	if (err) {
+		dev_err(dev, "dma_set_mask_and_coherent() failed, err:%pe\n",
+			ERR_PTR(err));
+		goto disable_dev;
+	}
+
+	err = pci_request_mem_regions(pdev, KBUILD_MODNAME);
+	if (err) {
+		dev_err(dev, "pci_request_regions() failed, err:%pe\n",
+			ERR_PTR(err));
+		goto disable_dev;
+	}
+
+	pci_set_master(pdev);
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		err = -ENOMEM;
+		goto release_mem_regions;
+	}
+
+	priv->pdev = pdev;
+	len = pci_resource_len(pdev, NETC_TMR_REGS_BAR);
+	priv->base = ioremap(pci_resource_start(pdev, NETC_TMR_REGS_BAR), len);
+	if (!priv->base) {
+		err = -ENXIO;
+		dev_err(dev, "ioremap() failed\n");
+		goto free_priv;
+	}
+
+	pci_set_drvdata(pdev, priv);
+
+	return 0;
+
+free_priv:
+	kfree(priv);
+release_mem_regions:
+	pci_release_mem_regions(pdev);
+disable_dev:
+	pci_disable_device(pdev);
+
+	return err;
+}
+
+static void netc_timer_pci_remove(struct pci_dev *pdev)
+{
+	struct netc_timer *priv = pci_get_drvdata(pdev);
+
+	iounmap(priv->base);
+	kfree(priv);
+	pci_release_mem_regions(pdev);
+	pci_disable_device(pdev);
+}
+
+static void netc_timer_get_source_clk(struct netc_timer *priv)
+{
+	struct device *dev = &priv->pdev->dev;
+	struct device_node *np = dev->of_node;
+	const char *clk_name = NULL;
+	u64 ns = NSEC_PER_SEC;
+
+	if (!np)
+		goto select_system_clk;
+
+	of_property_read_string(np, "clock-names", &clk_name);
+	if (clk_name) {
+		priv->src_clk = devm_clk_get_optional(dev, clk_name);
+		if (IS_ERR_OR_NULL(priv->src_clk)) {
+			dev_warn(dev, "Failed to get source clock\n");
+			priv->src_clk = NULL;
+			goto select_system_clk;
+		}
+
+		priv->clk_freq = clk_get_rate(priv->src_clk);
+		if (!strcmp(clk_name, "system")) {
+			/* There is a 1/2 divider */
+			priv->clk_freq /= 2;
+			priv->clk_select = NETC_TMR_SYSTEM_CLK;
+		} else if (!strcmp(clk_name, "ccm_timer")) {
+			priv->clk_select = NETC_TMR_CCM_TIMER1;
+		} else if (!strcmp(clk_name, "ext_1588")) {
+			priv->clk_select = NETC_TMR_EXT_OSC;
+		} else {
+			dev_warn(dev, "Unknown clock source\n");
+			priv->src_clk = NULL;
+			goto select_system_clk;
+		}
+
+		goto cal_clk_period;
+	}
+
+select_system_clk:
+	priv->clk_select = NETC_TMR_SYSTEM_CLK;
+	priv->clk_freq = NETC_TMR_SYSCLK_333M;
+
+cal_clk_period:
+	priv->period = div_u64(ns << 32, priv->clk_freq);
+}
+
+static void netc_timer_parse_dt(struct netc_timer *priv)
+{
+	netc_timer_get_source_clk(priv);
+}
+
+static irqreturn_t netc_timer_isr(int irq, void *data)
+{
+	struct netc_timer *priv = data;
+	u32 tmr_event, tmr_emask;
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	tmr_event = netc_timer_rd(priv, NETC_TMR_TEVENT);
+	tmr_emask = netc_timer_rd(priv, NETC_TMR_TEMASK);
+
+	tmr_event &= tmr_emask;
+	if (tmr_event & TMR_TEVENT_ALM1EN)
+		netc_timer_alarm_write(priv, NETC_TMR_DEFAULT_ALARM, 0);
+
+	if (tmr_event & TMR_TEVENT_ALM2EN)
+		netc_timer_alarm_write(priv, NETC_TMR_DEFAULT_ALARM, 1);
+
+	/* Clear interrupts status */
+	netc_timer_wr(priv, NETC_TMR_TEVENT, tmr_event);
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return IRQ_HANDLED;
+}
+
+static int netc_timer_init_msix_irq(struct netc_timer *priv)
+{
+	struct pci_dev *pdev = priv->pdev;
+	char irq_name[64];
+	int err, n;
+
+	n = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX);
+	if (n != 1) {
+		err = (n < 0) ? n : -EPERM;
+		dev_err(&pdev->dev, "pci_alloc_irq_vectors() failed\n");
+		return err;
+	}
+
+	priv->irq = pci_irq_vector(pdev, 0);
+	snprintf(irq_name, sizeof(irq_name), "ptp-netc %s", pci_name(pdev));
+	err = request_irq(priv->irq, netc_timer_isr, 0, irq_name, priv);
+	if (err) {
+		dev_err(&pdev->dev, "request_irq() failed\n");
+		pci_free_irq_vectors(pdev);
+		return err;
+	}
+
+	return 0;
+}
+
+static void netc_timer_free_msix_irq(struct netc_timer *priv)
+{
+	struct pci_dev *pdev = priv->pdev;
+
+	disable_irq(priv->irq);
+	free_irq(priv->irq, priv);
+	pci_free_irq_vectors(pdev);
+}
+
+static int netc_timer_probe(struct pci_dev *pdev,
+			    const struct pci_device_id *id)
+{
+	struct device *dev = &pdev->dev;
+	struct netc_timer *priv;
+	int err;
+
+	err = netc_timer_pci_probe(pdev);
+	if (err)
+		return err;
+
+	priv = pci_get_drvdata(pdev);
+	netc_timer_parse_dt(priv);
+
+	priv->caps = netc_timer_ptp_caps;
+	priv->oclk_prsc = NETC_TMR_DEFAULT_PRSC;
+	priv->phc_index = -1; /* initialize it as an invalid index */
+	spin_lock_init(&priv->lock);
+
+	err = clk_prepare_enable(priv->src_clk);
+	if (err) {
+		dev_err(dev, "Failed to enable timer source clock\n");
+		goto timer_pci_remove;
+	}
+
+	err = netc_timer_init_msix_irq(priv);
+	if (err)
+		goto disable_clk;
+
+	netc_timer_init(priv);
+	priv->clock = ptp_clock_register(&priv->caps, dev);
+	if (IS_ERR(priv->clock)) {
+		err = PTR_ERR(priv->clock);
+		goto free_msix_irq;
+	}
+
+	priv->phc_index = ptp_clock_index(priv->clock);
+
+	return 0;
+
+free_msix_irq:
+	netc_timer_free_msix_irq(priv);
+disable_clk:
+	clk_disable_unprepare(priv->src_clk);
+timer_pci_remove:
+	netc_timer_pci_remove(pdev);
+
+	return err;
+}
+
+static void netc_timer_remove(struct pci_dev *pdev)
+{
+	struct netc_timer *priv = pci_get_drvdata(pdev);
+
+	ptp_clock_unregister(priv->clock);
+	netc_timer_free_msix_irq(priv);
+	clk_disable_unprepare(priv->src_clk);
+	netc_timer_pci_remove(pdev);
+}
+
+static const struct pci_device_id netc_timer_id_table[] = {
+	{ PCI_DEVICE(NETC_TMR_PCI_VENDOR, NETC_TMR_PCI_DEVID) },
+	{ 0, } /* End of table. */
+};
+MODULE_DEVICE_TABLE(pci, netc_timer_id_table);
+
+static struct pci_driver netc_timer_driver = {
+	.name = KBUILD_MODNAME,
+	.id_table = netc_timer_id_table,
+	.probe = netc_timer_probe,
+	.remove = netc_timer_remove,
+};
+module_pci_driver(netc_timer_driver);
+
+MODULE_DESCRIPTION("NXP NETC Timer PTP Driver");
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/include/linux/fsl/netc_global.h b/include/linux/fsl/netc_global.h
index fdecca8c90f0..59c835e67ada 100644
--- a/include/linux/fsl/netc_global.h
+++ b/include/linux/fsl/netc_global.h
@@ -1,10 +1,11 @@
 /* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
-/* Copyright 2024 NXP
+/* Copyright 2024-2025 NXP
  */
 #ifndef __NETC_GLOBAL_H
 #define __NETC_GLOBAL_H
 
 #include <linux/io.h>
+#include <linux/pci.h>
 
 static inline u32 netc_read(void __iomem *reg)
 {
@@ -16,4 +17,13 @@ static inline void netc_write(void __iomem *reg, u32 val)
 	iowrite32(val, reg);
 }
 
+#if IS_ENABLED(CONFIG_PTP_1588_CLOCK_NETC)
+int netc_timer_get_phc_index(struct pci_dev *timer_pdev);
+#else
+static inline int netc_timer_get_phc_index(struct pci_dev *timer_pdev)
+{
+	return -ENODEV;
+}
+#endif
+
 #endif
-- 
2.34.1

Re: [PATCH net-next 02/12] ptp: netc: add NETC Timer PTP driver support

[Andrew Lunn]

> +	of_property_read_string(np, "clock-names", &clk_name);
> +	if (clk_name) {
> +		priv->src_clk = devm_clk_get_optional(dev, clk_name);
> +		if (IS_ERR_OR_NULL(priv->src_clk)) {
> +			dev_warn(dev, "Failed to get source clock\n");
> +			priv->src_clk = NULL;
> +			goto select_system_clk;
> +		}
> +
> +		priv->clk_freq = clk_get_rate(priv->src_clk);
> +		if (!strcmp(clk_name, "system")) {
> +			/* There is a 1/2 divider */
> +			priv->clk_freq /= 2;
> +			priv->clk_select = NETC_TMR_SYSTEM_CLK;
> +		} else if (!strcmp(clk_name, "ccm_timer")) {
> +			priv->clk_select = NETC_TMR_CCM_TIMER1;
> +		} else if (!strcmp(clk_name, "ext_1588")) {
> +			priv->clk_select = NETC_TMR_EXT_OSC;
> +		} else {
> +			dev_warn(dev, "Unknown clock source\n");
> +			priv->src_clk = NULL;
> +			goto select_system_clk;
> +		}

That is pretty unusual. Generally, a clock is a clock, and you only
use the name to pick out a specific clock when there are multiple
listed.

Please expand the binding documentation to include a description of
how the clock name is used here.

I don't generally get involved with clock trees, but i'm wondering if
the tree is correctly described. Maybe you need to add a clk-divider.c
into the tree to represent the system clock being divided by two?

	Andrew

RE: [PATCH net-next 02/12] ptp: netc: add NETC Timer PTP driver support

[Wei Fang]

> > +	of_property_read_string(np, "clock-names", &clk_name);
> > +	if (clk_name) {
> > +		priv->src_clk = devm_clk_get_optional(dev, clk_name);
> > +		if (IS_ERR_OR_NULL(priv->src_clk)) {
> > +			dev_warn(dev, "Failed to get source clock\n");
> > +			priv->src_clk = NULL;
> > +			goto select_system_clk;
> > +		}
> > +
> > +		priv->clk_freq = clk_get_rate(priv->src_clk);
> > +		if (!strcmp(clk_name, "system")) {
> > +			/* There is a 1/2 divider */
> > +			priv->clk_freq /= 2;
> > +			priv->clk_select = NETC_TMR_SYSTEM_CLK;
> > +		} else if (!strcmp(clk_name, "ccm_timer")) {
> > +			priv->clk_select = NETC_TMR_CCM_TIMER1;
> > +		} else if (!strcmp(clk_name, "ext_1588")) {
> > +			priv->clk_select = NETC_TMR_EXT_OSC;
> > +		} else {
> > +			dev_warn(dev, "Unknown clock source\n");
> > +			priv->src_clk = NULL;
> > +			goto select_system_clk;
> > +		}
> 
> That is pretty unusual. Generally, a clock is a clock, and you only
> use the name to pick out a specific clock when there are multiple
> listed.
> 
> Please expand the binding documentation to include a description of
> how the clock name is used here.
> 
> I don't generally get involved with clock trees, but i'm wondering if
> the tree is correctly described. Maybe you need to add a clk-divider.c
> into the tree to represent the system clock being divided by two?
> 

Currently, for i.MX platforms, there is a fixed 1/2 divider inside the
NETCMIX, so the system clock rate is always divided by two. Another
solution is to add a " fixed-factor-clock" node to the DTS and set the
"clock-div" to 2. Then this clock is used as the system clock of NETC,
so that we can remove the 1/2 divider from the driver. This method
should be more appropriate, after all, future platforms may remove
the 1/2 divider or replace it with another divider.

Re: [PATCH net-next 02/12] ptp: netc: add NETC Timer PTP driver support

[Vadim Fedorenko]

On 11.07.2025 07:57, Wei Fang wrote:
> NETC 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. The enetc v4 driver can implement PTP
> synchronization through the relevant interfaces provided by the driver.
> Note that the current driver does not support PEROUT, PPS and EXTTS yet,
> and support will be added one by one in subsequent patches.
> 
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
>   drivers/ptp/Kconfig             |  11 +
>   drivers/ptp/Makefile            |   1 +
>   drivers/ptp/ptp_netc.c          | 565 ++++++++++++++++++++++++++++++++
>   include/linux/fsl/netc_global.h |  12 +-
>   4 files changed, 588 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/ptp/ptp_netc.c
> 
> diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
> index 204278eb215e..92eb2ff41180 100644
> --- a/drivers/ptp/Kconfig
> +++ b/drivers/ptp/Kconfig
> @@ -252,4 +252,15 @@ config PTP_S390
>   	  driver provides the raw clock value without the delta to
>   	  userspace. That way userspace programs like chrony could steer
>   	  the kernel clock.
> +
> +config PTP_1588_CLOCK_NETC
> +	bool "NXP NETC Timer PTP Driver"
> +	depends on PTP_1588_CLOCK
> +	depends on PCI_MSI
> +	help
> +	  This driver adds support for using the NXP NETC Timer as a PTP
> +	  clock. This clock is used by ENETC MAC or NETC Switch for PTP
> +	  synchronization. It also supports periodic output signal (e.g.
> +	  PPS) and external trigger timestamping.
> +
>   endmenu
> diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
> index 25f846fe48c9..d48fe4009fa4 100644
> --- a/drivers/ptp/Makefile
> +++ b/drivers/ptp/Makefile
> @@ -23,3 +23,4 @@ obj-$(CONFIG_PTP_1588_CLOCK_VMW)	+= ptp_vmw.o
>   obj-$(CONFIG_PTP_1588_CLOCK_OCP)	+= ptp_ocp.o
>   obj-$(CONFIG_PTP_DFL_TOD)		+= ptp_dfl_tod.o
>   obj-$(CONFIG_PTP_S390)			+= ptp_s390.o
> +obj-$(CONFIG_PTP_1588_CLOCK_NETC)	+= ptp_netc.o
> diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
> new file mode 100644
> index 000000000000..87d456fcadfd
> --- /dev/null
> +++ b/drivers/ptp/ptp_netc.c
> @@ -0,0 +1,565 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
> +/*
> + * NXP NETC Timer driver
> + * Copyright 2025 NXP
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/fsl/netc_global.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/ptp_clock_kernel.h>
> +
> +#define NETC_TMR_PCI_VENDOR		0x1131
> +#define NETC_TMR_PCI_DEVID		0xee02
> +
> +#define NETC_TMR_CTRL			0x0080
> +#define  TMR_CTRL_CK_SEL		GENMASK(1, 0)
> +#define  TMR_CTRL_TE			BIT(2)
> +#define  TMR_COMP_MODE			BIT(15)
> +#define  TMR_CTRL_TCLK_PERIOD		GENMASK(25, 16)
> +#define  TMR_CTRL_FS			BIT(28)
> +#define  TMR_ALARM1P			BIT(31)
> +
> +#define NETC_TMR_TEVENT			0x0084
> +#define  TMR_TEVENT_ALM1EN		BIT(16)
> +#define  TMR_TEVENT_ALM2EN		BIT(17)
> +
> +#define NETC_TMR_TEMASK			0x0088
> +#define NETC_TMR_CNT_L			0x0098
> +#define NETC_TMR_CNT_H			0x009c
> +#define NETC_TMR_ADD			0x00a0
> +#define NETC_TMR_PRSC			0x00a8
> +#define NETC_TMR_OFF_L			0x00b0
> +#define NETC_TMR_OFF_H			0x00b4
> +
> +/* i = 0, 1, i indicates the index of TMR_ALARM */
> +#define NETC_TMR_ALARM_L(i)		(0x00b8 + (i) * 8)
> +#define NETC_TMR_ALARM_H(i)		(0x00bc + (i) * 8)
> +
> +#define NETC_TMR_FIPER_CTRL		0x00dc
> +#define  FIPER_CTRL_DIS(i)		(BIT(7) << (i) * 8)
> +#define  FIPER_CTRL_PG(i)		(BIT(6) << (i) * 8)
> +
> +#define NETC_TMR_CUR_TIME_L		0x00f0
> +#define NETC_TMR_CUR_TIME_H		0x00f4
> +
> +#define NETC_TMR_REGS_BAR		0
> +
> +#define NETC_TMR_FIPER_NUM		3
> +#define NETC_TMR_DEFAULT_PRSC		2
> +#define NETC_TMR_DEFAULT_ALARM		GENMASK_ULL(63, 0)
> +
> +/* 1588 timer reference clock source select */
> +#define NETC_TMR_CCM_TIMER1		0 /* enet_timer1_clk_root, from CCM */
> +#define NETC_TMR_SYSTEM_CLK		1 /* enet_clk_root/2, from CCM */
> +#define NETC_TMR_EXT_OSC		2 /* tmr_1588_clk, from IO pins */
> +
> +#define NETC_TMR_SYSCLK_333M		333333333U
> +
> +struct netc_timer {
> +	void __iomem *base;
> +	struct pci_dev *pdev;
> +	spinlock_t lock; /* Prevent concurrent access to registers */
> +
> +	struct clk *src_clk;
> +	struct ptp_clock *clock;
> +	struct ptp_clock_info caps;
> +	int phc_index;
> +	u32 clk_select;
> +	u32 clk_freq;
> +	u32 oclk_prsc;
> +	/* High 32-bit is integer part, low 32-bit is fractional part */
> +	u64 period;
> +
> +	int irq;
> +};
> +
> +#define netc_timer_rd(p, o)		netc_read((p)->base + (o))
> +#define netc_timer_wr(p, o, v)		netc_write((p)->base + (o), v)
> +#define ptp_to_netc_timer(ptp)		container_of((ptp), struct netc_timer, caps)
> +
> +static u64 netc_timer_cnt_read(struct netc_timer *priv)
> +{
> +	u32 tmr_cnt_l, tmr_cnt_h;
> +	u64 ns;
> +
> +	/* The user must read the TMR_CNC_L register first to get
> +	 * correct 64-bit TMR_CNT_H/L counter values.
> +	 */
> +	tmr_cnt_l = netc_timer_rd(priv, NETC_TMR_CNT_L);
> +	tmr_cnt_h = netc_timer_rd(priv, NETC_TMR_CNT_H);
> +	ns = (((u64)tmr_cnt_h) << 32) | tmr_cnt_l;
> +
> +	return ns;
> +}
> +
> +static void netc_timer_cnt_write(struct netc_timer *priv, u64 ns)
> +{
> +	u32 tmr_cnt_h = upper_32_bits(ns);
> +	u32 tmr_cnt_l = lower_32_bits(ns);
> +
> +	/* The user must write to TMR_CNT_L register first. */
> +	netc_timer_wr(priv, NETC_TMR_CNT_L, tmr_cnt_l);
> +	netc_timer_wr(priv, NETC_TMR_CNT_H, tmr_cnt_h);
> +}
> +
> +static u64 netc_timer_offset_read(struct netc_timer *priv)
> +{
> +	u32 tmr_off_l, tmr_off_h;
> +	u64 offset;
> +
> +	tmr_off_l = netc_timer_rd(priv, NETC_TMR_OFF_L);
> +	tmr_off_h = netc_timer_rd(priv, NETC_TMR_OFF_H);
> +	offset = (((u64)tmr_off_h) << 32) | tmr_off_l;
> +
> +	return offset;
> +}
> +
> +static void netc_timer_offset_write(struct netc_timer *priv, u64 offset)
> +{
> +	u32 tmr_off_h = upper_32_bits(offset);
> +	u32 tmr_off_l = lower_32_bits(offset);
> +
> +	netc_timer_wr(priv, NETC_TMR_OFF_L, tmr_off_l);
> +	netc_timer_wr(priv, NETC_TMR_OFF_H, tmr_off_h);
> +}
> +
> +static u64 netc_timer_cur_time_read(struct netc_timer *priv)
> +{
> +	u32 time_h, time_l;
> +	u64 ns;
> +
> +	time_l = netc_timer_rd(priv, NETC_TMR_CUR_TIME_L);
> +	time_h = netc_timer_rd(priv, NETC_TMR_CUR_TIME_H);
> +	ns = (u64)time_h << 32 | time_l;

I assume that the high part is latched after reading low part, but would like
you confirm it and put a comment as you did for counter read.

> +
> +	return ns;
> +}
> +
> +static void netc_timer_alarm_write(struct netc_timer *priv,
> +				   u64 alarm, int index)
> +{
> +	u32 alarm_h = upper_32_bits(alarm);
> +	u32 alarm_l = lower_32_bits(alarm);
> +
> +	netc_timer_wr(priv, NETC_TMR_ALARM_L(index), alarm_l);
> +	netc_timer_wr(priv, NETC_TMR_ALARM_H(index), alarm_h);
> +}
> +
> +static void netc_timer_adjust_period(struct netc_timer *priv, u64 period)
> +{
> +	u32 fractional_period = lower_32_bits(period);
> +	u32 integral_period = upper_32_bits(period);
> +	u32 tmr_ctrl, old_tmr_ctrl;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->lock, flags);
> +
> +	old_tmr_ctrl = netc_timer_rd(priv, NETC_TMR_CTRL);
> +	tmr_ctrl = u32_replace_bits(old_tmr_ctrl, integral_period,
> +				    TMR_CTRL_TCLK_PERIOD);
> +	if (tmr_ctrl != old_tmr_ctrl)
> +		netc_timer_wr(priv, NETC_TMR_CTRL, tmr_ctrl);
> +
> +	netc_timer_wr(priv, NETC_TMR_ADD, fractional_period);
> +
> +	spin_unlock_irqrestore(&priv->lock, flags);
> +}
> +
> +static int netc_timer_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
> +{
> +	struct netc_timer *priv = ptp_to_netc_timer(ptp);
> +	u64 new_period;
> +
> +	if (!scaled_ppm)
> +		return 0;

why do you ignore value of 0 here? the adjustment will not happen if for some
reasons the offset will be aligned after previous adjustment. And there will be
inconsistency between hardware value and the last stored value in software.

> +
> +	new_period = adjust_by_scaled_ppm(priv->period, scaled_ppm);
> +	netc_timer_adjust_period(priv, new_period);
> +
> +	return 0;
> +}
> +
> +static int netc_timer_adjtime(struct ptp_clock_info *ptp, s64 delta)
> +{
> +	struct netc_timer *priv = ptp_to_netc_timer(ptp);
> +	u64 tmr_cnt, tmr_off;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->lock, flags);
> +
> +	tmr_off = netc_timer_offset_read(priv);
> +	if (delta < 0 && tmr_off < abs(delta)) {
> +		delta += tmr_off;
> +		if (!tmr_off)
> +			netc_timer_offset_write(priv, 0);
> +
> +		tmr_cnt = netc_timer_cnt_read(priv);
> +		tmr_cnt += delta;
> +		netc_timer_cnt_write(priv, tmr_cnt);
> +	} else {
> +		tmr_off += delta;
> +		netc_timer_offset_write(priv, tmr_off);
> +	}
> +
> +	spin_unlock_irqrestore(&priv->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int netc_timer_gettimex64(struct ptp_clock_info *ptp,
> +				 struct timespec64 *ts,
> +				 struct ptp_system_timestamp *sts)
> +{
> +	struct netc_timer *priv = ptp_to_netc_timer(ptp);
> +	unsigned long flags;
> +	u64 ns;
> +
> +	spin_lock_irqsave(&priv->lock, flags);
> +
> +	ptp_read_system_prets(sts);
> +	ns = netc_timer_cur_time_read(priv);
> +	ptp_read_system_postts(sts);
> +
> +	spin_unlock_irqrestore(&priv->lock, flags);
> +
> +	*ts = ns_to_timespec64(ns);
> +
> +	return 0;
> +}
> +
> +static int netc_timer_settime64(struct ptp_clock_info *ptp,
> +				const struct timespec64 *ts)
> +{
> +	struct netc_timer *priv = ptp_to_netc_timer(ptp);
> +	u64 ns = timespec64_to_ns(ts);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->lock, flags);
> +	netc_timer_offset_write(priv, 0);
> +	netc_timer_cnt_write(priv, ns);
> +	spin_unlock_irqrestore(&priv->lock, flags);
> +
> +	return 0;
> +}
> +
> +int netc_timer_get_phc_index(struct pci_dev *timer_pdev)
> +{
> +	struct netc_timer *priv;
> +
> +	if (!timer_pdev)
> +		return -ENODEV;

I'm not sure, but looks like this should never happen. Could you please explain
what are protecting from? If it's just safety, then it's better to remove the
check and let the kernel crash to figure out wrong usage.

> +
> +	priv = pci_get_drvdata(timer_pdev);
> +	if (!priv)
> +		return -EINVAL;
> +
> +	return priv->phc_index;
> +}
> +EXPORT_SYMBOL_GPL(netc_timer_get_phc_index);
> +
> +static const struct ptp_clock_info netc_timer_ptp_caps = {
> +	.owner		= THIS_MODULE,
> +	.name		= "NETC Timer PTP clock",
> +	.max_adj	= 500000000,
> +	.n_alarm	= 2,
> +	.n_pins		= 0,
> +	.adjfine	= netc_timer_adjfine,
> +	.adjtime	= netc_timer_adjtime,
> +	.gettimex64	= netc_timer_gettimex64,
> +	.settime64	= netc_timer_settime64,
> +};
> +
> +static void netc_timer_init(struct netc_timer *priv)
> +{
> +	u32 tmr_emask = TMR_TEVENT_ALM1EN | TMR_TEVENT_ALM2EN;
> +	u32 fractional_period = lower_32_bits(priv->period);
> +	u32 integral_period = upper_32_bits(priv->period);
> +	u32 tmr_ctrl, fiper_ctrl;
> +	struct timespec64 now;
> +	u64 ns;
> +	int i;
> +
> +	/* Software must enable timer first and the clock selected must be
> +	 * active, otherwise, the registers which are in the timer clock
> +	 * domain are not accessible.
> +	 */
> +	tmr_ctrl = (priv->clk_select & TMR_CTRL_CK_SEL) | TMR_CTRL_TE;
> +	netc_timer_wr(priv, NETC_TMR_CTRL, tmr_ctrl);
> +	netc_timer_wr(priv, NETC_TMR_PRSC, priv->oclk_prsc);
> +
> +	/* Disable FIPER by default */
> +	fiper_ctrl = netc_timer_rd(priv, NETC_TMR_FIPER_CTRL);
> +	for (i = 0; i < NETC_TMR_FIPER_NUM; i++) {
> +		fiper_ctrl |= FIPER_CTRL_DIS(i);
> +		fiper_ctrl &= ~FIPER_CTRL_PG(i);
> +	}
> +	netc_timer_wr(priv, NETC_TMR_FIPER_CTRL, fiper_ctrl);
> +
> +	ktime_get_real_ts64(&now);
> +	ns = timespec64_to_ns(&now);
> +	netc_timer_cnt_write(priv, ns);
> +
> +	/* Allow atomic writes to TCLK_PERIOD and TMR_ADD, An update to
> +	 * TCLK_PERIOD does not take effect until TMR_ADD is written.
> +	 */
> +	tmr_ctrl |= ((integral_period << 16) & TMR_CTRL_TCLK_PERIOD) |
> +		     TMR_COMP_MODE | TMR_CTRL_FS;
> +	netc_timer_wr(priv, NETC_TMR_CTRL, tmr_ctrl);
> +	netc_timer_wr(priv, NETC_TMR_ADD, fractional_period);
> +	netc_timer_wr(priv, NETC_TMR_TEMASK, tmr_emask);
> +}
> +
> +static int netc_timer_pci_probe(struct pci_dev *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct netc_timer *priv;
> +	int err, len;
> +
> +	pcie_flr(pdev);
> +	err = pci_enable_device_mem(pdev);
> +	if (err)
> +		return dev_err_probe(dev, err, "Failed to enable device\n");
> +
> +	err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> +	if (err) {
> +		dev_err(dev, "dma_set_mask_and_coherent() failed, err:%pe\n",
> +			ERR_PTR(err));
> +		goto disable_dev;
> +	}
> +
> +	err = pci_request_mem_regions(pdev, KBUILD_MODNAME);
> +	if (err) {
> +		dev_err(dev, "pci_request_regions() failed, err:%pe\n",
> +			ERR_PTR(err));
> +		goto disable_dev;
> +	}
> +
> +	pci_set_master(pdev);
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv) {
> +		err = -ENOMEM;
> +		goto release_mem_regions;
> +	}
> +
> +	priv->pdev = pdev;
> +	len = pci_resource_len(pdev, NETC_TMR_REGS_BAR);
> +	priv->base = ioremap(pci_resource_start(pdev, NETC_TMR_REGS_BAR), len);
> +	if (!priv->base) {
> +		err = -ENXIO;
> +		dev_err(dev, "ioremap() failed\n");
> +		goto free_priv;
> +	}
> +
> +	pci_set_drvdata(pdev, priv);
> +
> +	return 0;
> +
> +free_priv:
> +	kfree(priv);
> +release_mem_regions:
> +	pci_release_mem_regions(pdev);
> +disable_dev:
> +	pci_disable_device(pdev);
> +
> +	return err;
> +}
> +
> +static void netc_timer_pci_remove(struct pci_dev *pdev)
> +{
> +	struct netc_timer *priv = pci_get_drvdata(pdev);
> +
> +	iounmap(priv->base);
> +	kfree(priv);
> +	pci_release_mem_regions(pdev);
> +	pci_disable_device(pdev);
> +}
> +
> +static void netc_timer_get_source_clk(struct netc_timer *priv)
> +{
> +	struct device *dev = &priv->pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	const char *clk_name = NULL;
> +	u64 ns = NSEC_PER_SEC;
> +
> +	if (!np)
> +		goto select_system_clk;
> +
> +	of_property_read_string(np, "clock-names", &clk_name);
> +	if (clk_name) {
> +		priv->src_clk = devm_clk_get_optional(dev, clk_name);
> +		if (IS_ERR_OR_NULL(priv->src_clk)) {
> +			dev_warn(dev, "Failed to get source clock\n");
> +			priv->src_clk = NULL;
> +			goto select_system_clk;
> +		}
> +
> +		priv->clk_freq = clk_get_rate(priv->src_clk);
> +		if (!strcmp(clk_name, "system")) {
> +			/* There is a 1/2 divider */
> +			priv->clk_freq /= 2;
> +			priv->clk_select = NETC_TMR_SYSTEM_CLK;
> +		} else if (!strcmp(clk_name, "ccm_timer")) {
> +			priv->clk_select = NETC_TMR_CCM_TIMER1;
> +		} else if (!strcmp(clk_name, "ext_1588")) {
> +			priv->clk_select = NETC_TMR_EXT_OSC;
> +		} else {
> +			dev_warn(dev, "Unknown clock source\n");
> +			priv->src_clk = NULL;
> +			goto select_system_clk;
> +		}
> +
> +		goto cal_clk_period;
> +	}
> +
> +select_system_clk:
> +	priv->clk_select = NETC_TMR_SYSTEM_CLK;
> +	priv->clk_freq = NETC_TMR_SYSCLK_333M;
> +
> +cal_clk_period:
> +	priv->period = div_u64(ns << 32, priv->clk_freq);
> +}
> +
> +static void netc_timer_parse_dt(struct netc_timer *priv)
> +{
> +	netc_timer_get_source_clk(priv);
> +}
> +
> +static irqreturn_t netc_timer_isr(int irq, void *data)
> +{
> +	struct netc_timer *priv = data;
> +	u32 tmr_event, tmr_emask;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->lock, flags);
> +
> +	tmr_event = netc_timer_rd(priv, NETC_TMR_TEVENT);
> +	tmr_emask = netc_timer_rd(priv, NETC_TMR_TEMASK);
> +
> +	tmr_event &= tmr_emask;
> +	if (tmr_event & TMR_TEVENT_ALM1EN)
> +		netc_timer_alarm_write(priv, NETC_TMR_DEFAULT_ALARM, 0);
> +
> +	if (tmr_event & TMR_TEVENT_ALM2EN)
> +		netc_timer_alarm_write(priv, NETC_TMR_DEFAULT_ALARM, 1);
> +
> +	/* Clear interrupts status */
> +	netc_timer_wr(priv, NETC_TMR_TEVENT, tmr_event);
> +
> +	spin_unlock_irqrestore(&priv->lock, flags);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int netc_timer_init_msix_irq(struct netc_timer *priv)
> +{
> +	struct pci_dev *pdev = priv->pdev;
> +	char irq_name[64];
> +	int err, n;
> +
> +	n = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX);
> +	if (n != 1) {
> +		err = (n < 0) ? n : -EPERM;
> +		dev_err(&pdev->dev, "pci_alloc_irq_vectors() failed\n");
> +		return err;
> +	}
> +
> +	priv->irq = pci_irq_vector(pdev, 0);
> +	snprintf(irq_name, sizeof(irq_name), "ptp-netc %s", pci_name(pdev));
> +	err = request_irq(priv->irq, netc_timer_isr, 0, irq_name, priv);
> +	if (err) {
> +		dev_err(&pdev->dev, "request_irq() failed\n");
> +		pci_free_irq_vectors(pdev);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static void netc_timer_free_msix_irq(struct netc_timer *priv)
> +{
> +	struct pci_dev *pdev = priv->pdev;
> +
> +	disable_irq(priv->irq);
> +	free_irq(priv->irq, priv);
> +	pci_free_irq_vectors(pdev);
> +}
> +
> +static int netc_timer_probe(struct pci_dev *pdev,
> +			    const struct pci_device_id *id)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct netc_timer *priv;
> +	int err;
> +
> +	err = netc_timer_pci_probe(pdev);
> +	if (err)
> +		return err;
> +
> +	priv = pci_get_drvdata(pdev);
> +	netc_timer_parse_dt(priv);
> +
> +	priv->caps = netc_timer_ptp_caps;
> +	priv->oclk_prsc = NETC_TMR_DEFAULT_PRSC;
> +	priv->phc_index = -1; /* initialize it as an invalid index */
> +	spin_lock_init(&priv->lock);
> +
> +	err = clk_prepare_enable(priv->src_clk);
> +	if (err) {
> +		dev_err(dev, "Failed to enable timer source clock\n");
> +		goto timer_pci_remove;
> +	}
> +
> +	err = netc_timer_init_msix_irq(priv);
> +	if (err)
> +		goto disable_clk;
> +
> +	netc_timer_init(priv);
> +	priv->clock = ptp_clock_register(&priv->caps, dev);
> +	if (IS_ERR(priv->clock)) {
> +		err = PTR_ERR(priv->clock);
> +		goto free_msix_irq;
> +	}
> +
> +	priv->phc_index = ptp_clock_index(priv->clock);
> +
> +	return 0;
> +
> +free_msix_irq:
> +	netc_timer_free_msix_irq(priv);
> +disable_clk:
> +	clk_disable_unprepare(priv->src_clk);
> +timer_pci_remove:
> +	netc_timer_pci_remove(pdev);
> +
> +	return err;
> +}
> +
> +static void netc_timer_remove(struct pci_dev *pdev)
> +{
> +	struct netc_timer *priv = pci_get_drvdata(pdev);
> +
> +	ptp_clock_unregister(priv->clock);
> +	netc_timer_free_msix_irq(priv);
> +	clk_disable_unprepare(priv->src_clk);
> +	netc_timer_pci_remove(pdev);
> +}
> +
> +static const struct pci_device_id netc_timer_id_table[] = {
> +	{ PCI_DEVICE(NETC_TMR_PCI_VENDOR, NETC_TMR_PCI_DEVID) },
> +	{ 0, } /* End of table. */
> +};
> +MODULE_DEVICE_TABLE(pci, netc_timer_id_table);
> +
> +static struct pci_driver netc_timer_driver = {
> +	.name = KBUILD_MODNAME,
> +	.id_table = netc_timer_id_table,
> +	.probe = netc_timer_probe,
> +	.remove = netc_timer_remove,
> +};
> +module_pci_driver(netc_timer_driver);
> +
> +MODULE_DESCRIPTION("NXP NETC Timer PTP Driver");
> +MODULE_LICENSE("Dual BSD/GPL");
> diff --git a/include/linux/fsl/netc_global.h b/include/linux/fsl/netc_global.h
> index fdecca8c90f0..59c835e67ada 100644
> --- a/include/linux/fsl/netc_global.h
> +++ b/include/linux/fsl/netc_global.h
> @@ -1,10 +1,11 @@
>   /* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
> -/* Copyright 2024 NXP
> +/* Copyright 2024-2025 NXP
>    */
>   #ifndef __NETC_GLOBAL_H
>   #define __NETC_GLOBAL_H
>   
>   #include <linux/io.h>
> +#include <linux/pci.h>
>   
>   static inline u32 netc_read(void __iomem *reg)
>   {
> @@ -16,4 +17,13 @@ static inline void netc_write(void __iomem *reg, u32 val)
>   	iowrite32(val, reg);
>   }
>   
> +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK_NETC)
> +int netc_timer_get_phc_index(struct pci_dev *timer_pdev);
> +#else
> +static inline int netc_timer_get_phc_index(struct pci_dev *timer_pdev)
> +{
> +	return -ENODEV;
> +}
> +#endif
> +
>   #endif

RE: [PATCH net-next 02/12] ptp: netc: add NETC Timer PTP driver support

[Wei Fang]

> > +static void netc_timer_offset_write(struct netc_timer *priv, u64 offset)
> > +{
> > +	u32 tmr_off_h = upper_32_bits(offset);
> > +	u32 tmr_off_l = lower_32_bits(offset);
> > +
> > +	netc_timer_wr(priv, NETC_TMR_OFF_L, tmr_off_l);
> > +	netc_timer_wr(priv, NETC_TMR_OFF_H, tmr_off_h);
> > +}
> > +
> > +static u64 netc_timer_cur_time_read(struct netc_timer *priv)
> > +{
> > +	u32 time_h, time_l;
> > +	u64 ns;
> > +
> > +	time_l = netc_timer_rd(priv, NETC_TMR_CUR_TIME_L);
> > +	time_h = netc_timer_rd(priv, NETC_TMR_CUR_TIME_H);
> > +	ns = (u64)time_h << 32 | time_l;
> 
> I assume that the high part is latched after reading low part, but would like
> you confirm it and put a comment as you did for counter read.
> 

Yes, for the current implementation, we should reads from the TMR_CNT_L
register copy the entire 64-bit clock time into the TMR_CNT_H/L shadow
registers. I will add a comment here, thanks.

> > +
> > +	return ns;
> > +}
> > +
> > +static void netc_timer_alarm_write(struct netc_timer *priv,
> > +				   u64 alarm, int index)
> > +{
> > +	u32 alarm_h = upper_32_bits(alarm);
> > +	u32 alarm_l = lower_32_bits(alarm);
> > +
> > +	netc_timer_wr(priv, NETC_TMR_ALARM_L(index), alarm_l);
> > +	netc_timer_wr(priv, NETC_TMR_ALARM_H(index), alarm_h);
> > +}
> > +
> > +static void netc_timer_adjust_period(struct netc_timer *priv, u64 period)
> > +{
> > +	u32 fractional_period = lower_32_bits(period);
> > +	u32 integral_period = upper_32_bits(period);
> > +	u32 tmr_ctrl, old_tmr_ctrl;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&priv->lock, flags);
> > +
> > +	old_tmr_ctrl = netc_timer_rd(priv, NETC_TMR_CTRL);
> > +	tmr_ctrl = u32_replace_bits(old_tmr_ctrl, integral_period,
> > +				    TMR_CTRL_TCLK_PERIOD);
> > +	if (tmr_ctrl != old_tmr_ctrl)
> > +		netc_timer_wr(priv, NETC_TMR_CTRL, tmr_ctrl);
> > +
> > +	netc_timer_wr(priv, NETC_TMR_ADD, fractional_period);
> > +
> > +	spin_unlock_irqrestore(&priv->lock, flags);
> > +}
> > +
> > +static int netc_timer_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
> > +{
> > +	struct netc_timer *priv = ptp_to_netc_timer(ptp);
> > +	u64 new_period;
> > +
> > +	if (!scaled_ppm)
> > +		return 0;
> 
> why do you ignore value of 0 here? the adjustment will not happen if for some
> reasons the offset will be aligned after previous adjustment. And there will be
> inconsistency between hardware value and the last stored value in software.
> 

You're right, I wasn't aware of that, thank you.

> > +
> > +	new_period = adjust_by_scaled_ppm(priv->period, scaled_ppm);
> > +	netc_timer_adjust_period(priv, new_period);
> > +
> > +	return 0;
> > +}
> > +
> > +int netc_timer_get_phc_index(struct pci_dev *timer_pdev)
> > +{
> > +	struct netc_timer *priv;
> > +
> > +	if (!timer_pdev)
> > +		return -ENODEV;
> 
> I'm not sure, but looks like this should never happen. Could you please explain
> what are protecting from? If it's just safety, then it's better to remove the
> check and let the kernel crash to figure out wrong usage.

This function is exported for enetc driver to use, and the enetc supports SR-IOV,
if the VF driver and Timer driver run in the same OS, actually, the VF driver can
also support PTP synchronization. But if the VF driver runs in a guest OS, then the
VF driver cannot get the PCIe device of Timer, so the pointer will be NULL. This is
one of the use cases, there are other use cases that Timer is not available, so we
put this check inside this function.

> > +
> > +	priv = pci_get_drvdata(timer_pdev);
> > +	if (!priv)
> > +		return -EINVAL;
> > +
> > +	return priv->phc_index;
> > +}

Re: [PATCH net-next 02/12] ptp: netc: add NETC Timer PTP driver support

[kernel test robot]

Hi Wei,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Wei-Fang/dt-bindings-ptp-add-bindings-for-NETC-Timer/20250711-152311
base:   net-next/main
patch link:    https://lore.kernel.org/r/20250711065748.250159-3-wei.fang%40nxp.com
patch subject: [PATCH net-next 02/12] ptp: netc: add NETC Timer PTP driver support
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20250712/202507121901.cz1bRBUf-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250712/202507121901.cz1bRBUf-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/202507121901.cz1bRBUf-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/ptp/ptp_netc.c: In function 'netc_timer_adjust_period':
>> drivers/ptp/ptp_netc.c:161:20: error: implicit declaration of function 'u32_replace_bits' [-Wimplicit-function-declaration]
     161 |         tmr_ctrl = u32_replace_bits(old_tmr_ctrl, integral_period,
         |                    ^~~~~~~~~~~~~~~~


vim +/u32_replace_bits +161 drivers/ptp/ptp_netc.c

   150	
   151	static void netc_timer_adjust_period(struct netc_timer *priv, u64 period)
   152	{
   153		u32 fractional_period = lower_32_bits(period);
   154		u32 integral_period = upper_32_bits(period);
   155		u32 tmr_ctrl, old_tmr_ctrl;
   156		unsigned long flags;
   157	
   158		spin_lock_irqsave(&priv->lock, flags);
   159	
   160		old_tmr_ctrl = netc_timer_rd(priv, NETC_TMR_CTRL);
 > 161		tmr_ctrl = u32_replace_bits(old_tmr_ctrl, integral_period,
   162					    TMR_CTRL_TCLK_PERIOD);
   163		if (tmr_ctrl != old_tmr_ctrl)
   164			netc_timer_wr(priv, NETC_TMR_CTRL, tmr_ctrl);
   165	
   166		netc_timer_wr(priv, NETC_TMR_ADD, fractional_period);
   167	
   168		spin_unlock_irqrestore(&priv->lock, flags);
   169	}
   170	

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

Re: [PATCH net-next 02/12] ptp: netc: add NETC Timer PTP driver support

[Krzysztof Kozlowski]

On 11/07/2025 08:57, Wei Fang wrote:
> +
> +static void netc_timer_get_source_clk(struct netc_timer *priv)
> +{
> +	struct device *dev = &priv->pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	const char *clk_name = NULL;
> +	u64 ns = NSEC_PER_SEC;
> +
> +	if (!np)
> +		goto select_system_clk;
> +
> +	of_property_read_string(np, "clock-names", &clk_name);
> +	if (clk_name) {
> +		priv->src_clk = devm_clk_get_optional(dev, clk_name);
> +		if (IS_ERR_OR_NULL(priv->src_clk)) {
> +			dev_warn(dev, "Failed to get source clock\n");

No, look how deferred probe is handled.

This is really poor style of coding clk_get.


> +			priv->src_clk = NULL;
> +			goto select_system_clk;
> +		}
> +
> +		priv->clk_freq = clk_get_rate(priv->src_clk);
> +		if (!strcmp(clk_name, "system")) {
> +			/* There is a 1/2 divider */
> +			priv->clk_freq /= 2;
> +			priv->clk_select = NETC_TMR_SYSTEM_CLK;
> +		} else if (!strcmp(clk_name, "ccm_timer")) {
> +			priv->clk_select = NETC_TMR_CCM_TIMER1;
> +		} else if (!strcmp(clk_name, "ext_1588")) {
> +			priv->clk_select = NETC_TMR_EXT_OSC;
> +		} else {
> +			dev_warn(dev, "Unknown clock source\n");
> +			priv->src_clk = NULL;
> +			goto select_system_clk;
> +		}
> +
> +		goto cal_clk_period;


Why are you duplicating nxp,pps-channel logic?


Best regards,
Krzysztof

RE: [PATCH net-next 02/12] ptp: netc: add NETC Timer PTP driver support

[Wei Fang]

> On 11/07/2025 08:57, Wei Fang wrote:
> > +
> > +static void netc_timer_get_source_clk(struct netc_timer *priv) {
> > +	struct device *dev = &priv->pdev->dev;
> > +	struct device_node *np = dev->of_node;
> > +	const char *clk_name = NULL;
> > +	u64 ns = NSEC_PER_SEC;
> > +
> > +	if (!np)
> > +		goto select_system_clk;
> > +
> > +	of_property_read_string(np, "clock-names", &clk_name);
> > +	if (clk_name) {
> > +		priv->src_clk = devm_clk_get_optional(dev, clk_name);
> > +		if (IS_ERR_OR_NULL(priv->src_clk)) {
> > +			dev_warn(dev, "Failed to get source clock\n");
> 
> No, look how deferred probe is handled.
> 
> This is really poor style of coding clk_get.
> 
> 
> > +			priv->src_clk = NULL;
> > +			goto select_system_clk;
> > +		}
> > +
> > +		priv->clk_freq = clk_get_rate(priv->src_clk);
> > +		if (!strcmp(clk_name, "system")) {
> > +			/* There is a 1/2 divider */
> > +			priv->clk_freq /= 2;
> > +			priv->clk_select = NETC_TMR_SYSTEM_CLK;
> > +		} else if (!strcmp(clk_name, "ccm_timer")) {
> > +			priv->clk_select = NETC_TMR_CCM_TIMER1;
> > +		} else if (!strcmp(clk_name, "ext_1588")) {
> > +			priv->clk_select = NETC_TMR_EXT_OSC;
> > +		} else {
> > +			dev_warn(dev, "Unknown clock source\n");
> > +			priv->src_clk = NULL;
> > +			goto select_system_clk;
> > +		}
> > +
> > +		goto cal_clk_period;
> 
> 
> Why are you duplicating nxp,pps-channel logic?

I did not add "nxp,pps-channel" logic in this patch, did you misread
the patch?

[PATCH net-next 03/12] ptp: netc: add PPS support

[Wei Fang]

The NETC Timer has three channels, any one of which can generate the PPS
signal, since the kernel does not provide a parameter for setting the PPS
signal channel. And this is also related to the board design. Different
boards may choose different channels to output PPS signals. Therefore,
the "nxp,pps-channel" property is used to indicate which channel the
current board uses to output the PPS signal.

In addition, if there is a time drift when PPS is enabled, the PPS signal
won't be generated at an integral second of PHC. Based on the suggestion
from design team, it is better to disable FIPER before adjusting the
hardware time and then rearm ALARM after the time adjustment to make the
next PPS signal be generated at an integral second of PHC. Finally,
re-enable FIPER.

Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/ptp/ptp_netc.c | 194 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 191 insertions(+), 3 deletions(-)

diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
index 87d456fcadfd..abd637dab83b 100644
--- a/drivers/ptp/ptp_netc.c
+++ b/drivers/ptp/ptp_netc.c
@@ -23,6 +23,8 @@
 #define  TMR_ALARM1P			BIT(31)
 
 #define NETC_TMR_TEVENT			0x0084
+#define  TMR_TEVNET_PPEN(i)		BIT(7 - (i))
+#define  TMR_TEVENT_PPEN_ALL		GENMASK(7, 5)
 #define  TMR_TEVENT_ALM1EN		BIT(16)
 #define  TMR_TEVENT_ALM2EN		BIT(17)
 
@@ -38,9 +40,15 @@
 #define NETC_TMR_ALARM_L(i)		(0x00b8 + (i) * 8)
 #define NETC_TMR_ALARM_H(i)		(0x00bc + (i) * 8)
 
+/* i = 0, 1, 2. i indicates the index of TMR_FIPER. */
+#define NETC_TMR_FIPER(i)		(0x00d0 + (i) * 4)
+
 #define NETC_TMR_FIPER_CTRL		0x00dc
 #define  FIPER_CTRL_DIS(i)		(BIT(7) << (i) * 8)
 #define  FIPER_CTRL_PG(i)		(BIT(6) << (i) * 8)
+#define  FIPER_CTRL_FS_ALARM(i)		(BIT(5) << (i) * 8)
+#define  FIPER_CTRL_PW(i)		(GENMASK(4, 0) << (i) * 8)
+#define  FIPER_CTRL_SET_PW(i, v)	(((v) & GENMASK(4, 0)) << 8 * (i))
 
 #define NETC_TMR_CUR_TIME_L		0x00f0
 #define NETC_TMR_CUR_TIME_H		0x00f4
@@ -50,6 +58,9 @@
 #define NETC_TMR_FIPER_NUM		3
 #define NETC_TMR_DEFAULT_PRSC		2
 #define NETC_TMR_DEFAULT_ALARM		GENMASK_ULL(63, 0)
+#define NETC_TMR_DEFAULT_PPS_CHANNEL	0
+#define NETC_TMR_DEFAULT_FIPER		GENMASK(31, 0)
+#define NETC_TMR_FIPER_MAX_PW		GENMASK(4, 0)
 
 /* 1588 timer reference clock source select */
 #define NETC_TMR_CCM_TIMER1		0 /* enet_timer1_clk_root, from CCM */
@@ -74,6 +85,8 @@ struct netc_timer {
 	u64 period;
 
 	int irq;
+	u8 pps_channel;
+	bool pps_enabled;
 };
 
 #define netc_timer_rd(p, o)		netc_read((p)->base + (o))
@@ -148,6 +161,142 @@ static void netc_timer_alarm_write(struct netc_timer *priv,
 	netc_timer_wr(priv, NETC_TMR_ALARM_H(index), alarm_h);
 }
 
+static u32 netc_timer_get_integral_period(struct netc_timer *priv)
+{
+	u32 tmr_ctrl, integral_period;
+
+	tmr_ctrl = netc_timer_rd(priv, NETC_TMR_CTRL);
+	integral_period = FIELD_GET(TMR_CTRL_TCLK_PERIOD, tmr_ctrl);
+
+	return integral_period;
+}
+
+static u32 netc_timer_calculate_fiper_pw(struct netc_timer *priv,
+					 u32 fiper)
+{
+	u64 divisor, pulse_width;
+
+	/* Set the FIPER pulse width to half FIPER interval by default.
+	 * pulse_width = (fiper / 2) / TMR_GCLK_period,
+	 * TMR_GCLK_period = NSEC_PER_SEC / TMR_GCLK_freq,
+	 * TMR_GCLK_freq = (clk_freq / oclk_prsc) Hz,
+	 * so pulse_width = fiper * clk_freq / (2 * NSEC_PER_SEC * oclk_prsc).
+	 */
+	divisor = mul_u32_u32(2000000000U, priv->oclk_prsc);
+	pulse_width = div64_u64(mul_u32_u32(fiper, priv->clk_freq), divisor);
+
+	/* The FIPER_PW field only has 5 bits, need to update oclk_prsc */
+	if (pulse_width > NETC_TMR_FIPER_MAX_PW)
+		pulse_width = NETC_TMR_FIPER_MAX_PW;
+
+	return pulse_width;
+}
+
+static void netc_timer_set_pps_alarm(struct netc_timer *priv, int channel,
+				     u32 integral_period)
+{
+	u64 alarm;
+
+	/* Get the alarm value */
+	alarm = netc_timer_cur_time_read(priv) +  NSEC_PER_MSEC;
+	alarm = roundup_u64(alarm, NSEC_PER_SEC);
+	alarm = roundup_u64(alarm, integral_period);
+
+	netc_timer_alarm_write(priv, alarm, 0);
+}
+
+static int netc_timer_enable_pps(struct netc_timer *priv,
+				 struct ptp_clock_request *rq, int on)
+{
+	u32 tmr_emask, fiper, fiper_ctrl;
+	u8 channel = priv->pps_channel;
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	tmr_emask = netc_timer_rd(priv, NETC_TMR_TEMASK);
+	fiper_ctrl = netc_timer_rd(priv, NETC_TMR_FIPER_CTRL);
+
+	if (on) {
+		u32 integral_period, fiper_pw;
+
+		if (priv->pps_enabled)
+			goto unlock_spinlock;
+
+		integral_period = netc_timer_get_integral_period(priv);
+		fiper = NSEC_PER_SEC - integral_period;
+		fiper_pw = netc_timer_calculate_fiper_pw(priv, fiper);
+		fiper_ctrl &= ~(FIPER_CTRL_DIS(channel) | FIPER_CTRL_PW(channel) |
+				FIPER_CTRL_FS_ALARM(channel));
+		fiper_ctrl |= FIPER_CTRL_SET_PW(channel, fiper_pw);
+		tmr_emask |= TMR_TEVNET_PPEN(channel);
+		priv->pps_enabled = true;
+		netc_timer_set_pps_alarm(priv, channel, integral_period);
+	} else {
+		if (!priv->pps_enabled)
+			goto unlock_spinlock;
+
+		fiper = NETC_TMR_DEFAULT_FIPER;
+		tmr_emask &= ~TMR_TEVNET_PPEN(channel);
+		fiper_ctrl |= FIPER_CTRL_DIS(channel);
+		priv->pps_enabled = false;
+	}
+
+	netc_timer_wr(priv, NETC_TMR_TEMASK, tmr_emask);
+	netc_timer_wr(priv, NETC_TMR_FIPER(channel), fiper);
+	netc_timer_wr(priv, NETC_TMR_FIPER_CTRL, fiper_ctrl);
+
+unlock_spinlock:
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+static void netc_timer_disable_pps_fiper(struct netc_timer *priv)
+{
+	u32 fiper = NETC_TMR_DEFAULT_FIPER;
+	u8 channel = priv->pps_channel;
+	u32 fiper_ctrl;
+
+	if (!priv->pps_enabled)
+		return;
+
+	fiper_ctrl = netc_timer_rd(priv, NETC_TMR_FIPER_CTRL);
+	fiper_ctrl |= FIPER_CTRL_DIS(channel);
+	netc_timer_wr(priv, NETC_TMR_FIPER(channel), fiper);
+	netc_timer_wr(priv, NETC_TMR_FIPER_CTRL, fiper_ctrl);
+}
+
+static void netc_timer_enable_pps_fiper(struct netc_timer *priv)
+{
+	u32 fiper_ctrl, integral_period, fiper;
+	u8 channel = priv->pps_channel;
+
+	if (!priv->pps_enabled)
+		return;
+
+	integral_period = netc_timer_get_integral_period(priv);
+	fiper_ctrl = netc_timer_rd(priv, NETC_TMR_FIPER_CTRL);
+	fiper_ctrl &= ~FIPER_CTRL_DIS(channel);
+	fiper = NSEC_PER_SEC - integral_period;
+	netc_timer_set_pps_alarm(priv, channel, integral_period);
+	netc_timer_wr(priv, NETC_TMR_FIPER(channel), fiper);
+	netc_timer_wr(priv, NETC_TMR_FIPER_CTRL, fiper_ctrl);
+}
+
+static int netc_timer_enable(struct ptp_clock_info *ptp,
+			     struct ptp_clock_request *rq, int on)
+{
+	struct netc_timer *priv = ptp_to_netc_timer(ptp);
+
+	switch (rq->type) {
+	case PTP_CLK_REQ_PPS:
+		return netc_timer_enable_pps(priv, rq, on);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static void netc_timer_adjust_period(struct netc_timer *priv, u64 period)
 {
 	u32 fractional_period = lower_32_bits(period);
@@ -160,8 +309,11 @@ static void netc_timer_adjust_period(struct netc_timer *priv, u64 period)
 	old_tmr_ctrl = netc_timer_rd(priv, NETC_TMR_CTRL);
 	tmr_ctrl = u32_replace_bits(old_tmr_ctrl, integral_period,
 				    TMR_CTRL_TCLK_PERIOD);
-	if (tmr_ctrl != old_tmr_ctrl)
+	if (tmr_ctrl != old_tmr_ctrl) {
+		netc_timer_disable_pps_fiper(priv);
 		netc_timer_wr(priv, NETC_TMR_CTRL, tmr_ctrl);
+		netc_timer_enable_pps_fiper(priv);
+	}
 
 	netc_timer_wr(priv, NETC_TMR_ADD, fractional_period);
 
@@ -190,6 +342,8 @@ static int netc_timer_adjtime(struct ptp_clock_info *ptp, s64 delta)
 
 	spin_lock_irqsave(&priv->lock, flags);
 
+	netc_timer_disable_pps_fiper(priv);
+
 	tmr_off = netc_timer_offset_read(priv);
 	if (delta < 0 && tmr_off < abs(delta)) {
 		delta += tmr_off;
@@ -204,6 +358,8 @@ static int netc_timer_adjtime(struct ptp_clock_info *ptp, s64 delta)
 		netc_timer_offset_write(priv, tmr_off);
 	}
 
+	netc_timer_enable_pps_fiper(priv);
+
 	spin_unlock_irqrestore(&priv->lock, flags);
 
 	return 0;
@@ -238,8 +394,12 @@ static int netc_timer_settime64(struct ptp_clock_info *ptp,
 	unsigned long flags;
 
 	spin_lock_irqsave(&priv->lock, flags);
+
+	netc_timer_disable_pps_fiper(priv);
 	netc_timer_offset_write(priv, 0);
 	netc_timer_cnt_write(priv, ns);
+	netc_timer_enable_pps_fiper(priv);
+
 	spin_unlock_irqrestore(&priv->lock, flags);
 
 	return 0;
@@ -266,10 +426,12 @@ static const struct ptp_clock_info netc_timer_ptp_caps = {
 	.max_adj	= 500000000,
 	.n_alarm	= 2,
 	.n_pins		= 0,
+	.pps		= 1,
 	.adjfine	= netc_timer_adjfine,
 	.adjtime	= netc_timer_adjtime,
 	.gettimex64	= netc_timer_gettimex64,
 	.settime64	= netc_timer_settime64,
+	.enable		= netc_timer_enable,
 };
 
 static void netc_timer_init(struct netc_timer *priv)
@@ -422,14 +584,31 @@ static void netc_timer_get_source_clk(struct netc_timer *priv)
 	priv->period = div_u64(ns << 32, priv->clk_freq);
 }
 
-static void netc_timer_parse_dt(struct netc_timer *priv)
+static int netc_timer_parse_dt(struct netc_timer *priv)
 {
+	struct device *dev = &priv->pdev->dev;
+	struct device_node *np = dev->of_node;
+
+	if (!np || of_property_read_u8(np, "nxp,pps-channel",
+				       &priv->pps_channel))
+		priv->pps_channel = NETC_TMR_DEFAULT_PPS_CHANNEL;
+
+	if (priv->pps_channel >= NETC_TMR_FIPER_NUM) {
+		dev_err(dev, "pps_channel is %u, greater than %d\n",
+			priv->pps_channel, NETC_TMR_FIPER_NUM);
+
+		return -EINVAL;
+	}
+
 	netc_timer_get_source_clk(priv);
+
+	return 0;
 }
 
 static irqreturn_t netc_timer_isr(int irq, void *data)
 {
 	struct netc_timer *priv = data;
+	struct ptp_clock_event event;
 	u32 tmr_event, tmr_emask;
 	unsigned long flags;
 
@@ -445,6 +624,11 @@ static irqreturn_t netc_timer_isr(int irq, void *data)
 	if (tmr_event & TMR_TEVENT_ALM2EN)
 		netc_timer_alarm_write(priv, NETC_TMR_DEFAULT_ALARM, 1);
 
+	if (tmr_event & TMR_TEVENT_PPEN_ALL) {
+		event.type = PTP_CLOCK_PPS;
+		ptp_clock_event(priv->clock, &event);
+	}
+
 	/* Clear interrupts status */
 	netc_timer_wr(priv, NETC_TMR_TEVENT, tmr_event);
 
@@ -499,7 +683,11 @@ static int netc_timer_probe(struct pci_dev *pdev,
 		return err;
 
 	priv = pci_get_drvdata(pdev);
-	netc_timer_parse_dt(priv);
+	err = netc_timer_parse_dt(priv);
+	if (err) {
+		dev_err(dev, "Failed to parse DT node\n");
+		goto timer_pci_remove;
+	}
 
 	priv->caps = netc_timer_ptp_caps;
 	priv->oclk_prsc = NETC_TMR_DEFAULT_PRSC;
-- 
2.34.1

Re: [PATCH net-next 03/12] ptp: netc: add PPS support

[kernel test robot]

Hi Wei,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Wei-Fang/dt-bindings-ptp-add-bindings-for-NETC-Timer/20250711-152311
base:   net-next/main
patch link:    https://lore.kernel.org/r/20250711065748.250159-4-wei.fang%40nxp.com
patch subject: [PATCH net-next 03/12] ptp: netc: add PPS support
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20250712/202507122351.c9ut0TEq-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/20250712/202507122351.c9ut0TEq-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/202507122351.c9ut0TEq-lkp@intel.com/

All errors (new ones prefixed by >>):

>> ld.lld: error: undefined symbol: ptp_clock_event
   >>> referenced by ptp_netc.c
   >>>               drivers/ptp/ptp_netc.o:(netc_timer_isr) in archive vmlinux.a

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

[PATCH net-next 04/12] ptp: netc: add periodic pulse output support

[Wei Fang]

NETC Timer has three pulse channels, all of which support periodic pulse
output. Bind the channel to a ALARM register and then sets a future time
into the ALARM register. When the current time is greater than the ALARM
value, the FIPER register will be triggered to count down, and when the
count reaches 0, the pulse will be triggered. The PPS signal is also
implemented in this way. However, for i.MX95, only ALARM1 can be used for
periodic pulse output, and for i.MX943, ALARM1 and ALARM2 can be used for
periodic pulse output, but NETC Timer has three channels, so for i.MX95,
only one channel can work at the same time, and for i.MX943, at most two
channel can work at the same time. Otherwise, if multiple channels share
the same ALARM register, some channel pulses will not meet expectations.
Therefore, the current implementation does not allow multiple channels to
share the same ALARM register at the same time.

Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/ptp/ptp_netc.c | 281 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 250 insertions(+), 31 deletions(-)

diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
index abd637dab83b..5ea59bb20371 100644
--- a/drivers/ptp/ptp_netc.c
+++ b/drivers/ptp/ptp_netc.c
@@ -54,6 +54,10 @@
 #define NETC_TMR_CUR_TIME_H		0x00f4
 
 #define NETC_TMR_REGS_BAR		0
+#define NETC_GLOBAL_OFFSET		0x10000
+#define NETC_GLOBAL_IPBRR0		0xbf8
+#define  IPBRR0_IP_REV			GENMASK(15, 0)
+#define NETC_REV_4_1			0x0401
 
 #define NETC_TMR_FIPER_NUM		3
 #define NETC_TMR_DEFAULT_PRSC		2
@@ -61,6 +65,7 @@
 #define NETC_TMR_DEFAULT_PPS_CHANNEL	0
 #define NETC_TMR_DEFAULT_FIPER		GENMASK(31, 0)
 #define NETC_TMR_FIPER_MAX_PW		GENMASK(4, 0)
+#define NETC_TMR_ALARM_NUM		2
 
 /* 1588 timer reference clock source select */
 #define NETC_TMR_CCM_TIMER1		0 /* enet_timer1_clk_root, from CCM */
@@ -69,6 +74,19 @@
 
 #define NETC_TMR_SYSCLK_333M		333333333U
 
+enum netc_pp_type {
+	NETC_PP_PPS = 1,
+	NETC_PP_PEROUT,
+};
+
+struct netc_pp {
+	enum netc_pp_type type;
+	bool enabled;
+	int alarm_id;
+	u32 period; /* pulse period, ns */
+	u64 stime; /* start time, ns */
+};
+
 struct netc_timer {
 	void __iomem *base;
 	struct pci_dev *pdev;
@@ -86,7 +104,9 @@ struct netc_timer {
 
 	int irq;
 	u8 pps_channel;
-	bool pps_enabled;
+	u8 fs_alarm_num;
+	u8 fs_alarm_bitmap;
+	struct netc_pp pp[NETC_TMR_FIPER_NUM]; /* periodic pulse */
 };
 
 #define netc_timer_rd(p, o)		netc_read((p)->base + (o))
@@ -195,6 +215,7 @@ static u32 netc_timer_calculate_fiper_pw(struct netc_timer *priv,
 static void netc_timer_set_pps_alarm(struct netc_timer *priv, int channel,
 				     u32 integral_period)
 {
+	struct netc_pp *pp = &priv->pp[channel];
 	u64 alarm;
 
 	/* Get the alarm value */
@@ -202,26 +223,93 @@ static void netc_timer_set_pps_alarm(struct netc_timer *priv, int channel,
 	alarm = roundup_u64(alarm, NSEC_PER_SEC);
 	alarm = roundup_u64(alarm, integral_period);
 
-	netc_timer_alarm_write(priv, alarm, 0);
+	netc_timer_alarm_write(priv, alarm, pp->alarm_id);
+}
+
+static void netc_timer_set_perout_alarm(struct netc_timer *priv, int channel,
+					u32 integral_period)
+{
+	u64 cur_time = netc_timer_cur_time_read(priv);
+	struct netc_pp *pp = &priv->pp[channel];
+	u64 alarm, delta, min_time;
+	u32 period = pp->period;
+	u64 stime = pp->stime;
+
+	min_time = cur_time + NSEC_PER_MSEC + period;
+	if (stime < min_time) {
+		delta = min_time - stime;
+		stime += roundup_u64(delta, period);
+	}
+
+	alarm = roundup_u64(stime - period, integral_period);
+	netc_timer_alarm_write(priv, alarm, pp->alarm_id);
+}
+
+static int netc_timer_get_alarm_id(struct netc_timer *priv)
+{
+	int i;
+
+	for (i = 0; i < priv->fs_alarm_num; i++) {
+		if (!(priv->fs_alarm_bitmap & BIT(i))) {
+			priv->fs_alarm_bitmap |= BIT(i);
+			break;
+		}
+	}
+
+	return i;
+}
+
+static u64 netc_timer_get_gclk_period(struct netc_timer *priv)
+{
+	/* TMR_GCLK_freq = (clk_freq / oclk_prsc) Hz.
+	 * TMR_GCLK_period = NSEC_PER_SEC / TMR_GCLK_freq.
+	 * TMR_GCLK_period = (NSEC_PER_SEC * oclk_prsc) / clk_freq
+	 */
+
+	return div_u64(mul_u32_u32(NSEC_PER_SEC, priv->oclk_prsc),
+		       priv->clk_freq);
 }
 
 static int netc_timer_enable_pps(struct netc_timer *priv,
 				 struct ptp_clock_request *rq, int on)
 {
+	struct device *dev = &priv->pdev->dev;
 	u32 tmr_emask, fiper, fiper_ctrl;
 	u8 channel = priv->pps_channel;
 	unsigned long flags;
+	struct netc_pp *pp;
+	int alarm_id;
+	int err = 0;
 
 	spin_lock_irqsave(&priv->lock, flags);
 
+	pp = &priv->pp[channel];
+	if (pp->type == NETC_PP_PEROUT) {
+		dev_err(dev, "FIPER%u is being used for PEROUT\n", channel);
+		err = -EBUSY;
+		goto unlock_spinlock;
+	}
+
 	tmr_emask = netc_timer_rd(priv, NETC_TMR_TEMASK);
 	fiper_ctrl = netc_timer_rd(priv, NETC_TMR_FIPER_CTRL);
 
 	if (on) {
 		u32 integral_period, fiper_pw;
 
-		if (priv->pps_enabled)
+		if (pp->enabled)
+			goto unlock_spinlock;
+
+		alarm_id = netc_timer_get_alarm_id(priv);
+		if (alarm_id == priv->fs_alarm_num) {
+			dev_err(dev, "No available ALARMs\n");
+			err = -EBUSY;
 			goto unlock_spinlock;
+		}
+
+		pp->enabled = true;
+		pp->type = NETC_PP_PPS;
+		pp->alarm_id = alarm_id;
+		pp->period = NSEC_PER_SEC;
 
 		integral_period = netc_timer_get_integral_period(priv);
 		fiper = NSEC_PER_SEC - integral_period;
@@ -229,17 +317,19 @@ static int netc_timer_enable_pps(struct netc_timer *priv,
 		fiper_ctrl &= ~(FIPER_CTRL_DIS(channel) | FIPER_CTRL_PW(channel) |
 				FIPER_CTRL_FS_ALARM(channel));
 		fiper_ctrl |= FIPER_CTRL_SET_PW(channel, fiper_pw);
+		fiper_ctrl |= alarm_id ? FIPER_CTRL_FS_ALARM(channel) : 0;
 		tmr_emask |= TMR_TEVNET_PPEN(channel);
-		priv->pps_enabled = true;
 		netc_timer_set_pps_alarm(priv, channel, integral_period);
 	} else {
-		if (!priv->pps_enabled)
+		if (!pp->enabled)
 			goto unlock_spinlock;
 
+		priv->fs_alarm_bitmap &= ~BIT(pp->alarm_id);
+		memset(pp, 0, sizeof(*pp));
+
 		fiper = NETC_TMR_DEFAULT_FIPER;
 		tmr_emask &= ~TMR_TEVNET_PPEN(channel);
 		fiper_ctrl |= FIPER_CTRL_DIS(channel);
-		priv->pps_enabled = false;
 	}
 
 	netc_timer_wr(priv, NETC_TMR_TEMASK, tmr_emask);
@@ -249,38 +339,150 @@ static int netc_timer_enable_pps(struct netc_timer *priv,
 unlock_spinlock:
 	spin_unlock_irqrestore(&priv->lock, flags);
 
-	return 0;
+	return err;
 }
 
-static void netc_timer_disable_pps_fiper(struct netc_timer *priv)
+static int net_timer_enable_perout(struct netc_timer *priv,
+				   struct ptp_clock_request *rq, int on)
 {
-	u32 fiper = NETC_TMR_DEFAULT_FIPER;
-	u8 channel = priv->pps_channel;
-	u32 fiper_ctrl;
+	struct device *dev = &priv->pdev->dev;
+	u32 tmr_emask, fiper, fiper_ctrl;
+	u32 channel = rq->perout.index;
+	unsigned long flags;
+	struct netc_pp *pp;
+	int alarm_id;
+	int err = 0;
 
-	if (!priv->pps_enabled)
-		return;
+	spin_lock_irqsave(&priv->lock, flags);
+
+	pp = &priv->pp[channel];
+	if (pp->type == NETC_PP_PPS) {
+		dev_err(dev, "FIPER%u is being used for PPS\n", channel);
+		err = -EBUSY;
+		goto unlock_spinlock;
+	}
 
+	tmr_emask = netc_timer_rd(priv, NETC_TMR_TEMASK);
 	fiper_ctrl = netc_timer_rd(priv, NETC_TMR_FIPER_CTRL);
-	fiper_ctrl |= FIPER_CTRL_DIS(channel);
+	if (on) {
+		u64 period_ns, gclk_period, max_period, min_period;
+		struct timespec64 period, stime;
+		u32 integral_period, fiper_pw;
+
+		period.tv_sec = rq->perout.period.sec;
+		period.tv_nsec = rq->perout.period.nsec;
+		period_ns = timespec64_to_ns(&period);
+
+		integral_period = netc_timer_get_integral_period(priv);
+		max_period = (u64)NETC_TMR_DEFAULT_FIPER + integral_period;
+		gclk_period = netc_timer_get_gclk_period(priv);
+		min_period = gclk_period * 4 + integral_period;
+		if (period_ns > max_period || period_ns < min_period) {
+			dev_err(dev, "The period range is %llu ~ %llu\n",
+				min_period, max_period);
+			err = -EINVAL;
+			goto unlock_spinlock;
+		}
+
+		stime.tv_sec = rq->perout.start.sec;
+		stime.tv_nsec = rq->perout.start.nsec;
+
+		tmr_emask |= TMR_TEVNET_PPEN(channel);
+
+		/* Set to desired FIPER interval in ns - TCLK_PERIOD */
+		fiper = period_ns - integral_period;
+		fiper_pw = netc_timer_calculate_fiper_pw(priv, fiper);
+
+		if (pp->enabled) {
+			alarm_id = pp->alarm_id;
+		} else {
+			alarm_id = netc_timer_get_alarm_id(priv);
+			if (alarm_id == priv->fs_alarm_num) {
+				dev_err(dev, "No available ALARMs\n");
+				err = -EBUSY;
+				goto unlock_spinlock;
+			}
+
+			pp->type = NETC_PP_PEROUT;
+			pp->enabled = true;
+			pp->alarm_id = alarm_id;
+		}
+
+		pp->stime = timespec64_to_ns(&stime);
+		pp->period = period_ns;
+
+		fiper_ctrl &= ~(FIPER_CTRL_DIS(channel) | FIPER_CTRL_PW(channel) |
+				FIPER_CTRL_FS_ALARM(channel));
+		fiper_ctrl |= FIPER_CTRL_SET_PW(channel, fiper_pw);
+		fiper_ctrl |= alarm_id ? FIPER_CTRL_FS_ALARM(channel) : 0;
+
+		netc_timer_set_perout_alarm(priv, channel, integral_period);
+	} else {
+		if (!pp->enabled)
+			goto unlock_spinlock;
+
+		tmr_emask &= ~TMR_TEVNET_PPEN(channel);
+		fiper = NETC_TMR_DEFAULT_FIPER;
+		fiper_ctrl |= FIPER_CTRL_DIS(channel);
+
+		alarm_id = pp->alarm_id;
+		netc_timer_alarm_write(priv, NETC_TMR_DEFAULT_ALARM, alarm_id);
+		priv->fs_alarm_bitmap &= ~BIT(alarm_id);
+		memset(pp, 0, sizeof(*pp));
+	}
+
+	netc_timer_wr(priv, NETC_TMR_TEMASK, tmr_emask);
 	netc_timer_wr(priv, NETC_TMR_FIPER(channel), fiper);
 	netc_timer_wr(priv, NETC_TMR_FIPER_CTRL, fiper_ctrl);
+
+unlock_spinlock:
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return err;
 }
 
-static void netc_timer_enable_pps_fiper(struct netc_timer *priv)
+static void netc_timer_disable_fiper(struct netc_timer *priv)
 {
-	u32 fiper_ctrl, integral_period, fiper;
-	u8 channel = priv->pps_channel;
+	u32 fiper_ctrl = netc_timer_rd(priv, NETC_TMR_FIPER_CTRL);
+	int i;
 
-	if (!priv->pps_enabled)
-		return;
+	for (i = 0; i < NETC_TMR_FIPER_NUM; i++) {
+		struct netc_pp *pp = &priv->pp[i];
+
+		if (!pp->enabled)
+			continue;
+
+		fiper_ctrl |= FIPER_CTRL_DIS(i);
+		netc_timer_wr(priv, NETC_TMR_FIPER(i), NETC_TMR_DEFAULT_FIPER);
+	}
+
+	netc_timer_wr(priv, NETC_TMR_FIPER_CTRL, fiper_ctrl);
+}
+
+static void netc_timer_enable_fiper(struct netc_timer *priv)
+{
+	u32 integral_period = netc_timer_get_integral_period(priv);
+	u32 fiper_ctrl = netc_timer_rd(priv, NETC_TMR_FIPER_CTRL);
+	int i;
+
+	for (i = 0; i < NETC_TMR_FIPER_NUM; i++) {
+		struct netc_pp *pp = &priv->pp[i];
+		u32 fiper;
+
+		if (!pp->enabled)
+			continue;
+
+		fiper_ctrl &= ~FIPER_CTRL_DIS(i);
+
+		if (pp->type == NETC_PP_PPS)
+			netc_timer_set_pps_alarm(priv, i, integral_period);
+		else if (pp->type == NETC_PP_PEROUT)
+			netc_timer_set_perout_alarm(priv, i, integral_period);
+
+		fiper = pp->period - integral_period;
+		netc_timer_wr(priv, NETC_TMR_FIPER(i), fiper);
+	}
 
-	integral_period = netc_timer_get_integral_period(priv);
-	fiper_ctrl = netc_timer_rd(priv, NETC_TMR_FIPER_CTRL);
-	fiper_ctrl &= ~FIPER_CTRL_DIS(channel);
-	fiper = NSEC_PER_SEC - integral_period;
-	netc_timer_set_pps_alarm(priv, channel, integral_period);
-	netc_timer_wr(priv, NETC_TMR_FIPER(channel), fiper);
 	netc_timer_wr(priv, NETC_TMR_FIPER_CTRL, fiper_ctrl);
 }
 
@@ -292,6 +494,8 @@ static int netc_timer_enable(struct ptp_clock_info *ptp,
 	switch (rq->type) {
 	case PTP_CLK_REQ_PPS:
 		return netc_timer_enable_pps(priv, rq, on);
+	case PTP_CLK_REQ_PEROUT:
+		return net_timer_enable_perout(priv, rq, on);
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -310,9 +514,9 @@ static void netc_timer_adjust_period(struct netc_timer *priv, u64 period)
 	tmr_ctrl = u32_replace_bits(old_tmr_ctrl, integral_period,
 				    TMR_CTRL_TCLK_PERIOD);
 	if (tmr_ctrl != old_tmr_ctrl) {
-		netc_timer_disable_pps_fiper(priv);
+		netc_timer_disable_fiper(priv);
 		netc_timer_wr(priv, NETC_TMR_CTRL, tmr_ctrl);
-		netc_timer_enable_pps_fiper(priv);
+		netc_timer_enable_fiper(priv);
 	}
 
 	netc_timer_wr(priv, NETC_TMR_ADD, fractional_period);
@@ -342,7 +546,7 @@ static int netc_timer_adjtime(struct ptp_clock_info *ptp, s64 delta)
 
 	spin_lock_irqsave(&priv->lock, flags);
 
-	netc_timer_disable_pps_fiper(priv);
+	netc_timer_disable_fiper(priv);
 
 	tmr_off = netc_timer_offset_read(priv);
 	if (delta < 0 && tmr_off < abs(delta)) {
@@ -358,7 +562,7 @@ static int netc_timer_adjtime(struct ptp_clock_info *ptp, s64 delta)
 		netc_timer_offset_write(priv, tmr_off);
 	}
 
-	netc_timer_enable_pps_fiper(priv);
+	netc_timer_enable_fiper(priv);
 
 	spin_unlock_irqrestore(&priv->lock, flags);
 
@@ -395,10 +599,10 @@ static int netc_timer_settime64(struct ptp_clock_info *ptp,
 
 	spin_lock_irqsave(&priv->lock, flags);
 
-	netc_timer_disable_pps_fiper(priv);
+	netc_timer_disable_fiper(priv);
 	netc_timer_offset_write(priv, 0);
 	netc_timer_cnt_write(priv, ns);
-	netc_timer_enable_pps_fiper(priv);
+	netc_timer_enable_fiper(priv);
 
 	spin_unlock_irqrestore(&priv->lock, flags);
 
@@ -427,6 +631,7 @@ static const struct ptp_clock_info netc_timer_ptp_caps = {
 	.n_alarm	= 2,
 	.n_pins		= 0,
 	.pps		= 1,
+	.n_per_out	= 3,
 	.adjfine	= netc_timer_adjfine,
 	.adjtime	= netc_timer_adjtime,
 	.gettimex64	= netc_timer_gettimex64,
@@ -671,6 +876,15 @@ static void netc_timer_free_msix_irq(struct netc_timer *priv)
 	pci_free_irq_vectors(pdev);
 }
 
+static int netc_timer_get_global_ip_rev(struct netc_timer *priv)
+{
+	u32 val;
+
+	val = netc_timer_rd(priv, NETC_GLOBAL_OFFSET + NETC_GLOBAL_IPBRR0);
+
+	return val & IPBRR0_IP_REV;
+}
+
 static int netc_timer_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *id)
 {
@@ -700,6 +914,11 @@ static int netc_timer_probe(struct pci_dev *pdev,
 		goto timer_pci_remove;
 	}
 
+	if (netc_timer_get_global_ip_rev(priv) == NETC_REV_4_1)
+		priv->fs_alarm_num = 1;
+	else
+		priv->fs_alarm_num = NETC_TMR_ALARM_NUM;
+
 	err = netc_timer_init_msix_irq(priv);
 	if (err)
 		goto disable_clk;
-- 
2.34.1

[PATCH net-next 05/12] ptp: netc: add external trigger stamp support

[Wei Fang]

From: "F.S. Peng" <fushi.peng@nxp.com>

The NETC Timer is capable of recording the timestamp on receipt of an
external pulse on a GPIO pin. It supports two such external triggers.
The recorded value is saved in a 16 entry FIFO accessed by
TMR_ETTSa_H/L. An interrupt can be generated when the trigger occurs,
when the FIFO reaches a threshold, and if the FIFO overflows.

Signed-off-by: F.S. Peng <fushi.peng@nxp.com>
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/ptp/ptp_netc.c | 118 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 118 insertions(+)

diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
index 5ea59bb20371..b4c2f206752e 100644
--- a/drivers/ptp/ptp_netc.c
+++ b/drivers/ptp/ptp_netc.c
@@ -17,6 +17,8 @@
 #define NETC_TMR_CTRL			0x0080
 #define  TMR_CTRL_CK_SEL		GENMASK(1, 0)
 #define  TMR_CTRL_TE			BIT(2)
+#define  TMR_ETEP1			BIT(8)
+#define  TMR_ETEP2			BIT(9)
 #define  TMR_COMP_MODE			BIT(15)
 #define  TMR_CTRL_TCLK_PERIOD		GENMASK(25, 16)
 #define  TMR_CTRL_FS			BIT(28)
@@ -27,12 +29,26 @@
 #define  TMR_TEVENT_PPEN_ALL		GENMASK(7, 5)
 #define  TMR_TEVENT_ALM1EN		BIT(16)
 #define  TMR_TEVENT_ALM2EN		BIT(17)
+#define  TMR_TEVENT_ETS1_THREN		BIT(20)
+#define  TMR_TEVENT_ETS2_THREN		BIT(21)
+#define  TMR_TEVENT_ETS1EN		BIT(24)
+#define  TMR_TEVENT_ETS2EN		BIT(25)
+#define  TMR_TEVENT_ETS1_OVEN		BIT(28)
+#define  TMR_TEVENT_ETS2_OVEN		BIT(29)
+#define  TMR_TEVENT_ETS1		(TMR_TEVENT_ETS1_THREN | \
+					 TMR_TEVENT_ETS1EN | TMR_TEVENT_ETS1_OVEN)
+#define  TMR_TEVENT_ETS2		(TMR_TEVENT_ETS2_THREN | \
+					 TMR_TEVENT_ETS2EN | TMR_TEVENT_ETS2_OVEN)
 
 #define NETC_TMR_TEMASK			0x0088
+#define NETC_TMR_STAT			0x0094
+#define  TMR_STAT_ETS1_VLD		BIT(24)
+#define  TMR_STAT_ETS2_VLD		BIT(25)
 #define NETC_TMR_CNT_L			0x0098
 #define NETC_TMR_CNT_H			0x009c
 #define NETC_TMR_ADD			0x00a0
 #define NETC_TMR_PRSC			0x00a8
+#define NETC_TMR_ECTRL			0x00ac
 #define NETC_TMR_OFF_L			0x00b0
 #define NETC_TMR_OFF_H			0x00b4
 
@@ -50,6 +66,10 @@
 #define  FIPER_CTRL_PW(i)		(GENMASK(4, 0) << (i) * 8)
 #define  FIPER_CTRL_SET_PW(i, v)	(((v) & GENMASK(4, 0)) << 8 * (i))
 
+#define NETC_TMR_ETTS1_L		0x00e0
+#define NETC_TMR_ETTS1_H		0x00e4
+#define NETC_TMR_ETTS2_L		0x00e8
+#define NETC_TMR_ETTS2_H		0x00ec
 #define NETC_TMR_CUR_TIME_L		0x00f0
 #define NETC_TMR_CUR_TIME_H		0x00f4
 
@@ -66,6 +86,7 @@
 #define NETC_TMR_DEFAULT_FIPER		GENMASK(31, 0)
 #define NETC_TMR_FIPER_MAX_PW		GENMASK(4, 0)
 #define NETC_TMR_ALARM_NUM		2
+#define NETC_TMR_DEFAULT_ETTF_THR	7
 
 /* 1588 timer reference clock source select */
 #define NETC_TMR_CCM_TIMER1		0 /* enet_timer1_clk_root, from CCM */
@@ -441,6 +462,91 @@ static int net_timer_enable_perout(struct netc_timer *priv,
 	return err;
 }
 
+static void netc_timer_handle_etts_event(struct netc_timer *priv, int index,
+					 bool update_event)
+{
+	u32 regoff_l, regoff_h, etts_l, etts_h, ets_vld;
+	struct ptp_clock_event event;
+
+	switch (index) {
+	case 0:
+		ets_vld = TMR_STAT_ETS1_VLD;
+		regoff_l = NETC_TMR_ETTS1_L;
+		regoff_h = NETC_TMR_ETTS1_H;
+		break;
+	case 1:
+		ets_vld = TMR_STAT_ETS2_VLD;
+		regoff_l = NETC_TMR_ETTS2_L;
+		regoff_h = NETC_TMR_ETTS2_H;
+		break;
+	default:
+		return;
+	}
+
+	if (!(netc_timer_rd(priv, NETC_TMR_STAT) & ets_vld))
+		return;
+
+	do {
+		etts_l = netc_timer_rd(priv, regoff_l);
+		etts_h = netc_timer_rd(priv, regoff_h);
+	} while (netc_timer_rd(priv, NETC_TMR_STAT) & ets_vld);
+
+	if (update_event) {
+		event.type = PTP_CLOCK_EXTTS;
+		event.index = index;
+		event.timestamp = (u64)etts_h << 32;
+		event.timestamp |= etts_l;
+		ptp_clock_event(priv->clock, &event);
+	}
+}
+
+static int netc_timer_enable_extts(struct netc_timer *priv,
+				   struct ptp_clock_request *rq, int on)
+{
+	u32 ets_emask, tmr_emask, tmr_ctrl, ettp_bit;
+	unsigned long flags;
+
+	/* Reject requests to enable time stamping on both edges */
+	if ((rq->extts.flags & PTP_EXTTS_EDGES) == PTP_EXTTS_EDGES)
+		return -EOPNOTSUPP;
+
+	switch (rq->extts.index) {
+	case 0:
+		ettp_bit = TMR_ETEP1;
+		ets_emask = TMR_TEVENT_ETS1;
+		break;
+	case 1:
+		ettp_bit = TMR_ETEP2;
+		ets_emask = TMR_TEVENT_ETS2;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	netc_timer_handle_etts_event(priv, rq->extts.index, false);
+	tmr_emask = netc_timer_rd(priv, NETC_TMR_TEMASK);
+	if (on) {
+		tmr_ctrl = netc_timer_rd(priv, NETC_TMR_CTRL);
+		if (rq->extts.flags & PTP_FALLING_EDGE)
+			tmr_ctrl |= ettp_bit;
+		else
+			tmr_ctrl &= ~ettp_bit;
+
+		netc_timer_wr(priv, NETC_TMR_CTRL, tmr_ctrl);
+		tmr_emask |= ets_emask;
+	} else {
+		tmr_emask &= ~ets_emask;
+	}
+
+	netc_timer_wr(priv, NETC_TMR_TEMASK, tmr_emask);
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
 static void netc_timer_disable_fiper(struct netc_timer *priv)
 {
 	u32 fiper_ctrl = netc_timer_rd(priv, NETC_TMR_FIPER_CTRL);
@@ -496,6 +602,8 @@ static int netc_timer_enable(struct ptp_clock_info *ptp,
 		return netc_timer_enable_pps(priv, rq, on);
 	case PTP_CLK_REQ_PEROUT:
 		return net_timer_enable_perout(priv, rq, on);
+	case PTP_CLK_REQ_EXTTS:
+		return netc_timer_enable_extts(priv, rq, on);
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -632,6 +740,9 @@ static const struct ptp_clock_info netc_timer_ptp_caps = {
 	.n_pins		= 0,
 	.pps		= 1,
 	.n_per_out	= 3,
+	.n_ext_ts	= 2,
+	.supported_extts_flags = PTP_RISING_EDGE | PTP_FALLING_EDGE |
+				 PTP_STRICT_FLAGS,
 	.adjfine	= netc_timer_adjfine,
 	.adjtime	= netc_timer_adjtime,
 	.gettimex64	= netc_timer_gettimex64,
@@ -664,6 +775,7 @@ static void netc_timer_init(struct netc_timer *priv)
 		fiper_ctrl &= ~FIPER_CTRL_PG(i);
 	}
 	netc_timer_wr(priv, NETC_TMR_FIPER_CTRL, fiper_ctrl);
+	netc_timer_wr(priv, NETC_TMR_ECTRL, NETC_TMR_DEFAULT_ETTF_THR);
 
 	ktime_get_real_ts64(&now);
 	ns = timespec64_to_ns(&now);
@@ -834,6 +946,12 @@ static irqreturn_t netc_timer_isr(int irq, void *data)
 		ptp_clock_event(priv->clock, &event);
 	}
 
+	if (tmr_event & TMR_TEVENT_ETS1)
+		netc_timer_handle_etts_event(priv, 0, true);
+
+	if (tmr_event & TMR_TEVENT_ETS2)
+		netc_timer_handle_etts_event(priv, 1, true);
+
 	/* Clear interrupts status */
 	netc_timer_wr(priv, NETC_TMR_TEVENT, tmr_event);
 
-- 
2.34.1

[PATCH net-next 06/12] ptp: netc: add debugfs support to loop back pulse signal

[Wei Fang]

The NETC Timer supports to loop back the output pulse signal of Fiper-n
into Trigger-n input, so that we can leverage this feature to validate
some other features without external hardware support. For example, we
can use it to test external trigger stamp (EXTTS). And we can combine
EXTTS with loopback mode to check whether the generation time of PPS is
aligned with an integral second of PHC, or the periodic output signal
(PTP_CLK_REQ_PEROUT) whether is generated at the specified time. So add
the debugfs interfaces to enable the loopback mode of Fiper1 and Fiper2.

An example to test the generation time of PPS.

$ echo 1 > /sys/kernel/debug/netc_timer0/fiper1-loopback
$ echo 1 > /sys/class/ptp/ptp0/pps_enable
$ testptp -d /dev/ptp0 -e 3
external time stamp request okay
event index 0 at 108.000000018
event index 0 at 109.000000018
event index 0 at 110.000000018

An example to test the generation time of the periodic output signal.

$ echo 1 > /sys/kernel/debug/netc_timer0/fiper1-loopback
$ echo 0 260 0 1 500000000 > /sys/class/ptp/ptp0/period
$ testptp -d /dev/ptp0 -e 3
external time stamp request okay
event index 0 at 260.000000016
event index 0 at 261.500000015
event index 0 at 263.000000016

Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/ptp/ptp_netc.c | 119 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
index b4c2f206752e..7f3e401f51c5 100644
--- a/drivers/ptp/ptp_netc.c
+++ b/drivers/ptp/ptp_netc.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/debugfs.h>
 #include <linux/fsl/netc_global.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -21,6 +22,8 @@
 #define  TMR_ETEP2			BIT(9)
 #define  TMR_COMP_MODE			BIT(15)
 #define  TMR_CTRL_TCLK_PERIOD		GENMASK(25, 16)
+#define  TMR_CTRL_PP2L			BIT(26)
+#define  TMR_CTRL_PP1L			BIT(27)
 #define  TMR_CTRL_FS			BIT(28)
 #define  TMR_ALARM1P			BIT(31)
 
@@ -128,6 +131,7 @@ struct netc_timer {
 	u8 fs_alarm_num;
 	u8 fs_alarm_bitmap;
 	struct netc_pp pp[NETC_TMR_FIPER_NUM]; /* periodic pulse */
+	struct dentry *debugfs_root;
 };
 
 #define netc_timer_rd(p, o)		netc_read((p)->base + (o))
@@ -1003,6 +1007,119 @@ static int netc_timer_get_global_ip_rev(struct netc_timer *priv)
 	return val & IPBRR0_IP_REV;
 }
 
+static int netc_timer_get_fiper_loopback(struct netc_timer *priv,
+					 int fiper, u64 *val)
+{
+	unsigned long flags;
+	u32 tmr_ctrl;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	tmr_ctrl = netc_timer_rd(priv, NETC_TMR_CTRL);
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	switch (fiper) {
+	case 0:
+		*val = tmr_ctrl & TMR_CTRL_PP1L ? 1 : 0;
+		break;
+	case 1:
+		*val = tmr_ctrl & TMR_CTRL_PP2L ? 1 : 0;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int netc_timer_set_fiper_loopback(struct netc_timer *priv,
+					 int fiper, u64 val)
+{
+	unsigned long flags;
+	u32 tmr_ctrl;
+	int err = 0;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	tmr_ctrl = netc_timer_rd(priv, NETC_TMR_CTRL);
+	switch (fiper) {
+	case 0:
+		tmr_ctrl = u32_replace_bits(tmr_ctrl, val ? 1 : 0,
+					    TMR_CTRL_PP1L);
+		break;
+	case 1:
+		tmr_ctrl = u32_replace_bits(tmr_ctrl, val ? 1 : 0,
+					    TMR_CTRL_PP2L);
+		break;
+	default:
+		err = -EINVAL;
+	}
+
+	if (!err)
+		netc_timer_wr(priv, NETC_TMR_CTRL, tmr_ctrl);
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return err;
+}
+
+static int netc_timer_get_fiper1_loopback(void *data, u64 *val)
+{
+	struct netc_timer *priv = data;
+
+	return netc_timer_get_fiper_loopback(priv, 0, val);
+}
+
+static int netc_timer_set_fiper1_loopback(void *data, u64 val)
+{
+	struct netc_timer *priv = data;
+
+	return netc_timer_set_fiper_loopback(priv, 0, val);
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(netc_timer_fiper1_fops, netc_timer_get_fiper1_loopback,
+			 netc_timer_set_fiper1_loopback, "%llu\n");
+
+static int netc_timer_get_fiper2_loopback(void *data, u64 *val)
+{
+	struct netc_timer *priv = data;
+
+	return netc_timer_get_fiper_loopback(priv, 1, val);
+}
+
+static int netc_timer_set_fiper2_loopback(void *data, u64 val)
+{
+	struct netc_timer *priv = data;
+
+	return netc_timer_set_fiper_loopback(priv, 1, val);
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(netc_timer_fiper2_fops, netc_timer_get_fiper2_loopback,
+			 netc_timer_set_fiper2_loopback, "%llu\n");
+
+static void netc_timer_create_debugfs(struct netc_timer *priv)
+{
+	char debugfs_name[24];
+	struct dentry *root;
+
+	snprintf(debugfs_name, sizeof(debugfs_name), "netc_timer%d",
+		 priv->phc_index);
+	root = debugfs_create_dir(debugfs_name, NULL);
+	if (IS_ERR(root))
+		return;
+
+	priv->debugfs_root = root;
+	debugfs_create_file("fiper1-loopback", 0600, root, priv,
+			    &netc_timer_fiper1_fops);
+	debugfs_create_file("fiper2-loopback", 0600, root, priv,
+			    &netc_timer_fiper2_fops);
+}
+
+static void netc_timer_remove_debugfs(struct netc_timer *priv)
+{
+	debugfs_remove(priv->debugfs_root);
+	priv->debugfs_root = NULL;
+}
+
 static int netc_timer_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *id)
 {
@@ -1049,6 +1166,7 @@ static int netc_timer_probe(struct pci_dev *pdev,
 	}
 
 	priv->phc_index = ptp_clock_index(priv->clock);
+	netc_timer_create_debugfs(priv);
 
 	return 0;
 
@@ -1066,6 +1184,7 @@ static void netc_timer_remove(struct pci_dev *pdev)
 {
 	struct netc_timer *priv = pci_get_drvdata(pdev);
 
+	netc_timer_remove_debugfs(priv);
 	ptp_clock_unregister(priv->clock);
 	netc_timer_free_msix_irq(priv);
 	clk_disable_unprepare(priv->src_clk);
-- 
2.34.1

Re: [PATCH net-next 06/12] ptp: netc: add debugfs support to loop back pulse signal

[Andrew Lunn]

> +static void netc_timer_create_debugfs(struct netc_timer *priv)
> +{
> +	char debugfs_name[24];
> +	struct dentry *root;
> +
> +	snprintf(debugfs_name, sizeof(debugfs_name), "netc_timer%d",
> +		 priv->phc_index);
> +	root = debugfs_create_dir(debugfs_name, NULL);
> +	if (IS_ERR(root))
> +		return;

You should never check the return values from a debugfs_
calls. debugfs is full optional, and the driver should work without
it. debugfs will also happily accept a NULL or error code as a
parameter. So even if debugfs_create_dir() fails, keep going,
debugfs_create_file() won't explode.

	Andrew

RE: [PATCH net-next 06/12] ptp: netc: add debugfs support to loop back pulse signal

[Wei Fang]

> > +static void netc_timer_create_debugfs(struct netc_timer *priv)
> > +{
> > +	char debugfs_name[24];
> > +	struct dentry *root;
> > +
> > +	snprintf(debugfs_name, sizeof(debugfs_name), "netc_timer%d",
> > +		 priv->phc_index);
> > +	root = debugfs_create_dir(debugfs_name, NULL);
> > +	if (IS_ERR(root))
> > +		return;
> 
> You should never check the return values from a debugfs_
> calls. debugfs is full optional, and the driver should work without
> it. debugfs will also happily accept a NULL or error code as a
> parameter. So even if debugfs_create_dir() fails, keep going,
> debugfs_create_file() won't explode.
> 
You are right, I will remove the check, thanks

[PATCH net-next 07/12] MAINTAINERS: add NETC Timer PTP clock driver section

[Wei Fang]

Add a section entry for NXP NETC Timer PTP clock driver.

Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d1554f33d0ac..dacc5824dca6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18005,6 +18005,15 @@ F:	Documentation/devicetree/bindings/clock/imx*
 F:	drivers/clk/imx/
 F:	include/dt-bindings/clock/imx*
 
+NXP NETC TIMER PTP CLOCK DRIVER
+M:	Wei Fang <wei.fang@nxp.com>
+M:	Clark Wang <xiaoning.wang@nxp.com>
+L:	imx@lists.linux.dev
+L:	netdev@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/ptp/nxp,ptp-netc.yaml
+F:	drivers/ptp/ptp_netc.c
+
 NXP PF8100/PF8121A/PF8200 PMIC REGULATOR DEVICE DRIVER
 M:	Jagan Teki <jagan@amarulasolutions.com>
 S:	Maintained
-- 
2.34.1

[PATCH net-next 08/12] net: enetc: save the parsed information of PTP packet to skb->cb

[Wei Fang]

Currently, the Tx PTP packets are parsed twice in the enetc driver, once
in enetc_xmit() and once in enetc_map_tx_buffs(). The latter is duplicate
and is unnecessary, since the parsed information can be saved to skb->cb
so that enetc_map_tx_buffs() can get the previously parsed data from
skb->cb. Therefore, we add struct enetc_skb_cb as the format of the data
in the skb->cb buffer to save the parsed information of PTP packet.

Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 65 ++++++++++----------
 drivers/net/ethernet/freescale/enetc/enetc.h |  9 +++
 2 files changed, 43 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index e4287725832e..c1373163a096 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -225,13 +225,12 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 {
 	bool do_vlan, do_onestep_tstamp = false, do_twostep_tstamp = false;
 	struct enetc_ndev_priv *priv = netdev_priv(tx_ring->ndev);
+	struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
 	struct enetc_hw *hw = &priv->si->hw;
 	struct enetc_tx_swbd *tx_swbd;
 	int len = skb_headlen(skb);
 	union enetc_tx_bd temp_bd;
-	u8 msgtype, twostep, udp;
 	union enetc_tx_bd *txbd;
-	u16 offset1, offset2;
 	int i, count = 0;
 	skb_frag_t *frag;
 	unsigned int f;
@@ -280,16 +279,10 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 	count++;
 
 	do_vlan = skb_vlan_tag_present(skb);
-	if (skb->cb[0] & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
-		if (enetc_ptp_parse(skb, &udp, &msgtype, &twostep, &offset1,
-				    &offset2) ||
-		    msgtype != PTP_MSGTYPE_SYNC || twostep)
-			WARN_ONCE(1, "Bad packet for one-step timestamping\n");
-		else
-			do_onestep_tstamp = true;
-	} else if (skb->cb[0] & ENETC_F_TX_TSTAMP) {
+	if (enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP)
+		do_onestep_tstamp = true;
+	else if (enetc_cb->flag & ENETC_F_TX_TSTAMP)
 		do_twostep_tstamp = true;
-	}
 
 	tx_swbd->do_twostep_tstamp = do_twostep_tstamp;
 	tx_swbd->qbv_en = !!(priv->active_offloads & ENETC_F_QBV);
@@ -333,6 +326,8 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 		}
 
 		if (do_onestep_tstamp) {
+			u16 tstamp_off = enetc_cb->origin_tstamp_off;
+			u16 corr_off = enetc_cb->correction_off;
 			__be32 new_sec_l, new_nsec;
 			u32 lo, hi, nsec, val;
 			__be16 new_sec_h;
@@ -362,32 +357,32 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 			new_sec_h = htons((sec >> 32) & 0xffff);
 			new_sec_l = htonl(sec & 0xffffffff);
 			new_nsec = htonl(nsec);
-			if (udp) {
+			if (enetc_cb->udp) {
 				struct udphdr *uh = udp_hdr(skb);
 				__be32 old_sec_l, old_nsec;
 				__be16 old_sec_h;
 
-				old_sec_h = *(__be16 *)(data + offset2);
+				old_sec_h = *(__be16 *)(data + tstamp_off);
 				inet_proto_csum_replace2(&uh->check, skb, old_sec_h,
 							 new_sec_h, false);
 
-				old_sec_l = *(__be32 *)(data + offset2 + 2);
+				old_sec_l = *(__be32 *)(data + tstamp_off + 2);
 				inet_proto_csum_replace4(&uh->check, skb, old_sec_l,
 							 new_sec_l, false);
 
-				old_nsec = *(__be32 *)(data + offset2 + 6);
+				old_nsec = *(__be32 *)(data + tstamp_off + 6);
 				inet_proto_csum_replace4(&uh->check, skb, old_nsec,
 							 new_nsec, false);
 			}
 
-			*(__be16 *)(data + offset2) = new_sec_h;
-			*(__be32 *)(data + offset2 + 2) = new_sec_l;
-			*(__be32 *)(data + offset2 + 6) = new_nsec;
+			*(__be16 *)(data + tstamp_off) = new_sec_h;
++			*(__be32 *)(data + tstamp_off + 2) = new_sec_l;
++			*(__be32 *)(data + tstamp_off + 6) = new_nsec;
 
 			/* Configure single-step register */
 			val = ENETC_PM0_SINGLE_STEP_EN;
-			val |= ENETC_SET_SINGLE_STEP_OFFSET(offset1);
-			if (udp)
+			val |= ENETC_SET_SINGLE_STEP_OFFSET(corr_off);
+			if (enetc_cb->udp)
 				val |= ENETC_PM0_SINGLE_STEP_CH;
 
 			enetc_port_mac_wr(priv->si, ENETC_PM0_SINGLE_STEP,
@@ -938,12 +933,13 @@ static int enetc_map_tx_tso_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb
 static netdev_tx_t enetc_start_xmit(struct sk_buff *skb,
 				    struct net_device *ndev)
 {
+	struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
 	struct enetc_ndev_priv *priv = netdev_priv(ndev);
 	struct enetc_bdr *tx_ring;
 	int count;
 
 	/* Queue one-step Sync packet if already locked */
-	if (skb->cb[0] & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
+	if (enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
 		if (test_and_set_bit_lock(ENETC_TX_ONESTEP_TSTAMP_IN_PROGRESS,
 					  &priv->flags)) {
 			skb_queue_tail(&priv->tx_skbs, skb);
@@ -1005,24 +1001,29 @@ static netdev_tx_t enetc_start_xmit(struct sk_buff *skb,
 
 netdev_tx_t enetc_xmit(struct sk_buff *skb, struct net_device *ndev)
 {
+	struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
 	struct enetc_ndev_priv *priv = netdev_priv(ndev);
 	u8 udp, msgtype, twostep;
 	u16 offset1, offset2;
 
-	/* Mark tx timestamp type on skb->cb[0] if requires */
+	/* Mark tx timestamp type on enetc_cb->flag if requires */
 	if ((skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
-	    (priv->active_offloads & ENETC_F_TX_TSTAMP_MASK)) {
-		skb->cb[0] = priv->active_offloads & ENETC_F_TX_TSTAMP_MASK;
-	} else {
-		skb->cb[0] = 0;
-	}
+	    (priv->active_offloads & ENETC_F_TX_TSTAMP_MASK))
+		enetc_cb->flag = priv->active_offloads & ENETC_F_TX_TSTAMP_MASK;
+	else
+		enetc_cb->flag = 0;
 
 	/* Fall back to two-step timestamp if not one-step Sync packet */
-	if (skb->cb[0] & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
+	if (enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
 		if (enetc_ptp_parse(skb, &udp, &msgtype, &twostep,
 				    &offset1, &offset2) ||
-		    msgtype != PTP_MSGTYPE_SYNC || twostep != 0)
-			skb->cb[0] = ENETC_F_TX_TSTAMP;
+		    msgtype != PTP_MSGTYPE_SYNC || twostep != 0) {
+			enetc_cb->flag = ENETC_F_TX_TSTAMP;
+		} else {
+			enetc_cb->udp = !!udp;
+			enetc_cb->correction_off = offset1;
+			enetc_cb->origin_tstamp_off = offset2;
+		}
 	}
 
 	return enetc_start_xmit(skb, ndev);
@@ -1214,7 +1215,9 @@ static bool enetc_clean_tx_ring(struct enetc_bdr *tx_ring, int napi_budget)
 		if (xdp_frame) {
 			xdp_return_frame(xdp_frame);
 		} else if (skb) {
-			if (unlikely(skb->cb[0] & ENETC_F_TX_ONESTEP_SYNC_TSTAMP)) {
+			struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
+
+			if (unlikely(enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP)) {
 				/* Start work to release lock for next one-step
 				 * timestamping packet. And send one skb in
 				 * tx_skbs queue if has.
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h
index 62e8ee4d2f04..ce3fed95091b 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc.h
@@ -54,6 +54,15 @@ struct enetc_tx_swbd {
 	u8 qbv_en:1;
 };
 
+struct enetc_skb_cb {
+	u8 flag;
+	bool udp;
+	u16 correction_off;
+	u16 origin_tstamp_off;
+};
+
+#define ENETC_SKB_CB(skb) ((struct enetc_skb_cb *)((skb)->cb))
+
 struct enetc_lso_t {
 	bool	ipv6;
 	bool	tcp;
-- 
2.34.1

Re: [PATCH net-next 08/12] net: enetc: save the parsed information of PTP packet to skb->cb

[Vadim Fedorenko]

On 11.07.2025 07:57, Wei Fang wrote:
> Currently, the Tx PTP packets are parsed twice in the enetc driver, once
> in enetc_xmit() and once in enetc_map_tx_buffs(). The latter is duplicate
> and is unnecessary, since the parsed information can be saved to skb->cb
> so that enetc_map_tx_buffs() can get the previously parsed data from
> skb->cb. Therefore, we add struct enetc_skb_cb as the format of the data
> in the skb->cb buffer to save the parsed information of PTP packet.
> 
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
>   drivers/net/ethernet/freescale/enetc/enetc.c | 65 ++++++++++----------
>   drivers/net/ethernet/freescale/enetc/enetc.h |  9 +++
>   2 files changed, 43 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> index e4287725832e..c1373163a096 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -225,13 +225,12 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
>   {
>   	bool do_vlan, do_onestep_tstamp = false, do_twostep_tstamp = false;
>   	struct enetc_ndev_priv *priv = netdev_priv(tx_ring->ndev);
> +	struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
>   	struct enetc_hw *hw = &priv->si->hw;
>   	struct enetc_tx_swbd *tx_swbd;
>   	int len = skb_headlen(skb);
>   	union enetc_tx_bd temp_bd;
> -	u8 msgtype, twostep, udp;
>   	union enetc_tx_bd *txbd;
> -	u16 offset1, offset2;
>   	int i, count = 0;
>   	skb_frag_t *frag;
>   	unsigned int f;
> @@ -280,16 +279,10 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
>   	count++;
>   
>   	do_vlan = skb_vlan_tag_present(skb);
> -	if (skb->cb[0] & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
> -		if (enetc_ptp_parse(skb, &udp, &msgtype, &twostep, &offset1,
> -				    &offset2) ||
> -		    msgtype != PTP_MSGTYPE_SYNC || twostep)
> -			WARN_ONCE(1, "Bad packet for one-step timestamping\n");
> -		else
> -			do_onestep_tstamp = true;
> -	} else if (skb->cb[0] & ENETC_F_TX_TSTAMP) {
> +	if (enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP)
> +		do_onestep_tstamp = true;
> +	else if (enetc_cb->flag & ENETC_F_TX_TSTAMP)
>   		do_twostep_tstamp = true;
> -	}
>   
>   	tx_swbd->do_twostep_tstamp = do_twostep_tstamp;
>   	tx_swbd->qbv_en = !!(priv->active_offloads & ENETC_F_QBV);
> @@ -333,6 +326,8 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
>   		}
>   
>   		if (do_onestep_tstamp) {
> +			u16 tstamp_off = enetc_cb->origin_tstamp_off;
> +			u16 corr_off = enetc_cb->correction_off;
>   			__be32 new_sec_l, new_nsec;
>   			u32 lo, hi, nsec, val;
>   			__be16 new_sec_h;
> @@ -362,32 +357,32 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
>   			new_sec_h = htons((sec >> 32) & 0xffff);
>   			new_sec_l = htonl(sec & 0xffffffff);
>   			new_nsec = htonl(nsec);
> -			if (udp) {
> +			if (enetc_cb->udp) {
>   				struct udphdr *uh = udp_hdr(skb);
>   				__be32 old_sec_l, old_nsec;
>   				__be16 old_sec_h;
>   
> -				old_sec_h = *(__be16 *)(data + offset2);
> +				old_sec_h = *(__be16 *)(data + tstamp_off);
>   				inet_proto_csum_replace2(&uh->check, skb, old_sec_h,
>   							 new_sec_h, false);
>   
> -				old_sec_l = *(__be32 *)(data + offset2 + 2);
> +				old_sec_l = *(__be32 *)(data + tstamp_off + 2);
>   				inet_proto_csum_replace4(&uh->check, skb, old_sec_l,
>   							 new_sec_l, false);
>   
> -				old_nsec = *(__be32 *)(data + offset2 + 6);
> +				old_nsec = *(__be32 *)(data + tstamp_off + 6);
>   				inet_proto_csum_replace4(&uh->check, skb, old_nsec,
>   							 new_nsec, false);
>   			}
>   
> -			*(__be16 *)(data + offset2) = new_sec_h;
> -			*(__be32 *)(data + offset2 + 2) = new_sec_l;
> -			*(__be32 *)(data + offset2 + 6) = new_nsec;
> +			*(__be16 *)(data + tstamp_off) = new_sec_h;
> ++			*(__be32 *)(data + tstamp_off + 2) = new_sec_l;
> ++			*(__be32 *)(data + tstamp_off + 6) = new_nsec;

This looks like merge conflict artifact...

>   
>   			/* Configure single-step register */
>   			val = ENETC_PM0_SINGLE_STEP_EN;
> -			val |= ENETC_SET_SINGLE_STEP_OFFSET(offset1);
> -			if (udp)
> +			val |= ENETC_SET_SINGLE_STEP_OFFSET(corr_off);
> +			if (enetc_cb->udp)
>   				val |= ENETC_PM0_SINGLE_STEP_CH;
>   
>   			enetc_port_mac_wr(priv->si, ENETC_PM0_SINGLE_STEP,
> @@ -938,12 +933,13 @@ static int enetc_map_tx_tso_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb
>   static netdev_tx_t enetc_start_xmit(struct sk_buff *skb,
>   				    struct net_device *ndev)
>   {
> +	struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
>   	struct enetc_ndev_priv *priv = netdev_priv(ndev);
>   	struct enetc_bdr *tx_ring;
>   	int count;
>   
>   	/* Queue one-step Sync packet if already locked */
> -	if (skb->cb[0] & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
> +	if (enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
>   		if (test_and_set_bit_lock(ENETC_TX_ONESTEP_TSTAMP_IN_PROGRESS,
>   					  &priv->flags)) {
>   			skb_queue_tail(&priv->tx_skbs, skb);
> @@ -1005,24 +1001,29 @@ static netdev_tx_t enetc_start_xmit(struct sk_buff *skb,
>   
>   netdev_tx_t enetc_xmit(struct sk_buff *skb, struct net_device *ndev)
>   {
> +	struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
>   	struct enetc_ndev_priv *priv = netdev_priv(ndev);
>   	u8 udp, msgtype, twostep;
>   	u16 offset1, offset2;
>   
> -	/* Mark tx timestamp type on skb->cb[0] if requires */
> +	/* Mark tx timestamp type on enetc_cb->flag if requires */
>   	if ((skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
> -	    (priv->active_offloads & ENETC_F_TX_TSTAMP_MASK)) {
> -		skb->cb[0] = priv->active_offloads & ENETC_F_TX_TSTAMP_MASK;
> -	} else {
> -		skb->cb[0] = 0;
> -	}
> +	    (priv->active_offloads & ENETC_F_TX_TSTAMP_MASK))
> +		enetc_cb->flag = priv->active_offloads & ENETC_F_TX_TSTAMP_MASK;
> +	else
> +		enetc_cb->flag = 0;
>   
>   	/* Fall back to two-step timestamp if not one-step Sync packet */
> -	if (skb->cb[0] & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
> +	if (enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
>   		if (enetc_ptp_parse(skb, &udp, &msgtype, &twostep,
>   				    &offset1, &offset2) ||
> -		    msgtype != PTP_MSGTYPE_SYNC || twostep != 0)
> -			skb->cb[0] = ENETC_F_TX_TSTAMP;
> +		    msgtype != PTP_MSGTYPE_SYNC || twostep != 0) {
> +			enetc_cb->flag = ENETC_F_TX_TSTAMP;
> +		} else {
> +			enetc_cb->udp = !!udp;
> +			enetc_cb->correction_off = offset1;
> +			enetc_cb->origin_tstamp_off = offset2;
> +		}
>   	}
>   
>   	return enetc_start_xmit(skb, ndev);
> @@ -1214,7 +1215,9 @@ static bool enetc_clean_tx_ring(struct enetc_bdr *tx_ring, int napi_budget)
>   		if (xdp_frame) {
>   			xdp_return_frame(xdp_frame);
>   		} else if (skb) {
> -			if (unlikely(skb->cb[0] & ENETC_F_TX_ONESTEP_SYNC_TSTAMP)) {
> +			struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
> +
> +			if (unlikely(enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP)) {
>   				/* Start work to release lock for next one-step
>   				 * timestamping packet. And send one skb in
>   				 * tx_skbs queue if has.
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h
> index 62e8ee4d2f04..ce3fed95091b 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.h
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.h
> @@ -54,6 +54,15 @@ struct enetc_tx_swbd {
>   	u8 qbv_en:1;
>   };
>   
> +struct enetc_skb_cb {
> +	u8 flag;
> +	bool udp;
> +	u16 correction_off;
> +	u16 origin_tstamp_off;
> +};
> +
> +#define ENETC_SKB_CB(skb) ((struct enetc_skb_cb *)((skb)->cb))
> +
>   struct enetc_lso_t {
>   	bool	ipv6;
>   	bool	tcp;

RE: [PATCH net-next 08/12] net: enetc: save the parsed information of PTP packet to skb->cb

[Wei Fang]

> > @@ -225,13 +225,12 @@ static int enetc_map_tx_buffs(struct enetc_bdr
> *tx_ring, struct sk_buff *skb)
> >   {
> >   	bool do_vlan, do_onestep_tstamp = false, do_twostep_tstamp = false;
> >   	struct enetc_ndev_priv *priv = netdev_priv(tx_ring->ndev);
> > +	struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
> >   	struct enetc_hw *hw = &priv->si->hw;
> >   	struct enetc_tx_swbd *tx_swbd;
> >   	int len = skb_headlen(skb);
> >   	union enetc_tx_bd temp_bd;
> > -	u8 msgtype, twostep, udp;
> >   	union enetc_tx_bd *txbd;
> > -	u16 offset1, offset2;
> >   	int i, count = 0;
> >   	skb_frag_t *frag;
> >   	unsigned int f;
> > @@ -280,16 +279,10 @@ static int enetc_map_tx_buffs(struct enetc_bdr
> *tx_ring, struct sk_buff *skb)
> >   	count++;
> >
> >   	do_vlan = skb_vlan_tag_present(skb);
> > -	if (skb->cb[0] & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
> > -		if (enetc_ptp_parse(skb, &udp, &msgtype, &twostep, &offset1,
> > -				    &offset2) ||
> > -		    msgtype != PTP_MSGTYPE_SYNC || twostep)
> > -			WARN_ONCE(1, "Bad packet for one-step timestamping\n");
> > -		else
> > -			do_onestep_tstamp = true;
> > -	} else if (skb->cb[0] & ENETC_F_TX_TSTAMP) {
> > +	if (enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP)
> > +		do_onestep_tstamp = true;
> > +	else if (enetc_cb->flag & ENETC_F_TX_TSTAMP)
> >   		do_twostep_tstamp = true;
> > -	}
> >
> >   	tx_swbd->do_twostep_tstamp = do_twostep_tstamp;
> >   	tx_swbd->qbv_en = !!(priv->active_offloads & ENETC_F_QBV); @@
> > -333,6 +326,8 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring,
> struct sk_buff *skb)
> >   		}
> >
> >   		if (do_onestep_tstamp) {
> > +			u16 tstamp_off = enetc_cb->origin_tstamp_off;
> > +			u16 corr_off = enetc_cb->correction_off;
> >   			__be32 new_sec_l, new_nsec;
> >   			u32 lo, hi, nsec, val;
> >   			__be16 new_sec_h;
> > @@ -362,32 +357,32 @@ static int enetc_map_tx_buffs(struct enetc_bdr
> *tx_ring, struct sk_buff *skb)
> >   			new_sec_h = htons((sec >> 32) & 0xffff);
> >   			new_sec_l = htonl(sec & 0xffffffff);
> >   			new_nsec = htonl(nsec);
> > -			if (udp) {
> > +			if (enetc_cb->udp) {
> >   				struct udphdr *uh = udp_hdr(skb);
> >   				__be32 old_sec_l, old_nsec;
> >   				__be16 old_sec_h;
> >
> > -				old_sec_h = *(__be16 *)(data + offset2);
> > +				old_sec_h = *(__be16 *)(data + tstamp_off);
> >   				inet_proto_csum_replace2(&uh->check, skb, old_sec_h,
> >   							 new_sec_h, false);
> >
> > -				old_sec_l = *(__be32 *)(data + offset2 + 2);
> > +				old_sec_l = *(__be32 *)(data + tstamp_off + 2);
> >   				inet_proto_csum_replace4(&uh->check, skb, old_sec_l,
> >   							 new_sec_l, false);
> >
> > -				old_nsec = *(__be32 *)(data + offset2 + 6);
> > +				old_nsec = *(__be32 *)(data + tstamp_off + 6);
> >   				inet_proto_csum_replace4(&uh->check, skb, old_nsec,
> >   							 new_nsec, false);
> >   			}
> >
> > -			*(__be16 *)(data + offset2) = new_sec_h;
> > -			*(__be32 *)(data + offset2 + 2) = new_sec_l;
> > -			*(__be32 *)(data + offset2 + 6) = new_nsec;
> > +			*(__be16 *)(data + tstamp_off) = new_sec_h;
> > ++			*(__be32 *)(data + tstamp_off + 2) = new_sec_l;
> > ++			*(__be32 *)(data + tstamp_off + 6) = new_nsec;
> 
> This looks like merge conflict artifact...

No, I have changed the variable names (see below) , which are more easier to
understand what the variables mean. I should add a description to the commit
message to state this change.

-	u16 offset1, offset2;
+			u16 tstamp_off = enetc_cb->origin_tstamp_off;
+			u16 corr_off = enetc_cb->correction_off;

> 
> >
> >   			/* Configure single-step register */
> >   			val = ENETC_PM0_SINGLE_STEP_EN;
> > -			val |= ENETC_SET_SINGLE_STEP_OFFSET(offset1);
> > -			if (udp)
> > +			val |= ENETC_SET_SINGLE_STEP_OFFSET(corr_off);
> > +			if (enetc_cb->udp)
> >   				val |= ENETC_PM0_SINGLE_STEP_CH;
> >
> >   			enetc_port_mac_wr(priv->si, ENETC_PM0_SINGLE_STEP, @@

[PATCH net-next 09/12] net: enetc: Add enetc_update_ptp_sync_msg() to process PTP sync packet

[Wei Fang]

Currently, the PTP Sync packets are processed in enetc_map_tx_buffs(),
which makes the function too long and not concise enough. Secondly,
for the upcoming ENETC v4 one-step support, some appropriate changes
are also needed. Therefore, enetc_update_ptp_sync_msg() is extracted
from enetc_map_tx_buffs() as a helper function to process the PTP Sync
packets.

Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c  | 129 ++++++++++--------
 .../net/ethernet/freescale/enetc/enetc_hw.h   |   1 +
 2 files changed, 71 insertions(+), 59 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index c1373163a096..ef002ed2fdb9 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -221,12 +221,79 @@ static void enetc_unwind_tx_frame(struct enetc_bdr *tx_ring, int count, int i)
 	}
 }
 
+static u32 enetc_update_ptp_sync_msg(struct enetc_ndev_priv *priv,
+				     struct sk_buff *skb)
+{
+	struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
+	u16 tstamp_off = enetc_cb->origin_tstamp_off;
+	u16 corr_off = enetc_cb->correction_off;
+	struct enetc_si *si = priv->si;
+	struct enetc_hw *hw = &si->hw;
+	__be32 new_sec_l, new_nsec;
+	__be16 new_sec_h;
+	u32 lo, hi, nsec;
+	u8 *data;
+	u64 sec;
+	u32 val;
+
+	lo = enetc_rd_hot(hw, ENETC_SICTR0);
+	hi = enetc_rd_hot(hw, ENETC_SICTR1);
+	sec = (u64)hi << 32 | lo;
+	nsec = do_div(sec, 1000000000);
+
+	/* Update originTimestamp field of Sync packet
+	 * - 48 bits seconds field
+	 * - 32 bits nanseconds field
+	 *
+	 * In addition, the UDP checksum needs to be updated
+	 * by software after updating originTimestamp field,
+	 * otherwise the hardware will calculate the wrong
+	 * checksum when updating the correction field and
+	 * update it to the packet.
+	 */
+
+	data = skb_mac_header(skb);
+	new_sec_h = htons((sec >> 32) & 0xffff);
+	new_sec_l = htonl(sec & 0xffffffff);
+	new_nsec = htonl(nsec);
+	if (enetc_cb->udp) {
+		struct udphdr *uh = udp_hdr(skb);
+		__be32 old_sec_l, old_nsec;
+		__be16 old_sec_h;
+
+		old_sec_h = *(__be16 *)(data + tstamp_off);
+		inet_proto_csum_replace2(&uh->check, skb, old_sec_h,
+					 new_sec_h, false);
+
+		old_sec_l = *(__be32 *)(data + tstamp_off + 2);
+		inet_proto_csum_replace4(&uh->check, skb, old_sec_l,
+					 new_sec_l, false);
+
+		old_nsec = *(__be32 *)(data + tstamp_off + 6);
+		inet_proto_csum_replace4(&uh->check, skb, old_nsec,
+					 new_nsec, false);
+	}
+
+	*(__be16 *)(data + tstamp_off) = new_sec_h;
+	*(__be32 *)(data + tstamp_off + 2) = new_sec_l;
+	*(__be32 *)(data + tstamp_off + 6) = new_nsec;
+
+	/* Configure single-step register */
+	val = ENETC_PM0_SINGLE_STEP_EN;
+	val |= ENETC_SET_SINGLE_STEP_OFFSET(corr_off);
+	if (enetc_cb->udp)
+		val |= ENETC_PM0_SINGLE_STEP_CH;
+
+	enetc_port_mac_wr(priv->si, ENETC_PM0_SINGLE_STEP, val);
+
+	return lo & ENETC_TXBD_TSTAMP;
+}
+
 static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 {
 	bool do_vlan, do_onestep_tstamp = false, do_twostep_tstamp = false;
 	struct enetc_ndev_priv *priv = netdev_priv(tx_ring->ndev);
 	struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
-	struct enetc_hw *hw = &priv->si->hw;
 	struct enetc_tx_swbd *tx_swbd;
 	int len = skb_headlen(skb);
 	union enetc_tx_bd temp_bd;
@@ -326,67 +393,11 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 		}
 
 		if (do_onestep_tstamp) {
-			u16 tstamp_off = enetc_cb->origin_tstamp_off;
-			u16 corr_off = enetc_cb->correction_off;
-			__be32 new_sec_l, new_nsec;
-			u32 lo, hi, nsec, val;
-			__be16 new_sec_h;
-			u8 *data;
-			u64 sec;
-
-			lo = enetc_rd_hot(hw, ENETC_SICTR0);
-			hi = enetc_rd_hot(hw, ENETC_SICTR1);
-			sec = (u64)hi << 32 | lo;
-			nsec = do_div(sec, 1000000000);
+			u32 tstamp = enetc_update_ptp_sync_msg(priv, skb);
 
 			/* Configure extension BD */
-			temp_bd.ext.tstamp = cpu_to_le32(lo & 0x3fffffff);
+			temp_bd.ext.tstamp = cpu_to_le32(tstamp);
 			e_flags |= ENETC_TXBD_E_FLAGS_ONE_STEP_PTP;
-
-			/* Update originTimestamp field of Sync packet
-			 * - 48 bits seconds field
-			 * - 32 bits nanseconds field
-			 *
-			 * In addition, the UDP checksum needs to be updated
-			 * by software after updating originTimestamp field,
-			 * otherwise the hardware will calculate the wrong
-			 * checksum when updating the correction field and
-			 * update it to the packet.
-			 */
-			data = skb_mac_header(skb);
-			new_sec_h = htons((sec >> 32) & 0xffff);
-			new_sec_l = htonl(sec & 0xffffffff);
-			new_nsec = htonl(nsec);
-			if (enetc_cb->udp) {
-				struct udphdr *uh = udp_hdr(skb);
-				__be32 old_sec_l, old_nsec;
-				__be16 old_sec_h;
-
-				old_sec_h = *(__be16 *)(data + tstamp_off);
-				inet_proto_csum_replace2(&uh->check, skb, old_sec_h,
-							 new_sec_h, false);
-
-				old_sec_l = *(__be32 *)(data + tstamp_off + 2);
-				inet_proto_csum_replace4(&uh->check, skb, old_sec_l,
-							 new_sec_l, false);
-
-				old_nsec = *(__be32 *)(data + tstamp_off + 6);
-				inet_proto_csum_replace4(&uh->check, skb, old_nsec,
-							 new_nsec, false);
-			}
-
-			*(__be16 *)(data + tstamp_off) = new_sec_h;
-+			*(__be32 *)(data + tstamp_off + 2) = new_sec_l;
-+			*(__be32 *)(data + tstamp_off + 6) = new_nsec;
-
-			/* Configure single-step register */
-			val = ENETC_PM0_SINGLE_STEP_EN;
-			val |= ENETC_SET_SINGLE_STEP_OFFSET(corr_off);
-			if (enetc_cb->udp)
-				val |= ENETC_PM0_SINGLE_STEP_CH;
-
-			enetc_port_mac_wr(priv->si, ENETC_PM0_SINGLE_STEP,
-					  val);
 		} else if (do_twostep_tstamp) {
 			skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
 			e_flags |= ENETC_TXBD_E_FLAGS_TWO_STEP_PTP;
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
index 73763e8f4879..377c96325814 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
@@ -614,6 +614,7 @@ enum enetc_txbd_flags {
 #define ENETC_TXBD_STATS_WIN	BIT(7)
 #define ENETC_TXBD_TXSTART_MASK GENMASK(24, 0)
 #define ENETC_TXBD_FLAGS_OFFSET 24
+#define ENETC_TXBD_TSTAMP	GENMASK(29, 0)
 
 static inline __le32 enetc_txbd_set_tx_start(u64 tx_start, u8 flags)
 {
-- 
2.34.1

Re: [PATCH net-next 09/12] net: enetc: Add enetc_update_ptp_sync_msg() to process PTP sync packet

[Vadim Fedorenko]

On 11.07.2025 07:57, Wei Fang wrote:
> Currently, the PTP Sync packets are processed in enetc_map_tx_buffs(),
> which makes the function too long and not concise enough. Secondly,
> for the upcoming ENETC v4 one-step support, some appropriate changes
> are also needed. Therefore, enetc_update_ptp_sync_msg() is extracted
> from enetc_map_tx_buffs() as a helper function to process the PTP Sync
> packets.
> 
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
>   drivers/net/ethernet/freescale/enetc/enetc.c  | 129 ++++++++++--------
>   .../net/ethernet/freescale/enetc/enetc_hw.h   |   1 +
>   2 files changed, 71 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> index c1373163a096..ef002ed2fdb9 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -221,12 +221,79 @@ static void enetc_unwind_tx_frame(struct enetc_bdr *tx_ring, int count, int i)
>   	}
>   }
>   
> +static u32 enetc_update_ptp_sync_msg(struct enetc_ndev_priv *priv,
> +				     struct sk_buff *skb)
> +{
> +	struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
> +	u16 tstamp_off = enetc_cb->origin_tstamp_off;
> +	u16 corr_off = enetc_cb->correction_off;
> +	struct enetc_si *si = priv->si;
> +	struct enetc_hw *hw = &si->hw;
> +	__be32 new_sec_l, new_nsec;
> +	__be16 new_sec_h;
> +	u32 lo, hi, nsec;
> +	u8 *data;
> +	u64 sec;
> +	u32 val;
> +
> +	lo = enetc_rd_hot(hw, ENETC_SICTR0);
> +	hi = enetc_rd_hot(hw, ENETC_SICTR1);
> +	sec = (u64)hi << 32 | lo;
> +	nsec = do_div(sec, 1000000000);
> +
> +	/* Update originTimestamp field of Sync packet
> +	 * - 48 bits seconds field
> +	 * - 32 bits nanseconds field
> +	 *
> +	 * In addition, the UDP checksum needs to be updated
> +	 * by software after updating originTimestamp field,
> +	 * otherwise the hardware will calculate the wrong
> +	 * checksum when updating the correction field and
> +	 * update it to the packet.
> +	 */
> +
> +	data = skb_mac_header(skb);
> +	new_sec_h = htons((sec >> 32) & 0xffff);
> +	new_sec_l = htonl(sec & 0xffffffff);
> +	new_nsec = htonl(nsec);
> +	if (enetc_cb->udp) {
> +		struct udphdr *uh = udp_hdr(skb);
> +		__be32 old_sec_l, old_nsec;
> +		__be16 old_sec_h;
> +
> +		old_sec_h = *(__be16 *)(data + tstamp_off);
> +		inet_proto_csum_replace2(&uh->check, skb, old_sec_h,
> +					 new_sec_h, false);
> +
> +		old_sec_l = *(__be32 *)(data + tstamp_off + 2);
> +		inet_proto_csum_replace4(&uh->check, skb, old_sec_l,
> +					 new_sec_l, false);
> +
> +		old_nsec = *(__be32 *)(data + tstamp_off + 6);
> +		inet_proto_csum_replace4(&uh->check, skb, old_nsec,
> +					 new_nsec, false);
> +	}
> +
> +	*(__be16 *)(data + tstamp_off) = new_sec_h;
> +	*(__be32 *)(data + tstamp_off + 2) = new_sec_l;
> +	*(__be32 *)(data + tstamp_off + 6) = new_nsec;
> +
> +	/* Configure single-step register */
> +	val = ENETC_PM0_SINGLE_STEP_EN;
> +	val |= ENETC_SET_SINGLE_STEP_OFFSET(corr_off);
> +	if (enetc_cb->udp)
> +		val |= ENETC_PM0_SINGLE_STEP_CH;
> +
> +	enetc_port_mac_wr(priv->si, ENETC_PM0_SINGLE_STEP, val);
> +
> +	return lo & ENETC_TXBD_TSTAMP;
> +}
> +
>   static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
>   {
>   	bool do_vlan, do_onestep_tstamp = false, do_twostep_tstamp = false;
>   	struct enetc_ndev_priv *priv = netdev_priv(tx_ring->ndev);
>   	struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
> -	struct enetc_hw *hw = &priv->si->hw;
>   	struct enetc_tx_swbd *tx_swbd;
>   	int len = skb_headlen(skb);
>   	union enetc_tx_bd temp_bd;
> @@ -326,67 +393,11 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
>   		}
>   
>   		if (do_onestep_tstamp) {
> -			u16 tstamp_off = enetc_cb->origin_tstamp_off;
> -			u16 corr_off = enetc_cb->correction_off;
> -			__be32 new_sec_l, new_nsec;
> -			u32 lo, hi, nsec, val;
> -			__be16 new_sec_h;
> -			u8 *data;
> -			u64 sec;
> -
> -			lo = enetc_rd_hot(hw, ENETC_SICTR0);
> -			hi = enetc_rd_hot(hw, ENETC_SICTR1);
> -			sec = (u64)hi << 32 | lo;
> -			nsec = do_div(sec, 1000000000);
> +			u32 tstamp = enetc_update_ptp_sync_msg(priv, skb);
>   
>   			/* Configure extension BD */
> -			temp_bd.ext.tstamp = cpu_to_le32(lo & 0x3fffffff);
> +			temp_bd.ext.tstamp = cpu_to_le32(tstamp);
>   			e_flags |= ENETC_TXBD_E_FLAGS_ONE_STEP_PTP;
> -
> -			/* Update originTimestamp field of Sync packet
> -			 * - 48 bits seconds field
> -			 * - 32 bits nanseconds field
> -			 *
> -			 * In addition, the UDP checksum needs to be updated
> -			 * by software after updating originTimestamp field,
> -			 * otherwise the hardware will calculate the wrong
> -			 * checksum when updating the correction field and
> -			 * update it to the packet.
> -			 */
> -			data = skb_mac_header(skb);
> -			new_sec_h = htons((sec >> 32) & 0xffff);
> -			new_sec_l = htonl(sec & 0xffffffff);
> -			new_nsec = htonl(nsec);
> -			if (enetc_cb->udp) {
> -				struct udphdr *uh = udp_hdr(skb);
> -				__be32 old_sec_l, old_nsec;
> -				__be16 old_sec_h;
> -
> -				old_sec_h = *(__be16 *)(data + tstamp_off);
> -				inet_proto_csum_replace2(&uh->check, skb, old_sec_h,
> -							 new_sec_h, false);
> -
> -				old_sec_l = *(__be32 *)(data + tstamp_off + 2);
> -				inet_proto_csum_replace4(&uh->check, skb, old_sec_l,
> -							 new_sec_l, false);
> -
> -				old_nsec = *(__be32 *)(data + tstamp_off + 6);
> -				inet_proto_csum_replace4(&uh->check, skb, old_nsec,
> -							 new_nsec, false);
> -			}
> -
> -			*(__be16 *)(data + tstamp_off) = new_sec_h;
> -+			*(__be32 *)(data + tstamp_off + 2) = new_sec_l;
> -+			*(__be32 *)(data + tstamp_off + 6) = new_nsec;

And again some artifacts...

> -
> -			/* Configure single-step register */
> -			val = ENETC_PM0_SINGLE_STEP_EN;
> -			val |= ENETC_SET_SINGLE_STEP_OFFSET(corr_off);
> -			if (enetc_cb->udp)
> -				val |= ENETC_PM0_SINGLE_STEP_CH;
> -
> -			enetc_port_mac_wr(priv->si, ENETC_PM0_SINGLE_STEP,
> -					  val);
>   		} else if (do_twostep_tstamp) {
>   			skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
>   			e_flags |= ENETC_TXBD_E_FLAGS_TWO_STEP_PTP;
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> index 73763e8f4879..377c96325814 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> @@ -614,6 +614,7 @@ enum enetc_txbd_flags {
>   #define ENETC_TXBD_STATS_WIN	BIT(7)
>   #define ENETC_TXBD_TXSTART_MASK GENMASK(24, 0)
>   #define ENETC_TXBD_FLAGS_OFFSET 24
> +#define ENETC_TXBD_TSTAMP	GENMASK(29, 0)
>   
>   static inline __le32 enetc_txbd_set_tx_start(u64 tx_start, u8 flags)
>   {

[PATCH net-next 10/12] net: enetc: remove unnecessary CONFIG_FSL_ENETC_PTP_CLOCK check

[Wei Fang]

The ENETC_F_RX_TSTAMP flag of priv->active_offloads can only be set when
CONFIG_FSL_ENETC_PTP_CLOCK is enabled. Similarly, rx_ring->ext_en can
only be set when CONFIG_FSL_ENETC_PTP_CLOCK is enabled as well. So it is
safe to remove unnecessary CONFIG_FSL_ENETC_PTP_CLOCK check.

Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 3 +--
 drivers/net/ethernet/freescale/enetc/enetc.h | 4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index ef002ed2fdb9..4325eb3d9481 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -1411,8 +1411,7 @@ static void enetc_get_offloads(struct enetc_bdr *rx_ring,
 		__vlan_hwaccel_put_tag(skb, tpid, le16_to_cpu(rxbd->r.vlan_opt));
 	}
 
-	if (IS_ENABLED(CONFIG_FSL_ENETC_PTP_CLOCK) &&
-	    (priv->active_offloads & ENETC_F_RX_TSTAMP))
+	if (priv->active_offloads & ENETC_F_RX_TSTAMP)
 		enetc_get_rx_tstamp(rx_ring->ndev, rxbd, skb);
 }
 
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h
index ce3fed95091b..c65aa7b88122 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc.h
@@ -226,7 +226,7 @@ static inline union enetc_rx_bd *enetc_rxbd(struct enetc_bdr *rx_ring, int i)
 {
 	int hw_idx = i;
 
-	if (IS_ENABLED(CONFIG_FSL_ENETC_PTP_CLOCK) && rx_ring->ext_en)
+	if (rx_ring->ext_en)
 		hw_idx = 2 * i;
 
 	return &(((union enetc_rx_bd *)rx_ring->bd_base)[hw_idx]);
@@ -240,7 +240,7 @@ static inline void enetc_rxbd_next(struct enetc_bdr *rx_ring,
 
 	new_rxbd++;
 
-	if (IS_ENABLED(CONFIG_FSL_ENETC_PTP_CLOCK) && rx_ring->ext_en)
+	if (rx_ring->ext_en)
 		new_rxbd++;
 
 	if (unlikely(++new_index == rx_ring->bd_count)) {
-- 
2.34.1

Re: [PATCH net-next 10/12] net: enetc: remove unnecessary CONFIG_FSL_ENETC_PTP_CLOCK check

[Vadim Fedorenko]

On 11.07.2025 07:57, Wei Fang wrote:
> The ENETC_F_RX_TSTAMP flag of priv->active_offloads can only be set when
> CONFIG_FSL_ENETC_PTP_CLOCK is enabled. Similarly, rx_ring->ext_en can
> only be set when CONFIG_FSL_ENETC_PTP_CLOCK is enabled as well. So it is
> safe to remove unnecessary CONFIG_FSL_ENETC_PTP_CLOCK check.
> 
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
>   drivers/net/ethernet/freescale/enetc/enetc.c | 3 +--
>   drivers/net/ethernet/freescale/enetc/enetc.h | 4 ++--
>   2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> index ef002ed2fdb9..4325eb3d9481 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -1411,8 +1411,7 @@ static void enetc_get_offloads(struct enetc_bdr *rx_ring,
>   		__vlan_hwaccel_put_tag(skb, tpid, le16_to_cpu(rxbd->r.vlan_opt));
>   	}
>   
> -	if (IS_ENABLED(CONFIG_FSL_ENETC_PTP_CLOCK) &&
> -	    (priv->active_offloads & ENETC_F_RX_TSTAMP))
> +	if (priv->active_offloads & ENETC_F_RX_TSTAMP)
>   		enetc_get_rx_tstamp(rx_ring->ndev, rxbd, skb);
>   }
>   
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h
> index ce3fed95091b..c65aa7b88122 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.h
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.h
> @@ -226,7 +226,7 @@ static inline union enetc_rx_bd *enetc_rxbd(struct enetc_bdr *rx_ring, int i)
>   {
>   	int hw_idx = i;
>   
> -	if (IS_ENABLED(CONFIG_FSL_ENETC_PTP_CLOCK) && rx_ring->ext_en)
> +	if (rx_ring->ext_en)
>   		hw_idx = 2 * i;
>   
>   	return &(((union enetc_rx_bd *)rx_ring->bd_base)[hw_idx]);
> @@ -240,7 +240,7 @@ static inline void enetc_rxbd_next(struct enetc_bdr *rx_ring,
>   
>   	new_rxbd++;
>   
> -	if (IS_ENABLED(CONFIG_FSL_ENETC_PTP_CLOCK) && rx_ring->ext_en)
> +	if (rx_ring->ext_en)
>   		new_rxbd++;
>   
>   	if (unlikely(++new_index == rx_ring->bd_count)) {

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

[PATCH net-next 11/12] net: enetc: add PTP synchronization support for ENETC v4

[Wei Fang]

Regarding PTP, ENETC v4 has some changes compared to ENETC v1 (LS1028A),
mainly as follows.

1. ENETC v4 uses a different PTP driver, so the way to get phc_index is
different from LS1028A. Therefore, enetc_get_ts_info() has been modified
appropriately to be compatible with ENETC v1 and v4.

2. The hardware of ENETC v4 does not support "dma-coherent", therefore,
to support PTP one-step, the PTP sync packets must be modified before
calling dma_map_single() to map the DMA cache of the packets. Otherwise,
the modification is invalid, the originTimestamp and correction fields
of the sent packets will still be the values before the modification.

3. The PMa_SINGLE_STEP register has changed in ENETC v4, not only the
register offset, but also some register fields. Therefore, two helper
functions are added, enetc_set_one_step_ts() for ENETC v1 and
enetc4_set_one_step_ts() for ENETC v4.

Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c  | 55 ++++++++++-----
 drivers/net/ethernet/freescale/enetc/enetc.h  |  8 +++
 .../net/ethernet/freescale/enetc/enetc4_hw.h  |  6 ++
 .../net/ethernet/freescale/enetc/enetc4_pf.c  |  3 +
 .../ethernet/freescale/enetc/enetc_ethtool.c  | 69 +++++++++++++++----
 5 files changed, 112 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index 4325eb3d9481..6e04dd825a95 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -221,6 +221,31 @@ static void enetc_unwind_tx_frame(struct enetc_bdr *tx_ring, int count, int i)
 	}
 }
 
+static void enetc_set_one_step_ts(struct enetc_si *si, bool udp, int offset)
+{
+	u32 val = ENETC_PM0_SINGLE_STEP_EN;
+
+	val |= ENETC_SET_SINGLE_STEP_OFFSET(offset);
+	if (udp)
+		val |= ENETC_PM0_SINGLE_STEP_CH;
+
+	/* the "Correction" field of a packet is updated based on the
+	 * current time and the timestamp provided
+	 */
+	enetc_port_mac_wr(si, ENETC_PM0_SINGLE_STEP, val);
+}
+
+static void enetc4_set_one_step_ts(struct enetc_si *si, bool udp, int offset)
+{
+	u32 val = PM_SINGLE_STEP_EN;
+
+	val |= PM_SINGLE_STEP_OFFSET_SET(offset);
+	if (udp)
+		val |= PM_SINGLE_STEP_CH;
+
+	enetc_port_mac_wr(si, ENETC4_PM_SINGLE_STEP(0), val);
+}
+
 static u32 enetc_update_ptp_sync_msg(struct enetc_ndev_priv *priv,
 				     struct sk_buff *skb)
 {
@@ -234,7 +259,6 @@ static u32 enetc_update_ptp_sync_msg(struct enetc_ndev_priv *priv,
 	u32 lo, hi, nsec;
 	u8 *data;
 	u64 sec;
-	u32 val;
 
 	lo = enetc_rd_hot(hw, ENETC_SICTR0);
 	hi = enetc_rd_hot(hw, ENETC_SICTR1);
@@ -279,12 +303,10 @@ static u32 enetc_update_ptp_sync_msg(struct enetc_ndev_priv *priv,
 	*(__be32 *)(data + tstamp_off + 6) = new_nsec;
 
 	/* Configure single-step register */
-	val = ENETC_PM0_SINGLE_STEP_EN;
-	val |= ENETC_SET_SINGLE_STEP_OFFSET(corr_off);
-	if (enetc_cb->udp)
-		val |= ENETC_PM0_SINGLE_STEP_CH;
-
-	enetc_port_mac_wr(priv->si, ENETC_PM0_SINGLE_STEP, val);
+	if (is_enetc_rev1(si))
+		enetc_set_one_step_ts(si, enetc_cb->udp, corr_off);
+	else
+		enetc4_set_one_step_ts(si, enetc_cb->udp, corr_off);
 
 	return lo & ENETC_TXBD_TSTAMP;
 }
@@ -303,6 +325,7 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 	unsigned int f;
 	dma_addr_t dma;
 	u8 flags = 0;
+	u32 tstamp;
 
 	enetc_clear_tx_bd(&temp_bd);
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
@@ -327,6 +350,13 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 		}
 	}
 
+	if (enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
+		do_onestep_tstamp = true;
+		tstamp = enetc_update_ptp_sync_msg(priv, skb);
+	} else if (enetc_cb->flag & ENETC_F_TX_TSTAMP) {
+		do_twostep_tstamp = true;
+	}
+
 	i = tx_ring->next_to_use;
 	txbd = ENETC_TXBD(*tx_ring, i);
 	prefetchw(txbd);
@@ -346,11 +376,6 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 	count++;
 
 	do_vlan = skb_vlan_tag_present(skb);
-	if (enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP)
-		do_onestep_tstamp = true;
-	else if (enetc_cb->flag & ENETC_F_TX_TSTAMP)
-		do_twostep_tstamp = true;
-
 	tx_swbd->do_twostep_tstamp = do_twostep_tstamp;
 	tx_swbd->qbv_en = !!(priv->active_offloads & ENETC_F_QBV);
 	tx_swbd->check_wb = tx_swbd->do_twostep_tstamp || tx_swbd->qbv_en;
@@ -393,8 +418,6 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 		}
 
 		if (do_onestep_tstamp) {
-			u32 tstamp = enetc_update_ptp_sync_msg(priv, skb);
-
 			/* Configure extension BD */
 			temp_bd.ext.tstamp = cpu_to_le32(tstamp);
 			e_flags |= ENETC_TXBD_E_FLAGS_ONE_STEP_PTP;
@@ -3314,7 +3337,7 @@ int enetc_hwtstamp_set(struct net_device *ndev,
 	struct enetc_ndev_priv *priv = netdev_priv(ndev);
 	int err, new_offloads = priv->active_offloads;
 
-	if (!IS_ENABLED(CONFIG_FSL_ENETC_PTP_CLOCK))
+	if (!enetc_ptp_clock_is_enabled(priv->si))
 		return -EOPNOTSUPP;
 
 	switch (config->tx_type) {
@@ -3364,7 +3387,7 @@ int enetc_hwtstamp_get(struct net_device *ndev,
 {
 	struct enetc_ndev_priv *priv = netdev_priv(ndev);
 
-	if (!IS_ENABLED(CONFIG_FSL_ENETC_PTP_CLOCK))
+	if (!enetc_ptp_clock_is_enabled(priv->si))
 		return -EOPNOTSUPP;
 
 	if (priv->active_offloads & ENETC_F_TX_ONESTEP_SYNC_TSTAMP)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h
index c65aa7b88122..879bc6466e8b 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc.h
@@ -611,6 +611,14 @@ int enetc_psfp_init(struct enetc_ndev_priv *priv);
 int enetc_psfp_clean(struct enetc_ndev_priv *priv);
 int enetc_set_psfp(struct net_device *ndev, bool en);
 
+static inline bool enetc_ptp_clock_is_enabled(struct enetc_si *si)
+{
+	if (is_enetc_rev1(si))
+		return IS_ENABLED(CONFIG_FSL_ENETC_PTP_CLOCK);
+
+	return IS_ENABLED(CONFIG_PTP_1588_CLOCK_NETC);
+}
+
 static inline void enetc_get_max_cap(struct enetc_ndev_priv *priv)
 {
 	struct enetc_hw *hw = &priv->si->hw;
diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_hw.h b/drivers/net/ethernet/freescale/enetc/enetc4_hw.h
index aa25b445d301..a8113c9057eb 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc4_hw.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc4_hw.h
@@ -171,6 +171,12 @@
 /* Port MAC 0/1 Pause Quanta Threshold Register */
 #define ENETC4_PM_PAUSE_THRESH(mac)	(0x5064 + (mac) * 0x400)
 
+#define ENETC4_PM_SINGLE_STEP(mac)	(0x50c0 + (mac) * 0x400)
+#define  PM_SINGLE_STEP_CH		BIT(6)
+#define  PM_SINGLE_STEP_OFFSET		GENMASK(15, 7)
+#define   PM_SINGLE_STEP_OFFSET_SET(o)  FIELD_PREP(PM_SINGLE_STEP_OFFSET, o)
+#define  PM_SINGLE_STEP_EN		BIT(31)
+
 /* Port MAC 0 Interface Mode Control Register */
 #define ENETC4_PM_IF_MODE(mac)		(0x5300 + (mac) * 0x400)
 #define  PM_IF_MODE_IFMODE		GENMASK(2, 0)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
index b3dc1afeefd1..107f59169e67 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
@@ -569,6 +569,9 @@ static const struct net_device_ops enetc4_ndev_ops = {
 	.ndo_set_features	= enetc4_pf_set_features,
 	.ndo_vlan_rx_add_vid	= enetc_vlan_rx_add_vid,
 	.ndo_vlan_rx_kill_vid	= enetc_vlan_rx_del_vid,
+	.ndo_eth_ioctl		= enetc_ioctl,
+	.ndo_hwtstamp_get	= enetc_hwtstamp_get,
+	.ndo_hwtstamp_set	= enetc_hwtstamp_set,
 };
 
 static struct phylink_pcs *
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
index 961e76cd8489..cf33b405e76c 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
@@ -2,6 +2,7 @@
 /* Copyright 2017-2019 NXP */
 
 #include <linux/ethtool_netlink.h>
+#include <linux/fsl/netc_global.h>
 #include <linux/net_tstamp.h>
 #include <linux/module.h>
 #include "enetc.h"
@@ -877,23 +878,28 @@ static int enetc_set_coalesce(struct net_device *ndev,
 	return 0;
 }
 
-static int enetc_get_ts_info(struct net_device *ndev,
-			     struct kernel_ethtool_ts_info *info)
+static struct pci_dev *enetc4_get_timer_pdev(struct enetc_si *si)
 {
-	struct enetc_ndev_priv *priv = netdev_priv(ndev);
-	int *phc_idx;
-
-	phc_idx = symbol_get(enetc_phc_index);
-	if (phc_idx) {
-		info->phc_index = *phc_idx;
-		symbol_put(enetc_phc_index);
+	struct pci_bus *bus = si->pdev->bus;
+	int domain = pci_domain_nr(bus);
+	int bus_num = bus->number;
+	int devfn;
+
+	switch (si->revision) {
+	case ENETC_REV_4_1:
+		devfn = PCI_DEVFN(24, 0);
+		break;
+	default:
+		return NULL;
 	}
 
-	if (!IS_ENABLED(CONFIG_FSL_ENETC_PTP_CLOCK)) {
-		info->so_timestamping = SOF_TIMESTAMPING_TX_SOFTWARE;
+	return pci_dev_get(pci_get_domain_bus_and_slot(domain, bus_num, devfn));
+}
 
-		return 0;
-	}
+static void enetc_get_ts_generic_info(struct net_device *ndev,
+				      struct kernel_ethtool_ts_info *info)
+{
+	struct enetc_ndev_priv *priv = netdev_priv(ndev);
 
 	info->so_timestamping = SOF_TIMESTAMPING_TX_HARDWARE |
 				SOF_TIMESTAMPING_RX_HARDWARE |
@@ -908,6 +914,42 @@ static int enetc_get_ts_info(struct net_device *ndev,
 
 	info->rx_filters = (1 << HWTSTAMP_FILTER_NONE) |
 			   (1 << HWTSTAMP_FILTER_ALL);
+}
+
+static int enetc_get_ts_info(struct net_device *ndev,
+			     struct kernel_ethtool_ts_info *info)
+{
+	struct enetc_ndev_priv *priv = netdev_priv(ndev);
+	struct enetc_si *si = priv->si;
+	struct pci_dev *timer_pdev;
+	int *phc_idx;
+
+	if (!enetc_ptp_clock_is_enabled(si))
+		goto timestamp_tx_sw;
+
+	if (is_enetc_rev1(si)) {
+		phc_idx = symbol_get(enetc_phc_index);
+		if (phc_idx) {
+			info->phc_index = *phc_idx;
+			symbol_put(enetc_phc_index);
+		}
+	} else {
+		timer_pdev = enetc4_get_timer_pdev(si);
+		if (!timer_pdev)
+			goto timestamp_tx_sw;
+
+		info->phc_index = netc_timer_get_phc_index(timer_pdev);
+		pci_dev_put(timer_pdev);
+		if (info->phc_index < 0)
+			goto timestamp_tx_sw;
+	}
+
+	enetc_get_ts_generic_info(ndev, info);
+
+	return 0;
+
+timestamp_tx_sw:
+	info->so_timestamping = SOF_TIMESTAMPING_TX_SOFTWARE;
 
 	return 0;
 }
@@ -1296,6 +1338,7 @@ const struct ethtool_ops enetc4_pf_ethtool_ops = {
 	.get_rxfh = enetc_get_rxfh,
 	.set_rxfh = enetc_set_rxfh,
 	.get_rxfh_fields = enetc_get_rxfh_fields,
+	.get_ts_info = enetc_get_ts_info,
 };
 
 void enetc_set_ethtool_ops(struct net_device *ndev)
-- 
2.34.1

Re: [PATCH net-next 11/12] net: enetc: add PTP synchronization support for ENETC v4

[kernel test robot]

Hi Wei,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Wei-Fang/dt-bindings-ptp-add-bindings-for-NETC-Timer/20250711-152311
base:   net-next/main
patch link:    https://lore.kernel.org/r/20250711065748.250159-12-wei.fang%40nxp.com
patch subject: [PATCH net-next 11/12] net: enetc: add PTP synchronization support for ENETC v4
config: loongarch-randconfig-r062-20250712 (https://download.01.org/0day-ci/archive/20250713/202507130049.KLM4A8GG-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project 01c97b4953e87ae455bd4c41e3de3f0f0f29c61c)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250713/202507130049.KLM4A8GG-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/202507130049.KLM4A8GG-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/net/ethernet/freescale/enetc/enetc.c:3340:7: error: call to undeclared function 'enetc_ptp_clock_is_enabled'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    3340 |         if (!enetc_ptp_clock_is_enabled(priv->si))
         |              ^
   drivers/net/ethernet/freescale/enetc/enetc.c:3390:7: error: call to undeclared function 'enetc_ptp_clock_is_enabled'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    3390 |         if (!enetc_ptp_clock_is_enabled(priv->si))
         |              ^
   drivers/net/ethernet/freescale/enetc/enetc.c:3602:46: warning: shift count >= width of type [-Wshift-count-overflow]
    3602 |         err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
         |                                                     ^~~~~~~~~~~~~~~~
   include/linux/dma-mapping.h:73:54: note: expanded from macro 'DMA_BIT_MASK'
      73 | #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
         |                                                      ^ ~~~
   1 warning and 2 errors generated.
--
>> drivers/net/ethernet/freescale/enetc/enetc_ethtool.c:927:7: error: call to undeclared function 'enetc_ptp_clock_is_enabled'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     927 |         if (!enetc_ptp_clock_is_enabled(si))
         |              ^
   1 error generated.


vim +/enetc_ptp_clock_is_enabled +3340 drivers/net/ethernet/freescale/enetc/enetc.c

  3332	
  3333	int enetc_hwtstamp_set(struct net_device *ndev,
  3334			       struct kernel_hwtstamp_config *config,
  3335			       struct netlink_ext_ack *extack)
  3336	{
  3337		struct enetc_ndev_priv *priv = netdev_priv(ndev);
  3338		int err, new_offloads = priv->active_offloads;
  3339	
> 3340		if (!enetc_ptp_clock_is_enabled(priv->si))
  3341			return -EOPNOTSUPP;
  3342	
  3343		switch (config->tx_type) {
  3344		case HWTSTAMP_TX_OFF:
  3345			new_offloads &= ~ENETC_F_TX_TSTAMP_MASK;
  3346			break;
  3347		case HWTSTAMP_TX_ON:
  3348			new_offloads &= ~ENETC_F_TX_TSTAMP_MASK;
  3349			new_offloads |= ENETC_F_TX_TSTAMP;
  3350			break;
  3351		case HWTSTAMP_TX_ONESTEP_SYNC:
  3352			if (!enetc_si_is_pf(priv->si))
  3353				return -EOPNOTSUPP;
  3354	
  3355			new_offloads &= ~ENETC_F_TX_TSTAMP_MASK;
  3356			new_offloads |= ENETC_F_TX_ONESTEP_SYNC_TSTAMP;
  3357			break;
  3358		default:
  3359			return -ERANGE;
  3360		}
  3361	
  3362		switch (config->rx_filter) {
  3363		case HWTSTAMP_FILTER_NONE:
  3364			new_offloads &= ~ENETC_F_RX_TSTAMP;
  3365			break;
  3366		default:
  3367			new_offloads |= ENETC_F_RX_TSTAMP;
  3368			config->rx_filter = HWTSTAMP_FILTER_ALL;
  3369		}
  3370	
  3371		if ((new_offloads ^ priv->active_offloads) & ENETC_F_RX_TSTAMP) {
  3372			bool extended = !!(new_offloads & ENETC_F_RX_TSTAMP);
  3373	
  3374			err = enetc_reconfigure(priv, extended, NULL, NULL);
  3375			if (err)
  3376				return err;
  3377		}
  3378	
  3379		priv->active_offloads = new_offloads;
  3380	
  3381		return 0;
  3382	}
  3383	EXPORT_SYMBOL_GPL(enetc_hwtstamp_set);
  3384	

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

[PATCH net-next 12/12] net: enetc: don't update sync packet checksum if checksum offload is used

[Wei Fang]

For ENETC v4, the hardware has the capability to support Tx checksum
offload. so the enetc driver does not need to update the UDP checksum
of PTP sync packets if Tx checksum offload is enabled.

Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index 6e04dd825a95..cf72d50246a9 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -247,7 +247,7 @@ static void enetc4_set_one_step_ts(struct enetc_si *si, bool udp, int offset)
 }
 
 static u32 enetc_update_ptp_sync_msg(struct enetc_ndev_priv *priv,
-				     struct sk_buff *skb)
+				     struct sk_buff *skb, bool csum_offload)
 {
 	struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
 	u16 tstamp_off = enetc_cb->origin_tstamp_off;
@@ -269,18 +269,17 @@ static u32 enetc_update_ptp_sync_msg(struct enetc_ndev_priv *priv,
 	 * - 48 bits seconds field
 	 * - 32 bits nanseconds field
 	 *
-	 * In addition, the UDP checksum needs to be updated
-	 * by software after updating originTimestamp field,
-	 * otherwise the hardware will calculate the wrong
-	 * checksum when updating the correction field and
-	 * update it to the packet.
+	 * In addition, if csum_offload is false, the UDP checksum needs
+	 * to be updated by software after updating originTimestamp field,
+	 * otherwise the hardware will calculate the wrong checksum when
+	 * updating the correction field and update it to the packet.
 	 */
 
 	data = skb_mac_header(skb);
 	new_sec_h = htons((sec >> 32) & 0xffff);
 	new_sec_l = htonl(sec & 0xffffffff);
 	new_nsec = htonl(nsec);
-	if (enetc_cb->udp) {
+	if (enetc_cb->udp && !csum_offload) {
 		struct udphdr *uh = udp_hdr(skb);
 		__be32 old_sec_l, old_nsec;
 		__be16 old_sec_h;
@@ -319,6 +318,7 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 	struct enetc_tx_swbd *tx_swbd;
 	int len = skb_headlen(skb);
 	union enetc_tx_bd temp_bd;
+	bool csum_offload = false;
 	union enetc_tx_bd *txbd;
 	int i, count = 0;
 	skb_frag_t *frag;
@@ -345,6 +345,7 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 				temp_bd.l4_aux = FIELD_PREP(ENETC_TX_BD_L4T,
 							    ENETC_TXBD_L4T_UDP);
 			flags |= ENETC_TXBD_FLAGS_CSUM_LSO | ENETC_TXBD_FLAGS_L4CS;
+			csum_offload = true;
 		} else if (skb_checksum_help(skb)) {
 			return 0;
 		}
@@ -352,7 +353,7 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 
 	if (enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
 		do_onestep_tstamp = true;
-		tstamp = enetc_update_ptp_sync_msg(priv, skb);
+		tstamp = enetc_update_ptp_sync_msg(priv, skb, csum_offload);
 	} else if (enetc_cb->flag & ENETC_F_TX_TSTAMP) {
 		do_twostep_tstamp = true;
 	}
-- 
2.34.1