On Thursday 17 February 2022 15:16:33 Marek Behún wrote: > On Thu, 17 Feb 2022 10:26:18 +0100 > Pali Rohár <p...@kernel.org> wrote: > > > Allow to specify input parameters, define all available mbox commands > > supported by CZ.NIC secure firmware + Marvell fuse.bin firmware and fix > CZ.NIC's and also Marvell's > > parsing response from Marvell OTP commands. > > > > Signed-off-by: Pali Rohár <p...@kernel.org> > > ... > > > #define MBOX_STS_ERROR(s) ((s) & (3 << 30)) > > #define MBOX_STS_VALUE(s) (((s) >> 10) & 0xfffff) > > #define MBOX_STS_CMD(s) ((s) & 0x3ff) > > +#define MBOX_STS_MARVELL_ERROR(s) ((s) == 0 ? 0 : (s) == 2 ? ETIMEDOUT : > > (s) == 3 ? EINVAL : (s) == 4 ? ENOSYS : EIO) > > This is starting to become too complicated for a macro :-(
It is straightforward switch macro. I can reformat it to be more readable, e.g.: #define MBOX_STS_MARVELL_ERROR(s) ((s) == 0 ? 0 : \ (s) == 2 ? ETIMEDOUT : \ (s) == 3 ? EINVAL : \ (s) == 4 ? ENOSYS : \ EIO) > What do you think about converting all these to a static function? > Something like > > static int mbox_parse_status(u32 status, u16 *cmd, u32 *value, > bool marvell) > { > ... > } > > > > > #ifndef _MVEBU_MBOX_H > > @@ -16,8 +17,24 @@ enum mbox_cmd { > > > > MBOX_CMD_OTP_READ, > > MBOX_CMD_OTP_WRITE, > > + > > + MBOX_CMD_REBOOT, > > + > > + /* OTP read commands supported by Marvell fuse.bin firmware */ > Marvell's > > + MBOX_CMD_OTP_READ_1B = 257, > > + MBOX_CMD_OTP_READ_8B, > > + MBOX_CMD_OTP_READ_32B, > > + MBOX_CMD_OTP_READ_64B, > > + MBOX_CMD_OTP_READ_256B, > > + > > + /* OTP write commands supported by Marvell fuse.bin firmware */ > Marvell's