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 EEEA3C433F5 for ; Sun, 2 Oct 2022 16:30:42 +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-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=PEkDPx/Aoi+3MOGYjaet3FG5HAo/o8u9sloYAnRkgxY=; b=MMxKgTbC84Lhmw q3De7sBch+MAbJKZLMBGPQ73OIoOLamT0ZSdlfOQsli9uKl993kNPTRTBNkcJ0luF5BUhMdk2MwNZ IlPjzvrUxr2vVF4HTx2B97gK1bmOJLrsagyJDV/73tuUZCFdLNFCFdMIOt5r8IdWrU7Q2banDmrOc +NaV6z59zVbOdJynxpcW1wNh2fYqk1npVCpllPjWVkusd597wwzgk49YcMolEiHvPrYS3OxlkQ/ct RwnK/7ugXcTmEzuOYc1aW+58E4C3ucA1UjG8ZJGxTspXjx1UUALL+orCQHqYcMERdfimW4wnKYtNS xwc+NcAPycolIJspx38A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1of1qU-001nWq-HA; Sun, 02 Oct 2022 16:29:38 +0000 Received: from mail-pf1-x430.google.com ([2607:f8b0:4864:20::430]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1of1qR-001nWL-Nx for linux-arm-kernel@lists.infradead.org; Sun, 02 Oct 2022 16:29:37 +0000 Received: by mail-pf1-x430.google.com with SMTP id a29so8312881pfk.5 for ; Sun, 02 Oct 2022 09:29:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :sender:from:to:cc:subject:date; bh=W/mvBMf1l3yRdnUZGfVhcWENOCg0qj1cA36GbcBdRGs=; b=pc5lxh/93I0420BOdhHVU0ZejliAddDij6xeEyfW71RDgjct2r166qOY5lEydELyIr ocksYuZiMulcBEL75+c234yPrqfLWwCi9q5fdirjO4bzp5kmskw2dLkKfQ04ay0rUkHw LhbMT7dQ7Z565nj7Y2bD3K6NXeZhYbWfGcTHF4gffFsz76+nrY0eFoPb09zpHHkX/t2p fij+YPfKQxAXX1xgM7fg2EY5b6UY4GFho+2qMeA03IZJwDwT/j5HT52ohlTd1Y5Jwrtg WUWfwQf3BYyKJjhGvAW/zzeE/TENYc+6kvn/SF3vVONGNBSKHZf0yzm2pN58NuEGG+bF rsiw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :sender:x-gm-message-state:from:to:cc:subject:date; bh=W/mvBMf1l3yRdnUZGfVhcWENOCg0qj1cA36GbcBdRGs=; b=buMNCmgxOZiqHayG2tTxn+n1Q5nDMQdeZ2uBCfd1SFnlvqq3V+6QNOFT+8NWAUD9I/ 1Vh9IayQ1OSrWVh4gFbT0sPeQufr63bgcElkBOBX2xm9RVhz2AXpWe07qnX6EOtPN3q5 Uu7Qw9k+cvXiXqg1UligU+sEViFoAMmLL1cRxJc8bt/zVE2ojGKhe+sA8C7UkJrLkdyW OT0EXejTZ2SN3jRr23jLCeNzRWwVhNZZVifFwKqAbYK7cr6t7K5DXVjunVioLY4HrccA BoGcl5QChc0B3rrILkBKJZyh6ueadlvNSW5a4ubvLxLbBuXd+TALJVLISHolerrVF68N crKA== X-Gm-Message-State: ACrzQf3jqms9GHCeE89YUz8YGX28KXYm+WQMjbm72hLPNcvvoode9T/V kiDUwtFeeUvNRxNGq4Yhvgk0z/sqL84m4A== X-Google-Smtp-Source: AMsMyM4Xilj4glB6Ba2j6F0ZyyDyJsIw+Psqqfq8MIUonaXcQGuf/KYl9gbQF1Rrqgw2kl+kSrJGKg== X-Received: by 2002:a05:6a00:189d:b0:53e:79de:3fc1 with SMTP id x29-20020a056a00189d00b0053e79de3fc1mr18461328pfh.2.1664728174572; Sun, 02 Oct 2022 09:29:34 -0700 (PDT) Received: from ?IPV6:2600:1700:e321:62f0:329c:23ff:fee3:9d7c? ([2600:1700:e321:62f0:329c:23ff:fee3:9d7c]) by smtp.gmail.com with ESMTPSA id c6-20020a170902d48600b00178143a728esm5547388plg.275.2022.10.02.09.29.33 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 02 Oct 2022 09:29:33 -0700 (PDT) Message-ID: <1cb7fdb7-ebad-a61e-490c-80e6c9feab2b@roeck-us.net> Date: Sun, 2 Oct 2022 09:29:32 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH 2/3] watchdog: xilinx_wwdt: Add Versal window watchdog support Content-Language: en-US To: Krzysztof Kozlowski , Srinivas Neeli , wim@linux-watchdog.org, shubhrajyoti.datta@amd.com, michal.simek@amd.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org Cc: linux-kernel@vger.kernel.org, linux-watchdog@vger.kernel.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, git@amd.com References: <20220927110257.41963-1-srinivas.neeli@amd.com> <20220927110257.41963-3-srinivas.neeli@amd.com> From: Guenter Roeck In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221002_092935_808229_08E223A2 X-CRM114-Status: GOOD ( 20.19 ) 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 9/30/22 03:35, Krzysztof Kozlowski wrote: > On 27/09/2022 13:02, Srinivas Neeli wrote: >> + >> +static void xwwdt_clk_disable_unprepare(void *data) >> +{ >> + clk_disable_unprepare(data); > > If watchdog is stopped and then device unbound, don't you have double > disable? IOW, where is matching clk_enable? > After looking into the entire driver code, I see the problem: clk_disable() is called in the stop function. That is wrong; if the clock is disabled in the stop function, it needs to be enabled in the start function (which is not currently the case). In that situation, using the devm_ callback when unloading the driver isn't possible (or, rather, would require checking if the clock is enabled before disabling it). Thanks, Guenter >> +} >> + >> +static const struct watchdog_info xilinx_wwdt_ident = { >> + .options = WDIOF_KEEPALIVEPING | >> + WDIOF_SETTIMEOUT, >> + .firmware_version = 1, >> + .identity = "xlnx_window watchdog", >> +}; >> + >> +static const struct watchdog_ops xilinx_wwdt_ops = { >> + .owner = THIS_MODULE, >> + .start = xilinx_wwdt_start, >> + .stop = xilinx_wwdt_stop, >> + .set_timeout = xilinx_wwdt_set_timeout, >> + .ping = xilinx_wwdt_keepalive, >> +}; >> + >> +static int xwwdt_probe(struct platform_device *pdev) >> +{ >> + struct watchdog_device *xilinx_wwdt_wdd; >> + struct device *dev = &pdev->dev; >> + struct xwwdt_device *xdev; >> + int ret; >> + >> + xdev = devm_kzalloc(dev, sizeof(*xdev), GFP_KERNEL); >> + if (!xdev) >> + return -ENOMEM; >> + >> + xilinx_wwdt_wdd = &xdev->xilinx_wwdt_wdd; >> + xilinx_wwdt_wdd->info = &xilinx_wwdt_ident; >> + xilinx_wwdt_wdd->ops = &xilinx_wwdt_ops; >> + xilinx_wwdt_wdd->parent = dev; >> + >> + xdev->base = devm_platform_ioremap_resource(pdev, 0); >> + if (IS_ERR(xdev->base)) >> + return PTR_ERR(xdev->base); >> + >> + xdev->clk = devm_clk_get(dev, NULL); >> + if (IS_ERR(xdev->clk)) >> + return PTR_ERR(xdev->clk); >> + >> + xdev->freq = clk_get_rate(xdev->clk); >> + if (!xdev->freq) >> + return -EINVAL; >> + >> + ret = clk_prepare_enable(xdev->clk); >> + if (ret) { >> + dev_err(dev, "unable to enable clock\n"); >> + return ret; >> + } >> + >> + ret = devm_add_action_or_reset(dev, xwwdt_clk_disable_unprepare, >> + xdev->clk); >> + if (ret) >> + return ret; >> + >> + xilinx_wwdt_wdd->timeout = XWWDT_DEFAULT_TIMEOUT; >> + xilinx_wwdt_wdd->min_timeout = XWWDT_MIN_TIMEOUT; >> + xilinx_wwdt_wdd->max_timeout = XWWDT_MAX_TIMEOUT; >> + >> + ret = watchdog_init_timeout(xilinx_wwdt_wdd, >> + xwwdt_timeout, &pdev->dev); >> + if (ret) >> + dev_info(&pdev->dev, "Configured default timeout value\n"); >> + >> + spin_lock_init(&xdev->spinlock); >> + watchdog_set_drvdata(xilinx_wwdt_wdd, xdev); >> + >> + ret = devm_watchdog_register_device(dev, xilinx_wwdt_wdd); >> + if (ret) >> + return ret; >> + >> + dev_info(dev, "Xilinx window watchdog Timer with timeout %ds\n", >> + xilinx_wwdt_wdd->timeout); >> + >> + return 0; >> +} >> + >> +static const struct of_device_id xwwdt_of_match[] = { >> + { .compatible = "xlnx,versal-wwdt-1.0", }, >> + {}, >> +}; >> +MODULE_DEVICE_TABLE(of, xwwdt_of_match); >> + >> +static struct platform_driver xwwdt_driver = { >> + .probe = xwwdt_probe, >> + .driver = { >> + .name = "Xilinx window watchdog", > > Do you see spaces in other names of drivers? > > Best regards, > Krzysztof > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel