All of lore.kernel.org
 help / color / mirror / Atom feed
From: florian@openwrt.org (Florian Fainelli)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] Fix phy_attach - forward dev_flags for phy_attach
Date: Thu, 10 Jan 2013 18:12:53 +0100	[thread overview]
Message-ID: <50EEF695.9060001@openwrt.org> (raw)
In-Reply-To: <BC373419EB4337418B2B595BAEDC155F015896370062@IL-MB01.marvell.com>

Le 01/10/13 16:57, Kosta Zertsekel a ?crit :
>> By the way, most, if not all of the phy_connect() users in drivers/net/ethernet/ also do not ensure they pass the phy device flags, so you might
> want to fix this globally and not just for Marvell driver.
> Indeed, phy_connect() mostly just pass zero intead of phy_dev->dev_flags.
> But, I think, the guy that calls phy_connect() in its driver should know what he does, and,
> probably, we should rely on him knowing his stuff.
> The only evidence of the bug is when phy_dev->dev_flags was actually changed by PHY fixup callback
> (see dns323-setup.c for example) and was *not* propagated to phy_connect() or phy_attach() as in pxa168_eth.c phy_init().
> The code conforming to the former should be fixed IMHO.

Actually, I wonder if we should not rather remove entirely the flags 
argument, let the phy_connect() or phy_attach() callers modify the phy 
device dev_flags like it does today (e.g: tg3) and modify phy_connect() 
and phy_attach() to pass phy->dev_flags to phy_attach_direct().
--
Florian

WARNING: multiple messages have this Message-ID (diff)
From: Florian Fainelli <florian@openwrt.org>
To: Kosta Zertsekel <konszert@marvell.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"zertsekel@gmail.com" <zertsekel@gmail.com>,
	"andrew@lunn.ch" <andrew@lunn.ch>,
	Eran Ben-Avi <benavi@marvell.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Lior Amsalem <alior@marvell.com>
Subject: Re: [PATCH 2/2] Fix phy_attach - forward dev_flags for phy_attach
Date: Thu, 10 Jan 2013 18:12:53 +0100	[thread overview]
Message-ID: <50EEF695.9060001@openwrt.org> (raw)
In-Reply-To: <BC373419EB4337418B2B595BAEDC155F015896370062@IL-MB01.marvell.com>

Le 01/10/13 16:57, Kosta Zertsekel a écrit :
>> By the way, most, if not all of the phy_connect() users in drivers/net/ethernet/ also do not ensure they pass the phy device flags, so you might
> want to fix this globally and not just for Marvell driver.
> Indeed, phy_connect() mostly just pass zero intead of phy_dev->dev_flags.
> But, I think, the guy that calls phy_connect() in its driver should know what he does, and,
> probably, we should rely on him knowing his stuff.
> The only evidence of the bug is when phy_dev->dev_flags was actually changed by PHY fixup callback
> (see dns323-setup.c for example) and was *not* propagated to phy_connect() or phy_attach() as in pxa168_eth.c phy_init().
> The code conforming to the former should be fixed IMHO.

Actually, I wonder if we should not rather remove entirely the flags 
argument, let the phy_connect() or phy_attach() callers modify the phy 
device dev_flags like it does today (e.g: tg3) and modify phy_connect() 
and phy_attach() to pass phy->dev_flags to phy_attach_direct().
--
Florian

  reply	other threads:[~2013-01-10 17:12 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-10 12:00 Fix phy_init for Marvell network eth driver Kosta Zertsekel
2013-01-10 12:00 ` Kosta Zertsekel
2013-01-10 12:00 ` [PATCH 1/2] " Kosta Zertsekel
2013-01-10 12:00   ` Kosta Zertsekel
2013-01-10 13:22   ` Sergei Shtylyov
2013-01-10 13:22     ` Sergei Shtylyov
2013-01-10 12:00 ` [PATCH 2/2] Fix phy_attach - forward dev_flags for phy_attach Kosta Zertsekel
2013-01-10 12:00   ` Kosta Zertsekel
2013-01-10 14:40   ` Florian Fainelli
2013-01-10 14:40     ` Florian Fainelli
2013-01-10 15:57     ` Kosta Zertsekel
2013-01-10 15:57       ` Kosta Zertsekel
2013-01-10 17:12       ` Florian Fainelli [this message]
2013-01-10 17:12         ` Florian Fainelli
2013-01-10 12:17 ` Fix phy_init for Marvell network eth driver Jason Cooper
2013-01-10 12:17   ` Jason Cooper
2013-01-10 12:24 ` Andrew Lunn
2013-01-10 12:24   ` Andrew Lunn
2013-01-10 12:27   ` Kosta Zertsekel
2013-01-10 12:27     ` Kosta Zertsekel

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=50EEF695.9060001@openwrt.org \
    --to=florian@openwrt.org \
    --cc=linux-arm-kernel@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.