Hi Kyungmin Park,
Thank you for the review.
Please find my comments inline.
On Fri, Jul 6, 2012 at 7:51 PM, Kyungmin Park <kmpark@xxxxxxxxxxxxx> wrote:
> Hi,
>
> On Fri, Jul 6, 2012 at 11:00 PM, Arun Kumar K <arun.kk@xxxxxxxxxxx> wrote:
>> From: Jeongtae Park <jtp.park@xxxxxxxxxxx>
>>
>> Multi Format Codec 6.x is a hardware video coding acceleration
>> module fount in new Exynos5 SoC series.
>> It is capable of handling a range of video codecs and this driver
>> provides a V4L2 interface for video decoding and encoding.
>>
>> Signed-off-by: Jeongtae Park <jtp.park@xxxxxxxxxxx>
>> Singed-off-by: Janghyuck Kim <janghyuck.kim@xxxxxxxxxxx>
>> Singed-off-by: Jaeryul Oh <jaeryul.oh@xxxxxxxxxxx>
>> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@xxxxxxxxxxx>
>> Signed-off-by: Arun Kumar K <arun.kk@xxxxxxxxxxx>
>> Cc: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
>> Cc: Kamil Debski <k.debski@xxxxxxxxxxx>
>> ---
>> drivers/media/video/Kconfig | 16 +-
>> drivers/media/video/s5p-mfc/Makefile | 7 +-
>> drivers/media/video/s5p-mfc/regs-mfc-v6.h | 676 ++++++++++
>> drivers/media/video/s5p-mfc/regs-mfc.h | 29 +
>> drivers/media/video/s5p-mfc/s5p_mfc.c | 163 ++-
>> drivers/media/video/s5p-mfc/s5p_mfc_cmd.c | 6 +-
>> drivers/media/video/s5p-mfc/s5p_mfc_cmd.h | 3 +
>> drivers/media/video/s5p-mfc/s5p_mfc_cmd_v6.c | 96 ++
>> drivers/media/video/s5p-mfc/s5p_mfc_common.h | 123 ++-
>> drivers/media/video/s5p-mfc/s5p_mfc_ctrl.c | 160 ++-
>> drivers/media/video/s5p-mfc/s5p_mfc_ctrl.h | 1 +
>> drivers/media/video/s5p-mfc/s5p_mfc_dec.c | 210 +++-
>> drivers/media/video/s5p-mfc/s5p_mfc_dec.h | 1 +
>> drivers/media/video/s5p-mfc/s5p_mfc_enc.c | 191 ++--
>> drivers/media/video/s5p-mfc/s5p_mfc_enc.h | 1 +
>> drivers/media/video/s5p-mfc/s5p_mfc_intr.c | 1 -
>> drivers/media/video/s5p-mfc/s5p_mfc_opr.c | 278 +++--
>> drivers/media/video/s5p-mfc/s5p_mfc_opr.h | 25 +-
>> drivers/media/video/s5p-mfc/s5p_mfc_opr_v6.c | 1697 ++++++++++++++++++++++++++
>> drivers/media/video/s5p-mfc/s5p_mfc_opr_v6.h | 140 +++
>> drivers/media/video/s5p-mfc/s5p_mfc_pm.c | 6 +-
>> drivers/media/video/s5p-mfc/s5p_mfc_shm.c | 28 +-
>> drivers/media/video/s5p-mfc/s5p_mfc_shm.h | 13 +-
>> drivers/media/video/v4l2-ctrls.c | 1 -
>> 24 files changed, 3476 insertions(+), 396 deletions(-)
>
> Doesn't it too big for one patch? Can you split it into several patches?
>
Ok. I will split it in the next patch.
>> create mode 100644 drivers/media/video/s5p-mfc/regs-mfc-v6.h
>> create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_cmd_v6.c
>> create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_opr_v6.c
>> create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_opr_v6.h
>>
>> diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
>> index 99937c9..0d7fe77 100644
>> --- a/drivers/media/video/Kconfig
>> +++ b/drivers/media/video/Kconfig
>> @@ -1198,13 +1198,27 @@ config VIDEO_SAMSUNG_S5P_JPEG
>> This is a v4l2 driver for Samsung S5P and EXYNOS4 JPEG codec
>>
>> config VIDEO_SAMSUNG_S5P_MFC
>> + bool
>> +
>> +config VIDEO_SAMSUNG_S5P_MFC_V5
>> tristate "Samsung S5P MFC 5.1 Video Codec"
>> - depends on VIDEO_DEV && VIDEO_V4L2 && PLAT_S5P
>> + depends on VIDEO_DEV && VIDEO_V4L2 && ARCH_EXYNOS4
>> + select VIDEO_SAMSUNG_S5P_MFC
>> select VIDEOBUF2_DMA_CONTIG
>> default n
>> help
>> MFC 5.1 driver for V4L2.
>>
>> +config VIDEO_SAMSUNG_S5P_MFC_V6
>
> Yes, I know it's exynos5 series features. however, it's not good idea
> to add new config.
> It already handled platform device with proper name.
> e.g., s5p-mfc-v5, s5p-mfc-v6 and handle it with platform data.
>
Ok. Code changes are required for compiling both v5 and v6 together.
Will incorporate in next patch.
[snip]
>> -#define MFC_CLKNAME "sclk_mfc"
>> +#if defined(CONFIG_VIDEO_SAMSUNG_S5P_MFC_V5)
>> +#define MFC_CLKNAME "sclk_mfc"
>> +#elif defined(CONFIG_VIDEO_SAMSUNG_S5P_MFC_V6)
>> +#define MFC_CLKNAME "aclk_333"
> I think it can handle clkname without new config.
>
Yes I will change it.
Regards
Arunÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ¥Šwÿº{.nÇ+‰·¥Š{±þg?‰¯âžØ^n‡r¡ö¦zË?ëh™¨èÚ&£ûàz¿äz¹Þ—ú+€Ê+zf£¢·hšˆ§~††Ûiÿÿï?êÿ‘êçz_è®æj:+v‰¨þ)ߣøm
