RE: [PATCH v7 05/16] intel-ipu3: abi: Add structs

["Mani, Rajmohan" <rajmohan.mani@intel.com>]

Hi Sakari,

> Subject: Re: [PATCH v7 05/16] intel-ipu3: abi: Add structs
> 
> Hi Raj,
> 
> On Mon, Nov 05, 2018 at 07:05:53PM +0000, Mani, Rajmohan wrote:
> > Hi Sakari,
> >
> > > Subject: Re: [PATCH v7 05/16] intel-ipu3: abi: Add structs
> > >
> > > Hi Yong,
> > >
> > > On Mon, Oct 29, 2018 at 03:22:59PM -0700, Yong Zhi wrote:
> > > > This add all the structs of IPU3 firmware ABI.
> > > >
> > > > Signed-off-by: Yong Zhi <yong.zhi@intel.com>
> > > > Signed-off-by: Rajmohan Mani <rajmohan.mani@intel.com>
> > >
> > > ...
> > >
> > > > +struct imgu_abi_shd_intra_frame_operations_data {
> > > > +	struct imgu_abi_acc_operation
> > > > +		operation_list[IMGU_ABI_SHD_MAX_OPERATIONS]
> > > __attribute__((aligned(32)));
> > > > +	struct imgu_abi_acc_process_lines_cmd_data
> > > > +		process_lines_data[IMGU_ABI_SHD_MAX_PROCESS_LINES]
> > > __attribute__((aligned(32)));
> > > > +	struct imgu_abi_shd_transfer_luts_set_data
> > > > +		transfer_data[IMGU_ABI_SHD_MAX_TRANSFERS]
> > > > +__attribute__((aligned(32)));
> > >
> > > Could you replace this wth __aligned(32), please? The same for the
> > > rest of the header.
> > >
> >
> > Using __aligned(32) in the uAPI header resulted in compilation errors
> > in user space / camera HAL code.
> >
> > e.g
> > ../../../../../../../../usr/include/linux/intel-ipu3.h:464:57: error: expected ';'
> > at end of declaration list
> >  __u8 bayer_table[IPU3_UAPI_AWB_FR_BAYER_TABLE_MAX_SIZE]
> > __aligned(32);
> >
> > So we ended up using __attribute__((aligned(32))) format in uAPI
> > header and to be consistent, we followed the same format in ABI header as
> well.
> >
> > Let us know if it's okay to deviate between uAPI and ABI header for
> > this alignment qualifier.
> 
> There's a reason for using __attribute__((aligned(32))) in the uAPI header, but
> not in the in-kernel headers where __aligned(32) is preferred.
> 
> I have a patch for addressing this for the uAPI headers as well so
> __aligned(32) could be used there, too; I'll submit it soon. Let's see...
> there are kerneldoc issues still in this area.
> 

Great. Thanks for the patch to address this need.

> --
> Regards,
> 
> Sakari Ailus
> sakari.ailus@linux.intel.com