Hi, On 14 August 2015 at 16:51, Stephen Warren <swar...@wwwdotorg.org> wrote: > On 08/14/2015 04:40 PM, Bin Meng wrote: >> >> On Sat, Aug 15, 2015 at 12:59 AM, Simon Glass <s...@chromium.org> wrote: >>> >>> Hi Stephen, >>> >>> On 14 August 2015 at 10:58, Stephen Warren <swar...@wwwdotorg.org> wrote: >>>> >>>> On 08/14/2015 10:50 AM, Simon Glass wrote: >>>>> >>>>> >>>>> Hi Bin, >>>>> >>>>> On 14 August 2015 at 03:18, Bin Meng <bmeng...@gmail.com> wrote: >>>>>> >>>>>> >>>>>> Hi, >>>>>> >>>>>> Currently there are 5 dm serial drivers, all of which are ns16550 >>>>>> compatible drivers. They are: >>>>>> >>>>>> serial_omap.c >>>>>> serial_dw.c >>>>>> serial_tegra.c >>>>>> serial_x86.c >>>>>> serial_ppc.c >>>>>> >>>>>> All these drivers are pretty much similar. I think we can justmerge >>>>>> these into one ns16550 driver. >>>>>> >>>>>> If you think this is necessary, I will send a patch series to do this. >>>>> >>>>> >>>>> >>>>> The tegra one is there because it needs an input clock and Stephen >>>>> didn't want to add this to the device tree binding (the kernel has a >>>>> clock framework which gets around this problem). >>>>> >>>>> After that I followed the same pattern. I would support updating the >>>>> binding to support an input clock. Even with the new clock framework >>>>> in U-Boot it might be painful to fit it into SPL in some cases. >>>> >>>> >>>> >>>> The clock is already in the DT, in both Linux and U-Boot's copy, at >>>> least >>>> for Tegra DTs: >>>> >>>> uarta: serial@0,70006000 { >>>> compatible = "nvidia,tegra124-uart", ... >>>> ... >>>> clocks = <&tegra_car TEGRA124_CLK_UARTA>; >>>> >>> >>> I mean the clock-frequency property. However if there is a plan to >>> implement the clock framework in U-Boot that would be good too. >>> >> >> The clock-frequency is a fixed value on x86 super i/o chipset, and >> fixed on the PCI bus too. But for ARM and PPC, it might get >> dynamically calculated due to different PLL settings. We can implement >> a _weak function like the one in serial_ppc.c get_serial_clock() to >> initialize plat->clock with its return value. The _weak function gets >> clock-frequency from device tree. If there is not, platform codes >> which uses the ns16550 driver should provide the implementation of >> get_serial_clock(). Thoughts? > > > There is no clock-frequency property in DT, at least for the Tegra DT > binding. It looks like some other bindings have it. To obtain the clock > frequency from DT for Tegra, you'd need to parse the clocks property, find > the clock driver associated with the phandle in DT, and go and ask that > clock driver what the clock frequency is. > > I'd prefer not to have a weak function that parses clock-frequency, since > it's too easy to accidentally use it on systems where parsing that property > is incorrect. > > Certainly, a generic UART driver can call out to a platform-supplied > function to retrieve the clock, and we can provide driver-specific > implementations for x86 super IO and PCI, and generic implementations that > appropriate drivers can call to parse the clocks or clock-frequency property > from DT, and finally for Tegra if we can't parse the clocks property right > now, call the Tegra clock driver directly to look up the value.
I'm not a big fan of weak functions either. In fact I think with driver model we should avoid them. If we can't call a uclass to get the info then perhaps we should wait until we can. Pragmatically I wonder if a UART clock frequency would not be a useful compromise? Some bindings have it, some do not. Maybe we should just add it? Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot