Re: [PATCH] ARM: EXYNOS: Add MFC device tree support

2012-09-05 Thread Arun Kumar K
Hi Karol,
Thank you very much for the detailed explanation.
Its indeed very well explained and seems like a great approach 
to remove the hard codings.
I will go ahead with this implementation and post the updated patch.

Regards
Arun

On Wed, Sep 5, 2012 at 8:12 AM, Karol Lewandowski k.lewando...@samsung.com 
wrote:
 On 08/28/2012 07:08 PM, Arun Kumar K wrote:

 Hi Karol,
 Thanks for your comments.
 Please find my response inline.

 Hi... and sorry for so much delayed response.

 +
 +static void __init exynos5_reserve(void)
 +{
 + s5p_mfc_reserve_mem(0x4300, 8  20, 0x5100, 8  20);


 I think it would make sense to make this memory reservation dependent
 on mfc* node being present in DTS.  It's to early to use of_* functions
 (because tree is not populated at this stage) but fdt_* family of functions
 work just fine.


 As I can see the fdt_* functions are not used in any of the ARM based SoC
 init codes. Though I can see some references in powerpc.
 The implementation and includes are present in arch/arm/boot/compressed/
 which I think cannot be used directly in mach-exynos unless we make some
 comon makefile changes.


 It looks like I was writing from memory, and I actually mixed things
 up.  To be clear this time - we can't use regular device tree handling
 functions in reserve() as it's too early.  Namely, flattened device tree
 is not yet converted to kernel's-natural representation.  However,
 I think we can scan fdt just fine.  To do so one just needs to use
 functions declared here

   #include linux/of_fdt.h

 Actual architecture-independent code is in drivers/of/fdt.c.  This
 provides of_fdt_ family of functions. Please see below for example.


 Please clarify whether its ok to use fdt_* functions to parse the dts in
 exynos machine init or please point me to some sample implementations
 which I can refer to.


 It should be ok to use anything that works on flattened device tree
 rather than its uncompressed version.  I've experimented a bit and
 something like this worked for me just fine (it was around 3.3-kernel
 timeframe, but I don't think that fdt api has changed):

 [mach-exynos4-dt.c]

 #include linux/of_fdt.h

 int fdt_find_compat(unsigned long node, const char *uname, int depth, void 
 *data)
 {
 if (of_flat_dt_is_compatible(node, (char *)data))
 return 1;

 return 0;
 }

 static void __init exynos4210_dt_reserve(void)
 {
 /* Reserve memory for MFC only if it's available */
 if (of_scan_flat_dt(fdt_find_compat, samsung,s5pv210-mfc)) {
 printk(KERN_NOTICE exynos4-dt: mfc device node found - 
 setting up memory area for dma\n);
 s5p_mfc_reserve_mem(0x4300, 8  20, 0x5100, 8  20);
 }
 }

 [.dts]

  codec@some-addr {
   compatible = samsung,s5pv210-mfc;
  };


 So, in above code fragment I just check if mfc was defined in dts.
 This could probably stay as it is.

 Then I allocate _predefined_ region - and this part should be fixed.

 If you have nodes like mfc-r-size/offset, then you could just get
 this information directly from (f)dt rather than hardcoding it in the code.
 Precisely, after we find compatible node we could do something like
 following (untested):

 unsigned long lsize, loff, rsize, roff len;
 __be32 *prop;

 prop = of_get_flat_dt_prop(node, samsung,mfc-l-size, len);
 if (!prop)
return;
 lsize = of_read_ulong(prop, len/4);
 ...

 Regards,
 --
 Karol Lewandowski | Samsung Poland RD Center | Linux/Platform
 --
 To unsubscribe from this list: send the line unsubscribe linux-samsung-soc 
 in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: EXYNOS: Add MFC device tree support

2012-09-04 Thread Karol Lewandowski
On 08/28/2012 07:08 PM, Arun Kumar K wrote:

 Hi Karol,
 Thanks for your comments. 
 Please find my response inline.

Hi... and sorry for so much delayed response.

 +
 +static void __init exynos5_reserve(void)
 +{
 + s5p_mfc_reserve_mem(0x4300, 8  20, 0x5100, 8  20);


 I think it would make sense to make this memory reservation dependent
 on mfc* node being present in DTS.  It's to early to use of_* functions
 (because tree is not populated at this stage) but fdt_* family of functions
 work just fine.

 
 As I can see the fdt_* functions are not used in any of the ARM based SoC
 init codes. Though I can see some references in powerpc.
 The implementation and includes are present in arch/arm/boot/compressed/
 which I think cannot be used directly in mach-exynos unless we make some
 comon makefile changes. 


It looks like I was writing from memory, and I actually mixed things 
up.  To be clear this time - we can't use regular device tree handling
functions in reserve() as it's too early.  Namely, flattened device tree
is not yet converted to kernel's-natural representation.  However, 
I think we can scan fdt just fine.  To do so one just needs to use
functions declared here

  #include linux/of_fdt.h

Actual architecture-independent code is in drivers/of/fdt.c.  This
provides of_fdt_ family of functions. Please see below for example.


 Please clarify whether its ok to use fdt_* functions to parse the dts in 
 exynos machine init or please point me to some sample implementations
 which I can refer to.


It should be ok to use anything that works on flattened device tree
rather than its uncompressed version.  I've experimented a bit and
something like this worked for me just fine (it was around 3.3-kernel
timeframe, but I don't think that fdt api has changed):

[mach-exynos4-dt.c]

#include linux/of_fdt.h

int fdt_find_compat(unsigned long node, const char *uname, int depth, void 
*data)
{
if (of_flat_dt_is_compatible(node, (char *)data))
return 1;

return 0;
}

static void __init exynos4210_dt_reserve(void)
{
/* Reserve memory for MFC only if it's available */
if (of_scan_flat_dt(fdt_find_compat, samsung,s5pv210-mfc)) {
printk(KERN_NOTICE exynos4-dt: mfc device node found - setting 
up memory area for dma\n);
s5p_mfc_reserve_mem(0x4300, 8  20, 0x5100, 8  20);
}
}

[.dts]

 codec@some-addr {
  compatible = samsung,s5pv210-mfc;
 };


So, in above code fragment I just check if mfc was defined in dts.
This could probably stay as it is.

Then I allocate _predefined_ region - and this part should be fixed.

If you have nodes like mfc-r-size/offset, then you could just get
this information directly from (f)dt rather than hardcoding it in the code.
Precisely, after we find compatible node we could do something like
following (untested):

unsigned long lsize, loff, rsize, roff len;
__be32 *prop;

prop = of_get_flat_dt_prop(node, samsung,mfc-l-size, len);
if (!prop)
   return;
lsize = of_read_ulong(prop, len/4);
...

Regards,
-- 
Karol Lewandowski | Samsung Poland RD Center | Linux/Platform
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: EXYNOS: Add MFC device tree support

2012-08-28 Thread Arun Kumar K
Hi Karol,
Thanks for your comments. 
Please find my response inline.

On Mon, Aug 20, 2012 at 11:47 AM, Karol Lewandowski k.lewando...@samsung.com 
wrote:
 On 08/16/2012 08:42 PM, Thomas Abraham wrote:
 On 16 August 2012 18:01, Arun Kumar K arun.kk ... @public.gmane.org wrote:

 +  - interrupts : MFC interupt number to the CPU.
 +
 +  - samsung,mfc-r : Base address of the first memory bank used by MFC
 +   for DMA contiguous memory allocation.
 +
 +  - samsung,mfc-r-size : Size of the first memory bank.

 It is not allowed to pass buffer base address and size from device
 tree. Device tree node should describe only the MFC controller
 hardware. Any memory management related information should be handled
 outside of device tree. This helps the bindings to be reusable across
 multiple operating systems.

 The question is where elsewhere this should be described as this is strictly
 board-dependent option (number and size of RAM banks are important here).

 I agree that base addresses are bad, but I'm not aware of any functionality
 that would allow driver (actually, its platform dependent part in
 exynosN_reserve() function) to enumerate available memory banks and grab
 memory chunks from two distinct banks.

 My (lack of) knowledge ARM might be to blame here but I simply don't know
 how to achieve this. Any suggestions?


As suggested by kgene, I will pass it from the board specific dts file.


 On 08/16/2012 09:31 PM, Arun Kumar K wrote:

 +static void s5p_mfc_reserve_mem(phys_addr_t rbase, unsigned int rsize,
 + phys_addr_t lbase, unsigned int lsize) {
 +
 + if (memblock_remove(lbase, lsize)) {
 + pr_err(Failed to reserve bank1 memory for MFC device\n);
 + WARN_ON(1);
 + }
 +
 + if (memblock_remove(rbase, rsize)) {
 + pr_err(Failed to reserve bank2 memory for MFC device\n);
 + WARN_ON(1);
 + }
 +}


 non-static function with the same name is already defined in
 arch/arm/plat-samsung/s5p-dev-mfc.c. Please don't duplicate it,
 especially that you seem to be trying to do that twice!


Ok, I will use the existing function.


 diff --git a/arch/arm/mach-exynos/mach-exynos5-dt.c 
 b/arch/arm/mach-exynos/mach-exynos5-dt.c

 index ef770bc..898d2de 100644
 --- a/arch/arm/mach-exynos/mach-exynos5-dt.c
 +++ b/arch/arm/mach-exynos/mach-exynos5-dt.c
 ...
 +static void s5p_mfc_reserve_mem(phys_addr_t rbase, unsigned int rsize,
 + phys_addr_t lbase, unsigned int lsize) {
 +
 + if (memblock_remove(lbase, lsize)) {
 + pr_err(Failed to reserve bank1 memory for MFC device\n);
 + WARN_ON(1);
 + }
 +
 + if (memblock_remove(rbase, rsize)) {
 + pr_err(Failed to reserve bank2 memory for MFC device\n);
 + WARN_ON(1);
 + }
 +}


 See above.

 +
 +static void __init exynos5_reserve(void)
 +{
 + s5p_mfc_reserve_mem(0x4300, 8  20, 0x5100, 8  20);


 I think it would make sense to make this memory reservation dependent
 on mfc* node being present in DTS.  It's to early to use of_* functions
 (because tree is not populated at this stage) but fdt_* family of functions
 work just fine.


As I can see the fdt_* functions are not used in any of the ARM based SoC
init codes. Though I can see some references in powerpc.
The implementation and includes are present in arch/arm/boot/compressed/
which I think cannot be used directly in mach-exynos unless we make some
comon makefile changes. 
Please clarify whether its ok to use fdt_* functions to parse the dts in 
exynos machine init or please point me to some sample implementations
which I can refer to.

Regards
Arun

Regards
Arun

RE: [PATCH] ARM: EXYNOS: Add MFC device tree support

2012-08-27 Thread Arun Kumar K
Hi Kgene,
Thank you for the review comments.
Please find my response inline.

On Thu, Aug 23, 2012 at 1:46 PM, Kukjin Kim kgene@samsung.com wrote:
 Arun Kumar K wrote:

 Hi Thomas,
 Thank you for the comments.
 Please find my response inline.

 --- Original Message ---
 Sender : Thomas Abrahamthomas.abra...@linaro.org
 Date   : Aug 16, 2012 17:12 (GMT+05:30)
 Title  : Re: [PATCH] ARM: EXYNOS: Add MFC device tree support

 On 16 August 2012 18:01, Arun Kumar K arun...@samsung.com wrote:
  From: Naveen Krishna Chatradhi ch.nav...@samsung.com
 
  This patch adds device tree entry for MFC in the Exynos
  machines. Exynos4 SoCs support MFC v5 version and Exynos5 has
  MFC v6.x version. So making the required changes in the clock

 Since v6.1 is not used anywhere so just MFC v6 should be ok.


Ok.

  files and adds MFC to the DT device list.
 
  Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
  Signed-off-by: Arun Kumar K arun...@samsung.com
  ---
   .../devicetree/bindings/media/s5p-mfc.txt  |   24
 
   arch/arm/boot/dts/exynos4210.dtsi  |   10 
   arch/arm/boot/dts/exynos5250.dtsi  |   10 
   arch/arm/mach-exynos/clock-exynos5.c   |2 +-
   arch/arm/mach-exynos/mach-exynos4-dt.c |   22
 ++
   arch/arm/mach-exynos/mach-exynos5-dt.c |   22
 ++
   6 files changed, 89 insertions(+), 1 deletions(-)
   create mode 100644 Documentation/devicetree/bindings/media/s5p-mfc.txt
 
  diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt
 b/Documentation/devicetree/bindings/media/s5p-mfc.txt
  new file mode 100644
  index 000..b9bd266
  --- /dev/null
  +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
  @@ -0,0 +1,24 @@
  +* Samsung Multi Format Codec (MFC)
  +
  +Mult Format Codec (MFC) is the IP present in Samsung SoCs which
  +supports high resolution decoding and encoding functionalities.
 
  In addition to this, specifying that mfc is used for video
  encode/decode would be informative.

 Ok. I will make it more descriptive.

  +
  +Required properties:
  +  - compatible : value should be either one among the following
  +   (a) samsung,s5p-mfc-v5 for MFC v5 present in Exynos4 SoCs
  +   (b) samsung,s5p-mfc-v6 for MFC v6.x present in Exynos5 SoCs
 
  s5p should be dropped from the compatible values. For example, it
  should be samsung,mfc-v5, which is sufficient to identify the
  version of the mfc controller.

 Ok will remove s5p.

 Yeah, I agree, just 'mfc-vX' is enough.


Ok. Will change it.

  +
  +  - reg : Physical base address of the IP registers and length of
 memory
  + mapped region.
  +
  +  - interrupts : MFC interupt number to the CPU.
  +
  +  - samsung,mfc-r : Base address of the first memory bank used by MFC
  +   for DMA contiguous memory allocation.
  +
  +  - samsung,mfc-r-size : Size of the first memory bank.
 
  It is not allowed to pass buffer base address and size from device
  tree. Device tree node should describe only the MFC controller
  hardware. Any memory management related information should be handled
  outside of device tree. This helps the bindings to be reusable across
  multiple operating systems.

 The mfc-l and mfc-r base address and size is board specific info which has
 to
 be passed to the driver. This is used for DMA contiguous allocation by
 driver and this value
 can change on a different board.
 So in that case, i will pass it as platform data to the driver from mach-
 exynos5-dt.c file.
 I hope that would be ok.

 I don't think so. The mach-exynos5-dt is for all EXYNOS5 SoCs not
 platform_data. As I know, the addresses and sizes for buffer passed via
 platform data before, so it can be passed via device tree for board(.dtsi)?
 not SoC. In addition, it depends on board.


Ok. So as suggested the best option would be to move the mfc-l and r 
base address and size information to board dts file 
(exynos5250-smdk5250.dts) from exynos5250.dtsi. Hope this would be
good.


  +
  +  - samsung,mfc-l : Base address of the second memory bank used by MFC
  +   for DMA contiguous memory allocation.
  +
  +  - samsung,mfc-l-size : Size of the second memory bank.
 
  Same comment as above. And the bindings documentation is usually
  included in the same patch that adds device tree support for the
  driver.

 Ok

  diff --git a/arch/arm/boot/dts/exynos4210.dtsi
 b/arch/arm/boot/dts/exynos4210.dtsi
  index 02891fe..b5ee43d 100644
  --- a/arch/arm/boot/dts/exynos4210.dtsi
  +++ b/arch/arm/boot/dts/exynos4210.dtsi
  @@ -56,6 +56,16 @@
  interrupts = 0 43 0;
  };
 
  +   mfc {
  +   compatible = samsung,s5p-mfc;

 Maybe
 +   compatible = samsung,mfc-v5; ?

  +   reg = 0x1340 0x1;
  +   interrupts = 0 94 0;
  +   samsung,mfc-r = 0x4300;
  +   samsung,mfc

RE: [PATCH] ARM: EXYNOS: Add MFC device tree support

2012-08-23 Thread Kukjin Kim
Arun Kumar K wrote:
 
 Hi Thomas,
 Thank you for the comments.
 Please find my response inline.
 
 --- Original Message ---
 Sender : Thomas Abrahamthomas.abra...@linaro.org
 Date   : Aug 16, 2012 17:12 (GMT+05:30)
 Title  : Re: [PATCH] ARM: EXYNOS: Add MFC device tree support
 
 On 16 August 2012 18:01, Arun Kumar K arun...@samsung.com wrote:
  From: Naveen Krishna Chatradhi ch.nav...@samsung.com
 
  This patch adds device tree entry for MFC in the Exynos
  machines. Exynos4 SoCs support MFC v5 version and Exynos5 has
  MFC v6.x version. So making the required changes in the clock

Since v6.1 is not used anywhere so just MFC v6 should be ok.

  files and adds MFC to the DT device list.
 
  Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
  Signed-off-by: Arun Kumar K arun...@samsung.com
  ---
   .../devicetree/bindings/media/s5p-mfc.txt  |   24
 
   arch/arm/boot/dts/exynos4210.dtsi  |   10 
   arch/arm/boot/dts/exynos5250.dtsi  |   10 
   arch/arm/mach-exynos/clock-exynos5.c   |2 +-
   arch/arm/mach-exynos/mach-exynos4-dt.c |   22
 ++
   arch/arm/mach-exynos/mach-exynos5-dt.c |   22
 ++
   6 files changed, 89 insertions(+), 1 deletions(-)
   create mode 100644 Documentation/devicetree/bindings/media/s5p-mfc.txt
 
  diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt
 b/Documentation/devicetree/bindings/media/s5p-mfc.txt
  new file mode 100644
  index 000..b9bd266
  --- /dev/null
  +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
  @@ -0,0 +1,24 @@
  +* Samsung Multi Format Codec (MFC)
  +
  +Mult Format Codec (MFC) is the IP present in Samsung SoCs which
  +supports high resolution decoding and encoding functionalities.
 
  In addition to this, specifying that mfc is used for video
  encode/decode would be informative.
 
 Ok. I will make it more descriptive.
 
  +
  +Required properties:
  +  - compatible : value should be either one among the following
  +   (a) samsung,s5p-mfc-v5 for MFC v5 present in Exynos4 SoCs
  +   (b) samsung,s5p-mfc-v6 for MFC v6.x present in Exynos5 SoCs
 
  s5p should be dropped from the compatible values. For example, it
  should be samsung,mfc-v5, which is sufficient to identify the
  version of the mfc controller.
 
 Ok will remove s5p.
 
Yeah, I agree, just 'mfc-vX' is enough.

  +
  +  - reg : Physical base address of the IP registers and length of
 memory
  + mapped region.
  +
  +  - interrupts : MFC interupt number to the CPU.
  +
  +  - samsung,mfc-r : Base address of the first memory bank used by MFC
  +   for DMA contiguous memory allocation.
  +
  +  - samsung,mfc-r-size : Size of the first memory bank.
 
  It is not allowed to pass buffer base address and size from device
  tree. Device tree node should describe only the MFC controller
  hardware. Any memory management related information should be handled
  outside of device tree. This helps the bindings to be reusable across
  multiple operating systems.
 
 The mfc-l and mfc-r base address and size is board specific info which has
 to
 be passed to the driver. This is used for DMA contiguous allocation by
 driver and this value
 can change on a different board.
 So in that case, i will pass it as platform data to the driver from mach-
 exynos5-dt.c file.
 I hope that would be ok.
 
I don't think so. The mach-exynos5-dt is for all EXYNOS5 SoCs not
platform_data. As I know, the addresses and sizes for buffer passed via
platform data before, so it can be passed via device tree for board(.dtsi)?
not SoC. In addition, it depends on board.

  +
  +  - samsung,mfc-l : Base address of the second memory bank used by MFC
  +   for DMA contiguous memory allocation.
  +
  +  - samsung,mfc-l-size : Size of the second memory bank.
 
  Same comment as above. And the bindings documentation is usually
  included in the same patch that adds device tree support for the
  driver.
 
 Ok
 
  diff --git a/arch/arm/boot/dts/exynos4210.dtsi
 b/arch/arm/boot/dts/exynos4210.dtsi
  index 02891fe..b5ee43d 100644
  --- a/arch/arm/boot/dts/exynos4210.dtsi
  +++ b/arch/arm/boot/dts/exynos4210.dtsi
  @@ -56,6 +56,16 @@
  interrupts = 0 43 0;
  };
 
  +   mfc {
  +   compatible = samsung,s5p-mfc;

Maybe
+   compatible = samsung,mfc-v5; ?

  +   reg = 0x1340 0x1;
  +   interrupts = 0 94 0;
  +   samsung,mfc-r = 0x4300;
  +   samsung,mfc-r-size = 8388608;
  +   samsung,mfc-l = 0x5100;
  +   samsung,mfc-l-size = 8388608;

As I commented, the size depends on each board not SoC. So should be removed
here.

[...]

Thanks.

Best regards,
Kgene.
--
Kukjin Kim kgene@samsung.com, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

--
To unsubscribe from

Re: [PATCH] ARM: EXYNOS: Add MFC device tree support

2012-08-20 Thread Karol Lewandowski
On 08/16/2012 08:42 PM, Thomas Abraham wrote:
 On 16 August 2012 18:01, Arun Kumar K arun.kk ... @public.gmane.org wrote:

 +  - interrupts : MFC interupt number to the CPU.
 +
 +  - samsung,mfc-r : Base address of the first memory bank used by MFC
 +   for DMA contiguous memory allocation.
 +
 +  - samsung,mfc-r-size : Size of the first memory bank.
 
 It is not allowed to pass buffer base address and size from device
 tree. Device tree node should describe only the MFC controller
 hardware. Any memory management related information should be handled
 outside of device tree. This helps the bindings to be reusable across
 multiple operating systems.

The question is where elsewhere this should be described as this is strictly
board-dependent option (number and size of RAM banks are important here).

I agree that base addresses are bad, but I'm not aware of any functionality
that would allow driver (actually, its platform dependent part in
exynosN_reserve() function) to enumerate available memory banks and grab
memory chunks from two distinct banks.

My (lack of) knowledge ARM might be to blame here but I simply don't know
how to achieve this. Any suggestions?


On 08/16/2012 09:31 PM, Arun Kumar K wrote:

 +static void s5p_mfc_reserve_mem(phys_addr_t rbase, unsigned int rsize,
 + phys_addr_t lbase, unsigned int lsize) {
 +
 + if (memblock_remove(lbase, lsize)) {
 + pr_err(Failed to reserve bank1 memory for MFC device\n);
 + WARN_ON(1);
 + }
 +
 + if (memblock_remove(rbase, rsize)) {
 + pr_err(Failed to reserve bank2 memory for MFC device\n);
 + WARN_ON(1);
 + }
 +}


non-static function with the same name is already defined in
arch/arm/plat-samsung/s5p-dev-mfc.c. Please don't duplicate it,
especially that you seem to be trying to do that twice!


 diff --git a/arch/arm/mach-exynos/mach-exynos5-dt.c 
 b/arch/arm/mach-exynos/mach-exynos5-dt.c

 index ef770bc..898d2de 100644
 --- a/arch/arm/mach-exynos/mach-exynos5-dt.c
 +++ b/arch/arm/mach-exynos/mach-exynos5-dt.c
...
 +static void s5p_mfc_reserve_mem(phys_addr_t rbase, unsigned int rsize,
 + phys_addr_t lbase, unsigned int lsize) {
 +
 + if (memblock_remove(lbase, lsize)) {
 + pr_err(Failed to reserve bank1 memory for MFC device\n);
 + WARN_ON(1);
 + }
 +
 + if (memblock_remove(rbase, rsize)) {
 + pr_err(Failed to reserve bank2 memory for MFC device\n);
 + WARN_ON(1);
 + }
 +}


See above.

 +
 +static void __init exynos5_reserve(void)
 +{
 + s5p_mfc_reserve_mem(0x4300, 8  20, 0x5100, 8  20);


I think it would make sense to make this memory reservation dependent
on mfc* node being present in DTS.  It's to early to use of_* functions
(because tree is not populated at this stage) but fdt_* family of functions
work just fine.

Regards,
-- 
Karol Lewandowski | Samsung Poland RD Center | Linux/Platform
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: EXYNOS: Add MFC device tree support

2012-08-16 Thread Thomas Abraham
On 16 August 2012 18:01, Arun Kumar K arun...@samsung.com wrote:
 From: Naveen Krishna Chatradhi ch.nav...@samsung.com

 This patch adds device tree entry for MFC in the Exynos
 machines. Exynos4 SoCs support MFC v5 version and Exynos5 has
 MFC v6.x version. So making the required changes in the clock
 files and adds MFC to the DT device list.

 Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
 Signed-off-by: Arun Kumar K arun...@samsung.com
 ---
  .../devicetree/bindings/media/s5p-mfc.txt  |   24 
 
  arch/arm/boot/dts/exynos4210.dtsi  |   10 
  arch/arm/boot/dts/exynos5250.dtsi  |   10 
  arch/arm/mach-exynos/clock-exynos5.c   |2 +-
  arch/arm/mach-exynos/mach-exynos4-dt.c |   22 ++
  arch/arm/mach-exynos/mach-exynos5-dt.c |   22 ++
  6 files changed, 89 insertions(+), 1 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/media/s5p-mfc.txt

 diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt 
 b/Documentation/devicetree/bindings/media/s5p-mfc.txt
 new file mode 100644
 index 000..b9bd266
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
 @@ -0,0 +1,24 @@
 +* Samsung Multi Format Codec (MFC)
 +
 +Mult Format Codec (MFC) is the IP present in Samsung SoCs which
 +supports high resolution decoding and encoding functionalities.

In addition to this, specifying that mfc is used for video
encode/decode would be informative.

 +
 +Required properties:
 +  - compatible : value should be either one among the following
 +   (a) samsung,s5p-mfc-v5 for MFC v5 present in Exynos4 SoCs
 +   (b) samsung,s5p-mfc-v6 for MFC v6.x present in Exynos5 SoCs

s5p should be dropped from the compatible values. For example, it
should be samsung,mfc-v5, which is sufficient to identify the
version of the mfc controller.

 +
 +  - reg : Physical base address of the IP registers and length of memory
 + mapped region.
 +
 +  - interrupts : MFC interupt number to the CPU.
 +
 +  - samsung,mfc-r : Base address of the first memory bank used by MFC
 +   for DMA contiguous memory allocation.
 +
 +  - samsung,mfc-r-size : Size of the first memory bank.

It is not allowed to pass buffer base address and size from device
tree. Device tree node should describe only the MFC controller
hardware. Any memory management related information should be handled
outside of device tree. This helps the bindings to be reusable across
multiple operating systems.

 +
 +  - samsung,mfc-l : Base address of the second memory bank used by MFC
 +   for DMA contiguous memory allocation.
 +
 +  - samsung,mfc-l-size : Size of the second memory bank.

Same comment as above. And the bindings documentation is usually
included in the same patch that adds device tree support for the
driver.

 diff --git a/arch/arm/boot/dts/exynos4210.dtsi 
 b/arch/arm/boot/dts/exynos4210.dtsi
 index 02891fe..b5ee43d 100644
 --- a/arch/arm/boot/dts/exynos4210.dtsi
 +++ b/arch/arm/boot/dts/exynos4210.dtsi
 @@ -56,6 +56,16 @@
 interrupts = 0 43 0;
 };

 +   mfc {
 +   compatible = samsung,s5p-mfc;
 +   reg = 0x1340 0x1;
 +   interrupts = 0 94 0;
 +   samsung,mfc-r = 0x4300;
 +   samsung,mfc-r-size = 8388608;
 +   samsung,mfc-l = 0x5100;
 +   samsung,mfc-l-size = 8388608;
 +   };
 +
 rtc@1007 {
 compatible = samsung,s3c6410-rtc;
 reg = 0x1007 0x100;
 diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
 b/arch/arm/boot/dts/exynos5250.dtsi
 index 004aaa8..3c762a4 100644
 --- a/arch/arm/boot/dts/exynos5250.dtsi
 +++ b/arch/arm/boot/dts/exynos5250.dtsi
 @@ -58,6 +58,16 @@
 interrupts = 0 42 0;
 };

 +   mfc {
 +   compatible = samsung,s5p-mfc-v6;
 +   reg = 0x1100 0x1;
 +   interrupts = 0 96 0;
 +   samsung,mfc-r = 0x4300;
 +   samsung,mfc-r-size = 8388608;
 +   samsung,mfc-l = 0x5100;
 +   samsung,mfc-l-size = 8388608;
 +   };
 +
 rtc {
 compatible = samsung,s3c6410-rtc;
 reg = 0x101E 0x100;
 diff --git a/arch/arm/mach-exynos/clock-exynos5.c 
 b/arch/arm/mach-exynos/clock-exynos5.c
 index 3b00e29..c85e7b2 100644
 --- a/arch/arm/mach-exynos/clock-exynos5.c
 +++ b/arch/arm/mach-exynos/clock-exynos5.c
 @@ -607,7 +607,7 @@ static struct clk exynos5_init_clocks_off[] = {
 .ctrlbit= (1  25),
 }, {
 .name   = mfc,
 -   .devname= s5p-mfc,
 +   .devname= s5p-mfc-v6,
 .enable = exynos5_clk_ip_mfc_ctrl,
 .ctrlbit= (1  0),
 }, {
 diff --git 

Re: [PATCH] ARM: EXYNOS: Add MFC device tree support

2012-08-16 Thread Arun Kumar K
Hi Thomas,
Thank you for the comments.
Please find my response inline.

--- Original Message ---
Sender : Thomas Abrahamthomas.abra...@linaro.org 
Date   : Aug 16, 2012 17:12 (GMT+05:30)
Title  : Re: [PATCH] ARM: EXYNOS: Add MFC device tree support

On 16 August 2012 18:01, Arun Kumar K arun...@samsung.com wrote:
 From: Naveen Krishna Chatradhi ch.nav...@samsung.com

 This patch adds device tree entry for MFC in the Exynos
 machines. Exynos4 SoCs support MFC v5 version and Exynos5 has
 MFC v6.x version. So making the required changes in the clock
 files and adds MFC to the DT device list.

 Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
 Signed-off-by: Arun Kumar K arun...@samsung.com
 ---
  .../devicetree/bindings/media/s5p-mfc.txt  |   24 
 
  arch/arm/boot/dts/exynos4210.dtsi  |   10 
  arch/arm/boot/dts/exynos5250.dtsi  |   10 
  arch/arm/mach-exynos/clock-exynos5.c   |2 +-
  arch/arm/mach-exynos/mach-exynos4-dt.c |   22 ++
  arch/arm/mach-exynos/mach-exynos5-dt.c |   22 ++
  6 files changed, 89 insertions(+), 1 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/media/s5p-mfc.txt

 diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt 
 b/Documentation/devicetree/bindings/media/s5p-mfc.txt
 new file mode 100644
 index 000..b9bd266
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
 @@ -0,0 +1,24 @@
 +* Samsung Multi Format Codec (MFC)
 +
 +Mult Format Codec (MFC) is the IP present in Samsung SoCs which
 +supports high resolution decoding and encoding functionalities.

 In addition to this, specifying that mfc is used for video
 encode/decode would be informative.

Ok. I will make it more descriptive.

 +
 +Required properties:
 +  - compatible : value should be either one among the following
 +   (a) samsung,s5p-mfc-v5 for MFC v5 present in Exynos4 SoCs
 +   (b) samsung,s5p-mfc-v6 for MFC v6.x present in Exynos5 SoCs

 s5p should be dropped from the compatible values. For example, it
 should be samsung,mfc-v5, which is sufficient to identify the
 version of the mfc controller.

Ok will remove s5p.

 +
 +  - reg : Physical base address of the IP registers and length of memory
 + mapped region.
 +
 +  - interrupts : MFC interupt number to the CPU.
 +
 +  - samsung,mfc-r : Base address of the first memory bank used by MFC
 +   for DMA contiguous memory allocation.
 +
 +  - samsung,mfc-r-size : Size of the first memory bank.

 It is not allowed to pass buffer base address and size from device
 tree. Device tree node should describe only the MFC controller
 hardware. Any memory management related information should be handled
 outside of device tree. This helps the bindings to be reusable across
 multiple operating systems.

The mfc-l and mfc-r base address and size is board specific info which has to
be passed to the driver. This is used for DMA contiguous allocation by driver 
and this value
can change on a different board. 
So in that case, i will pass it as platform data to the driver from 
mach-exynos5-dt.c file.
I hope that would be ok.

 +
 +  - samsung,mfc-l : Base address of the second memory bank used by MFC
 +   for DMA contiguous memory allocation.
 +
 +  - samsung,mfc-l-size : Size of the second memory bank.

 Same comment as above. And the bindings documentation is usually
 included in the same patch that adds device tree support for the
 driver.

Ok

 diff --git a/arch/arm/boot/dts/exynos4210.dtsi 
 b/arch/arm/boot/dts/exynos4210.dtsi
 index 02891fe..b5ee43d 100644
 --- a/arch/arm/boot/dts/exynos4210.dtsi
 +++ b/arch/arm/boot/dts/exynos4210.dtsi
 @@ -56,6 +56,16 @@
 interrupts = 0 43 0;
 };

 +   mfc {
 +   compatible = samsung,s5p-mfc;
 +   reg = 0x1340 0x1;
 +   interrupts = 0 94 0;
 +   samsung,mfc-r = 0x4300;
 +   samsung,mfc-r-size = 8388608;
 +   samsung,mfc-l = 0x5100;
 +   samsung,mfc-l-size = 8388608;
 +   };
 +
 rtc@1007 {
 compatible = samsung,s3c6410-rtc;
 reg = 0x1007 0x100;
 diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
 b/arch/arm/boot/dts/exynos5250.dtsi
 index 004aaa8..3c762a4 100644
 --- a/arch/arm/boot/dts/exynos5250.dtsi
 +++ b/arch/arm/boot/dts/exynos5250.dtsi
 @@ -58,6 +58,16 @@
 interrupts = 0 42 0;
 };

 +   mfc {
 +   compatible = samsung,s5p-mfc-v6;
 +   reg = 0x1100 0x1;
 +   interrupts = 0 96 0;
 +   samsung,mfc-r = 0x4300;
 +   samsung,mfc-r-size = 8388608;
 +   samsung,mfc-l = 0x5100;
 +   samsung,mfc-l-size = 8388608;
 +   };
 +
 rtc {
 compatible = samsung