Re: [PATCH V2 2/3] i3c: master: svc: switch to bulk clk API for flexible clock support

[Frank Li <Frank.li@nxp.com>]

On Mon, Apr 21, 2025 at 07:15:12PM +0800, carlos.song@nxp.com wrote:
> From: Carlos Song <carlos.song@nxp.com>
>
> Use the clk_bulk API to handle clocks, so the code can support different
> numbers of clocks more easily. Make the code cleaner and more flexible.
>
> No change in functionality.
>
> Signed-off-by: Carlos Song <carlos.song@nxp.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
> Change for V2:
> - fix warning from bot test. Use -EINVA instead of uninitialized ret in
>   dev_err_probe
> - use master->fclk = master->clks[i].clk instead of devm_clk_get
>   (dev, "fast_clk");
> ---
>  drivers/i3c/master/svc-i3c-master.c | 76 ++++++++++-------------------
>  1 file changed, 26 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index 85e16de208d3..d8cb0b9a2597 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -203,9 +203,9 @@ struct svc_i3c_drvdata {
>   * @hj_work: Hot-join work
>   * @ibi_work: IBI work
>   * @irq: Main interrupt
> - * @pclk: System clock
> + * @num_clks: I3C clock number
>   * @fclk: Fast clock (bus)
> - * @sclk: Slow clock (other events)
> + * @clks: I3C clock array
>   * @xferqueue: Transfer queue structure
>   * @xferqueue.list: List member
>   * @xferqueue.cur: Current ongoing transfer
> @@ -231,9 +231,9 @@ struct svc_i3c_master {
>  	struct work_struct hj_work;
>  	struct work_struct ibi_work;
>  	int irq;
> -	struct clk *pclk;
> +	int num_clks;
>  	struct clk *fclk;
> -	struct clk *sclk;
> +	struct clk_bulk_data *clks;
>  	struct {
>  		struct list_head list;
>  		struct svc_i3c_xfer *cur;
> @@ -1875,42 +1875,11 @@ static const struct i3c_master_controller_ops svc_i3c_master_ops = {
>  	.set_speed = svc_i3c_master_set_speed,
>  };
>
> -static int svc_i3c_master_prepare_clks(struct svc_i3c_master *master)
> -{
> -	int ret = 0;
> -
> -	ret = clk_prepare_enable(master->pclk);
> -	if (ret)
> -		return ret;
> -
> -	ret = clk_prepare_enable(master->fclk);
> -	if (ret) {
> -		clk_disable_unprepare(master->pclk);
> -		return ret;
> -	}
> -
> -	ret = clk_prepare_enable(master->sclk);
> -	if (ret) {
> -		clk_disable_unprepare(master->pclk);
> -		clk_disable_unprepare(master->fclk);
> -		return ret;
> -	}
> -
> -	return 0;
> -}
> -
> -static void svc_i3c_master_unprepare_clks(struct svc_i3c_master *master)
> -{
> -	clk_disable_unprepare(master->pclk);
> -	clk_disable_unprepare(master->fclk);
> -	clk_disable_unprepare(master->sclk);
> -}
> -
>  static int svc_i3c_master_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct svc_i3c_master *master;
> -	int ret;
> +	int ret, i;
>
>  	master = devm_kzalloc(dev, sizeof(*master), GFP_KERNEL);
>  	if (!master)
> @@ -1924,27 +1893,31 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
>  	if (IS_ERR(master->regs))
>  		return PTR_ERR(master->regs);
>
> -	master->pclk = devm_clk_get(dev, "pclk");
> -	if (IS_ERR(master->pclk))
> -		return PTR_ERR(master->pclk);
> +	master->num_clks = devm_clk_bulk_get_all(dev, &master->clks);
> +	if (master->num_clks < 0)
> +		return dev_err_probe(dev, -EINVAL, "can't get I3C clocks\n");
> +
> +	for (i = 0; i < master->num_clks; i++) {
> +		if (!strcmp(master->clks[i].id, "fast_clk"))
> +			break;
> +	}
> +
> +	if (i == master->num_clks)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "can't get I3C peripheral clock\n");
>
> -	master->fclk = devm_clk_get(dev, "fast_clk");
> +	master->fclk = master->clks[i].clk;
>  	if (IS_ERR(master->fclk))
>  		return PTR_ERR(master->fclk);
>
> -	master->sclk = devm_clk_get(dev, "slow_clk");
> -	if (IS_ERR(master->sclk))
> -		return PTR_ERR(master->sclk);
> -
>  	master->irq = platform_get_irq(pdev, 0);
>  	if (master->irq < 0)
>  		return master->irq;
>
>  	master->dev = dev;
> -
> -	ret = svc_i3c_master_prepare_clks(master);
> +	ret = clk_bulk_prepare_enable(master->num_clks, master->clks);
>  	if (ret)
> -		return ret;
> +		return dev_err_probe(dev, ret, "can't enable I3C clocks\n");
>
>  	INIT_WORK(&master->hj_work, svc_i3c_master_hj_work);
>  	INIT_WORK(&master->ibi_work, svc_i3c_master_ibi_work);
> @@ -1998,7 +1971,7 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
>  	pm_runtime_set_suspended(&pdev->dev);
>
>  err_disable_clks:
> -	svc_i3c_master_unprepare_clks(master);
> +	clk_bulk_disable_unprepare(master->num_clks, master->clks);
>
>  	return ret;
>  }
> @@ -2036,7 +2009,7 @@ static int __maybe_unused svc_i3c_runtime_suspend(struct device *dev)
>  	struct svc_i3c_master *master = dev_get_drvdata(dev);
>
>  	svc_i3c_save_regs(master);
> -	svc_i3c_master_unprepare_clks(master);
> +	clk_bulk_disable_unprepare(master->num_clks, master->clks);
>  	pinctrl_pm_select_sleep_state(dev);
>
>  	return 0;
> @@ -2045,9 +2018,12 @@ static int __maybe_unused svc_i3c_runtime_suspend(struct device *dev)
>  static int __maybe_unused svc_i3c_runtime_resume(struct device *dev)
>  {
>  	struct svc_i3c_master *master = dev_get_drvdata(dev);
> +	int ret;
>
>  	pinctrl_pm_select_default_state(dev);
> -	svc_i3c_master_prepare_clks(master);
> +	ret = clk_bulk_prepare_enable(master->num_clks, master->clks);
> +	if (ret)
> +		return ret;
>
>  	svc_i3c_restore_regs(master);
>
> --
> 2.34.1
>