Skip to contentSkip to conversation

clk: clk-gpio-gate: Fix active low

PATCH Applied Martin Fuzzey 3 participants 3 messages Mar 18, 2015 – Apr 11, 2015 Common clk

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 clk: clk-gpio-gate: Fix active low

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.

---
 drivers/clk/clk-gpio-gate.c  |   31 +++++++++++++++++--------------
 include/linux/clk-provider.h |    2 +-
 2 files changed, 18 insertions(+), 15 deletions(-)
drivers/clk/clk-gpio-gate.c +17 -14
index 08e4322..a71cabe 100644
@@ -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;
include/linux/clk-provider.h +1 -1
index d936409..6f514d5 100644
@@ -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);
Acked-by on v1 1/1
#1 lore raw
Quoted 10 lines · Martin Fuzzey

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.

Quoted 117 lines
---
  drivers/clk/clk-gpio-gate.c  |   31 +++++++++++++++++--------------
  include/linux/clk-provider.h |    2 +-
  2 files changed, 18 insertions(+), 15 deletions(-)
drivers/clk/clk-gpio-gate.c +17 -14
index 08e4322..a71cabe 100644
@@ -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;
include/linux/clk-provider.h +1 -1
index d936409..6f514d5 100644
@@ -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);
#2 lore raw

Quoting Jyri Sarha (2015-03-18 07:52:14)

Quoted 7 lines · Martin Fuzzey
Quoted 6 lines

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.

Quoted 6 lines
Quoted 4 lines

Note that this patch changes the internal kernel API for clk_register_gpio_gate() but there are currently no other users.

Applied to clk-next.

Thanks,
Mike

Quoted 119 lines
Quoted 118 lines
---
  drivers/clk/clk-gpio-gate.c  |   31 +++++++++++++++++--------------
  include/linux/clk-provider.h |    2 +-
  2 files changed, 18 insertions(+), 15 deletions(-)
drivers/clk/clk-gpio-gate.c +17 -14
index 08e4322..a71cabe 100644
@@ -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;
include/linux/clk-provider.h +1 -1
index d936409..6f514d5 100644
@@ -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);