From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> To: Russell King - ARM Linux <linux@arm.linux.org.uk> Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>, Jean-Francois Moine <moinejf@free.fr>, Mark Rutland <mark.rutland@arm.com>, devicetree@vger.kernel.org, Jason Cooper <jason@lakedaemon.net>, Pawel Moll <pawel.moll@arm.com>, Stephen Warren <swarren@wwwdotorg.org>, linux-kernel@vger.kernel.org, Rob Herring <rob.herring@calxeda.com>, Gregory CLEMENT <gregory.clement@free-electrons.com>, linux-arm-kernel@lists.infradead.org, Ian Campbell <ian.campbell@citrix.com>, Lior Amsalem <alior@marvell.com>, Ezequiel Garcia <ezequiel.garcia@free-electrons.com> Subject: Re: [PATCH 1/2] ARM: Dove: Add the audio devices in DT Date: Wed, 28 Aug 2013 14:29:09 +0200 [thread overview] Message-ID: <20130828142909.3521064b@skate> (raw) In-Reply-To: <20130828121320.GK6617@n2100.arm.linux.org.uk> Dear Russell King - ARM Linux, On Wed, 28 Aug 2013 13:13:20 +0100, Russell King - ARM Linux wrote: > > > I guess, yes. > > > > Jean-François, could you cook and submit a patch to change the > > compatible string? > > I don't think this is a good idea. The configuration of this IP is > not based on the SoC as a single SoC can have a mixture of different > configurations. 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. > I think marvell,mvebu-audio is a reasonable compatible string for this, > and that the different configurations should be described by properties > indicating which inputs and outputs have been implemented. > > For instance, on the Dove, there are two of these blocks. One has I2S > in and out only, but the other block has I2S in and out, and SPDIF out. > On some other Marvell devices, this block has I2S in and out and SPDIF > in and out. > > Otherwise, they're functionally the same. Right, that's why they can both use "kirkwood-audio" as the compatible string. > > Though, if the difference between the two units is the availability of > > SPDIF support, then we shouldn't encode the channel number, but instead > > the availability of SPDIF, i.e: > > > > audio0 { > > reg = <... ...>; > > compatible = "marvell,kirkwood-audio"; > > marvell,has-spdif; > > }; > > > > audio1 { > > reg = <... ...>; > > compatible = "marvell,kirkwood-audio"; > > }; > > ... 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? 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"). > 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. For example, for XOR engines, we have: xor@60900 { compatible = "marvell,orion-xor"; reg = <0x60900 0x100 0x60b00 0x100>; clocks = <&gateclk 22>; status = "okay"; xor10 { interrupts = <51>; dmacap,memcpy; dmacap,xor; }; xor11 { interrupts = <52>; dmacap,memcpy; dmacap,xor; dmacap,memset; }; }; because the first channel of each XOR engine has only memcpy and xor capabilities, while the second channel has memcpy, xor and memset capabilities. Thanks, Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
WARNING: multiple messages have this Message-ID (diff)
From: thomas.petazzoni@free-electrons.com (Thomas Petazzoni) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 1/2] ARM: Dove: Add the audio devices in DT Date: Wed, 28 Aug 2013 14:29:09 +0200 [thread overview] Message-ID: <20130828142909.3521064b@skate> (raw) In-Reply-To: <20130828121320.GK6617@n2100.arm.linux.org.uk> Dear Russell King - ARM Linux, On Wed, 28 Aug 2013 13:13:20 +0100, Russell King - ARM Linux wrote: > > > I guess, yes. > > > > Jean-Fran?ois, could you cook and submit a patch to change the > > compatible string? > > I don't think this is a good idea. The configuration of this IP is > not based on the SoC as a single SoC can have a mixture of different > configurations. 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. > I think marvell,mvebu-audio is a reasonable compatible string for this, > and that the different configurations should be described by properties > indicating which inputs and outputs have been implemented. > > For instance, on the Dove, there are two of these blocks. One has I2S > in and out only, but the other block has I2S in and out, and SPDIF out. > On some other Marvell devices, this block has I2S in and out and SPDIF > in and out. > > Otherwise, they're functionally the same. Right, that's why they can both use "kirkwood-audio" as the compatible string. > > Though, if the difference between the two units is the availability of > > SPDIF support, then we shouldn't encode the channel number, but instead > > the availability of SPDIF, i.e: > > > > audio0 { > > reg = <... ...>; > > compatible = "marvell,kirkwood-audio"; > > marvell,has-spdif; > > }; > > > > audio1 { > > reg = <... ...>; > > compatible = "marvell,kirkwood-audio"; > > }; > > ... 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? 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"). > 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. For example, for XOR engines, we have: xor at 60900 { compatible = "marvell,orion-xor"; reg = <0x60900 0x100 0x60b00 0x100>; clocks = <&gateclk 22>; status = "okay"; xor10 { interrupts = <51>; dmacap,memcpy; dmacap,xor; }; xor11 { interrupts = <52>; dmacap,memcpy; dmacap,xor; dmacap,memset; }; }; because the first channel of each XOR engine has only memcpy and xor capabilities, while the second channel has memcpy, xor and memset capabilities. Thanks, Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
next prev parent reply other threads:[~2013-08-28 12:29 UTC|newest] Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-08-28 9:34 [PATCH 1/2] ARM: Dove: Add the audio devices in DT Jean-Francois Moine 2013-08-28 9:34 ` Jean-Francois Moine 2013-08-28 10:13 ` Sebastian Hesselbarth 2013-08-28 10:13 ` Sebastian Hesselbarth 2013-08-28 10:13 ` Sebastian Hesselbarth 2013-08-28 10:19 ` Thomas Petazzoni 2013-08-28 10:19 ` Thomas Petazzoni 2013-08-28 10:19 ` Thomas Petazzoni 2013-08-28 10:26 ` Sebastian Hesselbarth 2013-08-28 10:26 ` Sebastian Hesselbarth 2013-08-28 10:26 ` Sebastian Hesselbarth 2013-08-28 11:15 ` Thomas Petazzoni 2013-08-28 11:15 ` Thomas Petazzoni 2013-08-28 11:44 ` Sebastian Hesselbarth 2013-08-28 11:44 ` Sebastian Hesselbarth 2013-08-28 11:58 ` Thomas Petazzoni 2013-08-28 11:58 ` Thomas Petazzoni 2013-08-28 12:13 ` Russell King - ARM Linux 2013-08-28 12:13 ` Russell King - ARM Linux 2013-08-28 12:29 ` Thomas Petazzoni [this message] 2013-08-28 12:29 ` Thomas Petazzoni 2013-08-28 12:42 ` Russell King - ARM Linux 2013-08-28 12:42 ` Russell King - ARM Linux 2013-08-28 12:51 ` Thomas Petazzoni 2013-08-28 12:51 ` Thomas Petazzoni 2013-08-28 13:58 ` Russell King - ARM Linux 2013-08-28 13:58 ` Russell King - ARM Linux 2013-08-28 12:16 ` Sebastian Hesselbarth 2013-08-28 12:16 ` Sebastian Hesselbarth 2013-08-29 10:07 ` Jean-Francois Moine 2013-08-29 10:07 ` Jean-Francois Moine 2013-08-29 10:13 ` Russell King - ARM Linux 2013-08-29 10:13 ` Russell King - ARM Linux 2013-08-29 11:01 ` Sebastian Hesselbarth 2013-08-29 11:01 ` Sebastian Hesselbarth 2013-08-30 15:08 ` Russell King - ARM Linux 2013-08-30 15:08 ` Russell King - ARM Linux 2013-08-29 9:46 ` Jean-Francois Moine 2013-08-29 9:46 ` Jean-Francois Moine 2013-08-29 16:12 ` Mark Brown 2013-08-29 16:12 ` Mark Brown 2013-08-29 16:33 ` Russell King - ARM Linux 2013-08-29 16:33 ` Russell King - ARM Linux 2013-08-29 17:12 ` Mark Brown 2013-08-29 17:12 ` Mark Brown 2013-08-29 18:02 ` Sebastian Hesselbarth 2013-08-29 18:02 ` Sebastian Hesselbarth 2013-08-29 18:20 ` Mark Brown 2013-08-29 18:20 ` Mark Brown 2013-08-29 18:34 ` Russell King - ARM Linux 2013-08-29 18:34 ` Russell King - ARM Linux 2013-08-28 19:49 ` Sergei Shtylyov 2013-08-28 19:49 ` Sergei Shtylyov 2013-08-29 9:38 ` Jean-Francois Moine 2013-08-29 9:38 ` Jean-Francois Moine 2013-08-29 14:13 ` Sergei Shtylyov 2013-08-29 14:13 ` Sergei Shtylyov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20130828142909.3521064b@skate \ --to=thomas.petazzoni@free-electrons.com \ --cc=alior@marvell.com \ --cc=devicetree@vger.kernel.org \ --cc=ezequiel.garcia@free-electrons.com \ --cc=gregory.clement@free-electrons.com \ --cc=ian.campbell@citrix.com \ --cc=jason@lakedaemon.net \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@arm.linux.org.uk \ --cc=mark.rutland@arm.com \ --cc=moinejf@free.fr \ --cc=pawel.moll@arm.com \ --cc=rob.herring@calxeda.com \ --cc=sebastian.hesselbarth@gmail.com \ --cc=swarren@wwwdotorg.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.