pinctrl: pinctrl-imx: free allocated pinctrl_map structure only once and use kernel facilities for IMX_PMX_DUMP
Subsystems (2)
PATCH 1/2 pinctrl: pinctrl-imx: free allocated pinctrl_map structure only once and use kernel facilities for IMX_PMX_DUMP
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.
--- drivers/pinctrl/pinctrl-imx.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-)
drivers/pinctrl/pinctrl-imx.c +12 -16
@@ -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 ; }
Signature · 1 line
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.
Quoted 89 lines · Devendra Naga
a) as we allocate the
pinctrl_mapstructure atimx_dt_node_to_mapat line 167, anyway if its an element, or anum_elements* (sizeof(type)) elements allocated to one single pointer must be freed only once.CASE. A)
as
new_mapis 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_DEBUGfacility for theIMX_PMX_DUMPmacro.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(-)drivers/pinctrl/pinctrl-imx.c +12
-17@@ -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
Quoted 22 lines · Devendra Naga
a) as we allocate the
pinctrl_mapstructure atimx_dt_node_to_mapat line 167, anyway if its an element, or anum_elements* (sizeof(type)) elements allocated to one single pointer must be freed only once.CASE. A)
as
new_mapis 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);
Quoted 6 lines
it just needs kfree not as case A's.
b) use
KERN_DEBUGfacility for theIMX_PMX_DUMPmacro.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
Quoted 6 lines · Linus Walleij
Quoted 2 lines
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.
Quoted 2 lines
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?
Quoted 2 lines
Yours,
Linus Walleij
Quoted 1 line · devendra.aaru
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