[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJ9a7Vh6McpjtyRuBDPSYMWec0TuWkXeOLBOpKvv8tWUU0sV2A@mail.gmail.com>
Date: Fri, 25 Jan 2019 10:37:45 +0000
From: Mike Leach <mike.leach@...aro.org>
To: Sai Prakash Ranjan <saiprakash.ranjan@...eaurora.org>
Cc: linux@...linux.org.uk,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
coresight@...ts.linaro.org, linux-kernel@...r.kernel.org,
Mathieu Poirier <mathieu.poirier@...aro.org>,
Suzuki K Poulose <suzuki.poulose@....com>,
Rajendra Nayak <rnayak@...eaurora.org>,
Vivek Gautam <vivek.gautam@...eaurora.org>,
Sibi Sankar <sibis@...eaurora.org>
Subject: Re: [RESEND PATCH v3 1/2] drivers: amba: Updates to component
identification for driver matching.
Hello Sai
On Fri, 25 Jan 2019 at 07:20, Sai Prakash Ranjan
<saiprakash.ranjan@...eaurora.org> 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:
> > 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.
> >
>
> [..]
>
> > +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 <saiprakash.ranjan@...eaurora.org>
>
> 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
Powered by blists - more mailing lists