From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Wed, 10 May 2017 13:42:22 -0600 Subject: [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards In-Reply-To: References: <1494261403-12157-1-git-send-email-jorge.ramirez-ortiz@linaro.org> <1494261403-12157-4-git-send-email-jorge.ramirez-ortiz@linaro.org> <20170508172954.GR12511@bill-the-cat> <016bfa5b-ee73-1783-3ca9-34182d813eca@linaro.org> <20170510023000.GZ12511@bill-the-cat> <20170510144927.GF12511@bill-the-cat> <20170510174555.GH12511@bill-the-cat> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, On 10 May 2017 at 13:09, Rob Herring wrote: > On Wed, May 10, 2017 at 12:45 PM, Tom Rini wrote: >> On Wed, May 10, 2017 at 11:33:12AM -0500, Rob Herring wrote: >>> On Wed, May 10, 2017 at 9:49 AM, Tom Rini wrote: >>> > On Wed, May 10, 2017 at 11:33:35AM +0200, Jorge Ramirez wrote: >>> >> On 05/10/2017 04:30 AM, Tom Rini wrote: >>> >> >>hey Tom, I am not sure how to move this forward really so let me >>> >> >>clarify where I think we stand: >>> >> >> >>> >> >>1. The linux kernel does not need the clock property in the uart >>> >> >>nodes (only u-boot does: serial_pl01x.c needs fixing). >>> >> >>2. ehci is not present in the linux kernel poplar dts yet but it >>> >> >>will be eventually. >>> >> >> >>> >> >>with this in mind, what is blocking the acceptance of the patchset? >>> >> >> >>> >> >>I can do v5 using the linux kernel dts as is and creating a >>> >> >>hi3798cv200-u-boot.dtsi that simply adds the nodes above (this time >>> >> >>no #include required:) ) >>> >> >> >>> >> >>Then when ehci is added to the kernel, the ehci node can be removed >>> >> >>from u-boot.dtsi >>> >> >>And when uboot updates its pl01x.c serial driver, the uart0 node >>> >> >>can be removed and the u-boot.dtsi filed completely deleted. >>> >> >Can you take a stab at updating the pl01x driver? Thanks! >>> >> >>> >> updating pl01x is not a big deal I dont think; however this will >>> >> mean requiring a platform specific clock driver to just use the >>> >> pl01x - which will take me some time to get into uboot for my >>> >> platform (and the same might happen to other users). >>> > >>> > Ah right. So the flip side here, is one not allowed to have the clock >>> > property anymore? Yes, it may not be used in the kernel, but has >>> > someone argued that it's not part of the hardware description? >>> >>> First I've ever seen a "clock" property. We have "clocks" from the >>> clock binding which is a phandle plus #clock-cells number of args. We >>> also have "clock-frequency" which is just the frequency value and has >>> been around forever. Why u-boot went off and did something different i >>> don't know. Sigh. What I can say is a 3rd way is not going to be >>> accepted. >> >> Aw crap, I'm in the wrong. I was thinking this was "clock-frequency" >> and not that we had invented our own property here. >> >>> Generally, the clock binding replaces clock-frequency, but there are >>> some cases where clock binding would be overkill or too complicated >>> for early boot and using clock-frequency would be okay. But I don't >>> think "I haven't written my platform clock controller driver yet" is a >>> reason to use clock-frequency as generally most platforms are going to >>> have to have one. Providing a stub driver that just returns a fixed >>> frequency shouldn't be too hard IMO. >> >> So, trying to dig out of the hole we made here. The generic serial >> binding (bindings/serial/serial.txt) documents clock-frequency. The >> pl011 binding (and primecell which it also references) does not. Would >> adding clock-frequency to a pl011 node be valid or invalid? > > Valid in general. It's a common property in the DT spec. Though, it > should be listed in the pl011 binding doc as used just like we list > reg or interrupts. > >> If valid, >> would it also be acceptable to include in dts files that also fill in >> clocks/clock-names from that binding? > > Generally we treat that as not valid as they are mutually exclusive. > > There's 2 better options. You can define fixed clocks with > "fixed-clock" compatible and you already have infrastructure in u-boot > to use that. However, the upstream dts file already defines clocks, so > that doesn't really work here. The 2nd option is have a table of clock > ids and frequencies and register that list of clocks based on matching > the clock controller. You'd need a little bit of infrastructure to > support that, but otherwise a platform would just need to define a > table. It sounds like you are saying the first option isn't an option. The second option adds another layer of pain - we are trying to avoid having platform data. I'd prefer (in this order): 1. A clock driver 2. Use the existing clock-frequency property Regards, SImon