Re: [PATCH 12/23] cxl: Introduce endpoint decoders

[Ben Widawsky <ben.widawsky@intel.com>]

On 21-11-29 12:07:00, Dan Williams wrote:
> On Mon, Nov 29, 2021 at 12:05 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
> >
> > On 21-11-24 16:07:23, Dan Williams wrote:
> > > On Mon, Nov 22, 2021 at 11:38 AM Ben Widawsky <ben.widawsky@intel.com> wrote:
> > > >
> > > > On 21-11-22 16:20:39, Jonathan Cameron wrote:
> > > > > On Fri, 19 Nov 2021 16:02:39 -0800
> > > > > Ben Widawsky <ben.widawsky@intel.com> 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@intel.com>
> > > > >
> > > > > 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.