All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alison Schofield <alison.schofield@intel.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Ben Widawsky <ben.widawsky@intel.com>,
	Ira Weiny <ira.weiny@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	Linux NVDIMM <nvdimm@lists.linux.dev>,
	linux-cxl@vger.kernel.org
Subject: Re: [ndctl PATCH v3 5/6] libcxl: add interfaces for SET_PARTITION_INFO mailbox command
Date: Mon, 31 Jan 2022 17:34:45 -0800	[thread overview]
Message-ID: <20220201013445.GA913958@alison-desk> (raw)
In-Reply-To: <CAPcyv4h5+sMSKh-seNbmmTVuNzs5-8FTWUoYHw=LWtSrSNq1=g@mail.gmail.com>

> > snip
> > >
> > >
> > > I don't understand what this is for?
> > >
> > > Let's back up. In order to future proof against spec changes, and
> > > endianness, struct packing and all other weird things that make struct
> > > ABIs hard to maintain compatibility the ndctl project adopts the
> > > libabc template of just not letting library consumers see any raw data
> > > structures or bit fields by default [1]. For a situation like this
> > > since the command only has one flag that affects the mode of operation
> > > I would just go ahead and define an enum for that explicitly.
> > >
> > > enum cxl_setpartition_mode {
> > >     CXL_SETPART_NONE,
> > >     CXL_SETPART_NEXTBOOT,
> > >     CXL_SETPART_IMMEDIATE,
> > > };
> > >
> > > Then the main function prototype becomes:
> > >
> > > int cxl_cmd_new_setpartition(struct cxl_memdev *memdev, unsigned long
> > > long volatile_capacity);
> > >
> > > ...with a new:
> > >
> > > int cxl_cmd_setpartition_set_mode(struct cxl_cmd *cmd, enum
> > > cxl_setpartition_mode mode);
> > >
> >
> > I don't understand setting of the mode separately. Can it be:
> >
> > int cxl_cmd_new_setpartition(struct cxl_memdev *memdev,
> >                              unsigned long long volatile_capacity,
> >                              enum cxl_setpartition_mode mode);
> 
> It could be, but what happens when the specification defines a new
> flag for this command? Then we would have cxl_cmd_new_setpartition()
> and cxl_cmd_new_setpartition2()  to add the new parameters. A helper
> function after establishing the cxl_cmd context lets you have
> flexibility to extend the base command by as many new flags and modes
> that come along... hopefully none, but you never know.

Got it. Doing the 'new' followed by 'mode' set up you suggested.
(Sorry, I didn't update this thread after our offline chat.)

  reply	other threads:[~2022-02-01  1:30 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-18 20:25 [ndctl PATCH v3 0/6] Add partitioning support for CXL memdevs alison.schofield
2022-01-18 20:25 ` [ndctl PATCH v3 1/6] libcxl: add GET_PARTITION_INFO mailbox command and accessors alison.schofield
2022-01-26 16:07   ` Dan Williams
2022-01-26 16:37     ` Alison Schofield
2022-01-26 17:50       ` Verma, Vishal L
2022-01-18 20:25 ` [ndctl PATCH v3 2/6] libcxl: add accessors for capacity fields of the IDENTIFY command alison.schofield
2022-01-26 16:16   ` Dan Williams
2022-01-18 20:25 ` [ndctl PATCH v3 3/6] libcxl: return the partition alignment field in bytes alison.schofield
2022-01-26 16:40   ` Dan Williams
2022-01-18 20:25 ` [ndctl PATCH v3 4/6] cxl: add memdev partition information to cxl-list alison.schofield
2022-01-26 17:23   ` Dan Williams
2022-01-26 19:03     ` Alison Schofield
2022-01-18 20:25 ` [ndctl PATCH v3 5/6] libcxl: add interfaces for SET_PARTITION_INFO mailbox command alison.schofield
2022-01-26 23:41   ` Dan Williams
2022-01-27 20:50     ` Alison Schofield
2022-02-01  1:24       ` Dan Williams
2022-02-01  1:34         ` Alison Schofield [this message]
2022-02-01  1:25     ` Alison Schofield
2022-02-01  1:32       ` Dan Williams
2022-01-18 20:25 ` [ndctl PATCH v3 6/6] cxl: add command set-partition-info alison.schofield
2022-01-27  1:44   ` Dan Williams
2022-01-27  5:44     ` Alison Schofield
2022-01-27 19:03       ` Dan Williams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220201013445.GA913958@alison-desk \
    --to=alison.schofield@intel.com \
    --cc=ben.widawsky@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=ira.weiny@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=nvdimm@lists.linux.dev \
    --cc=vishal.l.verma@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.