Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1577699 > unrolled thread

Re: [PATCH v3 1/3] of: Support parsing phandle argument lists through a nexus node

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v3 1/3] of: Support parsing phandle argument lists  through a nexus node Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-09 16:40 +0100
    Re: [PATCH v3 1/3] of: Support parsing phandle argument lists through  a nexus node Rob Herring <robh+dt@kernel.org> - 2017-02-09 17:10 +0100
      Re: [PATCH v3 1/3] of: Support parsing phandle argument lists through a  nexus node Stephen Boyd <stephen.boyd@linaro.org> - 2017-02-09 21:00 +0100

#1577699 — Re: [PATCH v3 1/3] of: Support parsing phandle argument lists through a nexus node

On Thu, Feb 09, 2017 at 09:17:58AM -0600, Rob Herring wrote:
> Frank, any more comments on this? If not, I plan to apply this series.

Well, I find that a little annoying, because DT has the requirement
that new bindings are properly documented in Documentation, and it
appears that this comes with no documentation what so ever, despite
introducing new properties (like the -map-mask-passthru thing).

So I'm NAKing it until there's some documentation of how this
mechanism is supposed to work.

Merely providing an example in a commit log is (IMHO) insufficient.
An example doesn't explain how it was created, or how to create an
implementation.

Remember, we expect people to do exactly that, so we need to give
them this information so that they can make use of it.

I did try to work out from the code how the -map-mask thing worked,
but eventually gave up.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [next] | [standalone]

#1577728 — Re: [PATCH v3 1/3] of: Support parsing phandle argument lists through a nexus node

On Thu, Feb 9, 2017 at 9:35 AM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Thu, Feb 09, 2017 at 09:17:58AM -0600, Rob Herring wrote:
>> Frank, any more comments on this? If not, I plan to apply this series.
>
> Well, I find that a little annoying, because DT has the requirement
> that new bindings are properly documented in Documentation, and it
> appears that this comes with no documentation what so ever, despite
> introducing new properties (like the -map-mask-passthru thing).
>
> So I'm NAKing it until there's some documentation of how this
> mechanism is supposed to work.
>
> Merely providing an example in a commit log is (IMHO) insufficient.
> An example doesn't explain how it was created, or how to create an
> implementation.

Yes, you are right.

However, I'd like to see this documented in the DT spec, rather than
kernel Documentation/ as this is a core binding. Though that would
also imply first moving the GPIO bindings there. Perhaps just how this
works generically could be in the spec, but the GPIO specifics can
live with the rest of the GPIO bindings for now. This is intended to
extend to other bindings.

> Remember, we expect people to do exactly that, so we need to give
> them this information so that they can make use of it.
>
> I did try to work out from the code how the -map-mask thing worked,
> but eventually gave up.

The code is definitely hard to follow and I've not come up with any
ways to make it easier to read. It's largely copied from the
interrupt-map version, but different enough that sharing isn't really
possible either.

For interrupts, it's documented in the DT spec. The best (only?)
explanation for how interrupt-map works is here[1] (used to be at
devicetree.org).

Rob

[1] http://elinux.org/Device_Tree_Usage#Advanced_Interrupt_Mapping

[toc] | [prev] | [next] | [standalone]

#1577905 — Re: [PATCH v3 1/3] of: Support parsing phandle argument lists through a nexus node

Quoting Rob Herring (2017-02-09 08:00:05)
> On Thu, Feb 9, 2017 at 9:35 AM, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > On Thu, Feb 09, 2017 at 09:17:58AM -0600, Rob Herring wrote:
> >> Frank, any more comments on this? If not, I plan to apply this series.
> >
> > Well, I find that a little annoying, because DT has the requirement
> > that new bindings are properly documented in Documentation, and it
> > appears that this comes with no documentation what so ever, despite
> > introducing new properties (like the -map-mask-passthru thing).
> >
> > So I'm NAKing it until there's some documentation of how this
> > mechanism is supposed to work.
> >
> > Merely providing an example in a commit log is (IMHO) insufficient.
> > An example doesn't explain how it was created, or how to create an
> > implementation.
> 
> Yes, you are right.
> 
> However, I'd like to see this documented in the DT spec, rather than
> kernel Documentation/ as this is a core binding. Though that would
> also imply first moving the GPIO bindings there. Perhaps just how this
> works generically could be in the spec, but the GPIO specifics can
> live with the rest of the GPIO bindings for now. This is intended to
> extend to other bindings.

I will make a patch against the devicetree spec and send it as a reply
to this series.

> 
> > Remember, we expect people to do exactly that, so we need to give
> > them this information so that they can make use of it.
> >
> > I did try to work out from the code how the -map-mask thing worked,
> > but eventually gave up.
> 
> The code is definitely hard to follow and I've not come up with any
> ways to make it easier to read. It's largely copied from the
> interrupt-map version, but different enough that sharing isn't really
> possible either.
> 
> For interrupts, it's documented in the DT spec. The best (only?)
> explanation for how interrupt-map works is here[1] (used to be at
> devicetree.org).
> 

interrupt-map is also documented in the latest DT spec[2]. I can add
another section for "Generic Nexus Properties" and describe how this
code works.

[2] http://www.devicetree.org/specifications-pdf

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web