From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from avon.wwwdotorg.org ([70.85.31.133]:33791 "EHLO avon.wwwdotorg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753742Ab3HVT4h (ORCPT ); Thu, 22 Aug 2013 15:56:37 -0400 Message-ID: <52166CF0.9080402@wwwdotorg.org> Date: Thu, 22 Aug 2013 13:56:32 -0600 From: Stephen Warren MIME-Version: 1.0 Subject: Re: [PATCH v10 2/2] ASoC: fsl: Add S/PDIF machine driver References: <52150763.8020707@wwwdotorg.org> <20130822114027.GA4258@MrMyself> In-Reply-To: <20130822114027.GA4258@MrMyself> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: devicetree-owner@vger.kernel.org To: Nicolin Chen Cc: broonie@kernel.org, s.hauer@pengutronix.de, linuxppc-dev@lists.ozlabs.org, alsa-devel@alsa-project.org, devicetree@vger.kernel.org, lars@metafoo.de, p.zabel@pengutronix.de, timur@tabi.org, rob.herring@calxeda.com, shawn.guo@linaro.org, festevam@gmail.com, tomasz.figa@gmail.com, mark.rutland@arm.com, R65777@freescale.com List-ID: On 08/22/2013 05:40 AM, Nicolin Chen wrote: > Hi Stephen, > > On Wed, Aug 21, 2013 at 12:30:59PM -0600, Stephen Warren wrote: >> I still don't think those two properties are correct. >> >> Exactly what node will those phandles point at? >> >> There definitely should not be a DT node for any "dummy CODEC", >> irrespective of whether this binding calls the other node a "CODEC" or a >> "dummy CODEC". >> >> If these properties are to contain phandles, it would be acceptable for >> the referenced node to be: >> >> * A node representing the physical connector/jack on the board. >> >> * A node representing some other IP block on the board, such as an HDMI >> encoder/display-controller >> >> I think those options are unlikely in general, so I think instead these >> properties should just be Boolean indicating that "something" is >> connector to the S/PDIF RX/TX, without specifying what that "something" >> is. It doesn't matter what at least in the connector/jack case, although >> perhaps it does in the HDMI encoder/display-controller? > > Documentation/devicetree/bindings/sound/spdif-receiver.txt > If I understand correctly, this doc for the dummy codec should be invalid? Yes, I'm not convinced that binding is a good idea; it describes something that often doesn't actually exist in HW. (Sometimes there's a real S/PDIF receiving device on board, but sometimes there's nothing except a jack/connector). It'd be useful if other DT binding maintainers could weigh in on this to confirm/deny my thoughts. > But this patch, the spdif machine driver, is based on this codec driver, > pls check the following code: > > 164 + codec_rx_np = of_parse_phandle(np, "spdif-receiver", 0); > 165 + if (codec_rx_np) { > 169 + data->dai[num_links].codec_of_node = codec_rx_np; > 173 + } > > Accordingly, the binding I planned to add in DT: > > 27 + spdif_rx_codec: spdif-receiver { > 28 + compatible = "linux,spdif-dir"; > 29 + }; > 30 + > 31 + sound-spdif { > 32 + compatible = "fsl,imx-audio-spdif", > 33 + "fsl,imx-sabreauto-spdif"; > 34 + model = "imx-spdif"; > 35 + spdif-controller = <&spdif>; > 37 + spdif-receiver = <&spdif_rx_codec>; > 38 + }; > > So if the DT can't allow me to include this codec node, how could I > handle it in the current baseline. I would expect the machine driver's probe routine to create the dummy S/PDIF receiver object itself, and register it by calling snd_soc_register_codec(). That way, only the machine driver code need know it (dummy CODEC) exists.