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

[Sylwester Nawrocki <sylvester.nawrocki@gmail.com>]

Just a few small comments...

On 09/12/2012 03:34 PM, Arun Kumar K wrote:
> This patch adds device tree entry for MFC v6 in the Exynos5
> SoC. Makes the required changes in the clock files and adds
> MFC to the DT device list.
>
> Signed-off-by: Naveen Krishna Chatradhi<ch.naveen@samsung.com>
> Signed-off-by: Arun Kumar K<arun.kk@samsung.com>
> ---
...
> diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> new file mode 100644
> index 0000000..9a74d09
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> @@ -0,0 +1,27 @@
> +* Samsung Multi Format Codec (MFC)
> +
> +Mult Format Codec (MFC) is the IP present in Samsung SoCs which

Mult -> Multi

> +supports high resolution decoding and encoding functionalities.
> +The MFC device driver is a v4l2 driver which can encode/decode
> +video raw/elementary streams and has support for all popular
> +video codecs.
> +
> +Required properties:
> +  - compatible : value should be either one among the following
> +	(a) "samsung,mfc-v5" for MFC v5 present in Exynos4 SoCs
> +	(b) "samsung,mfc-v6" for MFC v6 present in Exynos5 SoCs
> +
> +  - reg : Physical base address of the IP registers and length of memory
> +	  mapped region.
> +
> +  - interrupts : MFC interupt number to the CPU.

interupt -> interrupt

> +
> +  - 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.
> +
> +  - samsung,mfc-l : Base address of the second memory bank used by MFC
> +		    for DMA contiguous memory allocation.
> +
...
> +struct mfc_dt_meminfo {
> +	unsigned long loff;
> +	unsigned long lsize;
> +	unsigned long roff;
> +	unsigned long rsize;
> +};
> +
> +int fdt_find_mfc_mem(unsigned long node, const char *uname, int depth,
> +		void *data)
> +{
> +	__be32 *prop;
> +	unsigned long len;
> +	struct mfc_dt_meminfo *mfc_mem = (struct mfc_dt_meminfo *)data;

No need for explicit cast from void *.

> +	if (of_flat_dt_is_compatible(node, "samsung,mfc-v6")) {

Might be worth to change it to

	if (!of_flat_dt_is_compatible(node, "samsung,mfc-v6"))
		return 0;

and avoid indentation.
> +		prop = of_get_flat_dt_prop(node, "samsung,mfc-l",&len);
> +		if (!prop)
> +			return 0;
> +		mfc_mem->loff = of_read_ulong(prop, len/4);
> +
> +		prop = of_get_flat_dt_prop(node, "samsung,mfc-l-size",&len);
> +		if (!prop)
> +			return 0;
> +		mfc_mem->lsize = of_read_ulong(prop, len/4);
> +
> +		prop = of_get_flat_dt_prop(node, "samsung,mfc-r",&len);
> +		if (!prop)
> +			return 0;
> +		mfc_mem->roff = of_read_ulong(prop, len/4);
> +
> +		prop = of_get_flat_dt_prop(node, "samsung,mfc-r-size",&len);
> +		if (!prop)
> +			return 0;
> +		mfc_mem->rsize = of_read_ulong(prop, len/4);
> +
> +		return 1;
> +	}
> +	return 0;
> +}

--
Regards,
Sylwester