Re: [RFC PATCH v2 09/28] cxl/acpi: Map single port host bridge component registers

[Dan Williams <dan.j.williams@intel.com>]

On Tue, Nov 2, 2021 at 2:16 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
>
> On 21-11-02 11:49:14, Dan Williams wrote:
> > On Tue, Nov 2, 2021 at 11:27 AM Ben Widawsky <ben.widawsky@intel.com> wrote:
> > >
> > > On 21-11-02 11:10:05, Dan Williams wrote:
> > > > On Tue, Nov 2, 2021 at 10:57 AM Ben Widawsky <ben.widawsky@intel.com> wrote:
> > > > >
> > > > > On 21-11-02 10:46:41, Dan Williams wrote:
> > > > > > On Tue, Nov 2, 2021 at 9:31 AM Ben Widawsky <ben.widawsky@intel.com> wrote:
> > > > > > [..]
> > > > > > > It seemed like a very simple thing to support given the port driver's existence
> > > > > > > so it was added to remove a TODO. However, I will drop it as you request.
> > > > > >
> > > > > > I'm honestly asking the question why this is needed and more
> > > > > > specifically why this is needed in this location?
> > > > > >
> > > > > > In other words, how is this case different than typical component
> > > > > > register probing that is done in the port driver itself? I would
> > > > > > expect the TODO just gets deleted by the port driver addition.
> > > > >
> > > > > Without this code, a decoder is added for the full address range regardless if
> > > > > there is an existing programmed HDM decoder. With this code when the port driver
> > > > > probes this host bridge HDM component registers, everything will be enumerated
> > > > > by the normal flow.
> > > > >
> > > > > This seemed easier than trying to have the port driver determine what cxl_acpi
> > > > > driver did and unwind that if there is a programmed decoder.
> > > >
> > > > I think this is more an argument to unify all decoder creation in the
> > > > port driver. When the port driver sees a single dport with no HDM
> > > > decoder registers it can create the passthrough decoder, if it sees
> > > > single dport with HDM decoder registers then it can create a
> > > > programmable decoder.
> > >
> > > That too, though I'm not sure how useful that will be for us.
> >
> > ?
> >
> > This is about the proper place to enumerate decoders. The proposal in
> > this patch is to do some part of the enumeration in cxl_acpi and some
> > part in cxl_port, I'm saying push it all to cxl_port. I.e. it was a
> > layering violation in retrospect to create a default passthrough
> > decoder from cxl_acpi.
>
> I think it makes sense to have cxl_decoder_add() not called from anything but
> the port driver. So long as there's a way to determine it's a CXL 2.0 Root Port,
> which, it seems we'll have, it should be straight forward.

Does it need to be root port aware? I expect any single ported switch
is treated the same, not that I expect single-ported switches to be a
real thing, but I don't expect our port driver would care.