Messages in this thread | | | Date | Mon, 11 May 2020 20:11:52 +0530 | From | Sai Prakash Ranjan <> | Subject | Re: [PATCH] coresight: dynamic-replicator: Fix handling of multiple connections |
| |
Hi Suzuki,
On 2020-05-11 20:00, Suzuki K Poulose wrote: > On 05/11/2020 03:16 PM, Sai Prakash Ranjan wrote: >> Hi Mike, >> >> On 2020-05-11 16:44, Mike Leach wrote: >> [...] >> >>>> >>>> I checked with the debug team and there is a limitation with >>>> the replicator(swao_replicator) in the AOSS group where it >>>> loses the idfilter register context when the clock is disabled. >>>> This is not just in SC7180 SoC but also reported on some latest >>>> upcoming QCOM SoCs as well and will need to be taken care in >>>> order to enable coresight on these chipsets. >>>> >>>> Here's what's happening - After the replicator is initialized, >>>> the clock is disabled in amba_pm_runtime_suspend() as a part of >>>> pm runtime workqueue with the assumption that there will be no >>>> loss of context after the replicator is initialized. But it doesn't >>>> hold good with the replicators with these unfortunate limitation >>>> and the idfilter register context is lost. >>>> >>>> [ 5.889406] amba_pm_runtime_suspend devname=6b06000.replicator >>>> ret=0 >>>> [ 5.914516] Workqueue: pm pm_runtime_work >>>> [ 5.918648] Call trace: >>>> [ 5.921185] dump_backtrace+0x0/0x1d0 >>>> [ 5.924958] show_stack+0x2c/0x38 >>>> [ 5.928382] dump_stack+0xc0/0x104 >>>> [ 5.931896] amba_pm_runtime_suspend+0xd8/0xe0 >>>> [ 5.936469] __rpm_callback+0xe0/0x140 >>>> [ 5.940332] rpm_callback+0x38/0x98 >>>> [ 5.943926] rpm_suspend+0xec/0x618 >>>> [ 5.947522] rpm_idle+0x5c/0x3f8 >>>> [ 5.950851] pm_runtime_work+0xa8/0xc0 >>>> [ 5.954718] process_one_work+0x1f8/0x4c0 >>>> [ 5.958848] worker_thread+0x50/0x468 >>>> [ 5.962623] kthread+0x12c/0x158 >>>> [ 5.965957] ret_from_fork+0x10/0x1c >>>> >>>> This is a platform/SoC specific replicator issue, so we can either >>>> introduce some DT property for replicators to identify which >>>> replicator >>>> has this limitation, check in replicator_enable() and reset the >>>> registers >>>> or have something like below diff to check the idfilter registers in >>>> replicator_enable() and then reset with clear comment specifying >>>> it’s >>>> the >>>> hardware limitation on some QCOM SoCs. Please let me know your >>>> thoughts >>>> on >>>> this? >>>> >> >> Sorry for hurrying up and sending the patch - >> https://lore.kernel.org/patchwork/patch/1239923/. >> I will send v2 based on further feedbacks here or there. >> >>> >>> 1) does this replicator part have a unique ID that differs from the >>> standard ARM designed replicators? >>> If so perhaps link the modification into this. (even if the part no >>> in >>> PIDR0/1 is the same the UCI should be different for a different >>> implementation) >>> >> >> pid=0x2bb909 for both replicators. So part number is same. >> UCI will be different for different implementation(QCOM maybe >> different from ARM), >> but will it be different for different replicators under the same >> impl(i.e., on QCOM). > > May be use PIDR4.DES_2 to match the Implementor and apply the work > around for all QCOM replicators ? > > To me that sounds the best option. >
Ok we can do this as well, but just for my understanding, why do we need to reset replicators in replicator_probe() and not in replicator_enable()? Are we accessing anything before we enable replicators?
Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
| |