Hi, Guennadi Happy New Year! Thank you for your review! >-----Original Message----- >From: Guennadi Liakhovetski [mailto:g.liakhovetski@xxxxxx] >Sent: Tuesday, 01 January, 2013 23:28 >To: Albert Wang >Cc: corbet@xxxxxxx; linux-media@xxxxxxxxxxxxxxx; Libin Yang >Subject: Re: [PATCH V3 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic >driver > >Hi Albert > >Looks quite good to me, thanks for addressing my comments! Just a minor >nitpick below: > >On Sat, 15 Dec 2012, Albert Wang wrote: > >> From: Libin Yang <lbyang@xxxxxxxxxxx> >> >> This patch adds the MIPI support for marvell-ccic. >> Board driver should determine whether using MIPI or not. >> >> Signed-off-by: Albert Wang <twang13@xxxxxxxxxxx> >> Signed-off-by: Libin Yang <lbyang@xxxxxxxxxxx> >> --- > >A general request for future revisions: it would help if you could list changes >since the last version here - after any Sob's and the "---" line. > >> drivers/media/platform/marvell-ccic/mcam-core.c | 70 ++++++++++++++++++++ >> drivers/media/platform/marvell-ccic/mcam-core.h | 24 ++++++- >> drivers/media/platform/marvell-ccic/mmp-driver.c | 75 +++++++++++++++++++++- >> include/media/mmp-camera.h | 10 +++ >> 4 files changed, 177 insertions(+), 2 deletions(-) > >[snip] > >> diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c >b/drivers/media/platform/marvell-ccic/mmp-driver.c >> index c4c17fe..603fa0a 100755 >> --- a/drivers/media/platform/marvell-ccic/mmp-driver.c >> +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c > >[snip] > >> @@ -183,8 +251,14 @@ static int mmpcam_probe(struct platform_device *pdev) >> mcam = &cam->mcam; >> mcam->plat_power_up = mmpcam_power_up; >> mcam->plat_power_down = mmpcam_power_down; >> + mcam->calc_dphy = mmpcam_calc_dphy; >> + mcam->pll1 = NULL; > >Strictly speaking this is not needed, it's allocated using kzalloc(). Kinda >pointless to use kzalloc() and then explicitly initialise each field, >including 0 / NULL... > [Albert Wang] OK, We will update it. >> mcam->dev = &pdev->dev; >> mcam->use_smbus = 0; >> + mcam->bus_type = pdata->bus_type; >> + mcam->dphy = pdata->dphy; >> + mcam->mipi_enabled = 0; > >ditto > >> + mcam->lane = pdata->lane; >> mcam->chip_id = V4L2_IDENT_ARMADA610; >> mcam->buffer_mode = B_DMA_sg; >> spin_lock_init(&mcam->dev_lock); > >Thanks >Guennadi >--- >Guennadi Liakhovetski, Ph.D. >Freelance Open-Source Software Developer >http://www.open-technology.de/ Thanks Albert Wang 86-21-61092656 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
