diff mbox

[3/3] media: mx2-emmaprp: Add devicetree support

Message ID 1399015119-24000-1-git-send-email-shc_work@mail.ru (mailing list archive)
State New, archived
Headers show

Commit Message

Alexander Shiyan May 2, 2014, 7:18 a.m. UTC 
This patch adds devicetree support for the Freescale enhanced Multimedia
Accelerator (eMMA) video Pre-processor (PrP).

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 .../devicetree/bindings/media/fsl-imx-emmaprp.txt     | 19 +++++++++++++++++++
 drivers/media/platform/mx2_emmaprp.c                  |  8 ++++++++
 2 files changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt

Comments

Sylwester Nawrocki/Kernel \(PLT\) /SRPOL/Staff Engineer/Samsung Electronics May 13, 2014, 5:09 p.m. UTC | #1 
Hi,

On 02/05/14 09:18, Alexander Shiyan wrote:
> This patch adds devicetree support for the Freescale enhanced Multimedia
> Accelerator (eMMA) video Pre-processor (PrP).
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  .../devicetree/bindings/media/fsl-imx-emmaprp.txt     | 19 +++++++++++++++++++
>  drivers/media/platform/mx2_emmaprp.c                  |  8 ++++++++
>  2 files changed, 27 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt b/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt
> new file mode 100644
> index 0000000..9e8238f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt
> @@ -0,0 +1,19 @@
> +* Freescale enhanced Multimedia Accelerator (eMMA) video Pre-processor (PrP)
> +  for i.MX.
> +
> +Required properties:
> +- compatible : Shall contain "fsl,imx21-emmaprp".
> +- reg        : Offset and length of the register set for the device.
> +- interrupts : Should contain eMMA PrP interrupt number.
> +- clocks     : Should contain the ahb and ipg clocks, in the order
> +               determined by the clock-names property.
> +- clock-names: Should be "ahb", "ipg".
> +
> +Example:
> +	emmaprp: emmaprp@10026400 {
> +		compatible = "fsl,imx27-emmaprp", "fsl,imx21-emmaprp";

Is "fsl,imx27-emmaprp" compatible documented somewhere ?

> +		reg = <0x10026400 0x100>;
> +		interrupts = <51>;
> +		clocks = <&clks 49>, <&clks 68>;
> +		clock-names = "ipg", "ahb";
> +	};
> diff --git a/drivers/media/platform/mx2_emmaprp.c b/drivers/media/platform/mx2_emmaprp.c
> index fa8f7ca..0646bda 100644
> --- a/drivers/media/platform/mx2_emmaprp.c
> +++ b/drivers/media/platform/mx2_emmaprp.c
> @@ -18,6 +18,7 @@
>   */
>  #include <linux/module.h>
>  #include <linux/clk.h>
> +#include <linux/of.h>
>  #include <linux/slab.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> @@ -1005,12 +1006,19 @@ static int emmaprp_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct of_device_id __maybe_unused emmaprp_dt_ids[] = {
> +	{ .compatible = "fsl,imx21-emmaprp", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, emmaprp_dt_ids);
> +
>  static struct platform_driver emmaprp_pdrv = {
>  	.probe		= emmaprp_probe,
>  	.remove		= emmaprp_remove,
>  	.driver		= {
>  		.name	= MEM2MEM_NAME,
>  		.owner	= THIS_MODULE,
> +		.of_match_table = of_match_ptr(emmaprp_dt_ids),
>  	},
>  };
>  module_platform_driver(emmaprp_pdrv);

I believe documentation and driver changes should be in separate patches.
Otherwise looks good.
Alexander Shiyan May 13, 2014, 5:23 p.m. UTC | #2 
Tue, 13 May 2014 19:09:30 +0200 ?? Sylwester Nawrocki <s.nawrocki@samsung.com>:
> Hi,
> 
> On 02/05/14 09:18, Alexander Shiyan wrote:
> > This patch adds devicetree support for the Freescale enhanced Multimedia
> > Accelerator (eMMA) video Pre-processor (PrP).
> > 
> > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > ---
> >  .../devicetree/bindings/media/fsl-imx-emmaprp.txt     | 19 +++++++++++++++++++
> >  drivers/media/platform/mx2_emmaprp.c                  |  8 ++++++++
> >  2 files changed, 27 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt b/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt
> > new file mode 100644
> > index 0000000..9e8238f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt
> > @@ -0,0 +1,19 @@
> > +* Freescale enhanced Multimedia Accelerator (eMMA) video Pre-processor (PrP)
> > +  for i.MX.
> > +
> > +Required properties:
> > +- compatible : Shall contain "fsl,imx21-emmaprp".
> > +- reg        : Offset and length of the register set for the device.
> > +- interrupts : Should contain eMMA PrP interrupt number.
> > +- clocks     : Should contain the ahb and ipg clocks, in the order
> > +               determined by the clock-names property.
> > +- clock-names: Should be "ahb", "ipg".
> > +
> > +Example:
> > +	emmaprp: emmaprp@10026400 {
> > +		compatible = "fsl,imx27-emmaprp", "fsl,imx21-emmaprp";
> 
> Is "fsl,imx27-emmaprp" compatible documented somewhere ?

The overall structure of the eMMA module is slightly different.
As for the part of the PrP, according to the datasheet they are compatible.

...

---
Sylwester Nawrocki/Kernel \(PLT\) /SRPOL/Staff Engineer/Samsung Electronics May 14, 2014, 10:49 a.m. UTC | #3 
On 13/05/14 19:23, Alexander Shiyan wrote:
> Tue, 13 May 2014 19:09:30 +0200 ?? Sylwester Nawrocki <s.nawrocki@samsung.com>:
>> > Hi,
>> > 
>> > On 02/05/14 09:18, Alexander Shiyan wrote:
>>> > > This patch adds devicetree support for the Freescale enhanced Multimedia
>>> > > Accelerator (eMMA) video Pre-processor (PrP).
>>> > > 
>>> > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
>>> > > ---
>>> > >  .../devicetree/bindings/media/fsl-imx-emmaprp.txt     | 19 +++++++++++++++++++
>>> > >  drivers/media/platform/mx2_emmaprp.c                  |  8 ++++++++
>>> > >  2 files changed, 27 insertions(+)
>>> > >  create mode 100644 Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt
>>> > > 
>>> > > diff --git a/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt b/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt
>>> > > new file mode 100644
>>> > > index 0000000..9e8238f
>>> > > --- /dev/null
>>> > > +++ b/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt
>>> > > @@ -0,0 +1,19 @@
>>> > > +* Freescale enhanced Multimedia Accelerator (eMMA) video Pre-processor (PrP)
>>> > > +  for i.MX.
>>> > > +
>>> > > +Required properties:
>>> > > +- compatible : Shall contain "fsl,imx21-emmaprp".
>>> > > +- reg        : Offset and length of the register set for the device.
>>> > > +- interrupts : Should contain eMMA PrP interrupt number.
>>> > > +- clocks     : Should contain the ahb and ipg clocks, in the order
>>> > > +               determined by the clock-names property.
>>> > > +- clock-names: Should be "ahb", "ipg".
>>> > > +
>>> > > +Example:
>>> > > +	emmaprp: emmaprp@10026400 {
>>> > > +		compatible = "fsl,imx27-emmaprp", "fsl,imx21-emmaprp";
>> > 
>> > Is "fsl,imx27-emmaprp" compatible documented somewhere ?
>
> The overall structure of the eMMA module is slightly different.
> As for the part of the PrP, according to the datasheet they are compatible.

Then can we please have all the valid compatible strings listed at the
'compatible' property's description above ? I think it is useful to have
an indication to which SoC each of them apply in documentation of the
binding.

--
Thanks,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Shiyan May 14, 2014, 4:59 p.m. UTC | #4 
Wed, 14 May 2014 12:49:03 +0200 ?? Sylwester Nawrocki <s.nawrocki@samsung.com>:
> On 13/05/14 19:23, Alexander Shiyan wrote:
> > Tue, 13 May 2014 19:09:30 +0200 ?? Sylwester Nawrocki <s.nawrocki@samsung.com>:
> >> > Hi,
> >> > 
> >> > On 02/05/14 09:18, Alexander Shiyan wrote:
> >>> > > This patch adds devicetree support for the Freescale enhanced Multimedia
> >>> > > Accelerator (eMMA) video Pre-processor (PrP).
> >>> > > 
> >>> > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> >>> > > ---
> >>> > >  .../devicetree/bindings/media/fsl-imx-emmaprp.txt     | 19 +++++++++++++++++++
> >>> > >  drivers/media/platform/mx2_emmaprp.c                  |  8 ++++++++
> >>> > >  2 files changed, 27 insertions(+)
> >>> > >  create mode 100644 Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt
> >>> > > 
> >>> > > diff --git a/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt b/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt
> >>> > > new file mode 100644
> >>> > > index 0000000..9e8238f
> >>> > > --- /dev/null
> >>> > > +++ b/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt
> >>> > > @@ -0,0 +1,19 @@
> >>> > > +* Freescale enhanced Multimedia Accelerator (eMMA) video Pre-processor (PrP)
> >>> > > +  for i.MX.
> >>> > > +
> >>> > > +Required properties:
> >>> > > +- compatible : Shall contain "fsl,imx21-emmaprp".
> >>> > > +- reg        : Offset and length of the register set for the device.
> >>> > > +- interrupts : Should contain eMMA PrP interrupt number.
> >>> > > +- clocks     : Should contain the ahb and ipg clocks, in the order
> >>> > > +               determined by the clock-names property.
> >>> > > +- clock-names: Should be "ahb", "ipg".
> >>> > > +
> >>> > > +Example:
> >>> > > +	emmaprp: emmaprp@10026400 {
> >>> > > +		compatible = "fsl,imx27-emmaprp", "fsl,imx21-emmaprp";
> >> > 
> >> > Is "fsl,imx27-emmaprp" compatible documented somewhere ?
> >
> > The overall structure of the eMMA module is slightly different.
> > As for the part of the PrP, according to the datasheet they are compatible.
> 
> Then can we please have all the valid compatible strings listed at the
> 'compatible' property's description above ? I think it is useful to have
> an indication to which SoC each of them apply in documentation of the
> binding.

Traditionally, i.MX drivers uses youngest chip for compatibility string.
The best example of this: drivers/bus/imx-weim.c

---
Shawn Guo May 15, 2014, 1:19 a.m. UTC | #5 
On Wed, May 14, 2014 at 08:59:54PM +0400, Alexander Shiyan wrote:
> Wed, 14 May 2014 12:49:03 +0200 ?? Sylwester Nawrocki <s.nawrocki@samsung.com>:
> > On 13/05/14 19:23, Alexander Shiyan wrote:
> > > Tue, 13 May 2014 19:09:30 +0200 ?? Sylwester Nawrocki <s.nawrocki@samsung.com>:
> > >> > Hi,
> > >> > 
> > >> > On 02/05/14 09:18, Alexander Shiyan wrote:
> > >>> > > This patch adds devicetree support for the Freescale enhanced Multimedia
> > >>> > > Accelerator (eMMA) video Pre-processor (PrP).
> > >>> > > 
> > >>> > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > >>> > > ---
> > >>> > >  .../devicetree/bindings/media/fsl-imx-emmaprp.txt     | 19 +++++++++++++++++++
> > >>> > >  drivers/media/platform/mx2_emmaprp.c                  |  8 ++++++++
> > >>> > >  2 files changed, 27 insertions(+)
> > >>> > >  create mode 100644 Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt
> > >>> > > 
> > >>> > > diff --git a/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt b/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt
> > >>> > > new file mode 100644
> > >>> > > index 0000000..9e8238f
> > >>> > > --- /dev/null
> > >>> > > +++ b/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt
> > >>> > > @@ -0,0 +1,19 @@
> > >>> > > +* Freescale enhanced Multimedia Accelerator (eMMA) video Pre-processor (PrP)
> > >>> > > +  for i.MX.
> > >>> > > +
> > >>> > > +Required properties:
> > >>> > > +- compatible : Shall contain "fsl,imx21-emmaprp".
> > >>> > > +- reg        : Offset and length of the register set for the device.
> > >>> > > +- interrupts : Should contain eMMA PrP interrupt number.
> > >>> > > +- clocks     : Should contain the ahb and ipg clocks, in the order
> > >>> > > +               determined by the clock-names property.
> > >>> > > +- clock-names: Should be "ahb", "ipg".
> > >>> > > +
> > >>> > > +Example:
> > >>> > > +	emmaprp: emmaprp@10026400 {
> > >>> > > +		compatible = "fsl,imx27-emmaprp", "fsl,imx21-emmaprp";
> > >> > 
> > >> > Is "fsl,imx27-emmaprp" compatible documented somewhere ?
> > >
> > > The overall structure of the eMMA module is slightly different.
> > > As for the part of the PrP, according to the datasheet they are compatible.
> > 
> > Then can we please have all the valid compatible strings listed at the
> > 'compatible' property's description above ? I think it is useful to have
> > an indication to which SoC each of them apply in documentation of the
> > binding.
> 
> Traditionally, i.MX drivers uses youngest chip for compatibility string.
> The best example of this: drivers/bus/imx-weim.c

I guess Sylwester's point is either "fsl,imx27-emmaprp" is documented in
the bindings or it shouldn't be used anywhere.

Shawn
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt b/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt
new file mode 100644
index 0000000..9e8238f
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/fsl-imx-emmaprp.txt
@@ -0,0 +1,19 @@ 
+* Freescale enhanced Multimedia Accelerator (eMMA) video Pre-processor (PrP)
+  for i.MX.
+
+Required properties:
+- compatible : Shall contain "fsl,imx21-emmaprp".
+- reg        : Offset and length of the register set for the device.
+- interrupts : Should contain eMMA PrP interrupt number.
+- clocks     : Should contain the ahb and ipg clocks, in the order
+               determined by the clock-names property.
+- clock-names: Should be "ahb", "ipg".
+
+Example:
+	emmaprp: emmaprp@10026400 {
+		compatible = "fsl,imx27-emmaprp", "fsl,imx21-emmaprp";
+		reg = <0x10026400 0x100>;
+		interrupts = <51>;
+		clocks = <&clks 49>, <&clks 68>;
+		clock-names = "ipg", "ahb";
+	};
diff --git a/drivers/media/platform/mx2_emmaprp.c b/drivers/media/platform/mx2_emmaprp.c
index fa8f7ca..0646bda 100644
--- a/drivers/media/platform/mx2_emmaprp.c
+++ b/drivers/media/platform/mx2_emmaprp.c
@@ -18,6 +18,7 @@ 
  */
 #include <linux/module.h>
 #include <linux/clk.h>
+#include <linux/of.h>
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -1005,12 +1006,19 @@  static int emmaprp_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id __maybe_unused emmaprp_dt_ids[] = {
+	{ .compatible = "fsl,imx21-emmaprp", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, emmaprp_dt_ids);
+
 static struct platform_driver emmaprp_pdrv = {
 	.probe		= emmaprp_probe,
 	.remove		= emmaprp_remove,
 	.driver		= {
 		.name	= MEM2MEM_NAME,
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(emmaprp_dt_ids),
 	},
 };
 module_platform_driver(emmaprp_pdrv);