Date:	Fri, 21 Dec 2012 14:18:28 -0800
From:	Pratik Patel <pratikp@...eaurora.org>
To:	Jon Hunter <jon-hunter@...com>
Cc:	"linux@....linux.org.uk" <linux@....linux.org.uk>,
	linux-arm-msm@...r.kernel.org,
	"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
	Will Deacon <will.deacon@....com>,
	linux-kernel@...r.kernel.org,
	"magnus.p.persson@...ricsson.com" <magnus.p.persson@...ricsson.com>,
	"david.rusling@...aro.org" <david.rusling@...aro.org>,
	"arve@...roid.com" <arve@...roid.com>,
	"dsaxena@...aro.org" <dsaxena@...aro.org>,
	"john.stultz@...aro.org" <john.stultz@...aro.org>,
	"d-deao@...com" <d-deao@...com>,
	"christian.bejram@...ricsson.com" <christian.bejram@...ricsson.com>,
	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: CoreSight framework and drivers

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists