Re: [PATCH v2 1/3] drivers/cxl: introduce cxl_region_driver field for cxl_region

[Gregory Price <gourry@gourry.net>]

> > :-/
> > 
> > I'm not opposed to this idea but I'm worried that this adds to the already
> > very implicit nature of the CXL subsystem.
> > 
> 

Took me a bit, but i follow this now.

The current flow of things:

   echo region0 > cxl/drivers/cxl_region/bind
      cxl_region_probe()
        - devm_cxl_add_dax_region()
        - dax_region is created and probed

      cxl_dax_region_probe()
        - devm_create_dev_dax(&data));
	  dax0.0 is created with IORESOURCE_DAX_KMEM

      dax/bus.c discovery
        - dax bus discovers new dev_dax
	- dax bus auto-sets driver type to dax_kmem
	  dax_bus_match() -> dax_match_type() 
          if (dev_dax->region->res.flags & IORESOURCE_DAX_KMEM)
                type = DAXDRV_KMEM_TYPE;
        - dax_bus_probe() calls dax_drv->probe() unconditionally
	  
      dev_dax_kmem_probe()
        - kmem driver starts memory hotplug procedure


So to my (your? our?) point - the following is implicit in cxl:

	if (IS_ENABLED(CONFIG_DEV_DAX_KMEM))
		create_ram_region  ===>  dax_kmem
	else
		create_ram_region  ===>  device_dax


What would have been nice:

echo region0 > decoder0.0/create_ram_region
  - region0 is created
  - program decoders

echo region0 > cxl/drivers/dax_region/bind
  - dax_region0 is created
  - dax_region0 creates dax0.0
  - dax/bus.c *does not* auto-probe the driver type
  - user configures dax0.0

echo dax0.0 > dax/drivers/kmem/bind
  - dax/kmem.c does the whole hotplug song and dance

But in an effort to make auto-onlining capacity user-friendly,
the middle piece was automated away.

I don't see how we dig ourself out of the former flow without either:

1) Breaking the current auto-config flow by adding:

      cxl/drivers/[xxx_region]/bind

2) Adding something akin to `region0/region_driver` which is basically
   used to replace the `echo region0 > cxl/drivers/[xxx_region]/bind

   you just get
   switch (region->region_driver) {
   case dax: do_dax_thing(); break;
   case sysram: do_sysram_thing(); break;
   ...
   }

   do_dax_thing() is otherwise just dax_region_probe()
   do_sysram_thing() is otherwise just sysram_region_probe()


~Gregory