| Message ID | 20130326190539.494d96d9@armhf |
|---|---|
| State | New |
| Headers | show |
On Tue, Mar 26, 2013 at 07:05:39PM +0100, 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). > > Signed-off-by: Jean-Francois Moine <moinejf@free.fr> > --- > .../devicetree/bindings/sound/kirkwood-i2s.txt | 24 +++++++++++++ > sound/soc/kirkwood/kirkwood-i2s.c | 36 ++++++++++++++++++-- > 2 files changed, 58 insertions(+), 2 deletions(-) > create mode 100644 Documentation/devicetree/bindings/sound/kirkwood-i2s.txt > > diff --git a/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt b/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt > new file mode 100644 > index 0000000..a089504 > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt > @@ -0,0 +1,24 @@ > +* Kirkwood I2S controller > + > +Required properties: > + > +- compatible: "marvell,kirkwood-i2s" Hi Jean-Francois We should maybe change this to marvell,mvebu-i2s. Kirkwood, Dove, and 370 have i2s. We know Dove and Kirkwood are compatible, and its likely 370 is also compatible. So a renaming will avoid confusion in the long run. We can leave the filename of the driver alone for the moment, since that is not an ABI. > +- 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: phandle of the internal or external clock. > + > +Non-standard property: There does not appear to be an i2s standard, so saying a property is non-standard is a bit odd. I would just say Optional Properties, and drop the (optional) below. > + > +- marvell,burst-size (optional): burst size which can be only 32 or 128. > + Default is 128. > + > +Example: > + > +i2s1: audio-controller@b4000 { > + compatible = "marvell,kirkwood-i2s"; > + reg = <0xb4000 0x4000>; I've not looked, but 0x4000 seem rather large for an register range. > + interrupts = <21>, <22>; > + clocks = <&gate_clk 13>; > +}; > diff --git a/sound/soc/kirkwood/kirkwood-i2s.c b/sound/soc/kirkwood/kirkwood-i2s.c > index afca1ec..dceb9f8 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" > @@ -485,10 +486,30 @@ static int kirkwood_i2s_dev_probe(struct platform_device *pdev) > return -ENXIO; > } > > +#ifdef CONFIG_OF > + if (!data) { > + struct device_node *np = pdev->dev.of_node; > + u32 val; > + > + if (!np > + || of_property_read_u32(np, "marvell,burst-size", &val)) { It is normal to put the || on the previous line. > + val = 128; > + } else if (val != 32 && val != 128) { > + dev_warn(&pdev->dev, > + "'marvell,burst-size' can be only 32 or 128" > + "- value forced to 128\n"); I would prefer to return EINVAL, or some other error code. Invalid DT contents will get noticed faster that way. > + val = 128; > + } > + priv->burst = val; > + } else { > + priv->burst = data->burst; > + } > +#else > if (!data) { > dev_err(&pdev->dev, "no platform data ?!\n"); > return -EINVAL; > } > +#endif > > priv->burst = data->burst; > > @@ -519,7 +540,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 { > @@ -556,12 +577,23 @@ static int kirkwood_i2s_dev_remove(struct platform_device *pdev) > return 0; > } > > +#ifdef CONFIG_OF > +static struct of_device_id kirkwood_i2s_of_match[] = { > + { .compatible = "marvell,kirkwood-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, > +#ifdef CONFIG_OF > + .of_match_table??=??kirkwood_i2s_of_match, > +#endif You should be able to skip all these #ifdefs. Andrew
On 03/26/2013 07:05 PM, 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). > > Signed-off-by: Jean-Francois Moine<moinejf@free.fr> > --- > .../devicetree/bindings/sound/kirkwood-i2s.txt | 24 +++++++++++++ > sound/soc/kirkwood/kirkwood-i2s.c | 36 ++++++++++++++++++-- > 2 files changed, 58 insertions(+), 2 deletions(-) > create mode 100644 Documentation/devicetree/bindings/sound/kirkwood-i2s.txt > > diff --git a/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt b/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt > new file mode 100644 > index 0000000..a089504 > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt > @@ -0,0 +1,24 @@ > +* Kirkwood I2S controller > + > +Required properties: > + > +- compatible: "marvell,kirkwood-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: phandle of the internal or external clock. Jean-Francois, have you tested the driver with external clock set instead of internal clock? I guess it will hang on access, as internal clock comes from a clock gate that needs to be enabled prior use of i2s core. I suggest to have two clocks, 0 for internal and optional (1) for external clock. You can get them with of_clk_get(np, 0) and of_clk_get(np, 1) respectively. > + > +Non-standard property: > + > +- marvell,burst-size (optional): burst size which can be only 32 or 128. > + Default is 128. Do we really want to expose this to the device node? If this is set to different values for kirkwood, dove, and a370, we can still have it set by corresponding compatible strings and match data. (See below) > + > +Example: > + > +i2s1: audio-controller@b4000 { > + compatible = "marvell,kirkwood-i2s"; > + reg =<0xb4000 0x4000>; > + interrupts =<21>,<22>; > + clocks =<&gate_clk 13>; > +}; > diff --git a/sound/soc/kirkwood/kirkwood-i2s.c b/sound/soc/kirkwood/kirkwood-i2s.c > index afca1ec..dceb9f8 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" > @@ -485,10 +486,30 @@ static int kirkwood_i2s_dev_probe(struct platform_device *pdev) > return -ENXIO; > } > > +#ifdef CONFIG_OF > + if (!data) { You can remove the #ifdef and check for pdev->dev.of_node instead. All of_xx functions should have no-op counterparts if CONFIG_OF is not set. > + struct device_node *np = pdev->dev.of_node; > + u32 val; > + > + if (!np > + || of_property_read_u32(np, "marvell,burst-size",&val)) { > + val = 128; > + } else if (val != 32&& val != 128) { > + dev_warn(&pdev->dev, > + "'marvell,burst-size' can be only 32 or 128" > + "- value forced to 128\n"); > + val = 128; > + } If you initialize val with 128 you can just use of_property_read as it will only modify val on success. You will have to check for valid values of course later on. > + priv->burst = val; > + } else { > + priv->burst = data->burst; > + } > +#else > if (!data) { > dev_err(&pdev->dev, "no platform data ?!\n"); > return -EINVAL; > } > +#endif > > priv->burst = data->burst; > > @@ -519,7 +540,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 { > @@ -556,12 +577,23 @@ static int kirkwood_i2s_dev_remove(struct platform_device *pdev) > return 0; > } > > +#ifdef CONFIG_OF > +static struct of_device_id kirkwood_i2s_of_match[] = { > + { .compatible = "marvell,kirkwood-i2s" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, kirkwood_i2s_of_match); > +#endif If there is SoC specific data (e.g. burst_size) you can use .data to pass it according to the compatible string: static struct of_device_id kirkwood_i2s_of_match[] = { { .compatible = "marvell,kirkwood-i2s", .data = (void *)32 }, { .compatible = "marvell,dove-i2s", .data = (void *)128 }, { .compatible = "marvell,armada-370-i2s", .data = (void *)128 }, { } }; Or have some SoC specific struct i2s_soc_data passed with .data, if there is more than one value you need to pass. > + > static struct platform_driver kirkwood_i2s_driver = { > .probe = kirkwood_i2s_dev_probe, > .remove = kirkwood_i2s_dev_remove, > .driver = { > .name = DRV_NAME, > .owner = THIS_MODULE, > +#ifdef CONFIG_OF > + .of_match_table = kirkwood_i2s_of_match, > +#endif Remove the $ifdef and have .of_match_table = of_match_ptr(kirkwood_i2s_of_match), > }, > }; > Sebastian
On Tue, 26 Mar 2013 19:47:58 +0100 Andrew Lunn <andrew@lunn.ch> wrote: [snip] > > diff --git a/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt b/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt > > new file mode 100644 > > index 0000000..a089504 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt > > @@ -0,0 +1,24 @@ > > +* Kirkwood I2S controller > > + > > +Required properties: > > + > > +- compatible: "marvell,kirkwood-i2s" > > Hi Jean-Francois > > We should maybe change this to marvell,mvebu-i2s. Kirkwood, Dove, and > 370 have i2s. We know Dove and Kirkwood are compatible, and its likely > 370 is also compatible. So a renaming will avoid confusion in the long > run. We can leave the filename of the driver alone for the moment, > since that is not an ABI. OK. I will do an other version of the dove DT extension. > > +- 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: phandle of the internal or external clock. > > + > > +Non-standard property: > > There does not appear to be an i2s standard, so saying a property is > non-standard is a bit odd. I would just say Optional Properties, and > drop the (optional) below. No problem. I just did a copy/yank from an other binding. > > + > > +- marvell,burst-size (optional): burst size which can be only 32 or 128. > > + Default is 128. > > + > > +Example: > > + > > +i2s1: audio-controller@b4000 { > > + compatible = "marvell,kirkwood-i2s"; > > + reg = <0xb4000 0x4000>; > > I've not looked, but 0x4000 seem rather large for an register range. I got the value from kirkwood/common.c (SZ_16K). But, yes, in the 88AP510 doc, it seems that the last register is 2208. I will put 2210. > > + interrupts = <21>, <22>; > > + clocks = <&gate_clk 13>; > > +}; > > diff --git a/sound/soc/kirkwood/kirkwood-i2s.c b/sound/soc/kirkwood/kirkwood-i2s.c > > index afca1ec..dceb9f8 100644 > > --- a/sound/soc/kirkwood/kirkwood-i2s.c > > +++ b/sound/soc/kirkwood/kirkwood-i2s.c [snip] > > @@ -485,10 +486,30 @@ static int kirkwood_i2s_dev_probe(struct platform_device *pdev) > > return -ENXIO; > > } > > > > +#ifdef CONFIG_OF > > + if (!data) { > > + struct device_node *np = pdev->dev.of_node; > > + u32 val; > > + > > + if (!np > > + || of_property_read_u32(np, "marvell,burst-size", &val)) { > > It is normal to put the || on the previous line. I find || and && are clearer on the next line when the condition is somewhat complex. I may change that, but I did not see any constraint in the CodingStyle... > > + val = 128; > > + } else if (val != 32 && val != 128) { > > + dev_warn(&pdev->dev, > > + "'marvell,burst-size' can be only 32 or 128" > > + "- value forced to 128\n"); > > I would prefer to return EINVAL, or some other error code. Invalid DT > contents will get noticed faster that way. OK. [snip] > > @@ -556,12 +577,23 @@ static int kirkwood_i2s_dev_remove(struct platform_device *pdev) > > return 0; > > } > > > > +#ifdef CONFIG_OF > > +static struct of_device_id kirkwood_i2s_of_match[] = { > > + { .compatible = "marvell,kirkwood-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, > > +#ifdef CONFIG_OF > > + .of_match_table??=??kirkwood_i2s_of_match, > > +#endif > > You should be able to skip all these #ifdefs. OK.
On Tue, 26 Mar 2013 20:37:34 +0100 Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> wrote: > On 03/26/2013 07:05 PM, 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). > > > > Signed-off-by: Jean-Francois Moine<moinejf@free.fr> > > --- [snip] > > diff --git a/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt b/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt > > new file mode 100644 > > index 0000000..a089504 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt > > @@ -0,0 +1,24 @@ > > +* Kirkwood I2S controller > > + > > +Required properties: > > + > > +- compatible: "marvell,kirkwood-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: phandle of the internal or external clock. > > Jean-Francois, > > have you tested the driver with external clock set instead of internal clock? Hi Sebastian, No, I don't know yet how to install and start the audio subsystem. > I guess it will hang on access, as internal clock comes from a clock gate that > needs to be enabled prior use of i2s core. I suggest to have two clocks, 0 for > internal and optional (1) for external clock. You can get them with > of_clk_get(np, 0) and of_clk_get(np, 1) respectively. You are right. Trying to code this, I came across an other (but surely known) problem: the synchronization of the module initialization. If you want to use some external clock given by, say, the si5351 module, as all modules are loaded at the same time, the module kirkwood-i2s may be initialized before the si5351 and, so, the clock will not be seen as available. Question: is there a module synchronization mechanism for the DT? (late_initcall does not work for modules) There is an other problem, tied to the si5351 module unload: how can the kirkwood-i2s know that the clock driver is unloaded, or how can it do to lock this driver? (the module of the clock driver is not seen from the clock user) A response to this problem could simply be: increase the use count of the clock module (try_module_get) when a clock is used (clk prepare or enable). > > + > > +Non-standard property: > > + > > +- marvell,burst-size (optional): burst size which can be only 32 or 128. > > + Default is 128. > > Do we really want to expose this to the device node? If this is set to different > values for kirkwood, dove, and a370, we can still have it set by corresponding > compatible strings and match data. (See below) [snip] > > diff --git a/sound/soc/kirkwood/kirkwood-i2s.c b/sound/soc/kirkwood/kirkwood-i2s.c > > index afca1ec..dceb9f8 100644 > > --- a/sound/soc/kirkwood/kirkwood-i2s.c > > +++ b/sound/soc/kirkwood/kirkwood-i2s.c [snip] > > @@ -556,12 +577,23 @@ static int kirkwood_i2s_dev_remove(struct platform_device *pdev) > > return 0; > > } > > > > +#ifdef CONFIG_OF > > +static struct of_device_id kirkwood_i2s_of_match[] = { > > + { .compatible = "marvell,kirkwood-i2s" }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(of, kirkwood_i2s_of_match); > > +#endif > > If there is SoC specific data (e.g. burst_size) you can use > .data to pass it according to the compatible string: > > static struct of_device_id kirkwood_i2s_of_match[] = { > { .compatible = "marvell,kirkwood-i2s", .data = (void *)32 }, > { .compatible = "marvell,dove-i2s", .data = (void *)128 }, > { .compatible = "marvell,armada-370-i2s", .data = (void *)128 }, > { } > }; I don't like this solution: the burst value is hidden and hard coded. If a new machine uses this same driver, it is easier to change the DT instead of recompiling a whole kernel. > Or have some SoC specific struct i2s_soc_data passed with .data, > if there is more than one value you need to pass. > > > + > > static struct platform_driver kirkwood_i2s_driver = { > > .probe = kirkwood_i2s_dev_probe, > > .remove = kirkwood_i2s_dev_remove, > > .driver = { > > .name = DRV_NAME, > > .owner = THIS_MODULE, > > +#ifdef CONFIG_OF > > + .of_match_table = kirkwood_i2s_of_match, > > +#endif > > Remove the $ifdef and have .of_match_table = of_match_ptr(kirkwood_i2s_of_match), > > > }, > > }; > > > > Sebastian OK.
> If you want to use some external clock given by, say, the si5351 > module, as all modules are loaded at the same time, the module > kirkwood-i2s may be initialized before the si5351 and, so, the clock > will not be seen as available. If the DT says the clock should exist, and of_clk_get() fails, return -EDEFER in the probe. The kernel will try again later once more modules have been loaded. > > If there is SoC specific data (e.g. burst_size) you can use > > .data to pass it according to the compatible string: > > > > static struct of_device_id kirkwood_i2s_of_match[] = { > > { .compatible = "marvell,kirkwood-i2s", .data = (void *)32 }, > > { .compatible = "marvell,dove-i2s", .data = (void *)128 }, > > { .compatible = "marvell,armada-370-i2s", .data = (void *)128 }, > > { } > > }; > > I don't like this solution: the burst value is hidden and hard coded. > If a new machine uses this same driver, it is easier to change the DT > instead of recompiling a whole kernel. If its a new SoC family, its not a change to DT, its a whole new DT, for that SoC family. There is no inheritance between SoC families. There is no sharing between dove, kirkwood, 370 for .dts or dtsi files. As far as i can see, burst is a SoC family property. So will not be expected to change from board to board, which is the typical use case for DT. Andrew
OOn Wed, 27 Mar 2013 13:01:19 +0100 Andrew Lunn <andrew@lunn.ch> wrote: > > If you want to use some external clock given by, say, the si5351 > > module, as all modules are loaded at the same time, the module > > kirkwood-i2s may be initialized before the si5351 and, so, the clock > > will not be seen as available. > > If the DT says the clock should exist, and of_clk_get() fails, return > -EDEFER in the probe. The kernel will try again later once more > modules have been loaded. Hi Andrew, I will try that. > > > If there is SoC specific data (e.g. burst_size) you can use > > > .data to pass it according to the compatible string: > > > > > > static struct of_device_id kirkwood_i2s_of_match[] = { > > > { .compatible = "marvell,kirkwood-i2s", .data = (void *)32 }, > > > { .compatible = "marvell,dove-i2s", .data = (void *)128 }, > > > { .compatible = "marvell,armada-370-i2s", .data = (void *)128 }, > > > { } > > > }; > > > > I don't like this solution: the burst value is hidden and hard coded. > > If a new machine uses this same driver, it is easier to change the DT > > instead of recompiling a whole kernel. > > If its a new SoC family, its not a change to DT, its a whole new DT, > for that SoC family. There is no inheritance between SoC > families. There is no sharing between dove, kirkwood, 370 for .dts or > dtsi files. As far as i can see, burst is a SoC family property. So > will not be expected to change from board to board, which is the > typical use case for DT. Maybe I did not explain clearly. Suppose Marvell creates a new machine, say "toto", which includes the same audio controller. In its DT, there would be something as "marvell,toto-i2s". So, to use it, you should add a line: { .compatible = "marvell,toto-i2s", .data = (void *)128 }, in the kirkwood-i2s.c. While, if the 'burst' value is defined in the DT, there would be no change in this file kirkwood-i2s.c, and, as you proposed, the of_device_id would be simply: { .compatible = "marvell,mvebu-i2s" },
diff --git a/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt b/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt new file mode 100644 index 0000000..a089504 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/kirkwood-i2s.txt @@ -0,0 +1,24 @@ +* Kirkwood I2S controller + +Required properties: + +- compatible: "marvell,kirkwood-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: phandle of the internal or external clock. + +Non-standard property: + +- marvell,burst-size (optional): burst size which can be only 32 or 128. + Default is 128. + +Example: + +i2s1: audio-controller@b4000 { + compatible = "marvell,kirkwood-i2s"; + reg = <0xb4000 0x4000>; + interrupts = <21>, <22>; + clocks = <&gate_clk 13>; +}; diff --git a/sound/soc/kirkwood/kirkwood-i2s.c b/sound/soc/kirkwood/kirkwood-i2s.c index afca1ec..dceb9f8 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" @@ -485,10 +486,30 @@ static int kirkwood_i2s_dev_probe(struct platform_device *pdev) return -ENXIO; } +#ifdef CONFIG_OF + if (!data) { + struct device_node *np = pdev->dev.of_node; + u32 val; + + if (!np + || of_property_read_u32(np, "marvell,burst-size", &val)) { + val = 128; + } else if (val != 32 && val != 128) { + dev_warn(&pdev->dev, + "'marvell,burst-size' can be only 32 or 128" + "- value forced to 128\n"); + val = 128; + } + priv->burst = val; + } else { + priv->burst = data->burst; + } +#else if (!data) { dev_err(&pdev->dev, "no platform data ?!\n"); return -EINVAL; } +#endif priv->burst = data->burst; @@ -519,7 +540,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 { @@ -556,12 +577,23 @@ static int kirkwood_i2s_dev_remove(struct platform_device *pdev) return 0; } +#ifdef CONFIG_OF +static struct of_device_id kirkwood_i2s_of_match[] = { + { .compatible = "marvell,kirkwood-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, +#ifdef CONFIG_OF + .of_match_table = kirkwood_i2s_of_match, +#endif }, };
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 <moinejf@free.fr> --- .../devicetree/bindings/sound/kirkwood-i2s.txt | 24 +++++++++++++ sound/soc/kirkwood/kirkwood-i2s.c | 36 ++++++++++++++++++-- 2 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/kirkwood-i2s.txt