linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: CoreSight framework and drivers
       [not found] ` <20121219112314.GA26329@mudshark.cambridge.arm.com>
@ 2012-12-19 21:06   ` Pratik Patel
       [not found]   ` <50D1F37E.6000804@ti.com>
  1 sibling, 0 replies; 17+ messages in thread
From: Pratik Patel @ 2012-12-19 21:06 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-arm-kernel@lists.infradead.org, linus.walleij@linaro.org,
	linux@arm.linux.org.uk, magnus.p.persson@stericsson.com,
	arve@android.com, jon-hunter@ti.com, david.rusling@linaro.org,
	dsaxena@linaro.org, john.stultz@linaro.org, d-deao@ti.com,
	christian.bejram@stericsson.com,
	devicetree-discuss@lists.ozlabs.org, linux-arm-msm, linux-kernel

On Wed, Dec 19, 2012 at 11:23:14AM +0000, Will Deacon wrote:
> Hi Pratik,
> 
> On Tue, Dec 18, 2012 at 07:19:17PM +0000, pratikp@codeaurora.org wrote:
> > This RFC is aimed at introducing CoreSight framework as well as
> > individual CoreSight trace component drivers adhering to ARM
> > CoreSight specification. Some prior discussion on this can be
> > referred at [1].
> > 
> > There are 3 kinds of CoreSight trace components:
> > 
> > * Sources: Responsible for producing trace data to provide
> >   visibility for the associated entity.
> > 
> > * Links: Transport components that carry trace data.
> > 
> > * Sinks: Collectors for storing trace data or acting as conduits
> >   for off-chip trace data collection.
> > 
> > These components can be connected in various topologies to suite
> > a particular SoCs tracing needs.
> > 
> > Framework is responsible for gathering and using the information
> > about the registered CoreSight components and their connections
> > to allow it to dynamically deduce the sequence of components
> > representing a connection from a CoreSight source to the
> > currently selected CoreSight sink. This helps the framework to
> > program the correct set of components to satisfy user request.
> 
> From a million miles up, this looks like a sensible sort of design but I
> really think you need to talk to Jon Hunter (he's at least on CC) about
> this, especially where bindings are concerned. If we can get something that
> you both agree on, then we can include the CTI with the other coresight
> components and do a more in-depth review at that point.
> 

Thanks for the feedback.

> Finally, how do you plan on exposing this data to the user? I think the only
> sane way is to have per-device file descriptors which act as pipes to the raw
> data but this means we need some readily available, open-source tooling (or
> at least public specifications of the trace formats).
> 

The sink drivers (ETB, TMC), expose a device node per device that
is used to collect the raw data.

Eg:, we have:
/dev/coresight-etb
/dev/coresight-tmc-etf
/dev/coresight-tmc-etr

where reading /dev/coresight-tmc-etf only works when the ETF is
in circular buffer mode.

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: CoreSight framework and drivers
       [not found]   ` <50D1F37E.6000804@ti.com>
@ 2012-12-19 21:24     ` Pratik Patel
  2012-12-20 17:46       ` Jon Hunter
  0 siblings, 1 reply; 17+ messages in thread
From: Pratik Patel @ 2012-12-19 21:24 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Will Deacon, linux@arm.linux.org.uk, linus.walleij@linaro.org,
	magnus.p.persson@stericsson.com, david.rusling@linaro.org,
	arve@android.com, dsaxena@linaro.org, john.stultz@linaro.org,
	d-deao@ti.com, christian.bejram@stericsson.com,
	devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, linux-arm-msm, linux-kernel

On Wed, Dec 19, 2012 at 11:03:58AM -0600, Jon Hunter wrote:
> 
> On 12/19/2012 05:23 AM, Will Deacon wrote:
> > Hi Pratik,
> > 
> > On Tue, Dec 18, 2012 at 07:19:17PM +0000, pratikp@codeaurora.org wrote:
> >> This RFC is aimed at introducing CoreSight framework as well as
> >> individual CoreSight trace component drivers adhering to ARM
> >> CoreSight specification. Some prior discussion on this can be
> >> referred at [1].
> >>
> >> There are 3 kinds of CoreSight trace components:
> >>
> >> * Sources: Responsible for producing trace data to provide
> >>   visibility for the associated entity.
> >>
> >> * Links: Transport components that carry trace data.
> >>
> >> * Sinks: Collectors for storing trace data or acting as conduits
> >>   for off-chip trace data collection.
> >>
> >> These components can be connected in various topologies to suite
> >> a particular SoCs tracing needs.
> >>
> >> Framework is responsible for gathering and using the information
> >> about the registered CoreSight components and their connections
> >> to allow it to dynamically deduce the sequence of components
> >> representing a connection from a CoreSight source to the
> >> currently selected CoreSight sink. This helps the framework to
> >> program the correct set of components to satisfy user request.
> > 
> > From a million miles up, this looks like a sensible sort of design but I
> > really think you need to talk to Jon Hunter (he's at least on CC) about
> > this, especially where bindings are concerned. If we can get something that
> > you both agree on, then we can include the CTI with the other coresight
> > components and do a more in-depth review at that point.
> 
> I need to look at these in closer detail, but there are a couple
> high-level items that need to be aligned on which are ...
> 
> 1. Why not use the AMBA bus framework?
> 
> You mentioned before that "AMBA bus is for PrimeCell devices (class 0xF)
> as opposed to CoreSight devices (class 0x9)". I will not claim to be
> that familiar with primecell and coresight devices and there
> differences, but this statement does not help me understand why a new
> bus type is needed. Furthermore, the existing ETM and ETB driver in
> arch/arm/kernel/etm.c uses the AMBA bus today and so at least those
> coresight devices are able to use the AMBA bus type.
> 

Currently we use the CoreSight virtual bus to conveniently list
sysfs configuration attributes for all the registered CoreSight
devices.

For eg:
/sys/bus/coresight/devices/coresight-etm0/<attribute>
/sys/bus/coresight/devices/coresight-etm1/<attribute>
/sys/bus/coresight/devices/coresight-stm/<attribute>
/sys/bus/coresight/devices/coresight-tmc-etf/<attribute>
...
...

Some of the attributes are based on device type (i.e. source,
link or sink) so they will exist for all devices of that type
while some are device specific.

Maybe I am misunderstanding the question but are you suggesting
to register CoreSight devices to the AMBA bus instead of the
CoreSight core layer code?

Will Deacon mentioned earlier that AMBA framework can be changed
to accomodate devices with a different class but I am more
concerned with losing some of the stuff that the core layer code
does (eg. coresight_register, coresight_enable, coresight_disable
in coresight.c) if we register CoreSight drivers to the AMBA bus
without letting the core layer know about the device connections.

BTW, sorry the patches didn't get posted to the list since they
are apparently awaiting moderator approval.

> 2. What about the existing ETM/ETB drivers?
> 
> We have existing drivers for ETM and ETB in arch/arm/kernel/etm.c. We
> should either move those drivers to the drivers directory or replace
> them with the new one. However, no point in having two drivers for the
> same coresight device.
> 

Agree we should ensure we don't lose any existing functionality.

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: CoreSight framework and drivers
  2012-12-19 21:24     ` Pratik Patel
@ 2012-12-20 17:46       ` Jon Hunter
  2012-12-20 19:51         ` Pratik Patel
  0 siblings, 1 reply; 17+ messages in thread
From: Jon Hunter @ 2012-12-20 17:46 UTC (permalink / raw)
  To: Pratik Patel
  Cc: Will Deacon, linux@arm.linux.org.uk, linus.walleij@linaro.org,
	magnus.p.persson@stericsson.com, david.rusling@linaro.org,
	arve@android.com, dsaxena@linaro.org, john.stultz@linaro.org,
	d-deao@ti.com, christian.bejram@stericsson.com,
	devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, linux-arm-msm, linux-kernel


On 12/19/2012 03:24 PM, Pratik Patel wrote:

[snip]

> Currently we use the CoreSight virtual bus to conveniently list
> sysfs configuration attributes for all the registered CoreSight
> devices.
> 
> For eg:
> /sys/bus/coresight/devices/coresight-etm0/<attribute>
> /sys/bus/coresight/devices/coresight-etm1/<attribute>
> /sys/bus/coresight/devices/coresight-stm/<attribute>
> /sys/bus/coresight/devices/coresight-tmc-etf/<attribute>
> ...
> ...
> 
> Some of the attributes are based on device type (i.e. source,
> link or sink) so they will exist for all devices of that type
> while some are device specific.
> 
> Maybe I am misunderstanding the question but are you suggesting
> to register CoreSight devices to the AMBA bus instead of the
> CoreSight core layer code?

Yes exactly.

> Will Deacon mentioned earlier that AMBA framework can be changed
> to accomodate devices with a different class but I am more
> concerned with losing some of the stuff that the core layer code
> does (eg. coresight_register, coresight_enable, coresight_disable
> in coresight.c) if we register CoreSight drivers to the AMBA bus
> without letting the core layer know about the device connections.

I may be missing something, but couldn't you keep all the
register/enable/disable stuff but just register the device with the amba
bus? Obviously some changes would need to be made.

Personally, I don't have strong feeling either way, but we have ETM/ETB
drivers using AMBA today and so I am hoping we can come to agreement on
this going forward.

Russell, Will, what are your thoughts?

Otherwise, looking at the code, I like what you have implemented. I
still need to look closer, but I am struggling to figure out how a
coresight device such as the cross-trigger-interface fits with this
model. This model appears to be geared towards coresight devices used
for traces purposes and are either source, links or sinks. The
cross-trigger-interface is not a source or a sink. However, although you
it could be considered as a link (routing events), it is not really, as
it may not link to other coresight sinks/source.

In my case, I have PMU-IRQ --> CTI --> GIC. So a non-coresight source
and sink. In away the CTI looks more like a 2nd-level interrupt
controller than anything else. Hence, another type of coresight device
may be needed in addition to source, links and sinks. Or link is
extended in some way to connect to non-coresight sources/sinks.

Let me know if you have any thoughts.

Cheers
Jon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: CoreSight framework and drivers
  2012-12-20 17:46       ` Jon Hunter
@ 2012-12-20 19:51         ` Pratik Patel
  2012-12-20 20:16           ` Jean Pihet
  2012-12-20 22:54           ` Jon Hunter
  0 siblings, 2 replies; 17+ messages in thread
From: Pratik Patel @ 2012-12-20 19:51 UTC (permalink / raw)
  To: Jon Hunter
  Cc: linux@arm.linux.org.uk, linux-arm-msm, linus.walleij@linaro.org,
	Will Deacon, linux-kernel, magnus.p.persson@stericsson.com,
	david.rusling@linaro.org, arve@android.com, dsaxena@linaro.org,
	john.stultz@linaro.org, d-deao@ti.com,
	christian.bejram@stericsson.com,
	devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org

On Thu, Dec 20, 2012 at 11:46:13AM -0600, Jon Hunter wrote:
> 
> On 12/19/2012 03:24 PM, Pratik Patel wrote:
> 
> [snip]
> 
> > Currently we use the CoreSight virtual bus to conveniently list
> > sysfs configuration attributes for all the registered CoreSight
> > devices.
> > 
> > For eg:
> > /sys/bus/coresight/devices/coresight-etm0/<attribute>
> > /sys/bus/coresight/devices/coresight-etm1/<attribute>
> > /sys/bus/coresight/devices/coresight-stm/<attribute>
> > /sys/bus/coresight/devices/coresight-tmc-etf/<attribute>
> > ...
> > ...
> > 
> > Some of the attributes are based on device type (i.e. source,
> > link or sink) so they will exist for all devices of that type
> > while some are device specific.
> > 
> > Maybe I am misunderstanding the question but are you suggesting
> > to register CoreSight devices to the AMBA bus instead of the
> > CoreSight core layer code?
> 
> Yes exactly.
> 
> > Will Deacon mentioned earlier that AMBA framework can be changed
> > to accomodate devices with a different class but I am more
> > concerned with losing some of the stuff that the core layer code
> > does (eg. coresight_register, coresight_enable, coresight_disable
> > in coresight.c) if we register CoreSight drivers to the AMBA bus
> > without letting the core layer know about the device connections.
> 
> I may be missing something, but couldn't you keep all the
> register/enable/disable stuff but just register the device with the amba
> bus? Obviously some changes would need to be made.
> 

Ok, so are you referring to making CoreSight devices register
with AMBA bus instead of platform bus keeping everything else
intact?

> Personally, I don't have strong feeling either way, but we have ETM/ETB
> drivers using AMBA today and so I am hoping we can come to agreement on
> this going forward.
> 
> Russell, Will, what are your thoughts?
> 
> Otherwise, looking at the code, I like what you have implemented. I
> still need to look closer, but I am struggling to figure out how a
> coresight device such as the cross-trigger-interface fits with this
> model. This model appears to be geared towards coresight devices used
> for traces purposes and are either source, links or sinks. The
> cross-trigger-interface is not a source or a sink. However, although you
> it could be considered as a link (routing events), it is not really, as
> it may not link to other coresight sinks/source.
> 
> In my case, I have PMU-IRQ --> CTI --> GIC. So a non-coresight source
> and sink. In away the CTI looks more like a 2nd-level interrupt
> controller than anything else. Hence, another type of coresight device
> may be needed in addition to source, links and sinks. Or link is
> extended in some way to connect to non-coresight sources/sinks.
> 
> Let me know if you have any thoughts.
> 

I had left the "None" type for miscellaneous stuff but I agree it
should be a separate type - maybe "debug".

Having said that I like the CTI driver you have uploaded. Need to
look at it in more detail. Since CTI connections can vary between
chip to chip, we need a generic way to deal with it.

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: CoreSight framework and drivers
  2012-12-20 19:51         ` Pratik Patel
@ 2012-12-20 20:16           ` Jean Pihet
  2012-12-21 22:12             ` Pratik Patel
  2012-12-20 22:54           ` Jon Hunter
  1 sibling, 1 reply; 17+ messages in thread
From: Jean Pihet @ 2012-12-20 20:16 UTC (permalink / raw)
  To: Pratik Patel
  Cc: Jon Hunter, linux@arm.linux.org.uk, linux-arm-msm,
	linus.walleij@linaro.org, Will Deacon, LKML,
	magnus.p.persson@stericsson.com, david.rusling@linaro.org,
	arve@android.com, dsaxena@linaro.org, john.stultz@linaro.org,
	d-deao@ti.com, christian.bejram@stericsson.com,
	devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org

Hi Pratik,

On Thu, Dec 20, 2012 at 8:51 PM, Pratik Patel <pratikp@codeaurora.org> wrote:
> On Thu, Dec 20, 2012 at 11:46:13AM -0600, Jon Hunter wrote:
>>
>> On 12/19/2012 03:24 PM, Pratik Patel wrote:
>>
>> [snip]
>>
>> > Currently we use the CoreSight virtual bus to conveniently list
>> > sysfs configuration attributes for all the registered CoreSight
>> > devices.
>> >
>> > For eg:
>> > /sys/bus/coresight/devices/coresight-etm0/<attribute>
>> > /sys/bus/coresight/devices/coresight-etm1/<attribute>
>> > /sys/bus/coresight/devices/coresight-stm/<attribute>
>> > /sys/bus/coresight/devices/coresight-tmc-etf/<attribute>
>> > ...
>> > ...
>> >
>> > Some of the attributes are based on device type (i.e. source,
>> > link or sink) so they will exist for all devices of that type
>> > while some are device specific.
>> >
>> > Maybe I am misunderstanding the question but are you suggesting
>> > to register CoreSight devices to the AMBA bus instead of the
>> > CoreSight core layer code?
>>
>> Yes exactly.
>>
>> > Will Deacon mentioned earlier that AMBA framework can be changed
>> > to accomodate devices with a different class but I am more
>> > concerned with losing some of the stuff that the core layer code
>> > does (eg. coresight_register, coresight_enable, coresight_disable
>> > in coresight.c) if we register CoreSight drivers to the AMBA bus
>> > without letting the core layer know about the device connections.
>>
>> I may be missing something, but couldn't you keep all the
>> register/enable/disable stuff but just register the device with the amba
>> bus? Obviously some changes would need to be made.
>>
>
> Ok, so are you referring to making CoreSight devices register
> with AMBA bus instead of platform bus keeping everything else
> intact?
>
>> Personally, I don't have strong feeling either way, but we have ETM/ETB
>> drivers using AMBA today and so I am hoping we can come to agreement on
>> this going forward.
>>
>> Russell, Will, what are your thoughts?
>>
>> Otherwise, looking at the code, I like what you have implemented. I
>> still need to look closer, but I am struggling to figure out how a
>> coresight device such as the cross-trigger-interface fits with this
>> model. This model appears to be geared towards coresight devices used
>> for traces purposes and are either source, links or sinks. The
>> cross-trigger-interface is not a source or a sink. However, although you
>> it could be considered as a link (routing events), it is not really, as
>> it may not link to other coresight sinks/source.
>>
>> In my case, I have PMU-IRQ --> CTI --> GIC. So a non-coresight source
>> and sink. In away the CTI looks more like a 2nd-level interrupt
>> controller than anything else. Hence, another type of coresight device
>> may be needed in addition to source, links and sinks. Or link is
>> extended in some way to connect to non-coresight sources/sinks.
>>
>> Let me know if you have any thoughts.
>>
>
> I had left the "None" type for miscellaneous stuff but I agree it
> should be a separate type - maybe "debug".
>
> Having said that I like the CTI driver you have uploaded. Need to
> look at it in more detail. Since CTI connections can vary between
> chip to chip, we need a generic way to deal with it.
I am also interested to look at such a framework in order to have a
clean implementation of the ETM, PMI/CMI (HW tracing IP on OMAP4+).
I am currently busy on a driver for PMI/CMI which requires some
configuration of the Coresight, ETM, ETB, ATB etc. IP blocks.

Could you send your patches to linux-arm and linux-omap mailing lists?
I will be pleased to review them.

Regards,
Jean

>
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: CoreSight framework and drivers
  2012-12-20 19:51         ` Pratik Patel
  2012-12-20 20:16           ` Jean Pihet
@ 2012-12-20 22:54           ` Jon Hunter
  2012-12-20 23:40             ` Russell King - ARM Linux
  2012-12-21 22:18             ` Pratik Patel
  1 sibling, 2 replies; 17+ messages in thread
From: Jon Hunter @ 2012-12-20 22:54 UTC (permalink / raw)
  To: Pratik Patel
  Cc: linux@arm.linux.org.uk, linux-arm-msm, linus.walleij@linaro.org,
	Will Deacon, linux-kernel, magnus.p.persson@stericsson.com,
	david.rusling@linaro.org, arve@android.com, dsaxena@linaro.org,
	john.stultz@linaro.org, d-deao@ti.com,
	christian.bejram@stericsson.com,
	devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org


On 12/20/2012 01:51 PM, Pratik Patel wrote:
> On Thu, Dec 20, 2012 at 11:46:13AM -0600, Jon Hunter wrote:
>>
>> On 12/19/2012 03:24 PM, Pratik Patel wrote:
>>
>> [snip]
>>
>>> Currently we use the CoreSight virtual bus to conveniently list
>>> sysfs configuration attributes for all the registered CoreSight
>>> devices.
>>>
>>> For eg:
>>> /sys/bus/coresight/devices/coresight-etm0/<attribute>
>>> /sys/bus/coresight/devices/coresight-etm1/<attribute>
>>> /sys/bus/coresight/devices/coresight-stm/<attribute>
>>> /sys/bus/coresight/devices/coresight-tmc-etf/<attribute>
>>> ...
>>> ...
>>>
>>> Some of the attributes are based on device type (i.e. source,
>>> link or sink) so they will exist for all devices of that type
>>> while some are device specific.
>>>
>>> Maybe I am misunderstanding the question but are you suggesting
>>> to register CoreSight devices to the AMBA bus instead of the
>>> CoreSight core layer code?
>>
>> Yes exactly.
>>
>>> Will Deacon mentioned earlier that AMBA framework can be changed
>>> to accomodate devices with a different class but I am more
>>> concerned with losing some of the stuff that the core layer code
>>> does (eg. coresight_register, coresight_enable, coresight_disable
>>> in coresight.c) if we register CoreSight drivers to the AMBA bus
>>> without letting the core layer know about the device connections.
>>
>> I may be missing something, but couldn't you keep all the
>> register/enable/disable stuff but just register the device with the amba
>> bus? Obviously some changes would need to be made.
>>
> 
> Ok, so are you referring to making CoreSight devices register
> with AMBA bus instead of platform bus keeping everything else
> intact?

Yes exactly. However, please note I am not saying that we should do
this, and I asking what direction does the community want us to take
here? Platform bus or AMBA bus?

>> Personally, I don't have strong feeling either way, but we have ETM/ETB
>> drivers using AMBA today and so I am hoping we can come to agreement on
>> this going forward.
>>
>> Russell, Will, what are your thoughts?
>>
>> Otherwise, looking at the code, I like what you have implemented. I
>> still need to look closer, but I am struggling to figure out how a
>> coresight device such as the cross-trigger-interface fits with this
>> model. This model appears to be geared towards coresight devices used
>> for traces purposes and are either source, links or sinks. The
>> cross-trigger-interface is not a source or a sink. However, although you
>> it could be considered as a link (routing events), it is not really, as
>> it may not link to other coresight sinks/source.
>>
>> In my case, I have PMU-IRQ --> CTI --> GIC. So a non-coresight source
>> and sink. In away the CTI looks more like a 2nd-level interrupt
>> controller than anything else. Hence, another type of coresight device
>> may be needed in addition to source, links and sinks. Or link is
>> extended in some way to connect to non-coresight sources/sinks.
>>
>> Let me know if you have any thoughts.
>>
> 
> I had left the "None" type for miscellaneous stuff but I agree it
> should be a separate type - maybe "debug".
> 
> Having said that I like the CTI driver you have uploaded. Need to
> look at it in more detail. Since CTI connections can vary between
> chip to chip, we need a generic way to deal with it.

Yes if you have any ideas let me know. As Will had mentioned it would be
good to have a common function table all these devices could use too. I
will take a closer look at what you have.

Cheers
Jon


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: CoreSight framework and drivers
  2012-12-20 22:54           ` Jon Hunter
@ 2012-12-20 23:40             ` Russell King - ARM Linux
  2012-12-21 22:17               ` Pratik Patel
  2012-12-21 22:18             ` Pratik Patel
  1 sibling, 1 reply; 17+ messages in thread
From: Russell King - ARM Linux @ 2012-12-20 23:40 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Pratik Patel, linux-arm-msm, linus.walleij@linaro.org,
	Will Deacon, linux-kernel, magnus.p.persson@stericsson.com,
	david.rusling@linaro.org, arve@android.com, dsaxena@linaro.org,
	john.stultz@linaro.org, d-deao@ti.com,
	christian.bejram@stericsson.com,
	devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org

On Thu, Dec 20, 2012 at 04:54:38PM -0600, Jon Hunter wrote:
> On 12/20/2012 01:51 PM, Pratik Patel wrote:
> > Ok, so are you referring to making CoreSight devices register
> > with AMBA bus instead of platform bus keeping everything else
> > intact?
> 
> Yes exactly. However, please note I am not saying that we should do
> this, and I asking what direction does the community want us to take
> here? Platform bus or AMBA bus?

One of the issues which worries me about mixing peripheral drivers on
random different buses is... what happens when we end up with a SoC
which gates the APB clock at bus level (there are SoCs which gate the
APB clock at peripheral level.)  In other words, an APB bus only gets
clocked upon request.

We can deal with that with the infrastructure we have in place in the
AMBA bus layer, but not with the platform bus - we'd have to teach the
platform bus driver about the special APB clock instead of having it
handled primerily at the bus layer.

At least the coresight ETM peripherals make use of the APB bus.  They
have a whole pile of registers on the APB bus, and they have the
primecell IDs stored in the last words of the peripheral, again just
like the other primecell devices we have using the AMBA bus layer.

What I'd say is... why stick it on a different bus type from the other
peripherals which might make things harder in the future?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: CoreSight framework and drivers
  2012-12-20 20:16           ` Jean Pihet
@ 2012-12-21 22:12             ` Pratik Patel
  0 siblings, 0 replies; 17+ messages in thread
From: Pratik Patel @ 2012-12-21 22:12 UTC (permalink / raw)
  To: Jean Pihet
  Cc: Jon Hunter, linux@arm.linux.org.uk, linux-arm-msm,
	linus.walleij@linaro.org, Will Deacon, LKML,
	magnus.p.persson@stericsson.com, david.rusling@linaro.org,
	arve@android.com, dsaxena@linaro.org, john.stultz@linaro.org,
	d-deao@ti.com, christian.bejram@stericsson.com,
	devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org

On Thu, Dec 20, 2012 at 09:16:15PM +0100, Jean Pihet wrote:
> Hi Pratik,
> 
> On Thu, Dec 20, 2012 at 8:51 PM, Pratik Patel <pratikp@codeaurora.org> wrote:
> > On Thu, Dec 20, 2012 at 11:46:13AM -0600, Jon Hunter wrote:
> >>
> >> On 12/19/2012 03:24 PM, Pratik Patel wrote:
> >>
> >> [snip]
> >>
> >> > Currently we use the CoreSight virtual bus to conveniently list
> >> > sysfs configuration attributes for all the registered CoreSight
> >> > devices.
> >> >
> >> > For eg:
> >> > /sys/bus/coresight/devices/coresight-etm0/<attribute>
> >> > /sys/bus/coresight/devices/coresight-etm1/<attribute>
> >> > /sys/bus/coresight/devices/coresight-stm/<attribute>
> >> > /sys/bus/coresight/devices/coresight-tmc-etf/<attribute>
> >> > ...
> >> > ...
> >> >
> >> > Some of the attributes are based on device type (i.e. source,
> >> > link or sink) so they will exist for all devices of that type
> >> > while some are device specific.
> >> >
> >> > Maybe I am misunderstanding the question but are you suggesting
> >> > to register CoreSight devices to the AMBA bus instead of the
> >> > CoreSight core layer code?
> >>
> >> Yes exactly.
> >>
> >> > Will Deacon mentioned earlier that AMBA framework can be changed
> >> > to accomodate devices with a different class but I am more
> >> > concerned with losing some of the stuff that the core layer code
> >> > does (eg. coresight_register, coresight_enable, coresight_disable
> >> > in coresight.c) if we register CoreSight drivers to the AMBA bus
> >> > without letting the core layer know about the device connections.
> >>
> >> I may be missing something, but couldn't you keep all the
> >> register/enable/disable stuff but just register the device with the amba
> >> bus? Obviously some changes would need to be made.
> >>
> >
> > Ok, so are you referring to making CoreSight devices register
> > with AMBA bus instead of platform bus keeping everything else
> > intact?
> >
> >> Personally, I don't have strong feeling either way, but we have ETM/ETB
> >> drivers using AMBA today and so I am hoping we can come to agreement on
> >> this going forward.
> >>
> >> Russell, Will, what are your thoughts?
> >>
> >> Otherwise, looking at the code, I like what you have implemented. I
> >> still need to look closer, but I am struggling to figure out how a
> >> coresight device such as the cross-trigger-interface fits with this
> >> model. This model appears to be geared towards coresight devices used
> >> for traces purposes and are either source, links or sinks. The
> >> cross-trigger-interface is not a source or a sink. However, although you
> >> it could be considered as a link (routing events), it is not really, as
> >> it may not link to other coresight sinks/source.
> >>
> >> In my case, I have PMU-IRQ --> CTI --> GIC. So a non-coresight source
> >> and sink. In away the CTI looks more like a 2nd-level interrupt
> >> controller than anything else. Hence, another type of coresight device
> >> may be needed in addition to source, links and sinks. Or link is
> >> extended in some way to connect to non-coresight sources/sinks.
> >>
> >> Let me know if you have any thoughts.
> >>
> >
> > I had left the "None" type for miscellaneous stuff but I agree it
> > should be a separate type - maybe "debug".
> >
> > Having said that I like the CTI driver you have uploaded. Need to
> > look at it in more detail. Since CTI connections can vary between
> > chip to chip, we need a generic way to deal with it.
> I am also interested to look at such a framework in order to have a
> clean implementation of the ETM, PMI/CMI (HW tracing IP on OMAP4+).
> I am currently busy on a driver for PMI/CMI which requires some
> configuration of the Coresight, ETM, ETB, ATB etc. IP blocks.
> 
> Could you send your patches to linux-arm and linux-omap mailing lists?
> I will be pleased to review them.
> 

Thanks. I did post them to linux-arm-kernel but they are waiting
on moderator approval. Hopefully they will get posted soon.

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: CoreSight framework and drivers
  2012-12-20 23:40             ` Russell King - ARM Linux
@ 2012-12-21 22:17               ` Pratik Patel
  0 siblings, 0 replies; 17+ messages in thread
From: Pratik Patel @ 2012-12-21 22:17 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Jon Hunter, linux-arm-msm, linus.walleij@linaro.org, Will Deacon,
	linux-kernel, magnus.p.persson@stericsson.com,
	david.rusling@linaro.org, arve@android.com, dsaxena@linaro.org,
	john.stultz@linaro.org, d-deao@ti.com,
	christian.bejram@stericsson.com,
	devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org

On Thu, Dec 20, 2012 at 11:40:11PM +0000, Russell King - ARM Linux wrote:
> On Thu, Dec 20, 2012 at 04:54:38PM -0600, Jon Hunter wrote:
> > On 12/20/2012 01:51 PM, Pratik Patel wrote:
> > > Ok, so are you referring to making CoreSight devices register
> > > with AMBA bus instead of platform bus keeping everything else
> > > intact?
> > 
> > Yes exactly. However, please note I am not saying that we should do
> > this, and I asking what direction does the community want us to take
> > here? Platform bus or AMBA bus?
> 
> One of the issues which worries me about mixing peripheral drivers on
> random different buses is... what happens when we end up with a SoC
> which gates the APB clock at bus level (there are SoCs which gate the
> APB clock at peripheral level.)  In other words, an APB bus only gets
> clocked upon request.
> 
> We can deal with that with the infrastructure we have in place in the
> AMBA bus layer, but not with the platform bus - we'd have to teach the
> platform bus driver about the special APB clock instead of having it
> handled primerily at the bus layer.
> 
> At least the coresight ETM peripherals make use of the APB bus.  They
> have a whole pile of registers on the APB bus, and they have the
> primecell IDs stored in the last words of the peripheral, again just
> like the other primecell devices we have using the AMBA bus layer.
> 
> What I'd say is... why stick it on a different bus type from the other
> peripherals which might make things harder in the future?

Thanks for the info. I will look into using the AMBA bus instead
of the platform bus but one issue I notice is that AMBA framework
seems to support one contiguous register space.

CoreSight STM typically has a config register space and a
stimulus port/channel register space and these can be
non-contiguous in the chip memory map.

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: CoreSight framework and drivers
  2012-12-20 22:54           ` Jon Hunter
  2012-12-20 23:40             ` Russell King - ARM Linux
@ 2012-12-21 22:18             ` Pratik Patel
  2012-12-23 11:32               ` Will Deacon
  2013-01-02 20:00               ` Jon Hunter
  1 sibling, 2 replies; 17+ messages in thread
From: Pratik Patel @ 2012-12-21 22:18 UTC (permalink / raw)
  To: Jon Hunter
  Cc: linux@arm.linux.org.uk, linux-arm-msm, linus.walleij@linaro.org,
	Will Deacon, linux-kernel, magnus.p.persson@stericsson.com,
	david.rusling@linaro.org, arve@android.com, dsaxena@linaro.org,
	john.stultz@linaro.org, d-deao@ti.com,
	christian.bejram@stericsson.com,
	devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org

On Thu, Dec 20, 2012 at 04:54:38PM -0600, Jon Hunter wrote:
> 
> On 12/20/2012 01:51 PM, Pratik Patel wrote:
> > On Thu, Dec 20, 2012 at 11:46:13AM -0600, Jon Hunter wrote:
> >>
> >> On 12/19/2012 03:24 PM, Pratik Patel wrote:
> >>
> >> [snip]
> >>
> >>> Currently we use the CoreSight virtual bus to conveniently list
> >>> sysfs configuration attributes for all the registered CoreSight
> >>> devices.
> >>>
> >>> For eg:
> >>> /sys/bus/coresight/devices/coresight-etm0/<attribute>
> >>> /sys/bus/coresight/devices/coresight-etm1/<attribute>
> >>> /sys/bus/coresight/devices/coresight-stm/<attribute>
> >>> /sys/bus/coresight/devices/coresight-tmc-etf/<attribute>
> >>> ...
> >>> ...
> >>>
> >>> Some of the attributes are based on device type (i.e. source,
> >>> link or sink) so they will exist for all devices of that type
> >>> while some are device specific.
> >>>
> >>> Maybe I am misunderstanding the question but are you suggesting
> >>> to register CoreSight devices to the AMBA bus instead of the
> >>> CoreSight core layer code?
> >>
> >> Yes exactly.
> >>
> >>> Will Deacon mentioned earlier that AMBA framework can be changed
> >>> to accomodate devices with a different class but I am more
> >>> concerned with losing some of the stuff that the core layer code
> >>> does (eg. coresight_register, coresight_enable, coresight_disable
> >>> in coresight.c) if we register CoreSight drivers to the AMBA bus
> >>> without letting the core layer know about the device connections.
> >>
> >> I may be missing something, but couldn't you keep all the
> >> register/enable/disable stuff but just register the device with the amba
> >> bus? Obviously some changes would need to be made.
> >>
> > 
> > Ok, so are you referring to making CoreSight devices register
> > with AMBA bus instead of platform bus keeping everything else
> > intact?
> 
> Yes exactly. However, please note I am not saying that we should do
> this, and I asking what direction does the community want us to take
> here? Platform bus or AMBA bus?
> 
> >> Personally, I don't have strong feeling either way, but we have ETM/ETB
> >> drivers using AMBA today and so I am hoping we can come to agreement on
> >> this going forward.
> >>
> >> Russell, Will, what are your thoughts?
> >>
> >> Otherwise, looking at the code, I like what you have implemented. I
> >> still need to look closer, but I am struggling to figure out how a
> >> coresight device such as the cross-trigger-interface fits with this
> >> model. This model appears to be geared towards coresight devices used
> >> for traces purposes and are either source, links or sinks. The
> >> cross-trigger-interface is not a source or a sink. However, although you
> >> it could be considered as a link (routing events), it is not really, as
> >> it may not link to other coresight sinks/source.
> >>
> >> In my case, I have PMU-IRQ --> CTI --> GIC. So a non-coresight source
> >> and sink. In away the CTI looks more like a 2nd-level interrupt
> >> controller than anything else. Hence, another type of coresight device
> >> may be needed in addition to source, links and sinks. Or link is
> >> extended in some way to connect to non-coresight sources/sinks.
> >>
> >> Let me know if you have any thoughts.
> >>
> > 
> > I had left the "None" type for miscellaneous stuff but I agree it
> > should be a separate type - maybe "debug".
> > 
> > Having said that I like the CTI driver you have uploaded. Need to
> > look at it in more detail. Since CTI connections can vary between
> > chip to chip, we need a generic way to deal with it.
> 
> Yes if you have any ideas let me know. As Will had mentioned it would be
> good to have a common function table all these devices could use too. I
> will take a closer look at what you have.
> 

I had started on a CTI driver much in line with your current
implementation but your approach looks good to me.

Looking at the code though, I couldn't find a way to perform a
mapping between two different CTIs. Reason I ask is because we
have use cases where we need to map one CTIs input to another
CTIs output on the same channel.

Do you intend to support different entities within kernel to map
different trig_in or trig_out on the same CTI? If so, it might
probably require reference counting for the enable/disable.

What user interface do you plan to provide for the CTI? Maybe
something consistent with other CoreSight components in sysfs to
allow users to enable, disable, map and unmap ???

Please let me know your thoughts.

CTIs can easily be made part of the CoreSight core layer using a
different type but apart from being part of the CoreSight list
containing all the trace and CTI components, not sure if there is
anything that is useful to do in the core layer. Need to think
about this.

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: CoreSight framework and drivers
  2012-12-21 22:18             ` Pratik Patel
@ 2012-12-23 11:32               ` Will Deacon
  2013-01-03 18:06                 ` Pratik Patel
  2013-01-02 20:00               ` Jon Hunter
  1 sibling, 1 reply; 17+ messages in thread
From: Will Deacon @ 2012-12-23 11:32 UTC (permalink / raw)
  To: Pratik Patel
  Cc: Jon Hunter, linux@arm.linux.org.uk, linux-arm-msm@vger.kernel.org,
	linus.walleij@linaro.org, linux-kernel@vger.kernel.org,
	magnus.p.persson@stericsson.com, david.rusling@linaro.org,
	arve@android.com, dsaxena@linaro.org, john.stultz@linaro.org,
	d-deao@ti.com, christian.bejram@stericsson.com,
	devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org

On Fri, Dec 21, 2012 at 10:18:28PM +0000, Pratik Patel wrote:
> What user interface do you plan to provide for the CTI? Maybe
> something consistent with other CoreSight components in sysfs to
> allow users to enable, disable, map and unmap ???
> 
> Please let me know your thoughts.

Rather than have your current approach of dev nodes + sysfs config files for
each coresight device, I think it might be better to follow something closer
to ftrace and stick per-device directories under debugfs/coresight/. Then you
can have a pipe file and some config files in the same directory for each
component. You also don't need to do any mapping operations with this (just
post-process the stream directly).

It might also be fun to play with file redirection for sources and sinks, but
that's probably a bit too invasive.

Will

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: CoreSight framework and drivers
  2012-12-21 22:18             ` Pratik Patel
  2012-12-23 11:32               ` Will Deacon
@ 2013-01-02 20:00               ` Jon Hunter
  2013-01-03 19:32                 ` Pratik Patel
  1 sibling, 1 reply; 17+ messages in thread
From: Jon Hunter @ 2013-01-02 20:00 UTC (permalink / raw)
  To: Pratik Patel
  Cc: linux@arm.linux.org.uk, linux-arm-msm, linus.walleij@linaro.org,
	Will Deacon, linux-kernel, magnus.p.persson@stericsson.com,
	david.rusling@linaro.org, arve@android.com, dsaxena@linaro.org,
	john.stultz@linaro.org, d-deao@ti.com,
	christian.bejram@stericsson.com,
	devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org


On 12/21/2012 04:18 PM, Pratik Patel wrote:
> On Thu, Dec 20, 2012 at 04:54:38PM -0600, Jon Hunter wrote:
>>
>> On 12/20/2012 01:51 PM, Pratik Patel wrote:
>>> On Thu, Dec 20, 2012 at 11:46:13AM -0600, Jon Hunter wrote:
>>>>
>>>> On 12/19/2012 03:24 PM, Pratik Patel wrote:
>>>>
>>>> [snip]
>>>>
>>>>> Currently we use the CoreSight virtual bus to conveniently list
>>>>> sysfs configuration attributes for all the registered CoreSight
>>>>> devices.
>>>>>
>>>>> For eg:
>>>>> /sys/bus/coresight/devices/coresight-etm0/<attribute>
>>>>> /sys/bus/coresight/devices/coresight-etm1/<attribute>
>>>>> /sys/bus/coresight/devices/coresight-stm/<attribute>
>>>>> /sys/bus/coresight/devices/coresight-tmc-etf/<attribute>
>>>>> ...
>>>>> ...
>>>>>
>>>>> Some of the attributes are based on device type (i.e. source,
>>>>> link or sink) so they will exist for all devices of that type
>>>>> while some are device specific.
>>>>>
>>>>> Maybe I am misunderstanding the question but are you suggesting
>>>>> to register CoreSight devices to the AMBA bus instead of the
>>>>> CoreSight core layer code?
>>>>
>>>> Yes exactly.
>>>>
>>>>> Will Deacon mentioned earlier that AMBA framework can be changed
>>>>> to accomodate devices with a different class but I am more
>>>>> concerned with losing some of the stuff that the core layer code
>>>>> does (eg. coresight_register, coresight_enable, coresight_disable
>>>>> in coresight.c) if we register CoreSight drivers to the AMBA bus
>>>>> without letting the core layer know about the device connections.
>>>>
>>>> I may be missing something, but couldn't you keep all the
>>>> register/enable/disable stuff but just register the device with the amba
>>>> bus? Obviously some changes would need to be made.
>>>>
>>>
>>> Ok, so are you referring to making CoreSight devices register
>>> with AMBA bus instead of platform bus keeping everything else
>>> intact?
>>
>> Yes exactly. However, please note I am not saying that we should do
>> this, and I asking what direction does the community want us to take
>> here? Platform bus or AMBA bus?
>>
>>>> Personally, I don't have strong feeling either way, but we have ETM/ETB
>>>> drivers using AMBA today and so I am hoping we can come to agreement on
>>>> this going forward.
>>>>
>>>> Russell, Will, what are your thoughts?
>>>>
>>>> Otherwise, looking at the code, I like what you have implemented. I
>>>> still need to look closer, but I am struggling to figure out how a
>>>> coresight device such as the cross-trigger-interface fits with this
>>>> model. This model appears to be geared towards coresight devices used
>>>> for traces purposes and are either source, links or sinks. The
>>>> cross-trigger-interface is not a source or a sink. However, although you
>>>> it could be considered as a link (routing events), it is not really, as
>>>> it may not link to other coresight sinks/source.
>>>>
>>>> In my case, I have PMU-IRQ --> CTI --> GIC. So a non-coresight source
>>>> and sink. In away the CTI looks more like a 2nd-level interrupt
>>>> controller than anything else. Hence, another type of coresight device
>>>> may be needed in addition to source, links and sinks. Or link is
>>>> extended in some way to connect to non-coresight sources/sinks.
>>>>
>>>> Let me know if you have any thoughts.
>>>>
>>>
>>> I had left the "None" type for miscellaneous stuff but I agree it
>>> should be a separate type - maybe "debug".
>>>
>>> Having said that I like the CTI driver you have uploaded. Need to
>>> look at it in more detail. Since CTI connections can vary between
>>> chip to chip, we need a generic way to deal with it.
>>
>> Yes if you have any ideas let me know. As Will had mentioned it would be
>> good to have a common function table all these devices could use too. I
>> will take a closer look at what you have.
>>
> 
> I had started on a CTI driver much in line with your current
> implementation but your approach looks good to me.
> 
> Looking at the code though, I couldn't find a way to perform a
> mapping between two different CTIs. Reason I ask is because we
> have use cases where we need to map one CTIs input to another
> CTIs output on the same channel.

The code is largely based upon the existing cti helpers, which just had
a cti_map_trigger() function. The use-case you described is not
supported by the current helpers and so also not supported in my initial
implementation (although we should add this). Hence, it would be
probably best to split the cti_map_trigger() into two functions say,
cti_map_triggerin() and cti_map_triggerout(), to map a trigger
input/output to a channel.

> Do you intend to support different entities within kernel to map
> different trig_in or trig_out on the same CTI? If so, it might
> probably require reference counting for the enable/disable.

I need to think about this some more, as this does make it a little more
complex. Right now only one entity can use a cti module at a time.

> What user interface do you plan to provide for the CTI? Maybe
> something consistent with other CoreSight components in sysfs to
> allow users to enable, disable, map and unmap ???

Yes that's possible.

> Please let me know your thoughts.
> 
> CTIs can easily be made part of the CoreSight core layer using a
> different type but apart from being part of the CoreSight list
> containing all the trace and CTI components, not sure if there is
> anything that is useful to do in the core layer. Need to think
> about this.

Yes it would be good to make this all part of the same coresight framework.

Cheers
Jon


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: CoreSight framework and drivers
  2012-12-23 11:32               ` Will Deacon
@ 2013-01-03 18:06                 ` Pratik Patel
  2013-01-07 11:58                   ` Will Deacon
  0 siblings, 1 reply; 17+ messages in thread
From: Pratik Patel @ 2013-01-03 18:06 UTC (permalink / raw)
  To: Will Deacon
  Cc: Jon Hunter, linux@arm.linux.org.uk, linux-arm-msm@vger.kernel.org,
	linus.walleij@linaro.org, linux-kernel@vger.kernel.org,
	magnus.p.persson@stericsson.com, david.rusling@linaro.org,
	arve@android.com, dsaxena@linaro.org, john.stultz@linaro.org,
	d-deao@ti.com, christian.bejram@stericsson.com,
	devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org

On Sun, Dec 23, 2012 at 11:32:39AM +0000, Will Deacon wrote:
> On Fri, Dec 21, 2012 at 10:18:28PM +0000, Pratik Patel wrote:
> > What user interface do you plan to provide for the CTI? Maybe
> > something consistent with other CoreSight components in sysfs to
> > allow users to enable, disable, map and unmap ???
> > 
> > Please let me know your thoughts.
> 
> Rather than have your current approach of dev nodes + sysfs config files for
> each coresight device, I think it might be better to follow something closer
> to ftrace and stick per-device directories under debugfs/coresight/. Then you
> can have a pipe file and some config files in the same directory for each
> component. You also don't need to do any mapping operations with this (just
> post-process the stream directly).
> 

Thanks for the suggestion. I had initially debated between debugfs
and sysfs but chose sysfs + dev nodes since using device attributes
relieves the drivers from manually managing directories and files,
its taken care of by the device and sysfs layers. Moreover, since
these are physical devices, device attributes made sense at the
time.

The map and unmap I was referring to was for the CTI trigger
mappings. Dev nodes are currently intended to provide the raw
data collected in the sinks.

Whats the advantage in using debugfs here?

> It might also be fun to play with file redirection for sources and sinks, but
> that's probably a bit too invasive.
> 
> Will

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: CoreSight framework and drivers
  2013-01-02 20:00               ` Jon Hunter
@ 2013-01-03 19:32                 ` Pratik Patel
  0 siblings, 0 replies; 17+ messages in thread
From: Pratik Patel @ 2013-01-03 19:32 UTC (permalink / raw)
  To: Jon Hunter
  Cc: linux@arm.linux.org.uk, linux-arm-msm, linus.walleij@linaro.org,
	Will Deacon, linux-kernel, magnus.p.persson@stericsson.com,
	david.rusling@linaro.org, arve@android.com, dsaxena@linaro.org,
	john.stultz@linaro.org, d-deao@ti.com,
	christian.bejram@stericsson.com,
	devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org

On Wed, Jan 02, 2013 at 02:00:21PM -0600, Jon Hunter wrote:
> 
> The code is largely based upon the existing cti helpers, which just had
> a cti_map_trigger() function. The use-case you described is not
> supported by the current helpers and so also not supported in my initial
> implementation (although we should add this). Hence, it would be
> probably best to split the cti_map_trigger() into two functions say,
> cti_map_triggerin() and cti_map_triggerout(), to map a trigger
> input/output to a channel.
> 

Yes, splitting the map function will provide more flexibility.
One possibility is to cue the enable and disable functions from
the mapping reference count for the CTI so that enable and
disable are internal only functions.

In this case the flow would look something like:

cti_get(cti1)
cti_map_triggerin(cti1) -> enable cti1
cti_get(cti2)
cti_map_triggerin(cti2) -> enable cti2
cti_map_triggerout(cti1) -> increment cti1 refcount

cti_unmap_triggerin(cti2) -> disable cti2
cti_put(cti2)
cti_unmap_triggerout(cti1) -> decrement cti1 refcount
cti_unmap_triggerin(cti1) -> disable cti1
cti_put(cti1)

With this, it is possible to push clock management inside enable
and disable as opposed to get and put though it would imply
enable being done before the actual mapping inside
cti_map_triggerin/out and disable after unmapping in
cti_unmap_triggerin/out. It might also mean cti_map_triggerin/out
and cti_unmap_triggerin/out having a non-atomic context call
semantics.

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: CoreSight framework and drivers
  2013-01-03 18:06                 ` Pratik Patel
@ 2013-01-07 11:58                   ` Will Deacon
  2013-01-16  0:14                     ` Pratik Patel
  0 siblings, 1 reply; 17+ messages in thread
From: Will Deacon @ 2013-01-07 11:58 UTC (permalink / raw)
  To: Pratik Patel
  Cc: Jon Hunter, linux@arm.linux.org.uk, linux-arm-msm@vger.kernel.org,
	linus.walleij@linaro.org, linux-kernel@vger.kernel.org,
	magnus.p.persson@stericsson.com, david.rusling@linaro.org,
	arve@android.com, dsaxena@linaro.org, john.stultz@linaro.org,
	d-deao@ti.com, christian.bejram@stericsson.com,
	devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org

On Thu, Jan 03, 2013 at 06:06:43PM +0000, Pratik Patel wrote:
> On Sun, Dec 23, 2012 at 11:32:39AM +0000, Will Deacon wrote:
> > On Fri, Dec 21, 2012 at 10:18:28PM +0000, Pratik Patel wrote:
> > > What user interface do you plan to provide for the CTI? Maybe
> > > something consistent with other CoreSight components in sysfs to
> > > allow users to enable, disable, map and unmap ???
> > > 
> > > Please let me know your thoughts.
> > 
> > Rather than have your current approach of dev nodes + sysfs config files for
> > each coresight device, I think it might be better to follow something closer
> > to ftrace and stick per-device directories under debugfs/coresight/. Then you
> > can have a pipe file and some config files in the same directory for each
> > component. You also don't need to do any mapping operations with this (just
> > post-process the stream directly).
> > 
> 
> Thanks for the suggestion. I had initially debated between debugfs
> and sysfs but chose sysfs + dev nodes since using device attributes
> relieves the drivers from manually managing directories and files,
> its taken care of by the device and sysfs layers. Moreover, since
> these are physical devices, device attributes made sense at the
> time.
> 
> The map and unmap I was referring to was for the CTI trigger
> mappings. Dev nodes are currently intended to provide the raw
> data collected in the sinks.
> 
> Whats the advantage in using debugfs here?

The main things I like about debugfs are (a) it's a text-driven interface
and easy to script with and (b) it matches what we do for ftrace.

Furthermore, it means that subtle differences between devices can be hidden
in the driver and not require different vendor tools for parsing the trace
data.

Will

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: CoreSight framework and drivers
  2013-01-07 11:58                   ` Will Deacon
@ 2013-01-16  0:14                     ` Pratik Patel
  2013-01-17 10:55                       ` Will Deacon
  0 siblings, 1 reply; 17+ messages in thread
From: Pratik Patel @ 2013-01-16  0:14 UTC (permalink / raw)
  To: Will Deacon
  Cc: Jon Hunter, linux@arm.linux.org.uk, linux-arm-msm@vger.kernel.org,
	linus.walleij@linaro.org, linux-kernel@vger.kernel.org,
	magnus.p.persson@stericsson.com, david.rusling@linaro.org,
	arve@android.com, dsaxena@linaro.org, john.stultz@linaro.org,
	d-deao@ti.com, christian.bejram@stericsson.com,
	devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org

On Mon, Jan 07, 2013 at 11:58:36AM +0000, Will Deacon wrote:
> On Thu, Jan 03, 2013 at 06:06:43PM +0000, Pratik Patel wrote:
> > On Sun, Dec 23, 2012 at 11:32:39AM +0000, Will Deacon wrote:
> > > On Fri, Dec 21, 2012 at 10:18:28PM +0000, Pratik Patel wrote:
> > > > What user interface do you plan to provide for the CTI? Maybe
> > > > something consistent with other CoreSight components in sysfs to
> > > > allow users to enable, disable, map and unmap ???
> > > > 
> > > > Please let me know your thoughts.
> > > 
> > > Rather than have your current approach of dev nodes + sysfs config files for
> > > each coresight device, I think it might be better to follow something closer
> > > to ftrace and stick per-device directories under debugfs/coresight/. Then you
> > > can have a pipe file and some config files in the same directory for each
> > > component. You also don't need to do any mapping operations with this (just
> > > post-process the stream directly).
> > > 
> > 
> > Thanks for the suggestion. I had initially debated between debugfs
> > and sysfs but chose sysfs + dev nodes since using device attributes
> > relieves the drivers from manually managing directories and files,
> > its taken care of by the device and sysfs layers. Moreover, since
> > these are physical devices, device attributes made sense at the
> > time.
> > 
> > The map and unmap I was referring to was for the CTI trigger
> > mappings. Dev nodes are currently intended to provide the raw
> > data collected in the sinks.
> > 
> > Whats the advantage in using debugfs here?
> 
> The main things I like about debugfs are (a) it's a text-driven interface
> and easy to script with and (b) it matches what we do for ftrace.
> 
> Furthermore, it means that subtle differences between devices can be hidden
> in the driver and not require different vendor tools for parsing the trace
> data.
> 
Sorry for the delay and maybe I am missing something but it seems
we can take care of such protocol or parsing/decoding differences
even with device nodes since that seems independent of the
interface used - per device debugfs attributes or per device
device nodes.

CoreSight trace solution is typically a SoC wide solution and
hence can get used by non-Linux processors or hardware. Using
debugfs would imply compiling it and exposing all the debug
knobs even if the use case was to use the CoreSight solution for
something that didn't need all that.

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: CoreSight framework and drivers
  2013-01-16  0:14                     ` Pratik Patel
@ 2013-01-17 10:55                       ` Will Deacon
  0 siblings, 0 replies; 17+ messages in thread
From: Will Deacon @ 2013-01-17 10:55 UTC (permalink / raw)
  To: Pratik Patel
  Cc: Jon Hunter, linux@arm.linux.org.uk, linux-arm-msm@vger.kernel.org,
	linus.walleij@linaro.org, linux-kernel@vger.kernel.org,
	magnus.p.persson@stericsson.com, david.rusling@linaro.org,
	arve@android.com, dsaxena@linaro.org, john.stultz@linaro.org,
	d-deao@ti.com, christian.bejram@stericsson.com,
	devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org

On Wed, Jan 16, 2013 at 12:14:59AM +0000, Pratik Patel wrote:
> On Mon, Jan 07, 2013 at 11:58:36AM +0000, Will Deacon wrote:
> > On Thu, Jan 03, 2013 at 06:06:43PM +0000, Pratik Patel wrote:
> > > Whats the advantage in using debugfs here?
> > 
> > The main things I like about debugfs are (a) it's a text-driven interface
> > and easy to script with and (b) it matches what we do for ftrace.
> > 
> > Furthermore, it means that subtle differences between devices can be hidden
> > in the driver and not require different vendor tools for parsing the trace
> > data.
> > 
> Sorry for the delay and maybe I am missing something but it seems
> we can take care of such protocol or parsing/decoding differences
> even with device nodes since that seems independent of the
> interface used - per device debugfs attributes or per device
> device nodes.

You seem to be arguing that the two interfaces are equivalent, in which case
I say that we should follow ftrace's lead and use debugfs for this...

...but I still maintain that debugfs is also far easier to work with from
userspace.

> CoreSight trace solution is typically a SoC wide solution and
> hence can get used by non-Linux processors or hardware. Using
> debugfs would imply compiling it and exposing all the debug
> knobs even if the use case was to use the CoreSight solution for
> something that didn't need all that.

Many debug features require debugfs to be compiled, so I don't buy that as a
show-stopping argument in favour of using dev nodes. I also think that exposing
all of the debug knobs is actually a good thing, given the low-level nature of
coresight devices (where we're offloading most of the knowledge to userspace
anyway).

Will

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2013-01-17 10:56 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1355858365-11849-1-git-send-email-pratikp@codeaurora.org>
     [not found] ` <20121219112314.GA26329@mudshark.cambridge.arm.com>
2012-12-19 21:06   ` CoreSight framework and drivers Pratik Patel
     [not found]   ` <50D1F37E.6000804@ti.com>
2012-12-19 21:24     ` Pratik Patel
2012-12-20 17:46       ` Jon Hunter
2012-12-20 19:51         ` Pratik Patel
2012-12-20 20:16           ` Jean Pihet
2012-12-21 22:12             ` Pratik Patel
2012-12-20 22:54           ` Jon Hunter
2012-12-20 23:40             ` Russell King - ARM Linux
2012-12-21 22:17               ` Pratik Patel
2012-12-21 22:18             ` Pratik Patel
2012-12-23 11:32               ` Will Deacon
2013-01-03 18:06                 ` Pratik Patel
2013-01-07 11:58                   ` Will Deacon
2013-01-16  0:14                     ` Pratik Patel
2013-01-17 10:55                       ` Will Deacon
2013-01-02 20:00               ` Jon Hunter
2013-01-03 19:32                 ` Pratik Patel

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).