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 3A864C7EE23 for ; Tue, 23 May 2023 09:26:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235795AbjEWJ0g (ORCPT ); Tue, 23 May 2023 05:26:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49928 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235175AbjEWJ0f (ORCPT ); Tue, 23 May 2023 05:26:35 -0400 Received: from mail-ej1-x62d.google.com (mail-ej1-x62d.google.com [IPv6:2a00:1450:4864:20::62d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 35970109 for ; Tue, 23 May 2023 02:26:30 -0700 (PDT) Received: by mail-ej1-x62d.google.com with SMTP id a640c23a62f3a-953343581a4so1065181066b.3 for ; Tue, 23 May 2023 02:26:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20221208.gappssmtp.com; s=20221208; t=1684833988; x=1687425988; h=in-reply-to:references:cc:to:from:subject:message-id:date :content-transfer-encoding:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=8F5s7KNQ8pR0l7gVQkr0wi0q05FJrKQ/4O2Ot0cPFmo=; b=bkb1ITlRNG6DYkPUwBGpIQCQlJBS64KcKl7OcxEhlDKQU0+X8WKtxMNEKF8ddlVep1 Qn2Oq4taRlVPpzuJdBdHwMyzXmwG3l6U2ljLciexshvbqv0B9cY0AikK3nB/kwJLqUfV KiVLxulUyVSEkjIkH9Dx4xj4CnFOnmPhFXgQzVuw44mKiPF9J5/i3GxdKYRmvZdlSxxg ubDmVn4dyaSsOs/jgmurwsjM0/lnsCf+376OhsQCiXvDkykEyyvhGnGAS3tXpgZyNqzC noCdtscq3Ni3JGtEBesgwDqyuA+my0a3gtZqWpuKQsSVqIcTm5yN1pZIUcMfdAYEtg+t h29A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684833988; x=1687425988; h=in-reply-to:references:cc:to:from:subject:message-id:date :content-transfer-encoding:mime-version:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=8F5s7KNQ8pR0l7gVQkr0wi0q05FJrKQ/4O2Ot0cPFmo=; b=JUK/RADYs1qJ7xGRqnu4H4undD1hclA/kG8hlNxFVUDk46rOF0Prj2f+1fR3ho9K+P RUDWKnQmHQGaHlW7r2t/KREZo7OFX+0f6azPzGTIX87pAvuXsf7nToxj0BSdIZSumMd5 lZ2CRVLkA4/6KhqiSFYcHu7haUoMsbKS8TrvHmsNF0OKslNT4gxFBsn1YENOsxd5Mo45 bvWyTh3uUXkv8CFfGCqTSF/MbNdegfi9AF1Pe0AYvStAv+e3bRGmbjCayeM+6BxwYDaS fPtrhevm9EUaytaeWJ8NAt6WvTpm2+OTnHiNTG1d1vhfbMVdnMMLgibP+W4KtWaSwNH8 HSig== X-Gm-Message-State: AC+VfDzO4ThGx76me1eHpHbAbA2Us2fG5mA9s2e5OZzA9E93CGE/ovu0 7Ew6UyGG1qf2WuYuhzZqorZmdg== X-Google-Smtp-Source: ACHHUZ7QRnooOds/zXLfgK+9lidBQnpV71o40aikCyZ11/YFivdpecTnbUZ5s8ET0k2VWK+tmhcJGQ== X-Received: by 2002:a17:906:fd8e:b0:96a:f688:db6e with SMTP id xa14-20020a170906fd8e00b0096af688db6emr11577426ejb.74.1684833988681; Tue, 23 May 2023 02:26:28 -0700 (PDT) Received: from localhost (abordeaux-655-1-129-86.w90-5.abo.wanadoo.fr. [90.5.10.86]) by smtp.gmail.com with ESMTPSA id x8-20020a1709064a8800b00965f5d778e3sm4194556eju.120.2023.05.23.02.26.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 23 May 2023 02:26:28 -0700 (PDT) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Tue, 23 May 2023 11:26:26 +0200 Message-Id: Subject: Re: [PATCH v4 2/3] pinctrl: tps6594: Add driver for TPS6594 pinctrl and GPIOs From: "Esteban Blanc" To: "Andy Shevchenko" Cc: , , , , , , , , , , , , X-Mailer: aerc 0.14.0 References: <20230512141755.1712358-1-eblanc@baylibre.com> <20230512141755.1712358-3-eblanc@baylibre.com> In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org On Wed May 17, 2023 at 5:04 PM CEST, Andy Shevchenko wrote: > On Wed, May 17, 2023 at 5:43=E2=80=AFPM Esteban Blanc wrote: > > On Wed May 17, 2023 at 3:51 PM CEST, Andy Shevchenko wrote: > > > On Wed, May 17, 2023 at 12:58=E2=80=AFPM Esteban Blanc wrote: > > > > On Tue May 16, 2023 at 6:48 PM CEST, Andy Shevchenko wrote: > > > > > On Tue, May 16, 2023 at 4:05=E2=80=AFPM Esteban Blanc wrote: > > > > > > On Fri May 12, 2023 at 7:07 PM CEST, wrote: > > > > > > > Fri, May 12, 2023 at 04:17:54PM +0200, Esteban Blanc kirjoitt= i: > > ... > > > > > > > > > -#define TPS6594_REG_GPIOX_CONF(gpio_inst) (0x31 += (gpio_inst)) > > > > > > > > +#define TPS6594_REG_GPIO1_CONF = 0x31 > > > > > > > > +#define TPS6594_REG_GPIOX_CONF(gpio_inst) (TPS6594_REG_GP= IO1_CONF + (gpio_inst)) > > > > > > > > > > > > > > Why? The original code with parameter 0 will issue the same. > > > > > > > > > > > > I felt that replacing 0x31 with a constant would make the compu= tation > > > > > > in TPS6594_REG_GPIOX_CONFIG more understandable. What do you th= ink? > > > > > > > > > > The question is why that register is so special that you need to = have > > > > > it as a constant explicitly? > > > > > > > > It is not special, it's just the first one of the serie of config > > > > registers. I felt like just having 0x31 without context was a bit w= eird > > > > > > I'm not sure I understand what 'context' you are talking about. > > I was trying to convey the fact that 0x31 was representing > > TPS6594_REG_GPIO1_CONF address. This way when looking at > > TPS6594_REG_GPIOX_CONF(...), one will better understand that this macro > > is just about offsetting from the first GPIO_CONF register. > > You can add a comment on top of the macro, so anybody can read and see > what this macro is doing. > > > This is pretty normal to have two kind of definitions (depending on t= he case): > > > 1/ > > > > > > #define FOO_1 ... > > > #define FOO_2 ... > > > > > > and so on > > > > > > 2/ > > > > > > #define FOO(x) (... (x) ...) > > > > > > Having a mix of them seems quite unusual. > > I did not know that. I will revert this change for next version then. > > Don't get me wrong, it's possible to have, but since it's unusual it > needs to be well justified. In the change you proposed you have > changed that, but I haven't seen where the new definition is used (in > *.c files). Actualy it used in 2 places: - In the switch case of `tps6594_gpio_regmap_xlate` - In `tps6594_pinctrl_probe` when setting `reg_dir_out_base` I already sent a v5 with this change but I managed to fail my .config and this driver was not compiled... and it is not compiling... I feel so stupid. I need to send a v6 now anyway. Should I convert all TPS6594_REG_GPIO1_CONF to TPS6594_REG_GPIOX_CONF(0)?