Mathieu Poirier [off-list ref] writes:

On 30 March 2015 at 08:04, Alexander Shishkin
[off-list ref] wrote:

quoted

As it looks from the above snippet, you're using a stream of DATA
packets for user's payload. I also noticed that you use an ioctl to
trigger timestamps.

Right, the ioctl() conveys user space intentions on that channel.
Options are kept and applied on every packet for as long as the
channel is open.

So this means that, for example, if you enable timestamps on a channel,
then every single data packet on that channel will be timestamped, which
is a lot of timestamps. Normally, you would only be interested in the
timestamp on the first data packet in the message (or frame or however
we decide to call it). This is one of the reasons why I'm suggesting a
common framing scheme or a "protocol".

quoted

Now, in the STP protocol there are, for example, marked data packets
that can be used to mark beginning of a higher-level message,
timestamped data packets that can be used to mean the same thing and
FLAG packets to mark message boundaries.

Same on my side, I simply haven't included them yet.  I'll do so in my
next iteration.

quoted

In my Intel TH code, I'm using D*TS packet for the beginning of a
message (or "frame") and FLAG packet for the the end of a message.

So my question is, is there any specific STP framing pattern that you
use with Coresight STM or should we perhaps figure out a generic framing
pattern and make it part of the stm class as well?

Now specific pattern... Sending a packet consists of MARK, DATA, FLAG.

Is this pattern mandated by a decoder that you use or is there any other
reason why it's exactly that?

quoted

For example, we can replace stm's .write callback with something like

    int (*packet)(struct stm_data *data,
                  unsigned int type,    /* data, flag, trig etc */
                  unsigned int options, /* timestamped, marked */
                  u64 payload);

and let the stm core do the "framing", which, then, will be common and
consistent across different architectures/stm implementations.

I think the framing should be left to individual drivers.  It's only a
matter of time before we get a weird device that doesn't play well
with others, forcing to carry the ugliness in the STM core rather than
the driver.

Not necessarily. If a device doesn't support one type of packet or the
other, it will be up to them to work around that in the above .packet
callback.

As for the devices that don't play well, there's a question of how much
one can violate the spec and still call oneself compliant.

And isn't carrying "options" redundant?  Using "container_of" on the
"data" field one can get back to the driver specific structure, which
is definitely a better place to keep that information.  I think the
general structure looks good right now, we simply need to find a way
to get rid of the ioctls.

No, what I mean by options here is a property of each individual packet,
not the whole channel. For example, if I want the underlying driver to
send a marked data packet, I do

     stm_data->packet(stm_data, STP_PACKET_D8, STP_OPTION_MARKED, payload);

or if I want to send a timestamped flag, I do

     stm_data->packet(stm_data, STP_PACKET_FLAG, STP_OPTION_TS, 0);

Like I said above, there seems little to be gained from enabling
timestamps for all packets in one channel.

Regarding the same "options", how did you plan on getting those from user space?

Ideally, if we have a framing convension, we don't need to get it from
userspace at all, all userspace should care about is writing data to the
character device and we wrap it up and feed it to the underlying driver.

Regards,
--
Alex