Re: [PATCH v6 06/12] tools/testing/cxl: Make mock CEDT parsing more robust

[Dan Williams <dan.j.williams@intel.com>]

Jonathan Cameron wrote:
> On Thu, 01 Dec 2022 13:33:54 -0800
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > Accept any cxl_test topology device as the first argument in
> > cxl_chbs_context.
> > 
> > This is in preparation for reworking the detection of the component
> > registers across VH and RCH topologies. Move
> > mock_acpi_table_parse_cedt() beneath the definition of is_mock_port()
> > and use is_mock_port() instead of the explicit mock cxl_acpi device
> > check.
> > 
> > Acked-by: Alison Schofield <alison.schofield@intel.com>
> > Reviewed-by: Robert Richter <rrichter@amd.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>  A comment inline on possible improvement elsewhere, but otherwise seems fine.
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> 
> > ---
> >  tools/testing/cxl/test/cxl.c |   10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> > index facfcd11cb67..8acf52b7dab2 100644
> > --- a/tools/testing/cxl/test/cxl.c
> > +++ b/tools/testing/cxl/test/cxl.c
> > @@ -320,10 +320,12 @@ static int populate_cedt(void)
> >  	return 0;
> >  }
> >  
> > +static bool is_mock_port(struct device *dev);
> > +
> >  /*
> > - * WARNING, this hack assumes the format of 'struct
> > - * cxl_cfmws_context' and 'struct cxl_chbs_context' share the property that
> > - * the first struct member is the device being probed by the cxl_acpi
> > + * WARNING, this hack assumes the format of 'struct cxl_cfmws_context'
> > + * and 'struct cxl_chbs_context' share the property that the first
> > + * struct member is cxl_test device being probed by the cxl_acpi
> >   * driver.
> Side note, but that requirement would be useful to add to the two
> struct definitions so that we don't change those in future without knowing
> we need to rethink this!

Sure, folded in these hunks:

diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index b8407b77aff6..2992bac4c0e4 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -70,6 +70,10 @@ static int cxl_acpi_cfmws_verify(struct device *dev,
        return 0;
 }
 
+/*
+ * Note, @dev must be the first member, see 'struct cxl_chbs_context'
+ * and mock_acpi_table_parse_cedt()
+ */
 struct cxl_cfmws_context {
        struct device *dev;
        struct cxl_port *root_port;
diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 8acf52b7dab2..4f9dc2b3f655 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -325,7 +325,7 @@ static bool is_mock_port(struct device *dev);
 /*
  * WARNING, this hack assumes the format of 'struct cxl_cfmws_context'
  * and 'struct cxl_chbs_context' share the property that the first
- * struct member is cxl_test device being probed by the cxl_acpi
+ * struct member is a cxl_test device being probed by the cxl_acpi
  * driver.
  */
 struct cxl_cedt_context {

> 
> Beyond that dark mutterings about reformatting lines above the change made
> and hence making this patch noisier than it needs to be...

True, I need to watch out for over auto-formatting.