[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230912-overplay-donated-080eb97803d6@spud>
Date: Tue, 12 Sep 2023 19:13:32 +0100
From: Conor Dooley <conor@...nel.org>
To: Herve Codina <herve.codina@...tlin.com>
Cc: "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Andrew Lunn <andrew@...n.ch>, Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>, Lee Jones <lee@...nel.org>,
Linus Walleij <linus.walleij@...aro.org>,
Qiang Zhao <qiang.zhao@....com>, Li Yang <leoyang.li@....com>,
Liam Girdwood <lgirdwood@...il.com>,
Mark Brown <broonie@...nel.org>, Jaroslav Kysela <perex@...ex.cz>,
Takashi Iwai <tiwai@...e.com>,
Shengjiu Wang <shengjiu.wang@...il.com>,
Xiubo Li <Xiubo.Lee@...il.com>, Fabio Estevam <festevam@...il.com>,
Nicolin Chen <nicoleotsuka@...il.com>,
Christophe Leroy <christophe.leroy@...roup.eu>,
Randy Dunlap <rdunlap@...radead.org>, netdev@...r.kernel.org,
linuxppc-dev@...ts.ozlabs.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, alsa-devel@...a-project.org,
Simon Horman <horms@...nel.org>,
Christophe JAILLET <christophe.jaillet@...adoo.fr>,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH v5 25/31] dt-bindings: net: Add the Lantiq PEF2256
E1/T1/J1 framer
Yo,
I'm not au fait enough with this to leave particularly meaningful
comments, so just some minor ones for you.
On Tue, Sep 12, 2023 at 12:14:44PM +0200, Herve Codina wrote:
> The Lantiq PEF2256 is a framer and line interface component designed to
> fulfill all required interfacing between an analog E1/T1/J1 line and the
> digital PCM system highway/H.100 bus.
>
> Signed-off-by: Herve Codina <herve.codina@...tlin.com>
> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
Missing a co-developed-by?
> ---
> .../bindings/net/lantiq,pef2256.yaml | 214 ++++++++++++++++++
> 1 file changed, 214 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/lantiq,pef2256.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/lantiq,pef2256.yaml b/Documentation/devicetree/bindings/net/lantiq,pef2256.yaml
> new file mode 100644
> index 000000000000..c4f21678bf6a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/lantiq,pef2256.yaml
> @@ -0,0 +1,214 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/lantiq,pef2256.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Lantiq PEF2256
> +
> +maintainers:
> + - Herve Codina <herve.codina@...tlin.com>
> +
> +description:
> + The Lantiq PEF2256, also known as Infineon PEF2256 or FALC56, is a framer and
> + line interface component designed to fulfill all required interfacing between
> + an analog E1/T1/J1 line and the digital PCM system highway/H.100 bus.
> +
> +properties:
> + compatible:
> + items:
> + - const: lantiq,pef2256
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + items:
> + - description: Master clock
My OCD is rather upset by the inconsistent capitalisation used here :/
> + - description: System Clock Receive
> + - description: System Clock Transmit
> +
> + clock-names:
> + items:
> + - const: mclk
> + - const: sclkr
> + - const: sclkx
> +
> + interrupts:
> + maxItems: 1
> +
> + reset-gpios:
> + description:
> + GPIO used to reset the device.
> + maxItems: 1
> +
> + pinctrl:
> + $ref: /schemas/pinctrl/pinctrl.yaml#
> + additionalProperties: false
> +
> + patternProperties:
> + '-pins$':
> + type: object
> + $ref: /schemas/pinctrl/pinmux-node.yaml#
> + additionalProperties: false
> +
> + properties:
> + pins:
> + enum: [ RPA, RPB, RPC, RPD, XPA, XPB, XPC, XPD ]
> +
> + function:
> + enum: [ SYPR, RFM, RFMB, RSIGM, RSIG, DLR, FREEZE, RFSP, LOS,
> + SYPX, XFMS, XSIG, TCLK, XMFB, XSIGM, DLX, XCLK, XLT,
> + GPI, GPOH, GPOL ]
> +
> + required:
> + - pins
> + - function
> +
> + lantiq,data-rate-bps:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [2048000, 4096000, 8192000, 16384000]
-kBps is a standard suffix, would it be worth using that instead here?
What you have would fit as even multiples.
Otherwise Rob, should dt-schema grow -bps as a standard suffix?
> + default: 2048000
> + description:
> + Data rate (bit per seconds) on the system highway.
> +
> + lantiq,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).
> +
> + lantiq,channel-phase:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1, 2, 3, 4, 5, 6, 7]
> + default: 0
> + description: |
> + The pef2256 delivers a full frame (32 8bit time-slots in E1 and 24 8bit
Just a wee nit, s/8bit/8-bit/ :)
Rest of this I don't really feel like I can really review.
Thanks,
Conor.
> + time-slots 8 8bit signaling in E1/J1) every 125us. This lead to a data
> + rate of 2048000 bit/s. When lantiq,data-rate-bps is more than 2048000
> + bit/s, the data (all 32 8bit) present in the frame are interleave with
> + unused time-slots. The lantiq,channel-phase property allows to set the
> + correct alignment of the interleave mechanism.
> + For instance, suppose lantiq,data-rate-bps = 8192000 (ie 4*2048000), and
> + lantiq,channel-phase = 2, the interleave schema with unused time-slots
> + (nu) and used time-slots (XX) for TSi is
> + nu nu XX nu nu nu XX nu nu nu XX nu
> + <-- TSi --> <- TSi+1 -> <- TSi+2 ->
> + With lantiq,data-rate-bps = 8192000, and lantiq,channel-phase = 1, the
> + interleave schema is
> + nu XX nu nu nu XX nu nu nu XX nu nu
> + <-- TSi --> <- TSi+1 -> <- TSi+2 ->
> + With lantiq,data-rate-bps = 4096000 (ie 2*2048000), and
> + lantiq,channel-phase = 1, the interleave schema is
> + nu XX nu XX nu XX
> + <-- TSi --> <- TSi+1 -> <- TSi+2 ->
> +
> +patternProperties:
> + '^codec(-([0-9]|[1-2][0-9]|3[0-1]))?$':
> + type: object
> + $ref: /schemas/sound/dai-common.yaml
> + unevaluatedProperties: false
> + description:
> + Codec provided by the pef2256. This codec allows to use some of the PCM
> + system highway time-slots as audio channels to transport audio data over
> + the E1/T1/J1 lines.
> + The time-slots used by the codec must be set and so, the properties
> + 'dai-tdm-slot-num', 'dai-tdm-slot-width', 'dai-tdm-slot-tx-mask' and
> + 'dai-tdm-slot-rx-mask' must be present in the sound card node for
> + sub-nodes that involve the codec. The codec uses 8bit time-slots.
> + 'dai-tdm-tdm-slot-with' must be set to 8.
> + The tx and rx masks define the pef2256 time-slots assigned to the codec.
> +
> + properties:
> + compatible:
> + const: lantiq,pef2256-codec
> +
> + '#sound-dai-cells':
> + const: 0
> +
> + required:
> + - compatible
> + - '#sound-dai-cells'
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + pef2256: framer@...0000 {
> + compatible = "lantiq,pef2256";
> + reg = <0x2000000 0x100>;
> + interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
> + interrupt-parent = <&intc>;
> + clocks = <&clk_mclk>, <&clk_sclkr>, <&clk_sclkx>;
> + clock-names = "mclk", "sclkr", "sclkx";
> + reset-gpios = <&gpio 11 GPIO_ACTIVE_LOW>;
> + lantiq,data-rate-bps = <4096000>;
> +
> + pinctrl {
> + pef2256_rpa_sypr: rpa-pins {
> + pins = "RPA";
> + function = "SYPR";
> + };
> + pef2256_xpa_sypx: xpa-pins {
> + pins = "XPA";
> + function = "SYPX";
> + };
> + };
> +
> + pef2256_codec0: codec-0 {
> + compatible = "lantiq,pef2256-codec";
> + #sound-dai-cells = <0>;
> + sound-name-prefix = "PEF2256_0";
> + };
> +
> + pef2256_codec1: codec-1 {
> + compatible = "lantiq,pef2256-codec";
> + #sound-dai-cells = <0>;
> + sound-name-prefix = "PEF2256_1";
> + };
> + };
> +
> + sound {
> + compatible = "simple-audio-card";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + simple-audio-card,dai-link@0 { /* CPU DAI1 - pef2256 codec 1 */
> + reg = <0>;
> + cpu {
> + sound-dai = <&cpu_dai1>;
> + };
> + codec {
> + sound-dai = <&pef2256_codec0>;
> + dai-tdm-slot-num = <4>;
> + dai-tdm-slot-width = <8>;
> + /* TS 1, 2, 3, 4 */
> + dai-tdm-slot-tx-mask = <0 1 1 1 1>;
> + dai-tdm-slot-rx-mask = <0 1 1 1 1>;
> + };
> + };
> + simple-audio-card,dai-link@1 { /* CPU DAI2 - pef2256 codec 2 */
> + reg = <1>;
> + cpu {
> + sound-dai = <&cpu_dai2>;
> + };
> + codec {
> + sound-dai = <&pef2256_codec1>;
> + dai-tdm-slot-num = <4>;
> + dai-tdm-slot-width = <8>;
> + /* TS 5, 6, 7, 8 */
> + dai-tdm-slot-tx-mask = <0 0 0 0 0 1 1 1 1>;
> + dai-tdm-slot-rx-mask = <0 0 0 0 0 1 1 1 1>;
> + };
> + };
> + };
> --
> 2.41.0
>
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists