Messages in this thread | | | Date | Mon, 14 Aug 2023 10:45:06 +0100 | Subject | Re: [PATCH v7 06/13] coresight-tpdm: Add reset node to TPDM node | From | Suzuki K Poulose <> |
| |
On 14/08/2023 08:08, Tao Zhang wrote: > > On 8/13/2023 11:38 PM, Tao Zhang wrote: >> >> On 8/9/2023 2:35 PM, Tao Zhang wrote: >>> >>> On 8/7/2023 5:36 PM, Suzuki K Poulose wrote: >>>> On 25/07/2023 08:15, 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. >>>>> >>>>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com> >>>>> --- >>>>> .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 10 ++++++++++ >>>>> drivers/hwtracing/coresight/coresight-tpdm.c | 22 >>>>> ++++++++++++++++++++++ >>>>> 2 files changed, 32 insertions(+) >>>>> >>>>> diff --git >>>>> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >>>>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >>>>> index 4a58e64..dbc2fbd0 100644 >>>>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >>>>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >>>>> @@ -11,3 +11,13 @@ Description: >>>>> Accepts only one of the 2 values - 1 or 2. >>>>> 1 : Generate 64 bits data >>>>> 2 : Generate 32 bits data >>>>> + >>>>> +What: /sys/bus/coresight/devices/<tpdm-name>/reset >>>>> +Date: March 2023 >>>>> +KernelVersion 6.5 >>>> >>>> >>>>> +Contact: Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao >>>>> Zhang (QUIC) <quic_taozha@quicinc.com> >>>>> +Description: >>>>> + (Write) Reset the dataset of the tpdm, and disable the tpdm. >>>> >>>> Please fix this, we don't disable TPDM. If it only ever resets the >>>> datasets, please could we rename this as such ? >>>> >>>> i.e., reset_dataset or reset_dsb_data ? >>> Sure, I will update this in the next patch series. >>>> >>>>> + >>>>> + Accepts only one value - 1. >>>>> + 1 : Reset the dataset of the tpdm >>>>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c >>>>> b/drivers/hwtracing/coresight/coresight-tpdm.c >>>>> index 52aa48a6..acc3eea 100644 >>>>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c >>>>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c >>>>> @@ -159,6 +159,27 @@ static int tpdm_datasets_setup(struct >>>>> tpdm_drvdata *drvdata) >>>>> return 0; >>>>> } >>>>> +static ssize_t reset_store(struct device *dev, >>>>> + struct device_attribute *attr, >>>>> + const char *buf, >>>>> + size_t size) >>>> >>>> Minor nit: alignment ? Could we have something like : >>>> >>>> static ssize_t reset_store(struct device *dev, >>>> struct device_attribute *attr, >>>> const char *buf, >>>> size_t size) >>>> >>> I will update this in the next patch series. >>> >>> >>> Best, >>> >>> Tao >> >> Hi Suzuki, >> >> >> With regards to the parameters alignment for the function, could you >> >> kindly remind me the rule of alignment? I'm using the thunderbird >> >> mail client and I'm not sure if the alignment I see is what you expect.
It is not about the mail client, I am looking at the code with this series applied to the code.
>> >> If the rule can be specified, I can align all the function parameters in >> >> this patch series according to this rule. >> >> >> Best, >> >> Tao > > Hi Suzuki, > > > Can I follow the following rules to handle the similar case? > > "Descendants are always substantially shorter than the parent and are > placed substantially to the right. >
> A very commonly used style is to align descendants to a function open > parenthesis."
Yes, this is what I expect.
Suzuki
> > > Best, > > Tao > >> >>> >>>> >>>>> +{ >>>>> + 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); >>>>> + tpdm_reset_datasets(drvdata); >>>>> + spin_unlock(&drvdata->spinlock); >>>>> + >>>>> + return size; >>>>> +} >>>>> +static DEVICE_ATTR_WO(reset); >>>>> + >>>>> /* >>>>> * value 1: 64 bits test data >>>>> * value 2: 32 bits test data >>>>> @@ -199,6 +220,7 @@ static ssize_t integration_test_store(struct >>>>> device *dev, >>>>> static DEVICE_ATTR_WO(integration_test); >>>>> static struct attribute *tpdm_attrs[] = { >>>>> + &dev_attr_reset.attr, >>>>> &dev_attr_integration_test.attr, >>>>> NULL, >>>>> }; >>>> >>>> Suzuki >>>> >>> _______________________________________________ >>> CoreSight mailing list -- coresight@lists.linaro.org >>> To unsubscribe send an email to coresight-leave@lists.linaro.org >> _______________________________________________ >> CoreSight mailing list -- coresight@lists.linaro.org >> To unsubscribe send an email to coresight-leave@lists.linaro.org
| |