On Tue, Mar 13, 2018 at 02:08:56PM +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. > > Signed-off-by: Jonas Rabenstein <jonas.rabenstein@xxxxxxxxxxxxxxxxxxxxxxx> > --- > block/sed-opal.c | 243 +++++++++++++++---------------------------------------- > 1 file changed, 63 insertions(+), 180 deletions(-) > > diff --git a/block/sed-opal.c b/block/sed-opal.c > index a228a13f0a08..22dbea7cf4d1 100644 > --- a/block/sed-opal.c > +++ b/block/sed-opal.c > @@ -659,6 +659,7 @@ static int cmd_finalize(struct opal_dev *cmd, u32 hsn, u32 tsn) > struct opal_header *hdr; > int err = 0; > > + 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); > @@ -1001,6 +1002,21 @@ 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; > + > + clear_opal_cmd(dev); > + set_comid(dev, dev->comid); > + > + add_token_u8(&err, dev, OPAL_CALL); > + add_token_bytestring(&err, dev, uid, OPAL_UID_LENGTH); > + add_token_bytestring(&err, dev, method, OPAL_METHOD_LENGTH); > + add_token_u8(&err, dev, OPAL_STARTLIST); Can you resend this patch (in a bit I'm revewing the rest) with a comment here and in cmd_finalize explaining that the start_list here gets terminiated by the new opal_endlist in cmd_finalize. I'm not a huge fan of having the start/list and end/list in separate functions since those are used to specify a parameter list for a opal method. I can get over it since it cleans the code up, as long as there are comments on both sides reminding me what they're opening/closing off.