[PATCH 1/2] ARM: Dove: Add the audio devices in DT
From: Thomas Petazzoni <hidden>
Date: 2013-08-28 12:52:04
Also in:
linux-devicetree, lkml
Dear Russell King - ARM Linux, On Wed, 28 Aug 2013 13:42:55 +0100, Russell King - ARM Linux wrote:
quoted
Using the name of the oldest SoC in the family that had the IP block is the norm, because it's really what "compatible" means: the IP block in Dove is *compatible* with the one that was originally introduced in Kirkwood. See what Rob Herring (one of the DT maintainer) says in http://lists.infradead.org/pipermail/linux-mtd/2012-March/040417.html: """ There is no reason all machines can't use "st,spear600-smi" in their dts. It doesn't have to be a spear600, just compatible with it. Really you want the string to be the oldest SOC the block is in and then newer SOCs can claim compatibility with the old version. """ The thread was precisely about replacing a SoC-specific compatible string "st,spear600-smi" by a more generic "st,spear-smi" and Rob Herring (above) was opposing to that.We're not talking about replacing a pre-existing string, we're talking about adding one, which is a different situation.
I don't see how this makes this a different situation. See for example http://lists.infradead.org/pipermail/linux-arm-kernel/2013-April/161065.html and http://lists.infradead.org/pipermail/linux-arm-kernel/2012-March/087702.html where Arnd also said using the oldest SoC that has the same IP block as the compatible string was the right thing to do.
quoted
quoted
... which means there's no problem with using marvell,mvebu-audio as the compatible string if you're going to use properties to describe what facilities are available.I disagree, because how do you know if a future "mvebu" SOC such as Armada 370, or one that doesn't exist yet, will not have a different audio IP block?The Dove already contains _three_ audio blocks, two of which are this one, and another which is block for driving an AC'97 codec (which doesn't have a driver.) That's no problem because you won't call that one an "audio" block but an AC'97 block. So...
And? If that's a different IP block, it'll have a different compatible string, that's it. That doesn't change my point: using "marvell,mvebu-audio" as the compatible string is stupid, because you have absolutely no idea what the future of audio in mvebu SOCs will be. However, you do know, *today* that Kirkwood and Dove have compatible IP blocks for audio, and that they were first introduced with Kirkwood.
quoted
It will still be audio, it will still be mvebu, but it will not be able to use a "marvell,mvebu-audio" driver. Or maybe it can use the same driver, but with a few variations, so a different compatible string will be needed to identify the original IP ("marvell,kirkwood-audio", used on Kirkwood/Dove) and slightly newer versions of the IP ("marvell,some-funky-soc-audio").I don't think this really applies.
It does. We're exactly in this situation, as I will soon be working on Armada 370 audio support, and while the IP looks similar, I have checked all the details to see if it's exactly identical. And Armada 370 is really a mvebu architecture: it's even supported in mach-mvebu/, while Kirkwood and Dove are not (yet).
quoted
quoted
In any case "marvell,has-spdif" is too generic - as I've indicated above, there's versions with spdif out, and other versions with spdif in and out.Right, the above was just an example to illustrate that we can have additional properties to encode the differences between each instance of the audio devices.I think this is a mistake too: these properties will just tell us what may be possible, and the driver will take no real action on them. I suppose that a property specifying whether there is a SPDIF output could be used to control whether the IEC958 channel status controls are registered. However... What's more important is which outputs are actually wired up, and therefore which bits of this hardware are enabled. Even then, we wouldn't want to expose (eg) the IEC958 channel status controls if the SPDIF output isn't wired. So all in all, I don't see any point to a set of properties saying "we have SPDIF" etc. That information should come solely from whether the SPDIF output has been "wired up". Let me put that another way: we _can_ provide those properties to indicate what facilities the hardware has, we just wouldn't use them at all - and to provide them seems like over-design to me.
I am not arguing about the properties, as I haven't looked at the specific problem that needs to be solved. By suggesting properties, I was merely suggesting one possible solution to the problem that Sebastian was raising, where the different instances of the IP block don't have the same capabilities. What I am however strongly arguing on is the choice of the compatible string. marvell,mvebu-audio is a wrong choice. Best regards, Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com