* [PATCH] pinctrl: pinctrl-imx: fix map setting problem if NO_PAD_CTL
@ 2012-06-20 7:41 Hui Wang
2012-06-20 8:51 ` Linus Walleij
2012-06-20 9:17 ` Dong Aisheng
0 siblings, 2 replies; 5+ messages in thread
From: Hui Wang @ 2012-06-20 7:41 UTC (permalink / raw)
To: linux-arm-kernel
new_map is allocated according to map_num instead of grp->npins,
if a pin or some pins of a group has NO_PAD_CTL property, the map_num
and the grp->npin are different definitely.
When we set mapping information to the new_map[], we should skip those
pins with NO_PAD_CTL from index, otherwise it is possible the driver
will aceesss to a unallocated region.
Cc: Dong Aisheng <dong.aisheng@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Hui Wang <jason77.wang@gmail.com>
---
This problem is easily reproduced if we set pinctrl in the
${board}.dts like following:
fsl,pins = <pin_func_id1 config_with_NO_PAD_CTL
pin_func_id2 config_with_NO_PAD_CTL
pin_func_id3 config_normal>;
drivers/pinctrl/pinctrl-imx.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-imx.c b/drivers/pinctrl/pinctrl-imx.c
index dd6d93a..0e21abb 100644
--- a/drivers/pinctrl/pinctrl-imx.c
+++ b/drivers/pinctrl/pinctrl-imx.c
@@ -146,6 +146,7 @@ static int imx_dt_node_to_map(struct pinctrl_dev *pctldev,
struct pinctrl_map *new_map;
struct device_node *parent;
int map_num = 1;
+ int map_pos = 0;
int i;
/*
@@ -186,11 +187,11 @@ static int imx_dt_node_to_map(struct pinctrl_dev *pctldev,
new_map++;
for (i = 0; i < grp->npins; i++) {
if (!(grp->configs[i] & IMX_NO_PAD_CTL)) {
- new_map[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
- new_map[i].data.configs.group_or_pin =
+ new_map[map_pos].type = PIN_MAP_TYPE_CONFIGS_PIN;
+ new_map[map_pos].data.configs.group_or_pin =
pin_get_name(pctldev, grp->pins[i]);
- new_map[i].data.configs.configs = &grp->configs[i];
- new_map[i].data.configs.num_configs = 1;
+ new_map[map_pos].data.configs.configs = &grp->configs[i];
+ new_map[map_pos++].data.configs.num_configs = 1;
}
}
--
1.7.11
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] pinctrl: pinctrl-imx: fix map setting problem if NO_PAD_CTL
2012-06-20 7:41 [PATCH] pinctrl: pinctrl-imx: fix map setting problem if NO_PAD_CTL Hui Wang
@ 2012-06-20 8:51 ` Linus Walleij
2012-06-20 9:17 ` Dong Aisheng
1 sibling, 0 replies; 5+ messages in thread
From: Linus Walleij @ 2012-06-20 8:51 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jun 20, 2012 at 9:41 AM, Hui Wang <jason77.wang@gmail.com> wrote:
> new_map is allocated according to map_num instead of grp->npins,
> if a pin or some pins of a group has NO_PAD_CTL property, the map_num
> and the grp->npin are different definitely.
>
> When we set mapping information to the new_map[], we should skip those
> pins with NO_PAD_CTL from index, otherwise it is possible the driver
> will aceesss to a unallocated region.
>
> Cc: Dong Aisheng <dong.aisheng@linaro.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Signed-off-by: Hui Wang <jason77.wang@gmail.com>
Song/Shawn: can you check this patch and ACK/NACK it?
Should it go into the -rc:s?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] pinctrl: pinctrl-imx: fix map setting problem if NO_PAD_CTL
2012-06-20 7:41 [PATCH] pinctrl: pinctrl-imx: fix map setting problem if NO_PAD_CTL Hui Wang
2012-06-20 8:51 ` Linus Walleij
@ 2012-06-20 9:17 ` Dong Aisheng
2012-06-20 9:52 ` Hui Wang
1 sibling, 1 reply; 5+ messages in thread
From: Dong Aisheng @ 2012-06-20 9:17 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jun 20, 2012 at 03:41:41PM +0800, Hui Wang wrote:
> new_map is allocated according to map_num instead of grp->npins,
> if a pin or some pins of a group has NO_PAD_CTL property, the map_num
> and the grp->npin are different definitely.
>
> When we set mapping information to the new_map[], we should skip those
> pins with NO_PAD_CTL from index, otherwise it is possible the driver
> will aceesss to a unallocated region.
>
Good catching.
Thanks.
> Cc: Dong Aisheng <dong.aisheng@linaro.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Signed-off-by: Hui Wang <jason77.wang@gmail.com>
> ---
> This problem is easily reproduced if we set pinctrl in the
> ${board}.dts like following:
> fsl,pins = <pin_func_id1 config_with_NO_PAD_CTL
> pin_func_id2 config_with_NO_PAD_CTL
> pin_func_id3 config_normal>;
>
> drivers/pinctrl/pinctrl-imx.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-imx.c b/drivers/pinctrl/pinctrl-imx.c
> index dd6d93a..0e21abb 100644
> --- a/drivers/pinctrl/pinctrl-imx.c
> +++ b/drivers/pinctrl/pinctrl-imx.c
> @@ -146,6 +146,7 @@ static int imx_dt_node_to_map(struct pinctrl_dev *pctldev,
> struct pinctrl_map *new_map;
> struct device_node *parent;
> int map_num = 1;
> + int map_pos = 0;
> int i;
>
> /*
> @@ -186,11 +187,11 @@ static int imx_dt_node_to_map(struct pinctrl_dev *pctldev,
> new_map++;
> for (i = 0; i < grp->npins; i++) {
> if (!(grp->configs[i] & IMX_NO_PAD_CTL)) {
> - new_map[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
> - new_map[i].data.configs.group_or_pin =
> + new_map[map_pos].type = PIN_MAP_TYPE_CONFIGS_PIN;
> + new_map[map_pos].data.configs.group_or_pin =
> pin_get_name(pctldev, grp->pins[i]);
> - new_map[i].data.configs.configs = &grp->configs[i];
> - new_map[i].data.configs.num_configs = 1;
> + new_map[map_pos].data.configs.configs = &grp->configs[i];
> + new_map[map_pos++].data.configs.num_configs = 1;
I'm ok with the change, only a minor comment:
Does the change as follows look better?
- for (i = 0; i < grp->npins; i++) {
+ for (i = j = 0; i < grp->npins; i++) {
if (!(grp->configs[i] & IMX_NO_PAD_CTL)) {
- new_map[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
- new_map[i].data.configs.group_or_pin =
+ new_map[j].type = PIN_MAP_TYPE_CONFIGS_PIN;
+ new_map[j].data.configs.group_or_pin =
pin_get_name(pctldev, grp->pins[i]);
- new_map[i].data.configs.configs = &grp->configs[i];
- new_map[i].data.configs.num_configs = 1;
+ new_map[j].data.configs.configs = &grp->configs[i];
+ new_map[j].data.configs.num_configs = 1;
+ j++;
Regards
Dong Aisheng
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] pinctrl: pinctrl-imx: fix map setting problem if NO_PAD_CTL
2012-06-20 9:17 ` Dong Aisheng
@ 2012-06-20 9:52 ` Hui Wang
2012-06-20 10:14 ` Hui Wang
0 siblings, 1 reply; 5+ messages in thread
From: Hui Wang @ 2012-06-20 9:52 UTC (permalink / raw)
To: linux-arm-kernel
Dong Aisheng wrote:
> On Wed, Jun 20, 2012 at 03:41:41PM +0800, Hui Wang wrote:
>
>> new_map is allocated according to map_num instead of grp->npins,
>> if a pin or some pins of a group has NO_PAD_CTL property, the map_num
>> and the grp->npin are different definitely.
>>
>> When we set mapping information to the new_map[], we should skip those
>> pins with NO_PAD_CTL from index, otherwise it is possible the driver
>> will aceesss to a unallocated region.
>>
>>
> Good catching.
> Thanks.
>
>
>> Cc: Dong Aisheng <dong.aisheng@linaro.org>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Shawn Guo <shawn.guo@linaro.org>
>> Signed-off-by: Hui Wang <jason77.wang@gmail.com>
>> ---
>> This problem is easily reproduced if we set pinctrl in the
>> ${board}.dts like following:
>> fsl,pins = <pin_func_id1 config_with_NO_PAD_CTL
>> pin_func_id2 config_with_NO_PAD_CTL
>> pin_func_id3 config_normal>;
>>
>> drivers/pinctrl/pinctrl-imx.c | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-imx.c b/drivers/pinctrl/pinctrl-imx.c
>> index dd6d93a..0e21abb 100644
>> --- a/drivers/pinctrl/pinctrl-imx.c
>> +++ b/drivers/pinctrl/pinctrl-imx.c
>> @@ -146,6 +146,7 @@ static int imx_dt_node_to_map(struct pinctrl_dev *pctldev,
>> struct pinctrl_map *new_map;
>> struct device_node *parent;
>> int map_num = 1;
>> + int map_pos = 0;
>> int i;
>>
>> /*
>> @@ -186,11 +187,11 @@ static int imx_dt_node_to_map(struct pinctrl_dev *pctldev,
>> new_map++;
>> for (i = 0; i < grp->npins; i++) {
>> if (!(grp->configs[i] & IMX_NO_PAD_CTL)) {
>> - new_map[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
>> - new_map[i].data.configs.group_or_pin =
>> + new_map[map_pos].type = PIN_MAP_TYPE_CONFIGS_PIN;
>> + new_map[map_pos].data.configs.group_or_pin =
>> pin_get_name(pctldev, grp->pins[i]);
>> - new_map[i].data.configs.configs = &grp->configs[i];
>> - new_map[i].data.configs.num_configs = 1;
>> + new_map[map_pos].data.configs.configs = &grp->configs[i];
>> + new_map[map_pos++].data.configs.num_configs = 1;
>>
> I'm ok with the change, only a minor comment:
> Does the change as follows look better?
>
It is fine to me. Do you need me to prepare a new patch as your suggestion?
Regards,
Hui.
> - for (i = 0; i < grp->npins; i++) {
> + for (i = j = 0; i < grp->npins; i++) {
> if (!(grp->configs[i] & IMX_NO_PAD_CTL)) {
> - new_map[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
> - new_map[i].data.configs.group_or_pin =
> + new_map[j].type = PIN_MAP_TYPE_CONFIGS_PIN;
> + new_map[j].data.configs.group_or_pin =
> pin_get_name(pctldev, grp->pins[i]);
> - new_map[i].data.configs.configs = &grp->configs[i];
> - new_map[i].data.configs.num_configs = 1;
> + new_map[j].data.configs.configs = &grp->configs[i];
> + new_map[j].data.configs.num_configs = 1;
> + j++;
>
> Regards
> Dong Aisheng
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] pinctrl: pinctrl-imx: fix map setting problem if NO_PAD_CTL
2012-06-20 9:52 ` Hui Wang
@ 2012-06-20 10:14 ` Hui Wang
0 siblings, 0 replies; 5+ messages in thread
From: Hui Wang @ 2012-06-20 10:14 UTC (permalink / raw)
To: linux-arm-kernel
Hui Wang wrote:
> Dong Aisheng wrote:
>> On Wed, Jun 20, 2012 at 03:41:41PM +0800, Hui Wang wrote:
>>> new_map is allocated according to map_num instead of grp->npins,
>>> if a pin or some pins of a group has NO_PAD_CTL property, the map_num
>>> and the grp->npin are different definitely.
>>>
>>> When we set mapping information to the new_map[], we should skip those
>>> pins with NO_PAD_CTL from index, otherwise it is possible the driver
>>> will aceesss to a unallocated region.
>>>
>> Good catching.
>> Thanks.
>>
>>> Cc: Dong Aisheng <dong.aisheng@linaro.org>
>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>> Cc: Shawn Guo <shawn.guo@linaro.org>
>>> Signed-off-by: Hui Wang <jason77.wang@gmail.com>
>>> ---
>>> This problem is easily reproduced if we set pinctrl in the
>>> ${board}.dts like following:
>>> fsl,pins = <pin_func_id1 config_with_NO_PAD_CTL
>>> pin_func_id2 config_with_NO_PAD_CTL
>>> pin_func_id3 config_normal>;
>>>
>>> drivers/pinctrl/pinctrl-imx.c | 9 +++++----
>>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/pinctrl/pinctrl-imx.c
>>> b/drivers/pinctrl/pinctrl-imx.c
>>> index dd6d93a..0e21abb 100644
>>> --- a/drivers/pinctrl/pinctrl-imx.c
>>> +++ b/drivers/pinctrl/pinctrl-imx.c
>>> @@ -146,6 +146,7 @@ static int imx_dt_node_to_map(struct pinctrl_dev
>>> *pctldev,
>>> struct pinctrl_map *new_map;
>>> struct device_node *parent;
>>> int map_num = 1;
>>> + int map_pos = 0;
>>> int i;
>>>
>>> /*
>>> @@ -186,11 +187,11 @@ static int imx_dt_node_to_map(struct
>>> pinctrl_dev *pctldev,
>>> new_map++;
>>> for (i = 0; i < grp->npins; i++) {
>>> if (!(grp->configs[i] & IMX_NO_PAD_CTL)) {
>>> - new_map[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
>>> - new_map[i].data.configs.group_or_pin =
>>> + new_map[map_pos].type = PIN_MAP_TYPE_CONFIGS_PIN;
>>> + new_map[map_pos].data.configs.group_or_pin =
>>> pin_get_name(pctldev, grp->pins[i]);
>>> - new_map[i].data.configs.configs = &grp->configs[i];
>>> - new_map[i].data.configs.num_configs = 1;
>>> + new_map[map_pos].data.configs.configs = &grp->configs[i];
>>> + new_map[map_pos++].data.configs.num_configs = 1;
>> I'm ok with the change, only a minor comment:
>> Does the change as follows look better?
> It is fine to me. Do you need me to prepare a new patch as your
> suggestion?
>
> Regards,
> Hui.
>> - for (i = 0; i < grp->npins; i++) {
>> + for (i = j = 0; i < grp->npins; i++) {
>> if (!(grp->configs[i] & IMX_NO_PAD_CTL)) {
>> - new_map[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
>> - new_map[i].data.configs.group_or_pin =
>> + new_map[j].type = PIN_MAP_TYPE_CONFIGS_PIN;
>> + new_map[j].data.configs.group_or_pin =
>> pin_get_name(pctldev, grp->pins[i]);
>> - new_map[i].data.configs.configs = &grp->configs[i];
>> - new_map[i].data.configs.num_configs = 1;
>> + new_map[j].data.configs.configs = &grp->configs[i];
>> + new_map[j].data.configs.num_configs = 1;
>> + j++;
Changed it in the v2.
Regards,
Hui.
>>
>> Regards
>> Dong Aisheng
>>
>>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-06-20 10:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-20 7:41 [PATCH] pinctrl: pinctrl-imx: fix map setting problem if NO_PAD_CTL Hui Wang
2012-06-20 8:51 ` Linus Walleij
2012-06-20 9:17 ` Dong Aisheng
2012-06-20 9:52 ` Hui Wang
2012-06-20 10:14 ` Hui Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).