lkml.org 
[lkml]   [2020]   [Apr]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [RFC PATCH] coresight: dynamic-replicator: Fix handling of multiple connections
Hi Suzuki,

On 2020-04-07 20:23, Suzuki K Poulose wrote:
> On 04/07/2020 02:56 PM, Sai Prakash Ranjan wrote:
>> Hi Suzuki,
>>
>> On 2020-04-07 18:38, Suzuki K Poulose wrote:
>>> On 04/07/2020 12:29 PM, Sai Prakash Ranjan wrote:
>>>> Hi Suzuki,
>>>>
>>>> Thanks for looking into this issue.
>>>>
>>>> On 2020-04-07 15:54, Suzuki K Poulose wrote:
>>>>> On 04/07/2020 10:46 AM, Sai Prakash Ranjan wrote:
>>>>>
>>>>> There seems to be two replicators back to back here. What is
>>>>> connected
>>>>> to the other output of both of them ? Are there any TPIUs ? What
>>>>> happens
>>>>> if you choose a sink on the other end of "swao_replicator" (ETB ?)
>>>>>
>>>>
>>>> The other outport of swao replicator is connected to EUD which is a
>>>> QCOM specific HW which can be used as a sink like USB.
>>>> And the other outport of other replicator(replicator_out) is
>>>> connected to
>>>> TPIU.
>>>>
>>>>> After boot, what do the idfilter registers read for both the
>>>>> replicators ?
>>>>>
>>>>
>>>> Added some prints in replicator_probe.
>>>>
>>>>   replicator probe ret=-517 devname=6046000.replicator idfilter0=0x0
>>>> idfilter1=0x0
>>>>   replicator probe ret=0 devname=6b06000.replicator idfilter0=0xff
>>>> idfilter1=0xff
>>>>   replicator probe ret=0 devname=6046000.replicator idfilter0=0xff
>>>> idfilter1=0xff
>>>
>>> Curious to see how the idfilterX is set to 0:
>>>      if that is never used.
>>>         Or
>>>      if the user doesn't reset it back to 0xff.
>>>
>>
>> For both replicators, the default value seems to be 0x0.
>>
>>  replicator probe in res ret=0 devname=6046000.replicator
>> idfilter0=0x0 idfilter1=0x0
>>  replicator probe ret=-517 devname=6046000.replicator idfilter0=0x0
>> idfilter1=0x0
>>  replicator probe in res ret=0 devname=6b06000.replicator
>> idfilter0=0x0 idfilter1=0x0
>>  replicator probe ret=0 devname=6b06000.replicator idfilter0=0xff
>> idfilter1=0xff
>>  replicator probe in res ret=0 devname=6046000.replicator
>> idfilter0=0x0 idfilter1=0x0
>>  replicator probe ret=0 devname=6046000.replicator idfilter0=0xff
>> idfilter1=0xff
>
> I am not sure how you have added the debugs, but it looks like the
> drivers set 0xff for both the port filters on a successful probe.
>

Yes, thats done by replicator_reset in probe right? Below is the diff:

@@ -242,6 +244,9 @@ static int replicator_probe(struct device *dev,
struct resource *res)
}
drvdata->base = base;
desc.groups = replicator_groups;
+ pr_info("replicator probe in res ret=%d devname=%s
idfilter0=%#lx idfilter1=%#lx\n",
+ ret, dev_name(dev), (readl_relaxed(base +
REPLICATOR_IDFILTER0)),
+ (readl_relaxed(base + REPLICATOR_IDFILTER1)));
}

dev_set_drvdata(dev, drvdata);
@@ -272,6 +277,12 @@ static int replicator_probe(struct device *dev,
struct resource *res)
out_disable_clk:
if (ret && !IS_ERR_OR_NULL(drvdata->atclk))
clk_disable_unprepare(drvdata->atclk);
+
+ if (res)
+ pr_info("replicator probe ret=%d devname=%s
idfilter0=%#lx idfilter1=%#lx\n",
+ ret, dev_name(dev), (readl_relaxed(base +
REPLICATOR_IDFILTER0)),
+ (readl_relaxed(base + REPLICATOR_IDFILTER1)));
+
return ret;
}

>>> Does your test ever touch EUD (enable the port for EUD at
>>> swao-replicator) ? What are the values before you run your test ?
>>>
>>>
>>
>> No, we do not use EUD, downstream it is used as dummy sink.
>> And I just try to select the ETR as the sink and enable ETM0 as the
>> trace source.
>>
>> echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
>> echo 1 > /sys/bus/coresight/devices/etm0/enable_source
>>
>> Also I see the KASAN warning but that seems like some other issue.
>>
>
> Does your funnel have sparse input described ? I think we have an
> issue with the "refcnt" tracking for funnels (especially). When we
> have a sparse input ports described (ie. if only input ports 0, 3,
> 5 are described to protect the secure side connections), we could
> end up accessing beyond the memory allocated for csdev->refcnts.
> i.e, csdev->pdata->nr_inport = 3, and we could access
> csdev->refcnts[5],
> while sizeof(csdev->refcnts) = sizeof(atomic_t) * 3.
>
> I will send a patch.
>

Thanks, I can test it out.

Thanks,
Sai

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

\
 
 \ /
  Last update: 2020-04-07 17:19    [W:0.071 / U:2.728 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site