From patchwork Thu Jan 10 22:07:43 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Warren X-Patchwork-Id: 1962841 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork1.kernel.org (Postfix) with ESMTP id 9D4DA3FF0F for ; Thu, 10 Jan 2013 22:11:53 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1TtQHw-0001Rn-BT; Thu, 10 Jan 2013 22:07:52 +0000 Received: from avon.wwwdotorg.org ([2001:470:1f0f:bd7::2]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1TtQHt-0001RN-V9 for linux-arm-kernel@lists.infradead.org; Thu, 10 Jan 2013 22:07:50 +0000 Received: from severn.wwwdotorg.org (unknown [192.168.65.5]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by avon.wwwdotorg.org (Postfix) with ESMTPS id 3F7C26321; Thu, 10 Jan 2013 15:09:07 -0700 (MST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by severn.wwwdotorg.org (Postfix) with ESMTPSA id C8AE5E40EB; Thu, 10 Jan 2013 15:07:44 -0700 (MST) Message-ID: <50EF3BAF.4010200@wwwdotorg.org> Date: Thu, 10 Jan 2013 15:07:43 -0700 From: Stephen Warren User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: Linus Walleij Subject: Re: [PATCH v2] drivers/pinctrl: grab default handles from device core References: <1355343907-11535-1-git-send-email-linus.walleij@stericsson.com> <50EF27B4.60608@wwwdotorg.org> In-Reply-To: <50EF27B4.60608@wwwdotorg.org> X-Enigmail-Version: 1.4.6 X-Virus-Scanned: clamav-milter 0.96.5 at avon.wwwdotorg.org X-Virus-Status: Clean X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130110_170750_219065_5B0B71C3 X-CRM114-Status: GOOD ( 24.32 ) X-Spam-Score: -2.6 (--) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-2.6 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record -0.7 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Thomas Petazzoni , Kevin Hilman , Ulf Hansson , Dmitry Torokhov , Russell King , Benoit Cousson , Greg Kroah-Hartman , Linus Walleij , Mark Brown , linux-kernel@vger.kernel.org, Felipe Balbi , Rickard Andersson , "Rafael J. Wysocki" , linux-next@vger.kernel.org, Mitch Bradley , Jean-Christophe PLAGNIOL-VILLARD , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On 01/10/2013 01:42 PM, Stephen Warren wrote: > On 12/12/2012 01:25 PM, Linus Walleij wrote: >> From: Linus Walleij >> >> This makes the device core auto-grab the pinctrl handle and set >> the "default" (PINCTRL_STATE_DEFAULT) state for every device >> that is present in the device model right before probe. This will >> account for the lion's share of embedded silicon devcies. > > There are quite a few problems with this patch, and they end up > completely breaking at least Tegra in next-20130110. > >> diff --git a/drivers/base/pinctrl.c b/drivers/base/pinctrl.c > >> +int pinctrl_bind_pins(struct device *dev) >> +{ >> + struct dev_pin_info *dpi; >> + int ret; >> + >> + /* Allocate a pin state container on-the-fly */ >> + if (!dev->pins) { >> + dpi = devm_kzalloc(dev, sizeof(*dpi), GFP_KERNEL); >> + if (!dpi) >> + return -ENOMEM; >> + } else >> + dpi = dev->pins; >> + >> + /* >> + * Check if we already have a pinctrl handle, as we may arrive here >> + * after a deferral in the state selection below >> + */ >> + if (!dpi->p) { >> + dpi->p = devm_pinctrl_get(dev); > > That won't succeed for a pinctrl device that has a default state in > order to implement hogs. This will then cause the pin controller device > to always defer probe and never activate. This will leave HW > unconfigured and/or prevent other devices from successfully calling > pinctrl_get(). I see that an attempt was made to solve this problem, in the patch immediately preceding this one (at least, as applied in the pinctrl tree). However, that patch only addresses the case where the pin controller is being looked up in the map, and not the case when converting device tree to the map in the first place. The patch below solves this: diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c index fe2d1af..fd40a11 100644 --- a/drivers/pinctrl/devicetree.c +++ b/drivers/pinctrl/devicetree.c @@ -141,6 +141,11 @@ static int dt_to_map_one_config(struct pinctrl *p, const char *statename, pctldev = find_pinctrl_by_of_node(np_pctldev); if (pctldev) break; + /* Do not defer probing of hogs (circular loop) */ + if (np_pctldev == p->dev->of_node) { + of_node_put(np_pctldev); + return -ENODEV; + } } of_node_put(np_pctldev);