Re: [PATCH v6 1/9] mtk-mdp: propagate errors from clock_on

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 09.08.21 05:23, Eizan Miyamoto wrote:
Hi Dafna, thank you very much for spending time to review the patch,
your time spent is very much appreciated.

On Thu, Aug 5, 2021 at 4:06 PM Dafna Hirschfeld
<dafna.hirschfeld@xxxxxxxxxxxxx> wrote:
+err_clk_prepare_enable:
+     for (--i; i >= 0; i--) {
+             if (IS_ERR(comp->clk[i]))
+                     continue;
+             clk_disable_unprepare(comp->clk[i]);
       }
+
+     return status;

There is an API function clk_bulk_prepare_enable to prepare and enable an array of clks
so you can just use it.

As per Enric's suggestion earlier in this email thread, are you OK
with me making this change in a follow-up patch, particularly since
the logic as it is was preserved from previous functionality?

sure,
I just give suggestions.
A follow-up patch would be nice.


-static void mtk_mdp_clock_on(struct mtk_mdp_dev *mdp)
+static int mtk_mdp_clock_on(struct mtk_mdp_dev *mdp)
   {
-     struct device *dev = &mdp->pdev->dev;
       struct mtk_mdp_comp *comp_node;
+     int status;
+     struct device *dev = &mdp->pdev->dev;
+     int err;

-     list_for_each_entry(comp_node, &mdp->comp_list, node)
-             mtk_mdp_comp_clock_on(dev, comp_node);
+     list_for_each_entry(comp_node, &mdp->comp_list, node) {
+             err = mtk_mdp_comp_clock_on(dev, comp_node);
+             if (err) {
+                     status = err;

You can get rid of the new var 'status' and just return ret in case of error

This seems like a nit (please let me know if you disagree), and it's
also cleaned up in a follow-on patch in the series ("don't
pm_run_time_get/put for master comp in clock_on"). Is making the
change you are suggesting here something that should require uploading
a new series version?

Hi, no, I am fine with it. No need for new version.


Thanks,
Dafna


Eizan




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux