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

[Tom Rini]

On Wed, May 10, 2017 at 01:42:22PM -0600, Simon Glass wrote:
> Hi,
> 
> On 10 May 2017 at 13:09, Rob Herring <robh at kernel.org> wrote:
> > On Wed, May 10, 2017 at 12:45 PM, 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?
> >
> > 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.

No, I think we're going for overkill here by not doing serial_pl01x.c as
platform data.  ns16550 does platform data for this already.  This
sounds like the lowest overhead way to get the clock populated and not
have some DT data that's not going to be accepted upstream.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170511/f5ac1336/attachment.sig>