From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from co1outboundpool.messaging.microsoft.com (co1ehsobe003.messaging.microsoft.com [216.32.180.186]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (Client CN "mail.global.frontbridge.com", Issuer "MSIT Machine Auth CA 2" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 311A72C0285 for ; Mon, 19 Aug 2013 20:13:26 +1000 (EST) Date: Mon, 19 Aug 2013 18:13:04 +0800 From: Nicolin Chen To: Mark Rutland Subject: Re: [PATCH v7 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver Message-ID: <20130819101303.GB11402@MrMyself> References: <43416f3617951161b6e779277b4719438f844e49.1376901081.git.b42378@freescale.com> <20130819091809.GD3719@e106331-lin.cambridge.arm.com> <20130819093423.GB10950@MrMyself> <20130819095433.GG3719@e106331-lin.cambridge.arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" In-Reply-To: <20130819095433.GG3719@e106331-lin.cambridge.arm.com> Cc: "devicetree@vger.kernel.org" , "alsa-devel@alsa-project.org" , "lars@metafoo.de" , "swarren@wwwdotorg.org" , "festevam@gmail.com" , "s.hauer@pengutronix.de" , "timur@tabi.org" , "rob.herring@calxeda.com" , "tomasz.figa@gmail.com" , "broonie@kernel.org" , "p.zabel@pengutronix.de" , "R65777@freescale.com" , "shawn.guo@linaro.org" , "linuxppc-dev@lists.ozlabs.org" List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Mon, Aug 19, 2013 at 10:54:33AM +0100, Mark Rutland wrote: > > I see, so 'Compatible list, must contains "fsl,imx35-spdif"' would be okay? > > While that needs to be mentioned, other values which might be present > (e.g. "fsl,imx6q-spdif") must be mentioned, or we can't rely on them > if we want to use them in future from drivers to provide additional > information (and thus they're useless). Well, imx6q-spdif is identical to the imx35-spdif. So I think the 'must contains 35' would be enough for the current case. If any future modification happens to the list, I can update this doc later. > > > > > > + - interrupts : Contains spdif interrupt. > > > > > > Is that the only interrupt the device generates? > > > > Yes, how could I improve this description? > > It's probably not possible to make it much clearar to be honest, > "Contains the sole interrupt generated by the device" might be a little > overkill. Then I keep it no-change :) > > > > + "rxtx<0-7>" Clock source list for tx and rx clock. > > > > + This clock list should be identical to > > > > + the source list connecting to the spdif > > > > + clock mux in "SPDIF Transceiver Clock > > > > + Diagram" of SoC reference manual. It > > > > + can also be referred to TxClk_Source > > > > + bit of register SPDIF_STC. > > The list is also identical to the TxClk_Source bit value list of > > register SPDIF_STC. > > What I don't understand is how the value of the SPDIF_STC register > within the spdif IP block can describe the necessary details of clocks > coming from an external clock provider. > > What do the TxClk_Source bits represent? The configuration of the clock > inputs on the spdif IP block, or the outputs on some clock provider? Are > they writeable, or configured at integration time? If the clock provider > were replaced with another arbitrary clock provider, what would they > represent? Actually there's a clock mux for TxClk and it's connecting with 8 clock sources. So the TxClk_Source bit show the connection between its value with the correspond source on the clock mux: TxClk_Source 000 XTAL clk input 001 CCM spdif0_clk_root input 010 asrc_clk input 011 spdif_extclk input, from pads 100 esai_hckt input 101 frequency divided ipg_clk input 110 mlb_clk input 111 mlb phy clk input > > I guess it's better to drop the 'imx6q-spdif' here? > > That depends: > > * If the two IP blocks are identical, only the "imx35-spdif" name is > necessary, and we can forget about "fsl,imx6q-spdif". > > * If "fsl,imx6q-spdif" is a strict superset of "fsl,imx35-spdif", having > both names documented and in a compatible list for a "fsl,imx6q-spdif" > device makes sense. > > * If "fsl,imx6q-spdif" is a variation of "fsl,imx35-spdif", and the > "fsl,imx6q-spdif" cannot always be treated identically to a > "fsl,imx35-spdif", then it makes sense to have separate compatible > strings, with a device being listed as either "fsl,imx6q-spdif" or > "fsl,imx35-spdif". > > I don't know enough about the hardware to make that judgement call. Thank you for explaining! I will choose A, because they are internally identical except their external clock sources. Best regards, Nicolin