Re: [PATCH v3] i2c: mux: reg: use device property accessors

From: Abdurrahman Hussain

Date: Mon May 18 2026 - 16:16:13 EST


On Mon May 18, 2026 at 6:59 AM PDT, Peter Rosin wrote:
>
> Hi!
>
> After reading v2/v3, I finally got why you removed if (!mux->data.reg).
> I.e., since you removed the io-remapping from i2c_mux_reg_probe_fw()
> that part can now be unconditional in i2c_mux_reg_probe(). But,
> keeping the if just to accomodate potential out-of-tree code that uses
> the platform data hooking is not needed so v1 feels better than v2/v3.
> Sorry for being dense. However, see below.
>

Agreed -- v4 will drop the guard and make
devm_platform_get_and_ioremap_resource() unconditional.

Follow-up question while we're pruning the platdata path: with the
guard gone, the .reg and .reg_size fields in
struct i2c_mux_reg_platform_data are functionally dead (any value a
platdata user supplied through them is overwritten by
devm_platform_get_and_ioremap_resource()). Should I drop those two
fields from the struct in the same patch, or leave them as silent
no-ops in the public header? Happy either way.

>> - adapter = of_find_i2c_adapter_by_node(adapter_np);
>> - of_node_put(adapter_np);
>> +
>> + adapter = i2c_get_adapter_by_fwnode(fwnode);
>> + fwnode_handle_put(fwnode);
>
> Why did you not change to i2c_find_adapter_by_fwnode()? It would be
> closer to the original, but maybe that doesn't work for ACPI for some
> reason?
>

It works for ACPI -- both _find and _get bottom out in
i2c_find_adapter_by_fwnode(). My reason for picking the _get
variant was simply that it does try_module_get(adapter->owner) on
top. But i2c_get_adapter() further down in probe() already does
the same try_module_get() and -EPROBE_DEFERs on failure, so the
extra acquire-then-immediately-release in probe_fw() isn't earning
us anything. v4 will switch to i2c_find_adapter_by_fwnode() +
put_device(), matching the OF original more closely.

>> - /* map address from "reg" if exists */
>> - if (of_address_to_resource(np, 0, &res) == 0) {
>> - mux->data.reg_size = resource_size(&res);
>> - mux->data.reg = devm_ioremap_resource(&pdev->dev, &res);
>> - if (IS_ERR(mux->data.reg))
>> - return PTR_ERR(mux->data.reg);
>> - }
>> -
>
> By not doing the io-remapping here, the ordering is now subtly altered.
> Previously we had io-remapping before the call to i2c_get_adapter().
> With this change we instead have the call to i2c_get_adapter() before
> the io-remapping.
>
> Since it is apparently difficult the get this patch tested for the
> existing use cases, we should avoid as many of these innocent looking
> but subtle changes as possible.
>
> Cheers,
> Peter
>

Will fix that in v4 by reordering probe() so the
devm_platform_get_and_ioremap_resource() call comes before
i2c_get_adapter(), matching the master sequence. No need to push
the ioremap back into probe_fw() -- the helper signature stays
cleaner if probe() owns the call.

Thanks for the careful re-read.

Cheers,
Abdurrahman