On Mon, Apr 29, 2019 at 5:41 PM Simon Glass <sjg@xxxxxxxxxxxx> wrote: > > Most of these are hand-written, but xilinx-xadc.py is auto-generated by > binding_to_py.py as an example of the use of that tool. > > This is part of a proof-of-concept device-tree validator. See the patch > on the dtc mailing list for details: Honestly, we are pretty far down the path of using json-schema to consider changing to something else. We've already gone thru plenty of concepts over the years with different languages for the schema. While I think there are some cases where being able to do schema with code is useful or necessary, the vast majority of cases can be handled just fine with structured data. I'd rather see how we could augment the data with code. Maybe that's snippets of code within the schema or making the validation code more modular. I would like to see the dtc checks infrastructure be extendable without modifying dtc. That could include supporting checks written in python. One example where we need more than just schema data is validating properties that depend on a provider #.*-cells property. We can't really do that with json-schema. At least the number of cells being correct is covered by dtc already. So it would really be how do we validate the cell data itself. OTOH, I think that is pretty far down the list in priorities of things to validate. There's already thousands of warnings generated by dtc and the json-schema which are slow to get fixed (though some are really subjective and more what to avoid for new users). > > RFC: Python-based device-tree validation > > Signed-off-by: Simon Glass <sjg@xxxxxxxxxxxx> > --- I'll use this one to comment on. Comments are most around goals for the binding doc format. > diff --git a/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.py b/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.py > new file mode 100644 > index 0000000000000..9f55f48f7cde7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.py > @@ -0,0 +1,61 @@ > +# SPDX-License-Identifier: GPL-2.0+ > +# > + > +# Xilinx XADC device driver Having some defined structure at the top-level is beneficial for extracting data and automating review checks. > + > +from kschema import NodeDesc, PropBool, PropClocks, PropInt, PropIntList, PropInterrupts, PropReg, PropStringList > + > +schema = [ > + NodeDesc('xilinx-xadc', ['xlnx,zynq-xadc-1.00.a', 'xlnx,axi-xadc-1.00.a'], False, desc= If one desires to generate a list of all possible compatible strings (to find undocumented ones), how would you do that? > + 'This binding document describes the bindings for both of them since the' > + 'bindings are very similar. The Xilinx XADC is a ADC that can be found in the' > + 'series 7 FPGAs from Xilinx. The XADC has a DRP interface for communication.' > + 'Currently two different frontends for the DRP interface exist. One that is only' > + 'available on the ZYNQ family as a hardmacro in the SoC portion of the ZYNQ. The' > + 'other one is available on all series 7 platforms and is a softmacro with a AXI' > + 'interface. This binding document describes the bindings for both of them since' > + 'the bindings are very similar.', elements=[ One goal with the schema (at least core ones) is to generate documentation from it. That would need to be a format such as rST so we can have formatting. And we'd want to be able to parse the properties and generate tables from them. If someone really gets an itch, we'll rewrite sections of the DT spec in schema. > + PropReg(required=True, > + desc='Address and length of the register set for the device'), For any standard property, we'd have to create the class before bindings can use it. > + PropInterrupts(required=True, How would you handle a property being conditionally required? > + desc='Interrupt for the XADC control interface.'), > + PropClocks(required=True, > + desc='When using the ZYNQ this must be the ZYNQ PCAP clock,' > + 'when using the AXI-XADC pcore this must be the clock that provides the' > + 'clock to the AXI bus interface of the core.'), > + PropStringList('xlnx,external-mux', str_pattern='none|single|dual', > + desc=''), > + PropIntList('xlnx,external-mux-channel', valid_list='0|1|2|3|4|5|6|7|8|9|10|11|12|13|14|16|1|2|3|4|5|6|8', > + desc='Configures which pair of pins is used to' > + 'sample data in external mux mode.' > + 'Valid values for single external multiplexer mode are:' > + 'Valid values for dual external multiplexer mode are:' > + '' > + 'This property needs to be present if the device is configured for' > + 'external multiplexer mode (either single or dual). If the device is' > + 'not using external multiplexer mode the property is ignored.'), > + NodeDesc('xlnx,channels', None, False, desc= > + 'List of external channels that are connected to the ADC', elements=[ > + PropInt('#address-cells', required=True, > + desc='Should be 1.'), > + PropInt('#size-cells', required=True, > + desc='Should be 0.'), > + NodeDesc('None', None, False, desc= > + 'The child nodes of this node represent the external channels which are' > + 'connected to the ADC. If the property is no present no external' > + 'channels will be assumed to be connected.', elements=[ > + NodeDesc('None', None, False, desc= > + 'Each child node represents one channel and has the following' > + 'properties:', elements=[ > + PropIntList('reg', required=True, valid_list='0|1|2|3|4|5|6|7|8|9|10|11|12|13|14|16', We need a different method or arg for every possible way we need to express constraints? For example, say the value must be a power of 2. > + desc='Pair of pins the channel is connected to.' > + 'Note each channel number should only be used at most' > + 'once.'), > + PropBool('xlnx,bipolar', > + desc='If set the channel is used in bipolar' > + 'mode.'), > + ]), > + ]), > + ]), > + ]), > + ] > diff --git a/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt b/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt > index e0e0755cabd8a..24def33e6d6b8 100644 > --- a/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt > +++ b/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt > @@ -32,24 +32,26 @@ Optional properties: > - xlnx,external-mux-channel: Configures which pair of pins is used to > sample data in external mux mode. > Valid values for single external multiplexer mode are: > - 0: VP/VN > - 1: VAUXP[0]/VAUXN[0] > - 2: VAUXP[1]/VAUXN[1] > + * 0: VP/VN > + * 1: VAUXP[0]/VAUXN[0] > + * 2: VAUXP[1]/VAUXN[1] Not really automatic conversion if you have to tweak the source. Is your thought we'd make the txt files more structured to do automatic conversions or we'd commit the python files? Rob