Hi Shawn,
On Wed, Oct 30, 2019 at 05:05:06PM +0800, Shawnx Tu wrote:
...
> +enum {
> + HI556_LINK_FREQ_874MBPS,
Did you see my comments on v1 on the naming?
...
> +static const char * const hi556_test_pattern_menu[] = {
> + "Disabled",
> + "Solid Colour",
> + "100% Colour Bars",
> + "Fade To Grey’ Colour Bars",
"'" is extra.
...
> +static int hi556_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> + struct hi556 *hi556 = to_hi556(sd);
> +
> + mutex_lock(&hi556->mutex);
> + hi556_update_pad_format(&supported_modes[0],
> + v4l2_subdev_get_try_format(sd, fh->pad, 0));
The function assigns the given driver specific format to the mbus format
struct. How about calling it e.g. hi556_assign_pad_format() instead?
Also see my comments on v1 to this.
--
Regards,
Sakari Ailus
