Re: [PATCH] drivers: char: omap-uart: add "clock-hz" option

[Julien Grall <julien@xxxxxxx>]

Hi,

On 06/08/2024 11:35, Amneesh Singh wrote:
> On 10:56-20240806, Julien Grall wrote:
> > On 06/08/2024 10:50, Amneesh Singh wrote:
> > > On 10:16-20240806, Julien Grall wrote:
> > > > Hi,
> > > >
> > > > On 19/07/2024 12:33, Amneesh Singh wrote:
> > > > > Quite a few TI K3 devices do not have clock-frequency specified in their
> > > > > respective UART nodes.
> > > > Can you outline why fixing the device-tree is not solution?
> > > Because other frequencies, say 96MHz or 192 MHz are also valid inputs.
> > Are you saying this is configurable by the user? Or do you mean the
> > firmware can configure it?
> u-boot or some other bootloader are free to configure it. And usually,
> linux's clock driver will pick it up using clk_get_rate (if not
> specified in the DT), I think. Now, in case we add the frequency to the
> DT, it may not match with the actual frequency configured before Xen
> initialisation. Since, there is no equivalent to clk_get_rate under Xen,
> and the fact I'm using imagebuilder, I found it easier to pass the
> frequency the way I did.
Thanks for the explanation. I haven't looked in details, but how 
difficult would it be to implement clk_get_rate() (at least just enough 
to work for the UART) in Xen?
> > > > > However hard-coding the frequency is not a
> > > >    > solution as the function clock input can differ between SoCs.
> > > >
> > > > Can you provide some details how Linux is handling those SoCs?
> > > Yes, like omap-uart under xen, the 8250_omap driver also parses the DT,
> > > but unlike omap-uart, falls back to clk_get_rate() and if the value is
> > > still zero, it defaults to 48MHz.
> > Thanks for the information. Then my next question is why Linux can get
> > away with a default value and not Xen?
> Sure why not? I guess, we can use a default value if everything fails
> but there should still be a way for the user to specify the frequency.
I think I am still missing something. Why would Xen allow the user to 
specify the clock speed if Linux doesn't? At least to me, it is more 
likely that a user would want to boot Linux on your HW than Xen...
> Of course, we can instead just force the user to change the DT slightly
> by just specifying the frequency. However, I feel it is easier to add it
> here, especially when there's already a method to pass these options via
> the command-line in place.
> I believe, this is the best we can do with this.

The problem is the command line is OS/hypervisor specific. But the 
problem you describe doesn't seem Xen specific. This is where the DT is 
handy because you describe once and it can be used everywhere.
Overall, at the moment, I don't think the command line option is the 
right approach. If I am not mistaken, it would make Xen less 
user-friendly (compare to Linux) to boot on your HW as the user would 
need to specify the clockspeed on the command line. I think we should 
investigate other approaches such as implementing partially 
clk_get_rate() (if this is how Linux manage to retrieve the clock speed 
without any command line option).
Cheers,

--
Julien Grall