linux-kernel - Re: [PATCH v6 1/9] mtk-mdp: propagate errors from clock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 9 Aug 2021 13:23:04 +1000
From:   Eizan Miyamoto <eizan@...gle.com>
To:     Dafna Hirschfeld <dafna.hirschfeld@...labora.com>
Cc:     linux-kernel@...r.kernel.org, wenst@...omium.org,
        houlong.wei@...iatek.com, yong.wu@...iatek.com,
        enric.balletbo@...labora.com, devicetree@...r.kernel.org,
        chunkuang.hu@...nel.org,
        Andrew-CT Chen <andrew-ct.chen@...iatek.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Minghsiu Tsai <minghsiu.tsai@...iatek.com>,
        linux-arm-kernel@...ts.infradead.org, linux-media@...r.kernel.org,
        linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v6 1/9] mtk-mdp: propagate errors from clock_on

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@...labora.com> 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?

> > -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?

Eizan