On Mon, Mar 19, 2018 at 07:36:45PM +0100, Jonas Rabenstein wrote: > Every step starts with resetting the cmd buffer as well as the comid and > constructs the appropriate OPAL_CALL command. Consequently, those > actions may be combined into one generic function. On should take care, > that the opening and closing tokens for the argument list are already > emitted by those functions and thus must not be additionally added. > > Signed-off-by: Jonas Rabenstein <jonas.rabenstein@xxxxxxxxxxxxxxxxxxxxxxx> > > diff --git a/block/sed-opal.c b/block/sed-opal.c > index 771b4cfff95c..efe5d2a7f3dc 100644 > --- a/block/sed-opal.c > +++ b/block/sed-opal.c > @@ -656,6 +656,9 @@ static int cmd_finalize(struct opal_dev *cmd, u32 hsn, u32 tsn) > struct opal_header *hdr; > int err = 0; > > + /* close the parameter list opened from start_opal_cmd */ > + add_token_u8(&err, cmd, OPAL_ENDLIST); > + > add_token_u8(&err, cmd, OPAL_ENDOFDATA); > add_token_u8(&err, cmd, OPAL_STARTLIST); > add_token_u8(&err, cmd, 0); I think this should be a separate patch, independent of the newly added start_opal_cmd. > @@ -998,6 +1001,26 @@ static void clear_opal_cmd(struct opal_dev *dev) > memset(dev->cmd, 0, IO_BUFFER_LENGTH); > } > > +static int start_opal_cmd(struct opal_dev *dev, const u8 *uid, const u8 *method) > +{ > + int err = 0; start_opal_cmd and cmd_finalize don't really seem to match in terms of naming. I don't really care either way, but a little consistency would be nice. > + /* every method call is followed by its parameters enclosed within > + * OPAL_STARTLIST and OPAL_ENDLIST tokens. We automatically open the > + * parameter list here and close it later in cmd_finalize > + */ Normal Linux comment style would be: /* * Every method call is followed by its parameters enclosed within * OPAL_STARTLIST and OPAL_ENDLIST tokens. We automatically open the * parameter list here and close it later in cmd_finalize. */