RE: [PATCH v2 01/24] mpi3mr: add mpi30 Rev-R headers and Kconfig

[Kashyap Desai <kashyap.desai@broadcom.com>]

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

> >
> +/***************************************************************
> **************
> > + * Define MPI3_POINTER if it has not already been defined. By default
> > *
> > + * MPI3_POINTER is defined to be a near pointer. MPI3_POINTER can be
> defined *
> > + * as a far pointer by defining MPI3_POINTER as "far *" before this
> > header
> *
> > + * file is included.
> > *
> > +
> >
> +****************************************************************
> *****
> > +*******/
> > +#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.

>
> > +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.

>
> > +typedef __le32 U32;
> > +typedef __le64 U64 __aligned(4);
>
> Do all __le64 variables need four-byte alignment or only some of them?

Yes. U64 is used within MPI header file (Firmware interface) and all data
structure within a MPI spec require 4 bytes alignment.

>
> > +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.
I will post V3 series considering some changes which I can immediately
accommodate. Please review V3 and let me know if you still have any
concerns.

Kashyap

>
> Thanks,
>
> Bart.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]