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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E3805C433F5 for ; Fri, 7 Jan 2022 16:04:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=h+nvAnix4uATI3ikX9RRavvq/tb0jekoQ6/igEA51uw=; b=zTIT74r1Uk+UB/ 1R+COgLbpa/EVIGWG0f2dyDKAin/LLKd6Fxq5IetnhPJX9tgCD1qZ/zVsReGFl4doRI/CeCpSZCOM 67OYt+jENgyFtcUvGtUHXwzXMhGJYC4lHt9r/s12b/iky5tlw/gVpD7nNJZ41cCbVjwmY9V9TLk8W yPQJPUVofJKGe2m5ekGEEe/J66z6sNAQNHWvKhpc35y5Ha6ZD2q+Xifbl7xBp0K9kSERFRsrZn+xp 0gw+cFoqnCkGsQ7o8UDx0+0s/9ElCYVxFnI8u96DdJ93RWuQdnKjORMhN7lQ7w60ky59gVNUCHg3I gzqTiQLlbQcwqIb43A2Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n5rhl-004aFK-F7; Fri, 07 Jan 2022 16:03:01 +0000 Received: from mail-ot1-x32c.google.com ([2607:f8b0:4864:20::32c]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1n5rhg-004aD6-QY for linux-arm-kernel@lists.infradead.org; Fri, 07 Jan 2022 16:02:58 +0000 Received: by mail-ot1-x32c.google.com with SMTP id o3-20020a9d4043000000b0058f31f4312fso7075289oti.1 for ; Fri, 07 Jan 2022 08:02:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=gZBW2/flvQ2NdF5hDsI0zPW9aTZhIeh2D6Bq7nrP3As=; b=t08/8rcjKEFS01FodQemSpCn4DXs8X6KDsqLbHOXRYocZAG5dRP3vSozIqcqwAJ6ln j7v7u6nK6GtuXuyjmmhX19oBbd78BcPSoo36kl+WewDuLLAoJevAaGQm3mLtAT7jM1Pv ltyevByddtOjxA9FYv3/BYCsiwLJfJ2TCudT+OYKesPCKDYf5/Z1INqItITbmBTlPuYe C1GswOAB3RhqGLaWhRwIk84xMw+BRKp21J8p8yxw9IXBb/H9k1+FVUvwZSXad9GjowMX 2+rtkidJeUHLcG7k+a5WaS31O8+1QNWsgrGXBRQJN2fyP+PqElmLYk+5xCdj2IGOWXum Uf1w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=gZBW2/flvQ2NdF5hDsI0zPW9aTZhIeh2D6Bq7nrP3As=; b=se2QRzRrJLWIiurqSD6TOCqjTMMVIzye+ZxmJWLZ4gJjI/Jid0E7pkv6L8xVWfd/KB cgKzNWojuRGeiinj4gNxnDz+LxwwQyamEXlkTTiL5aYZpjyskBY6of94RNOzDZHSy0zf k5TB6JVX0rcd7kCR5NPAYkQLqn1MBXyep9q2mMvasPObd/GqIdxxptOItcKAlDx5IwS1 0gi7ax4AM53wCjKXTPPSwZpf0I1CYVzL6KBUUgSeSdlskqmizGIIrD5Rj2FzwYAmdz4S m4T+zcSzpE5jUlNyFouhPFJf2BTKjLRcOemlhXeboz97ybSDrPIzLJfxcbXPMsGDK2fP UJ5w== X-Gm-Message-State: AOAM531bdJpiPqqQUhmoOsFRhMU0SNUCAMJxGNsOkPg0vMUR3TnUffn5 E9ZX4pnRDk/DbU67Y6mgkOF8BQ== X-Google-Smtp-Source: ABdhPJyhZaR12yPuk9JnVjyI0IUjv7LheeaRzWkyB3MBGk08AygTkO6lC5AHtMWGNJHphihyEzIE9A== X-Received: by 2002:a9d:7409:: with SMTP id n9mr1275023otk.80.1641571374360; Fri, 07 Jan 2022 08:02:54 -0800 (PST) Received: from ripper (104-57-184-186.lightspeed.austtx.sbcglobal.net. [104.57.184.186]) by smtp.gmail.com with ESMTPSA id a2sm908176oon.37.2022.01.07.08.02.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 07 Jan 2022 08:02:53 -0800 (PST) Date: Fri, 7 Jan 2022 08:03:41 -0800 From: Bjorn Andersson To: Souradeep Chowdhury Cc: Thara Gopinath , Andy Gross , Rob Herring , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, Sai Prakash Ranjan , Sibi Sankar , Rajendra Nayak , vkoul@kernel.org Subject: Re: [PATCH V6 0/7] Add driver support for Data Capture and Compare Engine(DCC) for SM8150,SC7280,SC7180,SDM845 Message-ID: References: <396edd95-4f38-6830-99da-11e73d62a0cf@linaro.org> <705c280b-bced-476d-8e21-1a5afbf3d2f3@quicinc.com> <84d36c7f-d75e-61f9-7670-c651cc50d083@quicinc.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <84d36c7f-d75e-61f9-7670-c651cc50d083@quicinc.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220107_080256_910128_A3F0F792 X-CRM114-Status: GOOD ( 63.92 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri 07 Jan 07:43 PST 2022, Souradeep Chowdhury wrote: > = > On 1/7/2022 5:35 AM, Bjorn Andersson wrote: > > On Thu 06 Jan 07:20 PST 2022, Souradeep Chowdhury wrote: > > = > > > On 12/16/2021 9:18 PM, Thara Gopinath wrote: > > > > = > > > > On 8/10/21 1:54 PM, Souradeep Chowdhury wrote: > > > > > DCC(Data Capture and Compare) is a DMA engine designed for debugg= ing > > > > > purposes.In case of a system > > > > > crash or manual software triggers by the user the DCC hardware > > > > > stores the value at the register > > > > > addresses which can be used for debugging purposes.The DCC driver > > > > > provides the user with sysfs > > > > > interface to configure the register addresses.The options that the > > > > > DCC hardware provides include > > > > > reading from registers,writing to registers,first reading and then > > > > > writing to registers and looping > > > > > through the values of the same register. > > > > > = > > > > > In certain cases a register write needs to be executed for access= ing > > > > > the rest of the registers, > > > > > also the user might want to record the changing values of a regis= ter > > > > > with time for which he has the > > > > > option to use the loop feature. > > > > Hello Souradeep, > > > > = > > > > First of all, I think this is very a useful feature to have. I have= some > > > > generic design related queries/comments on driver and the interface > > > > exposed to the user space. Also, I do not understand the h/w well h= ere, > > > > so feel free to correct me if I am wrong. > > > > = > > > > 1. Linked list looks like a very internal feature to the h/w. It re= ally > > > > is not an info that user should be aware of. I tried reading the co= de a > > > > bit. IUC, every time a s/w trigger is issued the configs in all the > > > > enabled linked lists are executed. The final ram dump that you get = from > > > > /dev/dcc_sram is a dump of contents from all the enabled list? Is t= his > > > > understanding correct ? And we are talking of at-most 4 linked list? > > > > If yes, I think it might be better to have a folder per linked list= with > > > > config, config_write etc. Also if possible it will be better to dum= p the > > > > results to a file in the specific folder instead of reading from > > > > /dev/dcc_sram. > > > > If no, there is no real need for user to know the linked list, righ= t? > > > > Choosing of linked list can be done by kernel driver in this case w= ith > > > > no input needed from user. > > > > = > > > > 2. Now to the sysfs interface itself, I know lot of thought has gone > > > > into sysfs vs debugfs considerations. But, have you considered using > > > > netlink interface instead of sysfs. Netlink interface is used for > > > > asynchronous communication between kernel and user space. In case of > > > > DCC, the communication appears to be asynchronous, where in user as= ks > > > > the kernel to capture some info and kernel can indicate back to user > > > > when the info is captured. Also the entire mess surrounding echoing= addr > > > > / value / offset repeatedly into a sysfs entry can be avoided using > > > > netlink interface. > > > > = > > > Hello Thara, > > > = > > > Thanks for your review comments. Following are some points from my end > > > = > > > = > > > 1) Each linked list represent a particular block of memory in DCC_SRA= M which > > > is preserved for that particular list. That is why offset calculation= is > > > done on the driver based on the linked list chosen by the user. > > > = > > > =A0=A0=A0 This choice needs to be made by the user since the number = for the linked > > > list chosen is specific to the registers used to debug a particular > > > component.=A0 Also we are giving the user flexibility to configure mu= ltiple > > > = > > > =A0=A0=A0 linked lists at one go so that even if we don't have a sep= arate folder > > > for it , the dumps are collected as a separate list of registers. Als= o there > > > are certain curr_list values which may be supported by the dcc > > > = > > > =A0=A0=A0 hardware but may not be accessible to the user and so the = choice cannot > > > be made arbitrarily from the driver. > > > = > > But in the end, as you write out the SRAM content, is there really any > > linked lists? Afaict it's just a sequence of operations/commands. The > > linked list part seems to be your data structure of choice to keep track > > of these operations in the driver before flushing them out. > = > That is correct, the linked list defined in the driver is for storing the > addresses sequentially in DCC_SRAM and is just an internal > data structure of the driver. However, there is also a "list" from DCC > hardware perspective. The following driver code shows how > a list is initiated with the beginning and end sram offset so that DCC > hardware can treat it as a separate list of addresses and dump > the values separately. > = Makes sense. But I think you should use "list" (or "sequence") and not "linked list" in the API/documentation then. > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 /* 1. Take ownership of the list = */ > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dcc_writel(drvdata, BIT(0),= DCC_LL_LOCK(list)); > = > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 /* 2. Program linked-list i= n the SRAM */ > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 ram_cfg_base =3D drvdata->r= am_cfg; > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 ret =3D __dcc_ll_cfg(drvdat= a, list); > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 if (ret) { > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dcc= _writel(drvdata, 0, DCC_LL_LOCK(list)); > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 got= o err; > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 } > = > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 /* 3. program DCC_RAM_CFG r= eg */ > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dcc_writel(drvdata, ram_cfg= _base + > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 drv= data->ram_offset/4, DCC_LL_BASE(list)); > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dcc_writel(drvdata, drvdata= ->ram_start + > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 drv= data->ram_offset/4, DCC_FD_BASE(list)); > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dcc_writel(drvdata, 0xFFF, = DCC_LL_TIMEOUT(list)); > = > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 /* 4. Clears interrupt stat= us register */ > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dcc_writel(drvdata, 0, DCC_= LL_INT_ENABLE(list)); > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dcc_writel(drvdata, (BIT(0)= | BIT(1) | BIT(2)), > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 DCC_LL_INT_STATUS(list)); > = > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 drvdata->enable[list] =3D t= rue; > = > So when user enters multiple lists, the DCC hardware will process it as > separate group of register values. > = But as the DCC supports reading, writing, looping and rmw I don't think it's correct to say that a list is a "group of register values". It's a "sequence (or list) of operations". Regards, Bjorn > > = > > Regards, > > Bjorn > > = > > > 2) From opensource, I can see that Netlink has been used in most of t= he > > > cases where we need to notify stats to the user by taking the advanta= ge of > > > asynchronous communication. In this case, that requirement is not > > > = > > > =A0 =A0 there since it is mostly one way communication from user to = kernel. Also > > > since this is used for debugging purposes perhaps sysfs adds more > > > reliability than Netlink. In case of Netlink we have the additional > > > = > > > =A0=A0=A0=A0 overhead of dealing with socket calls. Let me know othe= rwise. > > > = > > > = > > > Thanks, > > > = > > > Souradeep > > > = > > > = > > > = > > > = > > > = _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel