Received: by 2002:a05:7412:bbc7:b0:fc:a2b0:25d7 with SMTP id kh7csp74078rdb; Thu, 1 Feb 2024 02:28:01 -0800 (PST) X-Google-Smtp-Source: AGHT+IEceeKt0G4fbzFJj3c1cW3/ZhbbJEE5IPrEQ810UICk92xUet4kEeR5ruROn3s51bGVd/a5 X-Received: by 2002:a17:902:ce91:b0:1d8:ffe8:125c with SMTP id f17-20020a170902ce9100b001d8ffe8125cmr6330575plg.24.1706783281234; Thu, 01 Feb 2024 02:28:01 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1706783281; cv=pass; d=google.com; s=arc-20160816; b=oNVFcdefIRc6HLQGc86xLxhkcsaLDtCpRkbHo7LhLgfW7SBtThQFtVWdEoY9QTT74Y dd338kPXyTAeVK+z04by8DPtbdM1XeB4+OILRcxBgz6j9lffzWvj8TJ1rsZ3+RT1BmOP mEH3P67CycrAdT3yABMYQEShzxlG+MBtFiUL0EquSZ8ZmAgluAWgRTyE5cXf0YLQkznO Mknuzm0Ccp55p+ZPBuN2P9MZOTgByhUcoC5C2g3pfhxAOxbQ3ZPp/fW3fikbJ4XrYWtV 4V1Ev7VLa9ZXZjYkIXpIiWAz5bmSj+bYzaNSwcQbY/OZ4WXoDkaRXglsMHDKz5T1P/rA 6wDA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:date:message-id; bh=I7O8/oCg/2/eONnx14qhdBy3Z0JsmdxvQNAFFSCMkHY=; fh=BEzgjp5PYSJGgIYiNFJD8rEnlrQ2cGhkRlN0Eaj8pJ8=; b=C79yz8jBbA4D9zNlGsRY11gVO4GD3OfbZPlJLxop4t04m5v89GSusHaZP1ULBid3b6 kutPTLLTKsnh38l+PcbFKpMfuReHC/mBb8vEnvg+uXJa++UZwIELd3z53P3UQZpm98OH Y2bQsT3kG7EE+z4PNnpHEQQCYydhhjCHj+5fVl3xTWCRx9fSaIuN1uX+6KftC8+wkuXF XDKV6aWT8TjbbFz45NGOk9HE0432E78S/ihGReQVYHFq5rggm5DPET1enztJvHq0d1oy dlmzlc+21EbnKs8xZVEpGADQW2vOMgZhIyMkvWv6H95B1rkd8gpFYu0YARR7o+J131U9 DGqQ==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; arc=pass (i=1 spf=pass spfdomain=arm.com dmarc=pass fromdomain=arm.com); spf=pass (google.com: domain of linux-kernel+bounces-47961-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) smtp.mailfrom="linux-kernel+bounces-47961-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com X-Forwarded-Encrypted: i=1; AJvYcCVvigQPJ8PkExmHfPI7YxWkKiThu07lBZiAzF+XlG4gguiF5nB1f8PO8g+0Zmvz6tmy5OUngBtNIIBoVUgvUKI6LBASaaOP3UgragBeig== Return-Path: Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [139.178.88.99]) by mx.google.com with ESMTPS id a20-20020a170902b59400b001d75eef04e1si11529529pls.73.2024.02.01.02.28.01 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 01 Feb 2024 02:28:01 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-47961-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99; Authentication-Results: mx.google.com; arc=pass (i=1 spf=pass spfdomain=arm.com dmarc=pass fromdomain=arm.com); spf=pass (google.com: domain of linux-kernel+bounces-47961-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) smtp.mailfrom="linux-kernel+bounces-47961-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 5B7EE291D3B for ; Thu, 1 Feb 2024 10:26:58 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 2153415B96C; Thu, 1 Feb 2024 10:26:50 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 593524DA06; Thu, 1 Feb 2024 10:26:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706783209; cv=none; b=jT1w2vZuzCuSCU5Nvx3561iob74GK5Q3ID6MBt2j+b6H11HwjwDTwoDU1PCDqIhRw0ccOuEvLOXGAqQ8Lr+tlCQN9QjX2lKYfA5K1ziE2IC3xvJRA4h0d/htZq3Xf/OxgcMXHz36VDT6xdWkRQCUr8HuIcJyung6LhSKQS78o3k= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706783209; c=relaxed/simple; bh=1Yr/AHhB6XmjVqcxBy7N7dT+IhyHnjIGtbEVFp9eSiM=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=HbEWmAhwOOC9/eTH2O34BDhJFuhdEnAuhYk99tB33HuIUbrTWTyR4U4OtjhOg73rttUMidbkGPN12+CCV2nSFXspHD5tOaRCW0t0Lnyetppoz5PTdqqGIECcqIQ4DA/JT4pq1zws3yRaOE52siY/wZsSKIB8kux6JnpUwWEvEyY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com 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 32AC8DA7; Thu, 1 Feb 2024 02:27:29 -0800 (PST) Received: from [10.1.197.1] (ewhatever.cambridge.arm.com [10.1.197.1]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 45A733F738; Thu, 1 Feb 2024 02:26:42 -0800 (PST) Message-ID: <812bd339-4728-40ee-b19e-c8a04cc740de@arm.com> Date: Thu, 1 Feb 2024 10:26:40 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 05/10] coresight-tpda: Add support to configure CMB element Content-Language: en-US To: Tao Zhang , Mathieu Poirier , Alexander Shishkin , Konrad Dybcio , Mike Leach , Rob Herring , Krzysztof Kozlowski Cc: Jinlong Mao , Leo Yan , 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 , Song Chai , linux-arm-msm@vger.kernel.org, andersson@kernel.org References: <1706605366-31705-1-git-send-email-quic_taozha@quicinc.com> <1706605366-31705-6-git-send-email-quic_taozha@quicinc.com> <6ccb98f2-2f68-45db-9941-1c7b05da84d0@arm.com> <6fff5991-01ed-44ea-aa08-9f302d2465e8@quicinc.com> <1f5a7c7b-56de-4f19-9d48-652ae6efe50f@arm.com> <9fca774e-3519-4d0c-ba61-6b84965f36c2@quicinc.com> From: Suzuki K Poulose In-Reply-To: <9fca774e-3519-4d0c-ba61-6b84965f36c2@quicinc.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 01/02/2024 02:25, Tao Zhang wrote: > > On 1/31/2024 6:02 PM, Suzuki K Poulose wrote: >> On 31/01/2024 01:39, Tao Zhang wrote: >>> >>> On 1/30/2024 8:35 PM, Suzuki K Poulose wrote: >>>> On 30/01/2024 09:02, Tao Zhang wrote: >>>>> Read the CMB element size from the device tree. Set the register >>>>> bit that controls the CMB element size of the corresponding port. >>>>> >>>>> Reviewed-by: James Clark >>>>> Signed-off-by: Tao Zhang >>>>> Signed-off-by: Mao Jinlong >>>>> --- >>>>>   drivers/hwtracing/coresight/coresight-tpda.c | 123 >>>>> +++++++++++-------- >>>>>   drivers/hwtracing/coresight/coresight-tpda.h |   6 + >>>>>   2 files changed, 79 insertions(+), 50 deletions(-) >>>>> >>>>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c >>>>> b/drivers/hwtracing/coresight/coresight-tpda.c >>>>> index 4ac954f4bc13..fcddff3ded81 100644 >>>>> --- a/drivers/hwtracing/coresight/coresight-tpda.c >>>>> +++ b/drivers/hwtracing/coresight/coresight-tpda.c >>>>> @@ -18,6 +18,7 @@ >>>>>   #include "coresight-priv.h" >>>>>   #include "coresight-tpda.h" >>>>>   #include "coresight-trace-id.h" >>>>> +#include "coresight-tpdm.h" >>>>>     DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda"); >>>>>   @@ -28,24 +29,57 @@ static bool coresight_device_is_tpdm(struct >>>>> coresight_device *csdev) >>>>>               CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM); >>>>>   } >>>>>   +static void tpdm_clear_element_size(struct coresight_device *csdev) >>>>> +{ >>>>> +    struct tpda_drvdata *drvdata = >>>>> dev_get_drvdata(csdev->dev.parent); >>>>> + >>>>> +    drvdata->dsb_esize = 0; >>>>> +    drvdata->cmb_esize = 0; >>>>> +} >>>>> + >>>>> +static void tpda_set_element_size(struct tpda_drvdata *drvdata, >>>>> u32 *val) >>>>> +{ >>>>> + >>>> >>>> >>>> >>>>> +    if (drvdata->dsb_esize == 64) >>>>> +        *val |= TPDA_Pn_CR_DSBSIZE; >>>> >>>> We don't seem to be clearing the fields we modify, before updating >>>> them. This may be OK in real world where the device connected to >>>> TPDA port >>>> may not change. But it is always safer to clear the bits and set it. >>>> >>>> e.g.: >>>>     *val &= ~(TPDA_Pn_CR_DSBSIZE | TPDA_Pn_CR_CMBSIZE); >>>> >>>> >>>> >>>>> +    else if (drvdata->dsb_esize == 32) >>>>> +        *val &= ~TPDA_Pn_CR_DSBSIZE; >>>>> + >>>>> +    if (drvdata->cmb_esize == 64) >>>>> +        *val |= FIELD_PREP(TPDA_Pn_CR_CMBSIZE, 0x2); >>>>> +    else if (drvdata->cmb_esize == 32) >>>>> +        *val |= FIELD_PREP(TPDA_Pn_CR_CMBSIZE, 0x1); >>>> >>>> Similarly here ^^^. I am happy to fix it up if you are OK with it >>>> (unless there are other changes that need a respin) >>> >>> Thank you. I would be very grateful if you could help for this. >> >> Given, you need to respin, please incorporate this change too. > > Sure. > > Is it OK if I modify the code as follow and update to this patch directly? > >     *val &= ~(TPDA_Pn_CR_DSBSIZE | TPDA_Pn_CR_CMBSIZE); > >     if (drvdata->dsb_esize == 64) >         *val |= TPDA_Pn_CR_DSBSIZE; >     else if (drvdata->dsb_esize == 32) >         *val &= ~TPDA_Pn_CR_DSBSIZE; > >     if (drvdata->cmb_esize == 64) >         *val |= FIELD_PREP(TPDA_Pn_CR_CMBSIZE, 0x2); >     else if (drvdata->cmb_esize == 32) >         *val |= FIELD_PREP(TPDA_Pn_CR_CMBSIZE, 0x1);. >     else if (drvdata->cmb_esize == 8) >         *val &= ~TPDA_Pn_CR_CMBSIZE; Yes, that looks good to me. Even though you would be clearing the fields for (DSB=32 and CMB=8) once again, it is good to leave it like that rather than skipping them, making people stare at it to figure out if this was correct or not. Suzuki > > Best, > > Tao > >> >> Suzuki >> >> >>