From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Fri, 14 Aug 2015 20:56:13 -0600 Subject: [U-Boot] [RFC] Merge all ns16550 dm serial drivers into one In-Reply-To: References: <55CE1E33.8040005@wwwdotorg.org> <55CE70F1.8080602@wwwdotorg.org> Message-ID: <55CEAA4D.8050706@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 08/14/2015 05:57 PM, Bin Meng wrote: > Hi, > > On Sat, Aug 15, 2015 at 7:18 AM, Simon Glass wrote: >> Hi, >> >> On 14 August 2015 at 16:51, Stephen Warren wrote: >>> On 08/14/2015 04:40 PM, Bin Meng wrote: >>>> >>>> On Sat, Aug 15, 2015 at 12:59 AM, Simon Glass wrote: >>>>> >>>>> Hi Stephen, >>>>> >>>>> On 14 August 2015 at 10:58, Stephen Warren wrote: >>>>>> >>>>>> On 08/14/2015 10:50 AM, Simon Glass wrote: >>>>>>> >>>>>>> >>>>>>> Hi Bin, >>>>>>> >>>>>>> On 14 August 2015 at 03:18, Bin Meng 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 at 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? >> > > I am not sure which bindings you are looking at, The binding for nvidia,tegra20-uart. In the kernel tree, that's at Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt. > but I checked the > Power.org ePAPR spec v1.1, the clock-frequency is there specially > listed for NS16550 compatible nodes. > > clock-frequency R Specifies the frequency (in Hz) of the baud > rate generator?s input clock > > If you don't have such clock-frequency in your device tree, I would > say you don't follow the spec. The binding for Tegra UARTs doesn't inherit from the generic NS16550 binding, so there's that binding isn't relevant.