Messages in this thread | | | Date | Thu, 18 May 2023 10:50:20 +0100 | From | Jonathan Cameron <> | Subject | Re: [PATCH 3/3] tools/testing/cxl: Document test configurations |
| |
On Wed, 17 May 2023 14:28:12 -0700 Ira Weiny <ira.weiny@intel.com> wrote:
> The devices created, their relationship, and intended testing purpose is > not extremely clear, especially for those unfamiliar with cxl-test. > > Document the purpose of each hierarchy. Add ASCII art to show the > relationship of devices. Group the device declarations together based > on the hierarchies. > > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Trivial nitpicks below :)
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> --- > tools/testing/cxl/test/cxl.c | 75 ++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 73 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c > index bf00dc52fe96..bd38a5fb60ae 100644 > --- a/tools/testing/cxl/test/cxl.c > +++ b/tools/testing/cxl/test/cxl.c > @@ -23,6 +23,31 @@ static int interleave_arithmetic; > #define NR_CXL_PORT_DECODERS 8 > #define NR_BRIDGES (NR_CXL_HOST_BRIDGES + NR_CXL_SINGLE_HOST + NR_CXL_RCH) > > +/* > + * Interleave testing
Doesn't include the cfmws, which will be tricky to draw, but maybe you could add something to indicate they interleave over the two HB sometimes?
> + * > + * +---------------+ +---------------+ > + * | host_bridge[0]| | host_bridge[1]| > + * +-/---------\---+ +--/---------\--+ Text for host bridges is right aligned. > + * /- -\ /- -\ > + * /- -\ /- -\ > + * +-------------+ +-------------+ +-------------+ +-------------+ > + * |root_port[0] | |root_port[1] | |root_port[2] | |root_port[3] | > + * +------|------+ +------|------+ +------|------+ +------|------+ and root ports are left aligned. I'd shrink both boxes so they are same as the switches below - or expand them to give a space on either side of the text.
> + * | | | | > + * +-------|-------+ +-------|-------+ +-------|-------+ +-------|-------+ > + * |switch_uport[0]| |switch_uport[1]| |switch_uport[2]| |switch_uport[3]| > + * +---|-------|---+ +---/-------|---+ +---/-------|---+ +---|-------\---+ > + * | \ / \ / \ / \ > + * +----|----++--|------++---------++----|----++---------++----|----++----|----++---------+ > + * |switch ||switch ||switch ||switch ||switch ||switch ||switch ||switch | > + * |_dport[0]||_dport[1]||_dport[2]||_dport[3]||_dport[4]||_dport[5]||_dport[6]||_dport[7]| > + * +----|----++--|------++----|----++----|----++----|----++----|----++----|----++----|----+ > + * | | | | | | | | > + * +---|--+ +-|----+ +---|--+ +---|--+ +--|---+ +---|--+ +---|--+ +---|--+ > + * |mem[0]| |mem[1]| |mem[2]| |mem[3]| |mem[4]| |mem[5]| |mem[6]| |mem[7]| > + * +------+ +------+ +------+ +------+ +------+ +------+ +------+ +------+ > + */ > static struct platform_device *cxl_acpi; > static struct platform_device *cxl_host_bridge[NR_CXL_HOST_BRIDGES]; > #define NR_MULTI_ROOT (NR_CXL_HOST_BRIDGES * NR_CXL_ROOT_PORTS) > @@ -31,16 +56,51 @@ static struct platform_device *cxl_switch_uport[NR_MULTI_ROOT]; > #define NR_MEM_MULTI \ > (NR_CXL_HOST_BRIDGES * NR_CXL_ROOT_PORTS * NR_CXL_SWITCH_PORTS) > static struct platform_device *cxl_switch_dport[NR_MEM_MULTI]; > +struct platform_device *cxl_mem[NR_MEM_MULTI]; > > +/* > + * 1) Preconfigured region support (Simulated BIOS configured region) > + * 2) 'Pass-through' decoder > + * > + * +---------------+ > + * | hb_single | > + * +------|--------+ > + * | > + * +------|--------+ > + * | root_single | > + * +------|--------+ > + * | > + * +----------|----------+ > + * | swu_single | > + * +-----|-----------|---+ > + * | | > + * +-----|-----+ +--|--------+ > + * |swd_single | | swd_single| > + * +-----|-----+ +----|------+ > + * | | > + * +------|-----+ +----|-------+ > + * |mem_single | |mem_single | > + * +------------+ +------------+ mem[0] etc? Also swd_single[0] etc?
For consistency with above.
> + */ > static struct platform_device *cxl_hb_single[NR_CXL_SINGLE_HOST]; > static struct platform_device *cxl_root_single[NR_CXL_SINGLE_HOST]; > static struct platform_device *cxl_swu_single[NR_CXL_SINGLE_HOST]; > #define NR_MEM_SINGLE (NR_CXL_SINGLE_HOST * NR_CXL_SWITCH_PORTS) > static struct platform_device *cxl_swd_single[NR_MEM_SINGLE]; > - > -struct platform_device *cxl_mem[NR_MEM_MULTI]; > struct platform_device *cxl_mem_single[NR_MEM_SINGLE]; > > +/* > + * 1) RCD > + * 2) Type-2 (Accelerator) > + * > + * +-----+ > + * | rch | > + * +--|--+ > + * | > + * +-|--+ > + * |rcd | > + * +----+ > + */ > static struct platform_device *cxl_rch[NR_CXL_RCH]; > static struct platform_device *cxl_rcd[NR_CXL_RCH]; > > @@ -64,6 +124,17 @@ static inline bool is_single_bridge(struct device *dev) > return false; > } > > +/* > + * +---------------+ +---------------+ > + * | host_bridge[0]| | host_bridge[1]| > + * +---------------+ +---------------+ > + * +---------------+ > + * | hb_single | (host_bridge[2]) > + * +---------------+ > + * +-----+ > + * | rch | (host_bridge[3]) > + * +-----+ > + */
Not sure what this diagram is illustrating...
> static struct acpi_device acpi0017_mock; > static struct acpi_device host_bridge[NR_BRIDGES] = { > [0] = { >
| |