[PATCH 0/5] Fix most ImgU driver compiler / checker warnings

[PATCH 0/5] Fix most ImgU driver compiler / checker warnings

[Sakari Ailus]

Hi Mauro, Hans,

This set addresses most of the compiler / checker warnings from the ImgU
driver. There are a few about stack usage from the compiler left but those
were not trivial to address so I postponed that. There are also a few
about memset but Hans suggested addressing that by increasing the maximum
size. I guess it wouldn't benefit anyone to split those memsets into
two...

Sakari Ailus (5):
  staging: imgu: Switch to __aligned() from __attribute__((aligned()))
  staging: imgu: Fix struct ipu3_uapi_awb_fr_config_s alignment
  staging: imgu: Remove redundant checks
  staging: imgu: Address compiler / checker warnings in MMU code
  Revert "media: ipu3: shut up warnings produced with W=1"

 drivers/staging/media/ipu3/Makefile             |  6 --
 drivers/staging/media/ipu3/TODO                 |  2 -
 drivers/staging/media/ipu3/include/intel-ipu3.h | 74 ++++++++++++-------------
 drivers/staging/media/ipu3/ipu3-css-fw.c        |  6 +-
 drivers/staging/media/ipu3/ipu3-mmu.c           | 39 +++++++++++--
 5 files changed, 74 insertions(+), 53 deletions(-)

-- 
2.11.0

[PATCH 1/5] staging: imgu: Switch to __aligned() from __attribute__((aligned()))

[Sakari Ailus]

__aligned() is preferred. The patch has been generated using the following
command in the drivers/staging/media/ipu3 directory:

$ git grep -l 'aligned(32)' | \
	xargs perl -i -pe \
	's/__attribute__\s*\(\(\s*aligned\s*\(([0-9]+)\s*\)\s*\)\)/__aligned($1)/g;'

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/staging/media/ipu3/include/intel-ipu3.h | 74 ++++++++++++-------------
 1 file changed, 37 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/media/ipu3/include/intel-ipu3.h b/drivers/staging/media/ipu3/include/intel-ipu3.h
index eb6f52aca9929..cbb62643172be 100644
--- a/drivers/staging/media/ipu3/include/intel-ipu3.h
+++ b/drivers/staging/media/ipu3/include/intel-ipu3.h
@@ -83,7 +83,7 @@ struct ipu3_uapi_grid_config {
  */
 struct ipu3_uapi_awb_raw_buffer {
 	__u8 meta_data[IPU3_UAPI_AWB_MAX_BUFFER_SIZE]
-		__attribute__((aligned(32)));
+		__aligned(32);
 } __packed;
 
 /**
@@ -104,7 +104,7 @@ struct ipu3_uapi_awb_config_s {
 	__u16 rgbs_thr_gb;
 	__u16 rgbs_thr_b;
 	struct ipu3_uapi_grid_config grid;
-} __attribute__((aligned(32))) __packed;
+} __aligned(32) __packed;
 
 /**
  * struct ipu3_uapi_awb_config - AWB config wrapper
@@ -112,7 +112,7 @@ struct ipu3_uapi_awb_config_s {
  * @config: config for auto white balance as defined by &ipu3_uapi_awb_config_s
  */
 struct ipu3_uapi_awb_config {
-	struct ipu3_uapi_awb_config_s config __attribute__((aligned(32)));
+	struct ipu3_uapi_awb_config_s config __aligned(32);
 } __packed;
 
 #define IPU3_UAPI_AE_COLORS				4	/* R, G, B, Y */
@@ -137,7 +137,7 @@ struct ipu3_uapi_ae_raw_buffer {
  * @buff: &ipu3_uapi_ae_raw_buffer to hold full frame meta data.
  */
 struct ipu3_uapi_ae_raw_buffer_aligned {
-	struct ipu3_uapi_ae_raw_buffer buff __attribute__((aligned(32)));
+	struct ipu3_uapi_ae_raw_buffer buff __aligned(32);
 } __packed;
 
 /**
@@ -243,10 +243,10 @@ struct ipu3_uapi_ae_ccm {
  * Calculate AE grid from image resolution, resample ae weights.
  */
 struct ipu3_uapi_ae_config {
-	struct ipu3_uapi_ae_grid_config grid_cfg __attribute__((aligned(32)));
+	struct ipu3_uapi_ae_grid_config grid_cfg __aligned(32);
 	struct ipu3_uapi_ae_weight_elem weights[
-			IPU3_UAPI_AE_WEIGHTS] __attribute__((aligned(32)));
-	struct ipu3_uapi_ae_ccm ae_ccm __attribute__((aligned(32)));
+			IPU3_UAPI_AE_WEIGHTS] __aligned(32);
+	struct ipu3_uapi_ae_ccm ae_ccm __aligned(32);
 } __packed;
 
 /**
@@ -388,7 +388,7 @@ struct ipu3_uapi_af_filter_config {
  *		each cell.
  */
 struct ipu3_uapi_af_raw_buffer {
-	__u8 y_table[IPU3_UAPI_AF_Y_TABLE_MAX_SIZE] __attribute__((aligned(32)));
+	__u8 y_table[IPU3_UAPI_AF_Y_TABLE_MAX_SIZE] __aligned(32);
 } __packed;
 
 /**
@@ -401,9 +401,9 @@ struct ipu3_uapi_af_raw_buffer {
  *	      grid size for large image and vice versa.
  */
 struct ipu3_uapi_af_config_s {
-	struct ipu3_uapi_af_filter_config filter_config __attribute__((aligned(32)));
+	struct ipu3_uapi_af_filter_config filter_config __aligned(32);
 	__u8 padding[4];
-	struct ipu3_uapi_grid_config grid_cfg __attribute__((aligned(32)));
+	struct ipu3_uapi_grid_config grid_cfg __aligned(32);
 } __packed;
 
 #define IPU3_UAPI_AWB_FR_MAX_SETS			24
@@ -424,7 +424,7 @@ struct ipu3_uapi_af_config_s {
  */
 struct ipu3_uapi_awb_fr_raw_buffer {
 	__u8 meta_data[IPU3_UAPI_AWB_FR_BAYER_TABLE_MAX_SIZE]
-		__attribute__((aligned(32)));
+		__aligned(32);
 } __packed;
 
 /**
@@ -450,7 +450,7 @@ struct ipu3_uapi_awb_fr_config_s {
 	__u32 bayer_sign;
 	__u8 bayer_nf;
 	__u8 reserved2[3];
-} __attribute__((aligned(32))) __packed;
+} __aligned(32) __packed;
 
 /**
  * struct ipu3_uapi_4a_config - 4A config
@@ -462,7 +462,7 @@ struct ipu3_uapi_awb_fr_config_s {
  * @awb_fr_config: &ipu3_uapi_awb_fr_config_s, default resolution 16x16
  */
 struct ipu3_uapi_4a_config {
-	struct ipu3_uapi_awb_config_s awb_config __attribute__((aligned(32)));
+	struct ipu3_uapi_awb_config_s awb_config __aligned(32);
 	struct ipu3_uapi_ae_grid_config ae_grd_config;
 	__u8 padding[20];
 	struct ipu3_uapi_af_config_s af_config;
@@ -485,7 +485,7 @@ struct ipu3_uapi_4a_config {
  * @padding3: padding bytes.
  */
 struct ipu3_uapi_bubble_info {
-	__u32 num_of_stripes __attribute__((aligned(32)));
+	__u32 num_of_stripes __aligned(32);
 	__u8 padding[28];
 	__u32 num_sets;
 	__u8 padding1[28];
@@ -517,7 +517,7 @@ struct ipu3_uapi_stats_3a_bubble_info_per_stripe {
  * @padding3: padding config
  */
 struct ipu3_uapi_ff_status {
-	__u32 awb_en __attribute__((aligned(32)));
+	__u32 awb_en __aligned(32);
 	__u8 padding[28];
 	__u32 ae_en;
 	__u8 padding1[28];
@@ -990,8 +990,8 @@ struct ipu3_uapi_gamma_corr_lut {
  * @gc_lut: lookup table of gamma correction &ipu3_uapi_gamma_corr_lut
  */
 struct ipu3_uapi_gamma_config {
-	struct ipu3_uapi_gamma_corr_ctrl gc_ctrl __attribute__((aligned(32)));
-	struct ipu3_uapi_gamma_corr_lut gc_lut __attribute__((aligned(32)));
+	struct ipu3_uapi_gamma_corr_ctrl gc_ctrl __aligned(32);
+	struct ipu3_uapi_gamma_corr_lut gc_lut __aligned(32);
 } __packed;
 
 /**
@@ -1194,8 +1194,8 @@ struct ipu3_uapi_shd_lut {
  * @shd_lut:	shading lookup table &ipu3_uapi_shd_lut
  */
 struct ipu3_uapi_shd_config {
-	struct ipu3_uapi_shd_config_static shd __attribute__((aligned(32)));
-	struct ipu3_uapi_shd_lut shd_lut __attribute__((aligned(32)));
+	struct ipu3_uapi_shd_config_static shd __aligned(32);
+	struct ipu3_uapi_shd_lut shd_lut __aligned(32);
 } __packed;
 
 /* Image Enhancement Filter directed */
@@ -2414,8 +2414,8 @@ struct ipu3_uapi_anr_stitch_config {
  * @stitch: create 4x4 patch from 4 surrounding 8x8 patches.
  */
 struct ipu3_uapi_anr_config {
-	struct ipu3_uapi_anr_transform_config transform __attribute__((aligned(32)));
-	struct ipu3_uapi_anr_stitch_config stitch __attribute__((aligned(32)));
+	struct ipu3_uapi_anr_transform_config transform __aligned(32);
+	struct ipu3_uapi_anr_stitch_config stitch __aligned(32);
 } __packed;
 
 /**
@@ -2456,21 +2456,21 @@ struct ipu3_uapi_anr_config {
 struct ipu3_uapi_acc_param {
 	struct ipu3_uapi_bnr_static_config bnr;
 	struct ipu3_uapi_bnr_static_config_green_disparity
-				green_disparity __attribute__((aligned(32)));
-	struct ipu3_uapi_dm_config dm __attribute__((aligned(32)));
-	struct ipu3_uapi_ccm_mat_config ccm __attribute__((aligned(32)));
-	struct ipu3_uapi_gamma_config gamma __attribute__((aligned(32)));
-	struct ipu3_uapi_csc_mat_config csc __attribute__((aligned(32)));
-	struct ipu3_uapi_cds_params cds __attribute__((aligned(32)));
-	struct ipu3_uapi_shd_config shd __attribute__((aligned(32)));
-	struct ipu3_uapi_yuvp1_iefd_config iefd __attribute__((aligned(32)));
-	struct ipu3_uapi_yuvp1_yds_config yds_c0 __attribute__((aligned(32)));
-	struct ipu3_uapi_yuvp1_chnr_config chnr_c0 __attribute__((aligned(32)));
-	struct ipu3_uapi_yuvp1_y_ee_nr_config y_ee_nr __attribute__((aligned(32)));
-	struct ipu3_uapi_yuvp1_yds_config yds __attribute__((aligned(32)));
-	struct ipu3_uapi_yuvp1_chnr_config chnr __attribute__((aligned(32)));
-	struct ipu3_uapi_yuvp1_yds_config yds2 __attribute__((aligned(32)));
-	struct ipu3_uapi_yuvp2_tcc_static_config tcc __attribute__((aligned(32)));
+				green_disparity __aligned(32);
+	struct ipu3_uapi_dm_config dm __aligned(32);
+	struct ipu3_uapi_ccm_mat_config ccm __aligned(32);
+	struct ipu3_uapi_gamma_config gamma __aligned(32);
+	struct ipu3_uapi_csc_mat_config csc __aligned(32);
+	struct ipu3_uapi_cds_params cds __aligned(32);
+	struct ipu3_uapi_shd_config shd __aligned(32);
+	struct ipu3_uapi_yuvp1_iefd_config iefd __aligned(32);
+	struct ipu3_uapi_yuvp1_yds_config yds_c0 __aligned(32);
+	struct ipu3_uapi_yuvp1_chnr_config chnr_c0 __aligned(32);
+	struct ipu3_uapi_yuvp1_y_ee_nr_config y_ee_nr __aligned(32);
+	struct ipu3_uapi_yuvp1_yds_config yds __aligned(32);
+	struct ipu3_uapi_yuvp1_chnr_config chnr __aligned(32);
+	struct ipu3_uapi_yuvp1_yds_config yds2 __aligned(32);
+	struct ipu3_uapi_yuvp2_tcc_static_config tcc __aligned(32);
 	struct ipu3_uapi_anr_config anr;
 	struct ipu3_uapi_awb_fr_config_s awb_fr;
 	struct ipu3_uapi_ae_config ae;
@@ -2758,7 +2758,7 @@ struct ipu3_uapi_flags {
  */
 struct ipu3_uapi_params {
 	/* Flags which of the settings below are to be applied */
-	struct ipu3_uapi_flags use __attribute__((aligned(32)));
+	struct ipu3_uapi_flags use __aligned(32);
 
 	/* Accelerator cluster parameters */
 	struct ipu3_uapi_acc_param acc_param;
-- 
2.11.0

[PATCH 2/5] staging: imgu: Fix struct ipu3_uapi_awb_fr_config_s alignment

[Sakari Ailus]

struct ipu3_uapi_awb_fr_config_s is labelled as to be aligned to 32 bytes
but that's not the case in the ISP parameter struct of the driver, struct
ipu3_uapi_acc_param which is packed.

Also there is no need for the alignment as the struct is only handled by the
driver. Remove the alignment from the struct.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/staging/media/ipu3/include/intel-ipu3.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/ipu3/include/intel-ipu3.h b/drivers/staging/media/ipu3/include/intel-ipu3.h
index cbb62643172be..4cdb4c791ecec 100644
--- a/drivers/staging/media/ipu3/include/intel-ipu3.h
+++ b/drivers/staging/media/ipu3/include/intel-ipu3.h
@@ -450,7 +450,7 @@ struct ipu3_uapi_awb_fr_config_s {
 	__u32 bayer_sign;
 	__u8 bayer_nf;
 	__u8 reserved2[3];
-} __aligned(32) __packed;
+} __packed;
 
 /**
  * struct ipu3_uapi_4a_config - 4A config
-- 
2.11.0

[PATCH 3/5] staging: imgu: Remove redundant checks

[Sakari Ailus]

Remove redundant checks for less than zero on unsigned variables.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/staging/media/ipu3/ipu3-css-fw.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/ipu3/ipu3-css-fw.c b/drivers/staging/media/ipu3/ipu3-css-fw.c
index 4122d4e42db6c..45aff76198e2c 100644
--- a/drivers/staging/media/ipu3/ipu3-css-fw.c
+++ b/drivers/staging/media/ipu3/ipu3-css-fw.c
@@ -200,13 +200,11 @@ int imgu_css_fw_init(struct imgu_css *css)
 			goto bad_fw;
 
 		for (j = 0; j < bi->info.isp.num_output_formats; j++)
-			if (bi->info.isp.output_formats[j] < 0 ||
-			    bi->info.isp.output_formats[j] >=
+			if (bi->info.isp.output_formats[j] >=
 			    IMGU_ABI_FRAME_FORMAT_NUM)
 				goto bad_fw;
 		for (j = 0; j < bi->info.isp.num_vf_formats; j++)
-			if (bi->info.isp.vf_formats[j] < 0 ||
-			    bi->info.isp.vf_formats[j] >=
+			if (bi->info.isp.vf_formats[j] >=
 			    IMGU_ABI_FRAME_FORMAT_NUM)
 				goto bad_fw;
 
-- 
2.11.0

[PATCH 4/5] staging: imgu: Address compiler / checker warnings in MMU code

[Sakari Ailus]

Address C compiler, sparse and smatch warnings and little style issues in
the IMGU MMU code.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/staging/media/ipu3/ipu3-mmu.c | 39 +++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/ipu3/ipu3-mmu.c b/drivers/staging/media/ipu3/ipu3-mmu.c
index cfc2bdfb14b33..a55ff39be1882 100644
--- a/drivers/staging/media/ipu3/ipu3-mmu.c
+++ b/drivers/staging/media/ipu3/ipu3-mmu.c
@@ -275,7 +275,17 @@ static size_t imgu_mmu_pgsize(unsigned long pgsize_bitmap,
 	return pgsize;
 }
 
-/* drivers/iommu/iommu.c/iommu_map() */
+/**
+ * imgu_mmu_map - map a buffer to a physical address
+ *
+ * @info: MMU mappable range
+ * @iova: the virtual address
+ * @paddr: the physical address
+ * @size: length of the mappable area
+ *
+ * The function has been adapted from iommu_map() in
+ * drivers/iommu/iommu.c .
+ */
 int imgu_mmu_map(struct imgu_mmu_info *info, unsigned long iova,
 		 phys_addr_t paddr, size_t size)
 {
@@ -321,7 +331,17 @@ int imgu_mmu_map(struct imgu_mmu_info *info, unsigned long iova,
 	return ret;
 }
 
-/* drivers/iommu/iommu.c/default_iommu_map_sg() */
+/**
+ * imgu_mmu_map_sg - Map a scatterlist
+ *
+ * @info: MMU mappable range
+ * @iova: the virtual address
+ * @sg: the scatterlist to map
+ * @nents: number of entries in the scatterlist
+ *
+ * The function has been adapted from default_iommu_map_sg() in
+ * drivers/iommu/iommu.c .
+ */
 size_t imgu_mmu_map_sg(struct imgu_mmu_info *info, unsigned long iova,
 		       struct scatterlist *sg, unsigned int nents)
 {
@@ -394,7 +414,16 @@ static size_t __imgu_mmu_unmap(struct imgu_mmu *mmu,
 	return unmap;
 }
 
-/* drivers/iommu/iommu.c/iommu_unmap() */
+/**
+ * imgu_mmu_unmap - Unmap a buffer
+ *
+ * @info: MMU mappable range
+ * @iova: the virtual address
+ * @size: the length of the buffer
+ *
+ * The function has been adapted from iommu_unmap() in
+ * drivers/iommu/iommu.c .
+ */
 size_t imgu_mmu_unmap(struct imgu_mmu_info *info, unsigned long iova,
 		      size_t size)
 {
@@ -444,6 +473,7 @@ size_t imgu_mmu_unmap(struct imgu_mmu_info *info, unsigned long iova,
 
 /**
  * imgu_mmu_init() - initialize IPU3 MMU block
+ *
  * @parent:	struct device parent
  * @base:	IOMEM base of hardware registers.
  *
@@ -523,7 +553,8 @@ struct imgu_mmu_info *imgu_mmu_init(struct device *parent, void __iomem *base)
 
 /**
  * imgu_mmu_exit() - clean up IPU3 MMU block
- * @info: IPU3 MMU private data
+ *
+ * @info: MMU mappable range
  */
 void imgu_mmu_exit(struct imgu_mmu_info *info)
 {
-- 
2.11.0

[PATCH 5/5] Revert "media: ipu3: shut up warnings produced with W=1"

[Sakari Ailus]

This reverts commit 0bdfc56c13c0ffe003f28395fcde2cd9b5ea0622.

The original patch suppressed compiler and checker warnings and those
warnings have been addressed. Do not attempt to suppress these warnings
anymore.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/staging/media/ipu3/Makefile | 6 ------
 drivers/staging/media/ipu3/TODO     | 2 --
 2 files changed, 8 deletions(-)

diff --git a/drivers/staging/media/ipu3/Makefile b/drivers/staging/media/ipu3/Makefile
index fa7fa3372bcbe..fb146d178bd4b 100644
--- a/drivers/staging/media/ipu3/Makefile
+++ b/drivers/staging/media/ipu3/Makefile
@@ -9,9 +9,3 @@ ipu3-imgu-objs += \
 		ipu3-css.o ipu3-v4l2.o ipu3.o
 
 obj-$(CONFIG_VIDEO_IPU3_IMGU) += ipu3-imgu.o
-
-# HACK! While this driver is in bad shape, don't enable several warnings
-#       that would be otherwise enabled with W=1
-ccflags-y += $(call cc-disable-warning, packed-not-aligned)
-ccflags-y += $(call cc-disable-warning, type-limits)
-ccflags-y += $(call cc-disable-warning, unused-const-variable)
diff --git a/drivers/staging/media/ipu3/TODO b/drivers/staging/media/ipu3/TODO
index 5e55baeaea1a3..8b95e74e43a0f 100644
--- a/drivers/staging/media/ipu3/TODO
+++ b/drivers/staging/media/ipu3/TODO
@@ -27,5 +27,3 @@ staging directory.
 - Document different operation modes, and which buffer queues are relevant
   in each mode. To process an image, which queues require a buffer an in
   which ones is it optional?
-
-- Make sure it builds fine with no warnings with W=1
-- 
2.11.0

Re: [PATCH 1/5] staging: imgu: Switch to __aligned() from __attribute__((aligned()))

[Tomasz Figa]

Hi Sakari,

On Wed, Feb 20, 2019 at 8:21 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> __aligned() is preferred. The patch has been generated using the following
> command in the drivers/staging/media/ipu3 directory:
>
> $ git grep -l 'aligned(32)' | \
>         xargs perl -i -pe \
>         's/__attribute__\s*\(\(\s*aligned\s*\(([0-9]+)\s*\)\s*\)\)/__aligned($1)/g;'

Thanks for the patch. These structs are expected to move to uapi/ once
the driver leaves staging. Is __aligned() now accessible to uapi
headers?

Best regards,
Tomasz

Re: [PATCH 1/5] staging: imgu: Switch to __aligned() from __attribute__((aligned()))

[Tomasz Figa]

On Thu, Mar 7, 2019 at 12:00 PM Tomasz Figa <tfiga@chromium.org> wrote:
>
> Hi Sakari,
>
> On Wed, Feb 20, 2019 at 8:21 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > __aligned() is preferred. The patch has been generated using the following
> > command in the drivers/staging/media/ipu3 directory:
> >
> > $ git grep -l 'aligned(32)' | \
> >         xargs perl -i -pe \
> >         's/__attribute__\s*\(\(\s*aligned\s*\(([0-9]+)\s*\)\s*\)\)/__aligned($1)/g;'
>
> Thanks for the patch. These structs are expected to move to uapi/ once
> the driver leaves staging. Is __aligned() now accessible to uapi
> headers?

Ah, just noticed the v2 of the series doesn't include this patch.
Sorry for the noise.

Best regards,
Tomasz

Re: [PATCH 1/5] staging: imgu: Switch to __aligned() from __attribute__((aligned()))

[Sakari Ailus]

On Thu, Mar 07, 2019 at 12:00:59PM +0900, Tomasz Figa wrote:
> On Thu, Mar 7, 2019 at 12:00 PM Tomasz Figa <tfiga@chromium.org> wrote:
> >
> > Hi Sakari,
> >
> > On Wed, Feb 20, 2019 at 8:21 PM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
> > >
> > > __aligned() is preferred. The patch has been generated using the following
> > > command in the drivers/staging/media/ipu3 directory:
> > >
> > > $ git grep -l 'aligned(32)' | \
> > >         xargs perl -i -pe \
> > >         's/__attribute__\s*\(\(\s*aligned\s*\(([0-9]+)\s*\)\s*\)\)/__aligned($1)/g;'
> >
> > Thanks for the patch. These structs are expected to move to uapi/ once
> > the driver leaves staging. Is __aligned() now accessible to uapi
> > headers?
> 
> Ah, just noticed the v2 of the series doesn't include this patch.
> Sorry for the noise.

No worries. I just intended to postpone it first but it seems it's better
to drop it. Handling __aligned(whatever) is a pain with sed --- as it may
be more than just numbers.

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

Re: [PATCH 2/5] staging: imgu: Fix struct ipu3_uapi_awb_fr_config_s alignment

[Jacopo Mondi]

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

Hi Sakari,
  what is the status of this patch?

When using the IPU3 header in libcamera we're hitting this very issue:

error: ‘ipu3_uapi_acc_param::awb_fr’ offset 36756 in ‘ipu3_uapi_acc_param’ isn’t aligned to 32
[-Werror=packed-not-aligned]

(Note that the warning is only reported in gcc8.3.0 from my testing not
by clang or older versions of gcc (5.4.0))

On Wed, Feb 20, 2019 at 01:19:50PM +0200, Sakari Ailus wrote:
> struct ipu3_uapi_awb_fr_config_s is labelled as to be aligned to 32 bytes
> but that's not the case in the ISP parameter struct of the driver, struct
> ipu3_uapi_acc_param which is packed.
>
> Also there is no need for the alignment as the struct is only handled by the
> driver. Remove the alignment from the struct.

Maybe I got mislead, but I thought memory access in the IPU3 DMA
engine should be 32 bytes aligned? Doesn't this apply here? Can we
safely drop the __aligned attribute?

I tend to think so, as all other members of 'struct
ipu3_uapi_acc_param' are not aligned.

With this confirmed:
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/staging/media/ipu3/include/intel-ipu3.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/ipu3/include/intel-ipu3.h b/drivers/staging/media/ipu3/include/intel-ipu3.h
> index cbb62643172be..4cdb4c791ecec 100644
> --- a/drivers/staging/media/ipu3/include/intel-ipu3.h
> +++ b/drivers/staging/media/ipu3/include/intel-ipu3.h
> @@ -450,7 +450,7 @@ struct ipu3_uapi_awb_fr_config_s {
>  	__u32 bayer_sign;
>  	__u8 bayer_nf;
>  	__u8 reserved2[3];
> -} __aligned(32) __packed;
> +} __packed;
>
>  /**
>   * struct ipu3_uapi_4a_config - 4A config
> --
> 2.11.0
>

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