Hello Sai
On Fri, 25 Jan 2019 at 07:20, Sai Prakash Ranjan
[off-list ref] wrote:

Hi Mike,

Thanks for the patch.

BTW somehow I can't find the latest series in my inbox, so commenting
on this here.

Mathieu pointed me to this patch series.This solves CPU debug module
sharing same PID as ETM on MSM8996. I will be posting patch for CPU
debug UCI table soon.

But please find my one comment inline.

On 12/19/2018 3:29 AM, Mike Leach wrote:

quoted

The CoreSight specification (ARM IHI 0029E), updates the ID register
requirements for components on an AMBA bus, to cover both traditional
ARM Primecell type devices, and newer CoreSight and other components.

The Peripheral ID (PID) / Component ID (CID) pair is extended in certain
cases to uniquely identify components. CoreSight components related to
a single function can share Peripheral ID values, and must be further
identified using a Unique Component Identifier (UCI). e.g. the ETM, CTI,
PMU and Debug hardware of the A35 all share the same PID.

[..]

quoted

+static const struct amba_id *
+amba_lookup(const struct amba_id *table, struct amba_device *dev)
+{
      while (table->mask) {
-             ret = (dev->periphid & table->mask) == table->id;
-             if (ret)
-                     break;
+             if (((dev->periphid & table->mask) == table->id) &&
+                     ((dev->cid != CORESIGHT_CID) ||
+                      (amba_cs_uci_id_match(table, dev))))

Shouldn't the check be (dev->cid == CORESIGHT_CID) ?
Without this STM fails to probe on both SDM845 and MSM8996.

I believe the test is correct

To expand the logic here:

if  (dev->periphid & table->mask) == table->id) {
      //** match on peripheral ID at this point
     if (CID != CORESIGHT_ID)
               return table; //** not coresight - match on peripheral ID only
     //** or
     if (amba_cs_uci_id_match() )
                return table;  //** is coresight - match on UCI if
available - otherwise peripheral ID only;

However - looking at the coresight STM driver - this is using the
private .data field in the amba_id for a name - which I had not
spotted before.
I will have to revisit this patchset to fix either the amba id struct
or the method for getting the uci data into the amba_id.data which
allows for multiple uses.

Regards

Mike

With this,

Tested-by: Sai Prakash Ranjan <redacted>

Thanks,
Sai

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel