a) as we allocate the pinctrl_map structure at imx_dt_node_to_map at line 167, anyway
if its an element, or a num_elements * (sizeof(type)) elements allocated to one single
pointer must be freed only once.
CASE. A)
as new_map is not moved and allocated like,
for (i = 0; i < MAX_ELEMS; i++) {
new_map[i] = kmalloc(numelems * size, GFP_KERNEL);
}
its freed as
for (i = 0; i < MAX_ELEMS; i++) {
kfree(new_map[i]);
}
CASE. B)
and its allocated like
new_map = kmalloc(numelems * size, GFP_KERNEL);
it just needs kfree not as case A's.
b) use KERN_DEBUG facility for the IMX_PMX_DUMP macro.
Signed-off-by: Devendra Naga <[email protected]>
Acked-by: Dong Aisheng <[email protected]>
---
drivers/pinctrl/pinctrl-imx.c | 28 ++++++++++++----------------
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-imx.c b/drivers/pinctrl/pinctrl-imx.c
index f6e7c67..09f3a30 100644
--- a/drivers/pinctrl/pinctrl-imx.c
+++ b/drivers/pinctrl/pinctrl-imx.c
@@ -27,16 +27,16 @@
#include "core.h"
#include "pinctrl-imx.h"
-#define IMX_PMX_DUMP(info, p, m, c, n) \
-{ \
- int i, j; \
- printk("Format: Pin Mux Config\n"); \
- for (i = 0; i < n; i++) { \
- j = p[i]; \
- printk("%s %d 0x%lx\n", \
- info->pins[j].name, \
- m[i], c[i]); \
- } \
+#define IMX_PMX_DUMP(info, p, m, c, n) \
+{ \
+ int i, j; \
+ printk(KERN_DEBUG "Format: Pin Mux Config\n"); \
+ for (i = 0; i < n; i++) { \
+ j = p[i]; \
+ printk(KERN_DEBUG "%s %d 0x%lx\n", \
+ info->pins[j].name, \
+ m[i], c[i]); \
+ } \
}
/* The bits in CONFIG cell defined in binding doc*/
@@ -201,10 +201,7 @@ static int imx_dt_node_to_map(struct pinctrl_dev *pctldev,
static void imx_dt_free_map(struct pinctrl_dev *pctldev,
struct pinctrl_map *map, unsigned num_maps)
{
- int i;
-
- for (i = 0; i < num_maps; i++)
- kfree(map);
+ kfree(map);
}
static struct pinctrl_ops imx_pctrl_ops = {
@@ -475,9 +472,8 @@ static int __devinit imx_pinctrl_parse_groups(struct device_node *np,
grp->configs[j] = config & ~IMX_PAD_SION;
}
-#ifdef DEBUG
IMX_PMX_DUMP(info, grp->pins, grp->mux_mode, grp->configs, grp->npins);
-#endif
+
return 0;
}
--
1.7.9.5
Hi Dong, Linus,
i sent a second patch previously which applies actually on top this
patch , so not sending it.
You want me to send it again?
Thanks,
Devendra.
On Sat, Jun 9, 2012 at 12:52 AM, Devendra Naga <[email protected]> wrote:
> a) as we allocate the pinctrl_map structure at imx_dt_node_to_map at ?line 167, anyway
> ? if its an element, or a num_elements * (sizeof(type)) elements allocated to one single
> ? pointer must be freed only once.
>
> CASE. A)
>
> as new_map is not moved and allocated like,
>
> for (i = 0; i < MAX_ELEMS; i++) {
> ? ?new_map[i] = kmalloc(numelems * size, GFP_KERNEL);
> }
>
> its freed as
>
> for (i = 0; i < MAX_ELEMS; i++) {
> ? ?kfree(new_map[i]);
> }
>
> CASE. B)
> and its allocated like
> ? ? ? ?new_map = kmalloc(numelems * size, GFP_KERNEL);
>
> it just needs kfree not as case A's.
>
> b) use KERN_DEBUG facility for the IMX_PMX_DUMP macro.
>
> Signed-off-by: Devendra Naga <[email protected]>
> Acked-by: Dong Aisheng <[email protected]>
> ---
> ?drivers/pinctrl/pinctrl-imx.c | ? 28 ++++++++++++----------------
> ?1 file changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-imx.c b/drivers/pinctrl/pinctrl-imx.c
> index f6e7c67..09f3a30 100644
> --- a/drivers/pinctrl/pinctrl-imx.c
> +++ b/drivers/pinctrl/pinctrl-imx.c
> @@ -27,16 +27,16 @@
> ?#include "core.h"
> ?#include "pinctrl-imx.h"
>
> -#define IMX_PMX_DUMP(info, p, m, c, n) ? ? ? ? \
> -{ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> - ? ? ? int i, j; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> - ? ? ? printk("Format: Pin Mux Config\n"); ? ? \
> - ? ? ? for (i = 0; i < n; i++) { ? ? ? ? ? ? ? \
> - ? ? ? ? ? ? ? j = p[i]; ? ? ? ? ? ? ? ? ? ? ? \
> - ? ? ? ? ? ? ? printk("%s %d 0x%lx\n", ? ? ? ? \
> - ? ? ? ? ? ? ? ? ? ? ? info->pins[j].name, ? ? \
> - ? ? ? ? ? ? ? ? ? ? ? m[i], c[i]); ? ? ? ? ? ?\
> - ? ? ? } ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> +#define IMX_PMX_DUMP(info, p, m, c, n) ? ? ? ? ? ? ? ? \
> +{ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> + ? ? ? int i, j; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> + ? ? ? printk(KERN_DEBUG "Format: Pin Mux Config\n"); ?\
> + ? ? ? for (i = 0; i < n; i++) { ? ? ? ? ? ? ? ? ? ? ? \
> + ? ? ? ? ? ? ? j = p[i]; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> + ? ? ? ? ? ? ? printk(KERN_DEBUG "%s %d 0x%lx\n", ? ? ?\
> + ? ? ? ? ? ? ? ? ? ? ? info->pins[j].name, ? ? ? ? ? ? \
> + ? ? ? ? ? ? ? ? ? ? ? m[i], c[i]); ? ? ? ? ? ? ? ? ? ?\
> + ? ? ? } ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> ?}
>
> ?/* The bits in CONFIG cell defined in binding doc*/
> @@ -201,10 +201,7 @@ static int imx_dt_node_to_map(struct pinctrl_dev *pctldev,
> ?static void imx_dt_free_map(struct pinctrl_dev *pctldev,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct pinctrl_map *map, unsigned num_maps)
> ?{
> - ? ? ? int i;
> -
> - ? ? ? for (i = 0; i < num_maps; i++)
> - ? ? ? ? ? ? ? kfree(map);
> + ? ? ? kfree(map);
> ?}
>
> ?static struct pinctrl_ops imx_pctrl_ops = {
> @@ -475,9 +472,8 @@ static int __devinit imx_pinctrl_parse_groups(struct device_node *np,
> ? ? ? ? ? ? ? ?grp->configs[j] = config & ~IMX_PAD_SION;
> ? ? ? ?}
>
> -#ifdef DEBUG
> ? ? ? ?IMX_PMX_DUMP(info, grp->pins, grp->mux_mode, grp->configs, grp->npins);
> -#endif
> +
> ? ? ? ?return 0;
> ?}
>
> --
> 1.7.9.5
>
On Fri, Jun 8, 2012 at 9:22 PM, Devendra Naga <[email protected]> wrote:
> a) as we allocate the pinctrl_map structure at imx_dt_node_to_map at ?line 167, anyway
> ? if its an element, or a num_elements * (sizeof(type)) elements allocated to one single
> ? pointer must be freed only once.
>
> CASE. A)
>
> as new_map is not moved and allocated like,
>
> for (i = 0; i < MAX_ELEMS; i++) {
> ? ?new_map[i] = kmalloc(numelems * size, GFP_KERNEL);
> }
>
> its freed as
>
> for (i = 0; i < MAX_ELEMS; i++) {
> ? ?kfree(new_map[i]);
> }
>
> CASE. B)
> and its allocated like
> ? ? ? ?new_map = kmalloc(numelems * size, GFP_KERNEL);
>
> it just needs kfree not as case A's.
>
> b) use KERN_DEBUG facility for the IMX_PMX_DUMP macro.
>
> Signed-off-by: Devendra Naga <[email protected]>
> Acked-by: Dong Aisheng <[email protected]>
OK I applied this, which I *think* is the last version of the patch,
please double-check that the right one is in my git.
This was send the 9th but not tagged v3 so I was a bit confused...
Yours,
Linus Walleij
On Tue, Jun 12, 2012 at 4:31 PM, Linus Walleij <[email protected]> wrote:
>> Signed-off-by: Devendra Naga <[email protected]>
>> Acked-by: Dong Aisheng <[email protected]>
>
> OK I applied this, which I *think* is the last version of the patch,
> please double-check that the right one is in my git.
>
Yeah correct This is the Latest one.
Thanks for taking this in.
> This was send the 9th but not tagged v3 so I was a bit confused...
>
Sorry for not changing the subject to V3 for this.
so for every review, do we need to increment version count of the patch?
> Yours,
> Linus Walleij
On Tue, Jun 12, 2012 at 2:34 PM, devendra.aaru <[email protected]> wrote:
> so for every review, do we need to increment version count of the patch?
No, just make sure I can understand which is the latest one... any way you
want :-)
Linus Walleij