From: Tony Lindgren <tony@atomide.com>
To: Stephen Warren <swarren@nvidia.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Linus Walleij <linus.walleij@stericsson.com>,
Barry Song <21cnbao@gmail.com>,
Haojian Zhuang <haojian.zhuang@marvell.com>,
Grant Likely <grant.likely@secretlab.ca>,
Thomas Abraham <thomas.abraham@linaro.org>,
Rajendra Nayak <rajendra.nayak@linaro.org>,
Dong Aisheng <dong.aisheng@linaro.org>,
Shawn Guo <shawn.guo@freescale.com>
Subject: Re: [PATCH 2/4] pinctrl: Fix pinmux_hog_maps when ctrl_dev_name is not set
Date: Tue, 24 Jan 2012 16:23:41 -0800 [thread overview]
Message-ID: <20120125002340.GV22818@atomide.com> (raw)
In-Reply-To: <20120120173915.GP22818@atomide.com>
* Tony Lindgren <tony@atomide.com> [120120 09:06]:
> * Stephen Warren <swarren@nvidia.com> [120120 08:24]:
> > Tony Lindgren wrote at Friday, January 20, 2012 9:17 AM:
> > > The ctrl_dev_name is optional for struct pinmux_map assuming
> > > that ctrl_dev is set. Without this patch we can get:
> > >
> > > Unable to handle kernel NULL pointer dereference at virtual address 00000000
> >
> > > diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> >
> >
> > > @@ -992,9 +992,12 @@ int pinmux_hog_maps(struct pinctrl_dev *pctldev)
> > >
> > > for (i = 0; i < pinmux_maps_num; i++) {
> > > struct pinmux_map const *map = &pinmux_maps[i];
> > > + int match_found = 0;
> > >
> > > - if (((map->ctrl_dev == dev) ||
> > > - !strcmp(map->ctrl_dev_name, devname)) &&
> > > + if (map->ctrl_dev_name && !strcmp(map->ctrl_dev_name, devname))
> > > + match_found = 1;
> > > +
> > > + if (((map->ctrl_dev == dev) || match_found) &&
> > > map->hog_on_boot) {
> > > /* OK time to hog! */
> > > ret = pinmux_hog_map(pctldev, map);
> >
> > Wouldn't it be better if match_found was true for all matching cases,
> > in other words for the new code to be:
> >
> > int match_found = 0;
> >
> > if (map->ctrl_dev_name && !strcmp(map->ctrl_dev_name, devname))
> > match_found = 1;
> > if (map->ctrl_dev == dev)
> > match_found = 1;
> > if (match_found && map->hog_on_boot) {
>
> OK will take a look, that would make the code easier to read.
Here's this one updated, looks like we can keep it readable
even without match_found if we continue the search early based
on !map->hog_on_boot.
Regards,
Tony
From: Tony Lindgren <tony@atomide.com>
Date: Fri, 20 Jan 2012 07:43:53 -0800
Subject: [PATCH] pinctrl: Fix pinmux_hog_maps when ctrl_dev_name is not set
The ctrl_dev_name is optional for struct pinmux_map assuming
that ctrl_dev is set. Without this patch we can get:
Unable to handle kernel NULL pointer dereference at virtual address 00000000
...
(pinmux_hog_maps+0xa4/0x20c)
(pinctrl_register+0x2a4/0x378)
...
Fix this by adding adding a test for map->ctrl_dev.
Additionally move the test for map->ctrl_dev earlier
to optimize out the loop a bit.
Signed-off-by: Tony Lindgren <tony@atomide.com>
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -993,9 +993,12 @@ int pinmux_hog_maps(struct pinctrl_dev *pctldev)
for (i = 0; i < pinmux_maps_num; i++) {
struct pinmux_map const *map = &pinmux_maps[i];
- if (((map->ctrl_dev == dev) ||
- !strcmp(map->ctrl_dev_name, devname)) &&
- map->hog_on_boot) {
+ if (!map->hog_on_boot)
+ continue;
+
+ if ((map->ctrl_dev == dev) ||
+ (map->ctrl_dev_name &&
+ !strcmp(map->ctrl_dev_name, devname))) {
/* OK time to hog! */
ret = pinmux_hog_map(pctldev, map);
if (ret)
next prev parent reply other threads:[~2012-01-25 0:23 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-20 16:17 [PATCH 0/4] Some pinctrl fixes for pinmux modules Tony Lindgren
2012-01-20 16:17 ` [PATCH 1/4] pinctrl: Free debugfs entries when unloading a pinmux driver Tony Lindgren
2012-01-24 21:51 ` Linus Walleij
2012-01-20 16:17 ` [PATCH 2/4] pinctrl: Fix pinmux_hog_maps when ctrl_dev_name is not set Tony Lindgren
2012-01-20 16:57 ` Stephen Warren
2012-01-20 17:39 ` Tony Lindgren
2012-01-25 0:23 ` Tony Lindgren [this message]
2012-01-25 0:47 ` Stephen Warren
2012-01-26 11:51 ` Linus Walleij
2012-01-20 16:17 ` [PATCH 3/4] pinctrl: Add checks for empty names in pinmux_search_function Tony Lindgren
2012-01-20 17:00 ` Stephen Warren
2012-01-20 17:41 ` Tony Lindgren
2012-01-25 22:08 ` Tony Lindgren
2012-01-26 13:14 ` Linus Walleij
2012-01-20 16:17 ` [PATCH 4/4] pinctrl: Fix some pinmux typos Tony Lindgren
2012-01-24 21:54 ` Linus Walleij
2012-01-24 21:55 ` [PATCH 0/4] Some pinctrl fixes for pinmux modules Linus Walleij
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120125002340.GV22818@atomide.com \
--to=tony@atomide.com \
--cc=21cnbao@gmail.com \
--cc=dong.aisheng@linaro.org \
--cc=grant.likely@secretlab.ca \
--cc=haojian.zhuang@marvell.com \
--cc=linus.walleij@stericsson.com \
--cc=linux-kernel@vger.kernel.org \
--cc=rajendra.nayak@linaro.org \
--cc=shawn.guo@freescale.com \
--cc=swarren@nvidia.com \
--cc=thomas.abraham@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).