Add support for the time slot assigner (TSA)
available in some PowerQUICC SoC such as MPC885
or MPC866.
Signed-off-by: Herve Codina <[email protected]>
---
.../bindings/soc/fsl/cpm_qe/fsl,tsa.yaml | 262 ++++++++++++++++++
include/dt-bindings/soc/fsl-tsa.h | 15 +
2 files changed, 277 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,tsa.yaml
create mode 100644 include/dt-bindings/soc/fsl-tsa.h
diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,tsa.yaml b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,tsa.yaml
new file mode 100644
index 000000000000..7542c0fd8435
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,tsa.yaml
@@ -0,0 +1,262 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/fsl/cpm_qe/fsl,tsa.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: PowerQUICC CPM Time-slot assigner (TSA) controller
+
+maintainers:
+ - Herve Codina <[email protected]>
+
+description: |
+ The TSA is the time-slot assigner that can be found on some
+ PowerQUICC SoC.
+ Its purpose is to route some TDM time-slots to other internal
+ serial controllers.
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - fsl,mpc885-tsa
+ - fsl,mpc866-tsa
+ - const: fsl,cpm1-tsa
+
+ reg:
+ items:
+ - description: SI (Serial Interface) register base
+ - description: SI RAM base
+
+ reg-names:
+ items:
+ - const: si_regs
+ - const: si_ram
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+patternProperties:
+ "^tdm@[0-1]$":
+ description:
+ The TDM managed by this controller
+ type: object
+
+ properties:
+ reg:
+ minimum: 0
+ maximum: 1
+ description:
+ The TDM number for this TDM, 0 for TDMa and 1 for TDMb
+
+ fsl,common-rxtx-pins:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Use common pins for both transmit and receive
+
+ clocks: true
+ clock-names: true
+
+ fsl,mode:
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [normal, echo, internal-loopback, control-loopback]
+ default: normal
+ description: |
+ Operational mode:
+ - normal:
+ Normal operation
+ - echo:
+ Automatic echo. Rx data is resent on Tx
+ - internal-loopback:
+ The TDM transmitter is connected to the receiver.
+ Data appears on Tx pin.
+ - control-loopback:
+ The TDM transmitter is connected to the receiver.
+ The Tx pin is disconnected.
+
+ fsl,rx-frame-sync-delay:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1, 2, 3]
+ default: 0
+ description: |
+ Receive frame sync delay.
+ Indicates the delay between the Rx sync and the first bit of the
+ Rx frame. 0 for no bit delay. 1, 2 or 3 for 1, 2 or 3 bits delay.
+
+ fsl,tx-frame-sync-delay:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1, 2, 3]
+ default: 0
+ description: |
+ Transmit frame sync delay.
+ Indicates the delay between the Tx sync and the first bit of the
+ Tx frame. 0 for no bit delay. 1, 2 or 3 for 1, 2 or 3 bits delay.
+
+ fsl,clock-falling-edge:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: |
+ Data is sent on falling edge of the clock (and received on the
+ rising edge).
+ If 'clock-falling-edge' is not present, data is sent on the
+ rising edge (and received on the falling edge).
+
+ fsl,fsync-rising-edge:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Frame sync pulses are sampled with the rising edge of the channel
+ clock. If 'fsync-rising-edge' is not present, pulses are sample
+ with e falling edge.
+
+ fsl,double-speed-clock:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ The channel clock is twice the data rate.
+
+ fsl,grant-mode:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Grant mode enabled.
+
+ tx_ts_routes:
+ $ref: /schemas/types.yaml#/definitions/uint32-matrix
+ description: |
+ A list of tupple that indicates the Tx time-slots routes.
+ tx_ts_routes =
+ < 2 0 0>, /* The first 2 time slots are not used */
+ < 3 1 0>, /* The next 3 ones are route to SCC2 */
+ < 4 0 0>, /* The next 4 ones are not used */
+ < 2 2 0>; /* The nest 2 ones are route to SCC3 */
+ items:
+ items:
+ - description:
+ The number of time-slots
+ minimum: 1
+ maximum: 64
+ - description: |
+ The source serial interface (dt-bindings/soc/fsl-tsa.h
+ defines these values)
+ - 0: No destination
+ - 1: SCC2
+ - 2: SCC3
+ - 3: SCC4
+ - 4: SMC1
+ - 5: SMC2
+ enum: [0, 1, 2, 3, 4, 5]
+ - description:
+ The route flags (reserved)
+ const: 0
+ minItems: 1
+ maxItems: 64
+
+ rx_ts_routes:
+ $ref: /schemas/types.yaml#/definitions/uint32-matrix
+ description: |
+ A list of tupple that indicates the Rx time-slots routes.
+ tx_ts_routes =
+ < 2 0 0>, /* The first 2 time slots are not used */
+ < 3 1 0>, /* The next 3 ones are route from SCC2 */
+ < 4 0 0>, /* The next 4 ones are not used */
+ < 2 2 0>; /* The nest 2 ones are route from SCC3 */
+ items:
+ items:
+ - description:
+ The number of time-slots
+ minimum: 1
+ maximum: 64
+ - description: |
+ The destination serial interface (dt-bindings/soc/fsl-tsa.h
+ defines these values)
+ - 0: No destination
+ - 1: SCC2
+ - 2: SCC3
+ - 3: SCC4
+ - 4: SMC1
+ - 5: SMC2
+ enum: [0, 1, 2, 3, 4, 5]
+ - description:
+ The route flags (reserved)
+ const: 0
+ minItems: 1
+ maxItems: 64
+
+ allOf:
+ - if:
+ properties:
+ fsl,common-rxtx-pins:
+ type: 'null'
+ then:
+ properties:
+ clocks:
+ items:
+ - description: External clock connected to L1RSYNC pin
+ - description: External clock connected to L1RCLK pin
+ - description: External clock connected to L1TSYNC pin
+ - description: External clock connected to L1TCLK pin
+ clock-names:
+ items:
+ - const: l1rsync
+ - const: l1rclk
+ - const: l1tsync
+ - const: l1tclk
+ else:
+ properties:
+ clocks:
+ items:
+ - description: External clock connected to L1RSYNC pin
+ - description: External clock connected to L1RCLK pin
+ clock-names:
+ items:
+ - const: l1rsync
+ - const: l1rclk
+
+ required:
+ - reg
+ - clocks
+ - clock-names
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - '#address-cells'
+ - '#size-cells'
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/soc/fsl-tsa.h>
+
+ tsa@ae0 {
+ compatible = "fsl,mpc885-tsa", "fsl,cpm1-tsa";
+ reg = <0xae0 0x10>,
+ <0xc00 0x200>;
+ reg-names = "si_regs", "si_ram";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ tdm@0 {
+ /* TDMa */
+ reg = <0>;
+
+ clocks = <&clk_l1rsynca>, <&clk_l1rclka>;
+ clock-names = "l1rsync", "l1rclk";
+
+ fsl,common-rxtx-pins;
+ fsl,fsync-rising-edge;
+
+ tx_ts_routes = < 2 0 0>, /* TS 0..1 */
+ < 24 FSL_CPM_TSA_SCC4 0>, /* TS 2..25 */
+ < 1 0 0>, /* TS 26 */
+ < 5 FSL_CPM_TSA_SCC3 0>; /* TS 27..31 */
+
+ rx_ts_routes = < 2 0 0>, /* TS 0..1 */
+ < 24 FSL_CPM_TSA_SCC4 0>, /* 2..25 */
+ < 1 0 0>, /* TS 26 */
+ < 5 FSL_CPM_TSA_SCC3 0>; /* TS 27..31 */
+ };
+ };
diff --git a/include/dt-bindings/soc/fsl-tsa.h b/include/dt-bindings/soc/fsl-tsa.h
new file mode 100644
index 000000000000..9d09468694a2
--- /dev/null
+++ b/include/dt-bindings/soc/fsl-tsa.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later OR MIT */
+
+#ifndef __DT_BINDINGS_SOC_FSL_TSA_H
+#define __DT_BINDINGS_SOC_FSL_TSA_H
+
+#define FSL_CPM_TSA_NU 0 /* Pseuso Cell Id for not used item */
+#define FSL_CPM_TSA_SCC2 1
+#define FSL_CPM_TSA_SCC3 2
+#define FSL_CPM_TSA_SCC4 3
+#define FSL_CPM_TSA_SMC1 4
+#define FSL_CPM_TSA_SMC2 5
+
+#define FSL_CPM_TSA_NBCELL 6
+
+#endif
--
2.38.1
On 06/01/2023 17:37, Herve Codina wrote:
> Add support for the time slot assigner (TSA)
> available in some PowerQUICC SoC such as MPC885
> or MPC866.
>
> Signed-off-by: Herve Codina <[email protected]>
> ---
> .../bindings/soc/fsl/cpm_qe/fsl,tsa.yaml | 262 ++++++++++++++++++
> include/dt-bindings/soc/fsl-tsa.h | 15 +
> 2 files changed, 277 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,tsa.yaml
> create mode 100644 include/dt-bindings/soc/fsl-tsa.h
>
> diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,tsa.yaml b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,tsa.yaml
> new file mode 100644
> index 000000000000..7542c0fd8435
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,tsa.yaml
> @@ -0,0 +1,262 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/fsl/cpm_qe/fsl,tsa.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: PowerQUICC CPM Time-slot assigner (TSA) controller
> +
> +maintainers:
> + - Herve Codina <[email protected]>
> +
> +description: |
> + The TSA is the time-slot assigner that can be found on some
> + PowerQUICC SoC.
> + Its purpose is to route some TDM time-slots to other internal
> + serial controllers.
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - fsl,mpc885-tsa
> + - fsl,mpc866-tsa
> + - const: fsl,cpm1-tsa
> +
> + reg:
> + items:
> + - description: SI (Serial Interface) register base
> + - description: SI RAM base
> +
> + reg-names:
> + items:
> + - const: si_regs
> + - const: si_ram
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> +patternProperties:
> + "^tdm@[0-1]$":
Use consistent quotes - either ' or "
> + description:
> + The TDM managed by this controller
> + type: object
> +
> + properties:
> + reg:
> + minimum: 0
> + maximum: 1
> + description:
> + The TDM number for this TDM, 0 for TDMa and 1 for TDMb
> +
> + fsl,common-rxtx-pins:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + Use common pins for both transmit and receive
What are the "common" pins? Without this property device is using
uncommon pins? This does not make sense...
> +
> + clocks: true
> + clock-names: true
Both need constraints.
> +
> + fsl,mode:
> + $ref: /schemas/types.yaml#/definitions/string
> + enum: [normal, echo, internal-loopback, control-loopback]
> + default: normal
> + description: |
> + Operational mode:
> + - normal:
> + Normal operation
> + - echo:
> + Automatic echo. Rx data is resent on Tx
> + - internal-loopback:
> + The TDM transmitter is connected to the receiver.
> + Data appears on Tx pin.
> + - control-loopback:
> + The TDM transmitter is connected to the receiver.
> + The Tx pin is disconnected.
> +
> + fsl,rx-frame-sync-delay:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1, 2, 3]
> + default: 0
> + description: |
> + Receive frame sync delay.
Delay in what units?
> + Indicates the delay between the Rx sync and the first bit of the
> + Rx frame. 0 for no bit delay. 1, 2 or 3 for 1, 2 or 3 bits delay.
> +
> + fsl,tx-frame-sync-delay:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1, 2, 3]
> + default: 0
> + description: |
> + Transmit frame sync delay.
> + Indicates the delay between the Tx sync and the first bit of the
> + Tx frame. 0 for no bit delay. 1, 2 or 3 for 1, 2 or 3 bits delay.
> +
> + fsl,clock-falling-edge:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description: |
> + Data is sent on falling edge of the clock (and received on the
> + rising edge).
> + If 'clock-falling-edge' is not present, data is sent on the
> + rising edge (and received on the falling edge).
> +
> + fsl,fsync-rising-edge:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + Frame sync pulses are sampled with the rising edge of the channel
> + clock. If 'fsync-rising-edge' is not present, pulses are sample
> + with e falling edge.
> +
> + fsl,double-speed-clock:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + The channel clock is twice the data rate.
> +
> + fsl,grant-mode:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + Grant mode enabled.
This we know from property name. You need to describe what it is and
what it does.
> +
> + tx_ts_routes:
No underscores, missing vendor prefix.
> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> + description: |
> + A list of tupple that indicates the Tx time-slots routes.
> + tx_ts_routes =
> + < 2 0 0>, /* The first 2 time slots are not used */
> + < 3 1 0>, /* The next 3 ones are route to SCC2 */
> + < 4 0 0>, /* The next 4 ones are not used */
> + < 2 2 0>; /* The nest 2 ones are route to SCC3 */
> + items:
> + items:
> + - description:
> + The number of time-slots
> + minimum: 1
> + maximum: 64
> + - description: |
> + The source serial interface (dt-bindings/soc/fsl-tsa.h
> + defines these values)
> + - 0: No destination
> + - 1: SCC2
> + - 2: SCC3
> + - 3: SCC4
> + - 4: SMC1
> + - 5: SMC2
> + enum: [0, 1, 2, 3, 4, 5]
> + - description:
> + The route flags (reserved)
Why part of binding is reserved?
> + const: 0
> + minItems: 1
> + maxItems: 64
> +
> + rx_ts_routes:
> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> + description: |
> + A list of tupple that indicates the Rx time-slots routes.
> + tx_ts_routes =
> + < 2 0 0>, /* The first 2 time slots are not used */
> + < 3 1 0>, /* The next 3 ones are route from SCC2 */
> + < 4 0 0>, /* The next 4 ones are not used */
> + < 2 2 0>; /* The nest 2 ones are route from SCC3 */
> + items:
> + items:
> + - description:
> + The number of time-slots
> + minimum: 1
> + maximum: 64
> + - description: |
> + The destination serial interface (dt-bindings/soc/fsl-tsa.h
> + defines these values)
> + - 0: No destination
> + - 1: SCC2
> + - 2: SCC3
> + - 3: SCC4
> + - 4: SMC1
> + - 5: SMC2
> + enum: [0, 1, 2, 3, 4, 5]
> + - description:
> + The route flags (reserved)
> + const: 0
> + minItems: 1
> + maxItems: 64
> +
> + allOf:
> + - if:
> + properties:
> + fsl,common-rxtx-pins:
> + type: 'null'
What is this exactly? If check for property present, it's wrong. Should
be test if it is in required.
> + then:
> + properties:
> + clocks:
> + items:
> + - description: External clock connected to L1RSYNC pin
> + - description: External clock connected to L1RCLK pin
> + - description: External clock connected to L1TSYNC pin
> + - description: External clock connected to L1TCLK pin
> + clock-names:
> + items:
> + - const: l1rsync
> + - const: l1rclk
> + - const: l1tsync
> + - const: l1tclk
> + else:
> + properties:
> + clocks:
> + items:
> + - description: External clock connected to L1RSYNC pin
> + - description: External clock connected to L1RCLK pin
> + clock-names:
> + items:
> + - const: l1rsync
> + - const: l1rclk
> +
> + required:
> + - reg
> + - clocks
> + - clock-names
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - '#address-cells'
> + - '#size-cells'
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/soc/fsl-tsa.h>
> +
> + tsa@ae0 {
> + compatible = "fsl,mpc885-tsa", "fsl,cpm1-tsa";
> + reg = <0xae0 0x10>,
> + <0xc00 0x200>;
> + reg-names = "si_regs", "si_ram";
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + tdm@0 {
> + /* TDMa */
> + reg = <0>;
> +
> + clocks = <&clk_l1rsynca>, <&clk_l1rclka>;
> + clock-names = "l1rsync", "l1rclk";
> +
> + fsl,common-rxtx-pins;
> + fsl,fsync-rising-edge;
> +
> + tx_ts_routes = < 2 0 0>, /* TS 0..1 */
> + < 24 FSL_CPM_TSA_SCC4 0>, /* TS 2..25 */
> + < 1 0 0>, /* TS 26 */
> + < 5 FSL_CPM_TSA_SCC3 0>; /* TS 27..31 */
> +
> + rx_ts_routes = < 2 0 0>, /* TS 0..1 */
> + < 24 FSL_CPM_TSA_SCC4 0>, /* 2..25 */
> + < 1 0 0>, /* TS 26 */
> + < 5 FSL_CPM_TSA_SCC3 0>; /* TS 27..31 */
> + };
> + };
> diff --git a/include/dt-bindings/soc/fsl-tsa.h b/include/dt-bindings/soc/fsl-tsa.h
> new file mode 100644
> index 000000000000..9d09468694a2
> --- /dev/null
> +++ b/include/dt-bindings/soc/fsl-tsa.h
Filename should match binding filename.
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later OR MIT */
A bit weird license... cannot be the same as binding?
> +
> +#ifndef __DT_BINDINGS_SOC_FSL_TSA_H
> +#define __DT_BINDINGS_SOC_FSL_TSA_H
> +
> +#define FSL_CPM_TSA_NU 0 /* Pseuso Cell Id for not used item */
Why defining unused IDs in binding header? These are IDs, not some
hardware/register values.
> +#define FSL_CPM_TSA_SCC2 1
> +#define FSL_CPM_TSA_SCC3 2
> +#define FSL_CPM_TSA_SCC4 3
> +#define FSL_CPM_TSA_SMC1 4
> +#define FSL_CPM_TSA_SMC2 5
> +
> +#define FSL_CPM_TSA_NBCELL 6
Drop.
> +
> +#endif
Best regards,
Krzysztof
Hi Krzysztof,
On Sun, 8 Jan 2023 16:10:38 +0100
Krzysztof Kozlowski <[email protected]> wrote:
[...]
> > + '#size-cells':
> > + const: 0
> > +
> > +patternProperties:
> > + "^tdm@[0-1]$":
>
> Use consistent quotes - either ' or "
Ok, I will change on v3.
I will also change them on the other bindings present in the
series.
>
> > + description:
> > + The TDM managed by this controller
> > + type: object
> > +
> > + properties:
> > + reg:
> > + minimum: 0
> > + maximum: 1
> > + description:
> > + The TDM number for this TDM, 0 for TDMa and 1 for TDMb
> > +
> > + fsl,common-rxtx-pins:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description:
> > + Use common pins for both transmit and receive
>
> What are the "common" pins? Without this property device is using
> uncommon pins? This does not make sense...
Common in the "shared" sense.
The hardware can use dedicated pins for Tx clock, Tx sync,
Rx clock and Rx sync or use only 2 pins, Tx/Rx clock and
Rx/Rx sync.
Without the property, we use the 4 pins and with the property,
we use 2 pins.
>
> > +
> > + clocks: true
> > + clock-names: true
>
> Both need constraints.
The constraints are present later in the file as the number
of clocks depends on the 'fsl,common-rxtx-pins' property.
I will remove these two lines in the v3 series as they are
not needed. 'clocks' and 'clock-names' are handled in the
conditional part.
>
[...]
> > +
> > + fsl,rx-frame-sync-delay:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + enum: [0, 1, 2, 3]
> > + default: 0
> > + description: |
> > + Receive frame sync delay.
>
> Delay in what units?
The unit is a number of bits.
I will rename to fsl,rx-frame-sync-delay-bits and change the description
to 'Receive frame sync delay in number of bits'
I will do also the same for fsl,tx-frame-sync-delay property.
>
> > + Indicates the delay between the Rx sync and the first bit of the
> > + Rx frame. 0 for no bit delay. 1, 2 or 3 for 1, 2 or 3 bits delay.
> > +
> > + fsl,tx-frame-sync-delay:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + enum: [0, 1, 2, 3]
> > + default: 0
> > + description: |
> > + Transmit frame sync delay.
> > + Indicates the delay between the Tx sync and the first bit of the
> > + Tx frame. 0 for no bit delay. 1, 2 or 3 for 1, 2 or 3 bits delay.
> > +
> > + fsl,clock-falling-edge:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description: |
> > + Data is sent on falling edge of the clock (and received on the
> > + rising edge).
> > + If 'clock-falling-edge' is not present, data is sent on the
> > + rising edge (and received on the falling edge).
> > +
> > + fsl,fsync-rising-edge:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description:
> > + Frame sync pulses are sampled with the rising edge of the channel
> > + clock. If 'fsync-rising-edge' is not present, pulses are sample
> > + with e falling edge.
> > +
> > + fsl,double-speed-clock:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description:
> > + The channel clock is twice the data rate.
> > +
> > + fsl,grant-mode:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description:
> > + Grant mode enabled.
>
> This we know from property name. You need to describe what it is and
> what it does.
Instead of describing it, I will simply remove the property (I should
have done already).
I cannot test the 'grant mode' enabled with my hardware and so
I prefer keeping it disabled.
This property, if needed, could be add later setting it optional
with default to 'disabled'.
>
> > +
> > + tx_ts_routes:
>
> No underscores, missing vendor prefix.
Indeed, will be change to fsl,tx-ts-routes (idem for rx_ts_routes).
>
> > + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > + description: |
> > + A list of tupple that indicates the Tx time-slots routes.
> > + tx_ts_routes =
> > + < 2 0 0>, /* The first 2 time slots are not used */
> > + < 3 1 0>, /* The next 3 ones are route to SCC2 */
> > + < 4 0 0>, /* The next 4 ones are not used */
> > + < 2 2 0>; /* The nest 2 ones are route to SCC3 */
> > + items:
> > + items:
> > + - description:
> > + The number of time-slots
> > + minimum: 1
> > + maximum: 64
> > + - description: |
> > + The source serial interface (dt-bindings/soc/fsl-tsa.h
> > + defines these values)
> > + - 0: No destination
> > + - 1: SCC2
> > + - 2: SCC3
> > + - 3: SCC4
> > + - 4: SMC1
> > + - 5: SMC2
> > + enum: [0, 1, 2, 3, 4, 5]
> > + - description:
> > + The route flags (reserved)
>
> Why part of binding is reserved?
The 'reserved' part will be removed in v3.
Same for the rx route table.
>
> > + const: 0
> > + minItems: 1
> > + maxItems: 64
> > +
> > + rx_ts_routes:
> > + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > + description: |
> > + A list of tupple that indicates the Rx time-slots routes.
> > + tx_ts_routes =
> > + < 2 0 0>, /* The first 2 time slots are not used */
> > + < 3 1 0>, /* The next 3 ones are route from SCC2 */
> > + < 4 0 0>, /* The next 4 ones are not used */
> > + < 2 2 0>; /* The nest 2 ones are route from SCC3 */
> > + items:
> > + items:
> > + - description:
> > + The number of time-slots
> > + minimum: 1
> > + maximum: 64
> > + - description: |
> > + The destination serial interface (dt-bindings/soc/fsl-tsa.h
> > + defines these values)
> > + - 0: No destination
> > + - 1: SCC2
> > + - 2: SCC3
> > + - 3: SCC4
> > + - 4: SMC1
> > + - 5: SMC2
> > + enum: [0, 1, 2, 3, 4, 5]
> > + - description:
> > + The route flags (reserved)
> > + const: 0
> > + minItems: 1
> > + maxItems: 64
> > +
> > + allOf:
> > + - if:
> > + properties:
> > + fsl,common-rxtx-pins:
> > + type: 'null'
>
> What is this exactly? If check for property present, it's wrong. Should
> be test if it is in required.
Yes, it was a check for the property presence.
If we not use the 'fsl,common-rxtx-pins', we need 4 clocks.
If we use the 'fsl,common-rxtx-pins', we need 2 clocks (Rx part and Tx
part use the same CLK and SYNC clocks).
How can I describe this ?
Is the check for the property presence incorrect ?
Should I always describe 4 clocks even if only 2 are used ?
>
> > + then:
> > + properties:
> > + clocks:
> > + items:
> > + - description: External clock connected to L1RSYNC pin
> > + - description: External clock connected to L1RCLK pin
> > + - description: External clock connected to L1TSYNC pin
> > + - description: External clock connected to L1TCLK pin
> > + clock-names:
> > + items:
> > + - const: l1rsync
> > + - const: l1rclk
> > + - const: l1tsync
> > + - const: l1tclk
> > + else:
> > + properties:
> > + clocks:
> > + items:
> > + - description: External clock connected to L1RSYNC pin
> > + - description: External clock connected to L1RCLK pin
> > + clock-names:
> > + items:
> > + - const: l1rsync
> > + - const: l1rclk
> > +
> > + required:
> > + - reg
> > + - clocks
> > + - clock-names
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - reg-names
> > + - '#address-cells'
> > + - '#size-cells'
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/soc/fsl-tsa.h>
> > +
> > + tsa@ae0 {
> > + compatible = "fsl,mpc885-tsa", "fsl,cpm1-tsa";
> > + reg = <0xae0 0x10>,
> > + <0xc00 0x200>;
> > + reg-names = "si_regs", "si_ram";
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + tdm@0 {
> > + /* TDMa */
> > + reg = <0>;
> > +
> > + clocks = <&clk_l1rsynca>, <&clk_l1rclka>;
> > + clock-names = "l1rsync", "l1rclk";
> > +
> > + fsl,common-rxtx-pins;
> > + fsl,fsync-rising-edge;
> > +
> > + tx_ts_routes = < 2 0 0>, /* TS 0..1 */
> > + < 24 FSL_CPM_TSA_SCC4 0>, /* TS 2..25 */
> > + < 1 0 0>, /* TS 26 */
> > + < 5 FSL_CPM_TSA_SCC3 0>; /* TS 27..31 */
> > +
> > + rx_ts_routes = < 2 0 0>, /* TS 0..1 */
> > + < 24 FSL_CPM_TSA_SCC4 0>, /* 2..25 */
> > + < 1 0 0>, /* TS 26 */
> > + < 5 FSL_CPM_TSA_SCC3 0>; /* TS 27..31 */
> > + };
> > + };
> > diff --git a/include/dt-bindings/soc/fsl-tsa.h b/include/dt-bindings/soc/fsl-tsa.h
> > new file mode 100644
> > index 000000000000..9d09468694a2
> > --- /dev/null
> > +++ b/include/dt-bindings/soc/fsl-tsa.h
>
> Filename should match binding filename.
Right, I will rename to fsl,tsa.h
>
> > @@ -0,0 +1,15 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later OR MIT */
>
> A bit weird license... cannot be the same as binding?
Yes, will be change to GPL-2.0-only OR BSD-2-Clause
>
> > +
> > +#ifndef __DT_BINDINGS_SOC_FSL_TSA_H
> > +#define __DT_BINDINGS_SOC_FSL_TSA_H
> > +
> > +#define FSL_CPM_TSA_NU 0 /* Pseuso Cell Id for not used item */
>
> Why defining unused IDs in binding header? These are IDs, not some
> hardware/register values.
It is the binding value for 'No destination' in the tx and rx route table.
This binding value means 'not used' or 'discard' the time-slot.
The data related to an item in the routing table with this value
will be discarded and not used.
>
> > +#define FSL_CPM_TSA_SCC2 1
> > +#define FSL_CPM_TSA_SCC3 2
> > +#define FSL_CPM_TSA_SCC4 3
> > +#define FSL_CPM_TSA_SMC1 4
> > +#define FSL_CPM_TSA_SMC2 5
> > +
> > +#define FSL_CPM_TSA_NBCELL 6
>
> Drop.
Ok, will be removed in v3.
>
> > +
> > +#endif
>
> Best regards,
> Krzysztof
>
Thanks for your review.
Best regards,
Hervé
--
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On 10/01/2023 09:04, Herve Codina wrote:
> Hi Krzysztof,
>
> On Sun, 8 Jan 2023 16:10:38 +0100
> Krzysztof Kozlowski <[email protected]> wrote:
>
> [...]
>
>>> + '#size-cells':
>>> + const: 0
>>> +
>>> +patternProperties:
>>> + "^tdm@[0-1]$":
>>
>> Use consistent quotes - either ' or "
>
> Ok, I will change on v3.
> I will also change them on the other bindings present in the
> series.
>
>>
>>> + description:
>>> + The TDM managed by this controller
>>> + type: object
>>> +
>>> + properties:
>>> + reg:
>>> + minimum: 0
>>> + maximum: 1
>>> + description:
>>> + The TDM number for this TDM, 0 for TDMa and 1 for TDMb
>>> +
>>> + fsl,common-rxtx-pins:
>>> + $ref: /schemas/types.yaml#/definitions/flag
>>> + description:
>>> + Use common pins for both transmit and receive
>>
>> What are the "common" pins? Without this property device is using
>> uncommon pins? This does not make sense...
>
> Common in the "shared" sense.
> The hardware can use dedicated pins for Tx clock, Tx sync,
> Rx clock and Rx sync or use only 2 pins, Tx/Rx clock and
> Rx/Rx sync.
>
> Without the property, we use the 4 pins and with the property,
> we use 2 pins.
Just use this as description.
>
>>
>>> +
>>> + clocks: true
>>> + clock-names: true
>>
>> Both need constraints.
>
> The constraints are present later in the file as the number
> of clocks depends on the 'fsl,common-rxtx-pins' property.
OK, but still top level properties need widest constraints.
>
> I will remove these two lines in the v3 series as they are
> not needed. 'clocks' and 'clock-names' are handled in the
> conditional part.
No, top level properties must contain them.
>
>>
> [...]
>>> +
>>> + fsl,rx-frame-sync-delay:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + enum: [0, 1, 2, 3]
>>> + default: 0
>>> + description: |
>>> + Receive frame sync delay.
>>
>> Delay in what units?
>
> The unit is a number of bits.
> I will rename to fsl,rx-frame-sync-delay-bits and change the description
> to 'Receive frame sync delay in number of bits'
>
> I will do also the same for fsl,tx-frame-sync-delay property.
OK
>
>>
>>> + Indicates the delay between the Rx sync and the first bit of the
>>> + Rx frame. 0 for no bit delay. 1, 2 or 3 for 1, 2 or 3 bits delay.
>>> +
>>> + fsl,tx-frame-sync-delay:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + enum: [0, 1, 2, 3]
>>> + default: 0
>>> + description: |
>>> + Transmit frame sync delay.
>>> + Indicates the delay between the Tx sync and the first bit of the
>>> + Tx frame. 0 for no bit delay. 1, 2 or 3 for 1, 2 or 3 bits delay.
>>> +
>>> + fsl,clock-falling-edge:
>>> + $ref: /schemas/types.yaml#/definitions/flag
>>> + description: |
>>> + Data is sent on falling edge of the clock (and received on the
>>> + rising edge).
>>> + If 'clock-falling-edge' is not present, data is sent on the
>>> + rising edge (and received on the falling edge).
>>> +
>>> + fsl,fsync-rising-edge:
>>> + $ref: /schemas/types.yaml#/definitions/flag
>>> + description:
>>> + Frame sync pulses are sampled with the rising edge of the channel
>>> + clock. If 'fsync-rising-edge' is not present, pulses are sample
>>> + with e falling edge.
>>> +
>>> + fsl,double-speed-clock:
>>> + $ref: /schemas/types.yaml#/definitions/flag
>>> + description:
>>> + The channel clock is twice the data rate.
>>> +
>>> + fsl,grant-mode:
>>> + $ref: /schemas/types.yaml#/definitions/flag
>>> + description:
>>> + Grant mode enabled.
>>
>> This we know from property name. You need to describe what it is and
>> what it does.
>
> Instead of describing it, I will simply remove the property (I should
> have done already).
> I cannot test the 'grant mode' enabled with my hardware and so
> I prefer keeping it disabled.
> This property, if needed, could be add later setting it optional
> with default to 'disabled'.
>
>>
>>> +
>>> + tx_ts_routes:
>>
>> No underscores, missing vendor prefix.
>
> Indeed, will be change to fsl,tx-ts-routes (idem for rx_ts_routes).
>
>>
>>> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
>>> + description: |
>>> + A list of tupple that indicates the Tx time-slots routes.
>>> + tx_ts_routes =
>>> + < 2 0 0>, /* The first 2 time slots are not used */
>>> + < 3 1 0>, /* The next 3 ones are route to SCC2 */
>>> + < 4 0 0>, /* The next 4 ones are not used */
>>> + < 2 2 0>; /* The nest 2 ones are route to SCC3 */
>>> + items:
>>> + items:
>>> + - description:
>>> + The number of time-slots
>>> + minimum: 1
>>> + maximum: 64
>>> + - description: |
>>> + The source serial interface (dt-bindings/soc/fsl-tsa.h
>>> + defines these values)
>>> + - 0: No destination
>>> + - 1: SCC2
>>> + - 2: SCC3
>>> + - 3: SCC4
>>> + - 4: SMC1
>>> + - 5: SMC2
>>> + enum: [0, 1, 2, 3, 4, 5]
>>> + - description:
>>> + The route flags (reserved)
>>
>> Why part of binding is reserved?
>
> The 'reserved' part will be removed in v3.
> Same for the rx route table.
>
>>
>>> + const: 0
>>> + minItems: 1
>>> + maxItems: 64
>>> +
>>> + rx_ts_routes:
>>> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
>>> + description: |
>>> + A list of tupple that indicates the Rx time-slots routes.
>>> + tx_ts_routes =
>>> + < 2 0 0>, /* The first 2 time slots are not used */
>>> + < 3 1 0>, /* The next 3 ones are route from SCC2 */
>>> + < 4 0 0>, /* The next 4 ones are not used */
>>> + < 2 2 0>; /* The nest 2 ones are route from SCC3 */
>>> + items:
>>> + items:
>>> + - description:
>>> + The number of time-slots
>>> + minimum: 1
>>> + maximum: 64
>>> + - description: |
>>> + The destination serial interface (dt-bindings/soc/fsl-tsa.h
>>> + defines these values)
>>> + - 0: No destination
>>> + - 1: SCC2
>>> + - 2: SCC3
>>> + - 3: SCC4
>>> + - 4: SMC1
>>> + - 5: SMC2
>>> + enum: [0, 1, 2, 3, 4, 5]
>>> + - description:
>>> + The route flags (reserved)
>>> + const: 0
>>> + minItems: 1
>>> + maxItems: 64
>>> +
>>> + allOf:
>>> + - if:
>>> + properties:
>>> + fsl,common-rxtx-pins:
>>> + type: 'null'
>>
>> What is this exactly? If check for property present, it's wrong. Should
>> be test if it is in required.
>
> Yes, it was a check for the property presence.
>
> If we not use the 'fsl,common-rxtx-pins', we need 4 clocks.
> If we use the 'fsl,common-rxtx-pins', we need 2 clocks (Rx part and Tx
> part use the same CLK and SYNC clocks).
https://elixir.bootlin.com/linux/v6.2-rc3/source/Documentation/devicetree/bindings/net/qcom,ipa.yaml#L174
>
> How can I describe this ?
> Is the check for the property presence incorrect ?
>
> Should I always describe 4 clocks even if only 2 are used ?
>
>>
>>> + then:
>>> + properties:
>>> + clocks:
>>> + items:
>>> + - description: External clock connected to L1RSYNC pin
>>> + - description: External clock connected to L1RCLK pin
>>> + - description: External clock connected to L1TSYNC pin
>>> + - description: External clock connected to L1TCLK pin
>>> + clock-names:
>>> + items:
>>> + - const: l1rsync
>>> + - const: l1rclk
>>> + - const: l1tsync
>>> + - const: l1tclk
>>> + else:
>>> + properties:
>>> + clocks:
>>> + items:
>>> + - description: External clock connected to L1RSYNC pin
>>> + - description: External clock connected to L1RCLK pin
>>> + clock-names:
>>> + items:
>>> + - const: l1rsync
>>> + - const: l1rclk
>>> +
>>> + required:
>>> + - reg
>>> + - clocks
>>> + - clock-names
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - reg-names
>>> + - '#address-cells'
>>> + - '#size-cells'
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + #include <dt-bindings/soc/fsl-tsa.h>
>>> +
>>> + tsa@ae0 {
>>> + compatible = "fsl,mpc885-tsa", "fsl,cpm1-tsa";
>>> + reg = <0xae0 0x10>,
>>> + <0xc00 0x200>;
>>> + reg-names = "si_regs", "si_ram";
>>> +
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + tdm@0 {
>>> + /* TDMa */
>>> + reg = <0>;
>>> +
>>> + clocks = <&clk_l1rsynca>, <&clk_l1rclka>;
>>> + clock-names = "l1rsync", "l1rclk";
>>> +
>>> + fsl,common-rxtx-pins;
>>> + fsl,fsync-rising-edge;
>>> +
>>> + tx_ts_routes = < 2 0 0>, /* TS 0..1 */
>>> + < 24 FSL_CPM_TSA_SCC4 0>, /* TS 2..25 */
>>> + < 1 0 0>, /* TS 26 */
>>> + < 5 FSL_CPM_TSA_SCC3 0>; /* TS 27..31 */
>>> +
>>> + rx_ts_routes = < 2 0 0>, /* TS 0..1 */
>>> + < 24 FSL_CPM_TSA_SCC4 0>, /* 2..25 */
>>> + < 1 0 0>, /* TS 26 */
>>> + < 5 FSL_CPM_TSA_SCC3 0>; /* TS 27..31 */
>>> + };
>>> + };
>>> diff --git a/include/dt-bindings/soc/fsl-tsa.h b/include/dt-bindings/soc/fsl-tsa.h
>>> new file mode 100644
>>> index 000000000000..9d09468694a2
>>> --- /dev/null
>>> +++ b/include/dt-bindings/soc/fsl-tsa.h
>>
>> Filename should match binding filename.
>
> Right, I will rename to fsl,tsa.h
If your binding was fsl,tsa.yaml.
Best regards,
Krzysztof