mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Sascha Hauer <s.hauer@pengutronix.de>
To: Alexander Aring <alex.aring@gmail.com>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH v2] pinctrl: mvebu: add pinctrl drivers for Dove and Kirkwood
Date: Thu, 26 Jun 2014 07:56:58 +0200	[thread overview]
Message-ID: <20140626055658.GW15686@pengutronix.de> (raw)
In-Reply-To: <20140625124006.GA25802@omega>

On Wed, Jun 25, 2014 at 02:40:08PM +0200, Alexander Aring wrote:
> Hi,
> 
> I resend this mail. My last one was malformed, because I tried to
> answert this mail via smartphone while boring lecture at university.
> 
> On Wed, Jun 25, 2014 at 08:56:15AM +0200, Sascha Hauer wrote:
> > On Tue, Jun 24, 2014 at 12:43:48PM +0200, Sebastian Hesselbarth wrote:
> > > This adds pinctrl drivers for Marvell Dove and Kirkwood SoCs based
> > > on a common driver stub. This design is based on the corresponding
> > > Linux driver and should ease additional drivers for Marvell Armada
> > > SoCs.
> > > 
> > > Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> > 
> > Applied with a small change:
> > 
> > > +static int kirkwood_pinctrl_probe(struct device_d *dev)
> > > +{
> > > +	const struct of_device_id *match =
> > > +		of_match_node(kirkwood_pinctrl_of_match, dev->device_node);
> > > +	struct mvebu_pinctrl_soc_info *soc =
> > > +		(struct mvebu_pinctrl_soc_info *)match->data;
> > > +
> > > +	mpp_base = dev_request_mem_region(dev, 0);
> > 
> > I added a return value check here. Not checking it means that the driver
> > could do NULL pointer dereferences during runtime.
> > 
> > I should really fix the places where the check is missing in the tree.
> > 
> 
> I spotted this also at my last patch for print warning for resource
> conflicts and I thought a zero base address is also valid for some
> cases.
> 
> Thats why I didn't add checks on null.
> 
> What I mean is that the dev_request_mem_region API reference can return
> NULL which is for example "(void *)0x00000000" and is also valid. We
> can't use this as error indicator.

We could also return an error pointer instead of NULL. Given that not
many drivers check the return value anyway this shouldn't be hard to
change.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

  reply	other threads:[~2014-06-26  5:57 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-24 10:43 Sebastian Hesselbarth
2014-06-25  6:56 ` Sascha Hauer
2014-06-25  7:35   ` Sebastian Hesselbarth
2014-06-25 12:40   ` Alexander Aring
2014-06-26  5:56     ` Sascha Hauer [this message]
2014-06-26 14:38       ` Alexander Aring
2014-06-26 19:01         ` Sascha Hauer
2014-06-25 13:52   ` [PATCH FIXUP] pinctrl: mvebu: fixup mpp_base check Sebastian Hesselbarth
2014-06-26  5:58     ` Sascha Hauer

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=20140626055658.GW15686@pengutronix.de \
    --to=s.hauer@pengutronix.de \
    --cc=alex.aring@gmail.com \
    --cc=barebox@lists.infradead.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