Hi Sakari, This is the right one and it's OK to swap the lines for local variables, I'll keep this in mind for next changes. Best regards, Hugues. On 03/07/2018 09:13 AM, Sakari Ailus wrote: > Hi Hugues, > > On Tue, Mar 06, 2018 at 06:04:39PM +0100, Hugues Fruchet wrote: >> Fix set of missing colorspace related fields in get_/set_fmt. >> Detected by v4l2-compliance tool. >> >> Signed-off-by: Hugues Fruchet <hugues.fruchet@xxxxxx> > > Could you confirm this is the one you intended to send? There are two > others with similar content. > > ... > >> @@ -2497,16 +2504,22 @@ static int ov5640_probe(struct i2c_client *client, >> struct fwnode_handle *endpoint; >> struct ov5640_dev *sensor; >> int ret; >> + struct v4l2_mbus_framefmt *fmt; > > This one I'd arrange before ret. The local variable declarations should > generally look like a Christmas tree but upside down. > > If you're happy with that, I can swap the two lines as well (no need for > v2). > >> >> sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL); >> if (!sensor) >> return -ENOMEM; >> >> sensor->i2c_client = client; >> - sensor->fmt.code = MEDIA_BUS_FMT_UYVY8_2X8; >> - sensor->fmt.width = 640; >> - sensor->fmt.height = 480; >> - sensor->fmt.field = V4L2_FIELD_NONE; >> + fmt = &sensor->fmt; >> + fmt->code = ov5640_formats[0].code; >> + fmt->colorspace = ov5640_formats[0].colorspace; >> + fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace); >> + fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE; >> + fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace); >> + fmt->width = 640; >> + fmt->height = 480; >> + fmt->field = V4L2_FIELD_NONE; >> sensor->frame_interval.numerator = 1; >> sensor->frame_interval.denominator = ov5640_framerates[OV5640_30_FPS]; >> sensor->current_fr = OV5640_30_FPS; >
