OpenEtherCATsociety / SOES

Simple Open Source EtherCAT Slave
Other
578 stars 249 forks source link

Add support for TIESC #42

Closed nakarlsson closed 6 years ago

nakarlsson commented 6 years ago

Add support for TIESC, this interface the tieschw, tiescbsp, tieschutils provided by TI. It is "tangled up" with SSC stuff so for now just navigate through as a proof-of-concept.

nakarlsson commented 6 years ago

Comments on https://github.com/OpenEtherCATsociety/SOES/pull/42/commits/655cd37ed90731cf930515d966a3977a5aaf15de.

The issue accour when running "TF-1100 Data Link Layer -> Mailbox Transfer(CoE) -> Mailbox Read Service Repeat 2" . The "ack mailbox read" don't clear the IntR and SM1 events (at least not as fast as HW ESCs does, seems to be a timing issue) causing the Repeat to store the next outmailbox before the master actually have read it. This has to be a TIESC issue. Putting this work around as an else if will hopfylly not affect existing ports since they already conforms and perform the "ack mailbox read" ok.

nakarlsson commented 6 years ago

I hear you, basically I have 2 options.

  1. Find a way to detect that "ESC_ackmbxread" have been triggered and is pending. This is the core issue.

or

  1. Trigger a second ESC_ackmbxread but withoutt manually setting the internal ESC state.

if(outpost && IntR) ESC_ackmbxread;

else if IntR ESC_ackmbxread;

nakarlsson commented 6 years ago

@ArthurKetels , A different approach to handle the TI implementation.

Root cause: The value of ESCvar.SM[1].IntR is not reliable for the TI ESC.

The tiescbsp that interface the TI ESC firmware indicate that there are no handling of IntR, when the mailbox first byte is written it only clear the interrupt without updating ESCvar.SM[1].IntR. This leads to a non-deterministic behavior.

From tiescbsp.c void bsp_pdi_mbx_write_start(PRUICSS_Handle pruIcssHandle) { uint16_t ALEvent = HW_GetALEventRegister_Isr();

if(ALEvent & MBX_READ_EVENT)
{
    bsp_send_command_to_firmware(pruIcssHandle, CMD_DL_USER_CLEAR_AL_EVENT_LOW,
                                 ~MBX_READ_EVENT, 0);
}

}

nakarlsson commented 6 years ago

@ArthurKetels , any thoughts on https://github.com/OpenEtherCATsociety/SOES/pull/42/commits/c105a801fbee8f2d787fdf5d9586b0a1e4f287d5?

The "solution" basically add an extra condition that needs to be true, (ESCvar.ALevent & ESCREG_ALEVENT_SM1) which it normally is in this situation. This condition seems to be implemented by TI as expected while the original condition ESCvar.SM[1].IntR don't seem to be or at least not in a deterministic way.

From what I see in SSC, they only use ESCvar.ALevent & ESCREG_ALEVENT_SM1 as a condition for Master Read MbxAck from a HW perspective.

Summary: this change shouldn't add any new condtion, simply adds a check for an existing expected condition, interrupt SM1.

ArthurKetels commented 6 years ago

As SOES does not rely on interrupts but is polling there is no need to guarantee correct state of the ESC interrupt flags as long the origin of the flag is observable. In this case the event is ESCREG_ALEVENT_SM1 and depending on the irq mask the IntR is flagged from this event (and only this event).

A read acknowledge can be done safely on all HW by just reading the ESCREG_ALEVENT_SM1 event. I propose to just use this and drop the ESCvar.SM[1].IntR flag all together. This makes the intent and readabillity better. Having both conditions in the code makes it very confusing for others to understand.

nakarlsson commented 6 years ago

Fine by me, I'll run CTT on the current changesets on TI K2GICE and XMC43relax

I updated the commits and do ack mailbox read on ESCREG_ALEVENT_SM1 instead of Status IntR

nakarlsson commented 6 years ago

CTT tested on TI K2GICE and XMC43relax, 100 iterations each with no error

nakarlsson commented 6 years ago

@ArthurKetels , I'll proceed and merge this? The TI part is proof of concept and the esc.c change is an TI enabler.

ArthurKetels commented 6 years ago

Perfect. Thanks for the testing. I was most concerned for the XMC, for that is my favorite ESC nowadays. Please merge.