On 09/14/2012 05:38 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.
Hi!
Thanks for working on this patch. Please allow me to add few
comments.
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi
> b/arch/arm/boot/dts/exynos5250.dtsi
> index b55794b..5df2f99 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -62,6 +62,12 @@
> interrupts = <0 42 0>;
> };
>
> + mfc {
Nitpick - shouldn't node names be generic? MFC is strictly
samsung specific, something like "codec"/"video-codec" would make
more sense (IMVHO). I would prefer to see address too (e.g.
codec@0x11000000).
However, I do see that rtc below doesn't specify address in node too,
so maybe I'm missing something here.
>
> +struct mfc_dt_meminfo {
> + unsigned long loff;
> + unsigned long lsize;
> + unsigned long roff;
> + unsigned long rsize;
char *compatible;
> +};
> +
> +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 = data;
> +
> + if (!of_flat_dt_is_compatible(node, "samsung,mfc-v6"))
> + return 0;
if (!of_flat_dt_is_compatible(node, mfc_mem->compatible))
return 0;
> +
> + 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;
> +}
Above function could be reused for mfc-v5 (exynos4-dt.c) if compatible
string weren't hardcoded. Thus, please consider changing that and
moving this function to some common(.c?) file - you can see one possible
solution inline.
> +
> +static void __init exynos5_reserve(void)
> +{
> + struct mfc_dt_meminfo mfc_mem;
mfc_mem.compatible = "samsung,mfc-v6";
> +
> + /* Reserve memory for MFC only if it's available */
> + if (of_scan_flat_dt(fdt_find_mfc_mem, &mfc_mem))
> + s5p_mfc_reserve_mem(mfc_mem.roff, mfc_mem.rsize, mfc_mem.loff,
> + mfc_mem.lsize);
> +}
> +
> DT_MACHINE_START(EXYNOS5_DT, "SAMSUNG EXYNOS5 (Flattened Device Tree)")
> /* Maintainer: Kukjin Kim <kgene....@samsung.com> */
> .init_irq = exynos5_init_irq,
> @@ -94,4 +148,5 @@ DT_MACHINE_START(EXYNOS5_DT, "SAMSUNG EXYNOS5 (Flattened
> Device Tree)")
> .timer = &exynos4_timer,
> .dt_compat = exynos5250_dt_compat,
> .restart = exynos5_restart,
> + .reserve = exynos5_reserve,
> MACHINE_END
Regards,
--
Karol Lewandowski | Samsung Poland R&D 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
