Re: [PATCH] cxl: Rename ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3

[Dan Williams]

Zhang, Rui wrote:
> CC Rafael,
> 
> On Sun, 2024-11-10 at 14:13 +0800, Huang, Ying wrote:
> > Dan Williams <dan.j.williams@xxxxxxxxx> writes:
> > 
> > > Alison Schofield wrote:
> > > [..]
> > > > > I think so too.  However, I prefer to keep this patch just
> > > > > mechanic
> > > > > renaming and do these changes in another patch.  Do you agree?
> > > > > 
> > > > 
> > > > I don't know. I was just questioning where and how far the naming
> > > > scheme
> > > > needs to change.
> > > > 
> > > > Maybe Jonathan, as the Suggested-by, can chime in and move this
> > > > ahead.
> > > 
> > > I feel like we are going to be living with the ghosts of the
> > > original
> > > "Type2 / Type3" naming problem for the rest of the subsystem's
> > > lifespan
> > > especially since they were encoded in the ABI and ABI is forever.
> > > 
> > > I am not opposed to this localized rename in drivers/cxl/acpi.c on
> > > principal, but in terms of incremental value relative to the
> > > thrash, it's
> > > questionable.
> > > 
> > > For example changes to include/acpi/actbl1.h need to be chased
> > > through
> > > ACPICA, at which point is this rename really worth it?
> > 
> > I think that it's not too hard to change ACPI tables definition. 
> > Added
> > Bob and Rui for ACPICA related change.
> 
> For the change below,
> 
> diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
> index 199afc2cd122..e195909928df 100644
> --- a/include/acpi/actbl1.h
> +++ b/include/acpi/actbl1.h
> @@ -551,11 +551,11 @@ struct acpi_cedt_cfmws_target_element {
>  
>  /* Values for Restrictions field above */
>  
> -#define ACPI_CEDT_CFMWS_RESTRICT_TYPE2      (1)
> -#define ACPI_CEDT_CFMWS_RESTRICT_TYPE3      (1<<1)
> -#define ACPI_CEDT_CFMWS_RESTRICT_VOLATILE   (1<<2)
> -#define ACPI_CEDT_CFMWS_RESTRICT_PMEM       (1<<3)
> -#define ACPI_CEDT_CFMWS_RESTRICT_FIXED      (1<<4)
> +#define ACPI_CEDT_CFMWS_RESTRICT_DEVMEM        (1)
> +#define ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM   (1<<1)
> +#define ACPI_CEDT_CFMWS_RESTRICT_VOLATILE      (1<<2)
> +#define ACPI_CEDT_CFMWS_RESTRICT_PMEM          (1<<3)
> +#define ACPI_CEDT_CFMWS_RESTRICT_FIXED         (1<<4)
> 
> This change is made based on the spec update, right?
> 
> Do we have any user(other than Linux) for the old version of spec?
> If yes, we probably need to keep the old ones. And IMO, if spec changes
> and the bit definition changes, we should introduce new Macros for the
> new definitions, together with spec revision info, say something like

At least for me the thrash does not have a signficant upside. The more
important terminology concepts are within the CXL core.

So I would just merge this rename below and call it a day, i.e. leave
the cxl_acpi rename alone.

diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 0fc96f8bf15c..b9083ce1cf74 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -315,13 +315,13 @@ resource_size_t cxl_rcd_component_reg_phys(struct device *dev,
  * Additionally indicate whether decoder settings were autodetected,
  * user customized.
  */
-#define CXL_DECODER_F_RAM   BIT(0)
-#define CXL_DECODER_F_PMEM  BIT(1)
-#define CXL_DECODER_F_TYPE2 BIT(2)
-#define CXL_DECODER_F_TYPE3 BIT(3)
-#define CXL_DECODER_F_LOCK  BIT(4)
-#define CXL_DECODER_F_ENABLE    BIT(5)
-#define CXL_DECODER_F_MASK  GENMASK(5, 0)
+#define CXL_DECODER_F_RAM         BIT(0)
+#define CXL_DECODER_F_PMEM        BIT(1)
+#define CXL_DECODER_F_DEVMEM      BIT(2)
+#define CXL_DECODER_F_HOSTONLYMEM BIT(3)
+#define CXL_DECODER_F_LOCK        BIT(4)
+#define CXL_DECODER_F_ENABLE      BIT(5)
+#define CXL_DECODER_F_MASK        GENMASK(5, 0)

 enum cxl_decoder_type {
        CXL_DECODER_DEVMEM = 2,