On 2022-03-21 06:50, Heiko Thiery wrote:
Hi Angus,
[snip]
So I'm not sure if the ipg clock is the right one for the boards that has different clock for ipg and per.
So I only looked at imx6qdl.dtsi where the clocks are different
clocks = <&clks IMX6QDL_CLK_UART_IPG>, <&clks IMX6QDL_CLK_UART_SERIAL>; clock-names = "ipg", "per";
And from that file it looks like the per clock would be the correct one.
Yes, 'per' seems to be the right one.
Should the clock be looked up by id instead of by name and then have a different code path for each imx board type ?
But how to get the right clk id? The id's for all the implementations are different. Or not?
Yeah you're correct that won't work.
}/* as fallback we try to get the clk rate that way */if (rate == 0)rate = imx_get_uartclk();Would it be better to re-write imx_get_uartclk so that both the getting and setting of clocks was correct ?
I do not understand what you mean with that.
There are other places in the code that imx_get_uartclk gets called. If an index was added to imx_get_uartclk(int index) then you wouldn't need the code above in the mxc_serial_setbrg function. That would also make all of the places where imx_get_uartclk gets called return the correct value.
By index do you mean the clk id?
No I was thinking number of the device uart[0-3].
Thinking about it some more it's probably not useful as you already have the udevice pointer. I was just trying to think of ways to reduce the amount of code change for the older SOCs. Using the 'per' clock is a better solution.
It might make sense to create a new function imx7_get_uartclk that gets called on newer SOCs so that the imx6 and earlier code doesn't need to get changed.
At what point should this be called instead of the imx_get_uartclk() function?
I was thinking a CONFIG_IS_ENBLED switch between the old version and the new one based on SOC.
Angus