lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 27 Mar 2023 15:11:04 +0800
From:   Tao Zhang <quic_taozha@...cinc.com>
To:     Suzuki K Poulose <suzuki.poulose@....com>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Konrad Dybcio <konradybcio@...il.com>,
        Mike Leach <mike.leach@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
CC:     Jinlong Mao <quic_jinlmao@...cinc.com>,
        Leo Yan <leo.yan@...aro.org>,
        "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
        <coresight@...ts.linaro.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
        Tingwei Zhang <quic_tingweiz@...cinc.com>,
        Yuanfang Zhang <quic_yuanfang@...cinc.com>,
        Trilok Soni <quic_tsoni@...cinc.com>,
        Hao Zhang <quic_hazha@...cinc.com>,
        <linux-arm-msm@...r.kernel.org>,
        Bjorn Andersson <andersson@...nel.org>
Subject: Re: [PATCH v3 04/11] coresight-tpdm: Add reset node to TPDM node

Hi Suzuki,

On 3/23/2023 10:48 PM, Suzuki K Poulose wrote:
> On 23/03/2023 14:41, Suzuki K Poulose wrote:
>> On 23/03/2023 06:04, Tao Zhang wrote:
>>> TPDM device need a node to reset the configurations and status of
>>> it. This change provides a node to reset the configurations and
>>> disable the TPDM if it has been enabled.
>
> Please justify why this "do everything" magic knob is required
> when there are tunables for individual controls in the later
> patches.

We want to have a single knob that resets all the datasets, which saves the

need to configure the individual controls one by one. Since it is often 
necessary

to reset all the datasets, this knob will be more user-friendly.


Tao

>
> Suzuki
>
>>>
>>> Signed-off-by: Tao Zhang <quic_taozha@...cinc.com>
>>> ---
>>>   drivers/hwtracing/coresight/coresight-tpdm.c | 28 
>>> ++++++++++++++++++++++++++++
>>>   1 file changed, 28 insertions(+)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c 
>>> b/drivers/hwtracing/coresight/coresight-tpdm.c
>>> index 5e1e2ba..104638d 100644
>>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
>>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
>>> @@ -161,6 +161,33 @@ static void tpdm_datasets_setup(struct 
>>> tpdm_drvdata *drvdata)
>>>       drvdata->datasets |= pidr & GENMASK(TPDM_DATASETS - 1, 0);
>>>   }
>>> +static ssize_t reset_store(struct device *dev,
>>> +                      struct device_attribute *attr,
>>> +                      const char *buf,
>>> +                      size_t size)
>>> +{
>>> +    int ret = 0;
>>> +    unsigned long val;
>>> +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>> +
>>> +    ret = kstrtoul(buf, 10, &val);
>>> +    if (ret || val != 1)
>>> +        return -EINVAL;
>>> +
>>> +    spin_lock(&drvdata->spinlock);
>>> +    /* Reset all datasets to ZERO, and init the default data*/
>>> +    tpdm_init_datasets(drvdata);
>>
>> With the suggested rename in the previous patch, you wouldn't need
>> a comment here.
>>
>>> +
>>> +    spin_unlock(&drvdata->spinlock);
>>> +
>>
>>
>>> +    /* Disable tpdm if enabled */
>>> +    if (drvdata->enable)
>>> +        coresight_disable(drvdata->csdev);
>>
>> Couldn't this be done via disable_source ? Please don't overload
>> the sysfs handle.
>>
>>> +
>>> +    return size;
>>> +}
>>> +static DEVICE_ATTR_WO(reset);
>>
>> Documentation for the sysfs node please ?
>>
>> Suzuki
>>
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ