Hi Mauro, On Wed, Dec 02, 2020 at 03:17:49PM +0100, Mauro Carvalho Chehab wrote: > Em Fri, 27 Nov 2020 12:32:57 +0200 > Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> escreveu: > > > Add register definitions of the MIPI CCS 1.1 standard. > > > > The CCS driver makes extended use of device's capability registers that > > are dependent on CCS version. This involves having an in-memory data > > structure for limit and capability information, creating that data > > structure and accessing it. > > > > The register definitions as well as the definitions of this data structure > > are generated from a text file using a Perl script. Add the generator > > script to make it easy to update the generated files. > > > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > > --- > > .../driver-api/media/drivers/ccs/ccs-regs.txt | 1041 +++++++++++++++++ > > .../driver-api/media/drivers/ccs/mk-ccs-regs | 433 +++++++ > > MAINTAINERS | 1 + > > 3 files changed, 1475 insertions(+) > > create mode 100644 Documentation/driver-api/media/drivers/ccs/ccs-regs.txt > > create mode 100755 Documentation/driver-api/media/drivers/ccs/mk-ccs-regs > > > > diff --git a/Documentation/driver-api/media/drivers/ccs/ccs-regs.txt b/Documentation/driver-api/media/drivers/ccs/ccs-regs.txt > > new file mode 100644 > > index 000000000000..93f0131aa304 > > --- /dev/null > > +++ b/Documentation/driver-api/media/drivers/ccs/ccs-regs.txt > > @@ -0,0 +1,1041 @@ > > +# Copyright (C) 2019--2020 Intel Corporation > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-3-Clause > > > Whenever technically possible, the SPDX header should be the first > line. I sent patches addressing this, as well as renaming it as ccs-regs.asc. > > - > > With regards to css-regs.txt file itself, I think it should be > added on a css-specific ReST file (index.rst?) using the > > .. include > > directive (or a similar one). > > Regards, > Mauro > > > + > > +# register rflags > > +# - f field LSB MSB rflags > > +# - e enum value # after a field > > +# - e enum value [LSB MSB] > > +# - b bool bit > > +# - l arg name min max elsize [discontig...] > > +# > > +# rflags > > +# 8, 16, 32 register bits (default is 8) > > +# v1.1 defined in version 1.1 > > +# f formula > > +# float_ireal iReal or IEEE 754; 32 bits > > +# ireal unsigned iReal > > > > + > > +# general status registers > > +module_model_id 0x0000 16 > > +module_revision_number_major 0x0002 8 > > +frame_count 0x0005 8 > > +pixel_order 0x0006 8 > > > For instance, the above could be, instead: > > .. general status registers > > :: > > module_model_id 0x0000 16 > module_revision_number_major 0x0002 8 > frame_count 0x0005 8 > pixel_order 0x0006 8 > > (which should be easy to parse) > > or, even better: > > general status registers: > > ==================================== ====== == > module_model_id 0x0000 16 > module_revision_number_major 0x0002 8 > frame_count 0x0005 8 > pixel_order 0x0006 8 > ==================================== ====== == > > > Parsing the later could be a little harder, although it would allow > placing the file inside the documentation, with could be interesting. Driver implemented in C uses the macros in the generated header, so the documentation should make use of those macro names. Generating developer documentation to support a single driver seems overkill to me. I develop this driver and over a decade I have received very, very few patches to this driver, so I consider the primary audience of such document to be myself. And I can tell I'm happy looking at the source code. If we'd still do that, then, instead of changing the format of this file, I'd generate the documentation using another script. Btw. mk-ccs-regs script isn't the only script interpreting this file, so changing the format requires changes elsewhere. -- Kind regards, Sakari Ailus
