Re: [PATCH v4 03/11] coresight-tpdm: Initialize DSB subunit configuration

[Suzuki K Poulose <suzuki.poulose@arm.com>]

On 25/05/2023 09:12, Tao Zhang wrote:
> 
> On 5/23/2023 9:42 PM, Suzuki K Poulose wrote:
>> On 27/04/2023 10:00, Tao Zhang wrote:
>>> DSB is used for monitoring “events”. Events are something that
>>> occurs at some point in time. It could be a state decode, the
>>> act of writing/reading a particular address, a FIFO being empty,
>>> etc. This decoding of the event desired is done outside TPDM.
>>> DSB subunit need to be configured in enablement and disablement.
>>> A struct that specifics associated to dsb dataset is needed. It
>>> saves the configuration and parameters of the dsb datasets. This
>>> change is to add this struct and initialize the configuration of
>>> DSB subunit.
>>>
>>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>

...

>>> + * dataset types. It covers Basic Counts(BC), Tenure Counts(TC),
>>> + * Continuous Multi-Bit(CMB), Multi-lane CMB(MCMB) and Discrete Single
>>> + * Bit(DSB). This function will initialize the configuration according
>>> + * to the dataset type supported by the TPDM.
>>> + */
>>>   static void __tpdm_enable(struct tpdm_drvdata *drvdata)
>>>   {
>>>       CS_UNLOCK(drvdata->base);
>>> @@ -110,15 +144,24 @@ static const struct coresight_ops tpdm_cs_ops = {
>>>       .source_ops    = &tpdm_source_ops,
>>>   };
>>>   -static void tpdm_init_default_data(struct tpdm_drvdata *drvdata)
>>> +static int tpdm_datasets_setup(struct tpdm_drvdata *drvdata)
>>>   {
>>>       u32 pidr;
>>>   -    CS_UNLOCK(drvdata->base);
>>>       /*  Get the datasets present on the TPDM. */
>>>       pidr = readl_relaxed(drvdata->base + CORESIGHT_PERIPHIDR0);
>>>       drvdata->datasets |= pidr & GENMASK(TPDM_DATASETS - 1, 0);
>>> -    CS_LOCK(drvdata->base);
>>
>> Why are we removing the CS_{UN,}LOCK here ?
> 
> CS_UNLOCK is used before writing data to Coresight registers. Here this 
> function
> 
> doesn't need to write data to any registers, so I remove the 
> CS_{UN,}LOCK here.

Please make this a separate patch to avoid confusing and keep it at the
beginning of the series.

Suzuki