Re: [PATCH v2] mailbox: sprd: Use devm_clk_get_enabled() helpers

From: Huan Yang
Date: Wed Aug 21 2024 - 03:09:12 EST



在 2024/8/21 15:00, Christophe JAILLET 写道:
Le 21/08/2024 à 03:39, Huan Yang a écrit :
The devm_clk_get_enabled() helpers:
      - call devm_clk_get()
      - call clk_prepare_enable() and register what is needed in order to
       call clk_disable_unprepare() when needed, as a managed resource.

This simplifies the code and avoids the calls to clk_disable_unprepare().

Due to clk only used in probe, not in suspend\resume, this pointer can
remove from sprd_mbox_priv to save a little memory.

Signed-off-by: Huan Yang <link@xxxxxxxx>
Suggested-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>


Nitpick: no need to add this S-b. I just made a comment when looking at your patch in order to improve it. I'm not the one that suggested the initial change. All merit is yours.
Your suggestion is helpfull, help this patch be better.

Also, I think that, if used, it should be above your Signed-off.
Hmmm, it's neccessary? If so, I'd like to update it.


Reviewed-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>

---
v1 -> v2: remove clk pointer from sprd_mbox_priv

  drivers/mailbox/sprd-mailbox.c | 25 ++++---------------------
  1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/mailbox/sprd-mailbox.c b/drivers/mailbox/sprd-mailbox.c
index 9ae57de77d4d..ee8539dfcef5 100644
--- a/drivers/mailbox/sprd-mailbox.c
+++ b/drivers/mailbox/sprd-mailbox.c
@@ -62,7 +62,6 @@ struct sprd_mbox_priv {
      void __iomem        *outbox_base;
      /*  Base register address for supplementary outbox */
      void __iomem        *supp_base;
-    struct clk        *clk;
      u32            outbox_fifo_depth;
        struct mutex        lock;
@@ -291,19 +290,13 @@ static const struct mbox_chan_ops sprd_mbox_ops = {
      .shutdown    = sprd_mbox_shutdown,
  };
  -static void sprd_mbox_disable(void *data)
-{
-    struct sprd_mbox_priv *priv = data;
-
-    clk_disable_unprepare(priv->clk);
-}
-
  static int sprd_mbox_probe(struct platform_device *pdev)
  {
      struct device *dev = &pdev->dev;
      struct sprd_mbox_priv *priv;
      int ret, inbox_irq, outbox_irq, supp_irq;
      unsigned long id, supp;
+    struct clk *clk;
        priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
      if (!priv)
@@ -331,20 +324,10 @@ static int sprd_mbox_probe(struct platform_device *pdev)
      if (IS_ERR(priv->outbox_base))
          return PTR_ERR(priv->outbox_base);
  -    priv->clk = devm_clk_get(dev, "enable");
-    if (IS_ERR(priv->clk)) {
+    clk = devm_clk_get_enabled(dev, "enable");
+    if (IS_ERR(clk)) {
          dev_err(dev, "failed to get mailbox clock\n");
-        return PTR_ERR(priv->clk);
-    }
-
-    ret = clk_prepare_enable(priv->clk);
-    if (ret)
-        return ret;
-
-    ret = devm_add_action_or_reset(dev, sprd_mbox_disable, priv);
-    if (ret) {
-        dev_err(dev, "failed to add mailbox disable action\n");
-        return ret;
+        return PTR_ERR(clk);
      }
        inbox_irq = platform_get_irq_byname(pdev, "inbox");