Re: [PATCH v2 1/3] resource: Introduce a new helper resource_contains_addr()

[Alison Schofield <alison.schofield@intel.com>]

On Wed, Jul 02, 2025 at 11:16:36AM +0300, Andy Shevchenko wrote:
> On Wed, Jul 02, 2025 at 03:20:06PM +0800, Li Ming wrote:
> > In CXL subsystem, many functions need to check an address availability
> > by checking if the resource range contains the address. Providing a new
> > helper function resource_contains_addr() to check if the resource range
> > contains the input address.
> 
> resources are about ranges and not addresses. At bare minimum naming #####
> here. Also there is no symmetry with the intersection API. But I would argue
> to use resource_contains() and just provide necessary parameter with both
> start and end to be set at the same value.
> 
> 	struct resource r = DEFINE_RES...(addr);
> 
> 	if (resource_contains, res, &r)
> 		...do stuff...

Hi Andy,

Thanks for the review. Two alternative approaches were considered:

1) A CXL-only helper with direct comparison:
This duplicates range checking logic that's already common in the
resource API. We'd end up with CXL specific when the resource API
already provides the semantic framework for containment checks.

2) A CXL helper using temporary resource + resource_contains():
While this maintains API consistency, it's unnecessarily heavyweight.
Creating a temporary resource struct just to check if a single address
falls within a range feels like overengineering.
(This is your suggestion above.)

This patch proposes, a new helper in ioport.h, provides a clean, reusable
API that follows the existing resource_contains() pattern while being
optimized for the single address case. I guess I can agree with the lack
of symmetry with the existing API, but that's only if one thinks an API
cannot be extended. The new helper actually complements resource_contains()
nicely: one checks resource vs resource containment, the other checks
address vs resource containment.

Revisited the name, I can see how extending an existing name, like 
resource_contains() is a poor choice because then one might expect the
pair to be resource_contains_resource() and resource_contains_addr()
which it is not.

Do either of these fit better, or something else?
- resource_includes_addr() 
- resource_has_addr()

The new helper would immediately be used 7 times in the CXL subsystem.
What are you thinking the bar is for adding to ioport.h?

-- Alison

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>