On Fri, Jan 23, 2026 at 12:18:48AM +0200, Vladimir Oltean wrote:

On Thu, Jan 22, 2026 at 04:38:37PM +0200, Andy Shevchenko wrote:

quoted

On Thu, Jan 22, 2026 at 03:47:04PM +0200, Vladimir Oltean wrote:

quoted

On Thu, Jan 22, 2026 at 02:13:01PM +0200, Vladimir Oltean wrote:

...

quoted

quoted

quoted

quoted

quoted

+	unsigned int base;

Hmm... resource_size_t ?

quoted

quoted

Well, regmap_read() takes "unsigned int reg".
https://elixir.bootlin.com/linux/v6.18.6/source/include/linux/regmap.h#L1297
So in practice, a truncation will be done somewhere if the register base
exceeds unsigned int storage capacity. But I didn't feel that it's worth
handling that.

Would this address your feedback?

Yes and no. See my remarks below.

...

quoted

quoted

-	if (config->resource)
+	if (config->resource) {

Btw, this might be not enough, one should check size and flags as well
before use. There was a discussion about this recently. Maybe we should
just move to a simple unsigned int in the config for now? Because handling
resources maybe considered as over engineering in this case.

The resource flags are never taken into consideration, but I can for
sure replace the resource in struct mdio_regmap_config with just an
unsigned int start and an end, but that doesn't get rid of the resource
usage. The dev_get_resource(dev->parent, NULL) call is how we learn of
where our register window is located in the "one big regmap" provided by
the parent (SJA1105). So we still need this check somewhere else if we
wanted to not fail silently in case of address bits truncation.

Hmm... Bu why we can't embed the full struct resource in such a case?
Because resource should have a flag check, otherwise it's a wrong check.

Discussion I mentioned is this:
https://lore.kernel.org/lkml/20251207215359.28895-1-ansuelsmth@gmail.com/ (local)

Fixes due to that finding:
https://lore.kernel.org/lkml/20251208200437.14199-1-ansuelsmth@gmail.com/ (local)
https://lore.kernel.org/lkml/20251208145654.5294-1-ilpo.jarvinen@linux.intel.com/ (local)

quoted

quoted

+		if (config->resource->start > U32_MAX ||
+		    config->resource->end > U32_MAX) {

Ideally it should be resource_overlaps() check. But see above.

resource_overlaps_with_what? The only problem is that the resource can
exceed the 32 bit representation that regmap works with.

Obviously with the 4G address space :-)

	struct resource r4g = DEFINE_RESOURCE...(..., 0, SZ_4G...);

	if (resource_overlaps(&r4g, config->resource))
		aiaiai! // using %pR to print the content

quoted

quoted

+			dev_err(config->parent,
+				"Resource exceeds regmap API addressing possibilities\n");
+			return ERR_PTR(-EINVAL);
+		}
 		mr->base = config->resource->start;
+	}

-- 
With Best Regards,
Andy Shevchenko