LKML Archive on
help / color / mirror / Atom feed
From: Mathieu Poirier <>
To: Alexander Shishkin <>
Cc: Greg KH <>,
	"" <>,
	Chunyan Zhang <>,
	Kaixu Xia <>,,,
	Al Grant <>
Subject: Re: [PATCH 4/5] coresight-stm: adding driver for CoreSight STM component
Date: Wed, 1 Apr 2015 08:28:00 -0600	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

+ Al Grant

On 1 April 2015 at 08:27, Mathieu Poirier <> wrote:
> Adding Al Grant to the conversation - his knowledge on HW tracing for
> the ARM architecture is definitely an asset for this kind of planning.
> Please add him to future patchset as well.
> On 31 March 2015 at 09:04, Alexander Shishkin
> <> wrote:
>> Mathieu Poirier <> writes:
>>> On 30 March 2015 at 08:04, Alexander Shishkin
>>> <> wrote:
>>>> 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.
> That is how the original coresight-stm driver was implemented.  My
> initial goal was to upstream something that could be used as a
> conversation starter or a foundation to start building on.  I had
> planned to look into the protocol specification itself in later steps.
> But addressing the issue now is just as worthy.
>>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".
>>>> 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.
>>>> 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.
> By the way, are you following the OST specification of this is a
> scheme you came up with?
>>>> 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?
> The driver was following the OST specification, or something close to
> that.  I don't have access to the standard itself and as such not in a
> position to assert how accurate the implementation.  That is one of
> the reason I left it out of my patchset.
>>>> 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.
> I understand your point of view.  On my side I'm trying to avoid
> painting ourselves in the corner.
>>> 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);
> Ah!  It's getting clearer now.
>> 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.
> What do you have in mind for "framing convention"?  As mentioned above
> codeAurora was using the OST specification but Al Grant tell me it
> isn't supported anymore.
> Thanks for the open dialogue,
> Mathieu
>> Regards,
>> --
>> Alex

  reply	other threads:[~2015-04-01 14:28 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-27 23:04 [PATCH 0/5] coresight: next Mathieu Poirier
2015-02-27 23:04 ` [PATCH 1/5] coresight: making cpu index lookup arm64 compliant Mathieu Poirier
2015-02-27 23:04 ` [PATCH 2/5] coresight: fixing compilation warnings picked up by 64bit compiler Mathieu Poirier
2015-02-27 23:04 ` [PATCH 3/5] coresight: Adding coresight support for arm64 architecture Mathieu Poirier
2015-02-27 23:04 ` [PATCH 4/5] coresight-stm: adding driver for CoreSight STM component Mathieu Poirier
2015-03-07 12:27   ` Alexander Shishkin
2015-03-30 14:04   ` Alexander Shishkin
2015-03-30 15:48     ` Mathieu Poirier
2015-03-31 15:04       ` Alexander Shishkin
2015-04-01 14:27         ` Mathieu Poirier
2015-04-01 14:28           ` Mathieu Poirier [this message]
2015-02-27 23:04 ` [PATCH 5/5] coresight-stm: Bindings for System Trace Macrocell Mathieu Poirier
2015-03-19 22:09 ` [PATCH 0/5] coresight: next Mathieu Poirier
2015-03-19 22:24   ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH 4/5] coresight-stm: adding driver for CoreSight STM component' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).