From patchwork Thu Dec 24 18:12:53 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Kocialkowski X-Patchwork-Id: 7919781 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 9A8F99F1AF for ; Thu, 24 Dec 2015 18:15:24 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B801420461 for ; Thu, 24 Dec 2015 18:15:23 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C8E2520460 for ; Thu, 24 Dec 2015 18:15:22 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1aCAOJ-0007Qq-C5; Thu, 24 Dec 2015 18:13:31 +0000 Received: from gagarine.paulk.fr ([109.190.93.129]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1aCAOF-0007Ot-Jr for linux-arm-kernel@lists.infradead.org; Thu, 24 Dec 2015 18:13:28 +0000 Received: by gagarine.paulk.fr (Postfix, from userid 65534) id 0BD121FEF4; Thu, 24 Dec 2015 19:13:04 +0100 (CET) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RCVD_IN_SORBS_WEB,RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Received: from [192.168.0.129] (armstrong.paulk.fr [82.233.88.171]) by gagarine.paulk.fr (Postfix) with ESMTPSA id 5BC411FEA2; Thu, 24 Dec 2015 19:12:54 +0100 (CET) Message-ID: <1450980773.11913.3.camel@collins> Subject: Re: [PATCH 4/6] regulator: lp872x: Add enable GPIO pin support From: Paul Kocialkowski To: Mark Brown Date: Thu, 24 Dec 2015 19:12:53 +0100 In-Reply-To: <20151223115632.GS16023@sirena.org.uk> References: <1450868319-20513-1-git-send-email-contact@paulk.fr> <1450868319-20513-5-git-send-email-contact@paulk.fr> <20151223115632.GS16023@sirena.org.uk> X-Mailer: Evolution 3.10.4-0ubuntu2+7.0trisquel1 Mime-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20151224_101327_915976_ED54224D X-CRM114-Status: GOOD ( 22.41 ) X-Spam-Score: -1.1 (-) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Milo Kim , Russell King , Pawel Moll , Ian Campbell , Tony Lindgren , linux-kernel@vger.kernel.org, Liam Girdwood , devicetree@vger.kernel.org, Rob Herring , =?ISO-8859-1?Q?Beno=EEt?= Cousson , Kumar Gala , linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Le mercredi 23 décembre 2015 à 11:56 +0000, Mark Brown a écrit : > On Wed, Dec 23, 2015 at 11:58:37AM +0100, Paul Kocialkowski wrote: > > > + gpio = lp->pdata->enable_gpio; > > + if (!gpio_is_valid(gpio)) > > + return 0; > > + > > + /* Always set enable GPIO high. */ > > + ret = devm_gpio_request_one(lp->dev, gpio, GPIOF_OUT_INIT_HIGH, "LP872X EN"); > > + if (ret) { > > + dev_err(lp->dev, "gpio request err: %d\n", ret); > > + return ret; > > + } > > This isn't really adding support for the enable GPIO as the changelog > suggests, it's requesting but not managing the GPIO. Since there is > core support for manging enable GPIOs this seems especially silly, > please tell the core about the GPIO and then it will work at runtime > too. It looks like the core bindings for GPIO can only be used instead of the rdev->desc->ops->enable callback, not jointly, which doesn't fit my use case, where both the GPIO and register write have to be used to enable regulators. I think it would be worth making it possible to use both in core, since that situation is probably shared with other regulators. I suggest the following diff (that would be split into a separate patch in v2 of this series): if (pin->enable_count > 1) { @@ -2063,6 +2062,14 @@ static int _regulator_do_enable(struct regulator_dev *rdev) } } + if (rdev->desc->ops->enable) { + ret = rdev->desc->ops->enable(rdev); + if (ret < 0) + return ret; + } else if (!rdev->ena_pin) { + return -EINVAL; + } + if (rdev->ena_pin) { if (!rdev->ena_gpio_state) { ret = regulator_ena_gpio_ctrl(rdev, true); @@ -2070,10 +2077,6 @@ static int _regulator_do_enable(struct regulator_dev *rdev) return ret; rdev->ena_gpio_state = 1; } - } else if (rdev->desc->ops->enable) { - ret = rdev->desc->ops->enable(rdev); - if (ret < 0) - return ret; } else { return -EINVAL; } diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index e92f344..dd0674f 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -1963,7 +1963,6 @@ static int regulator_ena_gpio_ctrl(struct regulator_dev *rdev, bool enable) if (pin->enable_count == 0) gpiod_set_value_cansleep(pin->gpiod, !pin->ena_gpio_invert); - pin->enable_count++; } else {