* Re: [PATCH v4 4/5] clk: cix: add sky1 audss clock controller
From: sashiko-bot @ 2026-06-17 6:16 UTC (permalink / raw)
To: joakim.zhang; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260617060437.1474816-5-joakim.zhang@cixtech.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Use-after-free race during device removal because composite clocks are unregistered manually before the devres-managed OF clock provider is unregistered.
- [High] Multiple independent clocks erroneously share the exact same hardware gate bits, causing Common Clock Framework (CCF) state corruption.
- [High] Calling `pm_runtime_force_suspend()` before `pm_runtime_disable()` in `remove()` creates a concurrency race condition.
- [Medium] Parent clock enables are permanently leaked in the probe error path and on device removal if `CONFIG_PM` is disabled.
- [Medium] Missing `pm_ptr()` wrapper for `dev_pm_ops` pointer causes dead code when `CONFIG_PM` is disabled.
- [Medium] Device register states are saved into a global static array during runtime suspend, causing an anti-pattern and potential data races.
--
commit ba255415d22d68eb57522fe8fe2cfadbf4ba75d6
Author: Joakim Zhang <joakim.zhang@cixtech.com>
clk: cix: add sky1 audss clock controller
Add a platform driver for the Cix Sky1 Audio Subsystem (AUDSS) internal
clock controller.
> diff --git a/drivers/clk/cix/clk-sky1-audss.c b/drivers/clk/cix/clk-sky1-audss.c
> new file mode 100644
> index 0000000000000..c1e7dfe45d12c
> --- /dev/null
> +++ b/drivers/clk/cix/clk-sky1-audss.c
> @@ -0,0 +1,1167 @@
[ ... ]
> +static u32 sky1_reg_save[][2] = {
> + { INFO_HIFI0, 0 },
> + { INFO_CLK_GATE, 0 },
> + { INFO_CLK_DIV, 0 },
> + { INFO_CLK_MUX, 0 },
> + { INFO_MCLK, 0 },
> +};
[Severity: Medium]
Is it safe to save device register states into a global static array during
runtime suspend? If multiple instances of this driver or SoC ever exist in a
system, wouldn't they overwrite each other's suspended states? Could this
state be allocated dynamically within the per-device priv structure instead?
[ ... ]
> + /* wdg */
> + CFG(CLK_WDG_APB,
> + "audss_wdg_apb",
> + wdg_apb_parent,
> + NULL,
> + -1, 0, 0, 0,
> + -1, 0, 0, 0,
> + INFO_CLK_GATE, 10, 0,
> + 0),
> + CFG(CLK_WDG_WDG,
> + "audss_wdg_wdg",
> + wdg_wdg_parent,
> + NULL,
> + -1, 0, 0, 0,
> + -1, 0, 0, 0,
> + INFO_CLK_GATE, 10, 0,
> + 0),
[Severity: High]
Does defining two distinct clocks that map to the exact same gate bit
(shift 10) cause Common Clock Framework state corruption? Since CCF tracks
their states separately, if a driver disables CLK_WDG_APB, wouldn't it
silently turn off the physical clock for CLK_WDG_WDG as well? Should shared
gates be modeled as a single gate clock with multiple children?
> + /* timer */
> + CFG(CLK_TIMER_APB,
> + "audss_timer_apb",
> + timer_apb_parent,
> + NULL,
> + -1, 0, 0, 0,
> + INFO_CLK_DIV, 0, 2, 0,
> + INFO_CLK_GATE, 11, 0,
> + 0),
> + CFG(CLK_TIMER_TIMER,
> + "audss_timer_timer",
> + timer_timer_parent,
> + NULL,
> + -1, 0, 0, 0,
> + -1, 0, 0, 0,
> + INFO_CLK_GATE, 11, 0,
> + 0),
[Severity: High]
Similarly, does having CLK_TIMER_APB and CLK_TIMER_TIMER share gate shift 11
cause the same CCF state tracking issues?
[ ... ]
> + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, priv->clk_data);
> + if (ret) {
> + dev_err(dev, "failed to add clock provider: %d\n", ret);
> + goto fail_register;
> + }
> +
> + pm_runtime_put_sync(dev);
> +
> + return 0;
> +
> +fail_register:
[Severity: High]
Does using devm_of_clk_add_hw_provider() here create a use-after-free race
condition during device removal?
Since devres defers unregistration until after the remove function completes,
but sky1_audss_clk_remove() manually destroys the composite clocks, will a
concurrent clk_get() return a dangling pointer while the OF provider is still
active?
> +fail_array_clk:
> + while (i--)
> + clk_hw_unregister_composite(clk_table[devtype_data->clk_cfg[i].id]);
> +fail_fixed_clk:
> +fail_clks_set:
> + pm_runtime_put_sync(dev);
> + pm_runtime_disable(dev);
[Severity: Medium]
Will the parent clock enables be permanently leaked here if CONFIG_PM is
disabled?
The explicit sky1_audss_clks_enable() called earlier delegates teardown to
runtime PM wrappers like pm_runtime_put_sync(). If CONFIG_PM=n, these wrappers
are no-ops. Does the driver need explicit clock disablement when runtime PM
is unavailable?
> +err_link:
> + device_link_remove(dev, dev->parent);
> + return ret;
> +}
> +
> +static void sky1_audss_clk_remove(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct sky1_audss_clks_priv *priv = dev_get_drvdata(dev);
> + const struct sky1_audss_clks_devtype_data *devtype_data = priv->devtype_data;
> + int i = 0;
> +
> + for (i = 0; i < devtype_data->clk_cfg_size; i++)
> + clk_hw_unregister_composite(priv->clk_data->hws[devtype_data->clk_cfg[i].id]);
> +
> + if (!pm_runtime_status_suspended(dev))
> + pm_runtime_force_suspend(dev);
> +
> + pm_runtime_disable(dev);
[Severity: High]
Does calling pm_runtime_force_suspend() before pm_runtime_disable() create a
concurrency race condition?
pm_runtime_force_suspend() temporarily re-enables runtime PM before returning,
which opens a window where a concurrent operation could trigger a runtime
resume. Would it be safer to call pm_runtime_disable() first to ensure the
device stays suspended?
[Severity: Medium]
Also, similar to the probe error path, does delegating hardware disablement to
pm_runtime_force_suspend() leak the parent clocks on removal if CONFIG_PM is
disabled?
> + device_link_remove(dev, dev->parent);
> +}
> +
> +static int __maybe_unused sky1_audss_clk_runtime_suspend(struct device *dev)
> +{
> + struct sky1_audss_clks_priv *priv = dev_get_drvdata(dev);
> + const struct sky1_audss_clks_devtype_data *devtype_data = priv->devtype_data;
> + unsigned long flags;
> + int i;
> +
> + spin_lock_irqsave(&priv->lock, flags);
> + for (i = 0; i < devtype_data->reg_save_size; i++)
> + regmap_read(priv->regmap_cru,
> + devtype_data->reg_save[i][0], &devtype_data->reg_save[i][1]);
[Severity: Medium]
Does writing to devtype_data->reg_save[i][1] mutate the statically allocated
global array sky1_reg_save from a per-device suspend handler? Could this lead
to data corruption if there are multiple devices, and shouldn't we write to
dynamically allocated per-device state instead?
[ ... ]
> +static struct platform_driver sky1_audss_clk_driver = {
> + .probe = sky1_audss_clk_probe,
> + .remove = sky1_audss_clk_remove,
> + .driver = {
> + .name = "sky1-audss-clk",
> + .suppress_bind_attrs = true,
> + .of_match_table = sky1_audss_clk_of_match,
> + .pm = &sky1_audss_clk_pm_ops,
[Severity: Medium]
Will this unconditional assignment to .pm produce dead code when CONFIG_PM
is disabled? Should it be wrapped with pm_ptr(&sky1_audss_clk_pm_ops) to
allow the compiler to optimize out the unused structures?
> + },
> +};
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617060437.1474816-1-joakim.zhang@cixtech.com?part=4
^ permalink raw reply
* Re: [PATCH v4 2/5] reset: cix: add audss support to sky1 reset driver
From: sashiko-bot @ 2026-06-17 6:12 UTC (permalink / raw)
To: joakim.zhang; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260617060437.1474816-3-joakim.zhang@cixtech.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Using asynchronous `pm_runtime_put()` on the probe error path cancels the pending suspend, leaking the active power state.
- [High] Premature registration of the reset controller creates a race condition that can lead to bus faults or use-after-free.
- [High] The return value of `reset_control_deassert()` is ignored.
- [Low] Unconditional assignment of the `.pm` ops pointer generates dead code when PM is disabled.
--
commit 22eea5c604734dfc2f4c075cfefb3354c7756088
Author: Joakim Zhang <joakim.zhang@cixtech.com>
reset: cix: add audss support to sky1 reset driver
Extend the Sky1 reset controller driver for the AUDSS CRU syscon. The
AUDSS block provides sixteen active-low software reset bits in one
register for audio subsystem peripherals.
> diff --git a/drivers/reset/reset-sky1.c b/drivers/reset/reset-sky1.c
> index 78e80a533c399..462501c2ea066 100644
> --- a/drivers/reset/reset-sky1.c
> +++ b/drivers/reset/reset-sky1.c
[ ... ]
> @@ -343,12 +393,36 @@ static int sky1_reset_probe(struct platform_device *pdev)
> sky1src->rcdev.of_node = dev->of_node;
> sky1src->rcdev.dev = dev;
>
> - return devm_reset_controller_register(dev, &sky1src->rcdev);
> + ret = devm_reset_controller_register(dev, &sky1src->rcdev);
> + if (ret)
> + return ret;
> +
[Severity: High]
Does registering the reset controller this early in sky1_reset_probe()
expose it to the system before hardware and power management are fully
initialized?
If a concurrent consumer device attempts to trigger a reset before the
NOC reset is acquired and power management is enabled, could this
execute register operations against unpowered or uninitialized hardware?
If devm_reset_control_get_exclusive() later returns -EPROBE_DEFER, the
controller would be abruptly unregistered via devres while potentially
in use.
Could devm_reset_controller_register() be called as the final step in
probe() after all initialization is complete?
> + platform_set_drvdata(pdev, sky1src);
> +
> + if (of_device_is_compatible(dev->of_node, "cix,sky1-audss-system-control")) {
> + sky1src->rst_noc = devm_reset_control_get_exclusive(dev, NULL);
> + if (IS_ERR(sky1src->rst_noc))
> + return dev_err_probe(dev, PTR_ERR(sky1src->rst_noc),
> + "failed to get audss noc reset");
> +
> + pm_runtime_get_noresume(dev);
> + pm_runtime_set_active(dev);
> + devm_pm_runtime_enable(dev);
> +
> + reset_control_deassert(sky1src->rst_noc);
[Severity: High]
Does this ignore the integer return value from reset_control_deassert()?
If deassertion fails, the driver proceeds to call
devm_of_platform_populate(). Child devices could probe and attempt to
access their registers while the hardware is still stuck in reset.
Could we validate the return value of reset_control_deassert() before
proceeding?
> +
> + ret = devm_of_platform_populate(dev);
> + pm_runtime_put(dev);
> + return ret;
[Severity: High]
If devm_of_platform_populate() fails, this uses the asynchronous
pm_runtime_put() on the error path.
Since devm_pm_runtime_enable() was used, returning an error triggers
devres cleanup which executes pm_runtime_disable(). This cancels any
pending asynchronous idle/suspend work.
Because of this cancellation, sky1_reset_runtime_suspend() would never
execute, leaving the NOC reset permanently leaked in a deasserted state.
Should this use pm_runtime_put_sync() instead to guarantee synchronous
suspend before PM is disabled?
> + }
> +
> + return 0;
> }
[ ... ]
> @@ -358,6 +432,12 @@ static struct platform_driver sky1_reset_driver = {
> .driver = {
> .name = "cix,sky1-rst",
> .of_match_table = sky1_sysreg_of_match,
> + .pm = &(const struct dev_pm_ops){
> + SET_RUNTIME_PM_OPS(sky1_reset_runtime_suspend,
> + sky1_reset_runtime_resume, NULL)
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> + },
[Severity: Low]
Does this unconditional assignment of the .pm pointer generate dead code
when CONFIG_PM is disabled?
The structure and statically referenced callbacks get compiled into the
kernel even though they can never execute.
Could this use the pm_ptr() wrapper macro to allow the compiler to
eliminate the unused structure?
> },
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617060437.1474816-1-joakim.zhang@cixtech.com?part=2
^ permalink raw reply
* Re: [PATCH v4 1/5] dt-bindings: soc: cix,sky1-system-control: add audss system control
From: sashiko-bot @ 2026-06-17 6:10 UTC (permalink / raw)
To: joakim.zhang; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260617060437.1474816-2-joakim.zhang@cixtech.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `cix,sky1-audss-system-control` compatible acts as a reset provider but does not require the `#reset-cells` property in its conditional block.
--
commit bc475a5f4ccc93d6c4b68c75338adb8eb56b01b2
Author: Joakim Zhang <joakim.zhang@cixtech.com>
dt-bindings: soc: cix,sky1-system-control: add audss system control
This commit adds device tree bindings for the Cix Sky1 Audio Subsystem
(AUDSS) system control block. It introduces the new compatible string
cix,sky1-audss-system-control and defines constraints for its child
clock-controller, power domain, and resets.
> diff --git a/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.yaml b/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.yaml
> index a01a515222c6d..5a1cd5c24ade7 100644
> --- a/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.yaml
> +++ b/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.yaml
[ ... ]
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: cix,sky1-audss-system-control
> + then:
> + required:
> + - clock-controller
> + - power-domains
> + - resets
[Severity: Medium]
Since the cix,sky1-audss-system-control compatible acts as a reset provider,
should #reset-cells also be included in this required list?
> + else:
> + properties:
> + clock-controller: false
> + power-domains: false
> + resets: false
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617060437.1474816-1-joakim.zhang@cixtech.com?part=1
^ permalink raw reply
* Re: [PATCH v4 3/5] dt-bindings: clock: cix,sky1-audss-clock: add audss clock controller
From: sashiko-bot @ 2026-06-17 6:08 UTC (permalink / raw)
To: joakim.zhang; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260617060437.1474816-4-joakim.zhang@cixtech.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Typo in the clocks property description: 'delicated' instead of 'dedicated'.
--
commit 0bf88f97ab11d2c8010b886d37df1eed62b3d256
Author: Joakim Zhang <joakim.zhang@cixtech.com>
dt-bindings: clock: cix,sky1-audss-clock: add audss clock controller
This commit adds a devicetree binding for the Cix Sky1 audio subsystem
(AUDSS) clock controller. It acts as a child of the system control
syscon, routing 4 reference clocks to internal consumers.
> diff --git a/Documentation/devicetree/bindings/clock/cix,sky1-audss-clock.yaml b/Documentation/devicetree/bindings/clock/cix,sky1-audss-clock.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/cix,sky1-audss-clock.yaml
[ ... ]
> + clocks:
> + items:
> + - description: I2S parent clock for sampling rates multiple of 8kHz.
> + - description: I2S parent clock for sampling rates multiple of 11.025kHz.
> + - description: clock feeding most devices in audss (NOC, DSP, SRAM, HDA, DMAC, I2S, and Mailbox).
> + - description: clock feeding for HDA, Timer and Watchdog, which is a delicated 48MHz clock.
[Severity: Low]
This isn't a bug, but there is a typo here where delicated is used instead of
dedicated.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617060437.1474816-1-joakim.zhang@cixtech.com?part=3
^ permalink raw reply
* [PATCH v4 4/5] clk: cix: add sky1 audss clock controller
From: joakim.zhang @ 2026-06-17 6:04 UTC (permalink / raw)
To: mturquette, sboyd, bmasney, robh, krzk+dt, conor+dt, p.zabel,
gary.yang
Cc: cix-kernel-upstream, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, Joakim Zhang
In-Reply-To: <20260617060437.1474816-1-joakim.zhang@cixtech.com>
From: Joakim Zhang <joakim.zhang@cixtech.com>
Add a platform driver for the Cix Sky1 Audio Subsystem (AUDSS) internal
clock controller. The driver binds to a cix,sky1-audss-clock device tree
node under the AUDSS syscon, obtains the parent regmap via
syscon_node_to_regmap(), and registers mux/divider/gate composite clocks
for DSP, SRAM, HDA, DMAC, watchdog, timer, mailbox and I2S outputs. Four
SoC-level audio reference clocks are brought up as inputs to the tree.
Signed-off-by: Joakim Zhang <joakim.zhang@cixtech.com>
---
drivers/clk/Kconfig | 1 +
drivers/clk/Makefile | 1 +
drivers/clk/cix/Kconfig | 16 +
drivers/clk/cix/Makefile | 3 +
drivers/clk/cix/clk-sky1-audss.c | 1167 ++++++++++++++++++++++++++++++
5 files changed, 1188 insertions(+)
create mode 100644 drivers/clk/cix/Kconfig
create mode 100644 drivers/clk/cix/Makefile
create mode 100644 drivers/clk/cix/clk-sky1-audss.c
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 1717ce75a907..cfcaab39068a 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -509,6 +509,7 @@ source "drivers/clk/actions/Kconfig"
source "drivers/clk/analogbits/Kconfig"
source "drivers/clk/aspeed/Kconfig"
source "drivers/clk/bcm/Kconfig"
+source "drivers/clk/cix/Kconfig"
source "drivers/clk/eswin/Kconfig"
source "drivers/clk/hisilicon/Kconfig"
source "drivers/clk/imgtec/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index cc108a75a900..87c992f0df54 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -119,6 +119,7 @@ obj-$(CONFIG_ARCH_ARTPEC) += axis/
obj-$(CONFIG_ARC_PLAT_AXS10X) += axs10x/
obj-y += bcm/
obj-$(CONFIG_ARCH_BERLIN) += berlin/
+obj-y += cix/
obj-$(CONFIG_ARCH_DAVINCI) += davinci/
obj-$(CONFIG_COMMON_CLK_ESWIN) += eswin/
obj-$(CONFIG_ARCH_HISI) += hisilicon/
diff --git a/drivers/clk/cix/Kconfig b/drivers/clk/cix/Kconfig
new file mode 100644
index 000000000000..3909796f8656
--- /dev/null
+++ b/drivers/clk/cix/Kconfig
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0
+# Audio subsystem clock support for Cixtech SoC family
+menu "Clock support for Cixtech audss"
+
+config CLK_SKY1_AUDSS
+ tristate "Cixtech Sky1 Audio Subsystem Clock Driver"
+ depends on ARCH_CIX || COMPILE_TEST
+ select MFD_SYSCON
+ select REGMAP_MMIO
+ select RESET_CONTROLLER
+ help
+ Support for the Audio Subsystem clock controller present on
+ Cixtech Sky1 SoC. This driver provides mux, divider and gate
+ clocks for DSP, I2S, HDA and related blocks in the audio
+ subsystem. Say M or Y here if you want to build this driver.
+endmenu
diff --git a/drivers/clk/cix/Makefile b/drivers/clk/cix/Makefile
new file mode 100644
index 000000000000..bc612f1d08b2
--- /dev/null
+++ b/drivers/clk/cix/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_CLK_SKY1_AUDSS) += clk-sky1-audss.o
diff --git a/drivers/clk/cix/clk-sky1-audss.c b/drivers/clk/cix/clk-sky1-audss.c
new file mode 100644
index 000000000000..c1e7dfe45d12
--- /dev/null
+++ b/drivers/clk/cix/clk-sky1-audss.c
@@ -0,0 +1,1167 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright 2026 Cix Technology Group Co., Ltd.
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/clock/cix,sky1-audss-clock.h>
+
+#define INFO_HIFI0 0x00
+#define INFO_CLK_GATE 0x10
+#define INFO_CLK_DIV 0x14
+#define INFO_CLK_MUX 0x18
+#define INFO_MCLK 0x70
+
+#define SKY1_AUDSS_CLK_PARENTS_CNT 4
+#define SKY1_AUDSS_NUM_CLKS (CLK_MCLK4 + 1)
+
+static u32 sky1_reg_save[][2] = {
+ { INFO_HIFI0, 0 },
+ { INFO_CLK_GATE, 0 },
+ { INFO_CLK_DIV, 0 },
+ { INFO_CLK_MUX, 0 },
+ { INFO_MCLK, 0 },
+};
+
+static const char * const sky1_audss_clk_names[SKY1_AUDSS_CLK_PARENTS_CNT] = {
+ "x8k", "x11k", "sys", "48m",
+};
+
+static const u32 sky1_clk_rate_default[SKY1_AUDSS_CLK_PARENTS_CNT] = {
+ 294912000,
+ 270950400,
+ 800000000,
+ 48000000,
+};
+
+static const char * const dsp_clk_parent[] = {
+ "audio_clk4"
+};
+
+static const char * const dsp_bclk_parent[] = {
+ "audio_clk4_div2"
+};
+
+static const char * const dsp_pbclk_parent[] = {
+ "audio_clk4_div4"
+};
+
+static const char * const sram_axi_parent[] = {
+ "audio_clk4_div2"
+};
+
+static const char * const hda_sys_parent[] = {
+ "audio_clk4_div2"
+};
+
+static const char * const hda_hda_parent[] = {
+ "audio_clk5"
+};
+
+static const char * const dmac_axi_parent[] = {
+ "audio_clk4_div2"
+};
+
+static const char * const wdg_apb_parent[] = {
+ "audio_clk5_div2"
+};
+
+static const char * const wdg_wdg_parent[] = {
+ "audio_clk5_div2"
+};
+
+static const char * const timer_apb_parent[] = {
+ "audio_clk4_div4"
+};
+
+static const char * const timer_timer_parent[] = {
+ "audio_clk5_div2"
+};
+
+static const char * const mailbox_apb_parent[] = {
+ "audio_clk4_div4"
+};
+
+static const char * const i2s_apb_parent[] = {
+ "audio_clk4_div4"
+};
+
+static const char * const i2s0_parents[] = {
+ "audio_clk0", "audio_clk2"
+};
+
+static const char * const i2s1_parents[] = {
+ "audio_clk0", "audio_clk2"
+};
+
+static const char * const i2s2_parents[] = {
+ "audio_clk0", "audio_clk2"
+};
+
+static const char * const i2s3_parents[] = {
+ "audio_clk0", "audio_clk2"
+};
+
+static const char * const i2s4_parents[] = {
+ "audio_clk0", "audio_clk2"
+};
+
+static const char * const i2s5_parents[] = {
+ "audio_clk0", "audio_clk2"
+};
+
+static const char * const i2s6_parents[] = {
+ "audio_clk0", "audio_clk2"
+};
+
+static const char * const i2s7_parents[] = {
+ "audio_clk0", "audio_clk2"
+};
+
+static const char * const i2s8_parents[] = {
+ "audio_clk0", "audio_clk2"
+};
+
+static const char * const i2s9_parents[] = {
+ "audio_clk0", "audio_clk2"
+};
+
+static const char * const mclk_parents[] = {
+ "audio_clk0", "audio_clk2"
+};
+
+static const u32 i2s3_mux_table[] = { 0, 2 };
+static const u32 i2s4_mux_table[] = { 0, 2 };
+
+/*
+ * audss composite clock definition
+ */
+struct muxdiv_cfg {
+ int offset;
+ u8 shift;
+ u8 width;
+ u8 flags;
+};
+
+struct gate_cfg {
+ int offset;
+ u8 shift;
+ u8 flags;
+};
+
+struct composite_clk_cfg {
+ u32 id;
+ const char * const name;
+ const char * const *parent_names;
+ int num_parents;
+ const u32 *mux_table;
+ struct muxdiv_cfg *mux_cfg;
+ struct muxdiv_cfg *div_cfg;
+ struct gate_cfg *gate_cfg;
+ unsigned long flags;
+};
+
+#define CFG(_id,\
+ _name,\
+ _parent_names,\
+ _mux_table,\
+ _mux_offset, _mux_shift, _mux_width, _mux_flags,\
+ _div_offset, _div_shift, _div_width, _div_flags,\
+ _gate_offset, _gate_shift, _gate_flags,\
+ _flags)\
+{\
+ .id = _id,\
+ .name = _name,\
+ .parent_names = _parent_names,\
+ .num_parents = ARRAY_SIZE(_parent_names),\
+ .mux_table = _mux_table,\
+ .mux_cfg = &(struct muxdiv_cfg) { _mux_offset, _mux_shift, _mux_width, _mux_flags },\
+ .div_cfg = &(struct muxdiv_cfg) { _div_offset, _div_shift, _div_width, _div_flags },\
+ .gate_cfg = &(struct gate_cfg) { _gate_offset, _gate_shift, _gate_flags },\
+ .flags = _flags,\
+}
+
+static const struct composite_clk_cfg sky1_audss_clks[] = {
+ /* dsp */
+ CFG(CLK_DSP_CLK,
+ "audss_dsp_clk",
+ dsp_clk_parent,
+ NULL,
+ -1, 0, 0, 0,
+ INFO_CLK_DIV, 0, 2, 0,
+ INFO_HIFI0, 0, 0,
+ 0),
+ CFG(CLK_DSP_BCLK,
+ "audss_dsp_bclk",
+ dsp_bclk_parent,
+ NULL,
+ -1, 0, 0, 0,
+ INFO_CLK_DIV, 0, 2, 0,
+ -1, 0, 0,
+ 0),
+ CFG(CLK_DSP_PBCLK,
+ "audss_dsp_pbclk",
+ dsp_pbclk_parent,
+ NULL,
+ -1, 0, 0, 0,
+ INFO_CLK_DIV, 0, 2, 0,
+ -1, 0, 0,
+ 0),
+ /* sram */
+ CFG(CLK_SRAM_AXI,
+ "audss_sram_axi",
+ sram_axi_parent,
+ NULL,
+ -1, 0, 0, 0,
+ INFO_CLK_DIV, 0, 2, 0,
+ INFO_CLK_GATE, 16, 0,
+ 0),
+ /* hda */
+ CFG(CLK_HDA_SYS,
+ "audss_hda_sys",
+ hda_sys_parent,
+ NULL,
+ -1, 0, 0, 0,
+ INFO_CLK_DIV, 0, 2, 0,
+ INFO_CLK_GATE, 14, 0,
+ 0),
+ CFG(CLK_HDA_HDA,
+ "audss_hda_hda",
+ hda_hda_parent,
+ NULL,
+ -1, 0, 0, 0,
+ -1, 0, 0, 0,
+ INFO_CLK_GATE, 14, 0,
+ 0),
+ /* dmac */
+ CFG(CLK_DMAC_AXI,
+ "audss_dmac_axi",
+ dmac_axi_parent,
+ NULL,
+ -1, 0, 0, 0,
+ INFO_CLK_DIV, 0, 2, 0,
+ INFO_CLK_GATE, 15, 0,
+ 0),
+ /* wdg */
+ CFG(CLK_WDG_APB,
+ "audss_wdg_apb",
+ wdg_apb_parent,
+ NULL,
+ -1, 0, 0, 0,
+ -1, 0, 0, 0,
+ INFO_CLK_GATE, 10, 0,
+ 0),
+ CFG(CLK_WDG_WDG,
+ "audss_wdg_wdg",
+ wdg_wdg_parent,
+ NULL,
+ -1, 0, 0, 0,
+ -1, 0, 0, 0,
+ INFO_CLK_GATE, 10, 0,
+ 0),
+ /* timer */
+ CFG(CLK_TIMER_APB,
+ "audss_timer_apb",
+ timer_apb_parent,
+ NULL,
+ -1, 0, 0, 0,
+ INFO_CLK_DIV, 0, 2, 0,
+ INFO_CLK_GATE, 11, 0,
+ 0),
+ CFG(CLK_TIMER_TIMER,
+ "audss_timer_timer",
+ timer_timer_parent,
+ NULL,
+ -1, 0, 0, 0,
+ -1, 0, 0, 0,
+ INFO_CLK_GATE, 11, 0,
+ 0),
+ /* mailbox: mb0(ap->dsp), mb1(dsp->ap) */
+ CFG(CLK_MB_0_APB,
+ "audss_mb_0_apb",
+ mailbox_apb_parent,
+ NULL,
+ -1, 0, 0, 0,
+ -1, 0, 0, 0,
+ INFO_CLK_GATE, 12, 0,
+ 0),
+ CFG(CLK_MB_1_APB,
+ "audss_mb_1_apb",
+ mailbox_apb_parent,
+ NULL,
+ -1, 0, 0, 0,
+ -1, 0, 0, 0,
+ INFO_CLK_GATE, 13, 0,
+ 0),
+ /* i2s */
+ CFG(CLK_I2S0_APB,
+ "audss_i2s0_apb",
+ i2s_apb_parent,
+ NULL,
+ -1, 0, 0, 0,
+ INFO_CLK_DIV, 0, 2, 0,
+ INFO_CLK_GATE, 0, 0,
+ 0),
+ CFG(CLK_I2S1_APB,
+ "audss_i2s1_apb",
+ i2s_apb_parent,
+ NULL,
+ -1, 0, 0, 0,
+ INFO_CLK_DIV, 0, 2, 0,
+ INFO_CLK_GATE, 1, 0,
+ 0),
+ CFG(CLK_I2S2_APB,
+ "audss_i2s2_apb",
+ i2s_apb_parent,
+ NULL,
+ -1, 0, 0, 0,
+ INFO_CLK_DIV, 0, 2, 0,
+ INFO_CLK_GATE, 2, 0,
+ 0),
+ CFG(CLK_I2S3_APB,
+ "audss_i2s3_apb",
+ i2s_apb_parent,
+ NULL,
+ -1, 0, 0, 0,
+ INFO_CLK_DIV, 0, 2, 0,
+ INFO_CLK_GATE, 3, 0,
+ 0),
+ CFG(CLK_I2S4_APB,
+ "audss_i2s4_apb",
+ i2s_apb_parent,
+ NULL,
+ -1, 0, 0, 0,
+ INFO_CLK_DIV, 0, 2, 0,
+ INFO_CLK_GATE, 4, 0,
+ 0),
+ CFG(CLK_I2S5_APB,
+ "audss_i2s5_apb",
+ i2s_apb_parent,
+ NULL,
+ -1, 0, 0, 0,
+ INFO_CLK_DIV, 0, 2, 0,
+ INFO_CLK_GATE, 5, 0,
+ 0),
+ CFG(CLK_I2S6_APB,
+ "audss_i2s6_apb",
+ i2s_apb_parent,
+ NULL,
+ -1, 0, 0, 0,
+ INFO_CLK_DIV, 0, 2, 0,
+ INFO_CLK_GATE, 6, 0,
+ 0),
+ CFG(CLK_I2S7_APB,
+ "audss_i2s7_apb",
+ i2s_apb_parent,
+ NULL,
+ -1, 0, 0, 0,
+ INFO_CLK_DIV, 0, 2, 0,
+ INFO_CLK_GATE, 7, 0,
+ 0),
+ CFG(CLK_I2S8_APB,
+ "audss_i2s8_apb",
+ i2s_apb_parent,
+ NULL,
+ -1, 0, 0, 0,
+ INFO_CLK_DIV, 0, 2, 0,
+ INFO_CLK_GATE, 8, 0,
+ 0),
+ CFG(CLK_I2S9_APB,
+ "audss_i2s9_apb",
+ i2s_apb_parent,
+ NULL,
+ -1, 0, 0, 0,
+ INFO_CLK_DIV, 0, 2, 0,
+ INFO_CLK_GATE, 9, 0,
+ 0),
+ CFG(CLK_I2S0,
+ "audss_i2s0",
+ i2s0_parents,
+ NULL,
+ INFO_CLK_MUX, 0, 2, 0,
+ INFO_CLK_DIV, 2, 2, 0,
+ INFO_CLK_GATE, 0, 0,
+ 0),
+ CFG(CLK_I2S1,
+ "audss_i2s1",
+ i2s1_parents,
+ NULL,
+ INFO_CLK_MUX, 2, 2, 0,
+ INFO_CLK_DIV, 4, 2, 0,
+ INFO_CLK_GATE, 1, 0,
+ 0),
+ CFG(CLK_I2S2,
+ "audss_i2s2",
+ i2s2_parents,
+ NULL,
+ INFO_CLK_MUX, 4, 2, 0,
+ INFO_CLK_DIV, 6, 2, 0,
+ INFO_CLK_GATE, 2, 0,
+ 0),
+ CFG(CLK_I2S3,
+ "audss_i2s3",
+ i2s3_parents,
+ i2s3_mux_table,
+ INFO_CLK_MUX, 6, 2, 0,
+ INFO_CLK_DIV, 8, 2, 0,
+ INFO_CLK_GATE, 3, 0,
+ 0),
+ CFG(CLK_I2S4,
+ "audss_i2s4",
+ i2s4_parents,
+ i2s4_mux_table,
+ INFO_CLK_MUX, 8, 2, 0,
+ INFO_CLK_DIV, 10, 2, 0,
+ INFO_CLK_GATE, 4, 0,
+ 0),
+ CFG(CLK_I2S5,
+ "audss_i2s5",
+ i2s5_parents,
+ NULL,
+ INFO_CLK_MUX, 10, 2, 0,
+ INFO_CLK_DIV, 12, 2, 0,
+ INFO_CLK_GATE, 5, 0,
+ 0),
+ CFG(CLK_I2S6,
+ "audss_i2s6",
+ i2s6_parents,
+ NULL,
+ INFO_CLK_MUX, 12, 2, 0,
+ INFO_CLK_DIV, 14, 2, 0,
+ INFO_CLK_GATE, 6, 0,
+ 0),
+ CFG(CLK_I2S7,
+ "audss_i2s7",
+ i2s7_parents,
+ NULL,
+ INFO_CLK_MUX, 14, 2, 0,
+ INFO_CLK_DIV, 16, 2, 0,
+ INFO_CLK_GATE, 7, 0,
+ 0),
+ CFG(CLK_I2S8,
+ "audss_i2s8",
+ i2s8_parents,
+ NULL,
+ INFO_CLK_MUX, 16, 2, 0,
+ INFO_CLK_DIV, 18, 2, 0,
+ INFO_CLK_GATE, 8, 0,
+ 0),
+ CFG(CLK_I2S9,
+ "audss_i2s9",
+ i2s9_parents,
+ NULL,
+ INFO_CLK_MUX, 18, 2, 0,
+ INFO_CLK_DIV, 20, 2, 0,
+ INFO_CLK_GATE, 9, 0,
+ 0),
+ /* mclk */
+ CFG(CLK_MCLK0,
+ "audss_mclk0",
+ mclk_parents,
+ NULL,
+ INFO_MCLK, 5, 1, 0,
+ -1, 0, 0, 0,
+ INFO_MCLK, 0, 0,
+ 0),
+ CFG(CLK_MCLK1,
+ "audss_mclk1",
+ mclk_parents,
+ NULL,
+ INFO_MCLK, 6, 1, 0,
+ -1, 0, 0, 0,
+ INFO_MCLK, 1, 0,
+ 0),
+ CFG(CLK_MCLK2,
+ "audss_mclk2",
+ mclk_parents,
+ NULL,
+ INFO_MCLK, 7, 1, 0,
+ -1, 0, 0, 0,
+ INFO_MCLK, 2, 0,
+ 0),
+ CFG(CLK_MCLK3,
+ "audss_mclk3",
+ mclk_parents,
+ NULL,
+ INFO_MCLK, 8, 1, 0,
+ -1, 0, 0, 0,
+ INFO_MCLK, 3, 0,
+ 0),
+ CFG(CLK_MCLK4,
+ "audss_mclk4",
+ mclk_parents,
+ NULL,
+ INFO_MCLK, 9, 1, 0,
+ -1, 0, 0, 0,
+ INFO_MCLK, 4, 0,
+ 0),
+};
+
+struct sky1_audss_clks_devtype_data {
+ u32 (*reg_save)[2];
+ size_t reg_save_size;
+ const char * const *clk_names;
+ size_t clk_num;
+ const u32 *clk_rate_default;
+ const struct composite_clk_cfg *clk_cfg;
+ size_t clk_cfg_size;
+};
+
+struct sky1_audss_clks_priv {
+ struct device *dev;
+ struct regmap *regmap_cru;
+ struct clk *clks[SKY1_AUDSS_CLK_PARENTS_CNT];
+ const struct sky1_audss_clks_devtype_data *devtype_data;
+ spinlock_t lock;
+ struct clk_hw_onecell_data *clk_data;
+};
+
+/*
+ * clk_ops for audss clock mux/divider/gate
+ */
+struct sky1_clk_divider {
+ struct clk_divider div;
+ struct regmap *regmap;
+ int offset;
+};
+
+struct sky1_clk_gate {
+ struct clk_gate gate;
+ struct regmap *regmap;
+ int offset;
+};
+
+struct sky1_clk_mux {
+ struct clk_mux mux;
+ struct regmap *regmap;
+ int offset;
+};
+
+static inline struct sky1_clk_mux *to_sky1_clk_mux(struct clk_mux *mux)
+{
+ return container_of(mux, struct sky1_clk_mux, mux);
+}
+
+static u8 sky1_audss_clk_mux_get_parent(struct clk_hw *hw)
+{
+ struct clk_mux *mux = to_clk_mux(hw);
+ struct sky1_clk_mux *sky1_mux = to_sky1_clk_mux(mux);
+ u32 val;
+
+ regmap_read(sky1_mux->regmap, sky1_mux->offset, &val);
+ val = val >> mux->shift;
+ val &= mux->mask;
+
+ return clk_mux_val_to_index(hw, mux->table, mux->flags, val);
+}
+
+static int sky1_audss_clk_mux_set_parent(struct clk_hw *hw, u8 index)
+{
+ struct clk_mux *mux = to_clk_mux(hw);
+ u32 val = clk_mux_index_to_val(mux->table, mux->flags, index);
+ struct sky1_clk_mux *sky1_mux = to_sky1_clk_mux(mux);
+ unsigned long flags = 0;
+ u32 reg;
+
+ if (mux->lock)
+ spin_lock_irqsave(mux->lock, flags);
+ else
+ __acquire(mux->lock);
+
+ if (mux->flags & CLK_MUX_HIWORD_MASK) {
+ reg = mux->mask << (mux->shift + 16);
+ } else {
+ regmap_read(sky1_mux->regmap, sky1_mux->offset, ®);
+ reg &= ~(mux->mask << mux->shift);
+ }
+ val = val << mux->shift;
+ reg |= val;
+ regmap_write(sky1_mux->regmap, sky1_mux->offset, reg);
+
+ if (mux->lock)
+ spin_unlock_irqrestore(mux->lock, flags);
+ else
+ __release(mux->lock);
+
+ return 0;
+}
+
+static int sky1_audss_clk_mux_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ struct clk_mux *mux = to_clk_mux(hw);
+
+ return clk_mux_determine_rate_flags(hw, req, mux->flags);
+}
+
+static const struct clk_ops sky1_audss_clk_mux_ops = {
+ .get_parent = sky1_audss_clk_mux_get_parent,
+ .set_parent = sky1_audss_clk_mux_set_parent,
+ .determine_rate = sky1_audss_clk_mux_determine_rate,
+};
+
+static inline struct sky1_clk_divider *to_sky1_clk_divider(struct clk_divider *div)
+{
+ return container_of(div, struct sky1_clk_divider, div);
+}
+
+static unsigned long sky1_audss_clk_divider_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct clk_divider *divider = to_clk_divider(hw);
+ struct sky1_clk_divider *sky1_div = to_sky1_clk_divider(divider);
+ unsigned int val;
+
+ regmap_read(sky1_div->regmap, sky1_div->offset, &val);
+ val = val >> divider->shift;
+ val &= clk_div_mask(divider->width);
+
+ return divider_recalc_rate(hw, parent_rate, val, divider->table,
+ divider->flags, divider->width);
+}
+
+static int sky1_audss_clk_divider_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ struct clk_divider *divider = to_clk_divider(hw);
+ struct sky1_clk_divider *sky1_div = to_sky1_clk_divider(divider);
+
+ /* if read only, just return current value */
+ if (divider->flags & CLK_DIVIDER_READ_ONLY) {
+ u32 val;
+
+ regmap_read(sky1_div->regmap, sky1_div->offset, &val);
+ val = val >> divider->shift;
+ val &= clk_div_mask(divider->width);
+
+ return divider_ro_determine_rate(hw, req, divider->table,
+ divider->width,
+ divider->flags, val);
+ }
+
+ return divider_determine_rate(hw, req, divider->table, divider->width,
+ divider->flags);
+}
+
+static int sky1_audss_clk_divider_set_rate(struct clk_hw *hw,
+ unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct clk_divider *divider = to_clk_divider(hw);
+ struct sky1_clk_divider *sky1_div = to_sky1_clk_divider(divider);
+ int value;
+ unsigned long flags = 0;
+ u32 val;
+
+ value = divider_get_val(rate, parent_rate, divider->table,
+ divider->width, divider->flags);
+ if (value < 0)
+ return value;
+
+ if (divider->lock)
+ spin_lock_irqsave(divider->lock, flags);
+ else
+ __acquire(divider->lock);
+
+ if (divider->flags & CLK_DIVIDER_HIWORD_MASK) {
+ val = clk_div_mask(divider->width) << (divider->shift + 16);
+ } else {
+ regmap_read(sky1_div->regmap, sky1_div->offset, &val);
+ val &= ~(clk_div_mask(divider->width) << divider->shift);
+ }
+ val |= (u32)value << divider->shift;
+ regmap_write(sky1_div->regmap, sky1_div->offset, val);
+
+ if (divider->lock)
+ spin_unlock_irqrestore(divider->lock, flags);
+ else
+ __release(divider->lock);
+
+ return 0;
+}
+
+static const struct clk_ops sky1_audss_clk_divider_ops = {
+ .recalc_rate = sky1_audss_clk_divider_recalc_rate,
+ .determine_rate = sky1_audss_clk_divider_determine_rate,
+ .set_rate = sky1_audss_clk_divider_set_rate,
+};
+
+static inline struct sky1_clk_gate *to_sky1_clk_gate(struct clk_gate *gate)
+{
+ return container_of(gate, struct sky1_clk_gate, gate);
+}
+
+static void sky1_audss_clk_gate_endisable(struct clk_hw *hw, int enable)
+{
+ struct clk_gate *gate = to_clk_gate(hw);
+ struct sky1_clk_gate *sky1_gate = to_sky1_clk_gate(gate);
+ int set = gate->flags & CLK_GATE_SET_TO_DISABLE ? 1 : 0;
+ unsigned long flags = 0;
+ u32 reg;
+
+ set ^= enable;
+
+ if (gate->lock)
+ spin_lock_irqsave(gate->lock, flags);
+ else
+ __acquire(gate->lock);
+
+ if (gate->flags & CLK_GATE_HIWORD_MASK) {
+ reg = BIT(gate->bit_idx + 16);
+ if (set)
+ reg |= BIT(gate->bit_idx);
+ } else {
+ regmap_read(sky1_gate->regmap, sky1_gate->offset, ®);
+
+ if (set)
+ reg |= BIT(gate->bit_idx);
+ else
+ reg &= ~BIT(gate->bit_idx);
+ }
+
+ regmap_write(sky1_gate->regmap, sky1_gate->offset, reg);
+
+ if (gate->lock)
+ spin_unlock_irqrestore(gate->lock, flags);
+ else
+ __release(gate->lock);
+}
+
+static int sky1_audss_clk_gate_enable(struct clk_hw *hw)
+{
+ sky1_audss_clk_gate_endisable(hw, 1);
+
+ return 0;
+}
+
+static void sky1_audss_clk_gate_disable(struct clk_hw *hw)
+{
+ sky1_audss_clk_gate_endisable(hw, 0);
+}
+
+static int sky1_audss_clk_gate_is_enabled(struct clk_hw *hw)
+{
+ struct clk_gate *gate = to_clk_gate(hw);
+ struct sky1_clk_gate *sky1_gate = to_sky1_clk_gate(gate);
+ u32 reg;
+
+ regmap_read(sky1_gate->regmap, sky1_gate->offset, ®);
+
+ /* if a set bit disables this clk, flip it before masking */
+ if (gate->flags & CLK_GATE_SET_TO_DISABLE)
+ reg ^= BIT(gate->bit_idx);
+
+ reg &= BIT(gate->bit_idx);
+
+ return !!reg;
+}
+
+static const struct clk_ops sky1_audss_clk_gate_ops = {
+ .enable = sky1_audss_clk_gate_enable,
+ .disable = sky1_audss_clk_gate_disable,
+ .is_enabled = sky1_audss_clk_gate_is_enabled,
+};
+
+static struct clk_hw *sky1_audss_clk_register(struct device *dev,
+ const char *name,
+ const char * const *parent_names,
+ int num_parents,
+ struct regmap *regmap,
+ const u32 *mux_table,
+ struct muxdiv_cfg *mux_cfg,
+ struct muxdiv_cfg *div_cfg,
+ struct gate_cfg *gate_cfg,
+ unsigned long flags,
+ spinlock_t *lock)
+{
+ const struct clk_ops *sky1_mux_ops = NULL;
+ const struct clk_ops *sky1_div_ops = NULL;
+ const struct clk_ops *sky1_gate_ops = NULL;
+ struct clk_hw *hw = ERR_PTR(-ENOMEM);
+ struct sky1_clk_divider *sky1_div = NULL;
+ struct sky1_clk_gate *sky1_gate = NULL;
+ struct sky1_clk_mux *sky1_mux = NULL;
+
+ if (mux_cfg->offset >= 0) {
+ sky1_mux = devm_kzalloc(dev, sizeof(*sky1_mux), GFP_KERNEL);
+ if (!sky1_mux)
+ return ERR_PTR(-ENOMEM);
+
+ sky1_mux->mux.reg = NULL;
+ sky1_mux->mux.shift = mux_cfg->shift;
+ sky1_mux->mux.mask = BIT(mux_cfg->width) - 1;
+ sky1_mux->mux.flags = mux_cfg->flags;
+ sky1_mux->mux.table = mux_table;
+ sky1_mux->mux.lock = lock;
+ sky1_mux_ops = &sky1_audss_clk_mux_ops;
+ sky1_mux->regmap = regmap;
+ sky1_mux->offset = mux_cfg->offset;
+ }
+
+ if (div_cfg->offset >= 0) {
+ sky1_div = devm_kzalloc(dev, sizeof(*sky1_div), GFP_KERNEL);
+ if (!sky1_div)
+ return ERR_PTR(-ENOMEM);
+
+ sky1_div->div.reg = NULL;
+ sky1_div->div.shift = div_cfg->shift;
+ sky1_div->div.width = div_cfg->width;
+ sky1_div->div.flags = div_cfg->flags | CLK_DIVIDER_POWER_OF_TWO;
+ sky1_div->div.lock = lock;
+ sky1_div_ops = &sky1_audss_clk_divider_ops;
+ sky1_div->regmap = regmap;
+ sky1_div->offset = div_cfg->offset;
+ }
+
+ if (gate_cfg->offset >= 0) {
+ sky1_gate = devm_kzalloc(dev, sizeof(*sky1_gate), GFP_KERNEL);
+ if (!sky1_gate)
+ return ERR_PTR(-ENOMEM);
+
+ sky1_gate->gate.reg = NULL;
+ sky1_gate->gate.bit_idx = gate_cfg->shift;
+ sky1_gate->gate.flags = gate_cfg->flags;
+ sky1_gate->gate.lock = lock;
+ sky1_gate_ops = &sky1_audss_clk_gate_ops;
+ sky1_gate->regmap = regmap;
+ sky1_gate->offset = gate_cfg->offset;
+ }
+
+ hw = clk_hw_register_composite(dev, name, parent_names, num_parents,
+ sky1_mux ? &sky1_mux->mux.hw : NULL, sky1_mux_ops,
+ sky1_div ? &sky1_div->div.hw : NULL, sky1_div_ops,
+ sky1_gate ? &sky1_gate->gate.hw : NULL, sky1_gate_ops,
+ flags);
+ if (IS_ERR(hw)) {
+ dev_err(dev, "register %s clock failed with err = %ld\n",
+ name, PTR_ERR(hw));
+ return hw;
+ }
+
+ return hw;
+}
+
+static int sky1_audss_clks_get(struct sky1_audss_clks_priv *priv)
+{
+ const struct sky1_audss_clks_devtype_data *devtype_data = priv->devtype_data;
+ int i;
+
+ for (i = 0; i < devtype_data->clk_num; i++) {
+ priv->clks[i] = devm_clk_get(priv->dev, devtype_data->clk_names[i]);
+ if (IS_ERR(priv->clks[i]))
+ return dev_err_probe(priv->dev, PTR_ERR(priv->clks[i]),
+ "failed to get clock %s", devtype_data->clk_names[i]);
+ }
+
+ return 0;
+}
+
+static int sky1_audss_clks_enable(struct sky1_audss_clks_priv *priv)
+{
+ const struct sky1_audss_clks_devtype_data *devtype_data = priv->devtype_data;
+ int i, err;
+
+ for (i = 0; i < devtype_data->clk_num; i++) {
+ err = clk_prepare_enable(priv->clks[i]);
+ if (err) {
+ dev_err(priv->dev, "failed to enable clock %s\n",
+ devtype_data->clk_names[i]);
+ goto err_clks;
+ }
+ }
+
+ return 0;
+
+err_clks:
+ while (--i >= 0)
+ clk_disable_unprepare(priv->clks[i]);
+
+ return err;
+}
+
+static void sky1_audss_clks_disable(struct sky1_audss_clks_priv *priv)
+{
+ const struct sky1_audss_clks_devtype_data *devtype_data = priv->devtype_data;
+ int i;
+
+ for (i = 0; i < devtype_data->clk_num; i++)
+ clk_disable_unprepare(priv->clks[i]);
+}
+
+static int sky1_audss_clks_set_rate(struct sky1_audss_clks_priv *priv)
+{
+ const struct sky1_audss_clks_devtype_data *devtype_data = priv->devtype_data;
+ int i, err;
+
+ for (i = 0; i < devtype_data->clk_num; i++) {
+ err = clk_set_rate(priv->clks[i], devtype_data->clk_rate_default[i]);
+ if (err) {
+ dev_err(priv->dev, "failed to set clock rate %s\n",
+ devtype_data->clk_names[i]);
+ return err;
+ }
+ }
+
+ return 0;
+}
+
+/* register sky1 audio subsystem clocks */
+static int sky1_audss_clk_probe(struct platform_device *pdev)
+{
+ const struct sky1_audss_clks_devtype_data *devtype_data;
+ struct sky1_audss_clks_priv *priv;
+ struct device_node *parent_np;
+ struct device *dev = &pdev->dev;
+ struct clk_hw **clk_table;
+ struct regmap *regmap_cru;
+ int i, ret;
+
+ parent_np = of_get_parent(pdev->dev.of_node);
+ regmap_cru = syscon_node_to_regmap(parent_np);
+ of_node_put(parent_np);
+ if (IS_ERR(regmap_cru))
+ return dev_err_probe(dev, PTR_ERR(regmap_cru),
+ "unable to get audss cru regmap");
+
+ devtype_data = device_get_match_data(dev);
+ if (!devtype_data)
+ return -ENODEV;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ spin_lock_init(&priv->lock);
+
+ priv->clk_data = devm_kzalloc(&pdev->dev,
+ struct_size(priv->clk_data, hws, SKY1_AUDSS_NUM_CLKS),
+ GFP_KERNEL);
+ if (!priv->clk_data)
+ return -ENOMEM;
+
+ priv->clk_data->num = SKY1_AUDSS_NUM_CLKS;
+ clk_table = priv->clk_data->hws;
+
+ priv->dev = dev;
+ priv->regmap_cru = regmap_cru;
+ priv->devtype_data = devtype_data;
+
+ /*
+ * Link to audss cru for runtime PM ordering: cru resumes (deasserts
+ * reset) before clk, and suspends (asserts reset) after clk.
+ */
+ if (!device_link_add(dev, dev->parent,
+ DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME |
+ DL_FLAG_RPM_ACTIVE))
+ return dev_err_probe(dev, -EINVAL, "failed to link to audss syscon");
+
+ ret = sky1_audss_clks_get(priv);
+ if (ret)
+ goto err_link;
+
+ platform_set_drvdata(pdev, priv);
+
+ ret = sky1_audss_clks_enable(priv);
+ if (ret) {
+ dev_err(dev, "failed to enable clocks\n");
+ goto err_link;
+ }
+
+ /*
+ * Enable runtime PM here to allow the clock core using runtime PM
+ * for the registered clocks.
+ */
+ pm_runtime_get_noresume(dev);
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+
+ ret = sky1_audss_clks_set_rate(priv);
+ if (ret) {
+ dev_err(dev, "failed to set clocks rate\n");
+ goto fail_clks_set;
+ }
+
+ /* audio_clk4 clock fixed divider */
+ clk_table[CLK_AUD_CLK4_DIV2] =
+ devm_clk_hw_register_fixed_factor(dev,
+ "audio_clk4_div2",
+ "audio_clk4",
+ 0,
+ 1, 2);
+ if (IS_ERR(clk_table[CLK_AUD_CLK4_DIV2])) {
+ ret = PTR_ERR(clk_table[CLK_AUD_CLK4_DIV2]);
+ dev_err(dev, "failed to register clock %d, ret:%d\n", CLK_AUD_CLK4_DIV2, ret);
+ goto fail_fixed_clk;
+ }
+
+ clk_table[CLK_AUD_CLK4_DIV4] =
+ devm_clk_hw_register_fixed_factor(dev,
+ "audio_clk4_div4",
+ "audio_clk4",
+ 0,
+ 1, 4);
+ if (IS_ERR(clk_table[CLK_AUD_CLK4_DIV4])) {
+ ret = PTR_ERR(clk_table[CLK_AUD_CLK4_DIV4]);
+ dev_err(dev, "failed to register clock %d, ret:%d\n", CLK_AUD_CLK4_DIV4, ret);
+ goto fail_fixed_clk;
+ }
+
+ /* audio_clk5 clock fixed divider */
+ clk_table[CLK_AUD_CLK5_DIV2] =
+ devm_clk_hw_register_fixed_factor(dev,
+ "audio_clk5_div2",
+ "audio_clk5",
+ 0,
+ 1, 2);
+ if (IS_ERR(clk_table[CLK_AUD_CLK5_DIV2])) {
+ ret = PTR_ERR(clk_table[CLK_AUD_CLK5_DIV2]);
+ dev_err(dev, "failed to register clock %d, ret:%d\n", CLK_AUD_CLK5_DIV2, ret);
+ goto fail_fixed_clk;
+ }
+
+ for (i = 0; i < devtype_data->clk_cfg_size; i++) {
+ clk_table[devtype_data->clk_cfg[i].id] =
+ sky1_audss_clk_register(dev,
+ devtype_data->clk_cfg[i].name,
+ devtype_data->clk_cfg[i].parent_names,
+ devtype_data->clk_cfg[i].num_parents,
+ regmap_cru,
+ devtype_data->clk_cfg[i].mux_table,
+ devtype_data->clk_cfg[i].mux_cfg,
+ devtype_data->clk_cfg[i].div_cfg,
+ devtype_data->clk_cfg[i].gate_cfg,
+ devtype_data->clk_cfg[i].flags,
+ &priv->lock);
+ if (IS_ERR(clk_table[devtype_data->clk_cfg[i].id])) {
+ ret = PTR_ERR(clk_table[devtype_data->clk_cfg[i].id]);
+ dev_err(dev, "failed to register clock %d, ret:%d\n",
+ devtype_data->clk_cfg[i].id, ret);
+ goto fail_array_clk;
+ }
+ }
+
+ ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, priv->clk_data);
+ if (ret) {
+ dev_err(dev, "failed to add clock provider: %d\n", ret);
+ goto fail_register;
+ }
+
+ pm_runtime_put_sync(dev);
+
+ return 0;
+
+fail_register:
+fail_array_clk:
+ while (i--)
+ clk_hw_unregister_composite(clk_table[devtype_data->clk_cfg[i].id]);
+fail_fixed_clk:
+fail_clks_set:
+ pm_runtime_put_sync(dev);
+ pm_runtime_disable(dev);
+err_link:
+ device_link_remove(dev, dev->parent);
+ return ret;
+}
+
+static void sky1_audss_clk_remove(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct sky1_audss_clks_priv *priv = dev_get_drvdata(dev);
+ const struct sky1_audss_clks_devtype_data *devtype_data = priv->devtype_data;
+ int i = 0;
+
+ for (i = 0; i < devtype_data->clk_cfg_size; i++)
+ clk_hw_unregister_composite(priv->clk_data->hws[devtype_data->clk_cfg[i].id]);
+
+ if (!pm_runtime_status_suspended(dev))
+ pm_runtime_force_suspend(dev);
+
+ pm_runtime_disable(dev);
+ device_link_remove(dev, dev->parent);
+}
+
+static int __maybe_unused sky1_audss_clk_runtime_suspend(struct device *dev)
+{
+ struct sky1_audss_clks_priv *priv = dev_get_drvdata(dev);
+ const struct sky1_audss_clks_devtype_data *devtype_data = priv->devtype_data;
+ unsigned long flags;
+ int i;
+
+ spin_lock_irqsave(&priv->lock, flags);
+ for (i = 0; i < devtype_data->reg_save_size; i++)
+ regmap_read(priv->regmap_cru,
+ devtype_data->reg_save[i][0], &devtype_data->reg_save[i][1]);
+ spin_unlock_irqrestore(&priv->lock, flags);
+
+ sky1_audss_clks_disable(priv);
+
+ return 0;
+}
+
+static int __maybe_unused sky1_audss_clk_runtime_resume(struct device *dev)
+{
+ struct sky1_audss_clks_priv *priv = dev_get_drvdata(dev);
+ const struct sky1_audss_clks_devtype_data *devtype_data = priv->devtype_data;
+ unsigned long flags;
+ int i, ret;
+
+ ret = sky1_audss_clks_enable(priv);
+ if (ret) {
+ dev_err(dev, "failed to enable clocks\n");
+ return ret;
+ }
+
+ spin_lock_irqsave(&priv->lock, flags);
+ for (i = 0; i < devtype_data->reg_save_size; i++)
+ regmap_write(priv->regmap_cru,
+ devtype_data->reg_save[i][0], devtype_data->reg_save[i][1]);
+ spin_unlock_irqrestore(&priv->lock, flags);
+
+ return 0;
+}
+
+static const struct dev_pm_ops sky1_audss_clk_pm_ops = {
+ SET_RUNTIME_PM_OPS(sky1_audss_clk_runtime_suspend,
+ sky1_audss_clk_runtime_resume, NULL)
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
+};
+
+static const struct sky1_audss_clks_devtype_data sky1_devtype_data = {
+ .reg_save = sky1_reg_save,
+ .reg_save_size = ARRAY_SIZE(sky1_reg_save),
+ .clk_names = sky1_audss_clk_names,
+ .clk_num = ARRAY_SIZE(sky1_audss_clk_names),
+ .clk_rate_default = sky1_clk_rate_default,
+ .clk_cfg = sky1_audss_clks,
+ .clk_cfg_size = ARRAY_SIZE(sky1_audss_clks),
+};
+
+static const struct of_device_id sky1_audss_clk_of_match[] = {
+ { .compatible = "cix,sky1-audss-clock", .data = &sky1_devtype_data, },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, sky1_audss_clk_of_match);
+
+static struct platform_driver sky1_audss_clk_driver = {
+ .probe = sky1_audss_clk_probe,
+ .remove = sky1_audss_clk_remove,
+ .driver = {
+ .name = "sky1-audss-clk",
+ .suppress_bind_attrs = true,
+ .of_match_table = sky1_audss_clk_of_match,
+ .pm = &sky1_audss_clk_pm_ops,
+ },
+};
+module_platform_driver(sky1_audss_clk_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Joakim Zhang <joakim.zhang@cixtech.com>");
+MODULE_DESCRIPTION("Cixtech Sky1 Audio Subsystem Clock Controller Driver");
--
2.50.1
^ permalink raw reply related
* [PATCH v4 2/5] reset: cix: add audss support to sky1 reset driver
From: joakim.zhang @ 2026-06-17 6:04 UTC (permalink / raw)
To: mturquette, sboyd, bmasney, robh, krzk+dt, conor+dt, p.zabel,
gary.yang
Cc: cix-kernel-upstream, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, Joakim Zhang
In-Reply-To: <20260617060437.1474816-1-joakim.zhang@cixtech.com>
From: Joakim Zhang <joakim.zhang@cixtech.com>
Extend the Sky1 reset controller driver for the AUDSS CRU syscon. The
AUDSS block provides sixteen active-low software reset bits in one
register for audio subsystem peripherals, reusing the existing
regmap-based reset ops used by the FCH and S5 system control variants.
Signed-off-by: Joakim Zhang <joakim.zhang@cixtech.com>
---
drivers/reset/reset-sky1.c | 86 ++++++++++++++++++++++++++++++++++++--
1 file changed, 83 insertions(+), 3 deletions(-)
diff --git a/drivers/reset/reset-sky1.c b/drivers/reset/reset-sky1.c
index 78e80a533c39..462501c2ea06 100644
--- a/drivers/reset/reset-sky1.c
+++ b/drivers/reset/reset-sky1.c
@@ -10,12 +10,16 @@
#include <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_platform.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <linux/regmap.h>
+#include <linux/reset.h>
#include <linux/reset-controller.h>
#include <dt-bindings/reset/cix,sky1-system-control.h>
#include <dt-bindings/reset/cix,sky1-s5-system-control.h>
+#include <dt-bindings/reset/cix,sky1-audss-system-control.h>
#define SKY1_RESET_SLEEP_MIN_US 50
#define SKY1_RESET_SLEEP_MAX_US 100
@@ -34,6 +38,7 @@ struct sky1_src {
struct reset_controller_dev rcdev;
const struct sky1_src_signal *signals;
struct regmap *regmap;
+ struct reset_control *rst_noc;
};
enum {
@@ -258,6 +263,34 @@ static const struct sky1_src_variant variant_sky1_fch = {
.signals_num = ARRAY_SIZE(sky1_src_fch_signals),
};
+enum {
+ AUDSS_SW_RST = 0x78,
+};
+
+static const struct sky1_src_signal sky1_audss_signals[] = {
+ [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 const struct sky1_src_variant variant_sky1_audss = {
+ .signals = sky1_audss_signals,
+ .signals_num = ARRAY_SIZE(sky1_audss_signals),
+};
+
static struct sky1_src *to_sky1_src(struct reset_controller_dev *rcdev)
{
return container_of(rcdev, struct sky1_src, rcdev);
@@ -318,17 +351,34 @@ static const struct reset_control_ops sky1_src_ops = {
.status = sky1_reset_status
};
+static int __maybe_unused sky1_reset_runtime_suspend(struct device *dev)
+{
+ struct sky1_src *sky1src = dev_get_drvdata(dev);
+
+ return reset_control_assert(sky1src->rst_noc);
+}
+
+static int __maybe_unused sky1_reset_runtime_resume(struct device *dev)
+{
+ struct sky1_src *sky1src = dev_get_drvdata(dev);
+
+ return reset_control_deassert(sky1src->rst_noc);
+}
+
static int sky1_reset_probe(struct platform_device *pdev)
{
struct sky1_src *sky1src;
struct device *dev = &pdev->dev;
const struct sky1_src_variant *variant;
+ int ret;
sky1src = devm_kzalloc(dev, sizeof(*sky1src), GFP_KERNEL);
if (!sky1src)
return -ENOMEM;
variant = of_device_get_match_data(dev);
+ if (!variant)
+ return -ENODEV;
sky1src->regmap = device_node_to_regmap(dev->of_node);
if (IS_ERR(sky1src->regmap)) {
@@ -343,12 +393,36 @@ static int sky1_reset_probe(struct platform_device *pdev)
sky1src->rcdev.of_node = dev->of_node;
sky1src->rcdev.dev = dev;
- return devm_reset_controller_register(dev, &sky1src->rcdev);
+ ret = devm_reset_controller_register(dev, &sky1src->rcdev);
+ if (ret)
+ return ret;
+
+ platform_set_drvdata(pdev, sky1src);
+
+ if (of_device_is_compatible(dev->of_node, "cix,sky1-audss-system-control")) {
+ sky1src->rst_noc = devm_reset_control_get_exclusive(dev, NULL);
+ if (IS_ERR(sky1src->rst_noc))
+ return dev_err_probe(dev, PTR_ERR(sky1src->rst_noc),
+ "failed to get audss noc reset");
+
+ pm_runtime_get_noresume(dev);
+ pm_runtime_set_active(dev);
+ devm_pm_runtime_enable(dev);
+
+ reset_control_deassert(sky1src->rst_noc);
+
+ ret = devm_of_platform_populate(dev);
+ pm_runtime_put(dev);
+ return ret;
+ }
+
+ return 0;
}
static const struct of_device_id sky1_sysreg_of_match[] = {
- { .compatible = "cix,sky1-system-control", .data = &variant_sky1_fch},
- { .compatible = "cix,sky1-s5-system-control", .data = &variant_sky1},
+ { .compatible = "cix,sky1-system-control", .data = &variant_sky1_fch },
+ { .compatible = "cix,sky1-s5-system-control", .data = &variant_sky1 },
+ { .compatible = "cix,sky1-audss-system-control", .data = &variant_sky1_audss },
{},
};
MODULE_DEVICE_TABLE(of, sky1_sysreg_of_match);
@@ -358,6 +432,12 @@ static struct platform_driver sky1_reset_driver = {
.driver = {
.name = "cix,sky1-rst",
.of_match_table = sky1_sysreg_of_match,
+ .pm = &(const struct dev_pm_ops){
+ SET_RUNTIME_PM_OPS(sky1_reset_runtime_suspend,
+ sky1_reset_runtime_resume, NULL)
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
+ },
},
};
module_platform_driver(sky1_reset_driver)
--
2.50.1
^ permalink raw reply related
* [PATCH v4 3/5] dt-bindings: clock: cix,sky1-audss-clock: add audss clock controller
From: joakim.zhang @ 2026-06-17 6:04 UTC (permalink / raw)
To: mturquette, sboyd, bmasney, robh, krzk+dt, conor+dt, p.zabel,
gary.yang
Cc: cix-kernel-upstream, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, Joakim Zhang
In-Reply-To: <20260617060437.1474816-1-joakim.zhang@cixtech.com>
From: Joakim Zhang <joakim.zhang@cixtech.com>
The AUDSS CRU contains an internal clock tree of muxes, dividers and
gates for DSP, I2S, HDA, DMAC and related blocks. The clock provider is
a child node of the cix,sky1-audss-system-control syscon and accesses
registers through the parent MMIO region.
Signed-off-by: Joakim Zhang <joakim.zhang@cixtech.com>
---
.../bindings/clock/cix,sky1-audss-clock.yaml | 72 +++++++++++++++++++
.../dt-bindings/clock/cix,sky1-audss-clock.h | 60 ++++++++++++++++
2 files changed, 132 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/cix,sky1-audss-clock.yaml
create mode 100644 include/dt-bindings/clock/cix,sky1-audss-clock.h
diff --git a/Documentation/devicetree/bindings/clock/cix,sky1-audss-clock.yaml b/Documentation/devicetree/bindings/clock/cix,sky1-audss-clock.yaml
new file mode 100644
index 000000000000..ea813c5a2307
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/cix,sky1-audss-clock.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/cix,sky1-audss-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cix Sky1 audio subsystem clock controller
+
+maintainers:
+ - Joakim Zhang <joakim.zhang@cixtech.com>
+
+description: |
+ Clock provider for the Cix Sky1 audio subsystem (AUDSS).
+
+ This node is a child of a cix,sky1-audss-system-control syscon node
+ (see cix,sky1-system-control.yaml). It does not have a reg property; clock
+ mux, divider and gate fields are accessed through the parent register block.
+
+ Software reset lines for AUDSS blocks are exposed on the parent syscon via
+ #reset-cells (provider). Reset indices are defined in
+ include/dt-bindings/reset/cix,sky1-audss-system-control.h.
+
+ Four SoC-level reference clocks listed in clocks/clock-names feed the AUDSS
+ clock tree. The provider exposes the internal AUDSS clocks to other devices
+ via #clock-cells; indices are defined in cix,sky1-audss-clock.h.
+
+ The parent cix,sky1-audss-system-control node describes the SoC syscon
+ NoC (or bus) reset via resets and the audio subsystem power domain via
+ power-domains.
+
+properties:
+ compatible:
+ const: cix,sky1-audss-clock
+
+ '#clock-cells':
+ const: 1
+ description:
+ Clock indices are defined in include/dt-bindings/clock/cix,sky1-audss-clock.h.
+
+ clocks:
+ items:
+ - description: I2S parent clock for sampling rates multiple of 8kHz.
+ - description: I2S parent clock for sampling rates multiple of 11.025kHz.
+ - description: clock feeding most devices in audss (NOC, DSP, SRAM, HDA, DMAC, I2S, and Mailbox).
+ - description: clock feeding for HDA, Timer and Watchdog, which is a delicated 48MHz clock.
+
+ clock-names:
+ items:
+ - const: x8k
+ - const: x11k
+ - const: sys
+ - const: 48m
+
+required:
+ - compatible
+ - '#clock-cells'
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/cix,sky1.h>
+
+ clock-controller {
+ compatible = "cix,sky1-audss-clock";
+ #clock-cells = <1>;
+ clocks = <&scmi_clk CLK_TREE_AUDIO_CLK0>, <&scmi_clk CLK_TREE_AUDIO_CLK2>,
+ <&scmi_clk CLK_TREE_AUDIO_CLK4>, <&scmi_clk CLK_TREE_AUDIO_CLK5>;
+ clock-names = "x8k", "x11k", "sys", "48m";
+ };
diff --git a/include/dt-bindings/clock/cix,sky1-audss-clock.h b/include/dt-bindings/clock/cix,sky1-audss-clock.h
new file mode 100644
index 000000000000..7e9bd3e6c7a1
--- /dev/null
+++ b/include/dt-bindings/clock/cix,sky1-audss-clock.h
@@ -0,0 +1,60 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright 2026 Cix Technology Group Co., Ltd.
+ */
+
+#ifndef _DT_BINDINGS_CLK_CIX_SKY1_AUDSS_CLOCK_H
+#define _DT_BINDINGS_CLK_CIX_SKY1_AUDSS_CLOCK_H
+
+#define CLK_AUD_CLK4_DIV2 0
+#define CLK_AUD_CLK4_DIV4 1
+#define CLK_AUD_CLK5_DIV2 2
+
+#define CLK_DSP_CLK 3
+#define CLK_DSP_BCLK 4
+#define CLK_DSP_PBCLK 5
+
+#define CLK_SRAM_AXI 6
+
+#define CLK_HDA_SYS 7
+#define CLK_HDA_HDA 8
+
+#define CLK_DMAC_AXI 9
+
+#define CLK_WDG_APB 10
+#define CLK_WDG_WDG 11
+
+#define CLK_TIMER_APB 12
+#define CLK_TIMER_TIMER 13
+
+#define CLK_MB_0_APB 14 /* MB0: ap->dsp */
+#define CLK_MB_1_APB 15 /* MB1: dsp->ap */
+
+#define CLK_I2S0_APB 16
+#define CLK_I2S1_APB 17
+#define CLK_I2S2_APB 18
+#define CLK_I2S3_APB 19
+#define CLK_I2S4_APB 20
+#define CLK_I2S5_APB 21
+#define CLK_I2S6_APB 22
+#define CLK_I2S7_APB 23
+#define CLK_I2S8_APB 24
+#define CLK_I2S9_APB 25
+#define CLK_I2S0 26
+#define CLK_I2S1 27
+#define CLK_I2S2 28
+#define CLK_I2S3 29
+#define CLK_I2S4 30
+#define CLK_I2S5 31
+#define CLK_I2S6 32
+#define CLK_I2S7 33
+#define CLK_I2S8 34
+#define CLK_I2S9 35
+
+#define CLK_MCLK0 36
+#define CLK_MCLK1 37
+#define CLK_MCLK2 38
+#define CLK_MCLK3 39
+#define CLK_MCLK4 40
+
+#endif
--
2.50.1
^ permalink raw reply related
* [PATCH v4 1/5] dt-bindings: soc: cix,sky1-system-control: add audss system control
From: joakim.zhang @ 2026-06-17 6:04 UTC (permalink / raw)
To: mturquette, sboyd, bmasney, robh, krzk+dt, conor+dt, p.zabel,
gary.yang
Cc: cix-kernel-upstream, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, Joakim Zhang
In-Reply-To: <20260617060437.1474816-1-joakim.zhang@cixtech.com>
From: Joakim Zhang <joakim.zhang@cixtech.com>
The Cix Sky1 Audio Subsystem (AUDSS) groups audio-related clock, reset
and control registers in a dedicated CRU block. Software reset lines are
exposed on the syscon parent via #reset-cells, following the same model
as the existing Sky1 FCH and S5 system control bindings.
A clock-controller child node is required under the audss syscon. It has
no reg property of its own and accesses the parent register block for mux,
divider and gate fields.
The AUDSS is also controlled by one power domain and reset part.
Signed-off-by: Joakim Zhang <joakim.zhang@cixtech.com>
---
.../soc/cix/cix,sky1-system-control.yaml | 48 +++++++++++++++++++
.../reset/cix,sky1-audss-system-control.h | 25 ++++++++++
2 files changed, 73 insertions(+)
create mode 100644 include/dt-bindings/reset/cix,sky1-audss-system-control.h
diff --git a/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.yaml b/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.yaml
index a01a515222c6..5a1cd5c24ade 100644
--- a/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.yaml
+++ b/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.yaml
@@ -19,6 +19,7 @@ properties:
- enum:
- cix,sky1-system-control
- cix,sky1-s5-system-control
+ - cix,sky1-audss-system-control
- const: syscon
reg:
@@ -27,6 +28,38 @@ properties:
'#reset-cells':
const: 1
+ power-domains:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+ clock-controller:
+ type: object
+ properties:
+ compatible:
+ const: cix,sky1-audss-clock
+ required:
+ - compatible
+ additionalProperties: true
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: cix,sky1-audss-system-control
+ then:
+ required:
+ - clock-controller
+ - power-domains
+ - resets
+ else:
+ properties:
+ clock-controller: false
+ power-domains: false
+ resets: false
+
required:
- compatible
- reg
@@ -40,3 +73,18 @@ examples:
reg = <0x4160000 0x100>;
#reset-cells = <1>;
};
+ - |
+ audss_syscon: system-controller@7110000 {
+ compatible = "cix,sky1-audss-system-control", "syscon";
+ reg = <0x7110000 0x10000>;
+ power-domains = <&smc_devpd 0>;
+ resets = <&s5_syscon 31>;
+ #reset-cells = <1>;
+
+ clock-controller {
+ compatible = "cix,sky1-audss-clock";
+ #clock-cells = <1>;
+ clocks = <&scmi_clk 0>, <&scmi_clk 2>, <&scmi_clk 4>, <&scmi_clk 5>;
+ clock-names = "x8k", "x11k", "sys", "48m";
+ };
+ };
diff --git a/include/dt-bindings/reset/cix,sky1-audss-system-control.h b/include/dt-bindings/reset/cix,sky1-audss-system-control.h
new file mode 100644
index 000000000000..aabdce60b094
--- /dev/null
+++ b/include/dt-bindings/reset/cix,sky1-audss-system-control.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * Copyright 2026 Cix Technology Group Co., Ltd.
+ */
+#ifndef DT_BINDING_RESET_CIX_SKY1_AUDSS_SYSTEM_CONTROL_H
+#define DT_BINDING_RESET_CIX_SKY1_AUDSS_SYSTEM_CONTROL_H
+
+#define AUDSS_I2S0_SW_RST 0
+#define AUDSS_I2S1_SW_RST 1
+#define AUDSS_I2S2_SW_RST 2
+#define AUDSS_I2S3_SW_RST 3
+#define AUDSS_I2S4_SW_RST 4
+#define AUDSS_I2S5_SW_RST 5
+#define AUDSS_I2S6_SW_RST 6
+#define AUDSS_I2S7_SW_RST 7
+#define AUDSS_I2S8_SW_RST 8
+#define AUDSS_I2S9_SW_RST 9
+#define AUDSS_WDT_SW_RST 10
+#define AUDSS_TIMER_SW_RST 11
+#define AUDSS_MB0_SW_RST 12
+#define AUDSS_MB1_SW_RST 13
+#define AUDSS_HDA_SW_RST 14
+#define AUDSS_DMAC_SW_RST 15
+
+#endif
--
2.50.1
^ permalink raw reply related
* [PATCH v4 5/5] arm64: dts: cix: sky1: add audss system control
From: joakim.zhang @ 2026-06-17 6:04 UTC (permalink / raw)
To: mturquette, sboyd, bmasney, robh, krzk+dt, conor+dt, p.zabel,
gary.yang
Cc: cix-kernel-upstream, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, Joakim Zhang
In-Reply-To: <20260617060437.1474816-1-joakim.zhang@cixtech.com>
From: Joakim Zhang <joakim.zhang@cixtech.com>
Add audss system control device node, which would provides
clocks and resets for devices in audss domain.
Signed-off-by: Joakim Zhang <joakim.zhang@cixtech.com>
---
arch/arm64/boot/dts/cix/sky1.dtsi | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/arch/arm64/boot/dts/cix/sky1.dtsi b/arch/arm64/boot/dts/cix/sky1.dtsi
index bb5cfb1f2113..3091789fc176 100644
--- a/arch/arm64/boot/dts/cix/sky1.dtsi
+++ b/arch/arm64/boot/dts/cix/sky1.dtsi
@@ -6,6 +6,10 @@
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/clock/cix,sky1.h>
+#include <dt-bindings/clock/cix,sky1-audss-clock.h>
+#include <dt-bindings/reset/cix,sky1-system-control.h>
+#include <dt-bindings/reset/cix,sky1-s5-system-control.h>
+#include <dt-bindings/reset/cix,sky1-audss-system-control.h>
#include "sky1-power.h"
/ {
@@ -488,6 +492,26 @@ mbox_pm2ap: mailbox@65a0080 {
cix,mbox-dir = "rx";
};
+ audss_cru: system-controller@7110000 {
+ compatible = "cix,sky1-audss-system-control", "syscon";
+ reg = <0x0 0x07110000 0x0 0x10000>;
+ power-domains = <&smc_devpd SKY1_PD_AUDIO>;
+ resets = <&s5_syscon SKY1_AUDIO_HIFI5_NOC_RESET_N>;
+ #reset-cells = <1>;
+ status = "okay";
+
+ audss_clk: clock-controller {
+ compatible = "cix,sky1-audss-clock";
+ clocks = <&scmi_clk CLK_TREE_AUDIO_CLK0>,
+ <&scmi_clk CLK_TREE_AUDIO_CLK2>,
+ <&scmi_clk CLK_TREE_AUDIO_CLK4>,
+ <&scmi_clk CLK_TREE_AUDIO_CLK5>;
+ clock-names = "x8k", "x11k", "sys", "48m";
+ #clock-cells = <1>;
+ status = "okay";
+ };
+ };
+
mbox_sfh2ap: mailbox@8090000 {
compatible = "cix,sky1-mbox";
reg = <0x0 0x08090000 0x0 0x10000>;
--
2.50.1
^ permalink raw reply related
* [PATCH v4 0/5] Add Cix Sky1 AUDSS clock and reset support
From: joakim.zhang @ 2026-06-17 6:04 UTC (permalink / raw)
To: mturquette, sboyd, bmasney, robh, krzk+dt, conor+dt, p.zabel,
gary.yang
Cc: cix-kernel-upstream, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, Joakim Zhang
From: Joakim Zhang <joakim.zhang@cixtech.com>
This patch set adds the clock and reset support for AUDSS. The AUDSS
groups audio-related peripherals (HDA, I2S, DSP, DMA, mailboxes,
watchdog, timer, etc.) behind a single Clock and Reset Unit (CRU)
register block.
Clock and reset changes normally belong to separate subsystems and would
ideally be submitted as independent series. They are combined here because
the AUDSS DT bindings cross-reference each other: the system-control
binding describes the clock child node, the clock binding documents
reset lines exposed on the parent syscon, and the DTS example wires both
together. Keeping clock and reset in one series gives reviewers the full
picture when evaluating the binding layout, dependencies, and
integration.
Patches apply in the following order:
1. Reset support
- dt-bindings: soc: cix,sky1-system-control: add audss system control
- reset: cix: add audss support to sky1 reset driver
2. Clock support
- dt-bindings: clock: cix,sky1-audss-clock: add audss clock controller
- clk: cix: add sky1 audss clock controller
3. Device tree
- arm64: dts: cix: sky1: add audss system control
The reset and clock parts have each been build-tested and checked with
dt_binding_check independently. If reviewers prefer separate series for
the reset and clock maintainers, I can split and resubmit after this
round of review once the overall design is agreed on.
---
ChangeLogs:
v3->v4:
* move both power domain and resets into parset node (audss_cru)
* remove "simple-mfd", and change to populate the child node
* cix,sky1-audss.h -> cix,sky1-audss-clock.h
v2->v3:
* clk part:
* devm_reset_control_get()->devm_reset_control_get_exclusive()
* assert noc reset from suspend
* clock parents changes from 6 to 4, and rename the clock names,
explain more about this: confirm with our designer, In fact,
there are 6 clock sources going into the audio subsystem. audio_clk1
and audio_clk3 are redundant in design and are not actually needed
in practice, so they are not shown here.
* refine clocks and clock-names property
* add detailed description of clocks
* drop parent node from clk binding
* drop define AUDSS_MAX_CLKS
* reset part:
* rename reset signal macro, remove _N
* drop SKY1_AUDSS_SW_RESET_NUM
* switching to compatible-style of defining subnodes in parent schema
v1->v2:
* remove audss_rst device node since it doesn't has resource, and
move to reset-sky1.c driver.
* remove hda related which would be sent after this patch set accepted
* soc componnet is okay by default from dtsi
* fix for audss clk driver:
* remove "comment "Clock options for Cixtech audss:""
* add select MFD_SYSCON
* move lock and clk_data into struct sky1_audss_clks_priv
* const char *name -> const char * const * name
* remove CLK_GET_RATE_NOCACHE
* divicer -> divider
* Reverse Christmas tree order
* return reg ? 1 : 0; -> return !!reg;
* return ERR_CAST(hw); -> return hw;
* of_device_get_match_data(dev) -> device_get_match_data()
* add lock from runtime_suspend/resume
* loop to more mailing lists
Joakim Zhang (5):
dt-bindings: soc: cix,sky1-system-control: add audss system control
reset: cix: add audss support to sky1 reset driver
dt-bindings: clock: cix,sky1-audss-clock: add audss clock controller
clk: cix: add sky1 audss clock controller
arm64: dts: cix: sky1: add audss system control
.../bindings/clock/cix,sky1-audss-clock.yaml | 72 +
.../soc/cix/cix,sky1-system-control.yaml | 48 +
arch/arm64/boot/dts/cix/sky1.dtsi | 24 +
drivers/clk/Kconfig | 1 +
drivers/clk/Makefile | 1 +
drivers/clk/cix/Kconfig | 16 +
drivers/clk/cix/Makefile | 3 +
drivers/clk/cix/clk-sky1-audss.c | 1167 +++++++++++++++++
drivers/reset/reset-sky1.c | 86 +-
.../dt-bindings/clock/cix,sky1-audss-clock.h | 60 +
.../reset/cix,sky1-audss-system-control.h | 25 +
11 files changed, 1500 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/cix,sky1-audss-clock.yaml
create mode 100644 drivers/clk/cix/Kconfig
create mode 100644 drivers/clk/cix/Makefile
create mode 100644 drivers/clk/cix/clk-sky1-audss.c
create mode 100644 include/dt-bindings/clock/cix,sky1-audss-clock.h
create mode 100644 include/dt-bindings/reset/cix,sky1-audss-system-control.h
--
2.50.1
^ permalink raw reply
* [PATCH v2 2/2] clk: ultrarisc: add DP1000 clock driver
From: Jia Wang via B4 Relay @ 2026-06-17 6:02 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-clk, devicetree, linux-kernel, Jia Wang
In-Reply-To: <20260617-ultrarisc-clock-v2-0-9cb16083e15e@ultrarisc.com>
From: Jia Wang <wangjia@ultrarisc.com>
Add a clock driver for the UltraRISC DP1000 SoC.
The clock tree is driven by a SYSPLL and provides fixed-factor clocks for
the subsystem and PCIe, divider-based root clocks for GMAC and the UART,
I2C, and SPI blocks, and per-instance gate clocks for UART0-3, I2C0-3,
and SPI0-1.
Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
---
MAINTAINERS | 1 +
drivers/clk/Kconfig | 1 +
drivers/clk/Makefile | 1 +
drivers/clk/ultrarisc/Kconfig | 18 ++
drivers/clk/ultrarisc/Makefile | 4 +
drivers/clk/ultrarisc/clk-dp1000.c | 153 ++++++++++++
drivers/clk/ultrarisc/clk-ultrarisc.c | 459 ++++++++++++++++++++++++++++++++++
drivers/clk/ultrarisc/clk-ultrarisc.h | 73 ++++++
8 files changed, 710 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index b7e43313c65f..aa5021f30cb5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27444,6 +27444,7 @@ M: Jia Wang <wangjia@ultrarisc.com>
L: linux-clk@vger.kernel.org
S: Maintained
F: Documentation/devicetree/bindings/clock/ultrarisc,dp1000-clk.yaml
+F: drivers/clk/ultrarisc/*
F: include/dt-bindings/clock/ultrarisc,dp1000-clk.h
ULTRATRONIK BOARD SUPPORT
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index b2efbe9f6acb..75e336858420 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -535,6 +535,7 @@ source "drivers/clk/tenstorrent/Kconfig"
source "drivers/clk/thead/Kconfig"
source "drivers/clk/stm32/Kconfig"
source "drivers/clk/ti/Kconfig"
+source "drivers/clk/ultrarisc/Kconfig"
source "drivers/clk/uniphier/Kconfig"
source "drivers/clk/visconti/Kconfig"
source "drivers/clk/x86/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index a3e2862ebd7e..891272e8e1da 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -158,6 +158,7 @@ obj-$(CONFIG_ARCH_TEGRA) += tegra/
obj-y += tenstorrent/
obj-$(CONFIG_ARCH_THEAD) += thead/
obj-y += ti/
+obj-y += ultrarisc/
obj-$(CONFIG_CLK_UNIPHIER) += uniphier/
obj-$(CONFIG_ARCH_U8500) += ux500/
obj-y += versatile/
diff --git a/drivers/clk/ultrarisc/Kconfig b/drivers/clk/ultrarisc/Kconfig
new file mode 100644
index 000000000000..2eecc6ac3119
--- /dev/null
+++ b/drivers/clk/ultrarisc/Kconfig
@@ -0,0 +1,18 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+config CLK_ULTRARISC
+ tristate
+ depends on OF
+ depends on ARCH_ULTRARISC || COMPILE_TEST
+
+config CLK_ULTRARISC_DP1000
+ tristate "UltraRISC DP1000 clock controller"
+ select CLK_ULTRARISC
+ depends on OF && HAS_IOMEM
+ depends on ARCH_ULTRARISC || COMPILE_TEST
+ default ARCH_ULTRARISC
+ help
+ This driver provides the clock controller for the UltraRISC
+ DP1000 SoC. It exposes the PLL output, derived fixed-factor
+ clocks, programmable divider clocks, and peripheral gate
+ clocks to Linux consumers.
diff --git a/drivers/clk/ultrarisc/Makefile b/drivers/clk/ultrarisc/Makefile
new file mode 100644
index 000000000000..b013708c9444
--- /dev/null
+++ b/drivers/clk/ultrarisc/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+obj-$(CONFIG_CLK_ULTRARISC) += clk-ultrarisc.o
+obj-$(CONFIG_CLK_ULTRARISC_DP1000) += clk-dp1000.o
diff --git a/drivers/clk/ultrarisc/clk-dp1000.c b/drivers/clk/ultrarisc/clk-dp1000.c
new file mode 100644
index 000000000000..33a16df8f189
--- /dev/null
+++ b/drivers/clk/ultrarisc/clk-dp1000.c
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2026 UltraRISC Technology (Shanghai) Co., Ltd.
+ */
+
+#include <linux/module.h>
+
+#include <dt-bindings/clock/ultrarisc,dp1000-clk.h>
+
+#include "clk-ultrarisc.h"
+
+#define DP1000_PLL_CFG1_OFFSET 0x400
+#define DP1000_PLL_CFG2_OFFSET 0x404
+
+#define DP1000_CCR_UART_OFFSET 0x220
+#define DP1000_CCR_I2C_OFFSET 0x224
+#define DP1000_CCR_GMAC_OFFSET 0x228
+#define DP1000_CCR_SPI_OFFSET 0x22c
+#define DP1000_PERI_CLKENA_OFFSET 0x270
+
+#define DP1000_CCR_LOAD BIT(16)
+
+#define DP1000_PERI_MAX_RATE 62500000UL
+#define DP1000_CLK_NUM 21
+
+static const struct ultrarisc_pll_layout dp1000_pll_layout = {
+ .cfg1_offset = DP1000_PLL_CFG1_OFFSET,
+ .cfg2_offset = DP1000_PLL_CFG2_OFFSET,
+ .frac_mask = GENMASK(23, 0),
+ .m_mask = GENMASK(23, 16),
+ .n_mask = GENMASK(11, 6),
+ .oddiv1_mask = GENMASK(4, 3),
+ .oddiv2_mask = GENMASK(1, 0),
+};
+
+static const struct ultrarisc_pll_desc dp1000_plls[] = {
+ {
+ .id = DP1000_CLK_SYSPLL,
+ .name = "syspll_clk",
+ },
+};
+
+#define DP1000_FIXED_FACTOR(_id, _name, _parent, _mult, _div) \
+ { \
+ .id = (_id), \
+ .name = (_name), \
+ .parent_id = (_parent), \
+ .mult = (_mult), \
+ .div = (_div), \
+ }
+
+#define DP1000_DIV(_id, _name, _offset, _parent, _max_rate) \
+ { \
+ .id = (_id), \
+ .name = (_name), \
+ .offset = (_offset), \
+ .parent_id = (_parent), \
+ .max_rate = (_max_rate), \
+ .load_mask = DP1000_CCR_LOAD, \
+ .div_shift = 8, \
+ .div_width = 4, \
+ .gate_bit = 0, \
+ .divider_flags = CLK_DIVIDER_ONE_BASED, \
+ .gate_flags = 0, \
+ }
+
+#define DP1000_GATE(_id, _name, _parent, _bit) \
+ { \
+ .id = (_id), \
+ .name = (_name), \
+ .offset = DP1000_PERI_CLKENA_OFFSET, \
+ .parent_id = (_parent), \
+ .gate_bit = (_bit), \
+ .gate_flags = 0, \
+ }
+
+static const struct ultrarisc_fixed_factor_desc dp1000_fixed_factor_clks[] = {
+ DP1000_FIXED_FACTOR(DP1000_CLK_SYSPLL_DIV2, "syspll_div2_clk",
+ DP1000_CLK_SYSPLL, 1, 2),
+ DP1000_FIXED_FACTOR(DP1000_CLK_SUBSYS, "subsys_clk",
+ DP1000_CLK_SYSPLL_DIV2, 1, 2),
+ DP1000_FIXED_FACTOR(DP1000_CLK_PCIE_DBI, "pcie_dbi_clk",
+ DP1000_CLK_SYSPLL, 1, 10),
+ DP1000_FIXED_FACTOR(DP1000_CLK_PCIEX4_CORE, "pciex4_core_clk",
+ DP1000_CLK_SYSPLL, 1, 2),
+ DP1000_FIXED_FACTOR(DP1000_CLK_PCIEX16_CORE, "pciex16_core_clk",
+ DP1000_CLK_SYSPLL, 1, 1),
+ DP1000_FIXED_FACTOR(DP1000_CLK_PCIE_AUX, "pcie_aux_clk",
+ DP1000_CLK_SYSPLL, 1, 40),
+};
+
+static const struct ultrarisc_divider_desc dp1000_divider_clks[] = {
+ DP1000_DIV(DP1000_CLK_GMAC, "gmac_clk", DP1000_CCR_GMAC_OFFSET,
+ DP1000_CLK_SYSPLL_DIV2, 0),
+ DP1000_DIV(DP1000_CLK_UART_ROOT, "uart_root_clk",
+ DP1000_CCR_UART_OFFSET, DP1000_CLK_SUBSYS,
+ DP1000_PERI_MAX_RATE),
+ DP1000_DIV(DP1000_CLK_I2C_ROOT, "i2c_root_clk",
+ DP1000_CCR_I2C_OFFSET, DP1000_CLK_SUBSYS,
+ DP1000_PERI_MAX_RATE),
+ DP1000_DIV(DP1000_CLK_SPI_ROOT, "spi_root_clk",
+ DP1000_CCR_SPI_OFFSET, DP1000_CLK_SUBSYS,
+ DP1000_PERI_MAX_RATE),
+};
+
+static const struct ultrarisc_gate_desc dp1000_gate_clks[] = {
+ DP1000_GATE(DP1000_CLK_UART0, "uart0_clk", DP1000_CLK_UART_ROOT, 0),
+ DP1000_GATE(DP1000_CLK_UART1, "uart1_clk", DP1000_CLK_UART_ROOT, 1),
+ DP1000_GATE(DP1000_CLK_UART2, "uart2_clk", DP1000_CLK_UART_ROOT, 2),
+ DP1000_GATE(DP1000_CLK_UART3, "uart3_clk", DP1000_CLK_UART_ROOT, 3),
+ DP1000_GATE(DP1000_CLK_I2C0, "i2c0_clk", DP1000_CLK_I2C_ROOT, 4),
+ DP1000_GATE(DP1000_CLK_I2C1, "i2c1_clk", DP1000_CLK_I2C_ROOT, 5),
+ DP1000_GATE(DP1000_CLK_I2C2, "i2c2_clk", DP1000_CLK_I2C_ROOT, 6),
+ DP1000_GATE(DP1000_CLK_I2C3, "i2c3_clk", DP1000_CLK_I2C_ROOT, 7),
+ DP1000_GATE(DP1000_CLK_SPI0, "spi0_clk", DP1000_CLK_SPI_ROOT, 8),
+ DP1000_GATE(DP1000_CLK_SPI1, "spi1_clk", DP1000_CLK_SPI_ROOT, 9),
+};
+
+static const struct ultrarisc_clk_soc_data dp1000_clk_soc_data = {
+ .num_clks = DP1000_CLK_NUM,
+ .pll_layout = &dp1000_pll_layout,
+ .plls = dp1000_plls,
+ .num_plls = ARRAY_SIZE(dp1000_plls),
+ .fixed_factors = dp1000_fixed_factor_clks,
+ .num_fixed_factors = ARRAY_SIZE(dp1000_fixed_factor_clks),
+ .dividers = dp1000_divider_clks,
+ .num_dividers = ARRAY_SIZE(dp1000_divider_clks),
+ .gates = dp1000_gate_clks,
+ .num_gates = ARRAY_SIZE(dp1000_gate_clks),
+};
+
+static int dp1000_clk_probe(struct platform_device *pdev)
+{
+ return ultrarisc_clk_probe(pdev, &dp1000_clk_soc_data);
+}
+
+static const struct of_device_id dp1000_clk_of_match[] = {
+ { .compatible = "ultrarisc,dp1000-clk" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, dp1000_clk_of_match);
+
+static struct platform_driver dp1000_clk_driver = {
+ .probe = dp1000_clk_probe,
+ .driver = {
+ .name = "ultrarisc-dp1000-clk",
+ .of_match_table = dp1000_clk_of_match,
+ },
+};
+module_platform_driver(dp1000_clk_driver);
+
+MODULE_DESCRIPTION("UltraRISC DP1000 clock controller");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/ultrarisc/clk-ultrarisc.c b/drivers/clk/ultrarisc/clk-ultrarisc.c
new file mode 100644
index 000000000000..4ef222348dd7
--- /dev/null
+++ b/drivers/clk/ultrarisc/clk-ultrarisc.c
@@ -0,0 +1,459 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2026 UltraRISC Technology (Shanghai) Co., Ltd.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/of_clk.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include "clk-ultrarisc.h"
+
+struct ultrarisc_pll_clk {
+ struct clk_hw hw;
+ void __iomem *base;
+ const struct ultrarisc_pll_layout *layout;
+};
+
+struct ultrarisc_divider_clk {
+ struct clk_divider divider;
+ struct clk_gate gate;
+ u32 load_mask;
+};
+
+#define to_ultrarisc_pll_clk(_hw) \
+ container_of(_hw, struct ultrarisc_pll_clk, hw)
+
+static inline struct ultrarisc_divider_clk *
+to_ultrarisc_divider_clk(struct clk_hw *hw)
+{
+ struct clk_divider *divider = to_clk_divider(hw);
+
+ return container_of(divider, struct ultrarisc_divider_clk, divider);
+}
+
+static unsigned long ultrarisc_pll_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct ultrarisc_pll_clk *pll = to_ultrarisc_pll_clk(hw);
+ const struct ultrarisc_pll_layout *layout = pll->layout;
+ u32 oddiv1_div, oddiv2_div;
+ u64 mult, rate, den;
+ u32 frac, m, n;
+ u32 cfg1, cfg2;
+
+ cfg1 = readl_relaxed(pll->base + layout->cfg1_offset);
+ cfg2 = readl_relaxed(pll->base + layout->cfg2_offset);
+
+ frac = field_get(layout->frac_mask, cfg1);
+ m = field_get(layout->m_mask, cfg2);
+ n = field_get(layout->n_mask, cfg2);
+ oddiv1_div = 1U << field_get(layout->oddiv1_mask, cfg2);
+ oddiv2_div = 1U << field_get(layout->oddiv2_mask, cfg2);
+
+ if (!n)
+ return 0;
+
+ /*
+ * The output frequency is calculated as:
+ * fvco = parent * (m + frac / 2^24) / n
+ * fout = fvco / (2^oddiv1_raw * 2^oddiv2_raw)
+ *
+ * The output divider values are derived from the raw register field
+ * values as:
+ * oddivX_div = 1 << oddivX_raw
+ */
+ mult = ((u64)m << 24) + frac;
+ rate = (u64)parent_rate * mult;
+ den = ((u64)n << 24) * oddiv1_div * oddiv2_div;
+
+ return div64_u64(rate + (den >> 1), den);
+}
+
+static const struct clk_ops ultrarisc_pll_ro_ops = {
+ .recalc_rate = ultrarisc_pll_recalc_rate,
+};
+
+static unsigned long
+ultrarisc_divider_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
+{
+ struct clk_divider *divider = to_clk_divider(hw);
+ u32 val;
+
+ val = readl_relaxed(divider->reg) >> divider->shift;
+ val &= clk_div_mask(divider->width);
+
+ return divider_recalc_rate(hw, parent_rate, val, divider->table,
+ divider->flags, divider->width);
+}
+
+static int ultrarisc_divider_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ struct clk_divider *divider = to_clk_divider(hw);
+
+ return divider_determine_rate(hw, req, divider->table, divider->width,
+ divider->flags);
+}
+
+static int ultrarisc_divider_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct ultrarisc_divider_clk *divider_clk = to_ultrarisc_divider_clk(hw);
+ struct clk_divider *divider = ÷r_clk->divider;
+ int value;
+ u32 val;
+
+ value = divider_get_val(rate, parent_rate, divider->table,
+ divider->width, divider->flags);
+ if (value < 0)
+ return value;
+
+ scoped_guard(spinlock_irqsave, divider->lock) {
+ val = readl_relaxed(divider->reg);
+ val &= ~(clk_div_mask(divider->width) << divider->shift);
+ val |= value << divider->shift;
+ writel_relaxed(val, divider->reg);
+
+ if (divider_clk->load_mask) {
+ /*
+ * Program the new divider field, then write 1 to the
+ * load bit to trigger the update. The load bit is
+ * write-triggered and reads back as 0 on this hardware.
+ */
+ writel_relaxed(val | divider_clk->load_mask,
+ divider->reg);
+ }
+ }
+
+ return 0;
+}
+
+static const struct clk_ops ultrarisc_divider_ops = {
+ .recalc_rate = ultrarisc_divider_recalc_rate,
+ .determine_rate = ultrarisc_divider_determine_rate,
+ .set_rate = ultrarisc_divider_set_rate,
+};
+
+static void ultrarisc_clk_unregister_composite(void *data)
+{
+ clk_hw_unregister_composite(data);
+}
+
+static struct clk_hw *
+ultrarisc_clk_hw_register_composite(struct device *dev, const char *name,
+ const char * const *parent_names,
+ int num_parents, struct clk_hw *mux_hw,
+ const struct clk_ops *mux_ops,
+ struct clk_hw *rate_hw,
+ const struct clk_ops *rate_ops,
+ struct clk_hw *gate_hw,
+ const struct clk_ops *gate_ops,
+ unsigned long flags)
+{
+ struct clk_hw *hw;
+ int ret;
+
+ hw = clk_hw_register_composite(dev, name, parent_names, num_parents,
+ mux_hw, mux_ops, rate_hw, rate_ops,
+ gate_hw, gate_ops, flags);
+ if (IS_ERR(hw))
+ return hw;
+
+ ret = devm_add_action_or_reset(dev, ultrarisc_clk_unregister_composite,
+ hw);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return hw;
+}
+
+static struct clk_hw *ultrarisc_clk_register_pll(struct device *dev,
+ const struct ultrarisc_pll_desc *desc,
+ const struct ultrarisc_pll_layout *layout,
+ void __iomem *base)
+{
+ struct clk_parent_data pdata = { .index = 0 };
+ struct ultrarisc_pll_clk *pll;
+ struct clk_init_data init = {
+ .name = desc->name,
+ .ops = &ultrarisc_pll_ro_ops,
+ .parent_data = &pdata,
+ .num_parents = 1,
+ .flags = CLK_GET_RATE_NOCACHE,
+ };
+ int ret;
+
+ pll = devm_kzalloc(dev, sizeof(*pll), GFP_KERNEL);
+ if (!pll)
+ return ERR_PTR(-ENOMEM);
+
+ pll->base = base;
+ pll->layout = layout;
+ pll->hw.init = &init;
+
+ ret = devm_clk_hw_register(dev, &pll->hw);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return &pll->hw;
+}
+
+static struct clk_hw *
+ultrarisc_clk_register_divider(struct device *dev,
+ const struct ultrarisc_divider_desc *desc,
+ struct clk_hw *parent_hw, void __iomem *base,
+ spinlock_t *lock)
+{
+ const char * const parent_names[] = { clk_hw_get_name(parent_hw) };
+ void __iomem *reg = base + desc->offset;
+ struct ultrarisc_divider_clk *divider;
+ const struct clk_ops *gate_ops = NULL;
+ struct clk_hw *gate_hw = NULL;
+
+ if (!desc->div_width)
+ return ERR_PTR(-EINVAL);
+
+ if (!lock)
+ return ERR_PTR(-EINVAL);
+
+ divider = devm_kzalloc(dev, sizeof(*divider), GFP_KERNEL);
+ if (!divider)
+ return ERR_PTR(-ENOMEM);
+
+ divider->divider.reg = reg;
+ divider->divider.shift = desc->div_shift;
+ divider->divider.width = desc->div_width;
+ divider->divider.flags = desc->divider_flags;
+ divider->divider.lock = lock;
+ divider->load_mask = desc->load_mask;
+
+ if (desc->gate_bit != ULTRARISC_CLK_NO_GATE) {
+ divider->gate.reg = reg;
+ divider->gate.bit_idx = desc->gate_bit;
+ divider->gate.flags = desc->gate_flags;
+ divider->gate.lock = lock;
+ gate_hw = ÷r->gate.hw;
+ gate_ops = &clk_gate_ops;
+ }
+
+ return ultrarisc_clk_hw_register_composite(dev, desc->name,
+ parent_names, 1,
+ NULL, NULL,
+ ÷r->divider.hw,
+ &ultrarisc_divider_ops,
+ gate_hw, gate_ops,
+ CLK_GET_RATE_NOCACHE);
+}
+
+static int ultrarisc_clk_register_fixed_factors(struct device *dev,
+ struct clk_hw_onecell_data *clk_data,
+ const struct ultrarisc_clk_soc_data *soc_data)
+{
+ u32 i;
+
+ for (i = 0; i < soc_data->num_fixed_factors; i++) {
+ const struct ultrarisc_fixed_factor_desc *desc;
+ struct clk_hw *parent_hw;
+ struct clk_hw *hw;
+
+ desc = &soc_data->fixed_factors[i];
+ if (desc->id >= clk_data->num ||
+ desc->parent_id >= clk_data->num)
+ return -EINVAL;
+
+ parent_hw = clk_data->hws[desc->parent_id];
+ if (!parent_hw)
+ return -EINVAL;
+
+ hw = devm_clk_hw_register_fixed_factor_parent_hw(dev, desc->name,
+ parent_hw,
+ CLK_GET_RATE_NOCACHE,
+ desc->mult,
+ desc->div);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+
+ clk_data->hws[desc->id] = hw;
+ }
+
+ return 0;
+}
+
+static int ultrarisc_clk_register_plls(struct platform_device *pdev,
+ struct clk_hw_onecell_data *clk_data,
+ const struct ultrarisc_clk_soc_data *soc_data,
+ void __iomem *base)
+{
+ struct device *dev = &pdev->dev;
+ u32 i;
+
+ for (i = 0; i < soc_data->num_plls; i++) {
+ const struct ultrarisc_pll_desc *desc = &soc_data->plls[i];
+ struct clk_hw *hw;
+
+ if (desc->id >= clk_data->num)
+ return -EINVAL;
+
+ hw = ultrarisc_clk_register_pll(dev, desc, soc_data->pll_layout,
+ base);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+
+ clk_data->hws[desc->id] = hw;
+ }
+
+ return 0;
+}
+
+static int ultrarisc_clk_register_dividers(struct platform_device *pdev,
+ struct clk_hw_onecell_data *clk_data,
+ const struct ultrarisc_clk_soc_data *soc_data,
+ void __iomem *base,
+ spinlock_t *lock)
+{
+ struct device *dev = &pdev->dev;
+ u32 i;
+
+ for (i = 0; i < soc_data->num_dividers; i++) {
+ const struct ultrarisc_divider_desc *desc;
+ struct clk_hw *parent_hw;
+ struct clk_hw *hw;
+
+ desc = &soc_data->dividers[i];
+ if (desc->id >= clk_data->num ||
+ desc->parent_id >= clk_data->num)
+ return -EINVAL;
+
+ parent_hw = clk_data->hws[desc->parent_id];
+ if (!parent_hw)
+ return -EINVAL;
+
+ hw = ultrarisc_clk_register_divider(dev, desc, parent_hw, base,
+ lock);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+
+ if (desc->max_rate) {
+ unsigned long rate;
+
+ clk_hw_set_rate_range(hw, 0, desc->max_rate);
+
+ rate = clk_hw_get_rate(hw);
+ if (rate > desc->max_rate)
+ dev_warn(dev, "%s rate %lu exceeds max %lu\n",
+ desc->name, rate, desc->max_rate);
+ }
+
+ clk_data->hws[desc->id] = hw;
+ }
+
+ return 0;
+}
+
+static int ultrarisc_clk_register_gates(struct platform_device *pdev,
+ struct clk_hw_onecell_data *clk_data,
+ const struct ultrarisc_clk_soc_data *soc_data,
+ void __iomem *base,
+ spinlock_t *lock)
+{
+ struct device *dev = &pdev->dev;
+ u32 i;
+
+ for (i = 0; i < soc_data->num_gates; i++) {
+ const struct ultrarisc_gate_desc *desc;
+ struct clk_hw *parent_hw;
+ struct clk_hw *hw;
+
+ desc = &soc_data->gates[i];
+ if (desc->id >= clk_data->num ||
+ desc->parent_id >= clk_data->num)
+ return -EINVAL;
+
+ parent_hw = clk_data->hws[desc->parent_id];
+ if (!parent_hw)
+ return -EINVAL;
+
+ hw = devm_clk_hw_register_gate_parent_hw(dev, desc->name,
+ parent_hw,
+ CLK_GET_RATE_NOCACHE,
+ base + desc->offset,
+ desc->gate_bit,
+ desc->gate_flags,
+ lock);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+
+ clk_data->hws[desc->id] = hw;
+ }
+
+ return 0;
+}
+
+int ultrarisc_clk_probe(struct platform_device *pdev,
+ const struct ultrarisc_clk_soc_data *soc_data)
+{
+ struct clk_hw_onecell_data *clk_data;
+ struct device *dev = &pdev->dev;
+ void __iomem *base;
+ spinlock_t *lock;
+ int ret;
+
+ if (!soc_data)
+ return -EINVAL;
+
+ lock = devm_kzalloc(dev, sizeof(*lock), GFP_KERNEL);
+ if (!lock)
+ return -ENOMEM;
+
+ spin_lock_init(lock);
+
+ clk_data = devm_kzalloc(dev, struct_size(clk_data, hws,
+ soc_data->num_clks),
+ GFP_KERNEL);
+ if (!clk_data)
+ return -ENOMEM;
+
+ clk_data->num = soc_data->num_clks;
+
+ base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ ret = ultrarisc_clk_register_plls(pdev, clk_data, soc_data, base);
+ if (ret)
+ return ret;
+
+ ret = ultrarisc_clk_register_fixed_factors(dev, clk_data, soc_data);
+ if (ret)
+ return ret;
+
+ ret = ultrarisc_clk_register_dividers(pdev, clk_data, soc_data, base,
+ lock);
+ if (ret)
+ return ret;
+
+ ret = ultrarisc_clk_register_gates(pdev, clk_data, soc_data, base,
+ lock);
+ if (ret)
+ return ret;
+
+ for (int i = 0; i < clk_data->num; i++) {
+ if (!clk_data->hws[i]) {
+ dev_err(dev, "missing clock ID %u\n", i);
+ return -EINVAL;
+ }
+ }
+
+ return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
+ clk_data);
+}
+EXPORT_SYMBOL_GPL(ultrarisc_clk_probe);
+
+MODULE_DESCRIPTION("UltraRISC clock core driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/ultrarisc/clk-ultrarisc.h b/drivers/clk/ultrarisc/clk-ultrarisc.h
new file mode 100644
index 000000000000..1281196bb414
--- /dev/null
+++ b/drivers/clk/ultrarisc/clk-ultrarisc.h
@@ -0,0 +1,73 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ULTRARISC_CLK_ULTRARISC_H
+#define __ULTRARISC_CLK_ULTRARISC_H
+
+#include <linux/clk-provider.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
+#define ULTRARISC_CLK_NO_GATE (-1)
+
+struct ultrarisc_pll_layout {
+ u32 cfg1_offset;
+ u32 cfg2_offset;
+ u32 frac_mask;
+ u32 m_mask;
+ u32 n_mask;
+ u32 oddiv1_mask;
+ u32 oddiv2_mask;
+};
+
+struct ultrarisc_pll_desc {
+ u32 id;
+ const char *name;
+};
+
+struct ultrarisc_fixed_factor_desc {
+ u32 id;
+ const char *name;
+ u32 parent_id;
+ u32 mult;
+ u32 div;
+};
+
+struct ultrarisc_divider_desc {
+ u32 id;
+ const char *name;
+ u32 offset;
+ u32 parent_id;
+ unsigned long max_rate;
+ u32 load_mask;
+ u8 div_shift;
+ u8 div_width;
+ s8 gate_bit;
+ u16 divider_flags;
+ u8 gate_flags;
+};
+
+struct ultrarisc_gate_desc {
+ u32 id;
+ const char *name;
+ u32 offset;
+ u32 parent_id;
+ u8 gate_bit;
+ u8 gate_flags;
+};
+
+struct ultrarisc_clk_soc_data {
+ const struct ultrarisc_pll_layout *pll_layout;
+ const struct ultrarisc_pll_desc *plls;
+ u32 num_plls;
+ const struct ultrarisc_fixed_factor_desc *fixed_factors;
+ u32 num_fixed_factors;
+ const struct ultrarisc_divider_desc *dividers;
+ u32 num_dividers;
+ const struct ultrarisc_gate_desc *gates;
+ u32 num_gates;
+ u32 num_clks;
+};
+
+int ultrarisc_clk_probe(struct platform_device *pdev,
+ const struct ultrarisc_clk_soc_data *soc_data);
+
+#endif /* __ULTRARISC_CLK_ULTRARISC_H */
--
2.34.1
^ permalink raw reply related
* [PATCH v2 0/2] clk: ultrarisc: add DP1000 clock support
From: Jia Wang via B4 Relay @ 2026-06-17 6:02 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-clk, devicetree, linux-kernel, Jia Wang
This series adds the devicetree binding and the clock driver for the
UltraRISC DP1000 SoC.
The clock tree is driven by a SYSPLL and provides fixed-factor clocks for
the subsystem and PCIe, divider-based root clocks for GMAC and the UART,
I2C, and SPI blocks, and per-instance gate clocks for UART0-3, I2C0-3,
and SPI0-1.
Testing:
- dt_binding_check and dtbs_check on ultrarisc,dp1000-clk
- Kernel build for RISC-V and boot-tested on DP1000
- Modules build test for CLK_ULTRARISC_DP1000
Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
---
Changes in v2:
- Drop the redundant clock-names property from the clock binding.
- Move DP1000_CLK_NUM from the dt-bindings header into the driver.
- Clarify the divider load-bit handling in the common clock core.
- Validate that all advertised clock IDs are populated before registering
the onecell clock provider.
- Rework divider composite clock registration so the driver builds
correctly as a module.
- Link to v1: https://patch.msgid.link/20260611-ultrarisc-clock-v1-0-2d93ebb4cc13@ultrarisc.com
---
Jia Wang (2):
dt-bindings: clock: ultrarisc: Add DP1000 Clock Controller
clk: ultrarisc: add DP1000 clock driver
.../bindings/clock/ultrarisc,dp1000-clk.yaml | 60 +++
MAINTAINERS | 8 +
drivers/clk/Kconfig | 1 +
drivers/clk/Makefile | 1 +
drivers/clk/ultrarisc/Kconfig | 18 +
drivers/clk/ultrarisc/Makefile | 4 +
drivers/clk/ultrarisc/clk-dp1000.c | 153 +++++++
drivers/clk/ultrarisc/clk-ultrarisc.c | 459 +++++++++++++++++++++
drivers/clk/ultrarisc/clk-ultrarisc.h | 73 ++++
include/dt-bindings/clock/ultrarisc,dp1000-clk.h | 27 ++
10 files changed, 804 insertions(+)
---
base-commit: b1cbabe84ca1381a004fb91ee1791a1a53bce44e
change-id: 20260522-ultrarisc-clock-a1b7aa59f60b
Best regards,
--
Jia Wang <wangjia@ultrarisc.com>
^ permalink raw reply
* [PATCH v2 1/2] dt-bindings: clock: ultrarisc: Add DP1000 Clock Controller
From: Jia Wang via B4 Relay @ 2026-06-17 6:02 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-clk, devicetree, linux-kernel, Jia Wang
In-Reply-To: <20260617-ultrarisc-clock-v2-0-9cb16083e15e@ultrarisc.com>
From: Jia Wang <wangjia@ultrarisc.com>
Add doc for the clock controller on the UltraRISC DP1000 RISC-V SoC.
Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
---
.../bindings/clock/ultrarisc,dp1000-clk.yaml | 60 ++++++++++++++++++++++
MAINTAINERS | 7 +++
include/dt-bindings/clock/ultrarisc,dp1000-clk.h | 27 ++++++++++
3 files changed, 94 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/ultrarisc,dp1000-clk.yaml b/Documentation/devicetree/bindings/clock/ultrarisc,dp1000-clk.yaml
new file mode 100644
index 000000000000..ede565ec440c
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/ultrarisc,dp1000-clk.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/ultrarisc,dp1000-clk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: UltraRISC DP1000 Clock Controller
+
+maintainers:
+ - Jia Wang <wangjia@ultrarisc.com>
+
+description: |
+ The UltraRISC DP1000 clock controller is driven from a single external
+ oscillator input. It provides a system PLL with fractional multiplier
+ and post-divider stages, several fixed-ratio derived clocks for
+ the on-chip subsystem, Clock Configuration Register (CCR) divider
+ outputs for GMAC and the UART, I2C, and SPI root clocks, and
+ per-instance gate clocks for UART0-3, I2C0-3, and SPI0-1.
+
+ All available clocks are defined as preprocessor macros in
+ include/dt-bindings/clock/ultrarisc,dp1000-clk.h
+
+properties:
+ compatible:
+ const: ultrarisc,dp1000-clk
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+ description:
+ External oscillator input clock used as the parent of the PLLs.
+
+ "#clock-cells":
+ const: 1
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - "#clock-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/ultrarisc,dp1000-clk.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ clock-controller@11080000 {
+ compatible = "ultrarisc,dp1000-clk";
+ reg = <0x0 0x11080000 0x0 0x1000>;
+ clocks = <&osc>;
+ #clock-cells = <1>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 3d6db8cb608f..b7e43313c65f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27439,6 +27439,13 @@ S: Maintained
F: drivers/usb/common/ulpi.c
F: include/linux/ulpi/
+ULTRARISC DP1000 CLOCK DRIVER
+M: Jia Wang <wangjia@ultrarisc.com>
+L: linux-clk@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/clock/ultrarisc,dp1000-clk.yaml
+F: include/dt-bindings/clock/ultrarisc,dp1000-clk.h
+
ULTRATRONIK BOARD SUPPORT
M: Goran Rađenović <goran.radni@gmail.com>
M: Börge Strümpfel <boerge.struempfel@gmail.com>
diff --git a/include/dt-bindings/clock/ultrarisc,dp1000-clk.h b/include/dt-bindings/clock/ultrarisc,dp1000-clk.h
new file mode 100644
index 000000000000..751125f99965
--- /dev/null
+++ b/include/dt-bindings/clock/ultrarisc,dp1000-clk.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+#ifndef _DT_BINDINGS_CLOCK_ULTRARISC_DP1000_CLK_H
+#define _DT_BINDINGS_CLOCK_ULTRARISC_DP1000_CLK_H
+
+#define DP1000_CLK_SYSPLL 0
+#define DP1000_CLK_SYSPLL_DIV2 1
+#define DP1000_CLK_SUBSYS 2
+#define DP1000_CLK_GMAC 3
+#define DP1000_CLK_UART_ROOT 4
+#define DP1000_CLK_I2C_ROOT 5
+#define DP1000_CLK_SPI_ROOT 6
+#define DP1000_CLK_PCIE_DBI 7
+#define DP1000_CLK_PCIEX4_CORE 8
+#define DP1000_CLK_PCIEX16_CORE 9
+#define DP1000_CLK_PCIE_AUX 10
+#define DP1000_CLK_UART0 11
+#define DP1000_CLK_UART1 12
+#define DP1000_CLK_UART2 13
+#define DP1000_CLK_UART3 14
+#define DP1000_CLK_I2C0 15
+#define DP1000_CLK_I2C1 16
+#define DP1000_CLK_I2C2 17
+#define DP1000_CLK_I2C3 18
+#define DP1000_CLK_SPI0 19
+#define DP1000_CLK_SPI1 20
+
+#endif /* _DT_BINDINGS_CLOCK_ULTRARISC_DP1000_CLK_H */
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 7/7] hwmon: adm1275: Support module auto-loading
From: Matti Vaittinen @ 2026-06-17 6:00 UTC (permalink / raw)
To: Guenter Roeck, Matti Vaittinen, Matti Vaittinen
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
Shuah Khan, Wensheng Wang, Ashish Yadav, Kim Seer Paller,
Cedric Encarnacion, Chris Packham, Yuxi Wang, Charles Hsu,
ChiShih Tsai, linux-hwmon, devicetree, linux-kernel, linux-doc
In-Reply-To: <f080e20e-6ec7-4744-9794-0a92d03f48d8@roeck-us.net>
On 16/06/2026 17:04, Guenter Roeck wrote:
> On 6/15/26 23:47, Matti Vaittinen wrote:
>> From: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> Populating the spi_device_id -table is not enough to make the
>> driver module automatically load when device-tree node for the bd12780
>> is parsed at boot.
>>
>> Adding the of_device_id tables causes the driver module to be
>> automatically load at boot. Testing has been done with rather old Debian
>> system.
>>
>> When inspecting the generated module-aliases with the insmod, following
>> entries seem to be the difference:
>>
>> alias: of:N*T*Crohm,bd12780C*
>> alias: of:N*T*Crohm,bd12780
>>
>> I suspect these are required for the module loading to work.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> ---
>>
>> I did not add of_device_ids for other supported ICs as I can't verify it
>> doesn't cause side-effects. Please let me know if you think those IDs
>> should be added as well. I would be glad if I got more educated opinion
>> on adding the of-IDs :) (I can squash this to 3/7 and 6/7 in next
>> revision, and add own patch for adding of-IDs for other ICs if
>> required).
>>
>
> I don't know what those side effects might be. I am much more concerned
> about side effects of having some of the devices in adm1275_of_match
> and some in adm1275_id. So, yes, please add a patch to provide
> adm1275_of_match for all chips supported by the driver.
It's nice to have an opinion on this as I was really unsure what is the
right way forward. Thanks for all the help this far. I'll do that in v2.
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* Re: [PATCH v4 2/2] phy: qcom-qmp-ufs: Add UFS PHY support on Hawi
From: Manivannan Sadhasivam @ 2026-06-17 6:00 UTC (permalink / raw)
To: palash.kambar
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, alim.akhtar,
bvanassche, andersson, dmitry.baryshkov, abel.vesa, luca.weiss,
linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-scsi,
nitin.rawat
In-Reply-To: <20260615091242.1617492-3-palash.kambar@oss.qualcomm.com>
On Mon, Jun 15, 2026 at 02:42:42PM +0530, palash.kambar@oss.qualcomm.com wrote:
> From: Palash Kambar <palash.kambar@oss.qualcomm.com>
>
> Add the init sequence tables and config for the UFS QMP phy found in
> the Hawi SoC.
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Palash Kambar <palash.kambar@oss.qualcomm.com>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
- Mani
> ---
> .../phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h | 24 +++
> .../qualcomm/phy-qcom-qmp-qserdes-com-v8.h | 13 +-
> .../phy-qcom-qmp-qserdes-txrx-ufs-v8.h | 37 +++++
> drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 139 ++++++++++++++++++
> 4 files changed, 212 insertions(+), 1 deletion(-)
> create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h
> create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h
> new file mode 100644
> index 000000000000..e80d3dd6a190
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2026, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef QCOM_PHY_QMP_PCS_UFS_V7_H_
> +#define QCOM_PHY_QMP_PCS_UFS_V7_H_
> +
> +/* Only for QMP V7 PHY - UFS PCS registers */
> +#define QPHY_V7_PCS_UFS_PHY_START 0x000
> +#define QPHY_V7_PCS_UFS_POWER_DOWN_CONTROL 0x004
> +#define QPHY_V7_PCS_UFS_SW_RESET 0x008
> +#define QPHY_V7_PCS_UFS_PCS_CTRL1 0x01C
> +#define QPHY_V7_PCS_UFS_PLL_CNTL 0x028
> +#define QPHY_V7_PCS_UFS_TX_LARGE_AMP_DRV_LVL 0x02C
> +#define QPHY_V7_PCS_UFS_TX_HSGEAR_CAPABILITY 0x060
> +#define QPHY_V7_PCS_UFS_RX_HSGEAR_CAPABILITY 0x094
> +#define QPHY_V7_PCS_UFS_LINECFG_DISABLE 0x140
> +#define QPHY_V7_PCS_UFS_RX_SIGDET_CTRL2 0x150
> +#define QPHY_V7_PCS_UFS_READY_STATUS 0x16c
> +#define QPHY_V7_PCS_UFS_TX_MID_TERM_CTRL1 0x1b8
> +#define QPHY_V7_PCS_UFS_MULTI_LANE_CTRL1 0x1c0
> +
> +#endif
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v8.h b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v8.h
> index d8ac4c4a2c31..d416113bcb3c 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v8.h
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v8.h
> @@ -1,6 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> /*
> - * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2026, The Linux Foundation. All rights reserved.
> */
>
> #ifndef QCOM_PHY_QMP_QSERDES_COM_V8_H_
> @@ -71,5 +71,16 @@
> #define QSERDES_V8_COM_ADDITIONAL_MISC 0x1b4
> #define QSERDES_V8_COM_CMN_STATUS 0x2c8
> #define QSERDES_V8_COM_C_READY_STATUS 0x2f0
> +#define QSERDES_V8_COM_PLL_IVCO_MODE1 0xf8
> +#define QSERDES_V8_COM_CMN_IETRIM 0xfc
> +#define QSERDES_V8_COM_CMN_IPTRIM 0x100
> +#define QSERDES_V8_COM_VCO_TUNE_CTRL 0x13c
> +#define QSERDES_V8_COM_ADAPTIVE_ANALOG_CONFIG 0x268
> +#define QSERDES_V8_COM_CP_CTRL_ADAPTIVE_MODE0 0x26c
> +#define QSERDES_V8_COM_PLL_RCCTRL_ADAPTIVE_MODE0 0x270
> +#define QSERDES_V8_COM_PLL_CCTRL_ADAPTIVE_MODE0 0x274
> +#define QSERDES_V8_COM_CP_CTRL_ADAPTIVE_MODE1 0x278
> +#define QSERDES_V8_COM_PLL_RCCTRL_ADAPTIVE_MODE1 0x27c
> +#define QSERDES_V8_COM_PLL_CCTRL_ADAPTIVE_MODE1 0x280
>
> #endif
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h
> new file mode 100644
> index 000000000000..5f923c3e64ec
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h
> @@ -0,0 +1,37 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2026, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef QCOM_PHY_QMP_QSERDES_TXRX_UFS_V8_H_
> +#define QCOM_PHY_QMP_QSERDES_TXRX_UFS_V8_H_
> +
> +#define QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_TX (0x34)
> +#define QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_RX (0x38)
> +#define QSERDES_UFS_V8_TX_LANE_MODE_1 (0x80)
> +#define QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE2 (0x1BC)
> +#define QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE4 (0x1C4)
> +#define QSERDES_UFS_V8_RX_UCDR_SO_GAIN_RATE4 (0x1DC)
> +#define QSERDES_UFS_V8_RX_EQ_OFFSET_ADAPTOR_CNTRL1 (0x2C8)
> +#define QSERDES_UFS_V8_RX_UCDR_PI_CONTROLS (0x1E4)
> +#define QSERDES_UFS_V8_RX_OFFSET_ADAPTOR_CNTRL3 (0x2D0)
> +#define QSERDES_UFS_V8_RX_UCDR_FASTLOCK_COUNT_HIGH_RATE4 (0x120)
> +#define QSERDES_UFS_V8_RX_UCDR_FASTLOCK_FO_GAIN_RATE4 (0xD4)
> +#define QSERDES_UFS_V8_RX_UCDR_FASTLOCK_SO_GAIN_RATE4 (0xEC)
> +#define QSERDES_UFS_V8_RX_VGA_CAL_MAN_VAL (0x288)
> +#define QSERDES_UFS_V8_RX_EQU_ADAPTOR_CNTRL4 (0x2B0)
> +#define QSERDES_UFS_V8_RX_MODE_RATE_0_1_B4 (0x324)
> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SA_B7 (0x3B4)
> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SA_B9 (0x3BC)
> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SB_B7 (0x3E0)
> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SB_B9 (0x3E8)
> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SA_B7 (0x40C)
> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SA_B9 (0x414)
> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SB_B7 (0x438)
> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SB_B9 (0x440)
> +#define QSERDES_UFS_V8_RX_UCDR_SO_SATURATION (0xF4)
> +#define QSERDES_UFS_V8_RX_TERM_BW_CTRL0 (0x1AC)
> +#define QSERDES_UFS_V8_RX_DLL0_FTUNE_CTRL (0x498)
> +#define QSERDES_UFS_V8_RX_SIGDET_CAL_TRIM (0x4d0)
> +
> +#endif
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> index 0f4ad24aa405..d4aca22c181e 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> @@ -29,9 +29,11 @@
> #include "phy-qcom-qmp-pcs-ufs-v4.h"
> #include "phy-qcom-qmp-pcs-ufs-v5.h"
> #include "phy-qcom-qmp-pcs-ufs-v6.h"
> +#include "phy-qcom-qmp-pcs-ufs-v7.h"
>
> #include "phy-qcom-qmp-qserdes-txrx-ufs-v6.h"
> #include "phy-qcom-qmp-qserdes-txrx-ufs-v7.h"
> +#include "phy-qcom-qmp-qserdes-txrx-ufs-v8.h"
>
> /* QPHY_PCS_READY_STATUS bit */
> #define PCS_READY BIT(0)
> @@ -84,6 +86,13 @@ static const unsigned int ufsphy_v6_regs_layout[QPHY_LAYOUT_SIZE] = {
> [QPHY_PCS_POWER_DOWN_CONTROL] = QPHY_V6_PCS_UFS_POWER_DOWN_CONTROL,
> };
>
> +static const unsigned int ufsphy_v7_regs_layout[QPHY_LAYOUT_SIZE] = {
> + [QPHY_START_CTRL] = QPHY_V7_PCS_UFS_PHY_START,
> + [QPHY_PCS_READY_STATUS] = QPHY_V7_PCS_UFS_READY_STATUS,
> + [QPHY_SW_RESET] = QPHY_V7_PCS_UFS_SW_RESET,
> + [QPHY_PCS_POWER_DOWN_CONTROL] = QPHY_V7_PCS_UFS_POWER_DOWN_CONTROL,
> +};
> +
> static const struct qmp_phy_init_tbl milos_ufsphy_serdes[] = {
> QMP_PHY_INIT_CFG(QSERDES_V6_COM_SYSCLK_EN_SEL, 0xd9),
> QMP_PHY_INIT_CFG(QSERDES_V6_COM_CMN_CONFIG_1, 0x16),
> @@ -1307,6 +1316,11 @@ static const struct regulator_bulk_data sm8750_ufsphy_vreg_l[] = {
> { .supply = "vdda-pll", .init_load_uA = 18300 },
> };
>
> +static const struct regulator_bulk_data hawi_ufsphy_vreg_l[] = {
> + { .supply = "vdda-phy", .init_load_uA = 324000 },
> + { .supply = "vdda-pll", .init_load_uA = 27000 },
> +};
> +
> static const struct qmp_ufs_offsets qmp_ufs_offsets = {
> .serdes = 0,
> .pcs = 0xc00,
> @@ -1325,6 +1339,15 @@ static const struct qmp_ufs_offsets qmp_ufs_offsets_v6 = {
> .rx2 = 0x1a00,
> };
>
> +static const struct qmp_ufs_offsets qmp_ufs_offsets_v7 = {
> + .serdes = 0,
> + .pcs = 0x0400,
> + .tx = 0x2000,
> + .rx = 0x2000,
> + .tx2 = 0x3000,
> + .rx2 = 0x3000,
> +};
> +
> static const struct qmp_phy_cfg milos_ufsphy_cfg = {
> .lanes = 2,
>
> @@ -1845,6 +1868,119 @@ static const struct qmp_phy_cfg sm8750_ufsphy_cfg = {
>
> };
>
> +static const struct qmp_phy_init_tbl hawi_ufsphy_serdes[] = {
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_SYSCLK_EN_SEL, 0xd9),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CMN_CONFIG_1, 0x16),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_HSCLK_SEL_1, 0x11),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_HSCLK_HS_SWITCH_SEL_1, 0x00),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP_EN, 0x01),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP_CFG, 0x60),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_IVCO, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_IVCO_MODE1, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CMN_IETRIM, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CMN_IPTRIM, 0x20),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_VCO_TUNE_MAP, 0x04),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_VCO_TUNE_CTRL, 0x40),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_ADAPTIVE_ANALOG_CONFIG, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_DEC_START_MODE0, 0x41),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_MODE0, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCTRL_MODE0, 0x18),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_MODE0, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_ADAPTIVE_MODE0, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCCTRL_ADAPTIVE_MODE0, 0x18),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_ADAPTIVE_MODE0, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP1_MODE0, 0x7f),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP2_MODE0, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE1_MODE0, 0x92),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE2_MODE0, 0x1e),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_DEC_START_MODE1, 0x4c),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_MODE1, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCTRL_MODE1, 0x18),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_MODE1, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_ADAPTIVE_MODE1, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCCTRL_ADAPTIVE_MODE1, 0x18),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_ADAPTIVE_MODE1, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP1_MODE1, 0x99),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP2_MODE1, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE1_MODE1, 0xbe),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE2_MODE1, 0x23),
> +};
> +
> +static const struct qmp_phy_init_tbl hawi_ufsphy_tx[] = {
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_TX_LANE_MODE_1, 0x0c),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_TX, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_RX, 0x17),
> +};
> +
> +static const struct qmp_phy_init_tbl hawi_ufsphy_rx[] = {
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE2, 0x0c),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE4, 0x0c),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_SO_GAIN_RATE4, 0x04),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_PI_CONTROLS, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_OFFSET_ADAPTOR_CNTRL3, 0x0e),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FASTLOCK_COUNT_HIGH_RATE4, 0x02),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FASTLOCK_FO_GAIN_RATE4, 0x1c),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FASTLOCK_SO_GAIN_RATE4, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_VGA_CAL_MAN_VAL, 0x8e),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_EQU_ADAPTOR_CNTRL4, 0x0f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE_0_1_B4, 0xb8),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SA_B7, 0x66),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SA_B9, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SB_B7, 0x66),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SB_B9, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SA_B7, 0x66),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SA_B9, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SB_B7, 0x66),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SB_B9, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_SO_SATURATION, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_TERM_BW_CTRL0, 0xfa),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_DLL0_FTUNE_CTRL, 0x30),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_SIGDET_CAL_TRIM, 0x77),
> +};
> +
> +static const struct qmp_phy_init_tbl hawi_ufsphy_pcs[] = {
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_TX_MID_TERM_CTRL1, 0x43),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_PCS_CTRL1, 0x42),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_TX_LARGE_AMP_DRV_LVL, 0x0f),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_RX_SIGDET_CTRL2, 0x68),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_MULTI_LANE_CTRL1, 0x02),
> +};
> +
> +static const struct qmp_phy_init_tbl hawi_ufsphy_g5_pcs[] = {
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_PLL_CNTL, 0x3b),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_TX_HSGEAR_CAPABILITY, 0x05),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_RX_HSGEAR_CAPABILITY, 0x05),
> +};
> +
> +static const struct qmp_phy_cfg hawi_ufsphy_cfg = {
> + .lanes = 2,
> +
> + .offsets = &qmp_ufs_offsets_v7,
> + .max_supported_gear = UFS_HS_G5,
> +
> + .tbls = {
> + .serdes = hawi_ufsphy_serdes,
> + .serdes_num = ARRAY_SIZE(hawi_ufsphy_serdes),
> + .tx = hawi_ufsphy_tx,
> + .tx_num = ARRAY_SIZE(hawi_ufsphy_tx),
> + .rx = hawi_ufsphy_rx,
> + .rx_num = ARRAY_SIZE(hawi_ufsphy_rx),
> + .pcs = hawi_ufsphy_pcs,
> + .pcs_num = ARRAY_SIZE(hawi_ufsphy_pcs),
> + },
> +
> + .tbls_hs_overlay[0] = {
> + .pcs = hawi_ufsphy_g5_pcs,
> + .pcs_num = ARRAY_SIZE(hawi_ufsphy_g5_pcs),
> + .max_gear = UFS_HS_G5,
> + },
> +
> + .vreg_list = hawi_ufsphy_vreg_l,
> + .num_vregs = ARRAY_SIZE(hawi_ufsphy_vreg_l),
> + .regs = ufsphy_v7_regs_layout,
> +};
> +
> static void qmp_ufs_serdes_init(struct qmp_ufs *qmp, const struct qmp_phy_cfg_tbls *tbls)
> {
> void __iomem *serdes = qmp->serdes;
> @@ -2259,6 +2395,9 @@ static int qmp_ufs_probe(struct platform_device *pdev)
>
> static const struct of_device_id qmp_ufs_of_match_table[] = {
> {
> + .compatible = "qcom,hawi-qmp-ufs-phy",
> + .data = &hawi_ufsphy_cfg,
> + }, {
> .compatible = "qcom,milos-qmp-ufs-phy",
> .data = &milos_ufsphy_cfg,
> }, {
> --
> 2.34.1
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH 6/7] hwmon: adm1275: Support ROHM BD12790
From: Matti Vaittinen @ 2026-06-17 5:57 UTC (permalink / raw)
To: Guenter Roeck, Matti Vaittinen, Matti Vaittinen
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
Shuah Khan, Wensheng Wang, Ashish Yadav, Kim Seer Paller,
Cedric Encarnacion, Chris Packham, Yuxi Wang, Charles Hsu,
ChiShih Tsai, linux-hwmon, devicetree, linux-kernel, linux-doc
In-Reply-To: <d66b9de3-db06-4f83-9c2a-b45e341bfc9c@roeck-us.net>
On 16/06/2026 17:15, Guenter Roeck wrote:
> On 6/15/26 23:44, Matti Vaittinen wrote:
>> From: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> Add support for ROHM BD12790 hot-swap controller which is largely
>> similar to Analog Devices adm1272.
>>
>> The BD12790 uses the same selectable 60V/100V voltage ranges and
>> 15mV/30mV current-sense ranges as the ADM1272, and the same VRANGE
>> (bit 5) and IRANGE (bit 0) layout in PMON_CONFIG. It therefore uses
>> a dedicated coefficient table that mirrors adm1272_coefficients, with
>> the following differences derived from BD12790 datasheet Table 1 (p.18):
>> - power 60V/30mV: m=17560 (vs. 17561)
>> - power 100V/30mV: m=10536 (vs. 10535)
>> - temperature: b=31880 (vs. 31871, reflecting T[11:0] = 4.2*T + 3188)
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>> Assisted-by: GitHub Copilot:claude-sonnet-4.6
>>
>> ---
// snip
>> -/* The BD12780 data sheets mark TSFILT bit as reserved. */
>> -#define BD12780_PMON_DEFCONFIG (ADM1278_VOUT_EN |
>> ADM1278_TEMP1_EN)
>> +/* The BD127x0 data sheets mark TSFILT bit as reserved. */
>> +#define BD127X0_PMON_DEFCONFIG (ADM1278_VOUT_EN |
>> ADM1278_TEMP1_EN)
>
> Please don't use such placeholders. Just use BD12780_PMON_DEFCONFIG
> for both chips, similar to how the defines for all other chips
> are handled.
Ok, thanks.
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* Re: [PATCH 6/7] hwmon: adm1275: Support ROHM BD12790
From: Matti Vaittinen @ 2026-06-17 5:56 UTC (permalink / raw)
To: Guenter Roeck, Matti Vaittinen, Matti Vaittinen
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
Shuah Khan, Wensheng Wang, Ashish Yadav, Kim Seer Paller,
Cedric Encarnacion, Chris Packham, Yuxi Wang, Charles Hsu,
ChiShih Tsai, linux-hwmon, devicetree, linux-kernel, linux-doc
In-Reply-To: <e1673147-a51f-460c-ad09-21f54d3d6803@roeck-us.net>
On 16/06/2026 17:08, Guenter Roeck wrote:
> On 6/15/26 23:44, Matti Vaittinen wrote:
>> From: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> Add support for ROHM BD12790 hot-swap controller which is largely
>> similar to Analog Devices adm1272.
>>
>> The BD12790 uses the same selectable 60V/100V voltage ranges and
>> 15mV/30mV current-sense ranges as the ADM1272, and the same VRANGE
>> (bit 5) and IRANGE (bit 0) layout in PMON_CONFIG. It therefore uses
>> a dedicated coefficient table that mirrors adm1272_coefficients, with
>> the following differences derived from BD12790 datasheet Table 1 (p.18):
>> - power 60V/30mV: m=17560 (vs. 17561)
>> - power 100V/30mV: m=10536 (vs. 10535)
>> - temperature: b=31880 (vs. 31871, reflecting T[11:0] = 4.2*T + 3188)
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>> Assisted-by: GitHub Copilot:claude-sonnet-4.6
>>
>> ---
>> Originally this patch was AI-generated. I did pretty much re-write the
>> probe changes by hand, and also fixed some of the coefficient math
>> afterwards :/ But yeah, this one was AI "assisted". :)
>>
>> drivers/hwmon/pmbus/Kconfig | 4 +--
>> drivers/hwmon/pmbus/adm1275.c | 53 +++++++++++++++++++++++++++++------
>> 2 files changed, 47 insertions(+), 10 deletions(-)
// snip
>> @@ -655,12 +681,23 @@ static int adm1275_probe(struct i2c_client *client)
>> break;
>> case adm1272:
>> case adm1273:
>> + case bd12790:
>
> Please don't overload the existing case statements.
> Just add separate case statements for the new chips.
>
Hmm. Ok, although, here, same as with the BD12780, I would like the code
to clearly show that the BD12790 is very very similar to another IC
(adm1272). When we have own case for it, this information gets kind of
lost as these cases are a tad too long to easily spot the differences.
If there are any ideas how to ease spotting this while having own cases
- I am keen to hear.
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* RE: [PATCH v6 3/3] PCI: imx6: Add root port reset to support link recovery
From: Hongxing Zhu (OSS) @ 2026-06-17 5:49 UTC (permalink / raw)
To: sashiko-reviews@lists.linux.dev, Hongxing Zhu (OSS)
Cc: devicetree@vger.kernel.org, imx@lists.linux.dev, robh@kernel.org,
conor+dt@kernel.org, Frank.Li@kernel.org,
linux-pci@vger.kernel.org
In-Reply-To: <20260603070533.641461F00893@smtp.kernel.org>
> -----Original Message-----
> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> Sent: Wednesday, June 3, 2026 3:06 PM
> To: Hongxing Zhu (OSS) <hongxing.zhu@oss.nxp.com>
> Cc: devicetree@vger.kernel.org; imx@lists.linux.dev; robh@kernel.org;
> conor+dt@kernel.org; Frank.Li@kernel.org; linux-pci@vger.kernel.org
> Subject: Re: [PATCH v6 3/3] PCI: imx6: Add root port reset to support link recovery
>
> Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to
> consider:
> - [High] Read-modify-write operations on mixed RW/W1C register silently drop
> interrupts.
> - [High] Shared interrupt handler checks status but fails to verify if the interrupt is
> enabled.
> - [High] Ignored return value of `dw_pcie_wait_for_link()` hides downstream link
> recovery failures.
> - [High] Lockless traversal of PCI bus devices in an asynchronous IRQ thread
> exposes the kernel to Use-After-Free and list corruption.
> - [High] Link down interrupt is not disabled during system shutdown, causing an
> AXI bus fault and system hang.
> - [High] Failed root port reset leaves the controller unclocked while still active,
> causing a crash on subsequent PM suspend.
> - [High] Unprotected hardware indirect register access during root port reset.
> - [Medium] Ignored negative error codes from
> `platform_get_irq_byname_optional()` break probe deferral.
> - [Low] Build failure due to undefined functions and struct members.
> --
>
> commit 5bee25b25210b0bc28349bc8474d10204a25cd72
> Author: Richard Zhu <hongxing.zhu@nxp.com>
>
> PCI: imx6: Add root port reset to support link recovery
>
> The PCIe link can go down due to various unexpected circumstances. Add root
> port reset support to enable link recovery for the i.MX PCIe controller when the
> optional "intr" interrupt is present.
>
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 773ab65b2afac..aa5c90be23df8 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
>
> [ ... ]
>
> > @@ -1394,6 +1401,13 @@ static int imx_pcie_host_init(struct dw_pcie_rp
> > *pp)
> >
> > imx_setup_phy_mpll(imx_pcie);
> >
> > + /*
> > + * Callback invoked by PCI core when link down is detected and
> > + * recovery is needed.
> > + */
> > + if (pp->bridge)
> > + pp->bridge->reset_root_port = imx_pcie_reset_root_port;
>
> [Severity: Low]
> Does the reset_root_port member actually exist in struct pci_host_bridge?
> This appears to cause a build failure since the struct member is undefined.
>
[Richard] the `reset_root_port` member and `pci_host_handle_link_down()`
function is introduced in a prerequisite patch that will be merged before
this one.
> [ ... ]
>
> > @@ -1661,6 +1675,9 @@ static int imx_pcie_suspend_noirq(struct device
> *dev)
> > if (!(imx_pcie->drvdata->flags & IMX_PCIE_FLAG_SUPPORTS_SUSPEND))
> > return 0;
> >
> > + if (imx_pcie->lnk_intr > 0)
> > + regmap_clear_bits(imx_pcie->iomuxc_gpr,
> IMX95_LINK_INT_CTRL_STS,
> > + IMX95_LINK_DOWN_INT_EN);
>
> [Severity: High]
> Since IMX95_LINK_INT_CTRL_STS mixes an RW enable bit and a write-1-to-clear
> status bit, will using regmap_clear_bits perform a read-modify-write cycle that
> inadvertently clears pending status bits if they are latched during the read?
>
[Richard] This is acceptable in the suspend path. Even if a pending interrupt
status bit is inadvertently cleared during the regmap_clear_bits() operation,
the link would be down at the end of suspend_noirq(), and retrained during
resume_noirq(). Same to the contents of link recovery.
> [ ... ]
>
> > +static irqreturn_t imx_pcie_lnk_irq_isr(int irq, void *priv) {
> > + struct imx_pcie *imx_pcie = priv;
> > + struct dw_pcie *pci = imx_pcie->pci;
> > + struct device *dev = pci->dev;
> > + u32 val;
> > +
> > + regmap_read(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS, &val);
> > + if (val & IMX95_LINK_DOWN_INT_STS) {
>
> [Severity: High]
> Is it possible for this shared interrupt handler to be invoked while the interrupt is
> disabled, see the latched status bit, and erroneously return IRQ_WAKE_THREAD?
>
> If a PME interrupt fires while the link down interrupt is intentionally disabled, the
> ISR will still claim it. Would it be safer to check if IMX95_LINK_DOWN_INT_EN is
> also set before proceeding?
[Richard]Good catch. I will add a check for IMX95_LINK_DOWN_INT_EN to ensure
the handler only processes the interrupt when it is enabled. This makes
the code more robust against spurious shared interrupt events.
>
> > + dev_dbg(dev, "PCIe link down detected, initiating recovery\n");
> > + /* Clear link down interrupt status by writing 1b'1 to it */
> > + regmap_set_bits(imx_pcie->iomuxc_gpr,
> IMX95_LINK_INT_CTRL_STS,
> > + IMX95_LINK_DOWN_INT_STS);
>
> [Severity: High]
> Similar to the regmap_clear_bits call above, could using regmap_set_bits here
> perform a read-modify-write cycle that inadvertently clears other pending write-
> 1-to-clear bits in the same register?
>
[Richard] This is safe here. The status bit is cleared immediately after being
read in the ISR, so there are no other pending W1C bits in this register that
could be inadvertently cleared by the read-modify-write operation.
> > + regmap_clear_bits(imx_pcie->iomuxc_gpr,
> IMX95_LINK_INT_CTRL_STS,
> > + IMX95_LINK_DOWN_INT_EN);
> > +
> > + return IRQ_WAKE_THREAD;
> > + }
> > +
> > + return IRQ_NONE;
> > +}
> > +
> > +static irqreturn_t imx_pcie_lnk_irq_thread(int irq, void *priv) {
> > + struct imx_pcie *imx_pcie = priv;
> > + struct dw_pcie *pci = imx_pcie->pci;
> > + struct dw_pcie_rp *pp = &pci->pp;
> > + struct pci_dev *port;
> > +
> > + for_each_pci_bridge(port, pp->bridge->bus)
>
> [Severity: High]
> Is it safe to iterate the bus list locklessly in this IRQ thread?
> Could concurrent modifications to the PCI bus list cause a use-after-free since the
> pci_bus_sem is not held?
>
[Richard] The PCI bus structure is stable at this point because devices are
only added/removed during enumeration or hotplug events, both of which hold
the appropriate locks. The threaded IRQ handler does not race with these
operations, so lockless traversal is safe here.
> > + if (pci_pcie_type(port) == PCI_EXP_TYPE_ROOT_PORT)
> > + pci_host_handle_link_down(port);
>
> [Severity: Low]
> Does the pci_host_handle_link_down function exist in the tree? This appears to
> be an undefined function call which will cause a build failure.
>
> > +
> > + regmap_set_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
> > + IMX95_LINK_DOWN_INT_EN);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int imx_pcie_reset_root_port(struct pci_host_bridge *bridge,
> > + struct pci_dev *pdev)
> > +{
> > + struct pci_bus *bus = bridge->bus;
> > + struct dw_pcie_rp *pp = bus->sysdata;
> > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > + struct imx_pcie *imx_pcie = to_imx_pcie(pci);
> > + int ret;
> > +
> > + imx_pcie_msi_save_restore(imx_pcie, true);
> > + if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
> > + imx_pcie_lut_save(imx_pcie);
>
> [Severity: High]
> The i.MX95 PCIe hardware uses an indirect access mechanism for LUTs. Since this
> reset executes asynchronously in a threaded IRQ, should imx_pcie->lock be
> acquired before calling imx_pcie_lut_save to prevent concurrent operations from
> clobbering the shared indirect address indices?
>
[Richard] The threaded IRQ handler is serialized by the kernel's IRQ subsystem,
so concurrent invocations of the same handler are not possible. Additionally,
no other code path accesses the LUT registers during link recovery, so there
is no race condition here.
> > + imx_pcie_stop_link(pci);
> > + imx_pcie_host_exit(pp);
> > +
> > + ret = imx_pcie_host_init(pp);
> > + if (ret) {
> > + dev_err(pci->dev, "Failed to re-init PCIe\n");
> > + return ret;
> > + }
> > + ret = dw_pcie_setup_rc(pp);
> > + if (ret)
> > + goto err_host_deinit;
> > +
> > + imx_pcie_start_link(pci);
> > + dw_pcie_wait_for_link(pci);
>
> [Severity: High]
> Should the return value of dw_pcie_wait_for_link be checked here?
> By ignoring it, the function might falsely report success to the PCI core even if
> downstream link recovery fails, which could lead to unsupported request aborts
> when accessing downstream devices.
>
[Richard] The return value of dw_pcie_wait_for_link() is intentionally not
checked here. Link recovery is best-effort; even if the link does not come
back up immediately, the MSI and LUT state has been restored, and the system
remains stable. Subsequent link training attempts may succeed.
> > +
> > + if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
> > + imx_pcie_lut_restore(imx_pcie);
> > + imx_pcie_msi_save_restore(imx_pcie, false);
> > +
> > + dev_dbg(pci->dev, "Root port reset completed\n");
> > + return 0;
> > +
> > +err_host_deinit:
> > + imx_pcie_host_exit(pp);
>
> [Severity: High]
> If the re-initialization fails and imx_pcie_host_exit disables the clocks, the device
> still remains bound. If the system later initiates a suspend, will
> imx_pcie_suspend_noirq execute unconditionally and trigger a fatal external
> abort by attempting to read from the unclocked DBI registers?
[Richard] You're right that this is a potential issue. If re-initialization
fails, the clocks are disabled but the device remains bound. Because that
host_deinit and host_init are already symmetrical, the host_deinit shouldn't
be in the error exit path again. Would leave the clocks enabled on failure
to maintain a safe state.
>
> > +
> > + return ret;
> > +}
> > +
>
> [ ... ]
>
> > @@ -1919,9 +2018,33 @@ static int imx_pcie_probe(struct platform_device
> *pdev)
> > val |= PCI_MSI_FLAGS_ENABLE;
> > dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
> > }
> > +
> > + /* Get link event irq if it is present */
> > + imx_pcie->lnk_intr = platform_get_irq_byname_optional(pdev,
> "intr");
> > + if (imx_pcie->lnk_intr > 0) {
>
> [Severity: Medium]
> If the interrupt controller is not yet ready, the platform irq function returns -
> EPROBE_DEFER. Does checking strictly for > 0 cause the driver to silently ignore
> this error code and fail to defer the probe, permanently disabling the link recovery
> functionality?
>
> [ ... ]
[Richard] Good point. I will update the code to explicitly handle -EPROBE_DEFER:
imx_pcie->lnk_intr = platform_get_irq_byname_optional(pdev, "intr");
if (imx_pcie->lnk_intr == -EPROBE_DEFER)
return -EPROBE_DEFER;
if (imx_pcie->lnk_intr > 0) {
/* setup interrupt handler */
}
>
> > +err_host_deinit:
> > + dw_pcie_host_deinit(&pci->pp);
> > +
> > + return ret;
> > }
> >
> > static void imx_pcie_shutdown(struct platform_device *pdev)
>
> [Severity: High]
> The patch enables the link down interrupt during probe, but looking at
> imx_pcie_shutdown(), it does not disable it before asserting core reset.
> Does asserting the core reset drop the link and trigger the interrupt?
> If so, this could wake the threaded handler to access DBI registers while the core
> is in reset, causing a synchronous external abort.
>
[Richard] Agreed. I will add code to disable the link down interrupt in
imx_pcie_shutdown() before asserting core reset
Best Regards
Richard Zhu
> --
> Sashiko AI
> review * https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsa
> shiko.dev%2F%23%2Fpatchset%2F20260603062510.3767610-1-
> hongxing.zhu%40oss.nxp.com%3Fpart%3D3&data=05%7C02%7Chongxing.zhu%4
> 0nxp.com%7C134330e3333a44f1683508dec13f9e4a%7C686ea1d3bc2b4c6fa92c
> d99c5c301635%7C0%7C0%7C639160676183298029%7CUnknown%7CTWFpbGZ
> sb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkF
> OIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=T12KsupOJBTSoTXb
> rB0CAYhsJrCThSsXkEpdoqsB1GY%3D&reserved=0
^ permalink raw reply
* Re: [PATCH 3/7] hwmon: adm1275: Support ROHM BD12780
From: Matti Vaittinen @ 2026-06-17 5:48 UTC (permalink / raw)
To: Guenter Roeck, Matti Vaittinen, Matti Vaittinen
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
Shuah Khan, Wensheng Wang, Ashish Yadav, Kim Seer Paller,
Cedric Encarnacion, Chris Packham, Yuxi Wang, Charles Hsu,
ChiShih Tsai, linux-hwmon, devicetree, linux-kernel, linux-doc
In-Reply-To: <67bec6ae-4f21-441a-8c5e-b56bd1a2ab84@roeck-us.net>
Thanks for taking the time to review this! Feedback is appreciated :)
On 16/06/2026 17:13, Guenter Roeck wrote:
> On 6/15/26 23:36, Matti Vaittinen wrote:
>> From: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> ROHM BD12780 and BD12780A are hot-swap controllers. They are largely
>> similar to Analog Devices ADM1278. Besides the ID registers and some
>> added functionality, the BD12780 and BD12780A mark PMON_CONFIG bits
>> [15:14] as reserved. Hence TSFILT setting must be omitted on these ICs.
>>
>> The BD12780 has 3 pins usable for configuring the I2C address. The
>> BD12780A lists the ADDR3-pin as "not connect".
>>
>> Support ROHM BD12780 and BD12780A controllers.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>> ---
>> drivers/hwmon/pmbus/Kconfig | 2 +-
>> drivers/hwmon/pmbus/adm1275.c | 46 +++++++++++++++++++++++++++++------
>> 2 files changed, 39 insertions(+), 9 deletions(-)
>>
// snip
>> @@ -487,6 +489,21 @@ static const struct i2c_device_id adm1275_id[] = {
>> { "adm1281", adm1281 },
>> { "adm1293", adm1293 },
>> { "adm1294", adm1294 },
>> + /*
>> + * The BD12780a is functionally identical to BD12780(*). Even the
>> pmbus ID
>> + * register contents are same. When instantiated from the DT, it
>> is required
>> + * to have the bd12780 as a fall-back. We still need the bd12780a
>> ID here,
>> + * because the i2c_device_id is created from the first
>> compatible, not from
>> + * the fall-back entry.
>> + * (*)Until proven to differ. I prefer having own compatible for
>> these
>> + * variants for that day. Please note that even though the probe
>> is called
>> + * based on the 'bd12780a' -entry, the ID is picked at probe
>> based on the
>> + * pmbus register contents and not by DT entry. Thus, if the
>> bd12780 and
>> + * bd12780a are found to require different handling, then this
>> needs to be
>> + * changed, or bd12780a is handled as bd12780.
>> + */
>> + { "bd12780", bd12780 },
>> + { "bd12780a", /* driver data unused, see --^ */ },
>
> We don't usually do that. There are various A/B/C variants for many chips,
> and we just use the base name unless a difference is warranted. Either this
> is needed, and driver data is needed as well, or it isn't. If it is not
> needed,
> it should be dropped.
At the moment the only difference I know is reduced amount of I2C slave
addresses. This shouldn't be visible to this driver.
My problem is that I don't know for sure if we later notice something
that requires differentiating. Thus I would like to have different DT
compatibles (or other source of I2C IDs if those are used to instantiate
the driver). If we don't do this, then we have problems if we later find
out that these ICs require different handling as users because we can't
differentiate these ICs if they are described with same compatible/I2C ID.
The "fun" thing is that both variants have exactly same MFR_MODEL and
MFR_REVISION register contents. Thus, these ICs can't be differentiated
by reading PMBus registers.
This is also why the driver data entry gets unused. The existing probe
mechanism in this driver, scans the names in this ID list and compares
it to the PMBus MFR_MODEL, and then picks-up the driver data to use. Now
because BD12780 and BD12780A both have same MFR_MODEL, the scan in probe
will always pick the same driver data entry, no matter what ID was
matched by bus code. That's why I added the comment here.
If I drop the { "bd12780a", /* driver data unused, see --^ */ } -entry
from the ID list and add the of_device_ids, then I think this problem is
solved for the DT-platforms. As far as I understand, this would still
cause any non DT platform to describe both variants as "bd12780", making
it impossible to later differentiate ICs in the driver, right? I can do
this, but for me it feels a bit like asking for problems...
My thinking was to have different IDs for these variants so hardware
description could have different IDs for different ICs. Then, if we
later need to differentiate these ICs, we still have an option to change
the probe to trust the i2c_get_match_data() when PMBus indicates the
bd12780.
This however is some extra complexity, and I would like to add it to the
probe only if it really is required.
But yeah, having an ID entry in the list and driver data not used even
when the matching IC is found, is unusual and would have caught me off
guard. Hence I added the (long) comment.
>
>> { "mc09c", sq24905c },
>> { }
>> };
// snip
>> @@ -712,7 +732,16 @@ static int adm1275_probe(struct i2c_client *client)
>> break;
>> case adm1278:
>> case adm1281:
>> + case bd12780:
>> case sq24905c:
>> + {
>> + u16 defconfig;
>> +
>> + if (data->id == bd12780)
>> + defconfig = BD12780_PMON_DEFCONFIG;
>> + else
>> + defconfig = ADM1278_PMON_DEFCONFIG;
>> +
>
> Please add a separate case statement for the new chip
> and do not overload existing chip data.
I originally did just that. I, however, was not happy with this as it
resulted quite long own case this IC, which is almost identical to this
other (adm1278, adm1281, sq24905c) case. I wanted the code to shout that
the DB12780 is indeed (almost) identical to the adm1278. But yeah, I
agree, having "if (foo)" in "switch (foo)" case can get confusing.
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* RE: [PATCH v2 4/5] arm64: dts: exynos: add initial support for Samsung Exynos8855 smdk
From: Alim Akhtar @ 2026-06-17 5:41 UTC (permalink / raw)
To: 'Peter Griffin'
Cc: krzk, robh, conor+dt, linusw, linux-samsung-soc, linux-kernel,
devicetree, linux-gpio, hajun.sung
In-Reply-To: <CADrjBPqS=d2Q02UhdpkSxHJ-RYe-hvurB-1meurTOLoUcUidFQ@mail.gmail.com>
Hi Peter
> -----Original Message-----
> From: Peter Griffin <peter.griffin@linaro.org>
> Sent: Monday, June 15, 2026 8:07 PM
> To: Alim Akhtar <alim.akhtar@samsung.com>
> Cc: krzk@kernel.org; robh@kernel.org; conor+dt@kernel.org;
> linusw@kernel.org; linux-samsung-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-
> gpio@vger.kernel.org; hajun.sung@samsung.com
> Subject: Re: [PATCH v2 4/5] arm64: dts: exynos: add initial support for
> Samsung Exynos8855 smdk
>
> Hi Alim,
>
> On Mon, 15 Jun 2026 at 09:34, Alim Akhtar <alim.akhtar@samsung.com>
> wrote:
> >
> > Add initial devicetree support for Samsung smdk board using
> > Exynos8855 SoC.
>
> I think it would be worthwhile adding a more verbose description of the
> Exynos8855 SoC in the commit message e.g. a brief list of the major IPs on
> the SoC.
>
Sure will add more details here
> >
> > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> [..]
> > diff --git a/arch/arm64/boot/dts/exynos/exynos8855-smdk.dts
> > b/arch/arm64/boot/dts/exynos/exynos8855-smdk.dts
> > new file mode 100644
> > index 000000000000..f5132bcaa47c
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/exynos/exynos8855-smdk.dts
> > @@ -0,0 +1,32 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Samsung Exynos8855 SMDK board device tree source
> > + *
> > + * Copyright (C) 2026 Samsung Electronics Co., Ltd.
> > + *
> > + * Device tree source file for WinLink's E850-96 board which is based
> > +on
> > + * Samsung Exynos8855 SoC.
>
> E850-96 isn't based on the Exynos8855 SoC. I guess it's leftover from a
> copy/paste.
>
Ah! My bad, will remove these, Thanks
> regards,
>
> Peter
>
>
^ permalink raw reply
* RE: [PATCH v2 3/5] pinctrl: samsung: Add Exynos8855 pinctrl configuration
From: Alim Akhtar @ 2026-06-17 5:38 UTC (permalink / raw)
To: 'Peter Griffin'
Cc: krzk, robh, conor+dt, linusw, linux-samsung-soc, linux-kernel,
devicetree, linux-gpio, hajun.sung
In-Reply-To: <CADrjBPpyNymPXtYgdeOGtp1KDkdp9gTrBAEcJR89B+wwF8uUXw@mail.gmail.com>
> -----Original Message-----
> From: Peter Griffin <peter.griffin@linaro.org>
> Sent: Monday, June 15, 2026 7:45 PM
> To: Alim Akhtar <alim.akhtar@samsung.com>
> Cc: krzk@kernel.org; robh@kernel.org; conor+dt@kernel.org;
> linusw@kernel.org; linux-samsung-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-
> gpio@vger.kernel.org; hajun.sung@samsung.com
> Subject: Re: [PATCH v2 3/5] pinctrl: samsung: Add Exynos8855 pinctrl
> configuration
>
> Hi Alim,
>
> Thanks for your patch. It's great to see more Exynos SoCs being supported!
>
Thanks Peter, more patches to follow.
> On Mon, 15 Jun 2026 at 09:34, Alim Akhtar <alim.akhtar@samsung.com>
> wrote:
> >
> > Add pinctrl configuration for Exynos8855. The bank type macros are
> > reused from Exynos850 SoC.
> >
> > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> > ---
> > .../pinctrl/samsung/pinctrl-exynos-arm64.c | 123 ++++++++++++++++++
> > drivers/pinctrl/samsung/pinctrl-samsung.c | 2 +
> > drivers/pinctrl/samsung/pinctrl-samsung.h | 1 +
> > 3 files changed, 126 insertions(+)
> >
> > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> > b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> > index fe9f92cb037e..db120ae4d847 100644
> > --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> > +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> > @@ -943,6 +943,129 @@ const struct samsung_pinctrl_of_match_data
> exynos850_of_data __initconst = {
> > .num_ctrl = ARRAY_SIZE(exynos850_pin_ctrl),
> > };
> >
>
> Are you sure you want to use E850 pinctrl macros and not the GS101 ones?
> The GS101 macros allow the fltcon offset to be specified, which I think is
> required for all Exynos9 (including e850 SoC). Youngmin sent a series
> previously https://lore.kernel.org/lkml/20251202093613.852109-1-
> youngmin.nam@samsung.com/
> fixing up some of this but it hasn't been re-spun in a while. In particular this
> patch https://lore.kernel.org/lkml/20251202093613.852109-4-
> youngmin.nam@samsung.com/.
>
Thanks for pointing out, let me re-look into this, according will change.
> > +/* pin banks of exynos8855 pin-controller 0 (ALIVE) */ static const
> > +struct samsung_pin_bank_data exynos8855_pin_banks0[] __initconst = {
> > + /* Must start with EINTG banks, ordered by EINT group number. */
[Snip]
> > +static const struct samsung_pin_ctrl exynos8855_pin_ctrl[] __initconst = {
> > + {
> > + /* pin-controller instance 0 ALIVE data */
> > + .pin_banks = exynos8855_pin_banks0,
> > + .nr_banks = ARRAY_SIZE(exynos8855_pin_banks0),
> > + .eint_wkup_init = exynos_eint_wkup_init,
> > + .eint_gpio_init = exynos_eint_gpio_init,
> > + }, {
>
> With fltcon_offset specified, you could then use
> gs101_pinctrl_suspend/gs101_pinctrl_resume callbacks here.
>
Let me cross check with UM and will add accordingly.
^ permalink raw reply
* RE: [PATCH v32 5/5] i2c: ast2600: Add target mode support
From: Ryan Chen @ 2026-06-17 5:27 UTC (permalink / raw)
To: sashiko-reviews@lists.linux.dev
Cc: robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org
In-Reply-To: <20260611054645.3FEEF1F00893@smtp.kernel.org>
Hello,
> Subject: Re: [PATCH v32 5/5] i2c: ast2600: Add target mode support
>
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to
> consider:
> - [High] Incorrect chronological ordering of I2C slave events and spurious event
> signaling in the target interrupt handler breaks backend state machines.
> - [High] Controller mode can be permanently locked out because
> `target_active` is not reliably cleared upon target transaction STOP.
> - [High] Race condition in the target mode master-abort fast path leaves stale
> controller interrupts pending, leading to use-after-free and data corruption of
> subsequent I2C transfers.
> - [High] Failure to W1C-clear target ISR error bits causes a permanent
> deadlock where all subsequent controller transfers fail with -EBUSY.
> --
>
> commit e5417daca64552c41b25f31b14fb3eb562d3e134
> Author: Ryan Chen <ryan_chen@aspeedtech.com>
>
> i2c: ast2600: Add target mode support
>
> Add target mode support to the AST2600 I2C driver, enabling dual-role
> capability. This includes adding target interrupt handling and address match
> response logic.
>
> > diff --git a/drivers/i2c/busses/i2c-ast2600.c
> > b/drivers/i2c/busses/i2c-ast2600.c
> > index 70f37dc77468a..aace0c7eeb721 100644
> > --- a/drivers/i2c/busses/i2c-ast2600.c
> > +++ b/drivers/i2c/busses/i2c-ast2600.c
> > @@ -348,6 +353,253 @@ static int ast2600_i2c_recover_bus(struct
> > ast2600_i2c_bus *i2c_bus)
> [ ... ]
> > +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> > +static void ast2600_i2c_target_packet_buff_irq(struct ast2600_i2c_bus
> > +*i2c_bus, u32 sts) {
> > + u8 value = 0;
> > + int target_rx_len = 0;
> > + u32 cmd = 0;
> > + int i;
> > +
> > + /* due to controller target is common buffer, need force the master stop
> not issue */
> > + if (readl(i2c_bus->reg_base + AST2600_I2CM_CMD_STS) & GENMASK(15,
> 0)) {
> > + writel(0, i2c_bus->reg_base + AST2600_I2CM_CMD_STS);
> > + writel(0, i2c_bus->reg_base + AST2600_I2CM_IER);
> > + writel(readl(i2c_bus->reg_base + AST2600_I2CM_ISR),
> > + i2c_bus->reg_base + AST2600_I2CM_ISR);
> > + i2c_bus->cmd_err = -EBUSY;
> > + WRITE_ONCE(i2c_bus->msgs, NULL);
> > + writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
> > + i2c_bus->reg_base + AST2600_I2CM_IER);
> > + complete(&i2c_bus->cmd_complete);
> > + }
>
> [Severity: High]
> Could this fast path abort leave stale controller interrupts pending?
>
> If the hardware asynchronously asserts PKT_DONE or PKT_ERROR shortly after
> the W1C ISR read here, the complete() call wakes the waiting thread, which
> might immediately start a new controller transfer.
>
> Would the stale interrupt then fire and incorrectly apply to the new transfer,
> potentially leading to a use-after-free on the new msgs buffer?
The abort path disables the controller IER, W1C-clears I2CM ISR, and
sets i2c_bus->msgs = NULL before complete(). Per the datasheet the controller ISR
is not re-asserted asynchronously once cleared, so the next transfer starts with I2CM_ISR == 0.
Even if a late IRQ were delivered, the controller handler bails out immediately because
msgs is NULL, so it cannot touch a subsequent transfer's buffer. There is no use-after-free.
>
> > +
> > + /* Handle i2c target timeout condition */
> > + if (sts & AST2600_I2CS_INACTIVE_TO) {
> > + /* Reset timeout counter */
> > + u32 ac_timing = readl(i2c_bus->reg_base +
> AST2600_I2CC_AC_TIMING) &
> > + AST2600_I2CC_AC_TIMING_MASK;
> > +
> > + writel(ac_timing, i2c_bus->reg_base + AST2600_I2CC_AC_TIMING);
> > + ac_timing |= AST2600_I2CC_TTIMEOUT(i2c_bus->timeout);
> > + writel(ac_timing, i2c_bus->reg_base + AST2600_I2CC_AC_TIMING);
> > + writel(TARGET_TRIGGER_CMD, i2c_bus->reg_base +
> AST2600_I2CS_CMD_STS);
> > + writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base +
> AST2600_I2CS_ISR);
> > + i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
> > + WRITE_ONCE(i2c_bus->target_active, false);
> > + return;
> > + }
>
> [Severity: High]
> Does this early return fail to clear the AST2600_I2CS_INACTIVE_TO bit from
> the hardware register?
>
> Since it is never W1C-cleared, the ISR register might remain non-zero
> permanently.
Per I2CS24[16], in packet mode clearing PKT_DONE also clears bits 0, 2, [5:4],
7, 15 and 17 — and bit 15 is INACTIVE_TO.
The handler does writel(I2CS_PKT_DONE, I2CS_ISR) on the timeout path,
which clears INACTIVE_TO along with PKT_DONE...
>
> > +
> > + sts &= ~(AST2600_I2CS_PKT_DONE | AST2600_I2CS_PKT_ERROR);
> > +
> > + if (sts & AST2600_I2CS_SLAVE_MATCH)
> > + WRITE_ONCE(i2c_bus->target_active, true);
> > +
> > + switch (sts) {
> > + case AST2600_I2CS_SLAVE_PENDING | AST2600_I2CS_WAIT_RX_DMA |
> > + AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE |
> AST2600_I2CS_STOP:
> > + case AST2600_I2CS_SLAVE_PENDING |
> > + AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE |
> AST2600_I2CS_STOP:
> > + case AST2600_I2CS_SLAVE_PENDING |
> > + AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_STOP:
> > + i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
> > + fallthrough;
>
> [Severity: High]
> Is the event ordering inverted here?
>
> This unconditionally emits I2C_SLAVE_STOP first, and then falls through to emit
> I2C_SLAVE_WRITE_REQUESTED and I2C_SLAVE_WRITE_RECEIVED.
>
> Emitting the STOP condition prior to receiving the write data leaves backend
> state machines (like i2c-slave-eeprom) without a terminating event for the
> data, which typically causes writes to be dropped.
The cases in question all carry both SLAVE_PENDING and SLAVE_MATCH ... I2CS24[29]
(SLAVE_PENDING) is read-only and means "a previous slave-receive has not been
serviced yet while a new address match is already pending".
So this single interrupt spans two back-to-back transactions:
the STOP belongs to the previous one, while SLAVE_MATCH and the following data
belong to the new one...
>
> [ ... ]
> > + case AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_WAIT_TX_DMA |
> AST2600_I2CS_RX_DONE:
> > + case AST2600_I2CS_WAIT_TX_DMA | AST2600_I2CS_RX_DONE:
> > + case AST2600_I2CS_WAIT_TX_DMA:
> > + if (sts & AST2600_I2CS_SLAVE_MATCH)
> > + i2c_slave_event(i2c_bus->target,
> I2C_SLAVE_WRITE_REQUESTED,
> > +&value);
>
> [Severity: High]
> Can this spuriously emit I2C_SLAVE_WRITE_REQUESTED during a repeated
> start transitioning to a Master Read (WAIT_TX_DMA)?
>
> If so, it would reset the backend's internal write pointers, corrupting previously
> received write data before I2C_SLAVE_READ_REQUESTED is processed.
The related WAIT_TX_DMA concern is the same: WRITE_REQUESTED is only
emitted when SLAVE_MATCH is also set (a fresh address match),
which is the offset-write phase of a combined register-read.
A plain repeated-start-to-read (no SLAVE_MATCH) does not enter that emit path,
so the backend write pointer is not reset.
>
> [ ... ]
> > + if (cmd)
> > + writel(cmd, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
> > +
> > + writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base +
> AST2600_I2CS_ISR);
> > + readl(i2c_bus->reg_base + AST2600_I2CS_ISR);
> > +
> > + if ((sts & AST2600_I2CS_STOP) && !(sts &
> AST2600_I2CS_SLAVE_PENDING))
> > + WRITE_ONCE(i2c_bus->target_active, false); }
>
> [Severity: High]
> Is this condition too strict?
>
> For standard target write transactions, the hardware explicitly combines
> AST2600_I2CS_STOP and AST2600_I2CS_SLAVE_PENDING in the same
> interrupt event.
>
> If this evaluates to false in those normal cases, target_active would remain
> permanently true, locking out all subsequent controller mode transfers with
> -EBUSY.
This is intentional. STOP combined with SLAVE_PENDING means the previous
transaction ended but a new back-to-back transaction is already pending,
so the target is still in use and must stay active. A standalone target transaction ends
with STOP without SLAVE_PENDING and is cleared normally.
The timeout path (INACTIVE_TO) also clears target_active explicitly...
>
> > @@ -682,6 +954,21 @@ static int ast2600_i2c_controller_xfer(struct
> i2c_adapter *adap, struct i2c_msg
> > return ret;
> > }
> >
> > +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> > + if (READ_ONCE(i2c_bus->target_active))
> > + return -EBUSY;
> > + /*
> > + * Controller and target share the same buffer register. A target
> > + * transaction can update buffer state asynchronously via IRQ, so block
> > + * controller transfers while target is active to avoid buffer corruption.
> > + */
> > + writel(0, i2c_bus->reg_base + AST2600_I2CS_IER);
> > + if (readl(i2c_bus->reg_base + AST2600_I2CS_ISR) ||
> READ_ONCE(i2c_bus->target_active)) {
> > + writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base +
> AST2600_I2CS_IER);
> > + return -EBUSY;
> > + }
> > +#endif
>
> [Severity: High]
> If AST2600_I2CS_INACTIVE_TO is left uncleared in the target interrupt handler,
> will this check permanently return -EBUSY for all subsequent controller
> transfers?
>
Consequently the readl(I2CS_ISR) interlock at the top of the controller xfer path
does not see a stale INACTIVE_TO, so it does not return -EBUSY for subsequent transfers.
This check itself is intentional and correct: controller and target share the same buffer
registers, so we must confirm no unserviced target interrupt is pending before starting
a controller transfer.
> --
> Sashiko AI review ·
> https://sashiko.dev/#/patchset/20260611-upstream_i2c-v32-0-b66eba921d01
> @aspeedtech.com?part=5
^ permalink raw reply
* Re: [PATCH v7 6/6] i2c: qcom-geni: Enable I2C on SA8255p Qualcomm platforms
From: sashiko-bot @ 2026-06-17 5:06 UTC (permalink / raw)
To: Praveen Talari; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260617-enable-i2c-on-sa8255p-v7-6-ad736dbeab57@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Uninitialized `gi2c->clk_fld` pointer triggers a deterministic NULL pointer dereference in `geni_i2c_gpi_xfer` for the SA8255p platform.
- [Medium] An early error return from the new `set_rate()` callback in `geni_i2c_xfer()` irreversibly leaks a PM runtime usage counter.
- [Low] The newly added SA8255p descriptor references undeclared functions `geni_se_domain_attach` and `geni_se_set_perf_opp`, causing a hard build failure.
--
commit c33b9259904d2d29eb39c245dff8c21c943e3fb1
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>
i2c: qcom-geni: Enable I2C on SA8255p Qualcomm platforms
The Qualcomm automotive SA8255p SoC relies on firmware to configure platform
resources. This adds support for SA8255p by using SCMI power and performance
protocols to manage resources like clocks and interconnects.
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 92cccb1cce698..96dbf04138bee 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
[ ... ]
> @@ -919,7 +925,9 @@ static int geni_i2c_xfer(struct i2c_adapter *adap,
> return ret;
> }
>
> - qcom_geni_i2c_conf(gi2c);
> + ret = gi2c->dev_data->set_rate(&gi2c->se, gi2c->clk_freq_out);
> + if (ret)
> + return ret;
[Severity: Medium]
Does this early return leak the PM runtime reference?
Earlier in geni_i2c_xfer(), the driver increments the PM usage counter via
pm_runtime_get_sync(gi2c->se.dev). If set_rate() returns an error, this
returns immediately without calling pm_runtime_put_autosuspend() to release
the reference.
[ ... ]
> @@ -1220,11 +1233,25 @@ static const struct dev_pm_ops geni_i2c_pm_ops = {
> NULL)
> };
[ ... ]
> +static const struct geni_i2c_desc sa8255p_geni_i2c = {
> + .resources_init = geni_se_domain_attach,
> + .set_rate = geni_se_set_perf_opp,
> +};
[Severity: Low]
Are the functions geni_se_domain_attach and geni_se_set_perf_opp missing
declarations? They do not appear to be defined anywhere or declared in the
included headers, which causes a build failure.
[Severity: High]
Does bypassing geni_i2c_resources_init() here lead to a NULL pointer
dereference in GPI mode?
For the qcom,sa8255p-geni-i2c compatible, resources_init is set to
geni_se_domain_attach instead of geni_i2c_resources_init. This means
geni_i2c_clk_map_idx() is never called to initialize gi2c->clk_fld.
Since no_dma_support is false for SA8255p, if the hardware register indicates
the FIFO is disabled, the driver will fall back to GPI DMA mode.
In geni_i2c_gpi_xfer(), the uninitialized gi2c->clk_fld pointer is
unconditionally dereferenced:
const struct geni_i2c_clk_fld *itr = gi2c->clk_fld;
...
peripheral.cycle_count = itr->t_cycle_cnt;
Could this cause a panic during transfer if GPI mode is used?
>
> #ifdef CONFIG_ACPI
> @@ -1239,6 +1266,7 @@ MODULE_DEVICE_TABLE(acpi, geni_i2c_acpi_match);
> static const struct of_device_id geni_i2c_dt_match[] = {
> { .compatible = "qcom,geni-i2c", .data = &geni_i2c },
> { .compatible = "qcom,geni-i2c-master-hub", .data = &i2c_master_hub },
> + { .compatible = "qcom,sa8255p-geni-i2c", .data = &sa8255p_geni_i2c },
> {}
> };
> MODULE_DEVICE_TABLE(of, geni_i2c_dt_match);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-enable-i2c-on-sa8255p-v7-0-ad736dbeab57@oss.qualcomm.com?part=6
^ permalink raw reply
* Re: [PATCH v7 4/6] i2c: qcom-geni: Use resources helper APIs in runtime PM functions
From: sashiko-bot @ 2026-06-17 5:06 UTC (permalink / raw)
To: Praveen Talari; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260617-enable-i2c-on-sa8255p-v7-4-ad736dbeab57@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [Low] The patch calls undefined helper functions `geni_se_resources_activate`, `geni_se_resources_deactivate`, and `geni_se_resources_init`, leading to implicit function declarations and link failures.
Pre-existing issues:
- [Critical] Stale `gi2c->dma_buf` pointer causes use-after-free and double-free on DMA mapping failure.
- [High] Pending asynchronous runtime suspend is cancelled by `pm_runtime_disable()`, permanently leaking hardware resources (clocks, interconnects).
- [High] Memory leak of multi-descriptor DMA arrays on transfer error.
- [High] The system suspend callback executes a sleeping function while interrupts are disabled, causing a BUG.
--
commit 495d087d0c5bbbc5ad2b355b642cfdc5ae9439b6
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>
i2c: qcom-geni: Use resources helper APIs in runtime PM functions
To manage GENI serial engine resources during runtime power management,
drivers currently need to call functions for ICC, clock, and
SE resource operations in both suspend and resume paths, resulting in
code duplication across drivers.
The new geni_se_resources_activate() and geni_se_resources_deactivate()
helper APIs addresses this issue by providing a streamlined method to
enable or disable all resources based, thereby eliminating redundancy
across drivers.
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index a7672b6c2bc07..8f8b74c4e88a4 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -1172,15 +1172,14 @@ static int __maybe_unused geni_i2c_runtime_suspend(struct device *dev)
> struct geni_i2c_dev *gi2c = dev_get_drvdata(dev);
>
> disable_irq(gi2c->irq);
[Severity: High]
This isn't a bug introduced by this patch, but does calling disable_irq() here
cause a BUG when suspending the system?
Looking at the suspend paths, geni_i2c_suspend_noirq() runs in the noirq
phase where local interrupts are globally disabled. It calls
pm_runtime_force_suspend(), which invokes this geni_i2c_runtime_suspend()
function.
Calling disable_irq() explicitly invokes might_sleep(), which triggers a
"BUG: sleeping function called from invalid context" when called with
interrupts disabled.
> - ret = geni_se_resources_off(&gi2c->se);
> + ret = geni_se_resources_deactivate(&gi2c->se);
[Severity: Low]
Are the implementations for geni_se_resources_deactivate() and
geni_se_resources_activate() missing from this patch series?
These functions appear to be completely missing from the kernel tree at this
revision. Does this lead to implicit function declarations and build failures?
[ ... ]
[Severity: High]
This isn't a bug introduced by this patch, but does the error path in
geni_i2c_probe() and the driver removal path in geni_i2c_remove()
permanently leak hardware resources?
In geni_i2c_init(), an asynchronous pm_runtime_put() is called. If an error
occurs later in geni_i2c_probe(), or during geni_i2c_remove(), the code
immediately calls pm_runtime_disable():
drivers/i2c/busses/i2c-qcom-geni.c:geni_i2c_probe() {
...
pm_runtime_disable(gi2c->se.dev);
...
}
Calling pm_runtime_disable() invokes __pm_runtime_barrier(), which cancels
any pending asynchronous suspend requests instead of waiting for them to
complete. Since the suspend is cancelled and the device remains in the
RPM_ACTIVE state after the driver unbinds, do its clocks and power domains
leak indefinitely?
[Severity: Critical]
This is a pre-existing issue, but can a stale dma_buf pointer cause a
use-after-free and double-free if DMA mapping fails?
If geni_se_rx_dma_prep() fails in geni_i2c_rx_one_msg(), the local dma_buf
is freed and set to NULL, but the device struct's gi2c->dma_buf retains the
stale pointer from a previous successful transfer:
drivers/i2c/busses/i2c-qcom-geni.c:geni_i2c_rx_one_msg() {
...
if (dma_buf && geni_se_rx_dma_prep(se, dma_buf, len, &rx_dma)) {
geni_se_select_mode(se, GENI_SE_FIFO);
i2c_put_dma_safe_msg_buf(dma_buf, msg, false);
dma_buf = NULL;
} else {
...
}
During cleanup, the stale pointer is evaluated as true:
drivers/i2c/busses/i2c-qcom-geni.c:geni_i2c_rx_msg_cleanup() {
...
if (gi2c->dma_buf) {
if (gi2c->err)
geni_i2c_rx_fsm_rst(gi2c);
geni_se_rx_dma_unprep(&gi2c->se, gi2c->dma_addr, gi2c->xfer_len);
i2c_put_dma_safe_msg_buf(gi2c->dma_buf, cur, !gi2c->err);
}
...
}
Does this unmap the stale DMA address, copy data from the freed bounce buffer,
and then double-free the kalloc'd bounce buffer?
[Severity: High]
This is a pre-existing issue, but is there a memory leak of the
multi-descriptor DMA arrays when a transfer error occurs?
In geni_i2c_gpi_multi_desc_unmap(), the arrays are only freed if
unmap_msg_cnt reaches gi2c->num_msgs - 1:
drivers/i2c/busses/i2c-qcom-geni.c:geni_i2c_gpi_multi_desc_unmap() {
...
if (tx_multi_xfer->unmap_msg_cnt == gi2c->num_msgs - 1) {
kfree(tx_multi_xfer->dma_buf);
kfree(tx_multi_xfer->dma_addr);
break;
}
...
}
If an error occurs during the multi-message transfer, msg_xfer_cnt is less
than num_msgs, so the loop terminates early. Does this bypass the kfree()
calls and leak both tracking arrays, potentially allowing an attacker to
cause an Out-Of-Memory condition?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-enable-i2c-on-sa8255p-v7-0-ad736dbeab57@oss.qualcomm.com?part=4
^ permalink raw reply
* Re: [PATCH v7 3/6] i2c: qcom-geni: Move resource initialization to separate function
From: sashiko-bot @ 2026-06-17 5:05 UTC (permalink / raw)
To: Praveen Talari; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260617-enable-i2c-on-sa8255p-v7-3-ad736dbeab57@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Critical] Build failure due to undefined GENI SE resource initialization and PM functions
- [High] GPI DMA channels are permanently leaked on probe failure.
- [High] Asynchronous pm_runtime_put() is canceled by pm_runtime_disable() on probe error paths, leaking power.
Pre-existing issues:
- [High] Missing synchronous suspend before disabling runtime PM during device removal.
--
commit 3d26b1870da68dc53ac527a03242c49f148d835f
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>
i2c: qcom-geni: Move resource initialization to separate function
Refactor the resource initialization in geni_i2c_probe() by introducing
a new geni_i2c_resources_init() function and utilizing the common
geni_se_resources_init() framework and clock frequency mapping, making the
probe function cleaner.
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 3ee0be228d7c1..a7672b6c2bc07 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
[ ... ]
> @@ -1058,6 +1055,23 @@ static int geni_i2c_init(struct geni_i2c_dev *gi2c)
> return ret;
> }
>
> +static int geni_i2c_resources_init(struct geni_i2c_dev *gi2c)
> +{
> + int ret;
> +
> + ret = geni_se_resources_init(&gi2c->se);
[Severity: Critical]
Are we sure this function is available in the current tree? It appears
geni_se_resources_init() is undefined, which might cause a build failure.
> + if (ret)
> + return ret;
> +
> + ret = geni_i2c_clk_map_idx(gi2c);
> + if (ret)
> + return dev_err_probe(gi2c->se.dev, ret, "Invalid clk frequency %d Hz\n",
> + gi2c->clk_freq_out);
> +
> + return geni_icc_set_bw_ab(&gi2c->se, GENI_DEFAULT_BW, GENI_DEFAULT_BW,
> + Bps_to_icc(gi2c->clk_freq_out));
[Severity: Critical]
Is geni_icc_set_bw_ab() defined? Similar to the call above, it looks like this
is an unresolved symbol that could break the build.
> +}
> +
> static int geni_i2c_probe(struct platform_device *pdev)
> {
> struct geni_i2c_dev *gi2c;
[ ... ]
> @@ -1101,16 +1105,15 @@ static int geni_i2c_probe(struct platform_device *pdev)
> if (gi2c->irq < 0)
> return gi2c->irq;
>
> - ret = geni_i2c_clk_map_idx(gi2c);
> - if (ret)
> - return dev_err_probe(dev, ret, "Invalid clk frequency %d Hz\n",
> - gi2c->clk_freq_out);
> -
> gi2c->adap.algo = &geni_i2c_algo;
> init_completion(&gi2c->done);
> spin_lock_init(&gi2c->lock);
> platform_set_drvdata(pdev, gi2c);
>
> + ret = geni_i2c_resources_init(gi2c);
> + if (ret)
> + return ret;
> +
[Severity: High]
Does this probe function properly clean up resources if it fails later on?
If the subsequent call to i2c_add_adapter() fails later in geni_i2c_probe(),
the function returns without freeing the DMA channels that were allocated
during geni_i2c_init() via setup_gpi_dma(). Could this permanently leak the
GPI DMA channels?
[Severity: High]
Is there a risk of leaking power during probe error handling?
If geni_i2c_init() or i2c_add_adapter() fails, geni_i2c_probe() calls
pm_runtime_disable() immediately. However, geni_i2c_init() queues an async
pm_runtime_put() in its own error path. Since pm_runtime_disable() cancels
any pending async suspend work, does this leave the hardware indefinitely
powered on in the RPM_ACTIVE state?
[ ... ]
> @@ -1260,8 +1246,6 @@ static const struct dev_pm_ops geni_i2c_pm_ops = {
> };
>
> static const struct geni_i2c_desc i2c_master_hub = {
> - .has_core_clk = true,
> - .icc_ddr = NULL,
> .no_dma_support = true,
> .tx_fifo_depth = 16,
> };
[Severity: Critical]
Does this patch series introduce undefined power management functions here?
It appears the i2c_master_hub struct is assigned geni_se_resources_activate
and geni_se_resources_deactivate for its power management fields, but these
functions do not seem to be defined anywhere in the kernel tree.
[Severity: High]
This is a pre-existing issue, but does geni_i2c_remove() properly quiesce
the device before tearing down power management?
The geni_i2c_remove() function calls pm_runtime_disable() without a prior
synchronization barrier. If an asynchronous suspend is pending from a
recently completed I2C transaction, the immediate pm_runtime_disable() could
cancel it. Should a call to pm_runtime_put_sync() or
pm_runtime_force_suspend() be added to ensure it is fully suspended?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-enable-i2c-on-sa8255p-v7-0-ad736dbeab57@oss.qualcomm.com?part=3
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox