Re: [RFC PATCH 6/7] dt-bindings: sound: sun4i-i2s: Add channel-dins property

From: Rob Herring <robh@kernel.org>
To: John Watts <contact@jookia.org>
Cc: alsa-devel@alsa-project.org,
	"Liam Girdwood" <lgirdwood@gmail.com>,
	"Mark Brown" <broonie@kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Chen-Yu Tsai" <wens@csie.org>,
	"Jernej Skrabec" <jernej.skrabec@gmail.com>,
	"Samuel Holland" <samuel@sholland.org>,
	"Jaroslav Kysela" <perex@perex.cz>,
	"Takashi Iwai" <tiwai@suse.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Maxime Ripard" <mripard@kernel.org>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 6/7] dt-bindings: sound: sun4i-i2s: Add channel-dins property
Date: Mon, 21 Aug 2023 12:34:40 -0500	[thread overview]
Message-ID: <20230821173440.GA2002457-robh@kernel.org> (raw)
In-Reply-To: <20230811201406.4096210-7-contact@jookia.org>

On Sat, Aug 12, 2023 at 06:14:05AM +1000, John Watts wrote:
> The R329 variant of the sun4i I2S controller supports multiple
> data input pins (din pins) for receiving data. Each channel can have
> its data input pin configured.
> 
> Allow this to be configured using a new channel-dins property.
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>  .../sound/allwinner,sun4i-a10-i2s.yaml        | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
> index 739114fb6549..402549f9941c 100644
> --- a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
> +++ b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
> @@ -52,6 +52,13 @@ properties:
>        - const: apb
>        - const: mod
>  
> +  channel-dins:

Needs a vendor prefix.

> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> +    description:
> +      This is a list of DIN pin numbers, each used for a receiving I2S
> +      channel. Pins are mapped to channels based on array index.
> +      Channel 0 is the first number, then channel 1, and so on.

Constraints on the values? 255 is valid pin number?

> +
>    # Even though it only applies to subschemas under the conditionals,
>    # not listing them here will trigger a warning because of the
>    # additionalsProperties set to false.
> @@ -144,4 +151,19 @@ examples:
>          dma-names = "rx", "tx";
>      };
>  
> +  - |
> +    i2s0_d1: i2s@2032000 {

Drop unused labels.

However, I don't really think adding 1 property justifies a whole other 
example.

> +            #sound-dai-cells = <0>;
> +            compatible = "allwinner,sun20i-d1-i2s",
> +                         "allwinner,sun50i-r329-i2s";
> +            reg = <0x2032000 0x1000>;
> +            interrupts = <0 26 0>;
> +            clocks = <&ccu 86>, <&ccu 82>;
> +            clock-names = "apb", "mod";
> +            resets = <&ccu 34>;
> +            dmas = <&dma 3>, <&dma 3>;
> +            dma-names = "rx", "tx";
> +            channel-dins = /bits/ 8 <0 0 1 1 2 2>;
> +    };
> +
>  ...
> -- 
> 2.41.0
> 