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
