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 397FDC47DD9 for ; Fri, 22 Mar 2024 16:03:35 +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-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:References:Cc:To:From: Subject:Message-Id:Date:Mime-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=wJy0zFvLx5DHFiIMRWHG1MgmJu+L7V+QzZr6WgOWtzs=; b=Bt2TW9Stajg2Cs FeE6OFE5VZIVLri74/2zijC2DDLQ1sLUjjDbOc1dGY3eQSmYddh65Tp4JhEPnzGOet8YZGhmzkPIF CKU6sQ+KJ34sHi8MwACsNrCIYZf3OYSLUvNOT/tLbX4gOqGM3qPVPWk7Iav142ar5iQV56JFD/YBm QAlqw+07ym0arqF+cGJ+oIZf+ZAXug1oE4UnumFGrUqUunMrW+GztGHuA84Kvfyg8Ong9dwHrZ1tV X+49qxrqJkiqOredAJzxehTUINi8A4/tmhLm7sNZhpFJMQGTBIs4Fi7nkS28u2ZDvOAdd9McZoBpV D4pXD43zQpONSDqtelbg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rnhMS-00000007rbU-3GMz; Fri, 22 Mar 2024 16:03:16 +0000 Received: from mail-lj1-x22f.google.com ([2a00:1450:4864:20::22f]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rnhMP-00000007rZI-0JKT for linux-arm-kernel@lists.infradead.org; Fri, 22 Mar 2024 16:03:15 +0000 Received: by mail-lj1-x22f.google.com with SMTP id 38308e7fff4ca-2d47a92cfefso28519851fa.1 for ; Fri, 22 Mar 2024 09:03:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1711123390; x=1711728190; darn=lists.infradead.org; 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=Kh0B0Op/7iFcRFrIOoiJQuhYzDeOdAJ2QNUlBpVkvVY=; b=cpt9SQz7hbit8CWY0qQA4bMCa1lprkkNiEH/V3NGEvMp7tIrd+WQKrW97t/xnEsHGZ M2npBxPckhK+UoIUdWWZE+n8ynogV7v2Z+alPRTwO1jeJJC/meRjUqujdncl31BmG78y kE77CQRlMGcDyy9AEVz8yjGtSuQfErV1cZmtmeJWwuryM+doptS5kWxouDGz5QreTJyN ClyQUNeeTA4EOfEh6zopVs+/GyHWEqbdUIveHAHeGa5akIOXJK2vY3TezhRmiIh1ttP0 5jOQEZ9GyL8NHt7ZwIvWko9LbxkBs6eWy9rqlj847B9/j8+vXLxSxuscGxl7HoGgiaeo 2l5w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711123390; x=1711728190; 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=Kh0B0Op/7iFcRFrIOoiJQuhYzDeOdAJ2QNUlBpVkvVY=; b=rkrlY6LpeVB7MzfepjZZZavYEwBkIiAM2SPcW+dElsKkaKrcJ4mKDhYgY3sCGNUYO/ bVbpoVjwppsXQB3yj8HlL303n3/l5UrS4wxNL6o9YBXixPZAPPiWQOQ5STk2oQ7biPZ2 K7f7uzsSiR4vpNps26Pn6PsllIEVyBUKSkt3JyTOgpw02YOkfBnS6KQwbfPb1GnRCubO +cldEGWuiA6+wqzEz5GTe6HUZDVpxUBYG2o7Dc6mscSwC2djOjN2qyuQhQabD0bGinup n7bRdMdn5UsjTlvUt9MuI69WvjFeY7EnzYvip6azXoKtlfkRemU6XJoNng+BVC3bQ3uY zYbw== X-Forwarded-Encrypted: i=1; AJvYcCXcIej/hvjYtwwdn1p6o0u8Nl8UNRuoBr8Tt1NLW5cs0sDDRx1+gqGoQ3NWHlAZuRiMzz6si+TTPS7O7gSUr8gFGMB3BDtyT86CNNMF7MyMLNzfXY4= X-Gm-Message-State: AOJu0YwnI/Kv+fMHyLvExDPvQvcaClc75gCEs43zPIaO+zVCblre+VdK Kp/zxtRbI1a6wuoLaoitUSGjOZv5ROXzb/JJHaltPZapXUJtqfLyftumStDvoVA= X-Google-Smtp-Source: AGHT+IEkR6PizaNuC62j5Grot13csXnQY6gVrSzN5nx/GfkID77oL8KO3GnXgbM82pcwsLxNamFMfQ== X-Received: by 2002:a2e:9f4c:0:b0:2d4:6910:2ee5 with SMTP id v12-20020a2e9f4c000000b002d469102ee5mr2129385ljk.8.1711123390044; Fri, 22 Mar 2024 09:03:10 -0700 (PDT) Received: from localhost (alyon-651-1-22-137.w82-122.abo.wanadoo.fr. [82.122.123.137]) by smtp.gmail.com with ESMTPSA id g11-20020a05600c310b00b0041408e16e6bsm3472839wmo.25.2024.03.22.09.03.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 22 Mar 2024 09:03:09 -0700 (PDT) Mime-Version: 1.0 Date: Fri, 22 Mar 2024 17:03:08 +0100 Message-Id: Subject: Re: [PATCH v4 10/11] pinctrl: pinctrl-tps6594: Add TPS65224 PMIC pinctrl and GPIO From: "Esteban Blanc" To: "Bhargav Raviprakash" , Cc: , , , , , , , , , , , , , , , , X-Mailer: aerc 0.15.2 References: <20240320102559.464981-1-bhargav.r@ltts.com> <20240320102559.464981-11-bhargav.r@ltts.com> In-Reply-To: <20240320102559.464981-11-bhargav.r@ltts.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240322_090313_150742_69AC2D56 X-CRM114-Status: GOOD ( 20.29 ) 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-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 Wed Mar 20, 2024 at 11:25 AM CET, Bhargav Raviprakash wrote: > From: Nirmala Devi Mal Nadar > > Add support for TPS65224 pinctrl and GPIOs to TPS6594 driver as they have > significant functional overlap. > TPS65224 PMIC has 6 GPIOS which can be configured as GPIO or other > dedicated device functions. > > Signed-off-by: Nirmala Devi Mal Nadar > Signed-off-by: Bhargav Raviprakash > Acked-by: Linus Walleij > --- > drivers/pinctrl/pinctrl-tps6594.c | 258 +++++++++++++++++++++++++----- > 1 file changed, 215 insertions(+), 43 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-tps6594.c b/drivers/pinctrl/pinctrl-tps6594.c > index 66985e54b..db0f5d2a8 100644 > --- a/drivers/pinctrl/pinctrl-tps6594.c > +++ b/drivers/pinctrl/pinctrl-tps6594.c > @@ -320,8 +451,18 @@ static int tps6594_pinctrl_probe(struct platform_device *pdev) > return -ENOMEM; > pctrl_desc->name = dev_name(dev); > pctrl_desc->owner = THIS_MODULE; > - pctrl_desc->pins = tps6594_pins; > - pctrl_desc->npins = ARRAY_SIZE(tps6594_pins); > + switch (tps->chip_id) { > + case TPS65224: > + pctrl_desc->pins = tps65224_pins; > + pctrl_desc->npins = ARRAY_SIZE(tps65224_pins); > + break; > + case TPS6594: > + pctrl_desc->pins = tps6594_pins; > + pctrl_desc->npins = ARRAY_SIZE(tps6594_pins); > + break; > + default: > + break; > + } > pctrl_desc->pctlops = &tps6594_pctrl_ops; > pctrl_desc->pmxops = &tps6594_pmx_ops; See below. > @@ -329,8 +470,28 @@ static int tps6594_pinctrl_probe(struct platform_device *pdev) > if (!pinctrl) > return -ENOMEM; > pinctrl->tps = dev_get_drvdata(dev->parent); > - pinctrl->funcs = pinctrl_functions; > - pinctrl->pins = tps6594_pins; > + switch (pinctrl->tps->chip_id) { You could use tps->chip_id like in the previous switch. > + case TPS65224: > + pinctrl->funcs = tps65224_pinctrl_functions; > + pinctrl->func_cnt = ARRAY_SIZE(tps65224_pinctrl_functions); > + pinctrl->pins = tps65224_pins; > + pinctrl->num_pins = ARRAY_SIZE(tps65224_pins); > + pinctrl->mux_sel_mask = TPS65224_MASK_GPIO_SEL; > + pinctrl->remap = tps65224_muxval_remap; > + pinctrl->remap_cnt = ARRAY_SIZE(tps65224_muxval_remap); > + break; > + case TPS6594: > + pinctrl->funcs = pinctrl_functions; This should be tps6594_pinctrl_functions > + pinctrl->func_cnt = ARRAY_SIZE(pinctrl_functions); > + pinctrl->pins = tps6594_pins; > + pinctrl->num_pins = ARRAY_SIZE(tps6594_pins); > + pinctrl->mux_sel_mask = TPS6594_MASK_GPIO_SEL; > + pinctrl->remap = tps6594_muxval_remap; > + pinctrl->remap_cnt = ARRAY_SIZE(tps6594_muxval_remap); > + break; > + default: > + break; > + } See blow. > pinctrl->pctl_dev = devm_pinctrl_register(dev, pctrl_desc, pinctrl); > if (IS_ERR(pinctrl->pctl_dev)) > return dev_err_probe(dev, PTR_ERR(pinctrl->pctl_dev), > @@ -338,8 +499,18 @@ static int tps6594_pinctrl_probe(struct platform_device *pdev) > > config.parent = tps->dev; > config.regmap = tps->regmap; > - config.ngpio = TPS6594_PINCTRL_PINS_NB; > - config.ngpio_per_reg = 8; > + switch (pinctrl->tps->chip_id) { Same here, use tps->chip_id > + case TPS65224: > + config.ngpio = ARRAY_SIZE(tps65224_gpio_func_group_names); > + config.ngpio_per_reg = TPS65224_NGPIO_PER_REG; > + break; > + case TPS6594: > + config.ngpio = ARRAY_SIZE(tps6594_gpio_func_group_names); > + config.ngpio_per_reg = TPS6594_NGPIO_PER_REG; > + break; > + default: > + break; > + } > config.reg_dat_base = TPS6594_REG_GPIO_IN_1; > config.reg_set_base = TPS6594_REG_GPIO_OUT_1; > config.reg_dir_out_base = TPS6594_REG_GPIOX_CONF(0); Regarding all the switch case, they should be use to set all the struct fields that are known at runtime only. For example, pinctrl->funcs, and pinctrl->func_cnt are known at compile time. You should create template structs, one for TPS6594 the other TPS65224, initialise the allocated struct with the template and then fill the remaining fields with the runtime values. Something like this: ```c struct test { int a; int *b; }; static struct test template = { .a = 42, }; int main(void) { struct test *test = malloc(sizeof(*test)); *test = sample; test->b = NULL; return 0; } ``` You could also try to reduce the number of switch case, there is no good reason to have 2 switch instead of one for pctrl_desc and pinctrl structs. Best regards, -- Esteban "Skallwar" Blanc BayLibre _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel