netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Aaro Koskinen <aaro.koskinen@iki.fi>
Cc: support@caviumnetworks.com,
	"David Daney" <david.daney@cavium.com>,
	"Greg KH" <gregkh@linuxfoundation.org>,
	driverdev-devel@linuxdriverproject.org,
	"Sebastian Pöhn" <sebastian.poehn@gmail.com>,
	netdev@vger.kernel.org
Subject: Re: [PATCH] staging: octeon-ethernet: trivial: Avoid OOPS if phydev is not set
Date: Tue, 15 Oct 2013 12:47:50 +0300	[thread overview]
Message-ID: <20131015094750.GA11731@mwanda> (raw)
In-Reply-To: <20131014194937.GD4260@blackmetal.musicnaut.iki.fi>

On Mon, Oct 14, 2013 at 10:49:37PM +0300, Aaro Koskinen wrote:
> On Mon, Oct 14, 2013 at 10:16:49PM +0300, Dan Carpenter wrote:
> > On Mon, Oct 14, 2013 at 09:39:06PM +0300, Aaro Koskinen wrote:
> > > It's initialized in cvm_oct_phy_setup_device():
> > > 
> > > 	priv->phydev = of_phy_connect(dev, phy_node, cvm_oct_adjust_link, 0,
> >       ^^^^^^^^^^^^                                 ^^^^^^^^^^^^^^^^^^^
> > 
> > Sorry I should have explained better.
> > 
> > We use cvm_oct_adjust_link() to initialize priv->phydev but
> > cvm_oct_adjust_link() depends on priv->phydev.  It seems like we would
> > hit the NULL dereference every time.  Weird huh?
> 
> It doesn't happen on my system (EdgeRouter Lite). I think you need to
> explain even more better. :-)
> 
> What you mean by "We use cvm_oct_adjust_link() to initialize
> priv->phydev..."? Sorry, maybe I'm just missing something really
> obvious...

Hm...  I didn't followed the state machine all the way through so I'm
not actually sure how this is called.  But it's weird that priv->phydev
is initialized on the left side of the assignment but dereferenced in
the function mentioned on the right side if the assignment.

Also this patch should not be marked as "trivial" in the subject.
"trivial" is only for spelling mistakes in comments etc.

regards,
dan carpenter

      reply	other threads:[~2013-10-15  9:52 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-13 18:59 [PATCH] staging: octeon-ethernet: trivial: Avoid OOPS if phydev is not set Sebastian Pöhn
2013-10-13 21:28 ` Greg KH
2013-10-14 10:10   ` Dan Carpenter
2013-10-14 18:39     ` Aaro Koskinen
2013-10-14 19:16       ` Dan Carpenter
2013-10-14 19:49         ` Aaro Koskinen
2013-10-15  9:47           ` Dan Carpenter [this message]

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=20131015094750.GA11731@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=aaro.koskinen@iki.fi \
    --cc=david.daney@cavium.com \
    --cc=driverdev-devel@linuxdriverproject.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=netdev@vger.kernel.org \
    --cc=sebastian.poehn@gmail.com \
    --cc=support@caviumnetworks.com \
    /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).