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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <61e6c90d-3811-41c2-853d-d93d9db38f21@kernel.org>
Date: Mon, 14 Jul 2025 09:35:21 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Wei Fang <wei.fang@....com>
Cc: "F.S. Peng" <fushi.peng@....com>,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "imx@...ts.linux.dev" <imx@...ts.linux.dev>,
 "robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
 <krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
 "richardcochran@...il.com" <richardcochran@...il.com>,
 Claudiu Manoil <claudiu.manoil@....com>,
 Vladimir Oltean <vladimir.oltean@....com>, Clark Wang
 <xiaoning.wang@....com>, "andrew+netdev@...n.ch" <andrew+netdev@...n.ch>,
 "davem@...emloft.net" <davem@...emloft.net>,
 "edumazet@...gle.com" <edumazet@...gle.com>,
 "kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>
Subject: Re: [PATCH net-next 01/12] dt-bindings: ptp: add bindings for NETC
 Timer

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@....com>
>>> +  - Clark Wang <xiaoning.wang@....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