Hi, Jonathan >-----Original Message----- >From: Jonathan Corbet [mailto:corbet@xxxxxxx] >Sent: Monday, 17 December, 2012 00:37 >To: Albert Wang >Cc: g.liakhovetski@xxxxxx; linux-media@xxxxxxxxxxxxxxx; Libin Yang >Subject: Re: [PATCH V3 10/15] [media] marvell-ccic: split mcam-core into 2 parts for >soc_camera support > >On Sat, 15 Dec 2012 17:57:59 +0800 >Albert Wang <twang13@xxxxxxxxxxx> wrote: > >> This patch splits mcam-core into 2 parts to prepare for soc_camera support. >> >> The first part remains in mcam-core.c. This part includes the HW operations >> and vb2 callback functions. >> >> The second part is moved to mcam-core-standard.c. This part is relevant with >> the implementation of using V4L2. > >OK, I'll confess I'm still not 100% sold on this part. Can I repeat >the questions I raised before? > > - Is the soc_camera mode necessary? Is there something you're trying > to do that can't be done without it? Or, at least, does it add > sufficient benefit to be worth this work? It would be nice if the > reasoning behind this change were put into the changelog. > [Albert Wang] We just want to add one more option for user. :) And we split it to 2 parts because we want to keep the original mode. > - If the soc_camera change is deemed to be worthwhile, is there > anything preventing you from doing it 100% so it's the only mode > used? > [Albert Wang] No, but current all Marvell platform have used the soc_camera in camera driver. :) So we just hope the marvell-ccic can have this option. :) >The split as you've done it here is an improvement over what came >before, but it still results in a lot of duplicated code; it also adds >a *lot* of symbols to the global namespace. If this is really the only >way then we'll find a way to make it work, but I'd like to be sure that >we can't do something better. > >Thanks, > >jon 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