Re: [PATCH 1/2] dt-bindings: ptp: Convert ptp-qoirq to yaml format

[Krzysztof Kozlowski <krzk@kernel.org>]

On 14/06/2024 22:33, Frank Li wrote:
> diff --git a/Documentation/devicetree/bindings/ptp/ptp-qoriq.yaml b/Documentation/devicetree/bindings/ptp/ptp-qoriq.yaml
> new file mode 100644
> index 0000000000000..585e8bffd90c9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ptp/ptp-qoriq.yaml

Filename based on compatible. Can be fsl,ptp.yaml

> @@ -0,0 +1,148 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/ptp/ptp-qoriq.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale QorIQ 1588 timer based PTP clock
> +
> +maintainers:
> +  - Frank Li <Frank.Li@nxp.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - fsl,etsec-ptp
> +      - fsl,fman-ptp-timer
> +      - fsl,dpaa2-ptp
> +      - fsl,enetc-ptp
> +    description: |
> +      Should be "fsl,etsec-ptp" for eTSEC
> +      Should be "fsl,fman-ptp-timer" for DPAA FMan
> +      Should be "fsl,dpaa2-ptp" for DPAA2
> +      Should be "fsl,enetc-ptp" for ENETC

You can write it simpler, e.g.
- fsl,etsec-ptp   # eTSEC

and then you see that this does not bring any new information - your
comment duplicates the compatible. Just drop.

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    minItems: 2
> +    maxItems: 4

Items should be described.

> +
> +  clocks:
> +    maxItems: 1
> +
> +  fsl,cksel:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Timer reference clock source.
> +
> +      Reference clock source is determined by the value, which is holded
> +      in CKSEL bits in TMR_CTRL register. "fsl,cksel" property keeps the
> +      value, which will be directly written in those bits, that is why,
> +      according to reference manual, the next clock sources can be used:
> +
> +      For eTSEC,
> +      <0> - external high precision timer reference clock (TSEC_TMR_CLK
> +            input is used for this purpose);
> +      <1> - eTSEC system clock;
> +      <2> - eTSEC1 transmit clock;
> +      <3> - RTC clock input.
> +
> +      For DPAA FMan,
> +      <0> - external high precision timer reference clock (TMR_1588_CLK)
> +      <1> - MAC system clock (1/2 FMan clock)
> +      <2> - reserved
> +      <3> - RTC clock oscillator
> +
> +  fsl,tclk-period:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Timer reference clock period in nanoseconds.
> +
> +  fsl,tmr-prsc:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Prescaler, divides the output clock.
> +
> +  fsl,tmr-add:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Frequency compensation value.
> +
> +  fsl,tmr-fiper1:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Fixed interval period pulse generator.
> +
> +  fsl,tmr-fiper2:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Fixed interval period pulse generator.
> +
> +  fsl,tmr-fiper3:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Fixed interval period pulse generator.
> +      Supported only on DPAA2 and ENETC hardware.
> +
> +  fsl,max-adj:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Maximum frequency adjustment in parts per billion.
> +
> +      These properties set the operational parameters for the PTP
> +      clock. You must choose these carefully for the clock to work right.
> +      Here is how to figure good values:
> +
> +      TimerOsc     = selected reference clock   MHz
> +      tclk_period  = desired clock period       nanoseconds
> +      NominalFreq  = 1000 / tclk_period         MHz
> +      FreqDivRatio = TimerOsc / NominalFreq     (must be greater that 1.0)
> +      tmr_add      = ceil(2^32 / FreqDivRatio)
> +      OutputClock  = NominalFreq / tmr_prsc     MHz
> +      PulseWidth   = 1 / OutputClock            microseconds
> +      FiperFreq1   = desired frequency in Hz
> +      FiperDiv1    = 1000000 * OutputClock / FiperFreq1
> +      tmr_fiper1   = tmr_prsc * tclk_period * FiperDiv1 - tclk_period
> +      max_adj      = 1000000000 * (FreqDivRatio - 1.0) - 1
> +
> +      The calculation for tmr_fiper2 is the same as for tmr_fiper1. The
> +      driver expects that tmr_fiper1 will be correctly set to produce a 1
> +      Pulse Per Second (PPS) signal, since this will be offered to the PPS
> +      subsystem to synchronize the Linux clock.
> +
> +      When this attribute is not used, the IEEE 1588 timer reference clock
> +      will use the eTSEC system clock (for Gianfar) or the MAC system
> +      clock (for DPAA).
> +
> +  fsl,extts-fifo:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      The presence of this property indicates hardware
> +      support for the external trigger stamp FIFO
> +
> +  little-endian:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      The presence of this property indicates the 1588 timer
> +      support for the external trigger stamp FIFO.
> +      IP block is little-endian mode. The default endian mode
> +      is big-endian.
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    ptp_clock@24e00 {

phc@

> +        compatible = "fsl,etsec-ptp";
> +        reg = <0x24E00 0xB0>;

Lowercase hex, in other places as well.


> +        interrupts = <12 0x8>, <13 0x8>;

Use proper defines for interrupt flags.

> +        interrupt-parent = <&ipic>;
> +        fsl,cksel       = <1>;


Best regards,
Krzysztof