Message ID | fe4c15dcc3074412326b8dc296b0cbccf79c49bf.1701422582.git.namcao@linutronix.de |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp990066vqy; Fri, 1 Dec 2023 01:24:00 -0800 (PST) X-Google-Smtp-Source: AGHT+IGxmEN5De7PHCgOy9abCooHRj9684e5rvAiPnRbAVzn+U9YJRD9Dbfp0dHvEpeXaXP8Xrsa X-Received: by 2002:a9d:7610:0:b0:6d8:6b36:97e7 with SMTP id k16-20020a9d7610000000b006d86b3697e7mr1332375otl.21.1701422640436; Fri, 01 Dec 2023 01:24:00 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701422640; cv=none; d=google.com; s=arc-20160816; b=GKXhQqlGr8jIHrgy1rWhw8F0W+qBoLVvCJnH/yLqVIF6m0iOS7hcNqwCdDxtLTRWKa Dzh9gX0Fle0yncj34ULhsVJCQPH1p8GSe5+UvdsZCxRFwKZ3Zh1FanmJjgukW6GZapgl t+TPzH5JsJ9F24pw9WjPakBcZPQ1bnw58dFKRTHPBVDNcqi0S6LhU51snny0w5v0El/w DEt1Fj/mh7hgLdLR0pP8cuA2gu2O10OVNZsKsyrQYOGC4+Bxa4ohvl7lXpdXs929czcB vp3oBsaDfV01kMG80qdG2JZSsRz2wNfy9ydMOt5/4kYuGf1UBupMviPr6oL0VkUKaFVo WVzg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:dkim-signature :dkim-signature:from; bh=nAuB+NWb1+h9nrirqejk/RLzcirj6WRcoSRAbARMZUA=; fh=IKK/SIZntHUrBqr6dWt/U2zb+FK4JqPWDhPbeJaAFoI=; b=i8YIQdshNo90bMg93R6aeLnQhG0Nfv3GDYtfZ79xGHWA8f3LWte/UpKsyRev7QmtEN IraB7EBwPXaOPEPftba0ufiAyjPNbm3X/B1XyQll23de5tf4Ln3AkrJbKyXZ7BTOCfHs DOgzqgl4FyYKwn9JrNE5BF2LBxoMmQDSDg7v6IyHAm7VUhY7xxTHuRjfLm6FBYM6V6Z4 tSzh7AkiAxi2ru1nskwx2FVWTul8Sf2IwPZceIsEsE3Uee/0y/kT1Iwb1itYc7cXw9Aq J0sqEzYkOSFzbAYPq6x+6Qbbkzjg9WPLLrhj/XdJYfuP0ImELovhnm2erNneKatwj9f6 GpgA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=tGDR9eZq; dkim=neutral (no key) header.i=@linutronix.de header.b=A9ARbpvm; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: from fry.vger.email (fry.vger.email. [2620:137:e000::3:8]) by mx.google.com with ESMTPS id y191-20020a638ac8000000b005b9b68add9asi2922133pgd.255.2023.12.01.01.23.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 01 Dec 2023 01:24:00 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8; Authentication-Results: mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=tGDR9eZq; dkim=neutral (no key) header.i=@linutronix.de header.b=A9ARbpvm; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by fry.vger.email (Postfix) with ESMTP id C91658108378; Fri, 1 Dec 2023 01:23:57 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at fry.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1378009AbjLAJXc (ORCPT <rfc822;ruipengqi7@gmail.com> + 99 others); Fri, 1 Dec 2023 04:23:32 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48510 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1377992AbjLAJXa (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 1 Dec 2023 04:23:30 -0500 Received: from galois.linutronix.de (Galois.linutronix.de [IPv6:2a0a:51c0:0:12e:550::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B37A2194; Fri, 1 Dec 2023 01:23:36 -0800 (PST) From: Nam Cao <namcao@linutronix.de> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1701422615; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=nAuB+NWb1+h9nrirqejk/RLzcirj6WRcoSRAbARMZUA=; b=tGDR9eZqdwVBv7/g90Ct9HkZutigEwF7ngGXBC+BJCXQQoFHN4SZeJEcPAk1kEfZK6uhUm jev2BdKZEWpF96N55lxD2jYDQO8NnCDrFcmrDHV9VSQg2CvYF1KJoXjQS3Ih7oTZTIgTQb p+1WMxrsmgMvM6U3hx8fQKu01nBI5+XqdYV1RBPRjOQ6OHIULZ9D9NIGTKEcGo4pidOeT6 Rcq7KgFbkeumyAQzK+S8hG2IvvAI3FV21O9L9iJSbHl1tyzcnjjYNy9qJF9yxC3sQzbjGP IoepgDGclqu+dwbYuaRJr0BcAsz35D9078zP+x+vnZeL/Nr4u710h/8ErzjxSQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1701422615; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=nAuB+NWb1+h9nrirqejk/RLzcirj6WRcoSRAbARMZUA=; b=A9ARbpvm7QOTKyyBB7JLD1eW7y/nyFyioCc01jf3h7pElP7NZmdrLwXNwXLV59pcm5uQGw X5zAVFsE//77j/Cw== To: Emil Renner Berthing <kernel@esmil.dk>, Jianlong Huang <jianlong.huang@starfivetech.com>, Hal Feng <hal.feng@starfivetech.com>, Linus Walleij <linus.walleij@linaro.org>, Huan Feng <huan.feng@starfivetech.com>, Andy Shevchenko <andy.shevchenko@gmail.com>, Drew Fustini <drew@beagleboard.org>, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Nam Cao <namcao@linutronix.de>, stable@vger.kernel.org Subject: [PATCH 2/2] pinctrl: starfive: jh7100: ignore disabled device tree nodes Date: Fri, 1 Dec 2023 10:23:29 +0100 Message-Id: <fe4c15dcc3074412326b8dc296b0cbccf79c49bf.1701422582.git.namcao@linutronix.de> In-Reply-To: <fd8bf044799ae50a6291ae150ef87b4f1923cacb.1701422582.git.namcao@linutronix.de> References: <fd8bf044799ae50a6291ae150ef87b4f1923cacb.1701422582.git.namcao@linutronix.de> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Fri, 01 Dec 2023 01:23:57 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1784070946595035362 X-GMAIL-MSGID: 1784070946595035362 |
Series |
[1/2] pinctrl: starfive: jh7110: ignore disabled device tree nodes
|
|
Commit Message
Nam Cao
Dec. 1, 2023, 9:23 a.m. UTC
The driver always registers pin configurations in device tree. This can
cause some inconvenience to users, as pin configurations in the base
device tree cannot be disabled in the device tree overlay, even when the
relevant devices are not used.
Ignore disabled pin configuration nodes in device tree.
Fixes: ec648f6b7686 ("pinctrl: starfive: Add pinctrl driver for StarFive SoCs")
Cc: stable@vger.kernel.org
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Comments
Nam Cao wrote: > The driver always registers pin configurations in device tree. This can > cause some inconvenience to users, as pin configurations in the base > device tree cannot be disabled in the device tree overlay, even when the > relevant devices are not used. > > Ignore disabled pin configuration nodes in device tree. > > Fixes: ec648f6b7686 ("pinctrl: starfive: Add pinctrl driver for StarFive SoCs") > Cc: stable@vger.kernel.org > Signed-off-by: Nam Cao <namcao@linutronix.de> > --- > drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c b/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c > index 530fe340a9a1..561fd0c6b9b0 100644 > --- a/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c > +++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c > @@ -492,7 +492,7 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev, > > nmaps = 0; > ngroups = 0; > - for_each_child_of_node(np, child) { > + for_each_available_child_of_node(np, child) { Hi Nam, Is this safe to do? I mean will the children considered "available" not change as drivers are loaded during boot so this is racy? Also arguably this is not a bugfix, but a new feature. Same comments apply to the JH7110 patch. /Emil > int npinmux = of_property_count_u32_elems(child, "pinmux"); > int npins = of_property_count_u32_elems(child, "pins"); > > @@ -527,7 +527,7 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev, > nmaps = 0; > ngroups = 0; > mutex_lock(&sfp->mutex); > - for_each_child_of_node(np, child) { > + for_each_available_child_of_node(np, child) { > int npins; > int i; > > -- > 2.39.2 >
Emil Renner Berthing wrote: > Nam Cao wrote: > > The driver always registers pin configurations in device tree. This can > > cause some inconvenience to users, as pin configurations in the base > > device tree cannot be disabled in the device tree overlay, even when the > > relevant devices are not used. > > > > Ignore disabled pin configuration nodes in device tree. > > > > Fixes: ec648f6b7686 ("pinctrl: starfive: Add pinctrl driver for StarFive SoCs") > > Cc: stable@vger.kernel.org > > Signed-off-by: Nam Cao <namcao@linutronix.de> > > --- > > drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c b/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c > > index 530fe340a9a1..561fd0c6b9b0 100644 > > --- a/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c > > +++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c > > @@ -492,7 +492,7 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev, > > > > nmaps = 0; > > ngroups = 0; > > - for_each_child_of_node(np, child) { > > + for_each_available_child_of_node(np, child) { > > Hi Nam, > > Is this safe to do? I mean will the children considered "available" not change > as drivers are loaded during boot so this is racy? I just noticed the Allwinner D1 device trees use /omit-if-no-ref/ in front of the pin group nodes. I think all current pin group nodes (for the JH7100 at least) are used by some peripheral, so if you're removing peripherals from the device tree you should be removing the reference too and this scheme should work for you. /Emil
Hi Emil, On Fri, Dec 01, 2023 at 03:28:27PM +0100, Emil Renner Berthing wrote: > Nam Cao wrote: > > diff --git a/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c b/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c > > index 530fe340a9a1..561fd0c6b9b0 100644 > > --- a/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c > > +++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c > > @@ -492,7 +492,7 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev, > > > > nmaps = 0; > > ngroups = 0; > > - for_each_child_of_node(np, child) { > > + for_each_available_child_of_node(np, child) { > > Is this safe to do? I mean will the children considered "available" not change > as drivers are loaded during boot so this is racy? I think if node removal like this causes race condition, we would already have race condition with node addition too: "what if the nodes are added while the drivers are being loaded?" At least with U-Boot, the device tree overlay is "merged" into the base device tree, before the kernel even runs, so no race there. I don't know if there are any cases where the device tree overlay is not guaranteed to be applied before driver loading, but those cases do not sound sane to me: they would cause race condition, regardless of whether nodes are added or removed. > Also arguably this is not a bugfix, but a new feature. I'm not sure myself, I haven't seen official documentation/rules about this. But many people do consider this to be a bug: "Though you can add/override 'status' with 'status = "disabled";' which should be treated very similar to a node not being present. I say similar because it's a source of bugs for the OS to fail to pay attention to status property." - Rob Herring [1]. "Linux has widespread use of the "status" property to indicate that a node does not exist. (...). Expect efforts to fix the kernel code to respect the "status" property." - elinux.org [2]. And I do agree with them. When someone write a device tree with some nodes with "status = disabled" for whatever reasons, clearly they intend to exclude these nodes. Though I must admit that I am still quite new, so please correct me if my reasoning/understanding is flawed. Best regards, Nam [1] https://lore.kernel.org/lkml/CAL_JsqLV5d5cL3o3Dx=--zGD37c5O09rL9AXyRFmceTfBHt3Zg@mail.gmail.com/ [2] https://elinux.org/Device_Tree_Linux#status_property
On Fri, Dec 01, 2023 at 03:43:04PM +0100, Emil Renner Berthing wrote: > I just noticed the Allwinner D1 device trees use /omit-if-no-ref/ in front of > the pin group nodes. I think all current pin group nodes (for the JH7100 at > least) are used by some peripheral, so if you're removing peripherals from the > device tree you should be removing the reference too and this scheme should > work for you. If I am not mistaken, /omit-if-no-ref/ (and similar stuffs like /delete-node/ and /delete-property/) is only for compile-time node removal. It doesn't do anything with device tree overlay. Best regards, Nam
On Fri, Dec 1, 2023 at 10:23 AM Nam Cao <namcao@linutronix.de> wrote: > The driver always registers pin configurations in device tree. This can > cause some inconvenience to users, as pin configurations in the base > device tree cannot be disabled in the device tree overlay, even when the > relevant devices are not used. > > Ignore disabled pin configuration nodes in device tree. > > Fixes: ec648f6b7686 ("pinctrl: starfive: Add pinctrl driver for StarFive SoCs") > Cc: stable@vger.kernel.org > Signed-off-by: Nam Cao <namcao@linutronix.de> Patch applied for fixes. If there is some doubts about the saneness of this patch, seek input from the devicetree maintainers. Yours, Linus Walleij
diff --git a/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c b/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c index 530fe340a9a1..561fd0c6b9b0 100644 --- a/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c +++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c @@ -492,7 +492,7 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev, nmaps = 0; ngroups = 0; - for_each_child_of_node(np, child) { + for_each_available_child_of_node(np, child) { int npinmux = of_property_count_u32_elems(child, "pinmux"); int npins = of_property_count_u32_elems(child, "pins"); @@ -527,7 +527,7 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev, nmaps = 0; ngroups = 0; mutex_lock(&sfp->mutex); - for_each_child_of_node(np, child) { + for_each_available_child_of_node(np, child) { int npins; int i;