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 04D01C7619A for ; Sat, 15 Apr 2023 05:40:37 +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=1/ZDByNOGWWhCEQ4WlU4fo6aOuAmGh1jluqy8ECQuGY=; b=TvEVxRUBDKPaZR lFjlRz2yRYvcZVANFUjMBZuat2SXvJBW/+5K8+xqWak9uTrtN6tDJ5UFWjIZSbcNd/qxo1w7MYkHa t8MgNPKfPSaR5RYRnWdPCylPPTF8AcTstg3uK+luWGTKYtUqkS8SO1HElsO8ZMUYVTVWLL7Uwkh9K Gxg6rOAY/Uyfcqq2hlLDvk5GFykBAFPqpmUTbNjIV/HT0IEQGkBXZiIhATeMTcj7cVSOnsKsJn+7d eNm5Y4kEdIOX0EhFh1SraRBijOMchfvPRPhDCZ2cY8fFWcbk0TRMsa+jSWKjitHEayJDki4uEE+Jq BWqRGB3yrrJiYtgqmx1g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1pnYdM-00BOOp-0I; Sat, 15 Apr 2023 05:39:36 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1pnYdJ-00BOO6-0R for linux-arm-kernel@lists.infradead.org; Sat, 15 Apr 2023 05:39:34 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id A026A61261; Sat, 15 Apr 2023 05:39:28 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5CC8BC433D2; Sat, 15 Apr 2023 05:39:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1681537168; bh=sgqkra4Gl7AEFkkAJD6KZbCVVWw5Ls3bOzVoxaPUYvs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=jmk1QDXOKGjBY0T2+F1Q0RNrD2CkZhgopDBDxA+5qOo6Tkeg6L7xcJMjZBPE0Fg5Q PecKJFwxwhVKxdk0DkkXG0Nj19rX7tI0Nu4hV7lQfT7z2dg+wASQpAuS4LW69Oqe1E ku5O4zQlD2JFGgMvATYQRbgLf0T7KA0octmvdJ2c= Date: Sat, 15 Apr 2023 07:39:24 +0200 From: Greg Kroah-Hartman To: Souradeep Chowdhury Cc: Andy Gross , Konrad Dybcio , Krzysztof Kozlowski , Bjorn Andersson , Rob Herring , Alex Elder , Arnd Bergmann , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, Sibi Sankar , Rajendra Nayak Subject: Re: [PATCH V1 2/3] drivers: misc: dcc: Add driver support for Data Capture and Compare unit(DCC) Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230414_223933_255228_239DCF0D X-CRM114-Status: GOOD ( 27.53 ) 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="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Apr 14, 2023 at 07:29:12PM +0530, Souradeep Chowdhury wrote: > The DCC is a DMA Engine designed to capture and store data > during system crash or software triggers. The DCC operates > based on user inputs via the debugfs interface. The user gives > addresses as inputs and these addresses are stored in the > dcc sram. In case of a system crash or a manual software > trigger by the user through the debugfs interface, > the dcc captures and stores the values at these addresses. > This patch contains the driver which has all the methods > pertaining to the debugfs interface, auxiliary functions to > support all the four fundamental operations of dcc namely > read, write, read/modify/write and loop. The probe method > here instantiates all the resources necessary for dcc to > operate mainly the dedicated dcc sram where it stores the > values. The DCC driver can be used for debugging purposes > without going for a reboot since it can perform software > triggers as well based on user inputs. You have 72 columns, why not use them all please? > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved. > + * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved. It is now 2023 :) > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define STATUS_READY_TIMEOUT 5000 /* microseconds */ > + > +#define DCC_SRAM_NODE "dcc_sram" You only use this once, why is a #define needed? > +static void dcc_create_debug_dir(struct dcc_drvdata *drvdata) > +{ > + int i; > + char list_num[10]; > + struct dentry *list; > + struct device *dev = drvdata->dev; > + > + drvdata->dbg_dir = debugfs_create_dir(dev_name(dev), NULL); You are creating a directory at the root of debugfs with just your device name? While that will work, that feels very odd. Please use a subdirectory. > + if (IS_ERR(drvdata->dbg_dir)) { > + pr_err("can't create debugfs dir\n"); There is no need to ever check the return value of a debugfs call. Nor do you really ever even need to save off the dentry here, just look it up when you need to remove it. > + return; > + } > + > + for (i = 0; i <= drvdata->nr_link_list; i++) { > + sprintf(list_num, "%d", i); > + list = debugfs_create_dir(list_num, drvdata->dbg_dir); > + debugfs_create_file("enable", 0600, list, drvdata, &enable_fops); > + debugfs_create_file("config", 0600, list, drvdata, &config_fops); > + } > + > + debugfs_create_file("trigger", 0200, drvdata->dbg_dir, drvdata, &trigger_fops); > + debugfs_create_file("ready", 0400, drvdata->dbg_dir, drvdata, &ready_fops); > + debugfs_create_file("config_reset", 0200, drvdata->dbg_dir, > + drvdata, &config_reset_fops); This really looks like you are using debugfs to control the device, not just for debugging information. How are you going to be able to use the device in a system that has debugfs disabled? thanks, greg k-h _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel