On Wed, Oct 02, 2019 at 01:55:59PM +0300, Sakari Ailus wrote:
> Hi Jungo, Hans,
>
> On Mon, May 13, 2019 at 10:46:46AM +0200, Hans Verkuil wrote:
> > On 5/10/19 3:58 AM, Jungo Lin wrote:
> ...
> > > +struct v4l2_ctrl_config mtk_cam_controls[] = {
> > > + {
> > > + .ops = &mtk_cam_dev_ctrl_ops,
> > > + .id = V4L2_CID_PRIVATE_GET_BIN_INFO,
> >
> > Don't use "PRIVATE" in the name. I'd replace that with MTK to indicate
> > that this is mediatek-specific. Same for the next control below.
> >
> > > + .name = "MTK CAM GET BIN INFO",
> > > + .type = V4L2_CTRL_TYPE_INTEGER,
> > > + .min = (IMG_MIN_WIDTH << 16) | IMG_MIN_HEIGHT,
> > > + .max = (IMG_MAX_WIDTH << 16) | IMG_MAX_HEIGHT,
> > > + .step = 1,
> > > + .def = (IMG_MAX_WIDTH << 16) | IMG_MAX_HEIGHT,
> > > + .flags = V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_VOLATILE,
> >
> > Don't mix width and height. I recommend splitting this into two controls.
> >
> > Sakari might have an opinion on this as well.
> >
> > > + },
> > > + {
> > > + .ops = &mtk_cam_dev_ctrl_ops,
> > > + .id = V4L2_CID_PRIVATE_RAW_PATH,
> > > + .name = "MTK CAM RAW PATH",
> > > + .type = V4L2_CTRL_TYPE_BOOLEAN,
> > > + .min = 0,
> > > + .max = 1,
> > > + .step = 1,
> > > + .def = 1,
> > > + },
> >
> > RAW_PATH is a very vague name. If it is 0, then it is pure raw, and if it
> > is 1, then it is 'processing raw'? If so, call it "Processing Raw".
>
> Jungo: what's the purpose of
Please ignore the comment. I'll review a later version separately.
--
Sakari Ailus
sakari.ai...@linux.intel.com
