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 38862EB64DC for ; Thu, 15 Jun 2023 12:51:01 +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=bQyBoJQfzmC4TFr6vYTRmT+OnaAqdHChu6jgv833h4w=; b=H9eHQB8uLRMJYL Sk54x5s1zA5YaVIiV+9v2Xug2KSpv3KkjqlG76kzA91TPL+YWFhJynBUwHpFW4suy2FcNFvltTspT BTjCaJDm7ufWCqcHIGiBJa0nfLgh5s7wsbD+GrnRq4bgnT3bSkwZVDbIN+W/QdUdV/FXmYweYdMrM qvk+dmSqZEY6htSIsVvQgZy0KysiwEet1RtGPuop0cxymI74IrZaneiJIf3SRjY3ktNd/RG6Xm1Bs sd61ohkxLUrhSx6STvLlX8kKNm0tlWnNN1Tr4Ar1HQ7YS3qCR8ch3fERTVJtXK2rWJb96doPYIpXQ LMkz8bYtAOXJuQ7+rTWg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q9mQw-00EveC-1t; Thu, 15 Jun 2023 12:50:38 +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 1q9mQt-00Evdk-2o for linux-arm-kernel@lists.infradead.org; Thu, 15 Jun 2023 12:50:37 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id CC660625CE; Thu, 15 Jun 2023 12:50:34 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BBA46C433C8; Thu, 15 Jun 2023 12:50:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1686833434; bh=9C0lIysBzhK2NDkLZ8zuWOZP34c6lCF4sNn1JWvzNkc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Yh4oUiVoqClwbgiExl0clwXIAhBdc+EGfyc1pP8k5YyWihS/NjiHOyXPKOW1APa+0 jO7L1bcu4ev6Hr9gPCVFxpMPK0L2ECM7+HUo1gVyMTWF669az9mbhAE+ACBvJochTz zhZVopAKEqBqa96MZ0tFFhKfstjFskNC9/t1nZQU= Date: Thu, 15 Jun 2023 14:50:31 +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 V23 2/3] misc: dcc: Add driver support for Data Capture and Compare unit(DCC) Message-ID: <2023061515-unbuckled-consonant-e207@gregkh> References: <2259ab0348282349e88905ea99bcb4aa815d941f.1683265984.git.quic_schowdhu@quicinc.com> <2023061542-reformed-unholy-10a3@gregkh> 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-20230615_055035_984878_A49D5621 X-CRM114-Status: GOOD ( 31.52 ) 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 Thu, Jun 15, 2023 at 06:13:53PM +0530, Souradeep Chowdhury wrote: > > > On 6/15/2023 4:03 PM, Greg Kroah-Hartman wrote: > > On Thu, May 04, 2023 at 11:36:22PM -0700, Souradeep Chowdhury wrote: > > > +/** > > > + * struct dcc_config_entry - configuration information related to each dcc instruction > > > + * @base: Base address of the register to be configured in dcc > > > > Why is this a u32 and not a bigger size? > > Currently only 32 bit register addresses are supported for DCC > configuration. > > > > > > + * @offset: Offset to the base address to be configured in dcc > > > + * @len: Length of the address in words to be configured in dcc > > > > What is a "word" here, 16 bits? > > Each word is 4 bytes(32 bits) See, I guess wrong, you should say what this is :) > > > + * @loop_cnt: The number of times to loop on the register address in case > > > + of loop instructions > > > + * @write_val: The value to be written on the register address in case of > > > + write instructions > > > + * @mask: Mask corresponding to the value to be written in case of > > > + write instructions > > > + * @apb_bus: Type of bus to be used for the instruction, can be either > > > + 'apb' or 'ahb' > > > > How can a bool be either "apb" or "ahb"? > > 1 stands for apb and 0 for ahb. Will update the same here. Why not have an enum? Will there ever be another "bus"? > > > +static ssize_t ready_read(struct file *filp, char __user *userbuf, > > > + size_t count, loff_t *ppos) > > > +{ > > > + int ret = 0; > > > + char *buf; > > > + struct dcc_drvdata *drvdata = filp->private_data; > > > + > > > + mutex_lock(&drvdata->mutex); > > > + > > > + if (!is_dcc_enabled(drvdata)) { > > > + ret = -EINVAL; > > > + goto out_unlock; > > > + } > > > + > > > + if (!FIELD_GET(BIT(1), readl(drvdata->base + dcc_status(drvdata->mem_map_ver)))) > > > + buf = "Y\n"; > > > + else > > > + buf = "N\n"; > > > +out_unlock: > > > + mutex_unlock(&drvdata->mutex); > > > + > > > + if (ret < 0) > > > + return -EINVAL; > > > + else > > > > You do the "lock, get a value, unlock, do something with the value" > > thing a bunch, but what prevents the value from changing after the lock > > happens? So why is the lock needed at all? > > The lock is used to prevent concurrent accesses of the drv_data when > scripts are being run from userspace. How would that matter? The state can change instantly after the lock is given up, and then the returned value is now incorrect. So no need for a lock at all as you really aren't "protecting" anything, or am I missing something else? thanks, greg k-h _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel