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

Alejandro Lucero Palau
2024-08-13T08:30:36Z

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
>> <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 ...
>
>
>

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.