[U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards

[Simon Glass]

Hi,

On 10 May 2017 at 11:45, Tom Rini <trini at konsulko.com> 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 <trini at konsulko.com> 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?  If valid,
> would it also be acceptable to include in dts files that also fill in
> clocks/clock-names from that binding?  Thanks!

clock-frequency should be OK and supported for boards which don't yet
have a clock driver. I don't think we need to explicitly update the
pl011 binding though.

Regards,
Simon