diff mbox

[U-Boot] building enc28j60 for omap3: SILICON ERRATA

Message ID 20110223233201.09aee7e4@wker
State Not Applicable
Delegated to: Sandeep Paulraj
Headers show

Commit Message

Anatolij Gustschin Feb. 23, 2011, 10:32 p.m. UTC
Hi,

On Tue, 22 Feb 2011 17:34:30 +0100
jacopo mondi <j.mondi@voltaelectronics.com> wrote:
...
> Second issue is related to enc identification.
> The following code section:
> 
> 	phid1 = phy_read(enc, PHY_REG_PHID1);
> 	phid2 = phy_read(enc, PHY_REG_PHID2) & ENC_PHID2_MASK;
> 	if (phid1 != ENC_PHID1_VALUE || phid2 != ENC_PHID2_VALUE) {
> 		printf("%s: failed to identify PHY. Found
> 		%04x:%04x\n",			enc->dev->name,
> 		phid1, phid2);
> 	 return -1;
> 	}
> 
> fails because phy_read instructions return 0 or random values (0xB0B0
> or 0xB000).
> Linux driver does not perform such tests, so I've tried removing them.

No, please do not remove them. Fix the register access
problem instead.

> Anyway all read and write to enc fails.
> Could that be related to omap3_spi implementation?

Yes. If you use the omap3_spi driver in current mainline tree,
then definitely the omap3_spi driver is the problem. enc28j60
register and buffer access can not work with this current driver
version.

We experienced problems with enc28j60 register access on a iMX31
based board as there were among other things some byte order issues
in the spi driver. When using spi clock above 2 MHz we also have
seen seemingly random data when reading the enc registers. Be aware
that there are two kinds of spi transfers for enc register access:
two bytes long transfers and three bytes long transfers with a dummy
byte. The spi chip select signal have to be active as long as
these tx/rx transfers didn't completed and may not be deactivated
in between. Also the spi transfer is full-duplex for enc28j60
register and buffers access. With a properly working spi driver
the enc28j60 driver in U-Boot works well.
 
> I can confirm that same same board I'm using for test works great
> under Linux, so it is not an hardware issue.

This is the omap3_spi driver issue. You can apply the attached
patch for enc28j60 driver to be able to start single register
accesses on the U-Boot command line. Then you can start fixing
the spi driver code. Implement fixes in the spi driver and test
the register access using the enc commands added by the path.

You will have to implement the Master Transmit-Receive Mode
(full-duplex) in the omap3_spi driver for proper enc28j60 register
and buffer access. Currently this driver is using Tx-Only Mode
for transmitting a Rx-Only Mode for receiving. But it shouldn't
be too hard to fix the spi driver. What is needed is a kind
of merge of omap3_spi_read/omap3_spi_write routines.

HTH
Anatolij

Comments

Wolfgang Denk Feb. 23, 2011, 11:32 p.m. UTC | #1
Dear Anatolij Gustschin,

In message <20110223233201.09aee7e4@wker> you wrote:
>
> No, please do not remove them. Fix the register access
> problem instead.
> 
> > Anyway all read and write to enc fails.
> > Could that be related to omap3_spi implementation?
> 
> Yes. If you use the omap3_spi driver in current mainline tree,
> then definitely the omap3_spi driver is the problem. enc28j60
> register and buffer access can not work with this current driver
> version.

There might also be other issues.

The omap3_spi driver was one of those affected by the bug fixed in
commit 495df3b "ARM: fix write*() I/O accessors", see
http://patchwork.ozlabs.org/patch/82841/

This is fixed in mainline now, so it might be wise to update the code
base and just try again.


Best regards,

Wolfgang Denk
jacopo mondi Feb. 25, 2011, 8:37 a.m. UTC | #2
On Wed, 23 Feb 2011 23:32:01 +0100
Anatolij Gustschin <agust@denx.de> wrote:

> You will have to implement the Master Transmit-Receive Mode
> (full-duplex) in the omap3_spi driver for proper enc28j60 register
> and buffer access. Currently this driver is using Tx-Only Mode
> for transmitting a Rx-Only Mode for receiving. But it shouldn't
> be too hard to fix the spi driver. What is needed is a kind
> of merge of omap3_spi_read/omap3_spi_write routines.
> 
> HTH
> Anatolij
Thank you Anatolij, I'm reading OMAP datasheet in these days...
Hope I'll provide something during next week.
jacopo mondi March 2, 2011, 2:08 p.m. UTC | #3
On Wed, 23 Feb 2011 23:32:01 +0100
Anatolij Gustschin <agust@denx.de> wrote:

> Hi,
> 
> On Tue, 22 Feb 2011 17:34:30 +0100
> jacopo mondi <j.mondi@voltaelectronics.com> wrote:
> ...
> > Second issue is related to enc identification.
> > The following code section:
> > 
> > 	phid1 = phy_read(enc, PHY_REG_PHID1);
> > 	phid2 = phy_read(enc, PHY_REG_PHID2) & ENC_PHID2_MASK;
> > 	if (phid1 != ENC_PHID1_VALUE || phid2 != ENC_PHID2_VALUE) {
> > 		printf("%s: failed to identify PHY. Found
> > 		%04x:%04x\n",			enc->dev->name,
> > 		phid1, phid2);
> > 	 return -1;
> > 	}
> > 
> > fails because phy_read instructions return 0 or random values
> > (0xB0B0 or 0xB000).
> > Linux driver does not perform such tests, so I've tried removing
> > them.
> 
> No, please do not remove them. Fix the register access
> problem instead.
> 

Anatolj,
  how could the single register access work if all the spi setup and
startup is done in the spi_claim_bus function? (which is a bad design
in my opinion, since if I claim bus, it does not mean I want to start
the communication).

Anyway I've implemented the Transmit Receive mode, and verified it
works issuing a ping, that initialize enc and spi bus, and now I can
successfully read phy identifiers and the CLKRDY bit (which means that
read and write are now correct).

I'm still unable to ping because "`ethaddr' not set", maybe because
actually the transmit and receive mode is used only if txp and rxp are
not NULL (tx only or rx only methods are used either).
I can change that, but that means changing the omap_spi interface.

Also, the whole omap3_spi implementation has to be reworked, because
several things need to be added (turbo mode for 3 bytes transfers) and
some initialization work has to be moved from the claim_bus function to
the spi_xfer one...

I'll submit the receive-transmit mode patch only for now...
thank you
   j
diff mbox

Patch

diff --git a/drivers/net/enc28j60.c b/drivers/net/enc28j60.c
index 5731bdb..784d1dd 100644
--- a/drivers/net/enc28j60.c
+++ b/drivers/net/enc28j60.c
@@ -21,6 +21,7 @@ 
  */
 
 #include <common.h>
+#include <command.h>
 #include <net.h>
 #include <spi.h>
 #include <malloc.h>
@@ -926,6 +927,7 @@  static void enc_halt(struct eth_device *dev)
 	enc_release_bus(enc);
 }
 
+enc_dev_t *genc;
 /*
  * This is the only exported function.
  *
@@ -974,5 +976,65 @@  int enc28j60_initialize(unsigned int bus, unsigned int cs,
 #if defined(CONFIG_CMD_MII)
 	miiphy_register(dev->name, enc_miiphy_read, enc_miiphy_write);
 #endif
+	genc = enc;
 	return 0;
 }
+
+
+int do_enc(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
+{
+	char *cmd;
+	u32 reg;
+	u32 val;
+
+	/* at least two arguments */
+	if (argc < 3)
+		return cmd_usage(cmdtp);
+
+	cmd = argv[1];
+	if (strcmp(cmd, "r") == 0) {
+		if (argc < 3)
+			return cmd_usage(cmdtp);
+
+		reg = simple_strtoul(argv[2], NULL, 16);
+		printf("reg. 0x%02lx: 0x%02lx\n", (ulong)reg, (ulong)enc_r8(genc, (u16)reg));
+		return 0;
+	}
+	if (strcmp(cmd, "pr") == 0) {
+		if (argc < 3)
+			return cmd_usage(cmdtp);
+
+		reg = simple_strtoul(argv[2], NULL, 16);
+		printf("phy reg. 0x%02lx: 0x%04lx\n", (ulong)reg, (ulong)phy_read(genc, (u8)reg));
+		return 0;
+	}
+	if (strcmp(cmd, "w") == 0) {
+		if (argc < 4)
+			return cmd_usage(cmdtp);
+
+		reg = simple_strtoul(argv[2], NULL, 16);
+		val = simple_strtoul(argv[3], NULL, 16);
+		enc_w8(genc, (u16)reg, (u8)val);
+		return 0;
+	}
+	if (strcmp(cmd, "pw") == 0) {
+		if (argc < 4)
+			return cmd_usage(cmdtp);
+
+		reg = simple_strtoul(argv[2], NULL, 16);
+		val = simple_strtoul(argv[3], NULL, 16);
+		phy_write(genc, (u8)reg, (u16)val);
+		return 0;
+	}
+	/* No subcommand */
+	return 1;
+}
+
+U_BOOT_CMD(
+	enc,	4, 1, do_enc,
+	"enc28j60 register read/write",
+	"r <reg> - read register\n"
+	"enc w <reg> <value> - write register\n"
+	"enc pr <reg> - read PHY register\n"
+	"enc pw <reg> <value> - write PHY register"
+);