Re: [RFC PATCH 2/5] Documentation: dt-bindings: add example DT binding document

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 8/28/2015 11:55 AM, Rob Herring wrote:
> On Fri, Aug 28, 2015 at 12:49 PM, Matt Porter <mporter@xxxxxxxxxxxx> wrote:
>> On Fri, Aug 28, 2015 at 09:53:06AM -0500, Rob Herring wrote:
>>> On Fri, Aug 28, 2015 at 12:23 AM, Matt Porter <mporter@xxxxxxxxxxxx> wrote:
>>>> Add a skeleton DT binding document that serves as the canonical
>>>> example for implementing YAML-based DT bindings documentation.
>>>> The skeleton binding illustrates use of all fields and variations
>>>> described in the dt-binding-format.txt documentation.
>>>>
>>>> Signed-off-by: Matt Porter <mporter@xxxxxxxxxxxx>
>>>> ---
>>>>  Documentation/devicetree/bindings/skeleton.yaml | 98 +++++++++++++++++++++++++
>>>>  1 file changed, 98 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/skeleton.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/skeleton.yaml b/Documentation/devicetree/bindings/skeleton.yaml
>>>> new file mode 100644
>>>> index 0000000..175965f
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/skeleton.yaml
>>>> @@ -0,0 +1,98 @@
>>>> +%YAML 1.2
>>>> +---
>>>> +id: skel-device
>>>> +
>>>> +title: Skeleton Device
>>>> +
>>>> +maintainer:
>>>> +  - name: Skeleton Person <skel@xxxxxxxxxx>
>>>
>>> We'd want to tie this into get_maintainers.pl obviously.
>>
>> Right, I broke my rule of "no new tags for content we don't already
>> have" by adding this. It stems out of the discussion at LPC where
>> Mark suggested we could avoid the core bindings being moved by adding
>> maintainer info into the binding itself.
> 
> If you stated that rule, I missed it...
> 
>> This is an area where more docs are needed. Ideally, on the first pass
>> conversion we would not attempt to populate these. I had considered
>> that we *could* only add maintainer: tags on core bindings to and
>> adjust get_maintainers.pl to use that info to override the standard
>> directory-based info if it exists. I don't think it's necessary to add
>> a specific maintainer for all of the peripheral bindings if the default
>> is the subsystem maintainer and the dt list. We could at least start
>> directing core binding discussion to the -spec list which is where I
>> think you'd like it.

I think that adding the maintainer: tags for core bindings in the conversion
process (and optionally add the tag for other bindings in the person
converting has the driver and subsystem knowledge to intelligently
do so -- not just a blind list from get_maintainer) would be a valid
exception to your desire to not add new information on the conversion
step.  As much as other uses of information from bindings is near and
dear to me, I think that throttling the binding review fire hose is
the highest priority use of additional information.


> Yes, certainly this can solve my problem around handling core bindings.
> 
> Some subsystem maintainers require a driver have a maintainer. We
> could certainly enforce that too. We could probably automatically
> populate this with the original author as a starting point. OTOH,
> bindings should not change frequently.
> 
>>
>>>> +
>>>> +description: >
>>>> +  The Skeleton Device binding represents the SK11 device produced by
>>>> +  the Skeleton Corporation. The binding can also support compatible
>>>> +  clones made by second source vendors.
>>>> +
>>>> +compatible:
>>>> +  - name: "skel,sk11"
>>>> +  - name: "faux,fx11"
>>>
>>> Is this an OR or AND? We need both.
>>
>> True, this only covers the OR case atm.
>>
>>> The complicated case is "one of {specific names} followed by {generic name}."
>>
>> I need to rethink these. I do have deprecated: tag for that case and
>> possibly "name:" gets split to "generic:" and "specific:" and we can
>> then do the right thing.
>>
>> For the above I would have:
>>
>> compatible:
>>   - specific: "skel,sk11"
>>   - specific: "faux,fx11"
> 
> This can still be an AND or OR relationship.
> 
>>
>> and something like the Allwinner simple framebuffer would be:
>>
>> compatible:
>>   - generic: "simple-framebuffer"
>>   - specific: "allwinner,simple-framebuffer"
>>
>> where our validator would insist on seeing one of the specific: tags
>> along with the generic: tag. A lot of bindings, given current doc
>> patterns would just have two tags like above.
> 
> We could follow Pantelis' suggestion and do C style logic expressions:
> 
> name: ("chip1-ipblock" || "chip2-ipblock") && "generic-ipblock"
> 
> 
>>
>>>> +    description: A clone of the original sk11 device
>>>> +
>>>> +required:
>>>> +  - name: "reg"
>>>
>>> We definitely need type info from the start.
>>
>> Are you sure? Ugh.
>>
>> My big contention here is that we don't carry that content in the
>> docs today so we shouldn't try to add it in the initial conversion.
> 
> What? We have that all over the place. Boolean properties, phandles, etc.

Piling on...  If the type information is obvious in the current binding
doc, then it should be captured in the conversion and not lost.  If the
information is not currently present then it should not be required in
the conversion step.

Which means that we need to define the how to specify the type information.


>> I think you are right in that we should have it documented in the
>> schema but I'm concerned that we make the starting conversion
>> effort too large by adding type info to all converted docs.
> 
> How we do the conversion certainly needs to be worked out. However,
> first we need some sort of agreement that yes this looks promising
> before we spend too much time on it. We need to wait for all the YAML
> haters to come out of hiding. :) Once the direction is settled, then
> we can iron out the details of the structure. Finally, then we can
> determine how to do the conversion. Just because we define how type
> information looks doesn't mean it has to be there day 1.
> 
>> I think we gain a lot even without at the start, but I understand
>> why we need it and how it will help reduce the review firehose
>> with the associated validation tools.
>>
>>>
>>>> +    description: chip select address of skeleton device
>>>> +    reference: spi-slave
>>>
>>> I would like to not have to list properties if the inherited binding
>>> lists it. The problem is we need to say how many cells and the order
>>> (not a problem here, but for mmio devices).
>>
>> Yeah, make sense.
>>
>>> Perhaps we can list the reference at the top level for the node
>>> instead of for every property.
>>
>> That's a good point. I was wondering if per prop references would
>> get unwieldy once we actually add them into all the converted docs.
>> There's a huge number of existing docs without proper references.
> 
> Yes, in many cases they are implied. It should be simple enough to
> generate the list though whether they point to the doc file or just
> list the properties.
> 
>>
>> Ok, I'll take a look at collecting references per group of properties.
>>
>>>
>>>> +  - name: "spi-max-frequency"
>>>> +    description: >
>>>> +      Maximum SPI clocking speed of skeleton device in Hz, must be
>>>> +      1000000
>>>> +    reference: spi-slave
>>>
>>> Rather than listing the property and having constraint in description,
>>> perhaps we could add constraints like this:
>>>
>>> - spi-max-frequency-range: 1000000 1000000
>>>
>>> Or groups of constraints:
>>>
>>> - spi-max-frequency-constraints:
>>>   range: 1000000 1000000
>>>   some-other-constraint: <value>
>>
>> I was hoping to avoid this to start due to my argument for keeping it
>> simple for the first pass at conversion. However, the latter looks
>> flexible enough. We have cases with enumerated values as well to handle
>> the require some thought.
> 
> We can always add constraints as a second step, but I'd like to define
> the structure at least.

Agreed, this is something that I would like to see defined, just to make
sure that we can do it in a reasonable way with the proposed tool.


> Rob
> 


-Frank
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux