Messages in this thread | | | Date | Tue, 24 May 2022 15:15:37 +0800 | Subject | Re: [PATCH v7 04/10] coresight-tpdm: Add DSB dataset support | From | Jinlong Mao <> |
| |
Hi Suzuki,
On 5/23/2022 5:24 PM, Suzuki K Poulose wrote: > On 23/05/2022 10:11, Suzuki K Poulose wrote: >> Hi >> >> On 09/05/2022 14:39, Mao Jinlong wrote: >>> TPDM serves as data collection component for various dataset types. >>> DSB(Discrete Single Bit) is one of the dataset types. DSB subunit >>> can be enabled for data collection by writing 1 to the first bit of >>> DSB_CR register. This change is to add enable/disable function for >>> DSB dataset by writing DSB_CR register. >> >> The patch looks good to me, except for some minor comment below. >> >>> >>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com> >>> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> >>> --- >>> drivers/hwtracing/coresight/coresight-tpdm.c | 58 >>> ++++++++++++++++++++ >>> drivers/hwtracing/coresight/coresight-tpdm.h | 23 ++++++++ >>> 2 files changed, 81 insertions(+) >>> >>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c >>> b/drivers/hwtracing/coresight/coresight-tpdm.c >>> index 6a4e2a35053d..70df888ac565 100644 >>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c >>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c >>> @@ -20,7 +20,28 @@ >>> DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm"); >>> +static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata) >>> +{ >>> + u32 val; >>> + >>> + /* Set the enable bit of DSB control register to 1 */ >>> + val = readl_relaxed(drvdata->base + TPDM_DSB_CR); >>> + val |= TPDM_DSB_CR_ENA; >>> + writel_relaxed(val, drvdata->base + TPDM_DSB_CR); >>> +} >>> + > > > >>> /* TPDM enable operations */ >>> +static void _tpdm_enable(struct tpdm_drvdata *drvdata) > > >>> /* TPDM disable operations */ >>> +static void _tpdm_disable(struct tpdm_drvdata *drvdata) > > Missed this. The general convention is to use: > > __tpdm_disable() > __tpdm_enable(); I will address your comments. > > So, please switch to the names above. > > Suzuki
| |