On Tue, 12 Mar 2019 09:49:21 -0700, Stephen Boyd wrote:

Quoting Michael Tretter (2019-03-12 04:00:12)

quoted

CLK_FRAC is not set in the divider->flags, but in the hw->flags.

The firmware sets CLK_FRAC for fractional clocks in the clkflag field.
When registering the devider, these clkflags are copied to hw->flags.

Moreover, divider->flags field is a u8 type, but CLK_FRAG is BIT(13). So
this check would never work.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/clk/zynqmp/divider.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/zynqmp/divider.c b/drivers/clk/zynqmp/divider.c
index a371c66e72ef..fc70950c1e24 100644
--- a/drivers/clk/zynqmp/divider.c
+++ b/drivers/clk/zynqmp/divider.c

@@ -117,7 +117,7 @@ static long zynqmp_clk_divider_round_rate(struct clk_hw *hw,
        bestdiv = zynqmp_divider_get_val(*prate, rate);
 
        if ((clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) &&
-           (divider->flags & CLK_FRAC))
+           (clk_hw_get_flags(hw) & CLK_FRAC))  

CLK_FRAC shouldn't be set in the struct clk_hw::core::flags field. It's
not a clk framework flag so it shouldn't go there. Please fix the user
of this flag to place the CLK_FRAC flag somewhere else. Even adding it
into divider::flags is not a good idea because that numberspace is for
dividers, and this flag seems to be zynqmp driver specific, so maybe
just add a bool to the zynqmp_clk_divider?

Thanks. The driver sets the clk_hw::core::flags based on a response
from the ATF and this response includes this flag with other clk
frameworks flags. I can test for the flag when registering the clock
and set another flag or a bool for the zynqmp_clk_divider and will do
so in v2.

However, this merely sounds like a workaround for an issue in the ATF,
which should not define and use this flag in the first place. 

Michael

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel