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

[Abdurrahman Hussain]

On Tue May 12, 2026 at 12:50 AM PDT, Peter Rosin wrote:
> [note change of email adress, I'm now using peda@xxxxxxxxxxxxxx]
>
> On Thu Feb 26, 2026 at 10:28 PM PST, Abdurrahman Hussain wrote:
>> Hi Andy, Peter,
>> 
>> It looks like the patch is marked as "Changes Requested". I haven't
>> heard any feedback lately. Did I miss something?
>
> Hi!
>
> First of all, as usual I'm sorry for being slow to respond.

Hi Peter!

>
> One specific thing I noticed with your patch that is a potential
> problem is that it replaces the open coded little-ending/big-endian
> logic with a call to device_is_big_endian() which is not an exact
> replacement. The logic provided by device_is_big_endian() is simply
> not equivalent. Further, the new behavior does not match the device
> tree bindings.
>
> At a minimum, you need a matching patch for the binding before doing
> something like this. But that would of course only expose the risk
> for regressions.
>
> Since there are apparently few who can test this driver, and even
> fewer with endian issues, I'm reluctant to make these changes. I
> would prefer to keep the endian logic exactly as it is.
>

You are right. Reviewing the helper:

  static inline bool fwnode_device_is_big_endian(const struct fwnode_handle *fwnode)
  {
          if (fwnode_property_present(fwnode, "big-endian"))
                  return true;
          if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) &&
              fwnode_property_present(fwnode, "native-endian"))
                  return true;
          return false;
  }

it does not consult "little-endian" at all, and on a big-endian host
with no endian property in DT it returns false (so the patch would
flip mux->data.little_endian from false to true, silently). That is
a real behavioral change against existing users and not something
this patch should be doing.

I will keep the original three-way logic in v2 and only switch the
accessors:

  if (device_property_read_bool(dev, "little-endian"))
          mux->data.little_endian = true;
  else if (device_property_read_bool(dev, "big-endian"))
          mux->data.little_endian = false;
  else
          mux->data.little_endian = IS_ENABLED(CONFIG_CPU_LITTLE_ENDIAN);

That preserves the documented binding behaviour byte-for-byte and
avoids touching device_is_big_endian().

> I have not looked all that closely, but you also make the call to
> devm_platform_get_and_ioremap_resource() unconditional. Why will
> that change not cause regressions?
>

You are right again — that was an oversight. The original code
guarded the platform-resource fallback with if (!mux->data.reg),
which preserved the pre-mapped reg that a platdata user may have
supplied through struct i2c_mux_reg_platform_data. Dropping that
guard would clobber the platdata pointer with whatever
platform_get_resource() returns (or fail outright if there is no
MEM resource on the platform device). I will restore the guard in
v2.

> Your commit message is lacking, you make these changes but only
> talk about extending support to non-OF platforms as if that is
> the only thing that is going on.
>

Fair point. In v2 the changelog will spell out:

- the conversion from of_* to device_property_* / fwnode_* accessors
and what each replacement is equivalent to,
- the new ACPI child-node handling via acpi_get_local_address(),
- that the endian and platform-resource fallback paths are
preserved byte-for-byte.

> And, the initail problem still stands. I would like to see some
> tags that indicate that the driver still works for existing users.
> Especially so since there are changes that look problematic.
>
> Cheers,
> Peter

Understood. I do not have any of the existing in-tree users of
i2c-mux-reg on hand (the DT consumers are on PowerPC and SH
boards, none of which I have access to), so I can only test the
ACPI/non-OF path I added. I will ask the existing DT board
maintainers on Cc for v2 explicitly for Tested-by tags. If that
does not produce coverage, I am happy to defer or drop the patch
rather than push it through unverified.

Thank you for the careful read.

Best regards,
Abdurrahman