diff mbox

[03/19] Ensure that the old style MDIO interface is active

Message ID 1035223108.1714.1474325416863.JavaMail.zimbra@raptorengineeringinc.com
State Rejected, archived
Headers show

Commit Message

Timothy Pearson Sept. 19, 2016, 10:50 p.m. UTC
before registering the MDIO bus on Faraday style
 MACs.  Without this patch the hardware may continue
 to expect new style commands, yielding MDIO timeouts
 and general lack of communication with the MII.

Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
---
 drivers/net/ethernet/faraday/ftgmac100.c |    6 ++++++
 drivers/net/ethernet/faraday/ftgmac100.h |    2 ++
 2 files changed, 8 insertions(+)

Comments

Andrew Jeffery Sept. 20, 2016, 12:29 a.m. UTC | #1
On Mon, 2016-09-19 at 17:50 -0500, Timothy Pearson wrote:
>  before registering the MDIO bus on Faraday style
>  MACs.  Without this patch the hardware may continue
>  to expect new style commands, yielding MDIO timeouts
>  and general lack of communication with the MII.
> 
> Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>

I've had a look at this patch from your git repo on github. Whilst git-
send-email is changing the patch in ways you're not expecting, it's
because your commit message isn't formatted in the manner expected by
git's tools. Your object looks like:

    $ git cat-file -p 322ca61d9a6adbbc70a89d64a07469e384ca3931
    tree bbbf2ad42043eee22d7daa755c42ce372356053a
    parent f06ba7f29a26de27a2feb1dd8ec26022c4fbd77e
    author Timothy Pearson <    tpearson@raptorengineering.com    > 1473775455 -0500
    committer Timothy Pearson <    tpearson@raptorengineering.com    > 1473775938 -0500

    Ensure that the old style MDIO interface is active before registering
    the MDIO bus on Faraday style MACs.  Without this patch the hardware
    may continue to expect new style commands, yielding MDIO timeouts
    and general lack of communication with the MII.

    Signed-off-by: Timothy Pearson <    tpearson@raptorengineering.com>

From `git help commit`:

    DISCUSSION
           Though not required, it’s a good idea to begin the commit message with a single short (less than 50 character) line summarizing the change, followed by a blank line
           and then a more thorough description. The text up to the first blank line in a commit message is treated as the commit title, and that title is used throughout Git.
           For example, git-format-patch(1) turns a commit into email, and it uses the title on the Subject line and the rest of the commit in the body.

As you don't have the blank line separator I expect one of git-format-
patch or git-send-email is treating your whole commit message as the
subject.

Separating your commit message into short and long descriptions as
detailed above should resolve your problem. For example, it might look
something like:

    ftgmac100: Enable old-style MDIO interface

    Ensure that the old style MDIO interface is active before registering
    the MDIO bus on Faraday style MACs.  Without this patch the hardware
    may continue to expect new style commands, yielding MDIO timeouts
    and general lack of communication with the MII.

Separately, I had some questions on the v1 of the patch. Can you
pleaserespond to those?

Hope that helps,

Andrew

> ---
>  drivers/net/ethernet/faraday/ftgmac100.c |    6 ++++++
>  drivers/net/ethernet/faraday/ftgmac100.h |    2 ++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index c20f767..778c625 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1250,6 +1250,7 @@ static int ftgmac100_setup_mdio(struct net_device *netdev)
>  {
>  	struct ftgmac100 *priv = netdev_priv(netdev);
>  	struct platform_device *pdev = to_platform_device(priv->dev);
> +	uint32_t revcr;
>  	int i, err = 0;
>  
>  	/* initialize mdio bus */
> @@ -1257,6 +1258,11 @@ static int ftgmac100_setup_mdio(struct net_device *netdev)
>  	if (!priv->mii_bus)
>  		return -EIO;
>  
> +	/* This driver only supports the old MDIO interface -- enable it */
> +	revcr = ioread32(priv->base + FTGMAC100_OFFSET_REVR);
> +	revcr &= ~FTGMAC100_OFFSET_REVR_NEW_INTERFACE;
> +	iowrite32(revcr, priv->base + FTGMAC100_OFFSET_REVR);
> +
>  	priv->mii_bus->name = "ftgmac100_mdio";
>  	snprintf(priv->mii_bus->id, MII_BUS_ID_SIZE, "%s-%d",
>  		 pdev->name, pdev->id);
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.h b/drivers/net/ethernet/faraday/ftgmac100.h
> index d07b6ea..ee5f758 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.h
> +++ b/drivers/net/ethernet/faraday/ftgmac100.h
> @@ -133,6 +133,8 @@
>  #define FTGMAC100_DMAFIFOS_RXDMA_REQ		(1 << 30)
>  #define FTGMAC100_DMAFIFOS_TXDMA_REQ		(1 << 31)
>  
> +#define FTGMAC100_OFFSET_REVR_NEW_INTERFACE	(1 << 31)
> +
>  /*
>   * Receive buffer size register
>   */
Timothy Pearson Sept. 20, 2016, 1:47 a.m. UTC | #2
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 09/19/2016 07:29 PM, Andrew Jeffery wrote:
> I've had a look at this patch from your git repo on github. Whilst git-
> send-email is changing the patch in ways you're not expecting, it's
> because your commit message isn't formatted in the manner expected by
> git's tools. Your object looks like:
> 
>     ftgmac100: Enable old-style MDIO interface
> 

Good catch.  Today was fairly hectic on this end and I had a grand total
of a couple of minutes to look into the problem. :-)

> Separately, I had some questions on the v1 of the patch. Can you
> pleaserespond to those?

Sure!  From a high level, this driver does not support anything other
than the old-style MDIO interface.  The hardware sets the new-style bit
when support is available, so the driver itself should be able to
determine which style of interface to use at runtime (in theory).

> Hope that helps,
> 
> Andrew



- -- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645 (direct line)
+1 (512) 690-0200 (switchboard)
https://www.raptorengineering.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJX4JUzAAoJEK+E3vEXDOFbmKoH/Ay2cGaQ4U9VlayODUiqUrcY
RAyG6iC1iHFxooPKKWqZiNMPcbey+Of1Eq+t2jKn0P5RDRo/8nwXiakJrkr+NmJc
j+nM96i9eTbg4bA3cdk7aGaVd7jjmAeoqK3Fz64ansYYD55rlynfrCZli1zYZgkC
3rf22JP6A6H/ibshLHjjawZF5mU9p+ngv0iYMyryK5sBzXWGXrZAfc7xmBR9fXpA
yZbhFzwL4CnbLSclWvx/wMoTMejrDe3V4n/7XGrNZFspt6jt1WkjrmMIDQcJUPnq
PYqZmBowsi9EsAMufUKhZ4WQcHfJKMMFG90m3PXdkJyD85qX3c5PvJ77aZYa4sQ=
=vN3E
-----END PGP SIGNATURE-----
Andrew Jeffery Sept. 20, 2016, 2:14 a.m. UTC | #3
On Mon, 2016-09-19 at 20:47 -0500, Timothy Pearson wrote:
> > Separately, I had some questions on the v1 of the patch. Can you
> > pleaserespond to those?
> 
> Sure!  From a high level, this driver does not support anything other
> than the old-style MDIO interface.  The hardware sets the new-style
> bit
> when support is available, so the driver itself should be able to
> determine which style of interface to use at runtime (in theory).

Yes, so if we continue down this road we can remove the hack in the
board file for the ast2500 evb. I guess that can be a separate patch
dependent on this series. Then if/when we add support to the driver for
the new MDIO interface we can add a new compatible string.

Andrew
Timothy Pearson Sept. 20, 2016, 2:16 a.m. UTC | #4
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 09/19/2016 09:14 PM, Andrew Jeffery wrote:
> Yes, so if we continue down this road we can remove the hack in the
> board file for the ast2500 evb. I guess that can be a separate patch
> dependent on this series. Then if/when we add support to the driver for
> the new MDIO interface we can add a new compatible string.
> 
> Andrew

Sounds reasonable to me.

- -- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645 (direct line)
+1 (512) 690-0200 (switchboard)
https://www.raptorengineering.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJX4JwSAAoJEK+E3vEXDOFbICsIALB6VDRdAdPfUp78x9eNhTGY
n/iyDINqMfWeJjuqTXqwydHbOZ6LTa5RxoozZ4vV4KyZCpLXllkULacDN14uQ+a3
mC3+F8DMJ+4fkveK9lRu/BbHF15itwfJouOevLWwd7bGIphL3Un2nZHvZXd2uU1u
n0gn3UaO0dgc4T7EZOGYUXN4ghb9HG3UMol2u2ywLv4aFBafWggrffNKLANxiRDD
E4ZJfENvjXuDuuTEiuMsZ0KcHTm5cASi7SnOmPKsnrRQA+jUZ893Czzmls0vAqTl
92bl5r/mOhs/ogrMA+shPbh2HCI+YvNknuEPfsq2odiQ/UeQ5su2T6oLF/yG2Os=
=3wac
-----END PGP SIGNATURE-----
diff mbox

Patch

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index c20f767..778c625 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1250,6 +1250,7 @@  static int ftgmac100_setup_mdio(struct net_device *netdev)
 {
 	struct ftgmac100 *priv = netdev_priv(netdev);
 	struct platform_device *pdev = to_platform_device(priv->dev);
+	uint32_t revcr;
 	int i, err = 0;
 
 	/* initialize mdio bus */
@@ -1257,6 +1258,11 @@  static int ftgmac100_setup_mdio(struct net_device *netdev)
 	if (!priv->mii_bus)
 		return -EIO;
 
+	/* This driver only supports the old MDIO interface -- enable it */
+	revcr = ioread32(priv->base + FTGMAC100_OFFSET_REVR);
+	revcr &= ~FTGMAC100_OFFSET_REVR_NEW_INTERFACE;
+	iowrite32(revcr, priv->base + FTGMAC100_OFFSET_REVR);
+
 	priv->mii_bus->name = "ftgmac100_mdio";
 	snprintf(priv->mii_bus->id, MII_BUS_ID_SIZE, "%s-%d",
 		 pdev->name, pdev->id);
diff --git a/drivers/net/ethernet/faraday/ftgmac100.h b/drivers/net/ethernet/faraday/ftgmac100.h
index d07b6ea..ee5f758 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.h
+++ b/drivers/net/ethernet/faraday/ftgmac100.h
@@ -133,6 +133,8 @@ 
 #define FTGMAC100_DMAFIFOS_RXDMA_REQ		(1 << 30)
 #define FTGMAC100_DMAFIFOS_TXDMA_REQ		(1 << 31)
 
+#define FTGMAC100_OFFSET_REVR_NEW_INTERFACE	(1 << 31)
+
 /*
  * Receive buffer size register
  */