Re: [PATCH 1/5] clk: zynqmp: fix check for fractional clock

Quoting Michael Tretter (2019-03-14 01:38:36)

On Wed, 13 Mar 2019 09:24:04 -0700, Stephen Boyd wrote:

quoted

Quoting Michael Tretter (2019-03-12 10:25:46)

quoted

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. 
  

What is ATF doing with these flags? Hopefully ATF and the Linux kernel
aren't using the same numberspace to describe these things. For example,
I would be concerned if ATF was looking at the CLK_SET_RATE_PARENT flag
and passing that value from firmware to the kernel, blindly assuming
that the kernel wouldn't change those numbers to be something else.
Obviously that type of kernel change would be invasive but it's not an
ABI that we've ever published so we're free to do these sorts of things.

You mean that the ATF defines macros like

        #define CLK_SET_RATE_GATE       BIT(0) /* must be gated across rate change */
        #define CLK_SET_PARENT_GATE     BIT(1) /* must be gated across re-parent */
        #define CLK_SET_RATE_PARENT     BIT(2) /* propagate rate change up one level */

in plat/xilinx/zynqmp/pm_service/pm_api_clock.h, sets the flags in the
response to the Linux driver, and the Linux driver copies the flags
that it got from the ATF to clk_hw::core::flags like

        init.flags = nodes->flag;

where nodes is the response from the ATF? That's exactly what is happening.

Yes, that looks like a strong coupling between firmware and OS
implemented in a non-obvious way.

So instead of only translating CLK_FRAC, the driver should actually
translate all flags in the ATF response to proper clk framework flags
instead of blindly copying them, right?

Sure. The CLK_FRAC will have to be filtered out anyway it sounds like.


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