Re: [PATCH v2 4/5] clk: cix: add sky1 audss clock controller

[Philipp Zabel]

On Fr, 2026-06-05 at 11:22 +0800, joakim.zhang@xxxxxxxxxxx wrote:
> From: Joakim Zhang <joakim.zhang@xxxxxxxxxxx>
> 
> 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. Six
> SoC-level audio reference clocks are brought up as inputs to the tree.
> 
> Signed-off-by: Joakim Zhang <joakim.zhang@xxxxxxxxxxx>
> ---
>  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 | 1129 ++++++++++++++++++++++++++++++
>  5 files changed, 1150 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/cix/clk-sky1-audss.c b/drivers/clk/cix/clk-sky1-audss.c
> new file mode 100644
> index 000000000000..899452d5ed14
> --- /dev/null
> +++ b/drivers/clk/cix/clk-sky1-audss.c
> @@ -0,0 +1,1129 @@
[...]
> +/* 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 reset_control *rst_noc;
> +	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, AUDSS_MAX_CLKS),
> +				      GFP_KERNEL);
> +	if (!priv->clk_data)
> +		return -ENOMEM;
> +
> +	priv->clk_data->num = AUDSS_MAX_CLKS;
> +	clk_table = priv->clk_data->hws;
> +
> +	priv->dev = dev;
> +	priv->regmap_cru = regmap_cru;
> +	priv->devtype_data = devtype_data;
> +
> +	ret = sky1_audss_clks_get(priv);
> +	if (ret)
> +		return ret;
> +
> +	rst_noc = devm_reset_control_get(dev, NULL);

Please use devm_reset_control_get_exclusive() directly.

[...]
> +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;
> +	}
> +
> +	reset_control_deassert(priv->rst_noc);

Deasserted on resume but not asserted on suspend, is this on purpose?

regards
Philipp