On 21-11-29 12:07:00, Dan Williams wrote: > On Mon, Nov 29, 2021 at 12:05 PM Ben Widawsky <ben.widawsky@xxxxxxxxx> wrote: > > > > On 21-11-24 16:07:23, Dan Williams wrote: > > > On Mon, Nov 22, 2021 at 11:38 AM Ben Widawsky <ben.widawsky@xxxxxxxxx> wrote: > > > > > > > > On 21-11-22 16:20:39, Jonathan Cameron wrote: > > > > > On Fri, 19 Nov 2021 16:02:39 -0800 > > > > > Ben Widawsky <ben.widawsky@xxxxxxxxx> wrote: > > > > > > > > > > > Endpoints have decoders too. It is useful to share the same > > > > > > infrastructure from cxl_core. Endpoints do not have dports (downstream > > > > > > targets), only the underlying physical medium. As a result, some special > > > > > > casing is needed. > > > > > > > > > > > > There is no functional change introduced yet as endpoints don't actually > > > > > > enumerate decoders yet. > > > > > > > > > > > > Signed-off-by: Ben Widawsky <ben.widawsky@xxxxxxxxx> > > > > > > > > > > I'm not a fan of special values like using 0 here to indicate endpoint > > > > > device. I'd rather see a base cxl_decode_alloc(..., bool ep) > > > > > and possibly wrappers for the non ep case and ep one. > > > > > > > > > > Jonathan > > > > > > > > > > > > > My inclination is the opposite. However, I think you and Dan both brought up > > > > something to this effect in the previous RFCs. > > > > > > > > Dan, do you have a preference here? > > > > > > I was thinking something along the lines of what Jonathan wants, > > > explicit per-type APIs, but internal / private to the core can use > > > heuristics like nr_targets == 0 == endpoint. > > > > > > So unexport cxl_decoder_alloc() and have separate: > > > > > > cxl_root_decoder_alloc() > > > cxl_switch_decoder_alloc() > > > cxl_endpoint_decoder_alloc() > > > > > > ...apis that use a static cxl_decoder_alloc() internally. Probably > > > also wants a cxl_endpoint_decoder_add() that drops the need to pass a > > > NULL @target_map. > > > > Would you a like a prep patch to set up the APIs first, or just do it all in > > one? > > Prep patch to switch over the current usages to the new style before > introducing more helpers sounds good to me. Thanks for the suggestions... Looking again, I think it makes sense to squash it into the patch before this which documents and tightens up this exact API.
