Re: [PATCH v21 00/23] Type2 device basic support

[PJ Waskiewicz <ppwaskie@kernel.org>]

On Fri, 2025-11-21 at 10:40 +0000, Alejandro Lucero Palau wrote:
> 
> On 11/21/25 06:41, PJ Waskiewicz wrote:
> > On Wed, 2025-11-19 at 19:22 +0000, alejandro.lucero-palau@amd.com
> > wrote:
> > 
> > Hi Alejandro,
> > 
> > Sorry it's been a bit since I've been able to comment.  I've been
> > trying to test these patchsets with varying degrees of success. 
> > Still
> > haven't gotten things up and running fully.  One comment below.
> 
> 
> Hi,
> 
> 
> No worries!
> 
> 
> > > From: Alejandro Lucero <alucerop@amd.com>
> > > 
> > > The patchset should be applied on the described base commit then
> > > applying
> > > Terry's v13 about CXL error handling. The first 4 patches come
> > > from
> > > Dan's
> > > for-6.18/cxl-probe-order branch with minor modifications.
> > > 
> > > v21 changes;
> > > 
> > >    patch1-2: v20 patch1 splitted up doing the code move in the
> > > second
> > > 	    patch in v21. (Jonathan)
> > >   
> > >    patch1-4: adding my Signed-off tag along with Dan's
> > > 
> > >    patch5: fix duplication of CXL_NR_PARTITION definition
> > > 
> > >    patch7: dropped the cxl test fixes removing unused function.
> > > It was
> > > 	  sent independently ahead of this version.
> > > 
> > >    patch12: optimization for max free space calculation
> > > (Jonathan)
> > > 
> > >    patch19: optimization for returning on error (Jonathan)
> > I cannot test these v21 patches or the v20 patches for the same
> > reason.
> > I suspect v19 is also affected, but I was stuck on v17 for awhile
> > (b4
> > was really not likely the prereq patches you required to get the
> > tree
> > into a usable state to apply your patchset).
> > 
> > When I build and go to install the kernel mods, depmod fails:
> > 
> > DEPMOD  /lib/modules/6.18.0-rc6+
> > depmod: ERROR: Cycle detected: cxl_core -> cxl_mem -> cxl_port ->
> > cxl_core
> > depmod: ERROR: Cycle detected: cxl_core -> cxl_mem -> cxl_core
> > depmod: ERROR: Found 3 modules in dependency cycles!
> > 
> > I repro'd this on a few different systems, and just finally repro'd
> > this on a box outside of my work network.
> > 
> > This is unusable unfortunately, so I can't test this if I wanted
> > to.
> 
> 
> I have been able to reproduce this, and I think after the changes 
> introduced in patches 2 & 3, we also need this:
> 
> 
> 
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index 6b871cbbce13..94a3102ce86b 100644
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
> @@ -1,6 +1,6 @@
>   # SPDX-License-Identifier: GPL-2.0-only
>   menuconfig CXL_BUS
> -       tristate "CXL (Compute Express Link) Devices Support"
> +       bool "CXL (Compute Express Link) Devices Support"
>          depends on PCI
>          select FW_LOADER
>          select FW_UPLOAD
> 
> 
> This changes implies neither CXL_BUS optionally being a module nor
> cxl_mem.
> 
> 
> This should be enough for at least you able to test the patchset.
> 
> 
> If this is agreed, I will send a v22 with it.

This seems reasonable to me to make things work.

I'd definitely want Dave or Dan to weigh in though since this does make
CXL no longer be modular, and it's either built-in or not.  Personally,
if this is the price to pay for the non-asynchronous nature of memdev
creation for a Type2 driver, I'm fine with that.

I'm rebuilding now and will rebase my drivers onto this, and hopefully
be testing again over the weekend.  If things start looking good, I'll
send some Tested-by: and also review more deeply.

Cheers,
-PJ