diff mbox series

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

Message ID 166993043433.1882361.17651413716599606118.stgit@dwillia2-xfh.jf.intel.com
State Accepted
Commit 8b3b1c0dc500a00c34ab74fb8a0d9e7286220c04
Headers show
Series cxl: Add support for Restricted CXL hosts (RCD mode) | expand

Commit Message

Dan Williams Dec. 1, 2022, 9:33 p.m. UTC
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>
---
 tools/testing/cxl/test/cxl.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Dave Jiang Dec. 1, 2022, 9:57 p.m. UTC | #1
On 12/1/2022 2:33 PM, Dan Williams 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>

Reviewed-by: Dave Jiang <dave.jiang@intel.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.
>    */
>   struct cxl_cedt_context {
> @@ -340,7 +342,7 @@ static int mock_acpi_table_parse_cedt(enum acpi_cedt_type id,
>   	unsigned long end;
>   	int i;
>   
> -	if (dev != &cxl_acpi->dev)
> +	if (!is_mock_port(dev) && !is_mock_dev(dev))
>   		return acpi_table_parse_cedt(id, handler_arg, arg);
>   
>   	if (id == ACPI_CEDT_TYPE_CHBS)
>
Jonathan Cameron Dec. 2, 2022, 3:58 p.m. UTC | #2
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!

Beyond that dark mutterings about reformatting lines above the change made
and hence making this patch noisier than it needs to be...
 
>   */
>  struct cxl_cedt_context {
> @@ -340,7 +342,7 @@ static int mock_acpi_table_parse_cedt(enum acpi_cedt_type id,
>  	unsigned long end;
>  	int i;
>  
> -	if (dev != &cxl_acpi->dev)
> +	if (!is_mock_port(dev) && !is_mock_dev(dev))
>  		return acpi_table_parse_cedt(id, handler_arg, arg);
>  
>  	if (id == ACPI_CEDT_TYPE_CHBS)
>
Dan Williams Dec. 3, 2022, 7:22 a.m. UTC | #3
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.
diff mbox series

Patch

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.
  */
 struct cxl_cedt_context {
@@ -340,7 +342,7 @@  static int mock_acpi_table_parse_cedt(enum acpi_cedt_type id,
 	unsigned long end;
 	int i;
 
-	if (dev != &cxl_acpi->dev)
+	if (!is_mock_port(dev) && !is_mock_dev(dev))
 		return acpi_table_parse_cedt(id, handler_arg, arg);
 
 	if (id == ACPI_CEDT_TYPE_CHBS)