Re: [PATCH v4 00/30] coresight: Support for ACPI bindings

[Mathieu Poirier]

Good day,

On Tue, May 28, 2019 at 01:19:24PM +0800, Leo Yan wrote:
> Hi Suzuki,
> 
> On Wed, May 22, 2019 at 11:34:33AM +0100, Suzuki K Poulose wrote:
> > This series adds the support for CoreSight devices on ACPI based
> > platforms. The device connections are encoded as _DSD graph property[0],
> > with CoreSight specific extensions to indicate the direction of data
> > flow as described in [1]. Components attached to CPUs are listed
> > as child devices of the corresponding CPU, removing explicit links
> > to the CPU like we do in the DT.
> > 
> > The majority of the series cleans up the driver and prepares the subsystem
> > for platform agnostic firwmare probing, naming scheme, searching etc.
> > 
> > We introduce platform independent helpers to parse the platform supplied
> > information. Thus we rename the platform handling code from:
> > 	of_coresight.c  => coresight-platform.c
> > 
> > The CoreSight driver creates shadow devices that appear on the Coresight
> > bus, in addition to the real devices (e.g, AMBA bus devices). The name
> > of these devices match the real device. This makes the device name
> > a bit cryptic for ACPI platform. So this series also introduces a generic
> > platform agnostic device naming scheme for the shadow Coresight devices.
> > Towards this we also make changes to the way we lookup devices to resolve
> > the connections, as we can't use the names to identify the devices. So,
> > we use the "fwnode_handle" of the real device for the device lookups.
> > Towards that we clean up the drivers to keep track of the "CoreSight"
> > device rather than the "real" device. However, all real operations,
> > like DMA allocation, Power management etc. must be performed on
> > the real device which is the parent of the shadow device.
> > 
> > Finally we add the support for parsing the ACPI platform data. The power
> > management support is missing in the ACPI (and this is not specific to
> > CoreSight). The firmware must ensure that the respective power domains
> > are turned on.
> > 
> > Applies on v5.2-rc1
> > 
> > Tested on a Juno-r0 board with ACPI bindings patch (Patch 31/30) added on
> > top of [2]. You would need to make sure that the debug power domain is
> > turned on before the Linux kernel boots. (e.g, connect the DS-5 to the
> > Juno board while at UEFI). arm32 code is only compile tested.
> 
> After I applied this patch set, I found all device names under
> '/sys/bus/event_source/devices/cs_etm/sinks/' have been changed as
> below on my DB410c board:
> # ls /sys/bus/event_source/devices/cs_etm/sinks/
> tmc_etf0  tmc_etr0  tpiu0

Yes, that is the expected behavior.

> 
> This leads to below command failure when open PMU device:
> # perf record -e cs_etm/@826000.etr/ --per-thread uname
> failed to set sink "826000.etr" on event cs_etm/@826000.etr/ with 2 (No such file or directory)

Correct.

> 
> I must use below command so that perf can match string with the
> device name under '/sys/bus/event_source/devices/cs_etm/sinks/':
> # perf record -e cs_etm/@tmc_etr0/ --per-thread uname

Correct.

> 
> Seems to me, this is an unexpected change and when I worked on the
> patch set v2, IIRC that version still can use '826000.etr' to open PMU
> device.

Correct - v2 did not address the new naming convention for devices present under
'event_source', something that was corrected in v3.

 
> Please help confirm for this.  Thanks!
> 
> Leo.