From: Jeff Garzik <jeff@garzik.org>
To: Trent Piepho <tpiepho@freescale.com>
Cc: netdev@vger.kernel.org, Nate Case <ncase@xes-inc.com>,
linuxppc-dev@ozlabs.org
Subject: Re: [PATCH 1/2] gianfar: Fix race in TBI/SerDes configuration
Date: Fri, 31 Oct 2008 01:00:09 -0400 [thread overview]
Message-ID: <490A90D9.1080406@garzik.org> (raw)
In-Reply-To: <1225415827-8167-1-git-send-email-tpiepho@freescale.com>
Trent Piepho wrote:
> The init_phy() function attaches to the PHY, then configures the
> SerDes<->TBI link (in SGMII mode). The TBI is on the MDIO bus with the PHY
> (sort of) and is accessed via the gianfar's MDIO registers, using the
> functions gfar_local_mdio_read/write(), which don't do any locking.
>
> The previously attached PHY will start a work-queue on a timer, and
> probably an irq handler as well, which will talk to the PHY and thus use
> the MDIO bus. This uses phy_read/write(), which have locking, but not
> against the gfar_local_mdio versions.
>
> The result is that PHY code will try to use the MDIO bus at the same time
> as the SerDes setup code, corrupting the transfers.
>
> Setting up the SerDes before attaching to the PHY will insure that there is
> no race between the SerDes code and *our* PHY, but doesn't fix everything.
> Typically the PHYs for all gianfar devices are on the same MDIO bus, which
> is associated with the first gianfar device. This means that the first
> gianfar's SerDes code could corrupt the MDIO transfers for a different
> gianfar's PHY.
>
> The lock used by phy_read/write() is contained in the mii_bus structure,
> which is pointed to by the PHY. This is difficult to access from the
> gianfar drivers, as there is no link between a gianfar device and the
> mii_bus which shares the same MDIO registers. As far as the device layer
> and drivers are concerned they are two unrelated devices (which happen to
> share registers).
>
> Generally all gianfar devices' PHYs will be on the bus associated with the
> first gianfar. But this might not be the case, so simply locking the
> gianfar's PHY's mii bus might not lock the mii bus that the SerDes setup
> code is going to use.
>
> We solve this by having the code that creates the gianfar platform device
> look in the device tree for an mdio device that shares the gianfar's
> registers. If one is found the ID of its platform device is saved in the
> gianfar's platform data.
>
> A new function in the gianfar mii code, gfar_get_miibus(), can use the bus
> ID to search through the platform devices for a gianfar_mdio device with
> the right ID. The platform device's driver data is the mii_bus structure,
> which the SerDes setup code can use to lock the current bus.
>
> Signed-off-by: Trent Piepho <tpiepho@freescale.com>
> CC: Andy Fleming <afleming@freescale.com>
> ---
> arch/powerpc/sysdev/fsl_soc.c | 26 ++++++++++++++++++++++++++
> drivers/net/gianfar.c | 7 +++++++
> drivers/net/gianfar_mii.c | 21 +++++++++++++++++++++
> drivers/net/gianfar_mii.h | 3 +++
> include/linux/fsl_devices.h | 3 ++-
> 5 files changed, 59 insertions(+), 1 deletions(-)
applied 1-2
WARNING: multiple messages have this Message-ID (diff)
From: Jeff Garzik <jeff@garzik.org>
To: Trent Piepho <tpiepho@freescale.com>
Cc: netdev@vger.kernel.org, linuxppc-dev@ozlabs.org,
Nate Case <ncase@xes-inc.com>
Subject: Re: [PATCH 1/2] gianfar: Fix race in TBI/SerDes configuration
Date: Fri, 31 Oct 2008 01:00:09 -0400 [thread overview]
Message-ID: <490A90D9.1080406@garzik.org> (raw)
In-Reply-To: <1225415827-8167-1-git-send-email-tpiepho@freescale.com>
Trent Piepho wrote:
> The init_phy() function attaches to the PHY, then configures the
> SerDes<->TBI link (in SGMII mode). The TBI is on the MDIO bus with the PHY
> (sort of) and is accessed via the gianfar's MDIO registers, using the
> functions gfar_local_mdio_read/write(), which don't do any locking.
>
> The previously attached PHY will start a work-queue on a timer, and
> probably an irq handler as well, which will talk to the PHY and thus use
> the MDIO bus. This uses phy_read/write(), which have locking, but not
> against the gfar_local_mdio versions.
>
> The result is that PHY code will try to use the MDIO bus at the same time
> as the SerDes setup code, corrupting the transfers.
>
> Setting up the SerDes before attaching to the PHY will insure that there is
> no race between the SerDes code and *our* PHY, but doesn't fix everything.
> Typically the PHYs for all gianfar devices are on the same MDIO bus, which
> is associated with the first gianfar device. This means that the first
> gianfar's SerDes code could corrupt the MDIO transfers for a different
> gianfar's PHY.
>
> The lock used by phy_read/write() is contained in the mii_bus structure,
> which is pointed to by the PHY. This is difficult to access from the
> gianfar drivers, as there is no link between a gianfar device and the
> mii_bus which shares the same MDIO registers. As far as the device layer
> and drivers are concerned they are two unrelated devices (which happen to
> share registers).
>
> Generally all gianfar devices' PHYs will be on the bus associated with the
> first gianfar. But this might not be the case, so simply locking the
> gianfar's PHY's mii bus might not lock the mii bus that the SerDes setup
> code is going to use.
>
> We solve this by having the code that creates the gianfar platform device
> look in the device tree for an mdio device that shares the gianfar's
> registers. If one is found the ID of its platform device is saved in the
> gianfar's platform data.
>
> A new function in the gianfar mii code, gfar_get_miibus(), can use the bus
> ID to search through the platform devices for a gianfar_mdio device with
> the right ID. The platform device's driver data is the mii_bus structure,
> which the SerDes setup code can use to lock the current bus.
>
> Signed-off-by: Trent Piepho <tpiepho@freescale.com>
> CC: Andy Fleming <afleming@freescale.com>
> ---
> arch/powerpc/sysdev/fsl_soc.c | 26 ++++++++++++++++++++++++++
> drivers/net/gianfar.c | 7 +++++++
> drivers/net/gianfar_mii.c | 21 +++++++++++++++++++++
> drivers/net/gianfar_mii.h | 3 +++
> include/linux/fsl_devices.h | 3 ++-
> 5 files changed, 59 insertions(+), 1 deletions(-)
applied 1-2
next prev parent reply other threads:[~2008-10-31 5:00 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-10-28 22:53 [PATCH] gianfar: Omit TBI auto-negotiation based on device tree Nate Case
2008-10-31 1:07 ` Trent Piepho
2008-10-31 1:07 ` Trent Piepho
2008-10-31 1:17 ` [PATCH 1/2] gianfar: Fix race in TBI/SerDes configuration Trent Piepho
2008-10-31 5:00 ` Jeff Garzik [this message]
2008-10-31 5:00 ` Jeff Garzik
2008-10-31 1:17 ` [PATCH 2/2] gianfar: Don't reset TBI<->SerDes link if it's already up Trent Piepho
2008-11-03 18:55 ` [PATCH] gianfar: Omit TBI auto-negotiation based on device tree Nate Case
2008-11-03 18:55 ` Nate Case
2008-11-03 20:38 ` Kumar Gala
2008-11-03 20:38 ` Kumar Gala
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=490A90D9.1080406@garzik.org \
--to=jeff@garzik.org \
--cc=linuxppc-dev@ozlabs.org \
--cc=ncase@xes-inc.com \
--cc=netdev@vger.kernel.org \
--cc=tpiepho@freescale.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 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.