Re: [PATCH v6 2/3] mfd: syscon: Add managed registration for external regmaps

[James Hilliard]

On Tue, Aug 11, 2026 at 5:03 AM Arnd Bergmann <arnd@xxxxxxxx> wrote:
>
> On Tue, Aug 11, 2026, at 11:17, James Hilliard wrote:
> > On Tue, Aug 11, 2026 at 2:47 AM Arnd Bergmann <arnd@xxxxxxxx> wrote:
>
> >> I don't think this is the right way to do it. As far as I can tell,
> >> the device you have here is a generic mfd that uses a regmap, which
> >> is not the same thing we usually call a syscon.
> >>
> >> The of_syscon_register_regmap() code path was added specifically
> >> for chips that have a traditional syscon but depending on the
> >> firmware may have to access this by some other means. This is
> >> already stretching the definition of syscon. I don't think we
> >> should take this further and allow normal device drivers like
> >> yours to register through the syscon framework.
> >
> > This was suggested to me by Andrew:
> > https://lore.kernel.org/all/c78c2c35-52e7-4393-9714-06039d8a3f28@xxxxxxx/
>
> Maybe Andrew can clarify, but his reply can also be interpreted
> as saying that you should copy syscon_regmap_lookup_by_phandle()
> into your own driver, rather than changing the actual
> syscon code.

A literal copy would still depend on the syscon registry:
syscon_regmap_lookup_by_phandle() parses the phandle and then obtains
the regmap through syscon_node_to_regmap(). It does not find a normal
device associated with the referenced node.

A private AC200 implementation would instead have to find the I2C
device, establish lifetime ordering, verify that its driver is bound
and call dev_get_regmap(). That is effectively the v4 implementation
rather than a copy of the syscon helper.

Is that direct device lookup what you had in mind, or were you
suggesting another way for the private helper to reach the
I2C-created regmap?

> One problem I see with your current approach is that the lifetime of
> the regmap is not the lifetime of the user by the framework.
> Unloading the mfd driver while the phy driver is in use will
> destroy the regmap. This is a direct result of syscon being
> a very special case that must work during early boot instead
> of being a general-purpose abstraction for managing regmaps.

The PHY creates a managed device link to the AC200 I2C device before
obtaining its regmap. The driver core therefore unbinds the PHY consumer
before allowing the AC200 supplier to release its devres-managed regmap.

The PHY also checks under the supplier device lock that the AC200 probe
has completed before using the regmap. This prevents it from observing
the regmap while the supplier is only partially initialized.

See:
https://lore.kernel.org/all/20260811-submit-acx00-of-dynamic-v1-v6-2-0229b99a32f6@xxxxxxxxx/

> >> Since you already have a top-level mfd device here, just use
> >> that to pass the regmap to the child devices like we do for
> >> other mfd drivers. You can e.g. do this when populating the child
> >> devices through platform_data, or get the pointer from the
> >> parent drvdata.
> >
> > The EPHY is not an MFD-created platform child. Phylib enumerates it as a
> > struct phy_device on the SoC MDIO bus, so its device parent is the
> > struct mii_bus rather than the AC200 I2C device. It therefore cannot
> > directly obtain the AC200 regmap through parent drvdata or MFD child
> > platform data.
>
> I see, so the fundamental problem here is that you have a single
> device that is connected to two buses and both the OF devicetree
> and the Linux driver model are rather bad at handling this.

The link PHY is enumerated by phylib on the MDIO bus, while
its package-control registers are accessed through the separately
enumerated AC200 I2C device.

> I would probably do this in one of two ways:
>
>  a) have a driver module that registers both a phy driver and
>     a platform_driver and figures out the interaction between
>     them internally.

That would require creating a second platform device for the AC200
control side even though the actual PHY is enumerated on MDIO. The two
driver instances would still need explicit instance matching, probe
ordering and removal coordination, presumably using the same firmware
reference or an internal registry.

>  b) have the MFD driver export a private interface that lets
>     the phy_driver interact with the i2c registers and
>     make sure the i2c_driver sets suppress_bind_attrs=true
>     to prevent it from being unbound while the phy_driver
>     is loaded. The symbol dependency itself is enough to
>     prevent the mfd driver from being unloaded here.

If the private interface only returns the regmap, it would largely
duplicate dev_get_regmap(). It would also introduce a link-time
dependency from the combined AC200/AC300 PHY module on the AC200 MFD
module, including on AC300-only systems. Avoiding that would require
splitting the PHY module or adding configuration-dependent stubs.

suppress_bind_attrs would prevent manual sysfs unbind, but a managed
device link already provides per-device teardown ordering without
disabling unbind globally.

> In either case, you still have the choice between a proper
> abstraction that can deal with multiple instances of the
> ac200 device, or slightly cheaty but common assumption that
> only one of them can ever be present.

There is also the approach used in v4: resolve the exact AC200 I2C
device from the package phandle, establish the managed device link,
verify that the supplier is bound, and retrieve its attached regmap
with dev_get_regmap().

That uses the existing device and regmap interfaces, retains safe
unbind ordering through the device link, and supports multiple AC200
instances because lookup is keyed by the phandle. It does not require
an additional platform device, a private exported interface, or
disabling manual unbind.

Would that direct device-link and dev_get_regmap() approach be
acceptable?

v4:
https://lore.kernel.org/all/20260806-submit-acx00-of-dynamic-v1-v4-2-9bf2f7441632@xxxxxxxxx/

>
>       Arnd