Re: [RFC PATCH v2 05/28] cxl/core: Convert decoder range to resource

From: Ben Widawsky <ben.widawsky@intel.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: linux-cxl@vger.kernel.org,
	Chet Douglas <chet.r.douglas@intel.com>,
	Alison Schofield <alison.schofield@intel.com>,
	Ira Weiny <ira.weiny@intel.com>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Vishal Verma <vishal.l.verma@intel.com>
Subject: Re: [RFC PATCH v2 05/28] cxl/core: Convert decoder range to resource
Date: Mon, 1 Nov 2021 07:33:07 -0700	[thread overview]
Message-ID: <20211101143307.iqe4gle7gbs3vwcv@intel.com> (raw)
In-Reply-To: <20211029223751.qb7heeezdfqfhnt2@intel.com>

On 21-10-29 15:37:51, Ben Widawsky wrote:
> On 21-10-29 15:22:19, Dan Williams wrote:
> > On Fri, Oct 29, 2021 at 2:27 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
> > [..]
> > > > Note, this is just how I had it roughly mapped out in my head, I defer
> > > > making it a hard recommendation until I get deeper into this set to
> > > > see if we diverge.
> > > >
> > >
> > > You're correct.
> > >
> > > I had changed this already for my v3. I ended up with this:
> > > +       union {
> > > +               struct resource cfmws_res;
> > 
> > "cfmws" is something that will only appear on ACPI based CXL systems,
> > so if it needs a prefix it should be something that does not preclude
> > CXL on other platform firmware architectures. So, "platform_" or
> > "root_"?
> 
> Sure. Platform_ is better.
> 

Coming back to this. While "CFMWS" is indeed defined as part of ACPI, I do think
it could also be a generic name for what it represents. In other words, I don't
see any issue with using CFMWS, but, I'll start using "platform" since it seems
it's your preference (and I prefer that to "root").

> > 
> > > +               struct resource *decoder_res;
> > 
> > So the region device is going to be doing the __request_region() from
> > @cfmws_res, not decoders, so why is this a resource and not a range?
> 
> Well, sort of. Most of v3 was to ultimately go with the decoders doing the
> __request_region, although thinking about this further that will not work since
> multiple decoders might want the same ranges of the parent resource.
> 
> So I guess it has to be region. I can go back to renaming it res in that case.
> You're correct that the region device should own doing the __request_region.