On 11/02/2021 19:05, Stephen Boyd wrote:

quoted

Quoting Michael Tretter (2021-02-10 23:39:06)

quoted

On Wed, 10 Feb 2021 19:28:18 -0800, Stephen Boyd wrote:

quoted

Quoting Colin King (2021-02-10 10:49:38)

quoted

From: Colin Ian King <redacted>

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 <redacted>
---
 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@pengutronix.de to
reply

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

The fact that pointers are being checked and then bailing out of the
function early, vs. doing something if the pointer is non-NULL.

quoted

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.

OK, so I think I understand this better, should the order of
unregisteration be as follows:

diff --git a/drivers/clk/xilinx/xlnx_vcu.c b/drivers/clk/xilinx/xlnx_vcu.c
index d66b1315114e..66bac8421460 100644
--- a/drivers/clk/xilinx/xlnx_vcu.c
+++ b/drivers/clk/xilinx/xlnx_vcu.c

@@ -511,11 +511,11 @@ static void xvcu_clk_hw_unregister_leaf(struct

clk_hw *hw)
                return;

        mux = clk_hw_get_parent(divider);
-       clk_hw_unregister_mux(mux);
-       if (!divider)
+       clk_hw_unregister_mux(divider);