From mboxrd@z Thu Jan 1 00:00:00 1970 From: aisheng.dong@freescale.com (Dong Aisheng) Date: Wed, 20 Jun 2012 17:17:35 +0800 Subject: [PATCH] pinctrl: pinctrl-imx: fix map setting problem if NO_PAD_CTL In-Reply-To: <1340178101-21605-1-git-send-email-jason77.wang@gmail.com> References: <1340178101-21605-1-git-send-email-jason77.wang@gmail.com> Message-ID: <20120620091735.GI10387@shlinux2.ap.freescale.net> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 > Cc: Linus Walleij > Cc: Shawn Guo > Signed-off-by: Hui Wang > --- > This problem is easily reproduced if we set pinctrl in the > ${board}.dts like following: > fsl,pins = 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