The kirkwood i2s driver is used without DT in the Kirkwood machine.
This patch adds a DT compatible definition for use in other Marvell
machines as the Armada 88AP510 (Dove).
Signed-off-by: Jean-Francois Moine <[email protected]>
---
v3
- change compatible from kirkwood to mvebu (Andrew Lunn)
v2
- this patch replaces the previous
[PATCH 2/2] ARM: kirkwood: extend the kirkwood i2s driver for DT usage
- 2 possible clocks (Sebastian Hesselbarth)
- shorter io mapping (Andrew Lunn)
- less #ifdef's
---
Documentation/devicetree/bindings/sound/kirkwood-i2s.txt | 24 +++++++++++
sound/soc/kirkwood/kirkwood-i2s.c | 58 ++++++++++++++++++++------
2 files changed, 70 insertions(+), 12 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt b/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt
new file mode 100644
index 0000000..8f1e534
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt
@@ -0,0 +1,24 @@
+* mvebu (Kirkwood, Dove, Armada 370) I2S controller
+
+Required properties:
+
+- compatible: "marvell,mvebu-i2s"
+
+- reg: physical base address of the controller and length of memory mapped
+ region.
+
+- interrupts: list of two irq numbers.
+ The first irq is used for data flow and the second one is used for errors.
+
+- clocks: one or two phandles.
+ The first one is mandatory and defines the internal clock.
+ The second one is optional and defines an external clock.
+
+Example:
+
+i2s1: audio-controller@b4000 {
+ compatible = "marvell,mvebu-i2s";
+ reg = <0xb4000 0x2210>;
+ interrupts = <21>, <22>;
+ clocks = <&gate_clk 13>;
+};
diff --git a/sound/soc/kirkwood/kirkwood-i2s.c b/sound/soc/kirkwood/kirkwood-i2s.c
index 4c9dad3..019e340 100644
--- a/sound/soc/kirkwood/kirkwood-i2s.c
+++ b/sound/soc/kirkwood/kirkwood-i2s.c
@@ -12,7 +12,6 @@
#include <linux/init.h>
#include <linux/module.h>
-#include <linux/platform_device.h>
#include <linux/io.h>
#include <linux/slab.h>
#include <linux/mbus.h>
@@ -22,6 +21,8 @@
#include <sound/pcm_params.h>
#include <sound/soc.h>
#include <linux/platform_data/asoc-kirkwood.h>
+#include <linux/of.h>
+
#include "kirkwood.h"
#define DRV_NAME "kirkwood-i2s"
@@ -461,6 +462,7 @@ static int kirkwood_i2s_dev_probe(struct platform_device *pdev)
struct snd_soc_dai_driver *soc_dai = &kirkwood_i2s_dai;
struct kirkwood_dma_data *priv;
struct resource *mem;
+ struct device_node *np;
int err;
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
@@ -481,24 +483,45 @@ static int kirkwood_i2s_dev_probe(struct platform_device *pdev)
return -ENXIO;
}
- if (!data) {
- dev_err(&pdev->dev, "no platform data ?!\n");
- return -EINVAL;
+ /* get the DT or static parameters */
+ np = pdev->dev.of_node;
+ if (np) {
+ struct of_phandle_args clkspec;
+
+ priv->burst = 128; /* might be 32 or 128 */
+ priv->clk = of_clk_get(np, 0); /* internal clock */
+ err = of_parse_phandle_with_args(np,
+ "clocks", "#clock-cells", 1,
+ &clkspec);
+ if (err) {
+ priv->extclk = ERR_PTR(-EINVAL); /* no external clock */
+ } else {
+ priv->extclk = of_clk_get(np, 1);
+ if (IS_ERR(priv->extclk)) {
+ err = -EPROBE_DEFER;
+ goto fail;
+ }
+ }
+ } else {
+ if (!data) {
+ dev_err(&pdev->dev, "no platform data ?!\n");
+ return -EINVAL;
+ }
+ priv->burst = data->burst;
+ priv->clk = clk_get(&pdev->dev, NULL);
+ priv->extclk = clk_get(&pdev->dev, "extclk");
}
- priv->burst = data->burst;
-
- priv->clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(priv->clk)) {
dev_err(&pdev->dev, "no clock\n");
- return PTR_ERR(priv->clk);
+ err = PTR_ERR(priv->clk);
+ goto fail;
}
err = clk_prepare_enable(priv->clk);
if (err < 0)
- return err;
+ goto fail;
- priv->extclk = clk_get(&pdev->dev, "extclk");
if (!IS_ERR(priv->extclk)) {
if (priv->extclk == priv->clk) {
clk_put(priv->extclk);
@@ -515,7 +538,7 @@ static int kirkwood_i2s_dev_probe(struct platform_device *pdev)
priv->ctl_rec = KIRKWOOD_RECCTL_SIZE_24;
/* Select the burst size */
- if (data->burst == 32) {
+ if (priv->burst == 32) {
priv->ctl_play |= KIRKWOOD_PLAYCTL_BURST_32;
priv->ctl_rec |= KIRKWOOD_RECCTL_BURST_32;
} else {
@@ -528,12 +551,13 @@ static int kirkwood_i2s_dev_probe(struct platform_device *pdev)
if (!err)
return 0;
dev_err(&pdev->dev, "snd_soc_register_component failed\n");
-
+fail:
if (!IS_ERR(priv->extclk)) {
clk_disable_unprepare(priv->extclk);
clk_put(priv->extclk);
}
clk_disable_unprepare(priv->clk);
+ clk_put(priv->clk);
return err;
}
@@ -549,16 +573,26 @@ static int kirkwood_i2s_dev_remove(struct platform_device *pdev)
clk_put(priv->extclk);
}
clk_disable_unprepare(priv->clk);
+ clk_put(priv->clk);
return 0;
}
+#ifdef CONFIG_OF
+static struct of_device_id kirkwood_i2s_of_match[] = {
+ { .compatible = "marvell,mvebu-i2s" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, kirkwood_i2s_of_match);
+#endif
+
static struct platform_driver kirkwood_i2s_driver = {
.probe = kirkwood_i2s_dev_probe,
.remove = kirkwood_i2s_dev_remove,
.driver = {
.name = DRV_NAME,
.owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(kirkwood_i2s_of_match),
},
};
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
On Tue, Jul 23, 2013 at 10:46:15AM +0200, Jean-Francois Moine wrote:
> The kirkwood i2s driver is used without DT in the Kirkwood machine.
> This patch adds a DT compatible definition for use in other Marvell
> machines as the Armada 88AP510 (Dove).
Yet again, this illustrates why converting to DT causes backwards
steps in drivers: the conversion of devm_clk_get() to of_clk_get()
necessitates a clk_put().
Can someone please provide a devm_of_clk_get() function before we
accept any more patches doing this kind of thing?
The other issue is that kirkwood-dma.c and kirkwood-i2s.c should be
one driver; they're not separate hardware on these platforms, so that
needs fixing before coming up with any kind of DT model for this stuff.
On 07/23/2013 10:53 AM, Russell King - ARM Linux wrote:
> On Tue, Jul 23, 2013 at 10:46:15AM +0200, Jean-Francois Moine wrote:
>> The kirkwood i2s driver is used without DT in the Kirkwood machine.
>> This patch adds a DT compatible definition for use in other Marvell
>> machines as the Armada 88AP510 (Dove).
>
> Yet again, this illustrates why converting to DT causes backwards
> steps in drivers: the conversion of devm_clk_get() to of_clk_get()
> necessitates a clk_put().
That's just a very poor usage of the API in this patch. devm_clk_get() works
fine on DT and non-DT platforms. The driver shouldn't need any changes in
that regard.
- Lars
On Tue, Jul 23, 2013 at 09:53:46AM +0100, Russell King - ARM Linux wrote:
> On Tue, Jul 23, 2013 at 10:46:15AM +0200, Jean-Francois Moine wrote:
> > The kirkwood i2s driver is used without DT in the Kirkwood machine.
> > This patch adds a DT compatible definition for use in other Marvell
> > machines as the Armada 88AP510 (Dove).
>
> Yet again, this illustrates why converting to DT causes backwards
> steps in drivers: the conversion of devm_clk_get() to of_clk_get()
> necessitates a clk_put().
>
> Can someone please provide a devm_of_clk_get() function before we
> accept any more patches doing this kind of thing?
The question is why isn't regular [devm_]clk_get used here? I don't know
what the Marvell stuff is doing here, but if the clk stuff is
implemented correctly then
np = pdev->dev.of_node;
priv->clk = of_clk_get(np, 0);
and
devm_clk_get(&pdev->dev, "label");
should both work
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Tue, Jul 23, 2013 at 11:08:20AM +0200, Sascha Hauer wrote:
> On Tue, Jul 23, 2013 at 09:53:46AM +0100, Russell King - ARM Linux wrote:
> > On Tue, Jul 23, 2013 at 10:46:15AM +0200, Jean-Francois Moine wrote:
> > > The kirkwood i2s driver is used without DT in the Kirkwood machine.
> > > This patch adds a DT compatible definition for use in other Marvell
> > > machines as the Armada 88AP510 (Dove).
> >
> > Yet again, this illustrates why converting to DT causes backwards
> > steps in drivers: the conversion of devm_clk_get() to of_clk_get()
> > necessitates a clk_put().
> >
> > Can someone please provide a devm_of_clk_get() function before we
> > accept any more patches doing this kind of thing?
>
> The question is why isn't regular [devm_]clk_get used here? I don't know
> what the Marvell stuff is doing here, but if the clk stuff is
> implemented correctly then
>
> np = pdev->dev.of_node;
> priv->clk = of_clk_get(np, 0);
>
> and
>
> devm_clk_get(&pdev->dev, "label");
>
> should both work
You tell me - I keep seeing patches on this list converting drivers from
using devm_clk_get() to using of_clk_get(). Maybe of_clk_get() should be
marked deprecated to stop people using it?
On Tue, Jul 23, 2013 at 10:39:50AM +0100, Russell King - ARM Linux wrote:
> On Tue, Jul 23, 2013 at 11:08:20AM +0200, Sascha Hauer wrote:
> > On Tue, Jul 23, 2013 at 09:53:46AM +0100, Russell King - ARM Linux wrote:
> > > On Tue, Jul 23, 2013 at 10:46:15AM +0200, Jean-Francois Moine wrote:
> > > > The kirkwood i2s driver is used without DT in the Kirkwood machine.
> > > > This patch adds a DT compatible definition for use in other Marvell
> > > > machines as the Armada 88AP510 (Dove).
> > >
> > > Yet again, this illustrates why converting to DT causes backwards
> > > steps in drivers: the conversion of devm_clk_get() to of_clk_get()
> > > necessitates a clk_put().
> > >
> > > Can someone please provide a devm_of_clk_get() function before we
> > > accept any more patches doing this kind of thing?
> >
> > The question is why isn't regular [devm_]clk_get used here? I don't know
> > what the Marvell stuff is doing here, but if the clk stuff is
> > implemented correctly then
> >
> > np = pdev->dev.of_node;
> > priv->clk = of_clk_get(np, 0);
> >
> > and
> >
> > devm_clk_get(&pdev->dev, "label");
> >
> > should both work
>
> You tell me - I keep seeing patches on this list converting drivers from
> using devm_clk_get() to using of_clk_get(). Maybe of_clk_get() should be
> marked deprecated to stop people using it?
I don't think that's possible. There are some valid usecases for
of_clk_get() when there's no struct device * available, like in many
clocksource drivers.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Tue, Jul 23, 2013 at 10:46:15AM +0200, Jean-Francois Moine wrote:
> + np = pdev->dev.of_node;
> + if (np) {
> + struct of_phandle_args clkspec;
> +
> + priv->burst = 128; /* might be 32 or 128 */
The comment says this needs to be variable (depending on what?) but it's
hard coded.
> + priv->clk = of_clk_get(np, 0); /* internal clock */
> + err = of_parse_phandle_with_args(np,
> + "clocks", "#clock-cells", 1,
> + &clkspec);
As others have pointed out if you need to change the clock get code
there's something wrong here, DT should be handled transparently by the
clock API.
On 07/23/13 14:34, Mark Brown wrote:
> On Tue, Jul 23, 2013 at 10:46:15AM +0200, Jean-Francois Moine wrote:
>
>> + np = pdev->dev.of_node;
>> + if (np) {
>> + struct of_phandle_args clkspec;
>> +
>> + priv->burst = 128; /* might be 32 or 128 */
>
> The comment says this needs to be variable (depending on what?) but it's
> hard coded.
>
>> + priv->clk = of_clk_get(np, 0); /* internal clock */
>> + err = of_parse_phandle_with_args(np,
>> + "clocks", "#clock-cells", 1,
>> + &clkspec);
>
> As others have pointed out if you need to change the clock get code
> there's something wrong here, DT should be handled transparently by the
> clock API.
IMHO the reason why of_clk_get() was/is mis-used in that way is mostly
compatibility with legacy platform_data based setup.
Kirkwood-i2s never knew about anything else than internal clock, then
Dove allows additional external clock input, aso. All changes are
incremental and more or less sane. But now is a good opportunity to
clean up this.
As Sascha Hauer pointed out, clocks should be distinguished by names
(clock-names property) instead of position and then use
devm_clk_get(&pdev->dev, "internal") and
devm_clk_get(&pdev->dev, "external") respectively.
This will possibly also require to update platform_data and legacy
users of kirkwood-i2s or have different setup functions for non-DT
and DT.
Also, while ASoC API separates the audio-controller into cpu-side
and codec-side parts, the DT should not. IIRC and as Russell repeated
again, we mentioned to merge kirkwood-i2s.c and kirkwood-dma.c into
a single file, didn't we?
I know we didn't care that much in the past, but one last thing that I
catched while reading another thead about compatible strings:
We should really be more careful about those. The correct usage
of compatible strings should be "marvell,mvebu-i2s" as common fallback,
but also "marvell,dove-i2s" and "marvell,kirkwood-i2s" for the SoC dtsi
files. We do not need to have all possible compatible strings in the
_driver's_ of_device_id table but the dtsi should contain them.
Finally, I2S DT node will end up as:
i2s1: audio-controller@b4000 {
compatible = "marvell,dove-i2s", "marvell,mvebu-i2s";
reg = <0xb4000 0x2210>;
interrupts = <21>, <22>;
clocks = <&gate_clk 13>, <&si5351a 1>;
clock-names = "internal", "external";
};
Jean-Francois, can you re-spin your patches with the comments made by
others and the above summary? I really like to see i2s for DT soon,
although we will not be able to support multiple codecs per DAI, yet.
Sebastian
On Tue, Jul 23, 2013 at 02:59:06PM +0200, Sebastian Hesselbarth wrote:
> On 07/23/13 14:34, Mark Brown wrote:
> >As others have pointed out if you need to change the clock get code
> >there's something wrong here, DT should be handled transparently by the
> >clock API.
> IMHO the reason why of_clk_get() was/is mis-used in that way is mostly
> compatibility with legacy platform_data based setup.
I'm sorry, but this doesn't make a great deal of sense to me. Can you
be more specific?
> As Sascha Hauer pointed out, clocks should be distinguished by names
> (clock-names property) instead of position and then use
> devm_clk_get(&pdev->dev, "internal") and
> devm_clk_get(&pdev->dev, "external") respectively.
> This will possibly also require to update platform_data and legacy
> users of kirkwood-i2s or have different setup functions for non-DT
> and DT.
Why would this be required? The driver is already asking for multiple
clocks...
> Also, while ASoC API separates the audio-controller into cpu-side
> and codec-side parts, the DT should not. IIRC and as Russell repeated
You mean DAI and DMA here? I already commented on that in my review of
the DMA binding.
> again, we mentioned to merge kirkwood-i2s.c and kirkwood-dma.c into
> a single file, didn't we?
That's been discussed several times but nobody's actually done it.
On 07/23/13 15:20, Mark Brown wrote:
> On Tue, Jul 23, 2013 at 02:59:06PM +0200, Sebastian Hesselbarth wrote:
>> On 07/23/13 14:34, Mark Brown wrote:
>
>>> As others have pointed out if you need to change the clock get code
>>> there's something wrong here, DT should be handled transparently by the
>>> clock API.
>
>> IMHO the reason why of_clk_get() was/is mis-used in that way is mostly
>> compatibility with legacy platform_data based setup.
>
> I'm sorry, but this doesn't make a great deal of sense to me. Can you
> be more specific?
>
>> As Sascha Hauer pointed out, clocks should be distinguished by names
>> (clock-names property) instead of position and then use
>> devm_clk_get(&pdev->dev, "internal") and
>> devm_clk_get(&pdev->dev, "external") respectively.
>
>> This will possibly also require to update platform_data and legacy
>> users of kirkwood-i2s or have different setup functions for non-DT
>> and DT.
>
> Why would this be required? The driver is already asking for multiple
> clocks...
The driver is asking for multiple *DT based* clocks. Legacy
platform_data has never been updated to reflect that. Mainly because
multiple clocks are only supported on Dove, which has no active non-DT
board in mainline.
>> Also, while ASoC API separates the audio-controller into cpu-side
>> and codec-side parts, the DT should not. IIRC and as Russell repeated
>
> You mean DAI and DMA here? I already commented on that in my review of
> the DMA binding.
Yes.
>> again, we mentioned to merge kirkwood-i2s.c and kirkwood-dma.c into
>> a single file, didn't we?
>
> That's been discussed several times but nobody's actually done it.
Correct, that is why I repeated that request to Jean-Francois.
Sebastian
On Tue, Jul 23, 2013 at 03:30:57PM +0200, Sebastian Hesselbarth wrote:
> On 07/23/13 15:20, Mark Brown wrote:
>>> again, we mentioned to merge kirkwood-i2s.c and kirkwood-dma.c into
>>> a single file, didn't we?
>>
>> That's been discussed several times but nobody's actually done it.
>
> Correct, that is why I repeated that request to Jean-Francois.
On that, I have some initial patches which start doing that, but I've
not had time to really complete the effort - the problem is working
with one tree (mainline) while testing on Rabeeh's kernel tree to get
a fully working Cubox platform is quite a hinderance.
Not only that, but the lack of assistance from ASoC people on the
multiple substream stuff really doesn't help one iota when it comes to
making forward progress with SPDIF support. I've explained to them the
problem (SPDIF and I2S if used together must be enabled simultaneously
or only one is permitted at any one time); they've suggested using the
multiple PCM substream support, and then it's a case that "oh it's
undocumented" "oh there's no examples yet".
And then there's the problem of understanding the ASoC DAPM stuff
"oh it's just a graph walk, you don't need to know the internals"
is what I get from ASoC people. Well, yes I do need to know the
internals, so I can find out whether it's going to startup a second
substream while a first one is active, and end up with SPDIF enabled
non-concurrently with I2S.
On Tue, Jul 23, 2013 at 03:30:57PM +0200, Sebastian Hesselbarth wrote:
> On 07/23/13 15:20, Mark Brown wrote:
> >Why would this be required? The driver is already asking for multiple
> >clocks...
> The driver is asking for multiple *DT based* clocks. Legacy
> platform_data has never been updated to reflect that. Mainly because
> multiple clocks are only supported on Dove, which has no active non-DT
> board in mainline.
Why would platform data have anything to do with this? To repeat again
the way the clocks are mapped should be totally transparent to the
driver requesting them, if it isn't then the driver is not using the API
properly.
On Tue, Jul 23, 2013 at 04:01:50PM +0100, Mark Brown wrote:
> On Tue, Jul 23, 2013 at 03:30:57PM +0200, Sebastian Hesselbarth wrote:
> > On 07/23/13 15:20, Mark Brown wrote:
>
> > >Why would this be required? The driver is already asking for multiple
> > >clocks...
>
> > The driver is asking for multiple *DT based* clocks. Legacy
> > platform_data has never been updated to reflect that. Mainly because
> > multiple clocks are only supported on Dove, which has no active non-DT
> > board in mainline.
>
> Why would platform data have anything to do with this? To repeat again
> the way the clocks are mapped should be totally transparent to the
> driver requesting them, if it isn't then the driver is not using the API
> properly.
Total rubbish. Of course the driver needs to know what the clocks are,
so that it can program its hardware accordingly.
What you have here is an audio block which has two clock inputs. One
clock is the system clock, whose rate can be adjusted but has a massive
impact on the rest of the system. The other clock input is via an
external pin.
Internally, in one of the audio blocks registers is a set of control bits
which select which clock is to be used to generate the internal timing
for the block, and a divisor on that input.
>From that description, anyone can see that it is absolutely required for
the driver to know which clock is which, so it can program the clock
input selection bit appropriately.
In this case, it has always been the rule with the clock API that it
shall be used as:
clk_get(device, "internal");
to get the internal clock, and:
clk_get(device, "external");
to get the external clock - or whatever names are appropriate to name the
clock _inputs_.
On Tue, 23 Jul 2013 15:30:57 +0200
Sebastian Hesselbarth <[email protected]> wrote:
> >> again, we mentioned to merge kirkwood-i2s.c and kirkwood-dma.c into
> >> a single file, didn't we?
> >
> > That's been discussed several times but nobody's actually done it.
>
> Correct, that is why I repeated that request to Jean-Francois.
Thanks for all your remarks.
I am testing a new version, with mainly:
- "internal" and "external" clocks
- kirkwood-i2s and kirkwood-dma in one module
- no S/PDIF (I have not yet tried i2s with the tda998x)
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
On Tue, Jul 23, 2013 at 02:50:00PM +0100, Russell King - ARM Linux wrote:
> Not only that, but the lack of assistance from ASoC people on the
> multiple substream stuff really doesn't help one iota when it comes to
> making forward progress with SPDIF support. I've explained to them the
It'd probably help if the people looking at this hardware started to
contribute some of the work that's being done here; this should help
move things forward by looking at concrete code rather than the very
general discussion that's happening at the minute.
> problem (SPDIF and I2S if used together must be enabled simultaneously
> or only one is permitted at any one time); they've suggested using the
> multiple PCM substream support, and then it's a case that "oh it's
> undocumented" "oh there's no examples yet".
As previously mentioned the OMAP4 and Qualcomm stuff is there, it's just
out of tree - there's also Haswell but I'm not sure if that's been made
public yet (Liam?).
> And then there's the problem of understanding the ASoC DAPM stuff
> "oh it's just a graph walk, you don't need to know the internals"
> is what I get from ASoC people. Well, yes I do need to know the
> internals, so I can find out whether it's going to startup a second
> substream while a first one is active, and end up with SPDIF enabled
> non-concurrently with I2S.
As ever the answer is that DAPM will enable any connected audio path and
not enable other ones so it is possible to enable only one of multiple
paths (as you'd expect, really).
Can you be more specific about the problems you're having following the
code? It should be relatively simple to trace through what's going on
from dapm_power_widgets()...
On Tue, Jul 23, 2013 at 04:19:56PM +0100, Russell King - ARM Linux wrote:
> On Tue, Jul 23, 2013 at 04:01:50PM +0100, Mark Brown wrote:
> > Why would platform data have anything to do with this? To repeat again
> > the way the clocks are mapped should be totally transparent to the
> > driver requesting them, if it isn't then the driver is not using the API
> > properly.
> Total rubbish. Of course the driver needs to know what the clocks are,
> so that it can program its hardware accordingly.
Of course, but why through platform data? Though reading your mail I
think we're talking at cross purposes here...
> In this case, it has always been the rule with the clock API that it
> shall be used as:
> clk_get(device, "internal");
> to get the internal clock, and:
> clk_get(device, "external");
> to get the external clock - or whatever names are appropriate to name the
> clock _inputs_.
This is exactly what I'd expect, yes - but it doesn't need the driver to
have platform data as far as I can see. As you say the driver should be
requesting the two clock inputs the IP has by name which doesn't need it
to have any platform data, it just needs code like you have above. The
board specific configuration is all handled within the clk_get() calls.