Re: [PATCH v12 2/2] mfd: ac200: Add X-Powers AC200 support
From: Lee Jones
Date: Tue Sep 22 2026 - 06:09:32 EST
On Wed, 09 Sep 2026, James Hilliard wrote:
> The X-Powers AC200 is a mixed-signal companion IC with a paged register
> map accessed over I2C.
>
> Enable and rate-lock the shared input clock, retain the vendor resume
> path's 40 ms wait before creating the regmap and accessing registers, and
> deassert the common reset. No minimum delay is documented. Set only the
> deassert bit instead of forcing a reset cycle, avoiding a chip-wide reset
> of unrelated function registers. Leave the common reset deasserted during
> driver removal and system shutdown; function drivers own their block
> resets. Supplier unbind still tears down linked consumers and releases
> the provider's clock references.
>
> Cache only the common page selector. Individual functions can reset
> independently and invalidate their other registers, so leave all
> functional registers volatile.
>
> Register the audio codec and TV encoder using static MFD cells and
> automatically assigned platform device IDs. Their firmware properties
> belong to the parent node. Map their supply lookups to the parent with MFD
> supply aliases, without assigning separate OF nodes to the cells.
>
> When INTB is connected, pass its physical IRQ to the TV encoder cell.
> Select a cell array without IRQ resources when INTB is absent. No regmap
> IRQ controller or private IRQ domain is needed.
>
> Configure the level-triggered INTB output, defaulting to level-low when
> the upstream IRQ has no trigger type. Mask all sources before enabling
> INTB. Function drivers request shared threaded IRQs before enabling their
> own source, check and clear their own status, and mask their source before
> freeing the IRQ. Disable INTB after removing the children on failure or
> removal, while the regmap and clock are still available.
>
> The Ethernet PHY is enumerated on its MDIO bus rather than as an MFD
> child. It follows the x-powers,ac200 phandle and uses this regmap for
> ancillary package-control access.
>
> Co-developed-by: Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> Signed-off-by: James Hilliard <james.hilliard1@xxxxxxxxx>
Why is Andre in there?
> ---
> MAINTAINERS | 1 +
> drivers/mfd/Kconfig | 14 ++++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/ac200.c | 231 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 247 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 419340093c9b..1d03b0060bda 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -29510,6 +29510,7 @@ M: James Hilliard <james.hilliard1@xxxxxxxxx>
> L: linux-sunxi@xxxxxxxxxxxxxxx
> S: Maintained
> F: Documentation/devicetree/bindings/mfd/x-powers,ac200.yaml
> +F: drivers/mfd/ac200.c
>
> X-POWERS AXP288 PMIC DRIVERS
> M: Hans de Goede <hansg@xxxxxxxxxx>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 857ca3bb0d5b..071cba7c775b 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -205,6 +205,20 @@ 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 COMMON_CLK
> + depends on I2C
> + depends on OF
> + select MFD_CORE
> + 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 and instantiates the individual function
> + devices.
> +
> 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..af0d27ff5f45
> --- /dev/null
> +++ b/drivers/mfd/ac200.c
> @@ -0,0 +1,231 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * MFD core driver for the X-Powers AC200
No it isn't.
It's an "X-Powers AC200 Core driver".
> + *
> + * Copyright (C) 2019 Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
> + * Copyright (C) 2026 James Hilliard <james.hilliard1@xxxxxxxxx>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/irq.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +
> +#define AC200_SYS_CONTROL_REG 0x0002
> +#define AC200_SYS_CONTROL_CHIP_RESET_DEASSERT BIT(0)
> +#define AC200_SYS_IRQ_ENABLE_REG 0x0004
> +#define AC200_SYS_IRQ_INTB_ENABLE BIT(15)
> +#define AC200_SYS_IRQ_INTB_ACTIVE_HIGH BIT(14)
> +#define AC200_SYS_IRQ_RTC BIT(12)
> +#define AC200_SYS_IRQ_EPHY BIT(8)
> +#define AC200_SYS_IRQ_TVE BIT(4)
> +
> +/* Interface register accessible from every register page. */
> +#define AC200_TWI_REG_ADDR_H 0x00fe
> +#define AC200_MAX_REG 0xa1f2
> +
> +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 const char * const ac200_codec_supplies[] = {
> + "ac-ldoin",
> +};
> +
> +static const char * const ac200_tve_supplies[] = {
> + "tv-vcc",
> +};
> +
> +static const struct resource ac200_tve_resources[] = {
> + DEFINE_RES_IRQ_NAMED(0, "intb"),
> +};
> +
> +#define AC200_CELL(_name, _supplies, _resources) \
> + { \
> + .name = (_name), \
> + .parent_supplies = (_supplies), \
> + .num_parent_supplies = ARRAY_SIZE(_supplies), \
> + .resources = (_resources), \
> + .num_resources = MFD_RES_SIZE(_resources), \
> + }
> +
> +static const struct mfd_cell ac200_cells[] = {
> + AC200_CELL("ac200-codec", ac200_codec_supplies, NULL),
> + AC200_CELL("ac200-tve", ac200_tve_supplies, ac200_tve_resources),
> +};
> +
> +static const struct mfd_cell ac200_noirq_cells[] = {
> + AC200_CELL("ac200-codec", ac200_codec_supplies, NULL),
> + AC200_CELL("ac200-tve", ac200_tve_supplies, NULL),
> +};
> +
> +static void ac200_disable_intb(void *data)
> +{
> + struct regmap *regmap = data;
> + int ret;
> +
> + ret = regmap_clear_bits(regmap, AC200_SYS_IRQ_ENABLE_REG,
> + AC200_SYS_IRQ_INTB_ENABLE);
> + if (ret)
> + dev_err(regmap_get_device(regmap), "failed to disable INTB: %d\n",
> + ret);
Nit: Avoid these line-wraps by using up to 100-chars.
This goes for the rest of the file too - I'll not comment at each location.
> +}
> +
> +static int ac200_init_irq(struct device *dev, struct regmap *regmap, int irq)
> +{
> + unsigned int trigger;
> + u16 value = 0;
> + int ret;
> +
> + trigger = irq_get_trigger_type(irq);
> + switch (trigger) {
> + case IRQ_TYPE_LEVEL_HIGH:
> + value |= AC200_SYS_IRQ_INTB_ACTIVE_HIGH;
> + break;
> + case IRQ_TYPE_NONE:
> + case IRQ_TYPE_LEVEL_LOW:
> + break;
> + default:
> + return dev_err_probe(dev, -EINVAL,
> + "INTB is level triggered, not type %u\n",
> + trigger);
> + }
> +
> + /* Mask every source before enabling the shared output. */
> + ret = regmap_update_bits(regmap, AC200_SYS_IRQ_ENABLE_REG,
> + AC200_SYS_IRQ_INTB_ENABLE |
> + AC200_SYS_IRQ_INTB_ACTIVE_HIGH |
> + AC200_SYS_IRQ_RTC | AC200_SYS_IRQ_EPHY |
> + AC200_SYS_IRQ_TVE, value);
> + if (ret)
> + return ret;
> +
> + /* Children are removed before INTB is disabled and the regmap released. */
> + ret = devm_add_action_or_reset(dev, ac200_disable_intb, regmap);
> + if (ret)
> + return ret;
> +
> + if (trigger == IRQ_TYPE_NONE) {
> + ret = irq_set_irq_type(irq, IRQ_TYPE_LEVEL_LOW);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to set INTB trigger\n");
> + }
> +
> + /*
> + * Function drivers request INTB with IRQF_SHARED | IRQF_ONESHOT before
> + * enabling their own source. They check and clear their own status and
> + * mask their source before freeing the IRQ. No IRQ domain is needed.
> + */
> + return regmap_set_bits(regmap, AC200_SYS_IRQ_ENABLE_REG,
> + AC200_SYS_IRQ_INTB_ENABLE);
> +}
> +
> +static int ac200_probe(struct i2c_client *client)
> +{
> + const struct mfd_cell *cells = ac200_noirq_cells;
> + unsigned int num_cells = ARRAY_SIZE(ac200_noirq_cells);
> + struct device *dev = &client->dev;
> + struct regmap *regmap;
> + struct clk *clk;
> + int ret;
> +
> + 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");
> +
> + /*
> + * No minimum delay is documented. Retain the vendor resume path's 40 ms
> + * wait after enabling the input clock and before register access.
> + */
> + msleep(40);
> +
> + regmap = devm_regmap_init_i2c(client, &ac200_regmap_config);
> + if (IS_ERR(regmap))
> + return dev_err_probe(dev, PTR_ERR(regmap),
> + "failed to initialize regmap\n");
> +
> + ret = regmap_set_bits(regmap, AC200_SYS_CONTROL_REG,
> + AC200_SYS_CONTROL_CHIP_RESET_DEASSERT);
> + if (ret)
> + return ret;
> +
> + if (client->irq > 0) {
> + ret = ac200_init_irq(dev, regmap, client->irq);
> + if (ret)
> + return ret;
> + cells = ac200_cells;
> + num_cells = ARRAY_SIZE(ac200_cells);
> + }
> +
> + /* Function drivers read their firmware properties from the parent. */
> + ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, cells, num_cells,
> + NULL, client->irq, NULL);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to add function devices\n");
> +
> + return 0;
> +}
> +
> +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,
> + .id_table = ac200_i2c_ids,
> +};
> +module_i2c_driver(ac200_driver);
> +
> +MODULE_AUTHOR("Jernej Skrabec <jernej.skrabec@xxxxxxxxx>");
> +MODULE_AUTHOR("James Hilliard <james.hilliard1@xxxxxxxxx>");
> +MODULE_DESCRIPTION("X-Powers AC200 MFD core driver");
> +MODULE_LICENSE("GPL");
>
> --
> 2.53.0
>
--
Lee Jones