From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752556AbbJTHPB (ORCPT ); Tue, 20 Oct 2015 03:15:01 -0400 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:50681 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752497AbbJTHPA (ORCPT ); Tue, 20 Oct 2015 03:15:00 -0400 Date: Tue, 20 Oct 2015 09:14:55 +0200 From: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= To: Alexandre Courbot Cc: Russell King - ARM Linux , Linus Walleij , linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org Subject: Re: gpiod API considerations [Was: [GIT PULL] On-demand device probing] Message-ID: <20151020071454.GE4931@pengutronix.de> References: <20151018195330.GB14956@sirena.org.uk> <1445247881.53393.146.camel@infradead.org> <1445258870.53393.173.camel@infradead.org> <20151019145048.GI14956@sirena.org.uk> <1445268580.53393.183.camel@infradead.org> <20151019154324.GN32532@n2100.arm.linux.org.uk> <20151019182744.GD4931@pengutronix.de> <20151019183905.GS32532@n2100.arm.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, [trimming list of recipients considerably because of changed topic] On Tue, Oct 20, 2015 at 08:47:21AM +0900, Alexandre Courbot wrote: > On Tue, Oct 20, 2015 at 3:39 AM, Russell King - ARM Linux > wrote: > > On Mon, Oct 19, 2015 at 08:27:44PM +0200, Uwe Kleine-König wrote: > >> On Mon, Oct 19, 2015 at 04:43:24PM +0100, Russell King - ARM Linux wrote: > >> > It's a bit ironic that you've chosen GPIO as an example there. The > >> > "new" GPIO API (the gpiod_* stuff) only has a fwnode way to get the > >> > gpio descriptor. There's no of_* method. > >> > >> Without following all that fwnode discussion: > >> gpiod_get et al. should work for you here, doesn't it? It just takes a > >> struct device * and I'm happy with it. > > > > What if you don't have a struct device? I had that problem recently > > when modifying the mvebu PCIe code. The 'struct device' node doesn't > > contain the GPIOs, it's the PCIe controller. Individual ports on the > > controller are described in DT as sub-nodes, and the sub-nodes can > > have a GPIO for card reset purposes. These sub-nodes don't have a > > struct device. > > > > Right now, I'm having to do this to work around this issue: > > > > reset_gpio = of_get_named_gpio_flags(child, "reset-gpios", 0, &flags); > > if (reset_gpio == -EPROBE_DEFER) { > > ret = reset_gpio; > > goto err; > > } > > > > if (gpio_is_valid(reset_gpio)) { > > unsigned long gpio_flags; > > > > port->reset_name = devm_kasprintf(dev, GFP_KERNEL, "%s-reset", > > port->name); > > if (!port->reset_name) { > > ret = -ENOMEM; > > goto err; > > } > > > > if (flags & OF_GPIO_ACTIVE_LOW) { > > dev_info(dev, "%s: reset gpio is active low\n", > > of_node_full_name(child)); > > gpio_flags = GPIOF_ACTIVE_LOW | > > GPIOF_OUT_INIT_LOW; > > } else { > > gpio_flags = GPIOF_OUT_INIT_HIGH; > > } > > > > ret = devm_gpio_request_one(dev, reset_gpio, gpio_flags, > > port->reset_name); > > if (ret) { > > if (ret == -EPROBE_DEFER) > > goto err; > > goto skip; > > } > > > > port->reset_gpio = gpio_to_desc(reset_gpio); > > } > > > > Not nice, is it? Not nice to have that in lots of drivers either. > > > > However, switching to use any of_* or fwnode_* thing also carries with > > it another problem: you can't control the name appearing in the > > allocation, so you end up with a bunch of GPIOs requested with a "reset" > > name - meaning you lose any identification of which port the GPIO was > > bound to. > > There are a few holes in the gpiod API. I see two solutions here: > > 1) extend devm_get_gpiod_from_child() to take an optional name argument > 2) add a function to explicitly change a GPIO's name > > 2) seems to be the most generic solution, would that do the trick? I would prefer 1) without "optional". A third alternative is to add at least dev_name(dev) and maybe index to the name where applicable. Also note that gpiod_request is called with label=NULL (in fwnode_get_named_gpiod which is used in devm_get_gpiod_from_child), so /sys/kernel/debug/gpio doesn't even contain "reset". I only see question marks (using v4.3-rc5). Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |