From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jorge Ramirez Date: Wed, 10 May 2017 19:49:50 +0200 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> 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 On 05/10/2017 06:33 PM, 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. > > 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. agreed > 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. I also agree but please do notice that this was not quite what I was saying. what I am saying is that writing a stub driver to only provide a single UART clock rate and nothing else is also an overkill (this platform has no need for other clocks in u-boot) Incidentally the value that I need to retrieve is itself hard-coded in an array in the kernel source code and set up via clk_register_fixed_rate instead of using a fixed-clock node in the device tree. So there is not much value that I can see in providing such a stub driver really... > > Rob