From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755326AbbCROwW (ORCPT ); Wed, 18 Mar 2015 10:52:22 -0400 Received: from devils.ext.ti.com ([198.47.26.153]:52768 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754862AbbCROwT (ORCPT ); Wed, 18 Mar 2015 10:52:19 -0400 Message-ID: <5509911E.7070400@ti.com> Date: Wed, 18 Mar 2015 16:52:14 +0200 From: Jyri Sarha User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Martin Fuzzey , Mike Turquette CC: Subject: Re: [PATCH] clk: clk-gpio-gate: Fix active low References: <20150318135317.4817.92802.stgit@localhost> In-Reply-To: <20150318135317.4817.92802.stgit@localhost> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/18/15 15:53, Martin Fuzzey wrote: > The active low flag in the DT cell is currently ignored. > > This occurs because of_get_named_gpio_flags() does not apply the flags > to the underlying struct gpio_desc so the test in clk_register_gpio_gate() > was bogus. > > Note that this patch changes the internal kernel API for > clk_register_gpio_gate() but there are currently no other users. > > Signed-off-by: Martin Fuzzey Acked-by: Jyri Sarha > --- > drivers/clk/clk-gpio-gate.c | 31 +++++++++++++++++-------------- > include/linux/clk-provider.h | 2 +- > 2 files changed, 18 insertions(+), 15 deletions(-) > > diff --git a/drivers/clk/clk-gpio-gate.c b/drivers/clk/clk-gpio-gate.c > index 08e4322..a71cabe 100644 > --- a/drivers/clk/clk-gpio-gate.c > +++ b/drivers/clk/clk-gpio-gate.c > @@ -65,10 +65,12 @@ EXPORT_SYMBOL_GPL(clk_gpio_gate_ops); > * @dev: device that is registering this clock > * @name: name of this clock > * @parent_name: name of this clock's parent > - * @gpiod: gpio descriptor to gate this clock > + * @gpio: gpio number to gate this clock > + * @active_low: true if gpio should be set to 0 to enable clock > + * @flags: clock flags > */ > struct clk *clk_register_gpio_gate(struct device *dev, const char *name, > - const char *parent_name, struct gpio_desc *gpiod, > + const char *parent_name, unsigned gpio, bool active_low, > unsigned long flags) > { > struct clk_gpio *clk_gpio = NULL; > @@ -77,20 +79,19 @@ struct clk *clk_register_gpio_gate(struct device *dev, const char *name, > unsigned long gpio_flags; > int err; > > - if (gpiod_is_active_low(gpiod)) > - gpio_flags = GPIOF_OUT_INIT_HIGH; > + if (active_low) > + gpio_flags = GPIOF_ACTIVE_LOW | GPIOF_OUT_INIT_HIGH; > else > gpio_flags = GPIOF_OUT_INIT_LOW; > > if (dev) > - err = devm_gpio_request_one(dev, desc_to_gpio(gpiod), > - gpio_flags, name); > + err = devm_gpio_request_one(dev, gpio, gpio_flags, name); > else > - err = gpio_request_one(desc_to_gpio(gpiod), gpio_flags, name); > + err = gpio_request_one(gpio, gpio_flags, name); > > if (err) { > pr_err("%s: %s: Error requesting clock control gpio %u\n", > - __func__, name, desc_to_gpio(gpiod)); > + __func__, name, gpio); > return ERR_PTR(err); > } > > @@ -111,7 +112,7 @@ struct clk *clk_register_gpio_gate(struct device *dev, const char *name, > init.parent_names = (parent_name ? &parent_name : NULL); > init.num_parents = (parent_name ? 1 : 0); > > - clk_gpio->gpiod = gpiod; > + clk_gpio->gpiod = gpio_to_desc(gpio); > clk_gpio->hw.init = &init; > > clk = clk_register(dev, &clk_gpio->hw); > @@ -123,7 +124,8 @@ struct clk *clk_register_gpio_gate(struct device *dev, const char *name, > kfree(clk_gpio); > > clk_register_gpio_gate_err: > - gpiod_put(gpiod); > + if (!dev) > + gpio_free(gpio); > > return clk; > } > @@ -149,8 +151,8 @@ static struct clk *of_clk_gpio_gate_delayed_register_get( > struct clk *clk; > const char *clk_name = data->node->name; > const char *parent_name; > - struct gpio_desc *gpiod; > int gpio; > + enum of_gpio_flags of_flags; > > mutex_lock(&data->lock); > > @@ -159,7 +161,8 @@ static struct clk *of_clk_gpio_gate_delayed_register_get( > return data->clk; > } > > - gpio = of_get_named_gpio_flags(data->node, "enable-gpios", 0, NULL); > + gpio = of_get_named_gpio_flags(data->node, "enable-gpios", 0, > + &of_flags); > if (gpio < 0) { > mutex_unlock(&data->lock); > if (gpio != -EPROBE_DEFER) > @@ -167,11 +170,11 @@ static struct clk *of_clk_gpio_gate_delayed_register_get( > __func__, clk_name); > return ERR_PTR(gpio); > } > - gpiod = gpio_to_desc(gpio); > > parent_name = of_clk_get_parent_name(data->node, 0); > > - clk = clk_register_gpio_gate(NULL, clk_name, parent_name, gpiod, 0); > + clk = clk_register_gpio_gate(NULL, clk_name, parent_name, gpio, > + of_flags & OF_GPIO_ACTIVE_LOW, 0); > if (IS_ERR(clk)) { > mutex_unlock(&data->lock); > return clk; > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > index d936409..6f514d5 100644 > --- a/include/linux/clk-provider.h > +++ b/include/linux/clk-provider.h > @@ -515,7 +515,7 @@ struct clk_gpio { > > extern const struct clk_ops clk_gpio_gate_ops; > struct clk *clk_register_gpio_gate(struct device *dev, const char *name, > - const char *parent_name, struct gpio_desc *gpio, > + const char *parent_name, unsigned gpio, bool active_low, > unsigned long flags); > > void of_gpio_clk_gate_setup(struct device_node *node); >