linux-kernel - Re: [PATCH][next] soc: xilinx: vcu: remove deadcode on null divider check

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]

Message-ID: <20210211073906.GC30300@pengutronix.de>
Date:   Thu, 11 Feb 2021 08:39:06 +0100
From:   Michael Tretter <m.tretter@...gutronix.de>
To:     Stephen Boyd <sboyd@...nel.org>
Cc:     Colin King <colin.king@...onical.com>,
        Michael Turquette <mturquette@...libre.com>,
        Michal Simek <michal.simek@...inx.com>,
        linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org,
        kernel-janitors@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH][next] soc: xilinx: vcu: remove deadcode on null divider
 check

On Wed, 10 Feb 2021 19:28:18 -0800, Stephen Boyd wrote:
> Quoting Colin King (2021-02-10 10:49:38)
> > From: Colin Ian King <colin.king@...onical.com>
> > 
> > The pointer 'divider' has previously been null checked followed by
> > a return, hence the subsequent null check is redundant deadcode
> > that can be removed.  Clean up the code and remove it.
> > 
> > Fixes: 9c789deea206 ("soc: xilinx: vcu: implement clock provider for output clocks")
> > Signed-off-by: Colin Ian King <colin.king@...onical.com>
> > ---
> >  drivers/clk/xilinx/xlnx_vcu.c | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/drivers/clk/xilinx/xlnx_vcu.c b/drivers/clk/xilinx/xlnx_vcu.c
> > index d66b1315114e..607936d7a413 100644
> > --- a/drivers/clk/xilinx/xlnx_vcu.c
> > +++ b/drivers/clk/xilinx/xlnx_vcu.c
> > @@ -512,9 +512,6 @@ static void xvcu_clk_hw_unregister_leaf(struct clk_hw *hw)
> >  
> >         mux = clk_hw_get_parent(divider);
> >         clk_hw_unregister_mux(mux);
> > -       if (!divider)
> > -               return;
> > -
> 
> This code is pretty confusing. Waiting for m.tretter@...gutronix.de to
> reply

Can you elaborate what you find confusing about this code. I would gladly try
to clarify and improve the code.

What happens here is that the driver registers a mux -> divider -> gate chain
for each output clock, but only stores the gate clock. When unregistering the
clocks, the driver starts at the gate and walks up to the mux while
unregistering the clocks.

Michael

> 
> >         clk_hw_unregister_divider(divider);
> >  }
> >
>