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=-17.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham 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 A0D7FC47082 for ; Thu, 3 Jun 2021 09:04:33 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 45F0F61361 for ; Thu, 3 Jun 2021 09:04:31 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 45F0F61361 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=foundries.io Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 060B782CBE; Thu, 3 Jun 2021 11:04:30 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=foundries.io Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=foundries.io header.i=@foundries.io header.b="D3L3/MR4"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id C9CBD82E52; Thu, 3 Jun 2021 11:04:27 +0200 (CEST) Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 88D3582B0E for ; Thu, 3 Jun 2021 11:04:24 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=foundries.io Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=jorge@foundries.io Received: by mail-wm1-x32b.google.com with SMTP id h5-20020a05600c3505b029019f0654f6f1so4884287wmq.0 for ; Thu, 03 Jun 2021 02:04:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foundries.io; s=google; h=from:date:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=UWqtMbIT9XN4ACq+fjjBLRlR/Wa7LjqF0FwWsTPhjfs=; b=D3L3/MR4xi4/+hoOgoawfPCds6w+6Fanh8WoKu1jbAlcWN4XU9O3pUx7lN4POGoLvC 9cTKXbyiw4Mm7+ZMbq8BrTlyfmKFpMvKjbRANrE94Gx/RepNuynC7r0s7A0pmhQxSnj9 KyMNV8h8XdfrxgbznaB8JTGbKVDEAasrVrahqxwbKn1Y1y7iVeMsffn3O4wYIC6WOl82 OR4mSQDYln6ZsTbur7tszTw8xdWIfABxeMOVaoH7jt2LNoEguLy6BwhTpuUx2x3Q8i8V 7rOWjb15nBTmWkR1LCCw1LOFT2B9iV50uzTsNAd7iouI6Ocs7lfjH691No3W9A6zEkXG NxcQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:date:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=UWqtMbIT9XN4ACq+fjjBLRlR/Wa7LjqF0FwWsTPhjfs=; b=p3eafLvKy7OIvr8U1yDTkMfR+/8FGPtbF60MwnyJ5Vs0y9sEhKqw/u1ogH447PKvCU 6ifOTPvNj6LbBcyipJsaatjGwo/Xn2ibpvFs0Imp/y0ahvhc4yFCgbRecMdWYoflay8w 3lySCgDxm/ACYVuQGrFdoswa5n20q4U2IeKxru4g4s8ErCo9wrR4pNf05dyXv/LDq4kG Y3B2zr+saFvJrb6/HLgCKv2AwcHGrTfLD0qGe3PA6JlJlGoZjH2BQdJXYI5XQjtkjoOQ FMhPXu2NTudr3ZlIElDUtMQuHRy0pt5rXQ4f8wCIoP8kXj6K1+KeqSK3l1VgtiRhuu4A VPyw== X-Gm-Message-State: AOAM5320xFLvovHc9nR6cJ5qNz+EjNGCaOBJMHkOCfYj+JI+1XplMABj 5ua1jp8hMeDVAS4ynfL2c/jH4g== X-Google-Smtp-Source: ABdhPJyFuGGG/hWA8ZKQOCz6LLqWNolH63XMgTxHLKakKreolfvbXsLOsIFxCEYp2Di9aUuGvvrcyQ== X-Received: by 2002:a1c:f213:: with SMTP id s19mr20403931wmc.61.1622711064108; Thu, 03 Jun 2021 02:04:24 -0700 (PDT) Received: from trex (113.red-79-154-202.dynamicip.rima-tde.net. [79.154.202.113]) by smtp.gmail.com with ESMTPSA id u2sm2519671wrn.38.2021.06.03.02.04.23 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Thu, 03 Jun 2021 02:04:23 -0700 (PDT) From: "Jorge Ramirez-Ortiz, Foundries" X-Google-Original-From: "Jorge Ramirez-Ortiz, Foundries" Date: Thu, 3 Jun 2021 11:04:22 +0200 To: Michal Simek Cc: "Jorge Ramirez-Ortiz, Foundries" , sjg@chromium.org, bruno.thomsen@gmail.com, u-boot@lists.denx.de Subject: Re: [PATCHv2] drivers: tpm2: update reset gpio semantics Message-ID: <20210603090422.GA19121@trex> References: <20210601060927.30894-1-jorge@foundries.io> <20210601073517.GA2523@trex> <121d591d-6136-c66e-967f-8c3ad77b0dc6@xilinx.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <121d591d-6136-c66e-967f-8c3ad77b0dc6@xilinx.com> User-Agent: Mutt/1.9.4 (2018-02-28) X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.102.4 at phobos.denx.de X-Virus-Status: Clean On 01/06/21, Michal Simek wrote: > > > On 6/1/21 9:35 AM, Jorge Ramirez-Ortiz, Foundries wrote: > > On 01/06/21, Michal Simek wrote: > >> > >> > >> On 6/1/21 8:09 AM, Jorge Ramirez-Ortiz wrote: > >>> Use the more generic reset-gpios propery name. > >>> > >>> Signed-off-by: Jorge Ramirez-Ortiz > >>> --- > >>> v2: kept gpio-reset as legacy > >>> > >>> .../tpm2/tis-tpm2-spi.txt | 2 +- > >>> drivers/tpm/tpm2_tis_spi.c | 21 ++++++++++++------- > >>> 2 files changed, 14 insertions(+), 9 deletions(-) > >>> > >>> diff --git a/doc/device-tree-bindings/tpm2/tis-tpm2-spi.txt b/doc/device-tree-bindings/tpm2/tis-tpm2-spi.txt > >>> index 3a2ee4bd17..bbcd12950f 100644 > >>> --- a/doc/device-tree-bindings/tpm2/tis-tpm2-spi.txt > >>> +++ b/doc/device-tree-bindings/tpm2/tis-tpm2-spi.txt > >>> @@ -6,7 +6,7 @@ Required properties: > >>> - reg : SPI Chip select > >>> > >>> Optional properties: > >>> -- gpio-reset : Reset GPIO (if not connected to the SoC reset line) > >> > >> As I said you shouldn't remove this. Just extend description that it is > >> deprecated and reset-gpios should be used instead. > > > > I dont really agree with that. IMO we should remove the documentation > > since it is obsolete after this commit and anyone reading it should > > not care about the gpio-reset property. > > Run this on linux kernel and you will see that normal style is to keep > it there. > git grep deprecated Documentation/devicetree/bindings um, ok then (although IMO it only makes sense when we have the tools to parse such documents and apply them at build time)...if you have to read the docs you might just as well change the property. will post later today (I am also adding an additional test to tpm2 to read the rng) > > > > >> > >>> +- reset-gpios : Reset GPIO (if not connected to the SoC reset line) > >>> - spi-max-frequency : See spi-bus.txt > >>> > >>> Example: > >>> diff --git a/drivers/tpm/tpm2_tis_spi.c b/drivers/tpm/tpm2_tis_spi.c > >>> index 4b33ac8fd3..1f9f89f68f 100644 > >>> --- a/drivers/tpm/tpm2_tis_spi.c > >>> +++ b/drivers/tpm/tpm2_tis_spi.c > >>> @@ -589,18 +589,23 @@ static int tpm_tis_spi_probe(struct udevice *dev) > >>> if (CONFIG_IS_ENABLED(DM_GPIO)) { > >>> struct gpio_desc reset_gpio; > >>> > >>> - ret = gpio_request_by_name(dev, "gpio-reset", 0, > >>> + ret = gpio_request_by_name(dev, "reset-gpios", 0, > >>> &reset_gpio, GPIOD_IS_OUT); > >>> if (ret) { > >>> - log(LOGC_NONE, LOGL_NOTICE, "%s: missing reset GPIO\n", > >>> - __func__); > >>> - } else { > >>> - dm_gpio_set_value(&reset_gpio, 1); > >>> - mdelay(1); > >>> - dm_gpio_set_value(&reset_gpio, 0); > >>> + /* legacy reset */ > >>> + ret = gpio_request_by_name(dev, "gpio-reset", 0, > >>> + &reset_gpio, GPIOD_IS_OUT); > >>> + if (ret) { > >>> + log(LOGC_NONE, LOGL_NOTICE, > >>> + "%s: missing reset GPIO\n", __func__); > >>> + goto init; > >>> + } > >> > >> And here it is clear that gpio-reset is used which should deprecated > >> that's why you should print message about it here. > > > > yes, I can do that. makes sense > > > >> > >> > >>> } > >>> + dm_gpio_set_value(&reset_gpio, 1); > >>> + mdelay(1); > >>> + dm_gpio_set_value(&reset_gpio, 0); > >>> } > >> > >> What about this to remove that goto? > > > > um, what is the problem with the goto (IMO tidier than yet another > > conditional); it is not as if this goto is making the code obscure. > > > > with the change below you just removed previous functionality > > (ie indicating that there is no GPIO reset provided, hence why at > > first sight might look cleaner than a goto) > > I tend to use goto unless there is no way around. But up2you. > > M >