Re: [PATCH RFC 2/2] cxl/memdev: Remove temporary variables from cxl_memdev_state

[Ira Weiny <ira.weiny@intel.com>]

Alejandro Lucero Palau wrote:
> 
> <snip>
> 
> >
> >> FWIW, I had (as part of current in-progress v10) similar struct than
> >> used here for Type2 initialization when there is no mailbox.
> >>
> >>
> >> I had added a specific function for initialising that struct, but my
> >> idea now with this change is to have cxl_mem_dev_info initialized by the
> >> driver before calling cxl_dev_state_identify,
> > Why is the accelerator calling cxl_dev_state_identify?  I did not see that
> > in v9.  My idea was that was a mailbox only call which is only needed for
> > memdevs.  And cxl_add_partition() can be called by accelerators as a
> > convenience function to aid in creating cxl_dpa_info.  (This and cxl_test
> > needed that function shared so it just got left in mbox.c)
> 
> 
> No in v9 ... that predates Dan's DPA patches.
> 
> 
> Type2 without an mbox needs to give those values obtained from 
> CXL_MBOX_OP_IDENTIFY. I'm using an struct for passing those values to 
> cxl_dev_state_identify for having same function serving Type2 with and 
> without mbox.

But in the case of no mbox where does the type2 get those values from
within cxl_dev_state_identify()?

The idea with this patch change is to only call cxl_dev_state_identify IFF
the device has a mailbox.  Are you saying the same thing?

> I could have another function instead and calling that one 
> for Type2 with mbox. My idea is to keep similar initialization than 
> Type3 pci driver.

Agreed.  But without an mbox I'm hoping you don't need to call
cxl_dev_state_identify at all if you don't need to query the device...  ie
don't have a mailbox.

> 
> 
> >> and inside that function
> > I'm not clear which 'that function' you are referring to here.
> 
> 
> cxl_dev_state_identify
> 
> 
> >> checking if total_bytes already != 0 for avoiding call the mbox command
> >> for getting the info. This will support both cases for Type2, with and
> >> without mailbox.
> > I think I agree with you except the != 0 to avoid mailbox commands.
> >
> > Unless I am miss-understanding Dan we need to get to a place where mailbox
> > commands stop filling in structures unless those work for both type 2 and
> > 3 __and__ are optional.  Because putting in special checks for the type
> > within a cxl/core/mailbox call is wrong IMO.
> 
> 
> As I said, I can avoid that check with a wrapper for Type2, then only 
> Type2 with mbox and supporting that command (is it mandatory if an 
> mbox?) will end up calling cxl_dev_state_identify.

Even if type 2 has a mailbox the partition information may or may not need
to be in the identify command.

Why force any type 2 to call that mailbox command?

Ira