[etherlab-dev] [PATCH] Default patchset 20160804

Knud Baastrup kba at deif.com
Wed Dec 21 09:25:03 CET 2016

I have added some patches that fixes or improves some of the prior patches. They can all be applied on top of the current patch series, but I would really recommend that they are squashed into the commits where they belong. The 3 last commits are an attempt/suggestion to preserve the backwards compatibility. The backwards compatibility is quite important for us and will prevent us from using the complete patch series. Be free to suggest another naming for the new methods in ecrt.h.


This patch is mandatory if using EoE!

Fix a double lock protection of ext_datagram_queue introduced during patch
refactoring. The double lock protection can result in kernel panic if
EoE mailbox protocol is used.


The "multiple mailbox support" patch introduced a read_mbox_busy flag that
is not cleared in a special case where an ecrt_master_deactivate is called
during a PDO download to a slave with PDO configuration enabled.


Fix to ensure that master bails out if SII data for some unlikely reason
could not be fetched successfully during scanning. Master could
previously get stucked in the same state in fsm_slave_config
and spam syslog.


Created new ecrt_sdo_request_write_with_size and reverted
ecrt_sdo_request_write to preserve backwards compatibility.


Created new ecrt_sdo_request_index_complete, ecrt_slave_config_create
_sdo_request_complete and ecrt_slave_config_alloc_sdo_request to
improve interface and preserve backwards compatibility.


Created new separate ec_slave_topology_info_t for upstream slave
port to preserve backwards compatibility.

Best regards

Knud Baastrup
Software Developer, R&D Products & Solutions (WPT)

T: +4596148458<tel:+4596148458>

[Description: http://www.deif.com/symprex/DEIF.jpg]<www.deifwindpower.com>


From: etherlab-dev [mailto:etherlab-dev-bounces at etherlab.org] On Behalf Of Gavin Lambert
Sent: 4. august 2016 05:54
To: Florian Pose; 'Ralf Roesch'; 'Christoph Permes'; Graeme Foot
Cc: etherlab-dev at etherlab.org
Subject: [etherlab-dev] [PATCH] Default patchset 20160804

Another default-branch patchset update is attached.  It is still based on 5a70ffc4644b.

For convenience, I've made an online patchset repository<https://sourceforge.net/u/uecasm/etherlab-patches/ci/default/tree/#readme>.  It contains a README that describes the individual patches (from prior posts, where I could find them; let me know if something is unclear) and gives the commands required to clone your own copy of the upstream repository and apply the patches to it.

Modified patches:

·         0051-fsm_change-external-datagram.patch:
Fixed some spots where it was accessing the wrong datagram.

New patches:

·         0043-ethercat-diag.patch:
Recently posted to the users list by Ralf Roesch, adding a "diag" command to the ethercat command line tool, to aid in locating lost links and other comms errors.

Other than rebasing within the patchset, I've made the following tweaks to the code (otherwise it is largely unchanged):

o   Various whitespace fixes.

o   Moved a few lines in main.cpp for consistent grouping.

o   Changed llc_reset to a bool, since it was being used as a bool.

o   Tweaked a few lines from assignment to compound assignment to simplify.

o   Removed the data type lookup in EscRegRead and EscRegWrite.

§  Since this is all internal and only used to get the size, which is well known to the caller, the lookup seemed unnecessary.

o   Made EscRegRead and EscRegWrite output errors to stderr instead of stdout.

o   Fixed a printf format issue that generated a compiler warning.

o   Made EscRegRead and EscRegWrite treat errors as non-fatal.

§  Some slaves do not implement all of these registers, and so trying to read them will produce an "I/O error" exception.  In this case it makes more sense to continue reading the other registers than to abort.

§  For example, Beckhoff EL3062 does not implement register 0x030C.

§  0x0308-0x030B can similarly be absent on some older slaves.

I'm a little hesitant about the command name being "diag" - while it's not a bad name for network diagnostics or error stats it might be confused with the "Diagnosis History" object as specified in ETG1020, which is an entirely different thing.  (And something that might be useful to add to the tool in the future.)  I'm open to alternative suggestions.

·         0044-diag-readwrite.patch:
This is a further modification on top of the previous patch which replaces several separate read and write requests with a single read or read+write request (plus one additional read) per slave.  (So naturally it depends on patch 0026.)

In theory this is more efficient, but most importantly since the reset occurs using the same datagram as the read, it's now atomic and there's no risk of losing counts (which could previously happen if the slave incremented its counter after the read but before the write).

·         0045-slave-config-position.patch:
Adds a "position" field to the structure returned by ecrt_slave_config_state.  This allows you to quickly get the ring position of a slave from its relative alias:offset address, which in turn allows you to call other APIs that require this (eg. ecrt_master_get_slave).

Note that the position is only valid if "online" is true, and that it is possible for the value to be stale (ie. the slave has moved to a different position) if the network changes and is consequently rescanned after this call.  So use it defensively.  (You're reasonably safe in the period between requesting the master and activating it, as rescans are inhibited during this time.  OTOH, only the application can request the master; an external tool can't.)

I'm considering whether it would be useful to make a general function available for this conversion, to avoid duplicating the alias:offset conversion logic in too many places (eg. the tool requires it as well, but can't use the slave_config-based conversion since it can't request the master).

·         0046-e1000e-link-detection.patch:
Fixed link detection in e1000e driver for 3.10-3.16.

This is Christoph Permes<http://lists.etherlab.org/pipermail/etherlab-dev/2016/000554.html>' 3.16 patch and 3.14-v2 patch, with the latter backported to 3.12 and 3.10.  I haven't tested these personally.

·         0057-fsm_foe-simplify.patch:
Removes some redundant fields from the FoE FSM; some were unused copy/paste holdovers from the CoE FSM while others were duplicated unnecessarily between the read and write operations, which can't be concurrent for a given slave anyway.

Also fixes the case where the incoming data exceeds the provided buffer to properly terminate the state machine instead of leaving things dangling.  Although note that this still leaves the FoE conversation itself dangling, so you'll likely get an error on the next request if this occurs.

·         0058-foe-password.patch:
Adds support for sending an FoE password along with read or write requests.

Also implements the -o option for the foe_read command (which was documented but not implemented).

Also makes the ioctl behind foe_read actually use the buffer size requested by the caller (instead of a hard-coded value); though note that foe_read itself still uses a hard-coded value of its own (but it's larger, so bigger files should be readable now).  It's possible that users on limited-memory embedded systems might need to reduce this value, but it's still fairly conservative as RAM sizes go.

·         0059-foe-requests.patch:
Makes FoE transfer requests into public ecrt_* API, similar to SDO requests.

Primarily (following my goal of "parallel all the things"), this allows FoE transfers to be non-blocking so that transfers to multiple slaves can occur concurrently from the same requesting thread (previously this was only possible by using separate threads, since the only API was blocking).  Note that due to patch 0018 you can call ecrt_master_deactivate() to delete these requests when you're done with them, even if you haven't called ecrt_master_activate() yet.

It has a possible side benefit that FoE transfers can now be started and monitored from realtime context, although as FoE is mostly used for firmware updates this is unlikely to be all that useful in practice.

I considered a few alternative approaches to this (the next leading contender was to make async versions of the existing FoE ioctls), but this seemed more consistent with existing APIs.  I'm open to suggestions here too though, since it does feel like a slightly odd fit.  (But works quite nicely.)

·         0060-foe-request-progress.patch:
Adds a way to get a "current progress" value (actually the byte offset) for async FoE transfers.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.etherlab.org/pipermail/etherlab-dev/attachments/20161221/7d42012f/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image001.jpg
Type: image/jpeg
Size: 16334 bytes
Desc: image001.jpg
URL: <http://lists.etherlab.org/pipermail/etherlab-dev/attachments/20161221/7d42012f/attachment-0003.jpg>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patches-knud-20161221.zip
Type: application/x-zip-compressed
Size: 13590 bytes
Desc: patches-knud-20161221.zip
URL: <http://lists.etherlab.org/pipermail/etherlab-dev/attachments/20161221/7d42012f/attachment-0003.bin>

More information about the Etherlab-dev mailing list