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]>
---
drivers/pinctrl/pinctrl-imx.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-imx.c b/drivers/pinctrl/pinctrl-imx.c
index f6e7c67..553ed95 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*/
@@ -203,8 +203,7 @@ static void imx_dt_free_map(struct pinctrl_dev *pctldev,
{
int i;
- for (i = 0; i < num_maps; i++)
- kfree(map);
+ kfree(map);
}
static struct pinctrl_ops imx_pctrl_ops = {
--
1.7.9.5
Anybody Please??
On Sat, Jun 2, 2012 at 10:33 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]>
> ---
> ?drivers/pinctrl/pinctrl-imx.c | ? 23 +++++++++++------------
> ?1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-imx.c b/drivers/pinctrl/pinctrl-imx.c
> index f6e7c67..553ed95 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*/
> @@ -203,8 +203,7 @@ static void imx_dt_free_map(struct pinctrl_dev *pctldev,
> ?{
> ? ? ? ?int i;
>
> - ? ? ? for (i = 0; i < num_maps; i++)
> - ? ? ? ? ? ? ? kfree(map);
> + ? ? ? kfree(map);
> ?}
>
> ?static struct pinctrl_ops imx_pctrl_ops = {
> --
> 1.7.9.5
>
On Sat, Jun 2, 2012 at 7:03 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]>
> ---
> ?drivers/pinctrl/pinctrl-imx.c | ? 23 +++++++++++------------
> ?1 file changed, 11 insertions(+), 12 deletions(-)
Shawn/Dong: can you look at this patch too?
Yours,
Linus Walleij
On Mon, Jun 4, 2012 at 1:37 PM, devendra.aaru <[email protected]> wrote:
> Anybody Please??
Take it easy. Give the Freescalers some time to review...
If nothing happens for a week I'll just apply it so they can shout
if there's something wrong with it :-D
Linus
Hi Linus,
Thanks for the reply,
On Thu, Jun 7, 2012 at 4:34 PM, Linus Walleij <[email protected]> wrote:
> On Mon, Jun 4, 2012 at 1:37 PM, devendra.aaru <[email protected]> wrote:
>
>> Anybody Please??
>
> Take it easy. Give the Freescalers some time to review...
>
> If nothing happens for a week I'll just apply it so they can shout
> if there's something wrong with it :-D
>
Sorry for shouting ... :(
> Linus
Thanks,
Devendra.
On Sat, Jun 02, 2012 at 10:33:53PM +0530, Devendra Naga 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]>
> ---
> drivers/pinctrl/pinctrl-imx.c | 23 +++++++++++------------
> 1 file changed, 11 insertions(+), 12 deletions(-)
>
Sorry for late reply.
I missed this patch series.
You can CC me next time. :)
> diff --git a/drivers/pinctrl/pinctrl-imx.c b/drivers/pinctrl/pinctrl-imx.c
> index f6e7c67..553ed95 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]); \
> + } \
I can't remember clearly but i encountered a strange issue before
that the format was not what i want with KERN_DEBUG...
But i just tried your patch and it did not appear.
So i'm ok with this change.
> }
>
> /* The bits in CONFIG cell defined in binding doc*/
> @@ -203,8 +203,7 @@ static void imx_dt_free_map(struct pinctrl_dev *pctldev,
> {
> int i;
Also need removed?
>
> - for (i = 0; i < num_maps; i++)
> - kfree(map);
> + kfree(map);
BTW, it would be better if the change the patch title to:
"pinctrl: pinctrl-imx: ...."
Otherwise,
Acked-by: Dong Aisheng <[email protected]>
Regards
Dong Aisheng
Hi Dong,
Thanks for the review.
On Thu, Jun 7, 2012 at 5:35 PM, Dong Aisheng <[email protected]> wrote:
> On Sat, Jun 02, 2012 at 10:33:53PM +0530, Devendra Naga 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]>
>> ---
>> ?drivers/pinctrl/pinctrl-imx.c | ? 23 +++++++++++------------
>> ?1 file changed, 11 insertions(+), 12 deletions(-)
>>
> Sorry for late reply.
> I missed this patch series.
> You can CC me next time. :)
>
>> diff --git a/drivers/pinctrl/pinctrl-imx.c b/drivers/pinctrl/pinctrl-imx.c
>> index f6e7c67..553ed95 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]); ? ? ? ? ? ? ? ? ? ?\
>> + ? ? } ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> I can't remember clearly but i encountered a strange issue before
> that the format was not what i want with KERN_DEBUG...
> But i just tried your patch and it did not appear.
> So i'm ok with this change.
>
>> ?}
>>
>> ?/* The bits in CONFIG cell defined in binding doc*/
>> @@ -203,8 +203,7 @@ static void imx_dt_free_map(struct pinctrl_dev *pctldev,
>> ?{
>> ? ? ? int i;
> Also need removed?
>
Yeah. sorry , will send again with you Ack.
>>
>> - ? ? for (i = 0; i < num_maps; i++)
>> - ? ? ? ? ? ? kfree(map);
>> + ? ? kfree(map);
>
> BTW, it would be better if the change the patch title to:
> "pinctrl: pinctrl-imx: ...."
Ok sure.
>
> Otherwise,
> Acked-by: Dong Aisheng <[email protected]>
>
> Regards
> Dong Aisheng
>
Thanks,
Devendra.