| Message ID | 1545498774-11754-13-git-send-email-akinobu.mita@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: mt9m001: switch soc_mt9m001 to a standard subdev sensor driver | expand |
Hi Mita-san, On Sun, Dec 23, 2018 at 02:12:54AM +0900, Akinobu Mita wrote: > This driver doesn't set all members of mbus format field when the > VIDIOC_SUBDEV_{S,G}_FMT ioctls are called. > > This is detected by v4l2-compliance. > > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com> > Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > --- > drivers/media/i2c/mt9m001.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c > index f4afbc9..82b89d5 100644 > --- a/drivers/media/i2c/mt9m001.c > +++ b/drivers/media/i2c/mt9m001.c > @@ -342,6 +342,9 @@ static int mt9m001_get_fmt(struct v4l2_subdev *sd, > mf->code = mt9m001->fmt->code; > mf->colorspace = mt9m001->fmt->colorspace; > mf->field = V4L2_FIELD_NONE; > + mf->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > + mf->quantization = V4L2_QUANTIZATION_DEFAULT; > + mf->xfer_func = V4L2_XFER_FUNC_DEFAULT; Instead of setting the fields individually, would it be feasible to just assign mt9m001->fmt to mf? > > return 0; > } > @@ -402,6 +405,10 @@ static int mt9m001_set_fmt(struct v4l2_subdev *sd, > } > > mf->colorspace = fmt->colorspace; > + mf->field = V4L2_FIELD_NONE; > + mf->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > + mf->quantization = V4L2_QUANTIZATION_DEFAULT; > + mf->xfer_func = V4L2_XFER_FUNC_DEFAULT; Ditto. > > if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) > return mt9m001_s_fmt(sd, fmt, mf);
2019年1月7日(月) 20:32 Sakari Ailus <sakari.ailus@linux.intel.com>: > > Hi Mita-san, > > On Sun, Dec 23, 2018 at 02:12:54AM +0900, Akinobu Mita wrote: > > This driver doesn't set all members of mbus format field when the > > VIDIOC_SUBDEV_{S,G}_FMT ioctls are called. > > > > This is detected by v4l2-compliance. > > > > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > > Cc: Sakari Ailus <sakari.ailus@linux.intel.com> > > Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com> > > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > > --- > > drivers/media/i2c/mt9m001.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c > > index f4afbc9..82b89d5 100644 > > --- a/drivers/media/i2c/mt9m001.c > > +++ b/drivers/media/i2c/mt9m001.c > > @@ -342,6 +342,9 @@ static int mt9m001_get_fmt(struct v4l2_subdev *sd, > > mf->code = mt9m001->fmt->code; > > mf->colorspace = mt9m001->fmt->colorspace; > > mf->field = V4L2_FIELD_NONE; > > + mf->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > > + mf->quantization = V4L2_QUANTIZATION_DEFAULT; > > + mf->xfer_func = V4L2_XFER_FUNC_DEFAULT; > > Instead of setting the fields individually, would it be feasible to just > assign mt9m001->fmt to mf? The data type of mt9m001->fmt is struct mt9m001_datafmt, so it can't simply assign to mf.
On Mon, Jan 07, 2019 at 11:18:47PM +0900, Akinobu Mita wrote: > 2019年1月7日(月) 20:32 Sakari Ailus <sakari.ailus@linux.intel.com>: > > > > Hi Mita-san, > > > > On Sun, Dec 23, 2018 at 02:12:54AM +0900, Akinobu Mita wrote: > > > This driver doesn't set all members of mbus format field when the > > > VIDIOC_SUBDEV_{S,G}_FMT ioctls are called. > > > > > > This is detected by v4l2-compliance. > > > > > > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > > > Cc: Sakari Ailus <sakari.ailus@linux.intel.com> > > > Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com> > > > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > > > --- > > > drivers/media/i2c/mt9m001.c | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c > > > index f4afbc9..82b89d5 100644 > > > --- a/drivers/media/i2c/mt9m001.c > > > +++ b/drivers/media/i2c/mt9m001.c > > > @@ -342,6 +342,9 @@ static int mt9m001_get_fmt(struct v4l2_subdev *sd, > > > mf->code = mt9m001->fmt->code; > > > mf->colorspace = mt9m001->fmt->colorspace; > > > mf->field = V4L2_FIELD_NONE; > > > + mf->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > > > + mf->quantization = V4L2_QUANTIZATION_DEFAULT; > > > + mf->xfer_func = V4L2_XFER_FUNC_DEFAULT; > > > > Instead of setting the fields individually, would it be feasible to just > > assign mt9m001->fmt to mf? > > The data type of mt9m001->fmt is struct mt9m001_datafmt, so it can't > simply assign to mf. Oh, good point. There are still three places in the driver where you're setting effectively the same fields; if there is a way to reasonably address that, it'd be nice to do that. Up to you.
diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c index f4afbc9..82b89d5 100644 --- a/drivers/media/i2c/mt9m001.c +++ b/drivers/media/i2c/mt9m001.c @@ -342,6 +342,9 @@ static int mt9m001_get_fmt(struct v4l2_subdev *sd, mf->code = mt9m001->fmt->code; mf->colorspace = mt9m001->fmt->colorspace; mf->field = V4L2_FIELD_NONE; + mf->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; + mf->quantization = V4L2_QUANTIZATION_DEFAULT; + mf->xfer_func = V4L2_XFER_FUNC_DEFAULT; return 0; } @@ -402,6 +405,10 @@ static int mt9m001_set_fmt(struct v4l2_subdev *sd, } mf->colorspace = fmt->colorspace; + mf->field = V4L2_FIELD_NONE; + mf->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; + mf->quantization = V4L2_QUANTIZATION_DEFAULT; + mf->xfer_func = V4L2_XFER_FUNC_DEFAULT; if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) return mt9m001_s_fmt(sd, fmt, mf);
This driver doesn't set all members of mbus format field when the VIDIOC_SUBDEV_{S,G}_FMT ioctls are called. This is detected by v4l2-compliance. Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de> Cc: Sakari Ailus <sakari.ailus@linux.intel.com> Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> --- drivers/media/i2c/mt9m001.c | 7 +++++++ 1 file changed, 7 insertions(+)