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

On 21-11-29 12:07:00, Dan Williams wrote:

On Mon, Nov 29, 2021 at 12:05 PM Ben Widawsky [off-list ref] wrote:

quoted

On 21-11-24 16:07:23, Dan Williams wrote:

quoted

On Mon, Nov 22, 2021 at 11:38 AM Ben Widawsky [off-list ref] wrote:

quoted

On 21-11-22 16:20:39, Jonathan Cameron wrote:

quoted

On Fri, 19 Nov 2021 16:02:39 -0800
Ben Widawsky [off-list ref] wrote:

quoted

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 <redacted>

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.