On Mon, Jul 16, 2018 at 02:34:47PM -0700, Vedang Patel wrote:
> This adds a new config option "avnu_ap" for ptp4l. Setting this option
> to 1 will enable the AVnu Automotive Profile (AVnu AP) [1] for the
> device.
This is not how we do things.
I have said this more than once before. We don't add a single
monolithic Boolean option for some random profile or extension.
Instead, for each different behavior or attribute, we add a granular
option. Then, the profile is simply a configuration file selecting
the appropriate options.
> - BMCA won't run. So, Announce messages (on master) and announce
> timeouts (on slave) will be disabled.
There is no need to avoid the BMCA. Just make a BMCA that returns
static values. We already have slaveOnly and masterOnly. Your
profile should use those.
For the Announce messages, a simple 'inhibit_announce' option will
do.
> - Source port identity won't be checked while processing Sync and
> Follow_Up messages.
Single option.
> - SLAVE devices will not transition to MASTER when it receives Sync
> timeout.
slaveOnly.
> diff --git a/fsm.c b/fsm.c
> index ce6efad30937..af05abdae294 100644
> --- a/fsm.c
> +++ b/fsm.c
> @@ -34,6 +34,17 @@ enum port_state ptp_fsm(enum port_state state, enum
> fsm_event event, int mdiff)
> case EV_INIT_COMPLETE:
> next = PS_LISTENING;
> break;
> + /*
> + * The following 2 cases are specific to AVnu AP. When we are
> + * running AVnu AP, BMCA is not run. So, we need to assign the
> + * state right after the ports initialization is complete.
> + */
> + case EV_RS_GRAND_MASTER:
> + next = PS_GRAND_MASTER;
> + break;
> + case EV_RS_SLAVE:
> + next = PS_UNCALIBRATED;
> + break;
Don't hack your "special" code into 1588's FSM. Write your own (if needed).
> @@ -1717,6 +1722,14 @@ int process_announce(struct port *p, struct
> ptp_message *m)
> return result;
> }
>
> + /*
> + * Do not process Announce messages when running AVnu Automotive
> + * Profile, conforming to Section 6.3 point 1 (lines 191-195).
> + */
> + if (clock_avnu_ap(p->clock)) {
> + return result;
> + }
This is totally unnecessary, and it hurts the end user by taking an
option away. If an Announce is received, you should process it.
> @@ -2460,7 +2509,18 @@ static enum fsm_event bc_event(struct port *p, int
> fd_index)
> port_renew_transport(p)) {
> return EV_FAULT_DETECTED;
> }
> - return EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES;
> +
> + /*
> + * When AVnu AP is active, we never want the slave to
> + * transition to the master state. So, returning
> + * EV_SYNCHRONIZATION_FAULT ensures it returns back to
> + * PS_UNCALIBRATED.
> + */
> + if (clock_avnu_ap(p->clock)) {
> + return EV_SYNCHRONIZATION_FAULT;
> + } else {
> + return EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES;
> + }
Oh man. Just make a proper FSM please.
Thanks,
Richard
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
