Re: [PATCH 1/1] ov5648: Don't pack controls struct

[Paul Kocialkowski <paul.kocialkowski@bootlin.com>]

[-- Attachment #1: Type: text/plain, Size: 1492 bytes --]

Hi Sakari,

On Tue 11 Jan 22, 00:48, Sakari Ailus wrote:
> Don't pack the driver specific struct containing control pointers. This
> lead to potential alignment issues when working with the pointers.

Thanks for looking into the report and making this fix.

Honestly I was a bit puzzled because I explicitly added the __packed
to avoid possible holes in the structures that could be problematic
when using v4l2_ctrl_auto_cluster and I think the problem still stands.

I feel like solving both issues at once would require having the controls
that belong in the same cluster declared as an array and not individual
members of the struct.

What do you think?

Cheers,

Paul

> Reported-by: kernel test robot <lkp@intel.com>
> Fixes: e43ccb0a045f ("media: i2c: Add support for the OV5648 image sensor")
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/i2c/ov5648.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/ov5648.c b/drivers/media/i2c/ov5648.c
> index 87f9b724cd7f..3478650ee732 100644
> --- a/drivers/media/i2c/ov5648.c
> +++ b/drivers/media/i2c/ov5648.c
> @@ -639,7 +639,7 @@ struct ov5648_ctrls {
>  	struct v4l2_ctrl *pixel_rate;
>  
>  	struct v4l2_ctrl_handler handler;
> -} __packed;
> +};
>  
>  struct ov5648_sensor {
>  	struct device *dev;
> -- 
> 2.30.2
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]