diff mbox

[v5,4/4] clk: basic clock hardware types

Message ID 20120307212025.GC26882@pengutronix.de
State New
Headers show

Commit Message

Sascha Hauer March 7, 2012, 9:20 p.m. UTC 
On Sat, Mar 03, 2012 at 12:29:01AM -0800, Mike Turquette wrote:
> +struct clk *clk_register_divider(struct device *dev, const char *name,
> +		const char *parent_name, unsigned long flags,
> +		void __iomem *reg, u8 shift, u8 width,
> +		u8 clk_divider_flags, spinlock_t *lock)
> +{
> +	struct clk_divider *div;
> +	char **parent_names = NULL;
> +	u8 len;
> +
> +	div = kmalloc(sizeof(struct clk_divider), GFP_KERNEL);
> +
> +	if (!div) {
> +		pr_err("%s: could not allocate divider clk\n", __func__);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	/* struct clk_divider assignments */
> +	div->reg = reg;
> +	div->shift = shift;
> +	div->width = width;
> +	div->flags = clk_divider_flags;
> +	div->lock = lock;
> +
> +	if (parent_name) {
> +		parent_names = kmalloc(sizeof(char *), GFP_KERNEL);
> +
> +		if (! parent_names)
> +			goto out;
> +
> +		len = sizeof(char) * strlen(parent_name);
> +
> +		parent_names[0] = kmalloc(len, GFP_KERNEL);
> +
> +		if (!parent_names[0])
> +			goto out;
> +
> +		strncpy(parent_names[0], parent_name, len);
> +	}
> +
> +out:
> +	return clk_register(dev, name,
> +			&clk_divider_ops, &div->hw,
> +			parent_names,
> +			(parent_name ? 1 : 0),
> +			flags);
> +}

clk_register_divider and also clk_register_gate have some problems.
First you allocate memory with exactly the string length without
the terminating 0. Then the functions leak memory when clk_register
fails. Could you fold in the following patch to fix this?

Sascha

8<--------------------------------------------------

fix divider/gate registration

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/clk/clk-divider.c    |   34 +++++++++++++++-------------------
 drivers/clk/clk-gate.c       |   33 ++++++++++++++-------------------
 include/linux/clk-provider.h |    2 ++
 3 files changed, 31 insertions(+), 38 deletions(-)

Comments

Mike Turquette March 9, 2012, 10:50 p.m. UTC | #1 
On Wed, Mar 7, 2012 at 1:20 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Sat, Mar 03, 2012 at 12:29:01AM -0800, Mike Turquette wrote:
>> +struct clk *clk_register_divider(struct device *dev, const char *name,
>> +             const char *parent_name, unsigned long flags,
>> +             void __iomem *reg, u8 shift, u8 width,
>> +             u8 clk_divider_flags, spinlock_t *lock)
>> +{
>> +     struct clk_divider *div;
>> +     char **parent_names = NULL;
>> +     u8 len;
>> +
>> +     div = kmalloc(sizeof(struct clk_divider), GFP_KERNEL);
>> +
>> +     if (!div) {
>> +             pr_err("%s: could not allocate divider clk\n", __func__);
>> +             return ERR_PTR(-ENOMEM);
>> +     }
>> +
>> +     /* struct clk_divider assignments */
>> +     div->reg = reg;
>> +     div->shift = shift;
>> +     div->width = width;
>> +     div->flags = clk_divider_flags;
>> +     div->lock = lock;
>> +
>> +     if (parent_name) {
>> +             parent_names = kmalloc(sizeof(char *), GFP_KERNEL);
>> +
>> +             if (! parent_names)
>> +                     goto out;
>> +
>> +             len = sizeof(char) * strlen(parent_name);
>> +
>> +             parent_names[0] = kmalloc(len, GFP_KERNEL);
>> +
>> +             if (!parent_names[0])
>> +                     goto out;
>> +
>> +             strncpy(parent_names[0], parent_name, len);
>> +     }
>> +
>> +out:
>> +     return clk_register(dev, name,
>> +                     &clk_divider_ops, &div->hw,
>> +                     parent_names,
>> +                     (parent_name ? 1 : 0),
>> +                     flags);
>> +}
>
> clk_register_divider and also clk_register_gate have some problems.
> First you allocate memory with exactly the string length without
> the terminating 0. Then the functions leak memory when clk_register
> fails. Could you fold in the following patch to fix this?

Thanks for the patch Sascha.  This has been pulled into v6 and tested.

Regards,
Mike

>
> Sascha
>
> 8<--------------------------------------------------
>
> fix divider/gate registration
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/clk/clk-divider.c    |   34 +++++++++++++++-------------------
>  drivers/clk/clk-gate.c       |   33 ++++++++++++++-------------------
>  include/linux/clk-provider.h |    2 ++
>  3 files changed, 31 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index 8f02930..99b6b55 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -156,14 +156,13 @@ struct clk *clk_register_divider(struct device *dev, const char *name,
>                u8 clk_divider_flags, spinlock_t *lock)
>  {
>        struct clk_divider *div;
> -       char **parent_names = NULL;
> -       u8 len;
> +       struct clk *clk;
>
> -       div = kmalloc(sizeof(struct clk_divider), GFP_KERNEL);
> +       div = kzalloc(sizeof(struct clk_divider), GFP_KERNEL);
>
>        if (!div) {
>                pr_err("%s: could not allocate divider clk\n", __func__);
> -               return ERR_PTR(-ENOMEM);
> +               return NULL;
>        }
>
>        /* struct clk_divider assignments */
> @@ -174,25 +173,22 @@ struct clk *clk_register_divider(struct device *dev, const char *name,
>        div->lock = lock;
>
>        if (parent_name) {
> -               parent_names = kmalloc(sizeof(char *), GFP_KERNEL);
> -
> -               if (! parent_names)
> -                       goto out;
> -
> -               len = sizeof(char) * strlen(parent_name);
> -
> -               parent_names[0] = kmalloc(len, GFP_KERNEL);
> -
> -               if (!parent_names[0])
> +               div->parent[0] = kstrdup(parent_name, GFP_KERNEL);
> +               if (!div->parent[0])
>                        goto out;
> -
> -               strncpy(parent_names[0], parent_name, len);
>        }
>
> -out:
> -       return clk_register(dev, name,
> +       clk = clk_register(dev, name,
>                        &clk_divider_ops, &div->hw,
> -                       parent_names,
> +                       div->parent,
>                        (parent_name ? 1 : 0),
>                        flags);
> +       if (clk)
> +               return clk;
> +
> +out:
> +       kfree(div->parent[0]);
> +       kfree(div);
> +
> +       return NULL;
>  }
> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
> index e831f7b..92c0489 100644
> --- a/drivers/clk/clk-gate.c
> +++ b/drivers/clk/clk-gate.c
> @@ -80,14 +80,13 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
>                u8 clk_gate_flags, spinlock_t *lock)
>  {
>        struct clk_gate *gate;
> -       char **parent_names = NULL;
> -       u8 len;
> +       struct clk *clk;
>
> -       gate = kmalloc(sizeof(struct clk_gate), GFP_KERNEL);
> +       gate = kzalloc(sizeof(struct clk_gate), GFP_KERNEL);
>
>        if (!gate) {
>                pr_err("%s: could not allocate gated clk\n", __func__);
> -               return ERR_PTR(-ENOMEM);
> +               return NULL;
>        }
>
>        /* struct clk_gate assignments */
> @@ -97,25 +96,21 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
>        gate->lock = lock;
>
>        if (parent_name) {
> -               parent_names = kmalloc(sizeof(char *), GFP_KERNEL);
> -
> -               if (! parent_names)
> -                       goto out;
> -
> -               len = sizeof(char) * strlen(parent_name);
> -
> -               parent_names[0] = kmalloc(len, GFP_KERNEL);
> -
> -               if (!parent_names[0])
> +               gate->parent[0] = kstrdup(parent_name, GFP_KERNEL);
> +               if (!gate->parent[0])
>                        goto out;
> -
> -               strncpy(parent_names[0], parent_name, len);
>        }
>
> -out:
> -       return clk_register(dev, name,
> +       clk = clk_register(dev, name,
>                        &clk_gate_ops, &gate->hw,
> -                       parent_names,
> +                       gate->parent,
>                        (parent_name ? 1 : 0),
>                        flags);
> +       if (clk)
> +               return clk;
> +out:
> +       kfree(gate->parent[0]);
> +       kfree(gate);
> +
> +       return NULL;
>  }
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 9b7dd5e..ed1f918 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -174,6 +174,7 @@ struct clk_gate {
>        u8              bit_idx;
>        u8              flags;
>        spinlock_t      *lock;
> +       char            *parent[1];
>  };
>
>  #define CLK_GATE_SET_TO_DISABLE                BIT(0)
> @@ -210,6 +211,7 @@ struct clk_divider {
>        u8              width;
>        u8              flags;
>        spinlock_t      *lock;
> +       char            *parent[1];
>  };
>
>  #define CLK_DIVIDER_ONE_BASED          BIT(0)
> --
> 1.7.9.1
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
diff mbox

Patch

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 8f02930..99b6b55 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -156,14 +156,13 @@  struct clk *clk_register_divider(struct device *dev, const char *name,
 		u8 clk_divider_flags, spinlock_t *lock)
 {
 	struct clk_divider *div;
-	char **parent_names = NULL;
-	u8 len;
+	struct clk *clk;
 
-	div = kmalloc(sizeof(struct clk_divider), GFP_KERNEL);
+	div = kzalloc(sizeof(struct clk_divider), GFP_KERNEL);
 
 	if (!div) {
 		pr_err("%s: could not allocate divider clk\n", __func__);
-		return ERR_PTR(-ENOMEM);
+		return NULL;
 	}
 
 	/* struct clk_divider assignments */
@@ -174,25 +173,22 @@  struct clk *clk_register_divider(struct device *dev, const char *name,
 	div->lock = lock;
 
 	if (parent_name) {
-		parent_names = kmalloc(sizeof(char *), GFP_KERNEL);
-
-		if (! parent_names)
-			goto out;
-
-		len = sizeof(char) * strlen(parent_name);
-
-		parent_names[0] = kmalloc(len, GFP_KERNEL);
-
-		if (!parent_names[0])
+		div->parent[0] = kstrdup(parent_name, GFP_KERNEL);
+		if (!div->parent[0])
 			goto out;
-
-		strncpy(parent_names[0], parent_name, len);
 	}
 
-out:
-	return clk_register(dev, name,
+	clk = clk_register(dev, name,
 			&clk_divider_ops, &div->hw,
-			parent_names,
+			div->parent,
 			(parent_name ? 1 : 0),
 			flags);
+	if (clk)
+		return clk;
+
+out:
+	kfree(div->parent[0]);
+	kfree(div);
+
+	return NULL;
 }
diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index e831f7b..92c0489 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -80,14 +80,13 @@  struct clk *clk_register_gate(struct device *dev, const char *name,
 		u8 clk_gate_flags, spinlock_t *lock)
 {
 	struct clk_gate *gate;
-	char **parent_names = NULL;
-	u8 len;
+	struct clk *clk;
 
-	gate = kmalloc(sizeof(struct clk_gate), GFP_KERNEL);
+	gate = kzalloc(sizeof(struct clk_gate), GFP_KERNEL);
 
 	if (!gate) {
 		pr_err("%s: could not allocate gated clk\n", __func__);
-		return ERR_PTR(-ENOMEM);
+		return NULL;
 	}
 
 	/* struct clk_gate assignments */
@@ -97,25 +96,21 @@  struct clk *clk_register_gate(struct device *dev, const char *name,
 	gate->lock = lock;
 
 	if (parent_name) {
-		parent_names = kmalloc(sizeof(char *), GFP_KERNEL);
-
-		if (! parent_names)
-			goto out;
-
-		len = sizeof(char) * strlen(parent_name);
-
-		parent_names[0] = kmalloc(len, GFP_KERNEL);
-
-		if (!parent_names[0])
+		gate->parent[0] = kstrdup(parent_name, GFP_KERNEL);
+		if (!gate->parent[0])
 			goto out;
-
-		strncpy(parent_names[0], parent_name, len);
 	}
 
-out:
-	return clk_register(dev, name,
+	clk = clk_register(dev, name,
 			&clk_gate_ops, &gate->hw,
-			parent_names,
+			gate->parent,
 			(parent_name ? 1 : 0),
 			flags);
+	if (clk)
+		return clk;
+out:
+	kfree(gate->parent[0]);
+	kfree(gate);
+
+	return NULL;
 }
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 9b7dd5e..ed1f918 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -174,6 +174,7 @@  struct clk_gate {
 	u8		bit_idx;
 	u8		flags;
 	spinlock_t	*lock;
+	char		*parent[1];
 };
 
 #define CLK_GATE_SET_TO_DISABLE		BIT(0)
@@ -210,6 +211,7 @@  struct clk_divider {
 	u8		width;
 	u8		flags;
 	spinlock_t	*lock;
+	char		*parent[1];
 };
 
 #define CLK_DIVIDER_ONE_BASED		BIT(0)