On Fri, Sep 3, 2021 at 5:59 AM Jonathan Cameron
<[email protected]> wrote:
>
> On Tue, 24 Aug 2021 09:07:39 -0700
> Dan Williams <[email protected]> wrote:
>
> > As found by cxl_test, the implementation populated the target_list for
> > the single dport exceptional case, it missed populating the target_list
> > for the typical multi-dport case.
>
> Description makes this sound like a fix, rather than what I think it is
> which is implementing a new feature...
It is finishing a feature where the unfinished state is broken. It
should never be the case that target_list_show() returns nothing.
[..]
> > diff --git a/drivers/cxl/core/bus.c b/drivers/cxl/core/bus.c
> > index 8073354ba232..9a755a37eadf 100644
> > --- a/drivers/cxl/core/bus.c
> > +++ b/drivers/cxl/core/bus.c
[..]
> > @@ -493,10 +494,19 @@ cxl_decoder_alloc(struct cxl_port *port, int
> > nr_targets, resource_size_t base,
> > .target_type = type,
> > };
> >
> > - /* handle implied target_list */
> > - if (interleave_ways == 1)
> > - cxld->target[0] =
> > - list_first_entry(&port->dports, struct cxl_dport,
> > list);
> > + device_lock(&port->dev);
> > + for (i = 0; target_map && i < nr_targets; i++) {
>
> Perhaps move target map check much earlier rather than putting it
> int he loop condition? I don't think the loop is modifying it...
The loop is not modifying target_map, but target_map is allowed to be
NULL. I was trying to avoid a non-error goto, but a better way to
solve that would be to make the loop a helper function taken under the
lock.
