Re: [PATCH 03/15] staging: comedi: ni_routing: Add NI signal routing info

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

 



On Mon, 2017-10-09 at 12:01 -0600, Spencer E Olson wrote:
> On Mon, 2017-10-09 at 10:56 +0100, Ian Abbott wrote:
> > On 08/10/17 07:44, Spencer E Olson wrote:
> > > On Thu, 2016-11-10 at 18:16 +0000, Ian Abbott wrote:
> > >> On 10/11/16 17:54, Greg Kroah-Hartman wrote:
> > >>> On Thu, Nov 10, 2016 at 05:08:36PM +0000, Ian Abbott wrote:
> > >>>> On 12/10/16 12:05, Spencer E. Olson wrote:
> > >>>>> See README for a thorough discussion of this content.
> > >>>>>
> > >>>>> Adds two different collections of CSV files that:
> > >>>>> 1) summarize the various register values for creating routes
> > >>>>>     for a particular family of NI hardware devices;
> > >>>>> 2) summarize all possible (direct) routes that a particular device can
> > >>>>>     make--in this case, one file per device (this data is currently only
> > >>>>>     known to be found by examining a screenshot of the "Available Routes"
> > >>>>>     tab of NI MAX control panel, which is only found on Windows
> > >>>>>     installations of the NI driver).
> > >>>>>
> > >>>>> The collection and maintenance of this information is somewhat tedious and
> > >>>>> requires frequent re-examination and comparison of NI-MAX and/or the NI-MHDDK
> > >>>>> documentation (register programming information) and NI-MHDDK examples.
> > >>>>> These CSV files are constructed so-as to allow near direct comparison
> > >>>>> to NI-MAX and NI-MHDDK.  As such, these serve to ease the task of
> > >>>>> maintaining this knowledge and more quickly enables addition of new NI
> > >>>>> devices.
> > >>>>>
> > >>>>> Signed-off-by: Spencer E. Olson <olsonse@xxxxxxxxx>
> > >>>>>
> > >>>>> *** PLEASE FIND ACTUAL PATCH AT:
> > >>>>> http://www.umich.edu/~olsonse/patches/comedi-devglobal-v1/0003-staging-comedi-ni_routing-Add-NI-signal-routing-info.patch
> > >>>>>
> > >>>>> (This patch included some lines that were too long for email)
> > >>>>> ---
> > >>>>>   drivers/staging/comedi/drivers/ni_routing/README   | 110 +++++++++++++++++++++
> > >>>>>   .../ni_routing/ni_device_routes/PCI-6070E.csv      |  40 ++++++++
> > >>>>>   .../ni_routing/ni_device_routes/PCI-6220.csv       |  46 +++++++++
> > >>>>>   .../ni_routing/ni_device_routes/PCI-6221.csv       |  50 ++++++++++
> > >>>>>   .../ni_routing/ni_device_routes/PCI-6251.csv       |  51 ++++++++++
> > >>>>>   .../ni_routing/ni_device_routes/PCI-6254.csv       |  47 +++++++++
> > >>>>>   .../ni_routing/ni_device_routes/PCI-6259.csv       |  51 ++++++++++
> > >>>>>   .../ni_routing/ni_device_routes/PCI-6534.csv       |  29 ++++++
> > >>>>>   .../ni_routing/ni_device_routes/PCI-6602.csv       |  78 +++++++++++++++
> > >>>>>   .../ni_routing/ni_device_routes/PCI-6713.csv       |  32 ++++++
> > >>>>>   .../ni_routing/ni_device_routes/PCI-6723.csv       |  32 ++++++
> > >>>>>   .../ni_routing/ni_device_routes/PCI-6733.csv       |  34 +++++++
> > >>>>>   .../ni_routing/ni_device_routes/PXI-6030E.csv      |  39 ++++++++
> > >>>>>   .../ni_routing/ni_device_routes/PXI-6224.csv       |  46 +++++++++
> > >>>>>   .../ni_routing/ni_device_routes/PXI-6225.csv       |  49 +++++++++
> > >>>>>   .../ni_routing/ni_device_routes/PXI-6251.csv       |  50 ++++++++++
> > >>>>>   .../ni_routing/ni_device_routes/PXI-6733.csv       |  35 +++++++
> > >>>>>   .../ni_routing/ni_device_routes/PXIe-6251.csv      |  52 ++++++++++
> > >>>>>   .../drivers/ni_routing/ni_route_values/ni_660x.csv | 100 +++++++++++++++++++
> > >>>>>   .../ni_routing/ni_route_values/ni_eseries.csv      |  78 +++++++++++++++
> > >>>>>   .../ni_routing/ni_route_values/ni_mseries.csv      |  90 +++++++++++++++++
> > >>>>>   21 files changed, 1139 insertions(+)
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/README
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6070E.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6220.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6221.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6251.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6254.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6259.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6534.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6602.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6713.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6723.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PCI-6733.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PXI-6030E.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PXI-6224.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PXI-6225.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PXI-6251.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PXI-6733.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_device_routes/PXIe-6251.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_route_values/ni_660x.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_route_values/ni_eseries.csv
> > >>>>>   create mode 100644 drivers/staging/comedi/drivers/ni_routing/ni_route_values/ni_mseries.csv
> > >>>>>
> > >>>>> <... SNIP ...>
> > >>>>>
> > >>>>
> > >>>> While it's nice to have the source data for the generated C files, I'm a bit
> > >>>> worried that any future patches to these .csv files will also be
> > >>>> incompatible with sending by email.  I'm not sure of the best way to deal
> > >>>> with this.
> > >>>
> > >>> They should work in email, it's just a text file and patch handles that
> > >>> fine.
> > >>>
> > >>
> > >> Its the line lengths I'm worried about, i.e. the reason this patch
> > >> couldn't be sent by email!
> > >>
> > > 
> > > I apologize for the severity of this delay, but I am finally getting
> > > back to this patch set.  I've rebased and am going through the various
> > > comments to implement suggested feedback.  Please bear with me by
> > > re-reading your email above and help me understand if there is a
> > > resolution to the issue that Ian brought up:  Namely, is there going to
> > > be a major problem with accepting this patch with its .csv files that
> > > naturally have very long lines?
> > 
> > Yes, I think it's still a problem.
> > 
> > > The reason why I picked using .csv files is stated in the original
> > > commit message: (1) enables easier long-term maintenance, (2) is an
> > > ascii format for textual comparisions, (3) allows direct comparison to
> > > hardware documentation.
> > 
> > Perhaps an intermediate format could be used that: (1) is email and 
> > patch friendly, (2) is well-supported by scripting languages, and (3) 
> > can be transliterated to and from your CSV format easily enough.  I was 
> > thinking that JSON might be a suitable candidate for that.
> > 
> 
> That is an interesting idea--having a well-supported format for
> scripting would make this certainly possible.  If I were to go this
> route, should I include two scripts (one to create CSV file to edit and
> another to convert back to JSON) to ease the maintenance?
> 
> I could take that one step further and suggest an XML format with an XML
> schema to validate value types (although I personally dislike reading
> XML and am not sure I'd like to create a schema at this point).
> 

Ok, so after playing around with this a bit, trying a JSON format and
converters, I've arrived at the following solution that I think can be
acceptable:  By slightly rearranging the data and creating some tools
for conversion, I can now store the data as the authoritative source in
c structures in c-files.

I have created tools to read this information to re-create the csv
spreadsheets to allow for visual comparison with information from NI-MAX
and other tools to convert new csv content back to c-files.  These tools
are comprised of (1) c-program and (3) python scripts.  None of these
tools or other scripting impact the kernel build as the content is
already stored in c source files.

This new development obviates the need to introduce Python/Perl/JSON
content into the kernel build as we discussed in the thread of [PATCH
04/15].  This also removes the issue of having very long lines within
patch files for this content.  I think this is the best approach.

I plan to finish re-implementing my patches with this change, testing,
and re-submitting.  Feel free to send any comments in the interim...

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux