[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAHp75Vefv2PCtd-xOw+7906g7MxXVzSE7cOm-xJ_fxe_arCx5w@mail.gmail.com>
Date: Fri, 16 Jan 2026 10:10:23 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Hamdan Khan <hamdankhan212@...il.com>
Cc: gregkh@...uxfoundation.org, andy@...nel.org, hansg@...nel.org,
mchehab@...nel.org, sakari.ailus@...ux.intel.com, dave.hansen@...ux.intel.com,
tony.luck@...el.com, linux-media@...r.kernel.org,
linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] staging: media: atomisp: Fix typos and formatting in headers
On Fri, Jan 16, 2026 at 9:21 AM Hamdan Khan <hamdankhan212@...il.com> wrote:
>
> This patch updates block and inline comments to follow kernel
s/This patch updates/Update/
The recommendation to use imperative mode is documented in the
Submitting Patches.
> commenting conventions, fixes typos and wording, and reformats
> long comments for clarity and line length consistency.
>
> Although some comments used the /** ... */ style, they are not
> kernel-doc comments and are converted to normal comment style.
>
> No functional changes are intended.
...
> struct atomisp_3a_statistics {
> struct atomisp_3a_output __user *data;
> struct atomisp_3a_rgby_output __user *rgby_data;
> u32 exp_id; /* exposure ID */
> - u32 isp_config_id; /* isp config ID */
> + u32 isp_config_id; /* ISP config ID */
Useless comment (no added value), just drop it completely.
> };
...
> struct atomisp_resolution {
> - u32 width; /** Width */
> - u32 height; /** Height */
> + u32 width; /* Width */
> + u32 height; /* Height */
> };
Ditto.
...
> struct atomisp_zoom_point {
> - s32 x; /** x coordinate */
> - s32 y; /** y coordinate */
> + s32 x; /* x coordinate */
> + s32 y; /* y coordinate */
> };
Ditto.
Also double check the rest. The rule of thumb is if the comment just
describes what's obvious from the variable name (without knowing the
driver code and design), drop the useless comment.
...
> /* GDC shit size [BQ] */
Probably we need to fix this "shit"...
> struct dvs2_bq_resolution gdc_shift_bq;
...
> + /*
> + * The overlay start x pixel position on output frame. It should be a
"start x"
on the output
> + * multiple of 2 * ISP_VEC_NELEMS.
> + */
> unsigned int overlay_start_x;
...
> + /*
> + * The overlay start y pixel position on output frame. It should be a
> + * multiple of 2.
> + */
> unsigned int overlay_start_y;
As per above.
...
> +/* MACC parameter control*/
Fix the spacing in all comments you touched.
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists