On 4/16/21 3:53 AM, Kashyap Desai wrote: [ ... ] >>> +#ifndef MPI3_POINTER >>> +#define MPI3_POINTER * >>> +#endif /* MPI3_POINTER */ >> >> As far as I know there are no far pointers in the Linux kernel. > > Hi Bart - I can remove the comment and just use below line in this file - > #define MPI3_POINTER * > > Common MPI header file which is directly derived from common repository > within a Broadcom like " mpi30_cnfg.h", " mpi30_transport.h" etc. has > reference of MPI3_POINTER instead of directly using "*" there. > It may be useful for some third party development (like SDK) and they can > replace MPI3_POINTER accordingly. Only mpi30_types.h is Linux only file and > we add wrapper in this file to make sure common header file compiles on > Linux instead of completely replacing whole MPI header file. How about changing all MPI3_POINTER occurrences into MPI3_POINTER_ATTR * and defining MPI3_POINTER_ATTR as an empty macro in this header file? >>> +typedef u8 U8; >>> +typedef __le16 U16; >> >> Introducing U16 as an alias for __le16 is confusing since there is already >> an >> 'u16' type in the Linux kernel. Please use the __le* types directly. > > I explain this issue in above comment. I'm not sure that explanation is sufficient. Has it been considered to change all U16 occurrences into __le16 and to add 'typedef uint16_t __le16;' in the appropriate header file if building for another operating system than Linux? >>> +typedef U8 * PU8; >>> +typedef U16 * PU16; >>> +typedef U32 * PU32; >>> +typedef U64 * PU64; >> >> Please use __le* directly instead of introducing the above aliases. >> Please also follow the Linux kernel coding style (no space after '*'). > > There is no reference in mpi3mr driver to use above defines. I will remove > them completely. Thanks! Bart.