From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 303ADEB64DD for ; Mon, 14 Aug 2023 09:46:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232942AbjHNJpg (ORCPT ); Mon, 14 Aug 2023 05:45:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46648 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233998AbjHNJpN (ORCPT ); Mon, 14 Aug 2023 05:45:13 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 820A3CC; Mon, 14 Aug 2023 02:45:11 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 7EEB92F4; Mon, 14 Aug 2023 02:45:53 -0700 (PDT) Received: from [10.57.90.230] (unknown [10.57.90.230]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7C27E3F6C4; Mon, 14 Aug 2023 02:45:08 -0700 (PDT) Message-ID: Date: Mon, 14 Aug 2023 10:45:06 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH v7 06/13] coresight-tpdm: Add reset node to TPDM node To: Tao Zhang , Mathieu Poirier , Alexander Shishkin , Konrad Dybcio , Mike Leach , Rob Herring , Krzysztof Kozlowski Cc: Jinlong Mao , Greg Kroah-Hartman , coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Tingwei Zhang , Yuanfang Zhang , Trilok Soni , Hao Zhang , linux-arm-msm@vger.kernel.org, andersson@kernel.org References: <1690269353-10829-1-git-send-email-quic_taozha@quicinc.com> <1690269353-10829-7-git-send-email-quic_taozha@quicinc.com> <1dba2258-e447-d019-da38-067bf776922a@quicinc.com> From: Suzuki K Poulose In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org 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 >>>>> --- >>>>>   .../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//reset >>>>> +Date:        March 2023 >>>>> +KernelVersion    6.5 >>>> >>>> >>>>> +Contact:    Jinlong Mao (QUIC) , Tao >>>>> Zhang (QUIC) >>>>> +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