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 6E083C27C55 for ; Mon, 10 Jun 2024 16:59:36 +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: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=O2205efKFmGtdFi2Cu/FFOiG0uaRN2OuyRkimuVmfn8=; b=lNmGdk6zYM8XnX xXNO+cuk+pZiT8zsat/wHdG5ZnyW/7ZxZgjbDVT2mmvcyKqPHPZw6XKFUWRuyrNyP36EeNLkMCPBu hKqC+Wd0JBcMmRQ1DRWa9SOFXRp1tXAL/c0dU3qqmiGtbz9Cm6WUTNshyy4ZExqRIxMAy94avpVH/ 0tW8IjJv/uBfNtBMMLfBlPuRPoP2UTx8JPZT40VrcHnF1Zo4vvUsgd0Z9KkDG3JgRT1VEU6fgKsVt zrvOQvaKIO1Q+2CUT4EvpJY2L8QDG4zzWq+XxYtWQpjpXo6w8chgQP9y5Ac3VPFHEQrMclnzeU2Yn hdsPGqodRhDtyqlmstEQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sGiMb-00000005uzb-3FFr; Mon, 10 Jun 2024 16:59:21 +0000 Received: from out-177.mta0.migadu.com ([91.218.175.177]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sGiMY-00000005uxk-0Eec for linux-arm-kernel@lists.infradead.org; Mon, 10 Jun 2024 16:59:19 +0000 X-Envelope-To: sai.krishna.potthuri@amd.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1718038750; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=fdhovgqT7zjoOedUP7hNDAxwuKT33nzYsDI2SddjkRk=; b=coekRUGyb9Y7ezSuJt1zIu+Ti/tusFcIA1RITV+M48P46CbeYbTz5p1uGp3/KiYPQ5lcuR sj7m+gJXWjWLQ9fR0VYHkAczIQRwGkyOJE/SYFUgD1zqngwV6wm/Nl1Q2xIzR+NOkK/c/3 NUBKbBMhw6wgpEtnuS3nKncuNlWmwYc= X-Envelope-To: linus.walleij@linaro.org X-Envelope-To: michal.simek@amd.com X-Envelope-To: linux-gpio@vger.kernel.org X-Envelope-To: linux-kernel@vger.kernel.org X-Envelope-To: andy.shevchenko@gmail.com X-Envelope-To: linux-arm-kernel@lists.infradead.org X-Envelope-To: srinivas.goud@amd.com Message-ID: Date: Mon, 10 Jun 2024 12:59:06 -0400 MIME-Version: 1.0 Subject: Re: [PATCH v2 2/2] pinctrl: zynqmp: Support muxing individual pins To: "Potthuri, Sai Krishna" Cc: Linus Walleij , "Simek, Michal" , "linux-gpio@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Andy Shevchenko , "linux-arm-kernel@lists.infradead.org" , "Goud, Srinivas" References: <20240520150424.2531458-1-sean.anderson@linux.dev> <20240520150424.2531458-3-sean.anderson@linux.dev> Content-Language: en-US X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Sean Anderson In-Reply-To: X-Migadu-Flow: FLOW_OUT X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240610_095918_482900_351597E8 X-CRM114-Status: GOOD ( 34.53 ) 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 Hi Sai Krishna, On 6/10/24 08:16, Potthuri, Sai Krishna wrote: > Hi Sean, > >> -----Original Message----- >> From: Sean Anderson >> Sent: Monday, May 20, 2024 8:34 PM >> To: Linus Walleij ; Simek, Michal >> ; linux-gpio@vger.kernel.org >> Cc: Potthuri, Sai Krishna ; linux- >> kernel@vger.kernel.org; Andy Shevchenko ; >> linux-arm-kernel@lists.infradead.org; Sean Anderson >> >> Subject: [PATCH v2 2/2] pinctrl: zynqmp: Support muxing individual pins >> >> While muxing groups of pins at once can be convenient for large interfaces, >> it can also be rigid. This is because the group is set to all pins which support >> a particular function, even though not all pins may be used. For example, >> the sdhci0 function may be used with a 8-bit eMMC, 4-bit SD card, or even a >> 1-bit SD card. In these cases, the extra pins may be repurposed for other >> uses, but this is not currently allowed. >> >> There is not too much point in pin "groups" when there are not actual pin >> groups at the hardware level. The pins can all be muxed individually, so >> there's no point in adding artificial groups on top. >> Just mux the pins like the hardware allows. >> >> To this effect, add a new group for each pin which can be muxed. These >> groups are part of each function the pin can be muxed to. We treat group >> selectors beyond the number of groups as "pin" groups. To set this up, we >> initialize groups before functions, and then create a bitmap of used pins for >> each function. These used pins are appended to the function's list of >> groups. >> >> Signed-off-by: Sean Anderson >> --- >> >> Changes in v2: >> - Use __set_bit instead of set_bit >> - Use size_add when calculating the number of kcalloc members >> - Expand commit message with some more motivation >> >> drivers/pinctrl/pinctrl-zynqmp.c | 61 ++++++++++++++++++++++---------- >> 1 file changed, 43 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/pinctrl/pinctrl-zynqmp.c b/drivers/pinctrl/pinctrl- >> zynqmp.c >> index 5c46b7d7ebcb..7cc1e43fb07c 100644 >> --- a/drivers/pinctrl/pinctrl-zynqmp.c >> +++ b/drivers/pinctrl/pinctrl-zynqmp.c >> @@ -10,6 +10,7 @@ >> >> #include >> >> +#include >> #include >> #include >> #include >> @@ -97,7 +98,7 @@ static int zynqmp_pctrl_get_groups_count(struct >> pinctrl_dev *pctldev) { >> struct zynqmp_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); >> >> - return pctrl->ngroups; >> + return pctrl->ngroups + zynqmp_desc.npins; >> } >> >> static const char *zynqmp_pctrl_get_group_name(struct pinctrl_dev >> *pctldev, @@ -105,7 +106,10 @@ static const char >> *zynqmp_pctrl_get_group_name(struct pinctrl_dev *pctldev, { >> struct zynqmp_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); >> >> - return pctrl->groups[selector].name; >> + if (selector < pctrl->ngroups) >> + return pctrl->groups[selector].name; >> + >> + return zynqmp_desc.pins[selector - pctrl->ngroups].name; >> } >> >> static int zynqmp_pctrl_get_group_pins(struct pinctrl_dev *pctldev, @@ - >> 115,8 +119,13 @@ static int zynqmp_pctrl_get_group_pins(struct >> pinctrl_dev *pctldev, { >> struct zynqmp_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); >> >> - *pins = pctrl->groups[selector].pins; >> - *npins = pctrl->groups[selector].npins; >> + if (selector < pctrl->ngroups) { >> + *pins = pctrl->groups[selector].pins; >> + *npins = pctrl->groups[selector].npins; >> + } else { >> + *pins = &zynqmp_desc.pins[selector - pctrl- >> >ngroups].number; >> + *npins = 1; >> + } >> >> return 0; >> } >> @@ -560,10 +569,12 @@ static int >> zynqmp_pinctrl_prepare_func_groups(struct device *dev, u32 fid, { >> u16 resp[NUM_GROUPS_PER_RESP] = {0}; >> const char **fgroups; >> - int ret, index, i; >> + int ret, index, i, pin; >> + unsigned int npins; >> + unsigned long *used_pins __free(bitmap) = >> + bitmap_zalloc(zynqmp_desc.npins, GFP_KERNEL); >> >> - fgroups = devm_kcalloc(dev, func->ngroups, sizeof(*fgroups), >> GFP_KERNEL); >> - if (!fgroups) >> + if (!used_pins) >> return -ENOMEM; >> >> for (index = 0; index < func->ngroups; index += >> NUM_GROUPS_PER_RESP) { @@ -578,23 +589,37 @@ static int >> zynqmp_pinctrl_prepare_func_groups(struct device *dev, u32 fid, >> if (resp[i] == RESERVED_GROUP) >> continue; >> >> - fgroups[index + i] = devm_kasprintf(dev, >> GFP_KERNEL, >> - "%s_%d_grp", >> - func->name, >> - index + i); >> - if (!fgroups[index + i]) >> - return -ENOMEM; >> - >> groups[resp[i]].name = devm_kasprintf(dev, >> GFP_KERNEL, >> "%s_%d_grp", >> func->name, >> index + i); >> if (!groups[resp[i]].name) >> return -ENOMEM; >> + >> + for (pin = 0; pin < groups[resp[i]].npins; pin++) >> + __set_bit(groups[resp[i]].pins[pin], >> used_pins); >> } >> } >> done: >> + npins = bitmap_weight(used_pins, zynqmp_desc.npins); >> + fgroups = devm_kcalloc(dev, size_add(func->ngroups, npins), >> + sizeof(*fgroups), GFP_KERNEL); >> + if (!fgroups) >> + return -ENOMEM; >> + >> + for (i = 0; i < func->ngroups; i++) { >> + fgroups[i] = devm_kasprintf(dev, GFP_KERNEL, "%s_%d_grp", >> + func->name, i); >> + if (!fgroups[i]) >> + return -ENOMEM; >> + } >> + >> + pin = 0; >> + for_each_set_bit(pin, used_pins, zynqmp_desc.npins) >> + fgroups[i++] = zynqmp_desc.pins[pin].name; >> + >> func->groups = fgroups; >> + func->ngroups += npins; >> >> return 0; >> } >> @@ -772,6 +797,10 @@ static int >> zynqmp_pinctrl_prepare_function_info(struct device *dev, >> if (!groups) >> return -ENOMEM; >> >> + ret = zynqmp_pinctrl_prepare_group_pins(dev, groups, pctrl- >> >ngroups); >> + if (ret) >> + return ret; >> + >> for (i = 0; i < pctrl->nfuncs; i++) { >> ret = zynqmp_pinctrl_prepare_func_groups(dev, i, &funcs[i], >> groups); >> @@ -779,10 +808,6 @@ static int >> zynqmp_pinctrl_prepare_function_info(struct device *dev, >> return ret; >> } >> >> - ret = zynqmp_pinctrl_prepare_group_pins(dev, groups, pctrl- >> >ngroups); >> - if (ret) >> - return ret; >> - >> pctrl->funcs = funcs; >> pctrl->groups = groups; >> > While testing this patch, observed that some more changes required in the > other functions like set_mux, pin_config_group_set. Pasted the diff below. Thanks for catching this. I will incorporate these changes into v3. > diff --git a/drivers/pinctrl/pinctrl-zynqmp.c b/drivers/pinctrl/pinctrl-zynqmp.c > index 636e56f7cd92..c82074819be3 100644 > --- a/drivers/pinctrl/pinctrl-zynqmp.c > +++ b/drivers/pinctrl/pinctrl-zynqmp.c > @@ -206,12 +206,18 @@ static int zynqmp_pinmux_set_mux(struct pinctrl_dev *pctldev, > unsigned int function, > unsigned int group) > { > - struct zynqmp_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); > - const struct zynqmp_pctrl_group *pgrp = &pctrl->groups[group]; > + const unsigned *pins; > + unsigned npins; > int ret, i; > > - for (i = 0; i < pgrp->npins; i++) { > - unsigned int pin = pgrp->pins[i]; > + ret = zynqmp_pctrl_get_group_pins(pctldev, group, &pins, &npins); > + if (ret) { This function cannot fail, so we don't need to check the return. > + dev_err(pctldev->dev, "Get group pins failed for group %u\n", group); > + return ret; > + } > + > + for (i = 0; i < npins; i++) { > + unsigned int pin = pins[i]; > > ret = zynqmp_pm_pinctrl_set_function(pin, function); > if (ret) { > @@ -476,13 +482,18 @@ static int zynqmp_pinconf_group_set(struct pinctrl_dev *pctldev, > unsigned long *configs, > unsigned int num_configs) > { > - int i, ret; > - struct zynqmp_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); > - const struct zynqmp_pctrl_group *pgrp = &pctrl->groups[selector]; > - > - for (i = 0; i < pgrp->npins; i++) { > - ret = zynqmp_pinconf_cfg_set(pctldev, pgrp->pins[i], configs, > - num_configs); > + const unsigned *pins; > + unsigned npins; > + int i, ret; > + > + ret = zynqmp_pctrl_get_group_pins(pctldev, selector, &pins, &npins); > + if (ret) { ditto > + dev_err(pctldev->dev, "Get group pins failed for group %u\n", selector); > + return ret; > + } > + > + for (i = 0; i < npins; i++) { > + ret = zynqmp_pinconf_cfg_set(pctldev, pins[i], configs, num_configs); > if (ret) > return ret; > } > > Regards > Sai Krishna --Sean _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel