This patch fixes a use-after-free introduced by
commit a79d8e93d3 and spotted by the
Coverity checker.
Signed-off-by: Adrian Bunk <bunk@kernel.org>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
PHY read/write functions can potentially sleep (e.g., a PHY accessed
via I2C). The following changes were made to account for this:
* Change spin locks to mutex locks
* Add a BUG_ON() to phy_read() phy_write() to warn against
calling them from an interrupt context.
* Use work queue for PHY state machine handling since
it can potentially sleep
* Change phydev lock from spinlock to mutex
Signed-off-by: Nate Case <ncase@xes-inc.com>
Acked-by: Andy Fleming <afleming@freescale.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
With that patch fixed.c now fully emulates MDIO bus, thus no need
to duplicate PHY layer functionality. That, in turn, drastically
simplifies the code, and drops down line count.
As an additional bonus, now there is no need to register MDIO bus
for each PHY, all emulated PHYs placed on the platform fixed MDIO bus.
There is also no more need to pre-allocate PHYs via .config option,
this is all now handled dynamically.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
Acked-by: Jeff Garzik <jeff@garzik.org>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Lately I've got this nice badness on mdio bus removal:
Device 'e0103120:06' does not have a release() function, it is broken and must be fixed.
------------[ cut here ]------------
Badness at drivers/base/core.c:107
NIP: c015c1a8 LR: c015c1a8 CTR: c0157488
REGS: c34bdcf0 TRAP: 0700 Not tainted (2.6.23-rc5-g9ebadfbb-dirty)
MSR: 00029032 <EE,ME,IR,DR> CR: 24088422 XER: 00000000
...
[c34bdda0] [c015c1a8] device_release+0x78/0x80 (unreliable)
[c34bddb0] [c01354cc] kobject_cleanup+0x80/0xbc
[c34bddd0] [c01365f0] kref_put+0x54/0x6c
[c34bdde0] [c013543c] kobject_put+0x24/0x34
[c34bddf0] [c015c384] put_device+0x1c/0x2c
[c34bde00] [c0180e84] mdiobus_unregister+0x2c/0x58
...
Though actually there is nothing broken, it just device
subsystem core expects another "pattern" of resource managment.
This patch implement phy device's release function, thus
we're getting rid of this badness.
Also small hidden bug fixed, hope none other introduced. ;-)
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Acked-by: Andy Fleming <afleming@freescale.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
This kind of sucks, and prevents the Fedora installer from using the
device for network installs...
[root@efika phy]# iwconfig eth0
Warning: Driver for device eth0 has been compiled with an ancient version
of Wireless Extension, while this program support version 11 and later.
Some things may be broken...
eth0 ESSID:off/any Nickname:""
NWID:0 Channel:0 Access Point: 00:00:BF:81:14:E0
Bit Rate:-1.08206e+06 kb/s Sensitivity=0/0
RTS thr:off Fragment thr:off
Encryption key:<too big>
Power Management:off
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
Previously, Internal Delay specification implied the delay be
applied to both TX and RX. This patch allows for separate TX/RX-only
internal delay specification.
Signed-off-by: Kim Phillips <kim.phillips@freescale.com>
Tested-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Acked-by: Li Yang <leoli@freescale.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
It gets quite verbose to see every single PHY driver being registered
by default.
Signed-off-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
Add PHY IDs for Marvell 88E1240. It seems to have close enough programming
models to 1111/1112 for basic support at least.
Also clean up whitespace in the ID list a bit.
Signed-off-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
Missing MODULE_LICENSE(), loading this module taints the kernel.
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
Previously, bitbanged MDIO was only supported in individual
hardware-specific drivers. This code factors out the higher level
protocol implementation, reducing the hardware-specific portion to
functions setting direction, data, and clock.
Signed-off-by: Scott Wood <scottwood@freescale.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
Ensure the PHY_HALTED state is not entered with the IRQ asserted as it
could lead to an interrupt loop.
There is a small window in phy_stop(), where the state of the PHY machine
indicates it has been halted, but its interrupt output might still be
unmasked. If an interrupt goes active right at this moment it will loop as
the phy_interrupt() handler exits immediately with IRQ_NONE if the halted
state is seen. It is unsafe to extend the phydev spinlock to cover
phy_interrupt(). It is safe to swap the order of the actions though as all
the competing places to unmask the interrupt output of the PHY, which are
phy_change() and phy_timer() are already covered with the lock as is the
sequence in question.
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
Cc: Andy Fleming <afleming@freescale.com>
Cc: Jeff Garzik <jgarzik@pobox.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
Keep track of disable_irq_nosync() invocations and call enable_irq() the
right number of times if work has been cancelled that would include them.
Now that the call to flush_work_keventd() (problematic because of
rtnl_mutex being held) has been replaced by cancel_work_sync() another
issue has arisen and been left unresolved. As the MDIO bus cannot be
accessed from the interrupt context the PHY interrupt handler uses
disable_irq_nosync() to prevent from looping and schedules some work to be
done as a softirq, which, apart from handling the state change of the
originating PHY, is responsible for reenabling the interrupt. Now if the
interrupt line is shared by another device and a call to the softirq
handler has been cancelled, that call to enable_irq() never happens and the
other device cannot use its interrupt anymore as its stuck disabled.
I decided to use a counter rather than a flag because there may be more
than one call to phy_change() cancelled in the queue -- a real one and a
fake one triggered by free_irq() if DEBUG_SHIRQ is used, if nothing else.
Therefore because of its nesting property enable_irq() has to be called the
right number of times to match the number disable_irq_nosync() was called
and restore the original state. This DEBUG_SHIRQ feature is also the
reason why free_irq() has to be called before cancel_work_sync().
While at it I updated the comment about phy_stop_interrupts() being called
from `keventd' -- this is no longer relevant as the use of
cancel_work_sync() makes such an approach unnecessary. OTOH a similar
comment referring to flush_scheduled_work() in phy_stop() still applies as
using cancel_work_sync() there would be dangerous.
Checked with checkpatch.pl and at the run time (with and without
DEBUG_SHIRQ).
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
Cc: Andy Fleming <afleming@freescale.com>
Cc: Jeff Garzik <jgarzik@pobox.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
Use spin_lock_bh()/spin_unlock_bh() for the phydev lock throughout as it
is used in phy_timer() that is called as a softirq and all the other
operations may happen in the user context.
There has been a change recently that did such a conversion for some of the
operations on the lock, but some have been left intact. Many of them,
perhaps all, may be called in the user context and I was able to trigger
recursive spinlock acquisition indeed, so I think for the sake of long-term
maintenance it is best to convert them all, even if unnecessarily for one
or two -- better safe than sorry.
Perhaps one in phy_timer() could actually be skipped as only called as a
softirq -- I can send an update if that sounds like a good idea.
Checked with checkpatch.pl and at the runtime.
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
device_bind_driver() error code returning has been fixed. release()
function has been written, so that to free resources in correct way; the
release path is now clean.
Before the rework, it used to cause
Device 'fixed@100:1' does not have a release() function, it is broken
and must be fixed.
BUG: at drivers/base/core.c:104 device_release()
Call Trace:
[<ffffffff802ec380>] kobject_cleanup+0x53/0x7e
[<ffffffff802ec3ab>] kobject_release+0x0/0x9
[<ffffffff802ecf3f>] kref_put+0x74/0x81
[<ffffffff8035493b>] fixed_mdio_register_device+0x230/0x265
[<ffffffff80564d31>] fixed_init+0x1f/0x35
[<ffffffff802071a4>] init+0x147/0x2fb
[<ffffffff80223b6e>] schedule_tail+0x36/0x92
[<ffffffff8020a678>] child_rip+0xa/0x12
[<ffffffff80311714>] acpi_ds_init_one_object+0x0/0x83
[<ffffffff8020705d>] init+0x0/0x2fb
[<ffffffff8020a66e>] child_rip+0x0/0x12
Also changed the notation of the fixed phy definition on
mdio bus to the form of <speed>+<duplex> to make it able to be used by
gianfar and ucc_geth that define phy_id strictly as "%d:%d" and cleaned up
the whitespace issues.
Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
Export phy_mii_ioctl, so network drivers can use it when built
as modules too.
Signed-off-by: Domen Puncer <domen@coderock.org>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
Lock debugging finds a problem in phy.c and phy_device.c,
this patch fixes it. Tested on an AT91SAM9263-EK board,
kernel 2.6.23-rc4.
Signed-off-by: Hans J. Koch <hjk@linutronix.de>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
Writing BMCR_RESET bit will reset MII_BMCR to default values. This is
clearly not what we want.
Signed-off-by: Domen Puncer <domen.puncer@telargo.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
phy_read() returns a negative number if there's an error, but the
error-checking code in the Vitesse driver's config_intr function
triggers if phy_read() returns non-zero. Correct that.
Signed-off-by: Andy Fleming <afleming@freescale.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
The Vitesse PHY on the 8641D needs to be set up with internal delay to
work in RGMII mode. So we add skew when it is set to RGMII_ID mode.
Signed-off-by: Andy Fleming <afleming@freescale.com>
Signed-off-by: Haruki Dai <Dai.Haruki@freescale.com>
Signed-off-by: Haiying Wang <Haiying.Wang@freescale.com>
The Vitesse 824x PHY doesn't allow an interrupt to be cleared if
the mask bit for that interrupt isn't set. This means that the PHY
Lib's order of handling interrupts (disable, then clear) breaks on this
PHY. However, clearing then disabling the interrupt opens up the code
for a silly race condition. So rather than change the PHY Lib, we change
the Vitesse driver so it always clears interrupts before disabling them.
Further, the ack function only clears the interrupt if interrupts are
enabled.
Signed-off-by: Andy Fleming <afleming@freescale.com>
Signed-off-by: York Sun <yorksun@freescale.com>
Acked-by: Haiying Wang <Haiying.Wang@freescale.com>
Add 88E1112 PHY ID to the marvell driver. Seems to do fine with the
88E1111 inits.
Signed-off-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
