Arun Kumar K wrote:
>
> Hi Thomas,
> Thank you for the comments.
> Please find my response inline.
>
> ------- Original Message -------
> Sender : Thomas Abraham<thomas.abraham@xxxxxxxxxx>
> 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.kk@xxxxxxxxxxx> wrote:
> > From: Naveen Krishna Chatradhi <ch.naveen@xxxxxxxxxxx>
> >
> > 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.naveen@xxxxxxxxxxx>
> > Signed-off-by: Arun Kumar K <arun.kk@xxxxxxxxxxx>
> > ---
> > .../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 0000000..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 = <0x13400000 0x10000>;
> > + interrupts = <0 94 0>;
> > + samsung,mfc-r = <0x43000000>;
> > + samsung,mfc-r-size = <8388608>;
> > + samsung,mfc-l = <0x51000000>;
> > + 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.kim@xxxxxxxxxxx>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
