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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 78CEAC7618A for ; Fri, 17 Mar 2023 18:38:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230036AbjCQSiM (ORCPT ); Fri, 17 Mar 2023 14:38:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46770 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229489AbjCQSiJ (ORCPT ); Fri, 17 Mar 2023 14:38:09 -0400 Received: from mail-io1-xd36.google.com (mail-io1-xd36.google.com [IPv6:2607:f8b0:4864:20::d36]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 12330584B9; Fri, 17 Mar 2023 11:37:59 -0700 (PDT) Received: by mail-io1-xd36.google.com with SMTP id g6so2686019iov.13; Fri, 17 Mar 2023 11:37:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679078278; h=content-transfer-encoding:in-reply-to:subject:from:references:cc:to :content-language:user-agent:mime-version:date:message-id:sender :from:to:cc:subject:date:message-id:reply-to; bh=doTBEIPrtJGrIVKGzCIV43Qeoz4CxJQQLeaZ0TGaW9Q=; b=hSbf2GewkqwWqibfrWgkVO7XXcHstCvhpYn/F4tkL6BYH44pB6P8kEuSebAOVrHMmL zi2RPkBYqkPVZsDvNup5EmBAlEd2IaokgDFflK8I3ytcHVnW0lF9l4kJzt16f5x9t0pM hGjPHt0szAsGj3sQuPmfzhj6nzmHG0BYF5TIJ2iwCGbVqLGNct5ZgSSyFsizv/S0b4BN lSSqXXzxJxZdp9sidj23nGsSMC5vnBJ8wS9Vwm1UFJqoDKxITXWkaDrmaPgS7+Sf+igo GhdG9GykYGdrQSoePQfnGTpSdUQiMAqsBWIp45fE9ALXsxFEoEALrvXjYF4S32E6IwsF LLRg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679078278; h=content-transfer-encoding:in-reply-to:subject:from:references:cc:to :content-language:user-agent:mime-version:date:message-id:sender :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=doTBEIPrtJGrIVKGzCIV43Qeoz4CxJQQLeaZ0TGaW9Q=; b=ZfmpharfZNH9LcCVAh9RbHeVTX0N87xfFfskWpiwtSlO/Rq7WP/GNP0nbVikSOfrN5 h50PEebuoFFTKf6zr/glLUp6zjEoJJS4ATrtHuafNT9reTzUluQuE/vXbQsFic58b55A GA8aKoIYG/kwNkyv5aPObn3aC5bAL2PI2aJNkZ89zSrJovUkpjwNvxddEcqnMdg+gZwu LmS83LxHJ90MJuz9EpS9XfOq7II3D8aiEWawFym1zujh6KIhRZ9uQyRE2vzt0QTHWedd a4kCLToa7GcVatBfmXtYA1QehyCAXKCYr74fqUXlksmrkLMEOkP4KV82h9X9AmsdYePo ZePQ== X-Gm-Message-State: AO0yUKX18XZaAG68ngHAk3iIp5WZdK20jSHIsHCY4qoOEgL6jwy254Ue uxeK2aP8jHPaMcxh6h+EWtU= X-Google-Smtp-Source: AK7set89qP2jQ0x7Zcy+DFqgPLQjIL0udBlesLSIV2ZSwz3/NAWdm44y9/1Aw9RHvJd3zAZMKUUTVA== X-Received: by 2002:a5d:9c89:0:b0:752:fd8e:5ee4 with SMTP id p9-20020a5d9c89000000b00752fd8e5ee4mr386005iop.1.1679078278421; Fri, 17 Mar 2023 11:37:58 -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 e30-20020a026d5e000000b0040631c91311sm875212jaf.91.2023.03.17.11.37.55 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 17 Mar 2023 11:37:57 -0700 (PDT) Sender: Guenter Roeck Message-ID: <4b2d6f4a-4868-9ffe-e1e1-9feafe41ccdd@roeck-us.net> Date: Fri, 17 Mar 2023 11:37:54 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Content-Language: en-US To: Andy Shevchenko Cc: Tomer Maimon , ulf.hansson@linaro.org, avifishman70@gmail.com, tali.perry1@gmail.com, joel@jms.id.au, venture@google.com, yuenn@google.com, benjaminfair@google.com, adrian.hunter@intel.com, skhan@linuxfoundation.org, davidgow@google.com, pbrobinson@gmail.com, gsomlo@gmail.com, briannorris@chromium.org, arnd@arndb.de, krakoczy@antmicro.com, openbmc@lists.ozlabs.org, linux-mmc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org References: <20221205085351.27566-1-tmaimon77@gmail.com> <20221205085351.27566-3-tmaimon77@gmail.com> From: Guenter Roeck Subject: Re: [PATCH v2 2/2] mmc: sdhci-npcm: Add NPCM SDHCI driver In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 3/17/23 10:36, Andy Shevchenko wrote: > On Fri, Mar 17, 2023 at 4:16 PM Guenter Roeck wrote: >> >> On Mon, Dec 05, 2022 at 10:53:51AM +0200, Tomer Maimon wrote: >>> Add Nuvoton NPCM BMC sdhci-pltfm controller driver. >>> >>> Signed-off-by: Tomer Maimon >> >> I still don't see this driver in the upstream kernel, or in linux-next. >> >> Couple of comments: >> >> - devm ordering does not really matter here. The devm resource >> is the clock, it does not depend on local data, and it will be >> released last, so that is ok. > > Not sure. Strictly speaking this is the problem. If you leave a clock > going on in a wrong period of time it (theoretically) might break your > hardware once and forever. Similar discussion about power, clock and > reset signals has been held for camera sensors. > In general I agree, but not here. The remove function (sdhci_pltfm_unregister) does call clk_disable_unprepare(), so the clock isn't left running. Also, I think it is worthwhile to point out that exactly the same sequence (sdhci_pltfm_init followed by devm_clk_get and cleanup/removal with sdhci_pltfm_unregister) is shared among several sdhci drivers (including the memory leak I pointed out, but only in the aspeed driver). On a higher level I do agree that the sdhci platform code is in need of cleanup, but I don't think it is appropriate to tie such a cleanup to this driver submission. Note that I don't really care much, I just realized that this patch is stuck when I tried to test booting from SD drive with qemu. Guenter 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 lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (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 13717C74A5B for ; Fri, 17 Mar 2023 18:39:05 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4PdXw80vBzz3cjM for ; Sat, 18 Mar 2023 05:39:04 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20210112 header.b=hSbf2Gew; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gmail.com (client-ip=2607:f8b0:4864:20::d36; helo=mail-io1-xd36.google.com; envelope-from=groeck7@gmail.com; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20210112 header.b=hSbf2Gew; dkim-atps=neutral Received: from mail-io1-xd36.google.com (mail-io1-xd36.google.com [IPv6:2607:f8b0:4864:20::d36]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4PdXv02Kvkz3cDp for ; Sat, 18 Mar 2023 05:38:02 +1100 (AEDT) Received: by mail-io1-xd36.google.com with SMTP id p17so953245ioj.10 for ; Fri, 17 Mar 2023 11:38:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679078278; h=content-transfer-encoding:in-reply-to:subject:from:references:cc:to :content-language:user-agent:mime-version:date:message-id:sender :from:to:cc:subject:date:message-id:reply-to; bh=doTBEIPrtJGrIVKGzCIV43Qeoz4CxJQQLeaZ0TGaW9Q=; b=hSbf2GewkqwWqibfrWgkVO7XXcHstCvhpYn/F4tkL6BYH44pB6P8kEuSebAOVrHMmL zi2RPkBYqkPVZsDvNup5EmBAlEd2IaokgDFflK8I3ytcHVnW0lF9l4kJzt16f5x9t0pM hGjPHt0szAsGj3sQuPmfzhj6nzmHG0BYF5TIJ2iwCGbVqLGNct5ZgSSyFsizv/S0b4BN lSSqXXzxJxZdp9sidj23nGsSMC5vnBJ8wS9Vwm1UFJqoDKxITXWkaDrmaPgS7+Sf+igo GhdG9GykYGdrQSoePQfnGTpSdUQiMAqsBWIp45fE9ALXsxFEoEALrvXjYF4S32E6IwsF LLRg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679078278; h=content-transfer-encoding:in-reply-to:subject:from:references:cc:to :content-language:user-agent:mime-version:date:message-id:sender :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=doTBEIPrtJGrIVKGzCIV43Qeoz4CxJQQLeaZ0TGaW9Q=; b=sqAFspiUuobIVMAMypTN4vTTNjAZfRL1Obbpklgo6LO99DWwL3uWcHDPo+y1mHUf4h BBm5UkxZTj160MO9j/Pu24Ypqt7862XMSLNX9euhAncCXNYTNRFpgZxpE1gl0E13gw3n lAu4yxDE7G7uFm9CEwK1NVRORUAc9cVGyGmBXMs8P5b4gVsg0G4UXuz+err/ug/AxgeW Y/jiJSFpH6IpE5W2Kf6ie5dMNZ21ndkL/ogoSdwUZMtu06ytFfx4HSBpeSI3c3M0Xgwn Z4DZr3sZgEaQcRPXxH3KFfXy50ls87/8sqzy0aSl2jCw6ak2AODcbX2hCD/kgMwSxw2O Huyw== X-Gm-Message-State: AO0yUKUp/vOxN1T/Lvr/bXMei/jecRQzfVHEWlESY99rEBWqlrX2vixu GVZClK53O+5cOu2vVbG5Cnk= X-Google-Smtp-Source: AK7set89qP2jQ0x7Zcy+DFqgPLQjIL0udBlesLSIV2ZSwz3/NAWdm44y9/1Aw9RHvJd3zAZMKUUTVA== X-Received: by 2002:a5d:9c89:0:b0:752:fd8e:5ee4 with SMTP id p9-20020a5d9c89000000b00752fd8e5ee4mr386005iop.1.1679078278421; Fri, 17 Mar 2023 11:37:58 -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 e30-20020a026d5e000000b0040631c91311sm875212jaf.91.2023.03.17.11.37.55 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 17 Mar 2023 11:37:57 -0700 (PDT) Message-ID: <4b2d6f4a-4868-9ffe-e1e1-9feafe41ccdd@roeck-us.net> Date: Fri, 17 Mar 2023 11:37:54 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Content-Language: en-US To: Andy Shevchenko References: <20221205085351.27566-1-tmaimon77@gmail.com> <20221205085351.27566-3-tmaimon77@gmail.com> From: Guenter Roeck Subject: Re: [PATCH v2 2/2] mmc: sdhci-npcm: Add NPCM SDHCI driver In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: openbmc@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development list for OpenBMC List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, ulf.hansson@linaro.org, Tomer Maimon , arnd@arndb.de, krakoczy@antmicro.com, avifishman70@gmail.com, venture@google.com, openbmc@lists.ozlabs.org, briannorris@chromium.org, linux-mmc@vger.kernel.org, adrian.hunter@intel.com, tali.perry1@gmail.com, gsomlo@gmail.com, joel@jms.id.au, davidgow@google.com, skhan@linuxfoundation.org, pbrobinson@gmail.com, linux-kernel@vger.kernel.org, benjaminfair@google.com Errors-To: openbmc-bounces+openbmc=archiver.kernel.org@lists.ozlabs.org Sender: "openbmc" On 3/17/23 10:36, Andy Shevchenko wrote: > On Fri, Mar 17, 2023 at 4:16 PM Guenter Roeck wrote: >> >> On Mon, Dec 05, 2022 at 10:53:51AM +0200, Tomer Maimon wrote: >>> Add Nuvoton NPCM BMC sdhci-pltfm controller driver. >>> >>> Signed-off-by: Tomer Maimon >> >> I still don't see this driver in the upstream kernel, or in linux-next. >> >> Couple of comments: >> >> - devm ordering does not really matter here. The devm resource >> is the clock, it does not depend on local data, and it will be >> released last, so that is ok. > > Not sure. Strictly speaking this is the problem. If you leave a clock > going on in a wrong period of time it (theoretically) might break your > hardware once and forever. Similar discussion about power, clock and > reset signals has been held for camera sensors. > In general I agree, but not here. The remove function (sdhci_pltfm_unregister) does call clk_disable_unprepare(), so the clock isn't left running. Also, I think it is worthwhile to point out that exactly the same sequence (sdhci_pltfm_init followed by devm_clk_get and cleanup/removal with sdhci_pltfm_unregister) is shared among several sdhci drivers (including the memory leak I pointed out, but only in the aspeed driver). On a higher level I do agree that the sdhci platform code is in need of cleanup, but I don't think it is appropriate to tie such a cleanup to this driver submission. Note that I don't really care much, I just realized that this patch is stuck when I tried to test booting from SD drive with qemu. Guenter