On Fri, 3 Sep 2021 15:43:25 -0700 Dan Williams <[email protected]> wrote:
> 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. Ah. Understood. I was not appreciating that check need to be under the device_lock(). Helper function would indeed make this clean - good plan. Thanks, Jonathan
