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

From: Jonathan Cameron <[email-masked]>
Date: Thu, 15 Aug 2024 17:38:12 +0100

On Tue, 13 Aug 2024 09:30:08 +0100
Alejandro Lucero Palau <[email-masked]> wrote:

> On 8/12/24 12:16, Alejandro Lucero Palau wrote:
> >
> > On 8/4/24 18:10, Jonathan Cameron wrote:  
> >> On Mon, 15 Jul 2024 18:28:21 +0100
> >> <[email-masked]> wrote:
> >>  
> >>> From: Alejandro Lucero <[email-masked]>
> >>>
> >>> 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/[email-masked]/T/#m52543f85d0e41ff7b3063fdb9caa7e845b446d0e
> >>>
> >>> Signed-off-by: Alejandro Lucero <[email-masked]>
> >>> Co-developed-by: Dan Williams <[email-masked]>  
> >>  
> >  
> >>> +
> >>> +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 ...
> >
> >
> >  
> 
> I have second thoughts about this.
> 
> 
> I consider this as an accessor  for, as you said in a previous exchange, 
> facilitating changes to the core structs without touching those accel 
> drivers using it.
> 
> Type3 driver is part of the CXL core and easy to change for these kind 
> of updates since it will only be one driver supporting all Type3, and an 
> accessor is not required then.
> 
> Let me know what you think.

It's less critical, but longer term I'd expect any stuff that makes
sense for accelerators and the type 3 driver to use the same
approaches and code paths.  Makes it easier to see where they
are related than opencoding the accesses in the type 3 driver will
do.  In the very long term, I'd expect the type 3 driver to just be
another CXL driver alongside many others.

Jonathan