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 X-Spam-Level: X-Spam-Status: No, score=-10.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A050FC433E0 for ; Sun, 9 Aug 2020 09:14:07 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6913A206B6 for ; Sun, 9 Aug 2020 09:14:07 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="gDsa5QcL"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="DOzwTXZc" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6913A206B6 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=VOQyGCqqrPimN/kQJZ/H9fsfiYZG716lt2Tn14weq5U=; b=gDsa5QcLagND9qji0JKcinkre OOt3uRqvOtXqWwDm/IUCSWb0bAxeL+bm9wUOI1GahKoqNt7pNbZKNykk56KuDUhY2HbLZuC+yGRam CUX6L/WOr8NjYfnRiNCpjeNrGh49O7AXXEHbqACxAPDnSkD6CqUeOjkvJyQSQVy12mSwGrDKsXi7J JzcDzsjnTlv8umlvjhNdH1b4hO6J53Nn4GdVWeNT211mXPCtLZe9cLRgsK0HFhGxrl6JoMZw1qZW8 nSqxWNuE/JGpEDX5Ur6qJafeWBN1GznFbXnlOiZkgn4XHrGB5b5viTSnH1QJTH7OHjNDaQmUA+vmp K5ixo1+mQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k4hNd-0002Lr-7K; Sun, 09 Aug 2020 09:12:37 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1k4hNb-0002LW-87 for linux-arm-kernel@lists.infradead.org; Sun, 09 Aug 2020 09:12:36 +0000 Received: from mail-lj1-f182.google.com (mail-lj1-f182.google.com [209.85.208.182]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 60944206B6 for ; Sun, 9 Aug 2020 09:12:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1596964353; bh=EvMkiAV/QPaNxSiF1QV6aIN8GUE6bYJn6W6e14MfThU=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=DOzwTXZctisZhFn3SMxaGkMAKPzOBqgDA8rZVkrzEHJTCnkctn56nOVEW8Bxh/KVm VKg3RJtRqoTwIPmAv1Nhw1q8b5G9H6u7dfMU/7CyXJUO8B2xnTmAgu7PrVH/1kUQRS sFbtIsBnnLeGufxim4j5A1e82yoJ4LPfGa4ztatU= Received: by mail-lj1-f182.google.com with SMTP id g6so6461622ljn.11 for ; Sun, 09 Aug 2020 02:12:33 -0700 (PDT) X-Gm-Message-State: AOAM532BXDUatWM5yn0JPTXTvdx7oC5wS6ZFmHJ94oNLZnVYBixXi4ZV fLroYTiObObQHLLSWyBJvNRs/LGgwLU/vZhsuNE= X-Google-Smtp-Source: ABdhPJznCulEjN/KcLzYGMk3mIIrcG1fmWkPOR29sS+cEcZOxbPrWV3J7BBHAjxBcBj1e9zHDrdwzNW8LNaWXqSZ+VM= X-Received: by 2002:a2e:86da:: with SMTP id n26mr10264938ljj.311.1596964351660; Sun, 09 Aug 2020 02:12:31 -0700 (PDT) MIME-Version: 1.0 References: <20200724180857.22119-1-krzk@kernel.org> <3522860a-8158-6e71-9d65-01d0e0c15f0d@arm.com> In-Reply-To: <3522860a-8158-6e71-9d65-01d0e0c15f0d@arm.com> From: Krzysztof Kozlowski Date: Sun, 9 Aug 2020 11:12:20 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFC] memory: exynos5422-dmc: Document mutex scope To: Lukasz Luba X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200809_051235_511108_755AF005 X-CRM114-Status: GOOD ( 27.44 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "linux-samsung-soc@vger.kernel.org" , Kukjin Kim , "linux-kernel@vger.kernel.org" , linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org 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 Tue, Aug 04, 2020 at 11:40:07AM +0100, Lukasz Luba wrote: > Hi Krzysztof, > > On 7/24/20 7:08 PM, Krzysztof Kozlowski wrote: > > Document scope of the mutex used by driver. > > > > Signed-off-by: Krzysztof Kozlowski > > > > --- > > > > It seems mutex was introduced to protect: > > 1. setting actual frequency/voltage, > > 2. dmc->curr_rate (in exynos5_dmc_get_cur_freq()). > > > > However dmc->curr_rate in exynos5_dmc_get_status() is not protected. Is > > it a bug? > > The callback get_dev_status() from devfreq->profile, which here is the > exynos5_dmc_get_status() should be already called with devfreq->lock > mutex hold, like e.g from simple_ondemand governor or directly > using update_devfreq exported function: > update_devfreq() > ->get_target_freq() > devfreq_update_stats() > df->profile->get_dev_status() > > The dmc->curr_rate is also used from sysfs interface from devfreq. > The local dmc lock serializes also this use case (when the HW freq > has changed but not set yet into curr_rate. These are different locks. You cannot protect dmc->curr_rate with devfreq->lock in one place and dmc-lock in other place. This won't protect it. > > --- > > drivers/memory/samsung/exynos5422-dmc.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c > > index 93e9c2429c0d..0388066a7d96 100644 > > --- a/drivers/memory/samsung/exynos5422-dmc.c > > +++ b/drivers/memory/samsung/exynos5422-dmc.c > > @@ -114,6 +114,7 @@ struct exynos5_dmc { > > void __iomem *base_drexi0; > > void __iomem *base_drexi1; > > struct regmap *clk_regmap; > > + /* Protects curr_rate and frequency/voltage setting section */ > > struct mutex lock; > > unsigned long curr_rate; > > unsigned long curr_volt; > > > > I assume this missing comment for the lock was required by some scripts. > In this case LGTM: > > Reviewed-by: Lukasz Luba Such comments are always useful. It is also pointed by strict checkpatch: CHECK: struct mutex definition without comment Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel