Re: [PATCH v2 01/15] cxl: add type2 device basic support

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Mon, 12 Aug 2024 12:16:02 +0100
Alejandro Lucero Palau <alucerop@amd.com> wrote:

> On 8/4/24 18:10, Jonathan Cameron wrote:
> > On Mon, 15 Jul 2024 18:28:21 +0100
> > <alejandro.lucero-palau@amd.com> wrote:
> >  
> >> From: Alejandro Lucero <alucerop@amd.com>
> >>
> >> Differientiate Type3, aka memory expanders, from Type2, aka device
> >> accelerators, with a new function for initializing cxl_dev_state.
> >>
> >> Create opaque struct to be used by accelerators relying on new access
> >> functions in following patches.
> >>
> >> Add SFC ethernet network driver as the client.
> >>
> >> Based on https://lore.kernel.org/linux-cxl/168592149709.1948938.8663425987110396027.stgit@dwillia2-xfh.jf.intel.com/T/#m52543f85d0e41ff7b3063fdb9caa7e845b446d0e
> >>
> >> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> >> Co-developed-by: Dan Williams <dan.j.williams@intel.com>  
> >  
> 
> >> +
> >> +void cxl_accel_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec)
> >> +{
> >> +	cxlds->cxl_dvsec = dvsec;  
> > Nothing to do with accel. If these make sense promote to cxl
> > core and a linux/cxl/ header.  Also we may want the type3 driver to
> > switch to them long term. If nothing else, making that handle the
> > cxl_dev_state as more opaque will show up what is still directly
> > accessed and may need to be wrapped up for a future accelerator driver
> > to use.
> >  
> 
> I will change the function name then, but not sure I follow the comment 
> about more opaque ...
If most code can't see the internals of cxl_dev_state because it
doesn't include the header that defines it, then we will generally
spot data that may not belong in that state structure in the first place
or where it is appropriate to have an accessor function mediating that
access.

Jonathan