diff mbox series

[2/3] arm64: dts: imx8mm: Add node for SPDIF

Message ID 20201008212706.870115-2-aford173@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/3] arm64: dts: imx8mm: Add support for micfil | expand

Commit Message

Adam Ford Oct. 8, 2020, 9:27 p.m. UTC
The i.MX8M Mini can support SPIDF which is very similar to the
IP used on the i.MX35.

This patch adds the SPIF node.

Signed-off-by: Adam Ford <aford173@gmail.com>

Comments

Krzysztof Kozlowski Oct. 12, 2020, 5 p.m. UTC | #1
On Thu, Oct 08, 2020 at 04:27:05PM -0500, Adam Ford wrote:
> The i.MX8M Mini can support SPIDF which is very similar to the

SPIDF -> SPDIF

> IP used on the i.MX35.
> 
> This patch adds the SPIF node.

Just "Add the SPDIF node", avoid "this patch". And SPIF->SPDIF.

> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> index fad1f9cdb241..44a59f59f65a 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -358,6 +358,30 @@ micfil: audio-controller@30080000 {
>  				status = "disabled";
>  			};
>  
> +			spdif1: spdif@30090000 {
> +				compatible = "fsl,imx8mm-spdif", "fsl,imx35-spdif";

This does not pass the dtschema validation, so only "fsl,imx35-spdif".

Best regards,
Krzysztof
Fabio Estevam Oct. 19, 2020, 4:55 p.m. UTC | #2
Hi Krzysztof,

On Mon, Oct 12, 2020 at 2:00 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:

> > +                     spdif1: spdif@30090000 {
> > +                             compatible = "fsl,imx8mm-spdif", "fsl,imx35-spdif";
>
> This does not pass the dtschema validation, so only "fsl,imx35-spdif".

Shouldn't the dt schema validation be changed then to accept this format?
Krzysztof Kozlowski Oct. 19, 2020, 5:06 p.m. UTC | #3
On Mon, 19 Oct 2020 at 18:56, Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Krzysztof,
>
> On Mon, Oct 12, 2020 at 2:00 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> > > +                     spdif1: spdif@30090000 {
> > > +                             compatible = "fsl,imx8mm-spdif", "fsl,imx35-spdif";
> >
> > This does not pass the dtschema validation, so only "fsl,imx35-spdif".
>
> Shouldn't the dt schema validation be changed then to accept this format?

Depends. If a given compatible is in the schema, then the answer is:
rather not. The DTS conform to schema, not the otherwise. When
dtschema is sent this is the ABI to which the kernel should conform.
User-space or other SW (e.g. U-Boot) could depend on it. Of course
there are exceptions...

If the compatible is not in the schema, then of course it could be
added if there is a need. In this case, device is simply compatible
with "fsl,imx35-spdif" so there is no real need to change the schema.
Just use the existing compatible. Look at one funny example in iMX:
drivers/pwm/pwm-imx27.c. Someone created multiple compatibles but does
not use them at all, because the hardware looks exactly the same.
Therefore driver implements only one compatible because entire HW is
compatible with fsl,imx27-pwm. In such clear case, why adding many
compatibles? It's just a churn.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index fad1f9cdb241..44a59f59f65a 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -358,6 +358,30 @@  micfil: audio-controller@30080000 {
 				status = "disabled";
 			};
 
+			spdif1: spdif@30090000 {
+				compatible = "fsl,imx8mm-spdif", "fsl,imx35-spdif";
+				reg = <0x30090000 0x10000>;
+				interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk IMX8MM_CLK_AUDIO_AHB>, /* core */
+					 <&clk IMX8MM_CLK_24M>, /* rxtx0 */
+					 <&clk IMX8MM_CLK_SPDIF1>, /* rxtx1 */
+					 <&clk IMX8MM_CLK_DUMMY>, /* rxtx2 */
+					 <&clk IMX8MM_CLK_DUMMY>, /* rxtx3 */
+					 <&clk IMX8MM_CLK_DUMMY>, /* rxtx4 */
+					 <&clk IMX8MM_CLK_AUDIO_AHB>, /* rxtx5 */
+					 <&clk IMX8MM_CLK_DUMMY>, /* rxtx6 */
+					 <&clk IMX8MM_CLK_DUMMY>, /* rxtx7 */
+					 <&clk IMX8MM_CLK_DUMMY>; /* spba */
+				clock-names = "core", "rxtx0",
+					      "rxtx1", "rxtx2",
+					      "rxtx3", "rxtx4",
+					      "rxtx5", "rxtx6",
+					      "rxtx7", "spba";
+				dmas = <&sdma2 28 18 0>, <&sdma2 29 18 0>;
+				dma-names = "rx", "tx";
+				status = "disabled";
+			};
+
 			gpio1: gpio@30200000 {
 				compatible = "fsl,imx8mm-gpio", "fsl,imx35-gpio";
 				reg = <0x30200000 0x10000>;