* [PATCH v6 0/3] mfd: add X-Powers AC200 support
@ 2026-08-11 8:27 James Hilliard
2026-08-11 8:27 ` [PATCH v6 1/3] dt-bindings: mfd: x-powers: Add AC200 James Hilliard
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: James Hilliard @ 2026-08-11 8:27 UTC (permalink / raw)
To: Lee Jones, Arnd Bergmann, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, James Hilliard
Cc: mfd, devicetree, linux-kernel
This submission contains only the AC200 binding, managed syscon helper
and I2C register provider and is intended for the MFD tree. The Ethernet
PHY binding and driver will be submitted separately to net-next. Neither
series has a compile-time dependency or required merge order. Their only
shared design point is the DT representation: the MDIO PHY-package node
references the separately described AC200 I2C node for package-control
register access.
The X-Powers AC200 is an I2C-controlled mixed-signal companion IC with a
paged register map shared by its audio, video, RTC and Fast Ethernet PHY
functions. Its shared input clock must remain enabled while the chip is in
use.
The provider enables and rate-locks that clock, matches the vendor
driver's 40 ms delay before the first register access, initializes the
paged regmap, reports the chip revision and applies common reset. It
publishes the regmap through syscon so separately enumerated function
devices can reach their control registers through a firmware reference.
The existing external syscon registration API cannot withdraw an entry
when a removable provider unbinds. Add a managed variant which removes
the entry before the provider's regmap is released and holds the device
node reference for the entry's lifetime. Consumers remain responsible for
teardown ordering, for example through a device link.
The three patches add the AC200 binding, managed external syscon
registration and the AC200 I2C register provider.
The AC200 work builds on earlier work by Jernej Skrabec and Andre
Przywara:
https://github.com/jernejsk/linux-1/commits/ac200-v4
Public AC200 documentation is linked from:
https://linux-sunxi.org/AC200
Validation completed for v6:
- x86_64 defconfig object builds of syscon and the AC200 provider with W=1;
- dt_binding_check for the AC200 schema; and
- strict checkpatch checks with no errors and only the generic new-file
MAINTAINERS prompts.
The running-kernel and hardware results remain those from v5 because the
code is unchanged. Managed syscon registration, lookup, provider removal
and re-registration were tested on a running kernel. On an H616 board the
AC200 provider reported revision 0x101 in package 1, supplied its regmap to
the PHY consumer and correctly ordered supplier unbind/rebind through the
device link.
Assisted-by: OpenAI Codex (gpt-5.6-sol, max)
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
Changes in v6:
- split the MFD work from the independently mergeable networking series
- put the merge routing and shared DT design at the start of the cover letter
- rebase onto the current MFD for-mfd-next branch
- Link to v5: https://patch.msgid.link/20260809-submit-acx00-of-dynamic-v1-v5-0-bfa1f8518b28@gmail.com
---
James Hilliard (3):
dt-bindings: mfd: x-powers: Add AC200
mfd: syscon: Add managed registration for external regmaps
mfd: ac200: Add X-Powers AC200 support
.../devicetree/bindings/mfd/x-powers,ac200.yaml | 49 ++++++
drivers/mfd/Kconfig | 12 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/ac200.c | 170 +++++++++++++++++++++
drivers/mfd/syscon.c | 87 ++++++++---
include/linux/mfd/syscon.h | 12 ++
6 files changed, 309 insertions(+), 22 deletions(-)
---
base-commit: ffc63a677b60247738b02a2520c6bfa740523c2b
change-id: 20260811-submit-ac200-mfd-8bad6d09e285
Best regards,
--
James Hilliard <james.hilliard1@gmail.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v6 1/3] dt-bindings: mfd: x-powers: Add AC200
2026-08-11 8:27 [PATCH v6 0/3] mfd: add X-Powers AC200 support James Hilliard
@ 2026-08-11 8:27 ` James Hilliard
2026-08-11 8:27 ` [PATCH v6 2/3] mfd: syscon: Add managed registration for external regmaps James Hilliard
2026-08-11 8:27 ` [PATCH v6 3/3] mfd: ac200: Add X-Powers AC200 support James Hilliard
2 siblings, 0 replies; 8+ messages in thread
From: James Hilliard @ 2026-08-11 8:27 UTC (permalink / raw)
To: Lee Jones, Arnd Bergmann, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, James Hilliard
Cc: mfd, devicetree, linux-kernel
The AC200 is an I2C-controlled mixed-signal companion IC containing
audio, video, RTC and Fast Ethernet PHY functions.
Describe the parent device and its shared input clock. The Ethernet PHY
is represented by a PHY package on its MDIO bus, with a phandle back to
this device for access to the package control registers, so it does not
require an MFD child node. Function-specific supplies belong to their
consumer bindings rather than the shared parent.
The input clock must run at 24 or 27 MHz when the Ethernet PHY is used,
matching the rates encoded by its documented clock selector.
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
.../devicetree/bindings/mfd/x-powers,ac200.yaml | 49 ++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/x-powers,ac200.yaml b/Documentation/devicetree/bindings/mfd/x-powers,ac200.yaml
new file mode 100644
index 000000000000..609831b15c80
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/x-powers,ac200.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/x-powers,ac200.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: X-Powers AC200 mixed-signal IC
+
+maintainers:
+ - James Hilliard <james.hilliard1@gmail.com>
+
+description:
+ The AC200 is a mixed-signal companion IC containing audio, video, RTC and
+ Fast Ethernet PHY functions. Its control registers are accessed over I2C.
+
+properties:
+ compatible:
+ const: x-powers,ac200
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+ description:
+ AC200 input clock. When using the Ethernet PHY, its configured rate must
+ be 24 or 27 MHz, matching the rates encoded by the documented EPHY clock
+ selector.
+
+required:
+ - compatible
+ - reg
+ - clocks
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ mixed-signal@10 {
+ compatible = "x-powers,ac200";
+ reg = <0x10>;
+ clocks = <&pwm 5>;
+ };
+ };
+...
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v6 2/3] mfd: syscon: Add managed registration for external regmaps
2026-08-11 8:27 [PATCH v6 0/3] mfd: add X-Powers AC200 support James Hilliard
2026-08-11 8:27 ` [PATCH v6 1/3] dt-bindings: mfd: x-powers: Add AC200 James Hilliard
@ 2026-08-11 8:27 ` James Hilliard
2026-08-11 8:46 ` Arnd Bergmann
2026-08-11 8:27 ` [PATCH v6 3/3] mfd: ac200: Add X-Powers AC200 support James Hilliard
2 siblings, 1 reply; 8+ messages in thread
From: James Hilliard @ 2026-08-11 8:27 UTC (permalink / raw)
To: Lee Jones, Arnd Bergmann, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, James Hilliard
Cc: mfd, devicetree, linux-kernel
of_syscon_register_regmap() publishes an externally owned regmap in the
global syscon list, but provides no way for a removable driver to
withdraw it. Registering a devm-managed regmap from such a driver would
therefore leave a stale pointer after unbind.
Factor external registration through an internal helper and add
devm_of_syscon_register_regmap(). The managed action removes the entry
under the syscon list lock before later devres actions release the
regmap. Hold a device-node reference for the lifetime of every
externally registered entry so its lookup key also remains valid.
Consumers of a removable provider remain responsible for ordering
teardown, for example with a managed device link.
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
drivers/mfd/syscon.c | 87 ++++++++++++++++++++++++++++++++++------------
include/linux/mfd/syscon.h | 12 +++++++
2 files changed, 77 insertions(+), 22 deletions(-)
diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index 21a7fcdd2737..448422fe4f21 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -10,6 +10,7 @@
#include <linux/cleanup.h>
#include <linux/clk.h>
+#include <linux/device.h>
#include <linux/err.h>
#include <linux/hwspinlock.h>
#include <linux/list.h>
@@ -193,53 +194,95 @@ static struct regmap *device_node_get_regmap(struct device_node *np,
return syscon->regmap;
}
-/**
- * of_syscon_register_regmap() - Register regmap for specified device node
- * @np: Device tree node
- * @regmap: Pointer to regmap object
- *
- * Register an externally created regmap object with syscon for the specified
- * device tree node. This regmap will then be returned to client drivers using
- * the syscon_regmap_lookup_by_phandle() API.
- *
- * Return: 0 on success, negative error code on failure.
- */
-int of_syscon_register_regmap(struct device_node *np, struct regmap *regmap)
+static struct syscon *
+of_syscon_register_regmap_internal(struct device_node *np,
+ struct regmap *regmap)
{
struct syscon *entry, *syscon = NULL;
- int ret;
if (!np || !regmap)
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
syscon = kzalloc_obj(*syscon);
if (!syscon)
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);
/* check if syscon entry already exists */
mutex_lock(&syscon_list_lock);
list_for_each_entry(entry, &syscon_list, list)
if (entry->np == np) {
- ret = -EEXIST;
- goto err_unlock;
+ mutex_unlock(&syscon_list_lock);
+ kfree(syscon);
+ return ERR_PTR(-EEXIST);
}
syscon->regmap = regmap;
- syscon->np = np;
+ syscon->np = of_node_get(np);
/* register the regmap in syscon list */
list_add_tail(&syscon->list, &syscon_list);
mutex_unlock(&syscon_list_lock);
- return 0;
+ return syscon;
+}
+
+/**
+ * of_syscon_register_regmap() - Register regmap for specified device node
+ * @np: Device tree node
+ * @regmap: Pointer to regmap object
+ *
+ * Register an externally created regmap object with syscon for the specified
+ * device tree node. This regmap will then be returned to client drivers using
+ * the syscon_regmap_lookup_by_phandle() API.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int of_syscon_register_regmap(struct device_node *np, struct regmap *regmap)
+{
+ return PTR_ERR_OR_ZERO(of_syscon_register_regmap_internal(np, regmap));
+}
+EXPORT_SYMBOL_GPL(of_syscon_register_regmap);
+
+static void devm_of_syscon_register_regmap_release(void *data)
+{
+ struct syscon *syscon = data;
-err_unlock:
+ mutex_lock(&syscon_list_lock);
+ list_del(&syscon->list);
mutex_unlock(&syscon_list_lock);
+
+ of_node_put(syscon->np);
kfree(syscon);
- return ret;
}
-EXPORT_SYMBOL_GPL(of_syscon_register_regmap);
+
+/**
+ * devm_of_syscon_register_regmap() - Register a managed external syscon regmap
+ * @dev: Device that owns the regmap
+ * @np: Device tree node associated with the regmap
+ * @regmap: Pointer to the externally created regmap
+ *
+ * Register an externally created regmap object with syscon and remove it when
+ * @dev is unbound. Consumers must stop using the regmap before the provider is
+ * unbound, for example by establishing a managed device link to @dev.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int devm_of_syscon_register_regmap(struct device *dev,
+ struct device_node *np,
+ struct regmap *regmap)
+{
+ struct syscon *syscon;
+
+ syscon = of_syscon_register_regmap_internal(np, regmap);
+ if (IS_ERR(syscon))
+ return PTR_ERR(syscon);
+
+ return devm_add_action_or_reset(dev,
+ devm_of_syscon_register_regmap_release,
+ syscon);
+}
+EXPORT_SYMBOL_GPL(devm_of_syscon_register_regmap);
/**
* device_node_to_regmap() - Get or create a regmap for specified device node
diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
index aad9c6b50463..3e804fc1b03e 100644
--- a/include/linux/mfd/syscon.h
+++ b/include/linux/mfd/syscon.h
@@ -15,6 +15,7 @@
#include <linux/errno.h>
struct device_node;
+struct device;
#ifdef CONFIG_MFD_SYSCON
struct regmap *device_node_to_regmap(struct device_node *np);
@@ -30,6 +31,9 @@ struct regmap *syscon_regmap_lookup_by_phandle_optional(struct device_node *np,
const char *property);
int of_syscon_register_regmap(struct device_node *np,
struct regmap *regmap);
+int devm_of_syscon_register_regmap(struct device *dev,
+ struct device_node *np,
+ struct regmap *regmap);
#else
static inline struct regmap *device_node_to_regmap(struct device_node *np)
{
@@ -75,6 +79,14 @@ static inline int of_syscon_register_regmap(struct device_node *np,
return -EOPNOTSUPP;
}
+static inline int
+devm_of_syscon_register_regmap(struct device *dev,
+ struct device_node *np,
+ struct regmap *regmap)
+{
+ return -EOPNOTSUPP;
+}
+
#endif
#endif /* __LINUX_MFD_SYSCON_H__ */
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v6 3/3] mfd: ac200: Add X-Powers AC200 support
2026-08-11 8:27 [PATCH v6 0/3] mfd: add X-Powers AC200 support James Hilliard
2026-08-11 8:27 ` [PATCH v6 1/3] dt-bindings: mfd: x-powers: Add AC200 James Hilliard
2026-08-11 8:27 ` [PATCH v6 2/3] mfd: syscon: Add managed registration for external regmaps James Hilliard
@ 2026-08-11 8:27 ` James Hilliard
2 siblings, 0 replies; 8+ messages in thread
From: James Hilliard @ 2026-08-11 8:27 UTC (permalink / raw)
To: Lee Jones, Arnd Bergmann, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, James Hilliard
Cc: mfd, devicetree, linux-kernel
The X-Powers AC200 is a mixed-signal companion IC with a paged register
map accessed over I2C.
Enable the shared input clock and prevent its rate from changing. Match
the vendor driver's 40 ms wait before the first register access,
initialize the paged regmap, report the chip and package revision, and
apply common reset.
The Ethernet PHY link endpoint is independently enumerated on its MDIO
bus, so publish the regmap through managed syscon registration for lookup
through its AC200 firmware reference rather than creating an artificial
MFD platform child. The managed entry is withdrawn before the regmap is
released, while the function driver separately manages the PHY analog
supply.
Cache only the common page selector. Individual functions can reset
independently and invalidate their other registers without regmap's
knowledge, so leave all functional registers volatile.
Reset the chip during managed teardown and system shutdown. Consumers
can use device links to ensure that they unbind before the shared AC200
resources are released.
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
drivers/mfd/Kconfig | 12 ++++
drivers/mfd/Makefile | 1 +
drivers/mfd/ac200.c | 170 +++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 183 insertions(+)
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index e4fd4572472f..f8354a0f6b5c 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -205,6 +205,18 @@ config MFD_AC100
This driver include only the core APIs. You have to select individual
components like codecs or RTC under the corresponding menus.
+config MFD_AC200
+ tristate "X-Powers AC200"
+ depends on I2C
+ depends on OF
+ select MFD_SYSCON
+ select REGMAP_I2C
+ help
+ Support for the X-Powers AC200 mixed-signal companion IC. The AC200
+ contains audio, video, RTC and Fast Ethernet PHY functions and is
+ co-packaged with some Allwinner H6 and H616 SoCs. This driver provides
+ the shared register access used by the individual function drivers.
+
config MFD_AXP20X
tristate
select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 72d3944b0ad8..f8101d2a9ce9 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -150,6 +150,7 @@ obj-$(CONFIG_MFD_DA9052_SPI) += da9052-spi.o
obj-$(CONFIG_MFD_DA9052_I2C) += da9052-i2c.o
obj-$(CONFIG_MFD_AC100) += ac100.o
+obj-$(CONFIG_MFD_AC200) += ac200.o
obj-$(CONFIG_MFD_AXP20X) += axp20x.o
obj-$(CONFIG_MFD_AXP20X_I2C) += axp20x-i2c.o
obj-$(CONFIG_MFD_AXP20X_RSB) += axp20x-rsb.o
diff --git a/drivers/mfd/ac200.c b/drivers/mfd/ac200.c
new file mode 100644
index 000000000000..25066bb36ac7
--- /dev/null
+++ b/drivers/mfd/ac200.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * MFD core driver for the X-Powers AC200
+ *
+ * Copyright (C) 2019 Jernej Skrabec <jernej.skrabec@gmail.com>
+ * Copyright (C) 2026 James Hilliard <james.hilliard1@gmail.com>
+ *
+ * Based on the AC100 driver:
+ * Copyright (C) 2016 Chen-Yu Tsai
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#define AC200_SYS_VERSION_REG 0x0000
+#define AC200_SYS_VERSION_PACKAGE_MASK GENMASK(15, 14)
+#define AC200_SYS_VERSION_CHIP_MASK GENMASK(11, 0)
+
+#define AC200_SYS_CONTROL_REG 0x0002
+#define AC200_SYS_CONTROL_CHIP_RESET_DEASSERT BIT(0)
+
+/* Interface register accessible from every register page. */
+#define AC200_TWI_REG_ADDR_H 0x00fe
+#define AC200_MAX_REG 0xa1f2
+
+struct ac200 {
+ struct regmap *regmap;
+};
+
+static const struct regmap_range_cfg ac200_range_cfg[] = {
+ {
+ .range_max = AC200_MAX_REG,
+ .selector_reg = AC200_TWI_REG_ADDR_H,
+ .selector_mask = 0xff,
+ .window_len = 256,
+ },
+};
+
+/*
+ * Each AC200 sub-block can reset independently, invalidating its register
+ * contents without regmap's knowledge. Cache only the common page selector;
+ * this avoids a selector read-modify-write for every access on the same page
+ * without ever returning stale functional-register values.
+ */
+static bool ac200_volatile_reg(struct device *dev, unsigned int reg)
+{
+ return reg != AC200_TWI_REG_ADDR_H;
+}
+
+static const struct regmap_config ac200_regmap_config = {
+ .name = "ac200",
+ .reg_bits = 8,
+ .reg_stride = 2,
+ .val_bits = 16,
+ .ranges = ac200_range_cfg,
+ .num_ranges = ARRAY_SIZE(ac200_range_cfg),
+ .max_register = AC200_MAX_REG,
+ .volatile_reg = ac200_volatile_reg,
+ .cache_type = REGCACHE_MAPLE,
+};
+
+static void ac200_disable(void *data)
+{
+ struct ac200 *ddata = data;
+
+ regmap_write(ddata->regmap, AC200_SYS_CONTROL_REG, 0);
+}
+
+static int ac200_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ unsigned int version;
+ struct ac200 *ddata;
+ struct clk *clk;
+ int ret;
+
+ ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
+ if (!ddata)
+ return -ENOMEM;
+
+ clk = devm_clk_get_enabled(dev, NULL);
+ if (IS_ERR(clk))
+ return dev_err_probe(dev, PTR_ERR(clk),
+ "failed to enable input clock\n");
+
+ ret = devm_clk_rate_exclusive_get(dev, clk);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to lock clock rate\n");
+
+ ddata->regmap = devm_regmap_init_i2c(client, &ac200_regmap_config);
+ if (IS_ERR(ddata->regmap))
+ return dev_err_probe(dev, PTR_ERR(ddata->regmap),
+ "failed to initialize regmap\n");
+
+ i2c_set_clientdata(client, ddata);
+
+ /*
+ * No minimum delay is documented. Match the vendor driver's 40 ms delay
+ * before its first AC200 register access after enabling the input clock.
+ */
+ msleep(40);
+
+ ret = regmap_read(ddata->regmap, AC200_SYS_VERSION_REG, &version);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to read chip version\n");
+
+ dev_info(dev, "AC200 revision %#lx in package %lu\n",
+ FIELD_GET(AC200_SYS_VERSION_CHIP_MASK, version),
+ FIELD_GET(AC200_SYS_VERSION_PACKAGE_MASK, version));
+
+ /* Reset the chip after dependent function drivers have unbound. */
+ ret = devm_add_action_or_reset(dev, ac200_disable, ddata);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(ddata->regmap, AC200_SYS_CONTROL_REG, 0);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(ddata->regmap, AC200_SYS_CONTROL_REG,
+ AC200_SYS_CONTROL_CHIP_RESET_DEASSERT);
+ if (ret)
+ return ret;
+
+ ret = devm_of_syscon_register_regmap(dev, dev->of_node,
+ ddata->regmap);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to register regmap\n");
+
+ return 0;
+}
+
+static void ac200_shutdown(struct i2c_client *client)
+{
+ ac200_disable(i2c_get_clientdata(client));
+}
+
+static const struct of_device_id ac200_of_match[] = {
+ { .compatible = "x-powers,ac200" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ac200_of_match);
+
+static const struct i2c_device_id ac200_i2c_ids[] = {
+ { .name = "ac200" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, ac200_i2c_ids);
+
+static struct i2c_driver ac200_driver = {
+ .driver = {
+ .name = "ac200",
+ .of_match_table = ac200_of_match,
+ },
+ .probe = ac200_probe,
+ .shutdown = ac200_shutdown,
+ .id_table = ac200_i2c_ids,
+};
+module_i2c_driver(ac200_driver);
+
+MODULE_AUTHOR("James Hilliard <james.hilliard1@gmail.com>");
+MODULE_DESCRIPTION("X-Powers AC200 MFD core driver");
+MODULE_LICENSE("GPL");
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v6 2/3] mfd: syscon: Add managed registration for external regmaps
2026-08-11 8:27 ` [PATCH v6 2/3] mfd: syscon: Add managed registration for external regmaps James Hilliard
@ 2026-08-11 8:46 ` Arnd Bergmann
2026-08-11 9:17 ` James Hilliard
0 siblings, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2026-08-11 8:46 UTC (permalink / raw)
To: James Hilliard, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: mfd, devicetree, linux-kernel
On Tue, Aug 11, 2026, at 10:27, James Hilliard wrote:
> of_syscon_register_regmap() publishes an externally owned regmap in the
> global syscon list, but provides no way for a removable driver to
> withdraw it. Registering a devm-managed regmap from such a driver would
> therefore leave a stale pointer after unbind.
>
> Factor external registration through an internal helper and add
> devm_of_syscon_register_regmap(). The managed action removes the entry
> under the syscon list lock before later devres actions release the
> regmap. Hold a device-node reference for the lifetime of every
> externally registered entry so its lookup key also remains valid.
>
> Consumers of a removable provider remain responsible for ordering
> teardown, for example with a managed device link.
>
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
Hi James,
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.
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.
Arnd
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v6 2/3] mfd: syscon: Add managed registration for external regmaps
2026-08-11 8:46 ` Arnd Bergmann
@ 2026-08-11 9:17 ` James Hilliard
2026-08-11 11:03 ` Arnd Bergmann
0 siblings, 1 reply; 8+ messages in thread
From: James Hilliard @ 2026-08-11 9:17 UTC (permalink / raw)
To: Arnd Bergmann, Andrew Lunn
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley, mfd,
devicetree, linux-kernel
On Tue, Aug 11, 2026 at 2:47 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Aug 11, 2026, at 10:27, James Hilliard wrote:
> > of_syscon_register_regmap() publishes an externally owned regmap in the
> > global syscon list, but provides no way for a removable driver to
> > withdraw it. Registering a devm-managed regmap from such a driver would
> > therefore leave a stale pointer after unbind.
> >
> > Factor external registration through an internal helper and add
> > devm_of_syscon_register_regmap(). The managed action removes the entry
> > under the syscon list lock before later devres actions release the
> > regmap. Hold a device-node reference for the lifetime of every
> > externally registered entry so its lookup key also remains valid.
> >
> > Consumers of a removable provider remain responsible for ordering
> > teardown, for example with a managed device link.
> >
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
>
> Hi James,
>
> 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@lunn.ch/
> 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.
Here's how I was doing it previously:
https://lore.kernel.org/all/20260806-submit-acx00-of-dynamic-v1-v4-2-9bf2f7441632@gmail.com/
>
> Arnd
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v6 2/3] mfd: syscon: Add managed registration for external regmaps
2026-08-11 9:17 ` James Hilliard
@ 2026-08-11 11:03 ` Arnd Bergmann
2026-08-11 16:56 ` James Hilliard
0 siblings, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2026-08-11 11:03 UTC (permalink / raw)
To: James Hilliard, Andrew Lunn
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley, mfd,
devicetree, linux-kernel
On Tue, Aug 11, 2026, at 11:17, James Hilliard wrote:
> On Tue, Aug 11, 2026 at 2:47 AM Arnd Bergmann <arnd@arndb.de> 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@lunn.ch/
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.
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.
>> 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.
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.
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.
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.
Arnd
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v6 2/3] mfd: syscon: Add managed registration for external regmaps
2026-08-11 11:03 ` Arnd Bergmann
@ 2026-08-11 16:56 ` James Hilliard
0 siblings, 0 replies; 8+ messages in thread
From: James Hilliard @ 2026-08-11 16:56 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Andrew Lunn, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, mfd, devicetree, linux-kernel
On Tue, Aug 11, 2026 at 5:03 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Aug 11, 2026, at 11:17, James Hilliard wrote:
> > On Tue, Aug 11, 2026 at 2:47 AM Arnd Bergmann <arnd@arndb.de> 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@lunn.ch/
>
> 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@gmail.com/
> >> 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@gmail.com/
>
> Arnd
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-08-11 16:57 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-11 8:27 [PATCH v6 0/3] mfd: add X-Powers AC200 support James Hilliard
2026-08-11 8:27 ` [PATCH v6 1/3] dt-bindings: mfd: x-powers: Add AC200 James Hilliard
2026-08-11 8:27 ` [PATCH v6 2/3] mfd: syscon: Add managed registration for external regmaps James Hilliard
2026-08-11 8:46 ` Arnd Bergmann
2026-08-11 9:17 ` James Hilliard
2026-08-11 11:03 ` Arnd Bergmann
2026-08-11 16:56 ` James Hilliard
2026-08-11 8:27 ` [PATCH v6 3/3] mfd: ac200: Add X-Powers AC200 support James Hilliard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox