[PATCH v4 05/23] mailbox: Add common header for RPMI messages sent via mailbox

[Anup Patel]

On Tue, May 27, 2025 at 4:46 PM Andy Shevchenko
<andriy.shevchenko at linux.intel.com> wrote:
>
> On Sun, May 25, 2025 at 02:16:52PM +0530, Anup Patel wrote:
> > The RPMI based mailbox controller drivers and mailbox clients need to
> > share defines related to RPMI messages over mailbox interface so add
> > a common header for this purpose.
>
> ...
>
> > +#include <linux/mailbox_client.h>
>
> This is not even closer to the list of the headers the header is using.
> E.g., types.h is missing.

Okay, I will add types.h

>
> > +/* RPMI version encode/decode macros */
> > +#define RPMI_VER_MAJOR(__ver)                (((__ver) >> 16) & 0xffff)
> > +#define RPMI_VER_MINOR(__ver)                ((__ver) & 0xffff)
>
> Same comment as per previous patch.

Okay, I will use macros from linux/wordpart.h

>
> ...
>
> > +     RPMI_ERR_NO_DATA                = -14,
> > +     RPMI_ERR_RESERVED_START         = -15,
> > +     RPMI_ERR_RESERVED_END           = -127,
> > +     RPMI_ERR_VENDOR_START           = -128
>
> Leave the trailing comma, as it doesn't sound like a terminator.

Okay

>
> ...
>
> > +             return -ETIMEDOUT;
> > +             return -ECOMM;
> > +             return -EOPNOTSUPP;
>
> + errno.h

Okay, I will add errno.h

>
> ...
>
> > +/* RPMI linux mailbox attribute IDs */
> > +enum rpmi_mbox_attribute_id {
> > +     RPMI_MBOX_ATTR_SPEC_VERSION = 0,
>
> Why do you need an explicit initialiser? If it's a HW requirement, all of them
> should be explicitly defined. This makes code robust against potential changes.

Explicit initializers are not needed. I will drop in the next revision.

>
> > +     RPMI_MBOX_ATTR_MAX_MSG_DATA_SIZE,
> > +     RPMI_MBOX_ATTR_SERVICEGROUP_ID,
> > +     RPMI_MBOX_ATTR_SERVICEGROUP_VERSION,
> > +     RPMI_MBOX_ATTR_MAX_ID
> > +};
>
> ...
>
> > +/* RPMI linux mailbox message types */
>
> linux --> Linux
> (everywhere)

Okay, I will update.

Regards,
Anup