| Message ID | 80f80f0d26e73cd6941d8530163a4bbd731d50ec.1697433770.git.alison.schofield@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl/region: Autodiscovery position repair | expand |
> On Oct 15, 2023, at 11:02 PM, alison.schofield@intel.com wrote: > > From: Alison Schofield <alison.schofield@intel.com> > > Introduce a calculation that determines a targets position in a region > interleave. Perform a selftest of the calculation on user-defined > regions. > > The region driver uses the kernel sort() function to put region > targets in relative order. Positions are assigned based on each > targets index in that sorted list. That relative sort doesn't > consider the offset of a port into its parent port which causes > some auto-discovered regions to fail creation. In one failure case, > a 2 + 2 config (2 host bridges each with 2 endpoints), the sort > puts all the targets of one port ahead of another port when they > were expected to be interleaved. > > In preparation for repairing the autodiscovery region assembly, > introduce a new method for discovering a target position in the > region interleave. > > cxl_interleave_pos() offers a method to determine a targets position > by ascending from an endpoint to a root decoder. The calculation starts > with the endpoints local position and its position in its parents port. > It traverses towards the root decoder and examines both position and > ways in order to allow the position to be refined all the way to the > root decoder. > > This calculation, applied iteratively, yields the correct position: > > position = position * parent_ways + parent_pos; > > ...with these rules: > > Rule #1 - When (parent_ways == region_ways), Stop! > position = parent_position; > This rule is applied in calc_interleave_pos() > > Rule #2 - Skip over siblings that come before this memdev in > the decoder list when searching for the parent position. > This rule is applied in the helper find_pos_and_ways(). > > Include a selftest that exercises this new position calculation against > every successfully configured user-defined region. > > Fixes: a32320b71f08 ("cxl/region: Add region autodiscovery") > Reported-by: Dmytro Adamenko <dmytro.adamenko@intel.com> > Signed-off-by: Alison Schofield <alison.schofield@intel.com> > --- > drivers/cxl/core/region.c | 102 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 102 insertions(+) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 64206fc4d99b..b451d215c3c5 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -1500,6 +1500,93 @@ static int match_switch_decoder_by_range(struct device *dev, void *data) > return range_contains(r1, r2); > } > > +/* Find the position of a port in it's parent and the parents ways */ > +static int find_pos_and_ways(struct cxl_port *port, struct range *range, > + int *pos, int *ways) > +{ > + struct cxl_switch_decoder *cxlsd; > + struct cxl_port *parent; > + int child_ways = *ways; > + int child_pos = *pos; > + struct device *dev; > + int skip = 0; > + int rc = -1; > + > + parent = next_port(port); > + if (!parent) > + return rc; > + > + dev = device_find_child(&parent->dev, range, > + match_switch_decoder_by_range); > + if (!dev) { > + dev_err(port->uport_dev, > + "failed to find decoder mapping %#llx-%#llx\n", > + range->start, range->end); > + return rc; > + } > + cxlsd = to_cxl_switch_decoder(dev); > + *ways = cxlsd->cxld.interleave_ways; > + > + /* Skip over this many siblings in the target list */ > + if (*ways > child_ways) > + skip = child_pos; > + > + for (int i = 0; i < *ways; i++) { > + if (cxlsd->target[i] == port->parent_dport) { > + if (skip--) > + continue; > + *pos = i; > + rc = 0; > + break; > + } > + } > + put_device(dev); > + > + return rc; > +} > + > +static int calc_interleave_pos(struct cxl_endpoint_decoder *cxled, > + int region_ways) > +{ > + struct cxl_port *iter, *port = cxled_to_port(cxled); > + struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); > + struct range *range = &cxled->cxld.hpa_range; > + int parent_ways = 0; > + int parent_pos = 0; > + int rc, pos; > + > + /* Initialize pos to its local position */ > + rc = find_pos_and_ways(port, range, &parent_pos, &parent_ways); > + if (rc) > + return -ENXIO; > + > + pos = parent_pos; > + > + if (parent_ways == region_ways) > + goto out; > + > + /* Iterate up the ancestral tree refining the position */ > + for (iter = next_port(port); iter; iter = next_port(iter)) { I think this can be simplified by initializing with ‘iter = port’ here, and then the earlier lines can be removed, as well as the ‘out’ label. > + if (is_cxl_root(iter)) > + break; > + > + rc = find_pos_and_ways(iter, range, &parent_pos, &parent_ways); > + if (rc) > + return -ENXIO; > + > + if (parent_ways == region_ways) { > + pos = parent_pos; > + break; > + } > + pos = pos * parent_ways + parent_pos; > + } > +out: > + dev_dbg(&cxlmd->dev, > + "decoder:%s parent:%s port:%s range:%#llx-%#llx pos:%d\n", > + dev_name(&cxled->cxld.dev), dev_name(cxlmd->dev.parent), > + dev_name(&port->dev), range->start, range->end, pos); > + > + return pos; > } > > static void find_positions(const struct cxl_switch_decoder *cxlsd, > @@ -1765,6 +1852,21 @@ static int cxl_region_attach(struct cxl_region *cxlr, > .end = p->res->end, > }; > > + if (p->nr_targets != p->interleave_ways) > + return 0; There is code just before this that compares nr_targets and interleave_ways before calling cxl_region_setup_targets(). Decoder fields are set between that comparison and here, but I see no reason these fields shouldn’t be set before that earlier comparison. It actually makes the code more consistent, otherwise in the case where cxl_region_setup_targets() fails, the first n-1 decoders have their fields set, but the last one would not. Then this return 0 can just be an else clause on the earlier comparison. > + > + /* Exercise position calculator on user-defined regions */ > + for (int i = 0; i < p->nr_targets; i++) { > + struct cxl_endpoint_decoder *cxled = p->targets[i]; > + int test_pos; > + > + test_pos = calc_interleave_pos(cxled, p->interleave_ways); > + dev_dbg(&cxled->cxld.dev, > + "Interleave calc match %s test_pos:%d cxled->pos:%d\n", > + (test_pos == cxled->pos) ? "Success" : "Fail", > + test_pos, cxled->pos); > + } > + > return 0; > > err_decrement: > -- > 2.37.3 > >
On Tue, Oct 17, 2023 at 05:33:54PM +0000, Jim Harris wrote: > > > > On Oct 15, 2023, at 11:02 PM, alison.schofield@intel.com wrote: > > > > From: Alison Schofield <alison.schofield@intel.com> > > > > Introduce a calculation that determines a targets position in a region > > interleave. Perform a selftest of the calculation on user-defined > > regions. > > > > The region driver uses the kernel sort() function to put region > > targets in relative order. Positions are assigned based on each > > targets index in that sorted list. That relative sort doesn't > > consider the offset of a port into its parent port which causes > > some auto-discovered regions to fail creation. In one failure case, > > a 2 + 2 config (2 host bridges each with 2 endpoints), the sort > > puts all the targets of one port ahead of another port when they > > were expected to be interleaved. > > > > In preparation for repairing the autodiscovery region assembly, > > introduce a new method for discovering a target position in the > > region interleave. > > > > cxl_interleave_pos() offers a method to determine a targets position > > by ascending from an endpoint to a root decoder. The calculation starts > > with the endpoints local position and its position in its parents port. > > It traverses towards the root decoder and examines both position and > > ways in order to allow the position to be refined all the way to the > > root decoder. > > > > This calculation, applied iteratively, yields the correct position: > > > > position = position * parent_ways + parent_pos; > > > > ...with these rules: > > > > Rule #1 - When (parent_ways == region_ways), Stop! > > position = parent_position; > > This rule is applied in calc_interleave_pos() > > > > Rule #2 - Skip over siblings that come before this memdev in > > the decoder list when searching for the parent position. > > This rule is applied in the helper find_pos_and_ways(). > > > > Include a selftest that exercises this new position calculation against > > every successfully configured user-defined region. > > > > Fixes: a32320b71f08 ("cxl/region: Add region autodiscovery") > > Reported-by: Dmytro Adamenko <dmytro.adamenko@intel.com> > > Signed-off-by: Alison Schofield <alison.schofield@intel.com> > > --- > > drivers/cxl/core/region.c | 102 ++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 102 insertions(+) > > > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > > index 64206fc4d99b..b451d215c3c5 100644 > > --- a/drivers/cxl/core/region.c > > +++ b/drivers/cxl/core/region.c > > @@ -1500,6 +1500,93 @@ static int match_switch_decoder_by_range(struct device *dev, void *data) > > return range_contains(r1, r2); > > } > > > > +/* Find the position of a port in it's parent and the parents ways */ > > +static int find_pos_and_ways(struct cxl_port *port, struct range *range, > > + int *pos, int *ways) > > +{ > > + struct cxl_switch_decoder *cxlsd; > > + struct cxl_port *parent; > > + int child_ways = *ways; > > + int child_pos = *pos; > > + struct device *dev; > > + int skip = 0; > > + int rc = -1; > > + > > + parent = next_port(port); > > + if (!parent) > > + return rc; > > + > > + dev = device_find_child(&parent->dev, range, > > + match_switch_decoder_by_range); > > + if (!dev) { > > + dev_err(port->uport_dev, > > + "failed to find decoder mapping %#llx-%#llx\n", > > + range->start, range->end); > > + return rc; > > + } > > + cxlsd = to_cxl_switch_decoder(dev); > > + *ways = cxlsd->cxld.interleave_ways; > > + > > + /* Skip over this many siblings in the target list */ > > + if (*ways > child_ways) > > + skip = child_pos; > > + > > + for (int i = 0; i < *ways; i++) { > > + if (cxlsd->target[i] == port->parent_dport) { > > + if (skip--) > > + continue; > > + *pos = i; > > + rc = 0; > > + break; > > + } > > + } > > + put_device(dev); > > + > > + return rc; > > +} > > + > > +static int calc_interleave_pos(struct cxl_endpoint_decoder *cxled, > > + int region_ways) > > +{ > > + struct cxl_port *iter, *port = cxled_to_port(cxled); > > + struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); > > + struct range *range = &cxled->cxld.hpa_range; > > + int parent_ways = 0; > > + int parent_pos = 0; > > + int rc, pos; > > + > > + /* Initialize pos to its local position */ > > + rc = find_pos_and_ways(port, range, &parent_pos, &parent_ways); > > + if (rc) > > + return -ENXIO; > > + > > + pos = parent_pos; > > + > > + if (parent_ways == region_ways) > > + goto out; > > + > > + /* Iterate up the ancestral tree refining the position */ > > + for (iter = next_port(port); iter; iter = next_port(iter)) { > > I think this can be simplified by initializing with ‘iter = port’ > here, and then the earlier lines can be removed, as well as the > ‘out’ label. Thanks for drawing my attn to this again. I had tried and failed to do that previously, but in trying again, I must not have init'd pos 'pos' to 0. I've made this change now and it's passing. Much nicer. Thanks! > > > + if (is_cxl_root(iter)) > > + break; > > + > > + rc = find_pos_and_ways(iter, range, &parent_pos, &parent_ways); > > + if (rc) > > + return -ENXIO; > > + > > + if (parent_ways == region_ways) { > > + pos = parent_pos; > > + break; > > + } > > + pos = pos * parent_ways + parent_pos; > > + } > > +out: > > + dev_dbg(&cxlmd->dev, > > + "decoder:%s parent:%s port:%s range:%#llx-%#llx pos:%d\n", > > + dev_name(&cxled->cxld.dev), dev_name(cxlmd->dev.parent), > > + dev_name(&port->dev), range->start, range->end, pos); > > + > > + return pos; > > } > > > > static void find_positions(const struct cxl_switch_decoder *cxlsd, > > @@ -1765,6 +1852,21 @@ static int cxl_region_attach(struct cxl_region *cxlr, > > .end = p->res->end, > > }; > > > > + if (p->nr_targets != p->interleave_ways) > > + return 0; > > There is code just before this that compares nr_targets and interleave_ways > before calling cxl_region_setup_targets(). > > Decoder fields are set between that comparison and here, but I see no reason > these fields shouldn’t be set before that earlier comparison. It actually > makes the code more consistent, otherwise in the case where > cxl_region_setup_targets() fails, the first n-1 decoders have their fields > set, but the last one would not. > > Then this return 0 can just be an else clause on the earlier comparison. > I see what you mean and my first thought was 'sure this could be a little cleanup before this patch', but then I struggle to justify it. Those first n-1 decoders have iw,ig,hpa fields set because we had hope that this region would create. On the last decoder, is there any value in setting up the decoder when we know it's a failed setup? Alison > > + > > + /* Exercise position calculator on user-defined regions */ > > + for (int i = 0; i < p->nr_targets; i++) { > > + struct cxl_endpoint_decoder *cxled = p->targets[i]; > > + int test_pos; > > + > > + test_pos = calc_interleave_pos(cxled, p->interleave_ways); > > + dev_dbg(&cxled->cxld.dev, > > + "Interleave calc match %s test_pos:%d cxled->pos:%d\n", > > + (test_pos == cxled->pos) ? "Success" : "Fail", > > + test_pos, cxled->pos); > > + } > > + > > return 0; > > > > err_decrement: > > -- > > 2.37.3 > > > > >
On Mon, Oct 23, 2023 at 11:10:29AM -0700, Alison Schofield wrote: > On Tue, Oct 17, 2023 at 05:33:54PM +0000, Jim Harris wrote: > > > > > On Oct 15, 2023, at 11:02 PM, alison.schofield@intel.com wrote: > > > > > > + if (p->nr_targets != p->interleave_ways) > > > + return 0; > > > > There is code just before this that compares nr_targets and interleave_ways > > before calling cxl_region_setup_targets(). > > > > Decoder fields are set between that comparison and here, but I see no reason > > these fields shouldn’t be set before that earlier comparison. It actually > > makes the code more consistent, otherwise in the case where > > cxl_region_setup_targets() fails, the first n-1 decoders have their fields > > set, but the last one would not. > > > > Then this return 0 can just be an else clause on the earlier comparison. > > > > I see what you mean and my first thought was 'sure this could be a little > cleanup before this patch', but then I struggle to justify it. Those first > n-1 decoders have iw,ig,hpa fields set because we had hope that this region > would create. On the last decoder, is there any value in setting up the > decoder when we know it's a failed setup? > > I have a patch outstanding in this same area, which may provide some additional context. https://lore.kernel.org/linux-cxl/169703589120.1202031.14696100866518083806.stgit@bgt-140510-bm03.eng.stellus.in/ I think the value is that the all of the decoders get treated the same. Note that before the nr_targets == interleave_ways check, it sets cxled->pos, as well as put the decoder in the targets array and increments the nr_targets. But ultimately my suggested changes probably belong more in my patch (or a follow-up to my patch), so I think it's fine to skip my feedback on this part. Jim
alison.schofield@ wrote: > From: Alison Schofield <alison.schofield@intel.com> > > Introduce a calculation that determines a targets position in a region > interleave. Perform a selftest of the calculation on user-defined > regions. > > The region driver uses the kernel sort() function to put region > targets in relative order. Positions are assigned based on each > targets index in that sorted list. That relative sort doesn't > consider the offset of a port into its parent port which causes > some auto-discovered regions to fail creation. In one failure case, > a 2 + 2 config (2 host bridges each with 2 endpoints), the sort > puts all the targets of one port ahead of another port when they > were expected to be interleaved. > > In preparation for repairing the autodiscovery region assembly, > introduce a new method for discovering a target position in the > region interleave. > > cxl_interleave_pos() offers a method to determine a targets position > by ascending from an endpoint to a root decoder. The calculation starts > with the endpoints local position and its position in its parents port. > It traverses towards the root decoder and examines both position and > ways in order to allow the position to be refined all the way to the > root decoder. > > This calculation, applied iteratively, yields the correct position: > > position = position * parent_ways + parent_pos; > > ...with these rules: > > Rule #1 - When (parent_ways == region_ways), Stop! > position = parent_position; > This rule is applied in calc_interleave_pos() > > Rule #2 - Skip over siblings that come before this memdev in > the decoder list when searching for the parent position. > This rule is applied in the helper find_pos_and_ways(). I feel like calc_interleave_pos() is missing some kernel-doc about these rules. It seems fundamental to maintaining this code going forward. This short summary here is sufficient for the changelog, but calc_interleave_pos() wants a few sentences on the theory of operation. > > Include a selftest that exercises this new position calculation against > every successfully configured user-defined region. > > Fixes: a32320b71f08 ("cxl/region: Add region autodiscovery") Again, this change is not a fix, it's a new diagnostic. It is a dependency for a fix, but that discussion will come out around backporting patch3. > Reported-by: Dmytro Adamenko <dmytro.adamenko@intel.com> > Signed-off-by: Alison Schofield <alison.schofield@intel.com> > --- > drivers/cxl/core/region.c | 102 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 102 insertions(+) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 64206fc4d99b..b451d215c3c5 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -1500,6 +1500,93 @@ static int match_switch_decoder_by_range(struct device *dev, void *data) > return range_contains(r1, r2); > } > > +/* Find the position of a port in it's parent and the parents ways */ > +static int find_pos_and_ways(struct cxl_port *port, struct range *range, > + int *pos, int *ways) > +{ > + struct cxl_switch_decoder *cxlsd; > + struct cxl_port *parent; > + int child_ways = *ways; > + int child_pos = *pos; > + struct device *dev; > + int skip = 0; > + int rc = -1; On the risk of this error code leaking higher, I think it should be initialized to -ENXIO directly, and not translated by the caller. > + > + parent = next_port(port); > + if (!parent) > + return rc; > + > + dev = device_find_child(&parent->dev, range, > + match_switch_decoder_by_range); > + if (!dev) { > + dev_err(port->uport_dev, > + "failed to find decoder mapping %#llx-%#llx\n", > + range->start, range->end); > + return rc; > + } > + cxlsd = to_cxl_switch_decoder(dev); > + *ways = cxlsd->cxld.interleave_ways; > + > + /* Skip over this many siblings in the target list */ > + if (*ways > child_ways) > + skip = child_pos; Maybe a clarification that "Since the switch target list is by definition sorted in region position order, siblings to skip are always at lower indices." > + > + for (int i = 0; i < *ways; i++) { > + if (cxlsd->target[i] == port->parent_dport) { > + if (skip--) > + continue; > + *pos = i; > + rc = 0; > + break; > + } > + } > + put_device(dev); > + > + return rc; > +} > + > +static int calc_interleave_pos(struct cxl_endpoint_decoder *cxled, > + int region_ways) > +{ > + struct cxl_port *iter, *port = cxled_to_port(cxled); > + struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); > + struct range *range = &cxled->cxld.hpa_range; > + int parent_ways = 0; > + int parent_pos = 0; > + int rc, pos; > + > + /* Initialize pos to its local position */ > + rc = find_pos_and_ways(port, range, &parent_pos, &parent_ways); > + if (rc) > + return -ENXIO; per above, this can become "return rc;". I had wondered if this code becomes easier to read to have separate inputs and outputs versus dual purpose the @pos and @ways paramters, but I can't come up with anything simpler. I think a bit of kernel-doc would help with the next casual reader that comes along and wonders about the theory of operation. > + > + pos = parent_pos; > + > + if (parent_ways == region_ways) > + goto out; > + > + /* Iterate up the ancestral tree refining the position */ > + for (iter = next_port(port); iter; iter = next_port(iter)) { > + if (is_cxl_root(iter)) > + break; > + > + rc = find_pos_and_ways(iter, range, &parent_pos, &parent_ways); > + if (rc) > + return -ENXIO; > + > + if (parent_ways == region_ways) { > + pos = parent_pos; > + break; > + } > + pos = pos * parent_ways + parent_pos; Nice simplification of the current mess! > + } > +out: > + dev_dbg(&cxlmd->dev, > + "decoder:%s parent:%s port:%s range:%#llx-%#llx pos:%d\n", > + dev_name(&cxled->cxld.dev), dev_name(cxlmd->dev.parent), > + dev_name(&port->dev), range->start, range->end, pos); > + > + return pos; > } > > static void find_positions(const struct cxl_switch_decoder *cxlsd, > @@ -1765,6 +1852,21 @@ static int cxl_region_attach(struct cxl_region *cxlr, > .end = p->res->end, > }; > > + if (p->nr_targets != p->interleave_ways) > + return 0; > + > + /* Exercise position calculator on user-defined regions */ > + for (int i = 0; i < p->nr_targets; i++) { > + struct cxl_endpoint_decoder *cxled = p->targets[i]; > + int test_pos; > + > + test_pos = calc_interleave_pos(cxled, p->interleave_ways); > + dev_dbg(&cxled->cxld.dev, > + "Interleave calc match %s test_pos:%d cxled->pos:%d\n", > + (test_pos == cxled->pos) ? "Success" : "Fail", > + test_pos, cxled->pos); Part of me wondered if the failure case should be louder here, but in the case of autodiscovery there is no position to compare against.
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index 64206fc4d99b..b451d215c3c5 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -1500,6 +1500,93 @@ static int match_switch_decoder_by_range(struct device *dev, void *data) return range_contains(r1, r2); } +/* Find the position of a port in it's parent and the parents ways */ +static int find_pos_and_ways(struct cxl_port *port, struct range *range, + int *pos, int *ways) +{ + struct cxl_switch_decoder *cxlsd; + struct cxl_port *parent; + int child_ways = *ways; + int child_pos = *pos; + struct device *dev; + int skip = 0; + int rc = -1; + + parent = next_port(port); + if (!parent) + return rc; + + dev = device_find_child(&parent->dev, range, + match_switch_decoder_by_range); + if (!dev) { + dev_err(port->uport_dev, + "failed to find decoder mapping %#llx-%#llx\n", + range->start, range->end); + return rc; + } + cxlsd = to_cxl_switch_decoder(dev); + *ways = cxlsd->cxld.interleave_ways; + + /* Skip over this many siblings in the target list */ + if (*ways > child_ways) + skip = child_pos; + + for (int i = 0; i < *ways; i++) { + if (cxlsd->target[i] == port->parent_dport) { + if (skip--) + continue; + *pos = i; + rc = 0; + break; + } + } + put_device(dev); + + return rc; +} + +static int calc_interleave_pos(struct cxl_endpoint_decoder *cxled, + int region_ways) +{ + struct cxl_port *iter, *port = cxled_to_port(cxled); + struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); + struct range *range = &cxled->cxld.hpa_range; + int parent_ways = 0; + int parent_pos = 0; + int rc, pos; + + /* Initialize pos to its local position */ + rc = find_pos_and_ways(port, range, &parent_pos, &parent_ways); + if (rc) + return -ENXIO; + + pos = parent_pos; + + if (parent_ways == region_ways) + goto out; + + /* Iterate up the ancestral tree refining the position */ + for (iter = next_port(port); iter; iter = next_port(iter)) { + if (is_cxl_root(iter)) + break; + + rc = find_pos_and_ways(iter, range, &parent_pos, &parent_ways); + if (rc) + return -ENXIO; + + if (parent_ways == region_ways) { + pos = parent_pos; + break; + } + pos = pos * parent_ways + parent_pos; + } +out: + dev_dbg(&cxlmd->dev, + "decoder:%s parent:%s port:%s range:%#llx-%#llx pos:%d\n", + dev_name(&cxled->cxld.dev), dev_name(cxlmd->dev.parent), + dev_name(&port->dev), range->start, range->end, pos); + + return pos; } static void find_positions(const struct cxl_switch_decoder *cxlsd, @@ -1765,6 +1852,21 @@ static int cxl_region_attach(struct cxl_region *cxlr, .end = p->res->end, }; + if (p->nr_targets != p->interleave_ways) + return 0; + + /* Exercise position calculator on user-defined regions */ + for (int i = 0; i < p->nr_targets; i++) { + struct cxl_endpoint_decoder *cxled = p->targets[i]; + int test_pos; + + test_pos = calc_interleave_pos(cxled, p->interleave_ways); + dev_dbg(&cxled->cxld.dev, + "Interleave calc match %s test_pos:%d cxled->pos:%d\n", + (test_pos == cxled->pos) ? "Success" : "Fail", + test_pos, cxled->pos); + } + return 0; err_decrement: