Re: [ndctl PATCH 5/7] libcxl: add interfaces for SET_PARTITION_INFO mailbox command

From: Alison Schofield <alison.schofield@intel.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Ben Widawsky <ben.widawsky@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	Linux NVDIMM <nvdimm@lists.linux.dev>,
	linux-cxl@vger.kernel.org
Subject: Re: [ndctl PATCH 5/7] libcxl: add interfaces for SET_PARTITION_INFO mailbox command
Date: Sun, 9 Jan 2022 18:13:38 -0800	[thread overview]
Message-ID: <20220110021338.GA813196@alison-desk> (raw)
In-Reply-To: <CAPcyv4jdt-936WpqNQv7hR2oPSFHbqsCDs40JgBJBaxZ-tHPJw@mail.gmail.com>

On Fri, Jan 07, 2022 at 06:27:40PM -0800, Dan Williams wrote:
> On Fri, Jan 7, 2022 at 5:46 PM Alison Schofield
> <alison.schofield@intel.com> wrote:
> >
> > On Thu, Jan 06, 2022 at 12:53:02PM -0800, Ira Weiny wrote:
> > > On Mon, Jan 03, 2022 at 12:16:16PM -0800, Schofield, Alison wrote:
> > > > From: Alison Schofield <alison.schofield@intel.com>
> > > >
> > > > Add APIs to allocate and send a SET_PARTITION_INFO mailbox command.
> > > >
> > > > +   le64 volatile_capacity;
> > > > +   u8 flags;
> > > > +} __attribute__((packed));
> > > > +
> > > > +/* CXL 2.0 8.2.9.5.2 Set Partition Info */
> > > > +#define CXL_CMD_SET_PARTITION_INFO_NO_FLAG                         (0)
> > > > +#define CXL_CMD_SET_PARTITION_INFO_IMMEDIATE_FLAG                  (1)
> > >
> > > BIT(0) and BIT(1)?
> > >
> > > I can't remember which bit is the immediate flag.
> > >
> > Immediate flag is BIT(0).
> > Seemed awkward/overkill to use bit macro -
> > +#define CXL_CMD_SET_PARTITION_INFO_NO_FLAG                             (0)
> > +#define CXL_CMD_SET_PARTITION_INFO_IMMEDIATE_FLAG                      BIT(1)
> >
> > I just added api to use this so you'll see it in action in v2
> > of this patchset and can comment again.
> 
> Why is a "no flag" definition needed? Isn't that just "!IMMEDIATE"
>
You are right. The no flag set case is !IMMEDIATE.

>Also BIT(1) == 0x2, so that should be BIT(0), right?
Yes IMMEDIATE FLAG IS BIT(0). 

This chatter is related to using something more descriptive that '0'
for !IMMEDIATE when the cxl command makes the call to the api. (Patch 7)
I added a new accessor in v2 that returns the bit.