[PATCH v6 3/4] reset: cix: add sky1 audss auxiliary reset driver
Joakim Zhang
joakim.zhang at cixtech.com
Mon Jun 29 02:11:46 PDT 2026
Hello, Philipp
> -----Original Message-----
> From: Philipp Zabel <p.zabel at pengutronix.de>
> Sent: Wednesday, June 24, 2026 4:30 PM
> To: Joakim Zhang <joakim.zhang at cixtech.com>; mturquette at baylibre.com;
> sboyd at kernel.org; bmasney at redhat.com; robh at kernel.org;
> krzk+dt at kernel.org; conor+dt at kernel.org; Gary Yang
> <gary.yang at cixtech.com>
> Cc: cix-kernel-upstream <cix-kernel-upstream at cixtech.com>; linux-
> clk at vger.kernel.org; devicetree at vger.kernel.org; linux-kernel at vger.kernel.org;
> linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH v6 3/4] reset: cix: add sky1 audss auxiliary reset driver
>
> EXTERNAL EMAIL
>
> CAUTION: Suspicious Email from unusual domain.
>
> On Di, 2026-06-23 at 15:08 +0800, joakim.zhang at cixtech.com wrote:
> > From: Joakim Zhang <joakim.zhang at cixtech.com>
> >
> > Add an auxiliary reset controller driver for the AUDSS CRU. Sixteen
> > software reset lines for audio subsystem peripherals are controlled
> > through one register in the CRU register map.
> >
> > The driver is created by the AUDSS clock platform driver and registers
> > the reset controller on the CRU device node.
> >
> > Signed-off-by: Joakim Zhang <joakim.zhang at cixtech.com>
> > ---
> > drivers/reset/Kconfig | 14 +++
> > drivers/reset/Makefile | 1 +
> > drivers/reset/reset-sky1-audss.c | 192
> > +++++++++++++++++++++++++++++++
> > 3 files changed, 207 insertions(+)
> > create mode 100644 drivers/reset/reset-sky1-audss.c
> >
> > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig index
> > d009eb0849a3..f74859b292ae 100644
> > --- a/drivers/reset/Kconfig
> > +++ b/drivers/reset/Kconfig
> > @@ -300,6 +300,20 @@ config RESET_SKY1
> > help
> > This enables the reset controller for Cix Sky1.
> >
> > +config RESET_SKY1_AUDSS
> > + tristate "Cix Sky1 Audio Subsystem reset controller"
> > + depends on ARCH_CIX || COMPILE_TEST
> > + select AUXILIARY_BUS
> > + select REGMAP_MMIO
> > + default CLK_SKY1_AUDSS
> > + help
> > + Support for block-level software reset lines in the Cix Sky1
> > + Audio Subsystem (AUDSS) Clock and Reset Unit. Sixteen reset
> > + outputs for audio peripherals are controlled through the CRU
> > + register map. The driver binds as an auxiliary device from
> > + the AUDSS clock driver. Say M or Y here if you want to build
> > + this driver.
> > +
> > config RESET_SOCFPGA
> > bool "SoCFPGA Reset Driver" if COMPILE_TEST && (!ARM
> || !ARCH_INTEL_SOCFPGA)
> > default ARM && ARCH_INTEL_SOCFPGA diff --git
> > a/drivers/reset/Makefile b/drivers/reset/Makefile index
> > 3e52569bd276..e81407ea3e29 100644
> > --- a/drivers/reset/Makefile
> > +++ b/drivers/reset/Makefile
> > @@ -39,6 +39,7 @@ obj-$(CONFIG_RESET_RZV2H_USB2PHY) +=
> > reset-rzv2h-usb2phy.o
> > obj-$(CONFIG_RESET_SCMI) += reset-scmi.o
> > obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
> > obj-$(CONFIG_RESET_SKY1) += reset-sky1.o
> > +obj-$(CONFIG_RESET_SKY1_AUDSS) += reset-sky1-audss.o
> > obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
> > obj-$(CONFIG_RESET_SUNPLUS) += reset-sunplus.o
> > obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o diff --git
> > a/drivers/reset/reset-sky1-audss.c b/drivers/reset/reset-sky1-audss.c
> > new file mode 100644
> > index 000000000000..20870f37d7d7
> > --- /dev/null
> > +++ b/drivers/reset/reset-sky1-audss.c
> > @@ -0,0 +1,192 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Cix Sky1 Audio Subsystem reset controller driver
> > + *
> > + * Copyright 2026 Cix Technology Group Co., Ltd.
> > + */
> > +
> > +#include <dt-bindings/reset/cix,sky1-audss-cru.h>
> > +
> > +#include <linux/auxiliary_bus.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/regmap.h>
> > +#include <linux/reset-controller.h>
> > +
> > +#define SKY1_RESET_SLEEP_MIN_US 50
> > +#define SKY1_RESET_SLEEP_MAX_US 100
> > +
> > +#define AUDSS_SW_RST 0x78
> > +
> > +struct sky1_audss_reset_map {
> > + unsigned int offset;
> > + unsigned int mask;
> > +};
> > +
> > +struct sky1_audss_reset {
> > + struct reset_controller_dev rcdev;
> > + struct regmap *regmap;
> > + const struct sky1_audss_reset_map *map; };
> > +
> > +static const struct sky1_audss_reset_map sky1_audss_reset_map[] = {
> > + [AUDSS_I2S0_SW_RST] = { AUDSS_SW_RST, BIT(0) },
> > + [AUDSS_I2S1_SW_RST] = { AUDSS_SW_RST, BIT(1) },
> > + [AUDSS_I2S2_SW_RST] = { AUDSS_SW_RST, BIT(2) },
> > + [AUDSS_I2S3_SW_RST] = { AUDSS_SW_RST, BIT(3) },
> > + [AUDSS_I2S4_SW_RST] = { AUDSS_SW_RST, BIT(4) },
> > + [AUDSS_I2S5_SW_RST] = { AUDSS_SW_RST, BIT(5) },
> > + [AUDSS_I2S6_SW_RST] = { AUDSS_SW_RST, BIT(6) },
> > + [AUDSS_I2S7_SW_RST] = { AUDSS_SW_RST, BIT(7) },
> > + [AUDSS_I2S8_SW_RST] = { AUDSS_SW_RST, BIT(8) },
> > + [AUDSS_I2S9_SW_RST] = { AUDSS_SW_RST, BIT(9) },
> > + [AUDSS_WDT_SW_RST] = { AUDSS_SW_RST, BIT(10) },
> > + [AUDSS_TIMER_SW_RST] = { AUDSS_SW_RST, BIT(11) },
> > + [AUDSS_MB0_SW_RST] = { AUDSS_SW_RST, BIT(12) },
> > + [AUDSS_MB1_SW_RST] = { AUDSS_SW_RST, BIT(13) },
> > + [AUDSS_HDA_SW_RST] = { AUDSS_SW_RST, BIT(14) },
> > + [AUDSS_DMAC_SW_RST] = { AUDSS_SW_RST, BIT(15) },
> > +};
> > +
> > +static struct sky1_audss_reset *to_sky1_audss_reset(struct
> > +reset_controller_dev *rcdev) {
> > + return container_of(rcdev, struct sky1_audss_reset, rcdev); }
> > +
> > +static int sky1_audss_reset_set(struct reset_controller_dev *rcdev,
> > + unsigned long id, bool assert) {
> > + struct sky1_audss_reset *priv = to_sky1_audss_reset(rcdev);
> > + const struct sky1_audss_reset_map *signal = &priv->map[id];
> > + unsigned int value = assert ? 0 : signal->mask;
> > +
> > + return regmap_update_bits(priv->regmap, signal->offset,
> > + signal->mask, value);
>
> Why does this propagate the return value ...
I'll propagate the return value in the ops callbacks.
> > +}
> > +
> > +static int sky1_audss_reset_assert(struct reset_controller_dev *rcdev,
> > + unsigned long id) {
> > + sky1_audss_reset_set(rcdev, id, true);
>
> ... only to be ignored? It'd be better to pass it on.
Yes, will add.
> > + usleep_range(SKY1_RESET_SLEEP_MIN_US,
> SKY1_RESET_SLEEP_MAX_US);
> > + return 0;
> > +}
> > +
> > +static int sky1_audss_reset_deassert(struct reset_controller_dev *rcdev,
> > + unsigned long id) {
> > + sky1_audss_reset_set(rcdev, id, false);
> > + usleep_range(SKY1_RESET_SLEEP_MIN_US,
> SKY1_RESET_SLEEP_MAX_US);
> > + return 0;
> > +}
> > +
> > +static int sky1_audss_reset(struct reset_controller_dev *rcdev,
> > + unsigned long id) {
> > + sky1_audss_reset_assert(rcdev, id);
> > + sky1_audss_reset_deassert(rcdev, id);
> > + return 0;
> > +}
>
> Will any AUDSS reset consumer use the reset_control_reset() API?
> If not, no need to implement this.
Will remove both .reset and .status.
> > +
> > +static int sky1_audss_reset_status(struct reset_controller_dev *rcdev,
> > + unsigned long id) {
> > + struct sky1_audss_reset *priv = to_sky1_audss_reset(rcdev);
> > + const struct sky1_audss_reset_map *signal = &priv->map[id];
> > + unsigned int value;
> > +
> > + regmap_read(priv->regmap, signal->offset, &value);
> > + return !!(value & signal->mask); }
> > +
> > +static const struct reset_control_ops sky1_audss_reset_ops = {
> > + .reset = sky1_audss_reset,
> > + .assert = sky1_audss_reset_assert,
> > + .deassert = sky1_audss_reset_deassert,
> > + .status = sky1_audss_reset_status,
> > +};
> > +
> > +static const struct regmap_config sky1_audss_regmap_config = {
> > + .reg_bits = 32,
> > + .val_bits = 32,
> > + .reg_stride = 4,
> > +};
> > +
> > +static void sky1_audss_reset_iounmap(void *data) {
> > + iounmap(data);
> > +}
> > +
> > +static int sky1_audss_reset_get_regmap(struct sky1_audss_reset *priv)
> > +{
> > + struct device *dev = priv->rcdev.dev;
> > + void __iomem *base;
> > + int ret;
> > +
> > + priv->regmap = dev_get_regmap(dev->parent, NULL);
> > + if (priv->regmap)
> > + return 0;
> > +
> > + base = of_iomap(dev->parent->of_node, 0);
> > + if (!base)
> > + return dev_err_probe(dev, -ENOMEM, "failed to iomap
> > + address space\n");
> > +
> > + ret = devm_add_action_or_reset(dev, sky1_audss_reset_iounmap, base);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to register
> > + iounmap action\n");
> > +
> > + priv->regmap = devm_regmap_init_mmio(dev, base,
> &sky1_audss_regmap_config);
> > + if (IS_ERR(priv->regmap))
> > + return dev_err_probe(dev, PTR_ERR(priv->regmap),
> > + "failed to initialize regmap\n");
>
> Why is there a fallback path? The clock driver creates the regmap before
> creating the reset aux device, so dev_get_regmap() can never fail.
Agreed.
> > +
> > + return 0;
> > +}
> > +
> > +static int sky1_audss_reset_probe(struct auxiliary_device *adev,
> > + const struct auxiliary_device_id *id)
> > +{
> > + struct sky1_audss_reset *priv;
> > + struct device *dev = &adev->dev;
> > + int ret;
> > +
> > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > +
> > + priv->map = sky1_audss_reset_map;
> > + priv->rcdev.owner = THIS_MODULE;
> > + priv->rcdev.nr_resets = ARRAY_SIZE(sky1_audss_reset_map);
> > + priv->rcdev.ops = &sky1_audss_reset_ops;
> > + priv->rcdev.of_node = dev->parent->of_node;
>
> auxiliary_device_create() uses device_set_of_node_from_dev() to inherit the
> parent of_node, so you can use dev->of_node here.
Done. rcdev.of_node now uses dev->of_node.
> > + priv->rcdev.dev = dev;
> > + priv->rcdev.of_reset_n_cells = 1;
>
> No need to set of_reset_n_cells.
>
> > +
> > + dev_set_drvdata(dev, priv);
>
> This seems unnecessary as well.
>
> > +
> > + ret = sky1_audss_reset_get_regmap(priv);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to get
> > + regmap\n");
> > +
> > + return devm_reset_controller_register(dev, &priv->rcdev); }
> > +
> > +static const struct auxiliary_device_id sky1_audss_reset_ids[] = {
> > + { .name = "clk_sky1_audss.reset" },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(auxiliary, sky1_audss_reset_ids);
> > +
> > +static struct auxiliary_driver sky1_audss_reset_driver = {
> > + .probe = sky1_audss_reset_probe,
> > + .id_table = sky1_audss_reset_ids, };
> > +
>
> Drop this empty line.
Removed dev_set_drvdata() and the extra blank line.
Thanks,
Joakim
More information about the linux-arm-kernel
mailing list