All of lore.kernel.org
 help / color / mirror / Atom feed
From: Scott Wood <scottwood@freescale.com>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Oded Gabbay <ogabbay@advaoptical.com>, <stef.van.os@prodrive.nl>,
	<timur@tabi.org>, <Dongsheng.Wang@freescale.com>,
	<paulus@samba.org>, <netdev@vger.kernel.org>,
	<linuxppc-dev@lists.ozlabs.org>, <davem@davemloft.net>,
	<linux-kernel@vger.kernel.org>, <B38951@freescale.com>
Subject: Re: [PATCH] MDIO: FSL_PQ_MDIO: Fix bug on incorrect offset of tbipa register
Date: Wed, 12 Jun 2013 13:31:11 -0500	[thread overview]
Message-ID: <1371061871.18413.44@snotra> (raw)
In-Reply-To: <51B88EED.6090709@linutronix.de> (from bigeasy@linutronix.de on Wed Jun 12 10:08:29 2013)

On 06/12/2013 10:08:29 AM, Sebastian Andrzej Siewior wrote:
> On 06/12/2013 02:47 PM, Oded Gabbay wrote:
> > This patch fixes a bug in the fsl_pq_mdio.c module and in relevant  
> device-tree
> > files regarding the correct offset of the tbipa register in the  
> eTSEC
> > controller in some of Freescale's PQ3 and QorIQ SoC.
> > The bug happens when the mdio in the device tree is configured to  
> be compatible
> > to "fsl,gianfar-tbi". Because the mdio device in the device tree  
> points to
> > addresses 25520, 26520 or 27520 (depends on the controller ID), the  
> variable
> > priv->map at function fsl_pq_mdio_probe, points to that address.  
> However,
> > later in the function there is a write to register tbipa that is  
> actually
> > located at 25030, 26030 or 27030. Because the correct address is  
> not io mapped,
> > the contents are written to a different register in the controller.
> > The fix sets the address of the mdio device to start at 25000,  
> 26000 or 27000
> > and changes the mii_offset field to 0x520 in the relevant entry
> > (fsl,gianfar-tbi) of the fsl_pq_mdio_match array.
> >
> > Note: This patch may break MDIO functionallity of some old  
> Freescale's SoC
> > until Freescale will fix their device tree files. Basically, every  
> device tree
> > which contains an mdio device that is compatible to  
> "fsl,gianfar-tbi" should be
> > examined.
> 
> Not as is.
> Please add a check for the original address. If it has 0x520 at the  
> end
> print a warning and fix it up. Please add to the patch description
> which register is modified instead if this patch is not applied.
> Depending on how critical this it might has to go stable.

I'm not sure it's stable material if this is something that has never  
worked...

The device tree binding will also need to be fixed to note the  
difference in "reg" between "fsl,gianfar-mdio" and "fsl-gianfar-tbi" --  
and should give an example of the latter.

-Scott

WARNING: multiple messages have this Message-ID (diff)
From: Scott Wood <scottwood@freescale.com>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: stef.van.os@prodrive.nl, Oded Gabbay <ogabbay@advaoptical.com>,
	netdev@vger.kernel.org, Dongsheng.Wang@freescale.com,
	timur@tabi.org, paulus@samba.org, linuxppc-dev@lists.ozlabs.org,
	davem@davemloft.net, linux-kernel@vger.kernel.org,
	B38951@freescale.com
Subject: Re: [PATCH] MDIO: FSL_PQ_MDIO: Fix bug on incorrect offset of tbipa register
Date: Wed, 12 Jun 2013 13:31:11 -0500	[thread overview]
Message-ID: <1371061871.18413.44@snotra> (raw)
In-Reply-To: <51B88EED.6090709@linutronix.de> (from bigeasy@linutronix.de on Wed Jun 12 10:08:29 2013)

On 06/12/2013 10:08:29 AM, Sebastian Andrzej Siewior wrote:
> On 06/12/2013 02:47 PM, Oded Gabbay wrote:
> > This patch fixes a bug in the fsl_pq_mdio.c module and in relevant =20
> device-tree
> > files regarding the correct offset of the tbipa register in the =20
> eTSEC
> > controller in some of Freescale's PQ3 and QorIQ SoC.
> > The bug happens when the mdio in the device tree is configured to =20
> be compatible
> > to "fsl,gianfar-tbi". Because the mdio device in the device tree =20
> points to
> > addresses 25520, 26520 or 27520 (depends on the controller ID), the =20
> variable
> > priv->map at function fsl_pq_mdio_probe, points to that address. =20
> However,
> > later in the function there is a write to register tbipa that is =20
> actually
> > located at 25030, 26030 or 27030. Because the correct address is =20
> not io mapped,
> > the contents are written to a different register in the controller.
> > The fix sets the address of the mdio device to start at 25000, =20
> 26000 or 27000
> > and changes the mii_offset field to 0x520 in the relevant entry
> > (fsl,gianfar-tbi) of the fsl_pq_mdio_match array.
> >
> > Note: This patch may break MDIO functionallity of some old =20
> Freescale's SoC
> > until Freescale will fix their device tree files. Basically, every =20
> device tree
> > which contains an mdio device that is compatible to =20
> "fsl,gianfar-tbi" should be
> > examined.
>=20
> Not as is.
> Please add a check for the original address. If it has 0x520 at the =20
> end
> print a warning and fix it up. Please add to the patch description
> which register is modified instead if this patch is not applied.
> Depending on how critical this it might has to go stable.

I'm not sure it's stable material if this is something that has never =20
worked...

The device tree binding will also need to be fixed to note the =20
difference in "reg" between "fsl,gianfar-mdio" and "fsl-gianfar-tbi" -- =20
and should give an example of the latter.

-Scott=

  reply	other threads:[~2013-06-12 18:31 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-12 12:47 [PATCH] MDIO: FSL_PQ_MDIO: Fix bug on incorrect offset of tbipa register Oded Gabbay
2013-06-12 12:47 ` Oded Gabbay
2013-06-12 13:04 ` Timur Tabi
2013-06-12 13:04   ` Timur Tabi
2013-06-12 13:09   ` Oded Gabbay
2013-06-12 13:09     ` Oded Gabbay
2013-06-12 15:08 ` Sebastian Andrzej Siewior
2013-06-12 15:08   ` Sebastian Andrzej Siewior
2013-06-12 18:31   ` Scott Wood [this message]
2013-06-12 18:31     ` Scott Wood
2013-06-13  6:44     ` Oded Gabbay
2013-06-13  6:44       ` Oded Gabbay
2013-06-15 22:57     ` Timur Tabi
2013-06-15 22:57       ` Timur Tabi
2015-10-09 15:15 ` [PATCH 1/2] net/fsl_pq_mdio: check TBI address for consistency with mapped range Gerlando Falauto
2015-10-09 15:15   ` [PATCH 2/2] net/fsl_pq_mdio: fix computed address for the TBI register Gerlando Falauto
2015-10-09 17:39   ` [PATCH 1/2] net/fsl_pq_mdio: check TBI address for consistency with mapped range kbuild test robot
2015-10-11 12:41   ` Timur Tabi
2015-10-12  7:18 ` [PATCH v2 " Gerlando Falauto
2015-10-12  7:18   ` Gerlando Falauto
2015-10-12  7:18   ` [PATCH v2 2/2] net/fsl_pq_mdio: fix computed address for the TBI register Gerlando Falauto
2015-10-12  7:18     ` Gerlando Falauto
2015-10-13 11:30     ` David Miller
2015-10-13 11:30   ` [PATCH v2 1/2] net/fsl_pq_mdio: check TBI address for consistency with mapped range David Miller

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=1371061871.18413.44@snotra \
    --to=scottwood@freescale.com \
    --cc=B38951@freescale.com \
    --cc=Dongsheng.Wang@freescale.com \
    --cc=bigeasy@linutronix.de \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=netdev@vger.kernel.org \
    --cc=ogabbay@advaoptical.com \
    --cc=paulus@samba.org \
    --cc=stef.van.os@prodrive.nl \
    --cc=timur@tabi.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.