All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] clk: clk-gpio-gate: Fix active low
@ 2015-03-18 13:53 Martin Fuzzey
  2015-03-18 14:52 ` Jyri Sarha
  0 siblings, 1 reply; 3+ messages in thread
From: Martin Fuzzey @ 2015-03-18 13:53 UTC (permalink / raw)
  To: Mike Turquette, Jyri Sarha; +Cc: linux-kernel

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 <mfuzzey@parkeon.com>
---
 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);


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] clk: clk-gpio-gate: Fix active low
  2015-03-18 13:53 [PATCH] clk: clk-gpio-gate: Fix active low Martin Fuzzey
@ 2015-03-18 14:52 ` Jyri Sarha
  2015-04-11  0:51   ` Michael Turquette
  0 siblings, 1 reply; 3+ messages in thread
From: Jyri Sarha @ 2015-03-18 14:52 UTC (permalink / raw)
  To: Martin Fuzzey, Mike Turquette; +Cc: linux-kernel

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 <mfuzzey@parkeon.com>

Acked-by: Jyri Sarha <jsarha@ti.com>

> ---
>   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);
>


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] clk: clk-gpio-gate: Fix active low
  2015-03-18 14:52 ` Jyri Sarha
@ 2015-04-11  0:51   ` Michael Turquette
  0 siblings, 0 replies; 3+ messages in thread
From: Michael Turquette @ 2015-04-11  0:51 UTC (permalink / raw)
  To: Jyri Sarha, Martin Fuzzey; +Cc: linux-kernel

Quoting Jyri Sarha (2015-03-18 07:52:14)
> 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 <mfuzzey@parkeon.com>
> 
> Acked-by: Jyri Sarha <jsarha@ti.com>

Applied to clk-next.

Thanks,
Mike

> 
> > ---
> >   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);
> >
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-04-11  0:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-18 13:53 [PATCH] clk: clk-gpio-gate: Fix active low Martin Fuzzey
2015-03-18 14:52 ` Jyri Sarha
2015-04-11  0:51   ` Michael Turquette

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.