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
