On Fri, May 31, 2019 at 1:24 PM Adam Ford aford173@gmail.com wrote:

On Fri, May 31, 2019 at 8:32 AM Bartosz Golaszewski brgl@bgdev.pl wrote:

...

From: Bartosz Golaszewski bgolaszewski@baylibre.com

Now that we removed all legacy boards selecting TI_EMAC we can completely convert the driver code to using the driver model. This patch also updates all remaining users of davinci_emac.

Nice job on the da850, it works perfectly. Unfortunately, it crashes the am3517_evm

Without your patch, I am able to get an IP address, but when I apply your patch and attempt an IP address, it crashes. I tried again, but I got the same result:

U-Boot 2019.07-rc3-05342-gebd2f49298 (May 31 2019 - 13:18:29 -0500)

AM35XX-GP ES2.0, CPU-OPP2, L3-165MHz, Max CPU Clock 600 MHz Model: TI AM3517 EVM (AM3517/05 TMDSEVM3517) AM3517EVM Board + LPDDR/NAND DRAM: 256 MiB NAND: 512 MiB MMC: OMAP SD/MMC: 0, OMAP SD/MMC: 1 Loading Environment from NAND... OK In: serial@49020000 Out: serial@49020000 Err: serial@49020000 OMAP die ID: 3a5c000100000000016096fa04011001 Net: eth0: ethernet@5c000000 Hit any key to stop autoboot: 0 AM3517_EVM # dhcp BOOTP broadcast 1 data abort pc : [<8ffac38e>] lr : [<8ffabe3b>] reloc pc : [<8015738e>] lr : [<80156e3b>] sp : 8df255b0 ip : 000000ff fp : 8ffeea70 r10: 8ffecbc8 r9 : 8df34ec8 r8 : 00000000 r7 : 0000ffff r6 : 00000fff r5 : 00000d88 r4 : 0000ffff r3 : 0000ffff r2 : 00000156 r1 : 00000fff r0 : 00000fff Flags: nZCv IRQs off FIQs off Mode SVC_32 Code: 46304638 ea410a21 b2892104 bf084299 (7b2c4631) Resetting CPU ...

resetting ...

adam

I got it to work 'once' so I think we might have a timing issue, but I am not sure. I'm trying to enable various debug messages. I was hoping it would be a simple fix, but it's not turning out that way.

adam

...

Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com

arch/arm/mach-davinci/cpu.c | 13 ----- arch/arm/mach-omap2/omap3/emac.c | 3 +- board/davinci/da8xxevm/da850evm.c | 6 -- board/davinci/da8xxevm/omapl138_lcdk.c | 14 ----- board/logicpd/am3517evm/am3517evm.c | 1 - board/ti/ti816x/evm.c | 3 +- configs/am3517_evm_defconfig | 1 + configs/da850evm_defconfig | 1 + configs/da850evm_direct_nor_defconfig | 1 + configs/da850evm_nand_defconfig | 1 + configs/omapl138_lcdk_defconfig | 1 + configs/ti816x_evm_defconfig | 1 + drivers/net/ti/davinci_emac.c | 77 ++++++++++++++------------ include/netdev.h | 1 - 14 files changed, 51 insertions(+), 73 deletions(-)

diff --git a/arch/arm/mach-davinci/cpu.c b/arch/arm/mach-davinci/cpu.c index f97ad3fc74..9fd6564d04 100644 --- a/arch/arm/mach-davinci/cpu.c +++ b/arch/arm/mach-davinci/cpu.c @@ -5,7 +5,6 @@ */

#include <common.h> -#include <netdev.h> #include <asm/arch/hardware.h> #include <asm/io.h>

@@ -90,15 +89,3 @@ int set_cpu_clk_info(void) gd->bd->bi_dsp_freq = 0; return 0; }

-/*

• • Initializes on-chip ethernet controllers.
• • to override, implement board_eth_init()
• */

-int cpu_eth_init(bd_t *bis) -{ -#if defined(CONFIG_DRIVER_TI_EMAC)

•   davinci_emac_initialize();
  

-#endif

•   return 0;
  

-} diff --git a/arch/arm/mach-omap2/omap3/emac.c b/arch/arm/mach-omap2/omap3/emac.c index c79e870183..fb0c9188f5 100644 --- a/arch/arm/mach-omap2/omap3/emac.c +++ b/arch/arm/mach-omap2/omap3/emac.c @@ -7,7 +7,6 @@ */

#include <common.h> -#include <netdev.h> #include <asm/io.h> #include <asm/arch/am35x_def.h>

@@ -24,5 +23,5 @@ int cpu_eth_init(bd_t *bis) reset &= ~CPGMACSS_SW_RST; writel(reset, &am35x_scm_general_regs->ip_sw_reset);

•   return davinci_emac_initialize();
  

•   return 0;
  

} diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c index 1bc26828bf..d090c00466 100644 --- a/board/davinci/da8xxevm/da850evm.c +++ b/board/davinci/da8xxevm/da850evm.c @@ -13,7 +13,6 @@ #include <environment.h> #include <i2c.h> #include <net.h> -#include <netdev.h> #include <spi.h> #include <spi_flash.h> #include <asm/arch/hardware.h> @@ -472,11 +471,6 @@ int board_eth_init(bd_t *bis) if (rmii_hw_init()) printf("RMII hardware init failed!!!\n"); #endif

•   if (!davinci_emac_initialize()) {
  

•           printf("Error: Ethernet init failed!\n");
  

•           return -1;
  

•   }
  

•   return 0;
  

} #endif /* CONFIG_DRIVER_TI_EMAC */ diff --git a/board/davinci/da8xxevm/omapl138_lcdk.c b/board/davinci/da8xxevm/omapl138_lcdk.c index 2c2f885d43..ef9656add8 100644 --- a/board/davinci/da8xxevm/omapl138_lcdk.c +++ b/board/davinci/da8xxevm/omapl138_lcdk.c @@ -11,7 +11,6 @@ #include <common.h> #include <i2c.h> #include <net.h> -#include <netdev.h> #include <spi.h> #include <spi_flash.h> #include <asm/arch/hardware.h> @@ -229,19 +228,6 @@ int board_init(void)

#ifdef CONFIG_DRIVER_TI_EMAC

-/*

• • Initializes on-board ethernet controllers.
• */

-int board_eth_init(bd_t *bis) -{

•   if (!davinci_emac_initialize()) {
  

•           printf("Error: Ethernet init failed!\n");
  

•           return -1;
  

•   }
  

•   return 0;
  

-}

#endif /* CONFIG_DRIVER_TI_EMAC */

#define CFG_MAC_ADDR_SPI_BUS 0 diff --git a/board/logicpd/am3517evm/am3517evm.c b/board/logicpd/am3517evm/am3517evm.c index 10031a4801..bfd4e78274 100644 --- a/board/logicpd/am3517evm/am3517evm.c +++ b/board/logicpd/am3517evm/am3517evm.c @@ -28,7 +28,6 @@ #include <linux/usb/gadget.h> #include <linux/usb/musb.h> #include <i2c.h> -#include <netdev.h> #include "am3517evm.h"

DECLARE_GLOBAL_DATA_PTR; diff --git a/board/ti/ti816x/evm.c b/board/ti/ti816x/evm.c index 07a084bab8..240df8cbe1 100644 --- a/board/ti/ti816x/evm.c +++ b/board/ti/ti816x/evm.c @@ -9,7 +9,6 @@ #include <common.h> #include <environment.h> #include <spl.h> -#include <netdev.h> #include <asm/cache.h> #include <asm/io.h> #include <asm/arch/clock.h> @@ -56,7 +55,7 @@ int board_eth_init(bd_t *bis) printf("Unable to read MAC address. Set <ethaddr>\n"); }

•   return davinci_emac_initialize();
  

•   return 0;
  

}

#ifdef CONFIG_SPL_BUILD diff --git a/configs/am3517_evm_defconfig b/configs/am3517_evm_defconfig index b9f59f3291..5cb76322df 100644 --- a/configs/am3517_evm_defconfig +++ b/configs/am3517_evm_defconfig @@ -44,6 +44,7 @@ CONFIG_SYS_NAND_BUSWIDTH_16BIT=y CONFIG_SYS_NAND_U_BOOT_LOCATIONS=y CONFIG_SYS_NAND_U_BOOT_OFFS=0x80000 CONFIG_SPL_NAND_SIMPLE=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_PINCTRL=y diff --git a/configs/da850evm_defconfig b/configs/da850evm_defconfig index 8c16d5c4f5..ca304ed78a 100644 --- a/configs/da850evm_defconfig +++ b/configs/da850evm_defconfig @@ -60,6 +60,7 @@ CONFIG_SF_DEFAULT_SPEED=30000000 CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y CONFIG_SPI_FLASH_MTD=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_PINCTRL=y diff --git a/configs/da850evm_direct_nor_defconfig b/configs/da850evm_direct_nor_defconfig index 166e77b8e3..9b1da07384 100644 --- a/configs/da850evm_direct_nor_defconfig +++ b/configs/da850evm_direct_nor_defconfig @@ -50,6 +50,7 @@ CONFIG_DM_SPI_FLASH=y CONFIG_SPI_FLASH=y CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_PINCTRL=y diff --git a/configs/da850evm_nand_defconfig b/configs/da850evm_nand_defconfig index b8eac0e659..eaf5d73dd8 100644 --- a/configs/da850evm_nand_defconfig +++ b/configs/da850evm_nand_defconfig @@ -60,6 +60,7 @@ CONFIG_SPI_FLASH=y CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y CONFIG_SPI_FLASH_MTD=y +CONFIG_DM_ETH=y CONFIG_PINCTRL=y CONFIG_PINCTRL_SINGLE=y CONFIG_DM_SERIAL=y diff --git a/configs/omapl138_lcdk_defconfig b/configs/omapl138_lcdk_defconfig index e43141844a..f93a06083e 100644 --- a/configs/omapl138_lcdk_defconfig +++ b/configs/omapl138_lcdk_defconfig @@ -50,6 +50,7 @@ CONFIG_SPI_FLASH=y CONFIG_SF_DEFAULT_SPEED=30000000 CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_DM_SERIAL=y diff --git a/configs/ti816x_evm_defconfig b/configs/ti816x_evm_defconfig index bf877f596b..b233ab8cc9 100644 --- a/configs/ti816x_evm_defconfig +++ b/configs/ti816x_evm_defconfig @@ -45,6 +45,7 @@ CONFIG_SYS_I2C_OMAP24XX=y CONFIG_MMC_OMAP_HS=y CONFIG_NAND=y CONFIG_SYS_NAND_BUSWIDTH_16BIT=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_SYS_NS16550=y diff --git a/drivers/net/ti/davinci_emac.c b/drivers/net/ti/davinci_emac.c index 9d53984973..4b0a98a4aa 100644 --- a/drivers/net/ti/davinci_emac.c +++ b/drivers/net/ti/davinci_emac.c @@ -26,7 +26,6 @@ #include <net.h> #include <miiphy.h> #include <malloc.h> -#include <netdev.h> #include <linux/compiler.h> #include <asm/arch/emac_defs.h> #include <asm/io.h> @@ -107,8 +106,9 @@ static u_int8_t num_phy;

phy_t phy[CONFIG_SYS_DAVINCI_EMAC_PHY_COUNT];

-static int davinci_eth_set_mac_addr(struct eth_device *dev) +static int davinci_emac_write_hwaddr(struct udevice *dev) {

•   struct eth_pdata *pdata = dev_get_platdata(dev);
    unsigned long           mac_hi;
    unsigned long           mac_lo;
  
  

@@ -118,12 +118,12 @@ static int davinci_eth_set_mac_addr(struct eth_device *dev) * Using channel 0 only - other channels are disabled * */ writel(0, &adap_emac->MACINDEX);

•   mac_hi = (dev->enetaddr[3] << 24) |
  

•            (dev->enetaddr[2] << 16) |
  

•            (dev->enetaddr[1] << 8)  |
  

•            (dev->enetaddr[0]);
  

•   mac_lo = (dev->enetaddr[5] << 8) |
  

•            (dev->enetaddr[4]);
  

•   mac_hi = (pdata->enetaddr[3] << 24) |
  

•            (pdata->enetaddr[2] << 16) |
  

•            (pdata->enetaddr[1] << 8)  |
  

•            (pdata->enetaddr[0]);
  

•   mac_lo = (pdata->enetaddr[5] << 8) |
  

•            (pdata->enetaddr[4]);
  
    writel(mac_hi, &adap_emac->MACADDRHI);
  

#if defined(DAVINCI_EMAC_VERSION2) @@ -411,7 +411,7 @@ static void __attribute__((unused)) davinci_eth_gigabit_enable(int phy_addr) }

/* Eth device open */ -static int davinci_eth_open(struct eth_device *dev, bd_t *bis) +static int davinci_emac_start(struct udevice *dev) { dv_reg_p addr; u_int32_t clkdiv, cnt, mac_control; @@ -447,7 +447,7 @@ static int davinci_eth_open(struct eth_device *dev, bd_t *bis) writel(1, &adap_emac->TXCONTROL); writel(1, &adap_emac->RXCONTROL);

•   davinci_eth_set_mac_addr(dev);
  

•   davinci_emac_write_hwaddr(dev);
  
    /* Set DMA 8 TX / 8 RX Head pointers to 0 */
    addr = &adap_emac->TX0HDP;
  

@@ -588,7 +588,7 @@ static void davinci_eth_ch_teardown(int ch) }

/* Eth device close */ -static void davinci_eth_close(struct eth_device *dev) +static void davinci_emac_stop(struct udevice *dev) { debug_emac("+ emac_close\n");

@@ -619,8 +619,8 @@ static int tx_send_loop = 0;

• This function sends a single packet on the network and returns
• positive number (number of bytes transmitted) or negative for error

*/ -static int davinci_eth_send_packet (struct eth_device *dev,

•                                   void *packet, int length)
  

+static int davinci_emac_send(struct udevice *dev,

•                        void *packet, int length)
  

{ int ret_status = -1; int index; @@ -672,7 +672,7 @@ static int davinci_eth_send_packet (struct eth_device *dev, /*

• This function handles receipt of a packet from the network

*/ -static int davinci_eth_rcv_packet (struct eth_device *dev) +static int davinci_emac_recv(struct udevice *dev, int flags, uchar **packetp) { volatile emac_desc *rx_curr_desc; volatile emac_desc *curr_desc; @@ -742,6 +742,9 @@ static int davinci_eth_rcv_packet (struct eth_device *dev) } return (ret); }

•   *packetp = rx_curr_desc->buffer;
  

•   return (0);
  

}

@@ -750,30 +753,12 @@ static int davinci_eth_rcv_packet (struct eth_device *dev)

• EMAC modules power or pin multiplexors, that is done by board_init()
• much earlier in bootup process. Returns 1 on success, 0 otherwise.

*/ -int davinci_emac_initialize(void) +static int davinci_emac_probe(struct udevice *dev) { u_int32_t phy_id; u_int16_t tmp; int i; int ret;

•   struct eth_device *dev;
  

•   dev = malloc(sizeof *dev);
  

•   if (dev == NULL)
  

•           return -1;
  

•   memset(dev, 0, sizeof *dev);
  

•   strcpy(dev->name, "DaVinci-EMAC");
  

•   dev->iobase = 0;
  

•   dev->init = davinci_eth_open;
  

•   dev->halt = davinci_eth_close;
  

•   dev->send = davinci_eth_send_packet;
  

•   dev->recv = davinci_eth_rcv_packet;
  

•   dev->write_hwaddr = davinci_eth_set_mac_addr;
  

•   eth_register(dev);
  
    davinci_eth_mdio_enable();
  
  

@@ -854,5 +839,29 @@ int davinci_emac_initialize(void) phy[i].auto_negotiate(i); } #endif

•   return(1);
  

•   return 0;
  

}

+static const struct eth_ops davinci_emac_ops = {

•   .start          = davinci_emac_start,
  

•   .send           = davinci_emac_send,
  

•   .recv           = davinci_emac_recv,
  

•   .stop           = davinci_emac_stop,
  

•   .write_hwaddr   = davinci_emac_write_hwaddr,
  

+};

+static const struct udevice_id davinci_emac_ids[] = {

•   { .compatible = "ti,davinci-dm6467-emac" },
  

•   { .compatible = "ti,am3517-emac", },
  

•   { .compatible = "ti,dm816-emac", },
  

•   { }
  

+};

+U_BOOT_DRIVER(davinci_emac) = {

•   .name           = "davinci_emac",
  

•   .id             = UCLASS_ETH,
  

•   .of_match       = davinci_emac_ids,
  

•   .probe          = davinci_emac_probe,
  

•   .ops            = &davinci_emac_ops,
  

•   .platdata_auto_alloc_size = sizeof(struct eth_pdata),
  

+}; diff --git a/include/netdev.h b/include/netdev.h index 0a1a3a2d8d..a40c4adaad 100644 --- a/include/netdev.h +++ b/include/netdev.h @@ -30,7 +30,6 @@ int bcm_sf2_eth_register(bd_t *bis, u8 dev_num); int bfin_EMAC_initialize(bd_t *bis); int calxedaxgmac_initialize(u32 id, ulong base_addr); int cs8900_initialize(u8 dev_num, int base_addr); -int davinci_emac_initialize(void); int dc21x4x_initialize(bd_t *bis); int designware_initialize(ulong base_addr, u32 interface); int dm9000_initialize(bd_t *bis); -- 2.21.0

On Fri, May 31, 2019 at 3:38 PM Adam Ford aford173@gmail.com wrote:

On Fri, May 31, 2019 at 3:11 PM Adam Ford aford173@gmail.com wrote:

...

On Fri, May 31, 2019 at 1:24 PM Adam Ford aford173@gmail.com wrote:

...

On Fri, May 31, 2019 at 8:32 AM Bartosz Golaszewski brgl@bgdev.pl wrote:

...

From: Bartosz Golaszewski bgolaszewski@baylibre.com

Now that we removed all legacy boards selecting TI_EMAC we can completely convert the driver code to using the driver model. This patch also updates all remaining users of davinci_emac.

Nice job on the da850, it works perfectly. Unfortunately, it crashes the am3517_evm

Without your patch, I am able to get an IP address, but when I apply your patch and attempt an IP address, it crashes. I tried again, but I got the same result:

U-Boot 2019.07-rc3-05342-gebd2f49298 (May 31 2019 - 13:18:29 -0500)

AM35XX-GP ES2.0, CPU-OPP2, L3-165MHz, Max CPU Clock 600 MHz Model: TI AM3517 EVM (AM3517/05 TMDSEVM3517) AM3517EVM Board + LPDDR/NAND DRAM: 256 MiB NAND: 512 MiB MMC: OMAP SD/MMC: 0, OMAP SD/MMC: 1 Loading Environment from NAND... OK In: serial@49020000 Out: serial@49020000 Err: serial@49020000 OMAP die ID: 3a5c000100000000016096fa04011001 Net: eth0: ethernet@5c000000 Hit any key to stop autoboot: 0 AM3517_EVM # dhcp BOOTP broadcast 1 data abort pc : [<8ffac38e>] lr : [<8ffabe3b>] reloc pc : [<8015738e>] lr : [<80156e3b>] sp : 8df255b0 ip : 000000ff fp : 8ffeea70 r10: 8ffecbc8 r9 : 8df34ec8 r8 : 00000000 r7 : 0000ffff r6 : 00000fff r5 : 00000d88 r4 : 0000ffff r3 : 0000ffff r2 : 00000156 r1 : 00000fff r0 : 00000fff Flags: nZCv IRQs off FIQs off Mode SVC_32 Code: 46304638 ea410a21 b2892104 bf084299 (7b2c4631) Resetting CPU ...

resetting ...

adam

I got it to work 'once' so I think we might have a timing issue, but I am not sure. I'm trying to enable various debug messages. I was hoping it would be a simple fix, but it's not turning out that way.

I enabled the debug code in the bootp driver and got:

AM3517_EVM # dhcp BOOTP broadcast 1 DHCPHandler: got packet: (src=67, dst=68, len=300) state: 3 Filtering pkt = 0 DHCPHandler: got DHCP packet: (src=67, dst=68, len=300) state: 3 DHCP: state=SELECTING bp_file: "" TRANSITIONING TO REQUESTING STATE dhcp_send_request_packet: Sending DHCPREQUEST Transmitting DHCPREQUEST packet: len = 342 data abort pc : [<8ffac476>] lr : [<8ffabf23>] reloc pc : [<80157476>] lr : [<80156f23>] sp : 8df255b0 ip : 00000005 fp : 8ffeed30

I don't know enough about the networking system to know what to do next. I haven't yet been able to reproduce my single successful dhcp connection

Here is more debug info:

AM3517_EVM # dhcp Trying ethernet@5c000000 BOOTP broadcast 1 net_send_packet(net_tx_packet, pktlen) Check packet size and if < EMAC_MIN_ETHERNET_PKT_SIZE, pad it up Populate the TX descriptor flush_dcache_range Send the packet /* Wait for packet to complete or link down ret_status = 342 eth_get_ops(current)->send returned with 342 DHCPHandler: got packet: (src=67, dst=68, len=300) state: 3 Filtering pkt = 0 DHCPHandler: got DHCP packet: (src=67, dst=68, len=300) state: 3 DHCP: state=SELECTING bp_file: "" TRANSITIONING TO REQUESTING STATE dhcp_send_request_packet: Sending DHCPREQUEST Transmitting DHCPREQUEST packet: len = 342 Check packet size and if < EMAC_MIN_ETHERNET_PKT_SIZE, pad it up Populate the TX descriptor flush_dcache_range Send the packet /* Wait for packet to complete or link down ret_status = 342 eth_get_ops(current)->send returned with 342 dhcp_send_request_packet(bp) completed return data abort

adam

...

adam

...

...

Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com

arch/arm/mach-davinci/cpu.c | 13 ----- arch/arm/mach-omap2/omap3/emac.c | 3 +- board/davinci/da8xxevm/da850evm.c | 6 -- board/davinci/da8xxevm/omapl138_lcdk.c | 14 ----- board/logicpd/am3517evm/am3517evm.c | 1 - board/ti/ti816x/evm.c | 3 +- configs/am3517_evm_defconfig | 1 + configs/da850evm_defconfig | 1 + configs/da850evm_direct_nor_defconfig | 1 + configs/da850evm_nand_defconfig | 1 + configs/omapl138_lcdk_defconfig | 1 + configs/ti816x_evm_defconfig | 1 + drivers/net/ti/davinci_emac.c | 77 ++++++++++++++------------ include/netdev.h | 1 - 14 files changed, 51 insertions(+), 73 deletions(-)

diff --git a/arch/arm/mach-davinci/cpu.c b/arch/arm/mach-davinci/cpu.c index f97ad3fc74..9fd6564d04 100644 --- a/arch/arm/mach-davinci/cpu.c +++ b/arch/arm/mach-davinci/cpu.c @@ -5,7 +5,6 @@ */

#include <common.h> -#include <netdev.h> #include <asm/arch/hardware.h> #include <asm/io.h>

@@ -90,15 +89,3 @@ int set_cpu_clk_info(void) gd->bd->bi_dsp_freq = 0; return 0; }

-/*

• • Initializes on-chip ethernet controllers.
• • to override, implement board_eth_init()
• */

-int cpu_eth_init(bd_t *bis) -{ -#if defined(CONFIG_DRIVER_TI_EMAC)

•   davinci_emac_initialize();
  

-#endif

•   return 0;
  

-} diff --git a/arch/arm/mach-omap2/omap3/emac.c b/arch/arm/mach-omap2/omap3/emac.c index c79e870183..fb0c9188f5 100644 --- a/arch/arm/mach-omap2/omap3/emac.c +++ b/arch/arm/mach-omap2/omap3/emac.c @@ -7,7 +7,6 @@ */

#include <common.h> -#include <netdev.h> #include <asm/io.h> #include <asm/arch/am35x_def.h>

@@ -24,5 +23,5 @@ int cpu_eth_init(bd_t *bis) reset &= ~CPGMACSS_SW_RST; writel(reset, &am35x_scm_general_regs->ip_sw_reset);

•   return davinci_emac_initialize();
  

•   return 0;
  

} diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c index 1bc26828bf..d090c00466 100644 --- a/board/davinci/da8xxevm/da850evm.c +++ b/board/davinci/da8xxevm/da850evm.c @@ -13,7 +13,6 @@ #include <environment.h> #include <i2c.h> #include <net.h> -#include <netdev.h> #include <spi.h> #include <spi_flash.h> #include <asm/arch/hardware.h> @@ -472,11 +471,6 @@ int board_eth_init(bd_t *bis) if (rmii_hw_init()) printf("RMII hardware init failed!!!\n"); #endif

•   if (!davinci_emac_initialize()) {
  

•           printf("Error: Ethernet init failed!\n");
  

•           return -1;
  

•   }
  

•   return 0;
  

} #endif /* CONFIG_DRIVER_TI_EMAC */ diff --git a/board/davinci/da8xxevm/omapl138_lcdk.c b/board/davinci/da8xxevm/omapl138_lcdk.c index 2c2f885d43..ef9656add8 100644 --- a/board/davinci/da8xxevm/omapl138_lcdk.c +++ b/board/davinci/da8xxevm/omapl138_lcdk.c @@ -11,7 +11,6 @@ #include <common.h> #include <i2c.h> #include <net.h> -#include <netdev.h> #include <spi.h> #include <spi_flash.h> #include <asm/arch/hardware.h> @@ -229,19 +228,6 @@ int board_init(void)

#ifdef CONFIG_DRIVER_TI_EMAC

-/*

• • Initializes on-board ethernet controllers.
• */

-int board_eth_init(bd_t *bis) -{

•   if (!davinci_emac_initialize()) {
  

•           printf("Error: Ethernet init failed!\n");
  

•           return -1;
  

•   }
  

•   return 0;
  

-}

#endif /* CONFIG_DRIVER_TI_EMAC */

#define CFG_MAC_ADDR_SPI_BUS 0 diff --git a/board/logicpd/am3517evm/am3517evm.c b/board/logicpd/am3517evm/am3517evm.c index 10031a4801..bfd4e78274 100644 --- a/board/logicpd/am3517evm/am3517evm.c +++ b/board/logicpd/am3517evm/am3517evm.c @@ -28,7 +28,6 @@ #include <linux/usb/gadget.h> #include <linux/usb/musb.h> #include <i2c.h> -#include <netdev.h> #include "am3517evm.h"

DECLARE_GLOBAL_DATA_PTR; diff --git a/board/ti/ti816x/evm.c b/board/ti/ti816x/evm.c index 07a084bab8..240df8cbe1 100644 --- a/board/ti/ti816x/evm.c +++ b/board/ti/ti816x/evm.c @@ -9,7 +9,6 @@ #include <common.h> #include <environment.h> #include <spl.h> -#include <netdev.h> #include <asm/cache.h> #include <asm/io.h> #include <asm/arch/clock.h> @@ -56,7 +55,7 @@ int board_eth_init(bd_t *bis) printf("Unable to read MAC address. Set <ethaddr>\n"); }

•   return davinci_emac_initialize();
  

•   return 0;
  

}

#ifdef CONFIG_SPL_BUILD diff --git a/configs/am3517_evm_defconfig b/configs/am3517_evm_defconfig index b9f59f3291..5cb76322df 100644 --- a/configs/am3517_evm_defconfig +++ b/configs/am3517_evm_defconfig @@ -44,6 +44,7 @@ CONFIG_SYS_NAND_BUSWIDTH_16BIT=y CONFIG_SYS_NAND_U_BOOT_LOCATIONS=y CONFIG_SYS_NAND_U_BOOT_OFFS=0x80000 CONFIG_SPL_NAND_SIMPLE=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_PINCTRL=y diff --git a/configs/da850evm_defconfig b/configs/da850evm_defconfig index 8c16d5c4f5..ca304ed78a 100644 --- a/configs/da850evm_defconfig +++ b/configs/da850evm_defconfig @@ -60,6 +60,7 @@ CONFIG_SF_DEFAULT_SPEED=30000000 CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y CONFIG_SPI_FLASH_MTD=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_PINCTRL=y diff --git a/configs/da850evm_direct_nor_defconfig b/configs/da850evm_direct_nor_defconfig index 166e77b8e3..9b1da07384 100644 --- a/configs/da850evm_direct_nor_defconfig +++ b/configs/da850evm_direct_nor_defconfig @@ -50,6 +50,7 @@ CONFIG_DM_SPI_FLASH=y CONFIG_SPI_FLASH=y CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_PINCTRL=y diff --git a/configs/da850evm_nand_defconfig b/configs/da850evm_nand_defconfig index b8eac0e659..eaf5d73dd8 100644 --- a/configs/da850evm_nand_defconfig +++ b/configs/da850evm_nand_defconfig @@ -60,6 +60,7 @@ CONFIG_SPI_FLASH=y CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y CONFIG_SPI_FLASH_MTD=y +CONFIG_DM_ETH=y CONFIG_PINCTRL=y CONFIG_PINCTRL_SINGLE=y CONFIG_DM_SERIAL=y diff --git a/configs/omapl138_lcdk_defconfig b/configs/omapl138_lcdk_defconfig index e43141844a..f93a06083e 100644 --- a/configs/omapl138_lcdk_defconfig +++ b/configs/omapl138_lcdk_defconfig @@ -50,6 +50,7 @@ CONFIG_SPI_FLASH=y CONFIG_SF_DEFAULT_SPEED=30000000 CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_DM_SERIAL=y diff --git a/configs/ti816x_evm_defconfig b/configs/ti816x_evm_defconfig index bf877f596b..b233ab8cc9 100644 --- a/configs/ti816x_evm_defconfig +++ b/configs/ti816x_evm_defconfig @@ -45,6 +45,7 @@ CONFIG_SYS_I2C_OMAP24XX=y CONFIG_MMC_OMAP_HS=y CONFIG_NAND=y CONFIG_SYS_NAND_BUSWIDTH_16BIT=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_SYS_NS16550=y diff --git a/drivers/net/ti/davinci_emac.c b/drivers/net/ti/davinci_emac.c index 9d53984973..4b0a98a4aa 100644 --- a/drivers/net/ti/davinci_emac.c +++ b/drivers/net/ti/davinci_emac.c @@ -26,7 +26,6 @@ #include <net.h> #include <miiphy.h> #include <malloc.h> -#include <netdev.h> #include <linux/compiler.h> #include <asm/arch/emac_defs.h> #include <asm/io.h> @@ -107,8 +106,9 @@ static u_int8_t num_phy;

phy_t phy[CONFIG_SYS_DAVINCI_EMAC_PHY_COUNT];

-static int davinci_eth_set_mac_addr(struct eth_device *dev) +static int davinci_emac_write_hwaddr(struct udevice *dev) {

•   struct eth_pdata *pdata = dev_get_platdata(dev);
    unsigned long           mac_hi;
    unsigned long           mac_lo;
  
  

@@ -118,12 +118,12 @@ static int davinci_eth_set_mac_addr(struct eth_device *dev) * Using channel 0 only - other channels are disabled * */ writel(0, &adap_emac->MACINDEX);

•   mac_hi = (dev->enetaddr[3] << 24) |
  

•            (dev->enetaddr[2] << 16) |
  

•            (dev->enetaddr[1] << 8)  |
  

•            (dev->enetaddr[0]);
  

•   mac_lo = (dev->enetaddr[5] << 8) |
  

•            (dev->enetaddr[4]);
  

•   mac_hi = (pdata->enetaddr[3] << 24) |
  

•            (pdata->enetaddr[2] << 16) |
  

•            (pdata->enetaddr[1] << 8)  |
  

•            (pdata->enetaddr[0]);
  

•   mac_lo = (pdata->enetaddr[5] << 8) |
  

•            (pdata->enetaddr[4]);
  
    writel(mac_hi, &adap_emac->MACADDRHI);
  

#if defined(DAVINCI_EMAC_VERSION2) @@ -411,7 +411,7 @@ static void __attribute__((unused)) davinci_eth_gigabit_enable(int phy_addr) }

/* Eth device open */ -static int davinci_eth_open(struct eth_device *dev, bd_t *bis) +static int davinci_emac_start(struct udevice *dev) { dv_reg_p addr; u_int32_t clkdiv, cnt, mac_control; @@ -447,7 +447,7 @@ static int davinci_eth_open(struct eth_device *dev, bd_t *bis) writel(1, &adap_emac->TXCONTROL); writel(1, &adap_emac->RXCONTROL);

•   davinci_eth_set_mac_addr(dev);
  

•   davinci_emac_write_hwaddr(dev);
  
    /* Set DMA 8 TX / 8 RX Head pointers to 0 */
    addr = &adap_emac->TX0HDP;
  

@@ -588,7 +588,7 @@ static void davinci_eth_ch_teardown(int ch) }

/* Eth device close */ -static void davinci_eth_close(struct eth_device *dev) +static void davinci_emac_stop(struct udevice *dev) { debug_emac("+ emac_close\n");

@@ -619,8 +619,8 @@ static int tx_send_loop = 0;

• This function sends a single packet on the network and returns
• positive number (number of bytes transmitted) or negative for error

*/ -static int davinci_eth_send_packet (struct eth_device *dev,

•                                   void *packet, int length)
  

+static int davinci_emac_send(struct udevice *dev,

•                        void *packet, int length)
  

{ int ret_status = -1; int index; @@ -672,7 +672,7 @@ static int davinci_eth_send_packet (struct eth_device *dev, /*

• This function handles receipt of a packet from the network

*/ -static int davinci_eth_rcv_packet (struct eth_device *dev) +static int davinci_emac_recv(struct udevice *dev, int flags, uchar **packetp) { volatile emac_desc *rx_curr_desc; volatile emac_desc *curr_desc; @@ -742,6 +742,9 @@ static int davinci_eth_rcv_packet (struct eth_device *dev) } return (ret); }

•   *packetp = rx_curr_desc->buffer;
  

•   return (0);
  

}

@@ -750,30 +753,12 @@ static int davinci_eth_rcv_packet (struct eth_device *dev)

• EMAC modules power or pin multiplexors, that is done by board_init()
• much earlier in bootup process. Returns 1 on success, 0 otherwise.

*/ -int davinci_emac_initialize(void) +static int davinci_emac_probe(struct udevice *dev) { u_int32_t phy_id; u_int16_t tmp; int i; int ret;

•   struct eth_device *dev;
  

•   dev = malloc(sizeof *dev);
  

•   if (dev == NULL)
  

•           return -1;
  

•   memset(dev, 0, sizeof *dev);
  

•   strcpy(dev->name, "DaVinci-EMAC");
  

•   dev->iobase = 0;
  

•   dev->init = davinci_eth_open;
  

•   dev->halt = davinci_eth_close;
  

•   dev->send = davinci_eth_send_packet;
  

•   dev->recv = davinci_eth_rcv_packet;
  

•   dev->write_hwaddr = davinci_eth_set_mac_addr;
  

•   eth_register(dev);
  
    davinci_eth_mdio_enable();
  
  

@@ -854,5 +839,29 @@ int davinci_emac_initialize(void) phy[i].auto_negotiate(i); } #endif

•   return(1);
  

•   return 0;
  

}

+static const struct eth_ops davinci_emac_ops = {

•   .start          = davinci_emac_start,
  

•   .send           = davinci_emac_send,
  

•   .recv           = davinci_emac_recv,
  

•   .stop           = davinci_emac_stop,
  

•   .write_hwaddr   = davinci_emac_write_hwaddr,
  

+};

+static const struct udevice_id davinci_emac_ids[] = {

•   { .compatible = "ti,davinci-dm6467-emac" },
  

•   { .compatible = "ti,am3517-emac", },
  

•   { .compatible = "ti,dm816-emac", },
  

•   { }
  

+};

+U_BOOT_DRIVER(davinci_emac) = {

•   .name           = "davinci_emac",
  

•   .id             = UCLASS_ETH,
  

•   .of_match       = davinci_emac_ids,
  

•   .probe          = davinci_emac_probe,
  

•   .ops            = &davinci_emac_ops,
  

•   .platdata_auto_alloc_size = sizeof(struct eth_pdata),
  

+}; diff --git a/include/netdev.h b/include/netdev.h index 0a1a3a2d8d..a40c4adaad 100644 --- a/include/netdev.h +++ b/include/netdev.h @@ -30,7 +30,6 @@ int bcm_sf2_eth_register(bd_t *bis, u8 dev_num); int bfin_EMAC_initialize(bd_t *bis); int calxedaxgmac_initialize(u32 id, ulong base_addr); int cs8900_initialize(u8 dev_num, int base_addr); -int davinci_emac_initialize(void); int dc21x4x_initialize(bd_t *bis); int designware_initialize(ulong base_addr, u32 interface); int dm9000_initialize(bd_t *bis); -- 2.21.0

On Mon, Jun 3, 2019 at 3:12 AM Bartosz Golaszewski brgl@bgdev.pl wrote:

sob., 1 cze 2019 o 05:24 Adam Ford aford173@gmail.com napisał(a):

...

On Fri, May 31, 2019 at 8:32 AM Bartosz Golaszewski brgl@bgdev.pl wrote:

...

From: Bartosz Golaszewski bgolaszewski@baylibre.com

Now that we removed all legacy boards selecting TI_EMAC we can completely convert the driver code to using the driver model. This patch also updates all remaining users of davinci_emac.

I took a break from this to come back, and I'm going to give some feedback about how the driver was written. I still do not know why I cannot get an IP address with this patch on the AM3517-evm.

Hi Adam,

thanks for all the testing. Unfortunately I can only test with da850-evm and da850-lcdk.

I was wondering if it is possible that the problem is caused by cpu_eth_init() from ./arch/arm/mach-omap2/omap3/emac. not being called with CONFIG_DM_ETH? The comments say that it brings the module out of reset, so maybe this is what causes the problem you're seeing?

I looked into that nearly right away, but there is a chunk of code in board/logicpd/am3517evm/am3517evm.c which has a function called misc_init_r() which does the same thing. Looking through the debug data, it appears as if the drive is communicating for a bit, but at some point it dies. I'm not going to be able to look at it for a few days. Is there any way you can submit a V2 to use #ifdef's to allow users to use the same driver with and without DM_ETH? That would give the da850/L138 boards the DM_ETH, and give me some time to troubleshoot the am3517-evm. I know Sekhar works for TI and the AM3517 was the official development kit for TI back in the day. If TI has some ideas, I'm open to trying them as well when I can get back to it.

adam

...

...

Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com

arch/arm/mach-davinci/cpu.c | 13 ----- arch/arm/mach-omap2/omap3/emac.c | 3 +- board/davinci/da8xxevm/da850evm.c | 6 -- board/davinci/da8xxevm/omapl138_lcdk.c | 14 ----- board/logicpd/am3517evm/am3517evm.c | 1 - board/ti/ti816x/evm.c | 3 +- configs/am3517_evm_defconfig | 1 + configs/da850evm_defconfig | 1 + configs/da850evm_direct_nor_defconfig | 1 + configs/da850evm_nand_defconfig | 1 + configs/omapl138_lcdk_defconfig | 1 + configs/ti816x_evm_defconfig | 1 + drivers/net/ti/davinci_emac.c | 77 ++++++++++++++------------ include/netdev.h | 1 - 14 files changed, 51 insertions(+), 73 deletions(-)

diff --git a/arch/arm/mach-davinci/cpu.c b/arch/arm/mach-davinci/cpu.c index f97ad3fc74..9fd6564d04 100644 --- a/arch/arm/mach-davinci/cpu.c +++ b/arch/arm/mach-davinci/cpu.c @@ -5,7 +5,6 @@ */

#include <common.h> -#include <netdev.h> #include <asm/arch/hardware.h> #include <asm/io.h>

@@ -90,15 +89,3 @@ int set_cpu_clk_info(void) gd->bd->bi_dsp_freq = 0; return 0; }

-/*

• • Initializes on-chip ethernet controllers.
• • to override, implement board_eth_init()
• */

-int cpu_eth_init(bd_t *bis) -{ -#if defined(CONFIG_DRIVER_TI_EMAC)

•   davinci_emac_initialize();
  

-#endif

•   return 0;
  

-} diff --git a/arch/arm/mach-omap2/omap3/emac.c b/arch/arm/mach-omap2/omap3/emac.c index c79e870183..fb0c9188f5 100644 --- a/arch/arm/mach-omap2/omap3/emac.c +++ b/arch/arm/mach-omap2/omap3/emac.c @@ -7,7 +7,6 @@ */

#include <common.h> -#include <netdev.h> #include <asm/io.h> #include <asm/arch/am35x_def.h>

@@ -24,5 +23,5 @@ int cpu_eth_init(bd_t *bis) reset &= ~CPGMACSS_SW_RST; writel(reset, &am35x_scm_general_regs->ip_sw_reset);

•   return davinci_emac_initialize();
  

•   return 0;
  

} diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c index 1bc26828bf..d090c00466 100644 --- a/board/davinci/da8xxevm/da850evm.c +++ b/board/davinci/da8xxevm/da850evm.c @@ -13,7 +13,6 @@ #include <environment.h> #include <i2c.h> #include <net.h> -#include <netdev.h> #include <spi.h> #include <spi_flash.h> #include <asm/arch/hardware.h> @@ -472,11 +471,6 @@ int board_eth_init(bd_t *bis) if (rmii_hw_init()) printf("RMII hardware init failed!!!\n"); #endif

•   if (!davinci_emac_initialize()) {
  

•           printf("Error: Ethernet init failed!\n");
  

•           return -1;
  

•   }
  

•   return 0;
  

} #endif /* CONFIG_DRIVER_TI_EMAC */ diff --git a/board/davinci/da8xxevm/omapl138_lcdk.c b/board/davinci/da8xxevm/omapl138_lcdk.c index 2c2f885d43..ef9656add8 100644 --- a/board/davinci/da8xxevm/omapl138_lcdk.c +++ b/board/davinci/da8xxevm/omapl138_lcdk.c @@ -11,7 +11,6 @@ #include <common.h> #include <i2c.h> #include <net.h> -#include <netdev.h> #include <spi.h> #include <spi_flash.h> #include <asm/arch/hardware.h> @@ -229,19 +228,6 @@ int board_init(void)

#ifdef CONFIG_DRIVER_TI_EMAC

-/*

• • Initializes on-board ethernet controllers.
• */

-int board_eth_init(bd_t *bis) -{

•   if (!davinci_emac_initialize()) {
  

•           printf("Error: Ethernet init failed!\n");
  

•           return -1;
  

•   }
  

•   return 0;
  

-}

#endif /* CONFIG_DRIVER_TI_EMAC */

#define CFG_MAC_ADDR_SPI_BUS 0 diff --git a/board/logicpd/am3517evm/am3517evm.c b/board/logicpd/am3517evm/am3517evm.c index 10031a4801..bfd4e78274 100644 --- a/board/logicpd/am3517evm/am3517evm.c +++ b/board/logicpd/am3517evm/am3517evm.c @@ -28,7 +28,6 @@ #include <linux/usb/gadget.h> #include <linux/usb/musb.h> #include <i2c.h> -#include <netdev.h> #include "am3517evm.h"

DECLARE_GLOBAL_DATA_PTR; diff --git a/board/ti/ti816x/evm.c b/board/ti/ti816x/evm.c index 07a084bab8..240df8cbe1 100644 --- a/board/ti/ti816x/evm.c +++ b/board/ti/ti816x/evm.c @@ -9,7 +9,6 @@ #include <common.h> #include <environment.h> #include <spl.h> -#include <netdev.h> #include <asm/cache.h> #include <asm/io.h> #include <asm/arch/clock.h> @@ -56,7 +55,7 @@ int board_eth_init(bd_t *bis) printf("Unable to read MAC address. Set <ethaddr>\n"); }

•   return davinci_emac_initialize();
  

•   return 0;
  

}

#ifdef CONFIG_SPL_BUILD diff --git a/configs/am3517_evm_defconfig b/configs/am3517_evm_defconfig index b9f59f3291..5cb76322df 100644 --- a/configs/am3517_evm_defconfig +++ b/configs/am3517_evm_defconfig @@ -44,6 +44,7 @@ CONFIG_SYS_NAND_BUSWIDTH_16BIT=y CONFIG_SYS_NAND_U_BOOT_LOCATIONS=y CONFIG_SYS_NAND_U_BOOT_OFFS=0x80000 CONFIG_SPL_NAND_SIMPLE=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_PINCTRL=y diff --git a/configs/da850evm_defconfig b/configs/da850evm_defconfig index 8c16d5c4f5..ca304ed78a 100644 --- a/configs/da850evm_defconfig +++ b/configs/da850evm_defconfig @@ -60,6 +60,7 @@ CONFIG_SF_DEFAULT_SPEED=30000000 CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y CONFIG_SPI_FLASH_MTD=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_PINCTRL=y diff --git a/configs/da850evm_direct_nor_defconfig b/configs/da850evm_direct_nor_defconfig index 166e77b8e3..9b1da07384 100644 --- a/configs/da850evm_direct_nor_defconfig +++ b/configs/da850evm_direct_nor_defconfig @@ -50,6 +50,7 @@ CONFIG_DM_SPI_FLASH=y CONFIG_SPI_FLASH=y CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_PINCTRL=y diff --git a/configs/da850evm_nand_defconfig b/configs/da850evm_nand_defconfig index b8eac0e659..eaf5d73dd8 100644 --- a/configs/da850evm_nand_defconfig +++ b/configs/da850evm_nand_defconfig @@ -60,6 +60,7 @@ CONFIG_SPI_FLASH=y CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y CONFIG_SPI_FLASH_MTD=y +CONFIG_DM_ETH=y CONFIG_PINCTRL=y CONFIG_PINCTRL_SINGLE=y CONFIG_DM_SERIAL=y diff --git a/configs/omapl138_lcdk_defconfig b/configs/omapl138_lcdk_defconfig index e43141844a..f93a06083e 100644 --- a/configs/omapl138_lcdk_defconfig +++ b/configs/omapl138_lcdk_defconfig @@ -50,6 +50,7 @@ CONFIG_SPI_FLASH=y CONFIG_SF_DEFAULT_SPEED=30000000 CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_DM_SERIAL=y diff --git a/configs/ti816x_evm_defconfig b/configs/ti816x_evm_defconfig index bf877f596b..b233ab8cc9 100644 --- a/configs/ti816x_evm_defconfig +++ b/configs/ti816x_evm_defconfig @@ -45,6 +45,7 @@ CONFIG_SYS_I2C_OMAP24XX=y CONFIG_MMC_OMAP_HS=y CONFIG_NAND=y CONFIG_SYS_NAND_BUSWIDTH_16BIT=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_SYS_NS16550=y diff --git a/drivers/net/ti/davinci_emac.c b/drivers/net/ti/davinci_emac.c index 9d53984973..4b0a98a4aa 100644 --- a/drivers/net/ti/davinci_emac.c +++ b/drivers/net/ti/davinci_emac.c @@ -26,7 +26,6 @@ #include <net.h> #include <miiphy.h> #include <malloc.h> -#include <netdev.h> #include <linux/compiler.h> #include <asm/arch/emac_defs.h> #include <asm/io.h> @@ -107,8 +106,9 @@ static u_int8_t num_phy;

phy_t phy[CONFIG_SYS_DAVINCI_EMAC_PHY_COUNT];

-static int davinci_eth_set_mac_addr(struct eth_device *dev) +static int davinci_emac_write_hwaddr(struct udevice *dev) {

•   struct eth_pdata *pdata = dev_get_platdata(dev);
    unsigned long           mac_hi;
    unsigned long           mac_lo;
  
  

@@ -118,12 +118,12 @@ static int davinci_eth_set_mac_addr(struct eth_device *dev) * Using channel 0 only - other channels are disabled * */ writel(0, &adap_emac->MACINDEX);

•   mac_hi = (dev->enetaddr[3] << 24) |
  

•            (dev->enetaddr[2] << 16) |
  

•            (dev->enetaddr[1] << 8)  |
  

•            (dev->enetaddr[0]);
  

•   mac_lo = (dev->enetaddr[5] << 8) |
  

•            (dev->enetaddr[4]);
  

•   mac_hi = (pdata->enetaddr[3] << 24) |
  

•            (pdata->enetaddr[2] << 16) |
  

•            (pdata->enetaddr[1] << 8)  |
  

•            (pdata->enetaddr[0]);
  

•   mac_lo = (pdata->enetaddr[5] << 8) |
  

•            (pdata->enetaddr[4]);
  
    writel(mac_hi, &adap_emac->MACADDRHI);
  

#if defined(DAVINCI_EMAC_VERSION2) @@ -411,7 +411,7 @@ static void __attribute__((unused)) davinci_eth_gigabit_enable(int phy_addr) }

/* Eth device open */ -static int davinci_eth_open(struct eth_device *dev, bd_t *bis) +static int davinci_emac_start(struct udevice *dev) { dv_reg_p addr; u_int32_t clkdiv, cnt, mac_control; @@ -447,7 +447,7 @@ static int davinci_eth_open(struct eth_device *dev, bd_t *bis) writel(1, &adap_emac->TXCONTROL); writel(1, &adap_emac->RXCONTROL);

•   davinci_eth_set_mac_addr(dev);
  

•   davinci_emac_write_hwaddr(dev);
  
    /* Set DMA 8 TX / 8 RX Head pointers to 0 */
    addr = &adap_emac->TX0HDP;
  

@@ -588,7 +588,7 @@ static void davinci_eth_ch_teardown(int ch) }

/* Eth device close */ -static void davinci_eth_close(struct eth_device *dev) +static void davinci_emac_stop(struct udevice *dev) { debug_emac("+ emac_close\n");

@@ -619,8 +619,8 @@ static int tx_send_loop = 0;

• This function sends a single packet on the network and returns
• positive number (number of bytes transmitted) or negative for error

*/ -static int davinci_eth_send_packet (struct eth_device *dev,

•                                   void *packet, int length)
  

+static int davinci_emac_send(struct udevice *dev,

•                        void *packet, int length)
  

{ int ret_status = -1; int index; @@ -672,7 +672,7 @@ static int davinci_eth_send_packet (struct eth_device *dev, /*

• This function handles receipt of a packet from the network

*/ -static int davinci_eth_rcv_packet (struct eth_device *dev) +static int davinci_emac_recv(struct udevice *dev, int flags, uchar **packetp) { volatile emac_desc *rx_curr_desc; volatile emac_desc *curr_desc; @@ -742,6 +742,9 @@ static int davinci_eth_rcv_packet (struct eth_device *dev) } return (ret); }

•   *packetp = rx_curr_desc->buffer;
  

•   return (0);
  

}

@@ -750,30 +753,12 @@ static int davinci_eth_rcv_packet (struct eth_device *dev)

• EMAC modules power or pin multiplexors, that is done by board_init()
• much earlier in bootup process. Returns 1 on success, 0 otherwise.

*/ -int davinci_emac_initialize(void) +static int davinci_emac_probe(struct udevice *dev)

From the device tree, we should be able to remove the hard-coded values referencing:

EMAC_BASE_ADDR; EMAC_WRAPPER_BASE_ADDR;

/* EMAC descriptors */ EMAC_WRAPPER_RAM_ADDR + EMAC_RX_DESC_BASE); EMAC_WRAPPER_RAM_ADDR + EMAC_TX_DESC_BASE);

From the device tree, EMAC_BASE_ADDR should point to the address of the emac node. In the case of the AM3517, it would be 5c000000 and this is how the name appears, so we should be able to use that. EMAC_WRAPPER_BASE_ADDR should the also read by the device tree as an offset from the base address by reading "ti,davinci-ctrl-reg-offset" and "ti,davinci-ctrl-mod-reg-offset" in the case of AM3517, 0x5c000000

• 0x10000 + 0 give the same correct address as the

EMAC_WRAPPER_BASE_ADDR currently does.

The EMAC_WRAPPER_RAM_ADDR is read by reading the "ti,davinci-ctrl-ram-offset" which for am3517 is 0x20000 and 5c000000

• 0x20000 matches the value currenty set for 0x5C020000.

I may have some of the math wrong a bit, but the linux driver doc explains how these tree entries are defined, and I would think the emac driver could be modeled after the Linux implementation.

I think we should try to extract these and calculator the addresses this way we can remove the hard-coded values and make the driver more generic.

Definitely and it's on my list, but first I wanted to do the conversion, make sure it works and then gradually improve the code.

Best regards, Bartosz Golaszewski

...

I am going to bed now, but I'll try to beat on the da850 on Sunday.

adam

...

{ u_int32_t phy_id; u_int16_t tmp; int i; int ret;

•   struct eth_device *dev;
  

•   dev = malloc(sizeof *dev);
  

•   if (dev == NULL)
  

•           return -1;
  

•   memset(dev, 0, sizeof *dev);
  

•   strcpy(dev->name, "DaVinci-EMAC");
  

•   dev->iobase = 0;
  

•   dev->init = davinci_eth_open;
  

•   dev->halt = davinci_eth_close;
  

•   dev->send = davinci_eth_send_packet;
  

•   dev->recv = davinci_eth_rcv_packet;
  

•   dev->write_hwaddr = davinci_eth_set_mac_addr;
  

•   eth_register(dev);
  
    davinci_eth_mdio_enable();
  
  

@@ -854,5 +839,29 @@ int davinci_emac_initialize(void) phy[i].auto_negotiate(i); } #endif

•   return(1);
  

•   return 0;
  

}

+static const struct eth_ops davinci_emac_ops = {

•   .start          = davinci_emac_start,
  

•   .send           = davinci_emac_send,
  

•   .recv           = davinci_emac_recv,
  

•   .stop           = davinci_emac_stop,
  

•   .write_hwaddr   = davinci_emac_write_hwaddr,
  

+};

+static const struct udevice_id davinci_emac_ids[] = {

•   { .compatible = "ti,davinci-dm6467-emac" },
  

•   { .compatible = "ti,am3517-emac", },
  

•   { .compatible = "ti,dm816-emac", },
  

•   { }
  

+};

+U_BOOT_DRIVER(davinci_emac) = {

•   .name           = "davinci_emac",
  

•   .id             = UCLASS_ETH,
  

•   .of_match       = davinci_emac_ids,
  

•   .probe          = davinci_emac_probe,
  

•   .ops            = &davinci_emac_ops,
  

•   .platdata_auto_alloc_size = sizeof(struct eth_pdata),
  

+}; diff --git a/include/netdev.h b/include/netdev.h index 0a1a3a2d8d..a40c4adaad 100644 --- a/include/netdev.h +++ b/include/netdev.h @@ -30,7 +30,6 @@ int bcm_sf2_eth_register(bd_t *bis, u8 dev_num); int bfin_EMAC_initialize(bd_t *bis); int calxedaxgmac_initialize(u32 id, ulong base_addr); int cs8900_initialize(u8 dev_num, int base_addr); -int davinci_emac_initialize(void); int dc21x4x_initialize(bd_t *bis); int designware_initialize(ulong base_addr, u32 interface); int dm9000_initialize(bd_t *bis); -- 2.21.0

On Mon, Jun 3, 2019 at 10:29 AM Bartosz Golaszewski brgl@bgdev.pl wrote:

pon., 3 cze 2019 o 15:03 Adam Ford aford173@gmail.com napisał(a):

...

On Mon, Jun 3, 2019 at 3:12 AM Bartosz Golaszewski brgl@bgdev.pl wrote:

...

sob., 1 cze 2019 o 05:24 Adam Ford aford173@gmail.com napisał(a):

...

On Fri, May 31, 2019 at 8:32 AM Bartosz Golaszewski brgl@bgdev.pl wrote:

...

From: Bartosz Golaszewski bgolaszewski@baylibre.com

Now that we removed all legacy boards selecting TI_EMAC we can completely convert the driver code to using the driver model. This patch also updates all remaining users of davinci_emac.

I took a break from this to come back, and I'm going to give some feedback about how the driver was written. I still do not know why I cannot get an IP address with this patch on the AM3517-evm.

Hi Adam,

thanks for all the testing. Unfortunately I can only test with da850-evm and da850-lcdk.

I was wondering if it is possible that the problem is caused by cpu_eth_init() from ./arch/arm/mach-omap2/omap3/emac. not being called with CONFIG_DM_ETH? The comments say that it brings the module out of reset, so maybe this is what causes the problem you're seeing?

I looked into that nearly right away, but there is a chunk of code in board/logicpd/am3517evm/am3517evm.c which has a function called misc_init_r() which does the same thing. Looking through the debug data, it appears as if the drive is communicating for a bit, but at some point it dies. I'm not going to be able to look at it for a few days. Is there any way you can submit a V2 to use #ifdef's to allow users to use the same driver with and without DM_ETH? That would give the da850/L138 boards the DM_ETH, and give me some time to troubleshoot the am3517-evm. I know Sekhar works for TI and the AM3517 was the official development kit for TI back in the day. If TI has some ideas, I'm open to trying them as well when I can get back to it.

adam

Hi Adam,

I'm trying to find out if I can get my hands on one of these boards somehow.

My priority for this week is the nand driver conversion. I'll also be off next week and the one after. I think that since we already removed a bunch of boards to make it possible to avoid the ifdef hell, it's better to wait a bit more than to merge something that we'll remove soon anyway.

If Sekhar agrees, we can push back merging of this patch until it's fixed.

There's also a big backlog of my other davinci patches on the list anyway, maybe they'll get picked up before we get back to it.

Bart

...

...

...

...

Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com

arch/arm/mach-davinci/cpu.c | 13 ----- arch/arm/mach-omap2/omap3/emac.c | 3 +- board/davinci/da8xxevm/da850evm.c | 6 -- board/davinci/da8xxevm/omapl138_lcdk.c | 14 ----- board/logicpd/am3517evm/am3517evm.c | 1 - board/ti/ti816x/evm.c | 3 +- configs/am3517_evm_defconfig | 1 + configs/da850evm_defconfig | 1 + configs/da850evm_direct_nor_defconfig | 1 + configs/da850evm_nand_defconfig | 1 + configs/omapl138_lcdk_defconfig | 1 + configs/ti816x_evm_defconfig | 1 + drivers/net/ti/davinci_emac.c | 77 ++++++++++++++------------ include/netdev.h | 1 - 14 files changed, 51 insertions(+), 73 deletions(-)

diff --git a/arch/arm/mach-davinci/cpu.c b/arch/arm/mach-davinci/cpu.c index f97ad3fc74..9fd6564d04 100644 --- a/arch/arm/mach-davinci/cpu.c +++ b/arch/arm/mach-davinci/cpu.c @@ -5,7 +5,6 @@ */

#include <common.h> -#include <netdev.h> #include <asm/arch/hardware.h> #include <asm/io.h>

@@ -90,15 +89,3 @@ int set_cpu_clk_info(void) gd->bd->bi_dsp_freq = 0; return 0; }

-/*

• • Initializes on-chip ethernet controllers.
• • to override, implement board_eth_init()
• */

-int cpu_eth_init(bd_t *bis) -{ -#if defined(CONFIG_DRIVER_TI_EMAC)

•   davinci_emac_initialize();
  

-#endif

•   return 0;
  

-} diff --git a/arch/arm/mach-omap2/omap3/emac.c b/arch/arm/mach-omap2/omap3/emac.c index c79e870183..fb0c9188f5 100644 --- a/arch/arm/mach-omap2/omap3/emac.c +++ b/arch/arm/mach-omap2/omap3/emac.c @@ -7,7 +7,6 @@ */

#include <common.h> -#include <netdev.h> #include <asm/io.h> #include <asm/arch/am35x_def.h>

@@ -24,5 +23,5 @@ int cpu_eth_init(bd_t *bis) reset &= ~CPGMACSS_SW_RST; writel(reset, &am35x_scm_general_regs->ip_sw_reset);

•   return davinci_emac_initialize();
  

•   return 0;
  

} diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c index 1bc26828bf..d090c00466 100644 --- a/board/davinci/da8xxevm/da850evm.c +++ b/board/davinci/da8xxevm/da850evm.c @@ -13,7 +13,6 @@ #include <environment.h> #include <i2c.h> #include <net.h> -#include <netdev.h> #include <spi.h> #include <spi_flash.h> #include <asm/arch/hardware.h> @@ -472,11 +471,6 @@ int board_eth_init(bd_t *bis) if (rmii_hw_init()) printf("RMII hardware init failed!!!\n"); #endif

•   if (!davinci_emac_initialize()) {
  

•           printf("Error: Ethernet init failed!\n");
  

•           return -1;
  

•   }
  

•   return 0;
  

} #endif /* CONFIG_DRIVER_TI_EMAC */ diff --git a/board/davinci/da8xxevm/omapl138_lcdk.c b/board/davinci/da8xxevm/omapl138_lcdk.c index 2c2f885d43..ef9656add8 100644 --- a/board/davinci/da8xxevm/omapl138_lcdk.c +++ b/board/davinci/da8xxevm/omapl138_lcdk.c @@ -11,7 +11,6 @@ #include <common.h> #include <i2c.h> #include <net.h> -#include <netdev.h> #include <spi.h> #include <spi_flash.h> #include <asm/arch/hardware.h> @@ -229,19 +228,6 @@ int board_init(void)

#ifdef CONFIG_DRIVER_TI_EMAC

-/*

• • Initializes on-board ethernet controllers.
• */

-int board_eth_init(bd_t *bis) -{

•   if (!davinci_emac_initialize()) {
  

•           printf("Error: Ethernet init failed!\n");
  

•           return -1;
  

•   }
  

•   return 0;
  

-}

#endif /* CONFIG_DRIVER_TI_EMAC */

#define CFG_MAC_ADDR_SPI_BUS 0 diff --git a/board/logicpd/am3517evm/am3517evm.c b/board/logicpd/am3517evm/am3517evm.c index 10031a4801..bfd4e78274 100644 --- a/board/logicpd/am3517evm/am3517evm.c +++ b/board/logicpd/am3517evm/am3517evm.c @@ -28,7 +28,6 @@ #include <linux/usb/gadget.h> #include <linux/usb/musb.h> #include <i2c.h> -#include <netdev.h> #include "am3517evm.h"

DECLARE_GLOBAL_DATA_PTR; diff --git a/board/ti/ti816x/evm.c b/board/ti/ti816x/evm.c index 07a084bab8..240df8cbe1 100644 --- a/board/ti/ti816x/evm.c +++ b/board/ti/ti816x/evm.c @@ -9,7 +9,6 @@ #include <common.h> #include <environment.h> #include <spl.h> -#include <netdev.h> #include <asm/cache.h> #include <asm/io.h> #include <asm/arch/clock.h> @@ -56,7 +55,7 @@ int board_eth_init(bd_t *bis) printf("Unable to read MAC address. Set <ethaddr>\n"); }

•   return davinci_emac_initialize();
  

•   return 0;
  

}

#ifdef CONFIG_SPL_BUILD diff --git a/configs/am3517_evm_defconfig b/configs/am3517_evm_defconfig index b9f59f3291..5cb76322df 100644 --- a/configs/am3517_evm_defconfig +++ b/configs/am3517_evm_defconfig @@ -44,6 +44,7 @@ CONFIG_SYS_NAND_BUSWIDTH_16BIT=y CONFIG_SYS_NAND_U_BOOT_LOCATIONS=y CONFIG_SYS_NAND_U_BOOT_OFFS=0x80000 CONFIG_SPL_NAND_SIMPLE=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_PINCTRL=y diff --git a/configs/da850evm_defconfig b/configs/da850evm_defconfig index 8c16d5c4f5..ca304ed78a 100644 --- a/configs/da850evm_defconfig +++ b/configs/da850evm_defconfig @@ -60,6 +60,7 @@ CONFIG_SF_DEFAULT_SPEED=30000000 CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y CONFIG_SPI_FLASH_MTD=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_PINCTRL=y diff --git a/configs/da850evm_direct_nor_defconfig b/configs/da850evm_direct_nor_defconfig index 166e77b8e3..9b1da07384 100644 --- a/configs/da850evm_direct_nor_defconfig +++ b/configs/da850evm_direct_nor_defconfig @@ -50,6 +50,7 @@ CONFIG_DM_SPI_FLASH=y CONFIG_SPI_FLASH=y CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_PINCTRL=y diff --git a/configs/da850evm_nand_defconfig b/configs/da850evm_nand_defconfig index b8eac0e659..eaf5d73dd8 100644 --- a/configs/da850evm_nand_defconfig +++ b/configs/da850evm_nand_defconfig @@ -60,6 +60,7 @@ CONFIG_SPI_FLASH=y CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y CONFIG_SPI_FLASH_MTD=y +CONFIG_DM_ETH=y CONFIG_PINCTRL=y CONFIG_PINCTRL_SINGLE=y CONFIG_DM_SERIAL=y diff --git a/configs/omapl138_lcdk_defconfig b/configs/omapl138_lcdk_defconfig index e43141844a..f93a06083e 100644 --- a/configs/omapl138_lcdk_defconfig +++ b/configs/omapl138_lcdk_defconfig @@ -50,6 +50,7 @@ CONFIG_SPI_FLASH=y CONFIG_SF_DEFAULT_SPEED=30000000 CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_DM_SERIAL=y diff --git a/configs/ti816x_evm_defconfig b/configs/ti816x_evm_defconfig index bf877f596b..b233ab8cc9 100644 --- a/configs/ti816x_evm_defconfig +++ b/configs/ti816x_evm_defconfig @@ -45,6 +45,7 @@ CONFIG_SYS_I2C_OMAP24XX=y CONFIG_MMC_OMAP_HS=y CONFIG_NAND=y CONFIG_SYS_NAND_BUSWIDTH_16BIT=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_SYS_NS16550=y diff --git a/drivers/net/ti/davinci_emac.c b/drivers/net/ti/davinci_emac.c index 9d53984973..4b0a98a4aa 100644 --- a/drivers/net/ti/davinci_emac.c +++ b/drivers/net/ti/davinci_emac.c @@ -26,7 +26,6 @@ #include <net.h> #include <miiphy.h> #include <malloc.h> -#include <netdev.h> #include <linux/compiler.h> #include <asm/arch/emac_defs.h> #include <asm/io.h> @@ -107,8 +106,9 @@ static u_int8_t num_phy;

phy_t phy[CONFIG_SYS_DAVINCI_EMAC_PHY_COUNT];

-static int davinci_eth_set_mac_addr(struct eth_device *dev) +static int davinci_emac_write_hwaddr(struct udevice *dev) {

•   struct eth_pdata *pdata = dev_get_platdata(dev);
    unsigned long           mac_hi;
    unsigned long           mac_lo;
  
  

@@ -118,12 +118,12 @@ static int davinci_eth_set_mac_addr(struct eth_device *dev) * Using channel 0 only - other channels are disabled * */ writel(0, &adap_emac->MACINDEX);

•   mac_hi = (dev->enetaddr[3] << 24) |
  

•            (dev->enetaddr[2] << 16) |
  

•            (dev->enetaddr[1] << 8)  |
  

•            (dev->enetaddr[0]);
  

•   mac_lo = (dev->enetaddr[5] << 8) |
  

•            (dev->enetaddr[4]);
  

•   mac_hi = (pdata->enetaddr[3] << 24) |
  

•            (pdata->enetaddr[2] << 16) |
  

•            (pdata->enetaddr[1] << 8)  |
  

•            (pdata->enetaddr[0]);
  

•   mac_lo = (pdata->enetaddr[5] << 8) |
  

•            (pdata->enetaddr[4]);
  
    writel(mac_hi, &adap_emac->MACADDRHI);
  

#if defined(DAVINCI_EMAC_VERSION2) @@ -411,7 +411,7 @@ static void __attribute__((unused)) davinci_eth_gigabit_enable(int phy_addr) }

/* Eth device open */ -static int davinci_eth_open(struct eth_device *dev, bd_t *bis) +static int davinci_emac_start(struct udevice *dev) { dv_reg_p addr; u_int32_t clkdiv, cnt, mac_control; @@ -447,7 +447,7 @@ static int davinci_eth_open(struct eth_device *dev, bd_t *bis) writel(1, &adap_emac->TXCONTROL); writel(1, &adap_emac->RXCONTROL);

•   davinci_eth_set_mac_addr(dev);
  

•   davinci_emac_write_hwaddr(dev);
  
    /* Set DMA 8 TX / 8 RX Head pointers to 0 */
    addr = &adap_emac->TX0HDP;
  

@@ -588,7 +588,7 @@ static void davinci_eth_ch_teardown(int ch) }

/* Eth device close */ -static void davinci_eth_close(struct eth_device *dev) +static void davinci_emac_stop(struct udevice *dev) { debug_emac("+ emac_close\n");

@@ -619,8 +619,8 @@ static int tx_send_loop = 0;

• This function sends a single packet on the network and returns
• positive number (number of bytes transmitted) or negative for error

*/ -static int davinci_eth_send_packet (struct eth_device *dev,

•                                   void *packet, int length)
  

+static int davinci_emac_send(struct udevice *dev,

•                        void *packet, int length)
  

{ int ret_status = -1; int index; @@ -672,7 +672,7 @@ static int davinci_eth_send_packet (struct eth_device *dev, /*

• This function handles receipt of a packet from the network

*/ -static int davinci_eth_rcv_packet (struct eth_device *dev) +static int davinci_emac_recv(struct udevice *dev, int flags, uchar **packetp) { volatile emac_desc *rx_curr_desc; volatile emac_desc *curr_desc; @@ -742,6 +742,9 @@ static int davinci_eth_rcv_packet (struct eth_device *dev) }

To fix my crashing on the am3517, I had to add another instance here. *packetp = rx_curr_desc->buffer;

...

...

...

...

            return (ret);
    }

I am not familiar enough with the driver to know why it works, so if it's wrong, we'll need to diagnose it further, but I am able to reliably connect with the am3517 now and I tested my da850-evm and it's still working as well.

adam

...

...

...

...

•   *packetp = rx_curr_desc->buffer;
  

•   return (0);
  

}

@@ -750,30 +753,12 @@ static int davinci_eth_rcv_packet (struct eth_device *dev)

• EMAC modules power or pin multiplexors, that is done by board_init()
• much earlier in bootup process. Returns 1 on success, 0 otherwise.

*/ -int davinci_emac_initialize(void) +static int davinci_emac_probe(struct udevice *dev)

From the device tree, we should be able to remove the hard-coded values referencing:

EMAC_BASE_ADDR; EMAC_WRAPPER_BASE_ADDR;

/* EMAC descriptors */ EMAC_WRAPPER_RAM_ADDR + EMAC_RX_DESC_BASE); EMAC_WRAPPER_RAM_ADDR + EMAC_TX_DESC_BASE);

From the device tree, EMAC_BASE_ADDR should point to the address of the emac node. In the case of the AM3517, it would be 5c000000 and this is how the name appears, so we should be able to use that. EMAC_WRAPPER_BASE_ADDR should the also read by the device tree as an offset from the base address by reading "ti,davinci-ctrl-reg-offset" and "ti,davinci-ctrl-mod-reg-offset" in the case of AM3517, 0x5c000000

• 0x10000 + 0 give the same correct address as the

EMAC_WRAPPER_BASE_ADDR currently does.

The EMAC_WRAPPER_RAM_ADDR is read by reading the "ti,davinci-ctrl-ram-offset" which for am3517 is 0x20000 and 5c000000

• 0x20000 matches the value currenty set for 0x5C020000.

I may have some of the math wrong a bit, but the linux driver doc explains how these tree entries are defined, and I would think the emac driver could be modeled after the Linux implementation.

I think we should try to extract these and calculator the addresses this way we can remove the hard-coded values and make the driver more generic.

Definitely and it's on my list, but first I wanted to do the conversion, make sure it works and then gradually improve the code.

Best regards, Bartosz Golaszewski

...

I am going to bed now, but I'll try to beat on the da850 on Sunday.

adam

...

{ u_int32_t phy_id; u_int16_t tmp; int i; int ret;

•   struct eth_device *dev;
  

•   dev = malloc(sizeof *dev);
  

•   if (dev == NULL)
  

•           return -1;
  

•   memset(dev, 0, sizeof *dev);
  

•   strcpy(dev->name, "DaVinci-EMAC");
  

•   dev->iobase = 0;
  

•   dev->init = davinci_eth_open;
  

•   dev->halt = davinci_eth_close;
  

•   dev->send = davinci_eth_send_packet;
  

•   dev->recv = davinci_eth_rcv_packet;
  

•   dev->write_hwaddr = davinci_eth_set_mac_addr;
  

•   eth_register(dev);
  
    davinci_eth_mdio_enable();
  
  

@@ -854,5 +839,29 @@ int davinci_emac_initialize(void) phy[i].auto_negotiate(i); } #endif

•   return(1);
  

•   return 0;
  

}

+static const struct eth_ops davinci_emac_ops = {

•   .start          = davinci_emac_start,
  

•   .send           = davinci_emac_send,
  

•   .recv           = davinci_emac_recv,
  

•   .stop           = davinci_emac_stop,
  

•   .write_hwaddr   = davinci_emac_write_hwaddr,
  

+};

+static const struct udevice_id davinci_emac_ids[] = {

•   { .compatible = "ti,davinci-dm6467-emac" },
  

•   { .compatible = "ti,am3517-emac", },
  

•   { .compatible = "ti,dm816-emac", },
  

•   { }
  

+};

+U_BOOT_DRIVER(davinci_emac) = {

•   .name           = "davinci_emac",
  

•   .id             = UCLASS_ETH,
  

•   .of_match       = davinci_emac_ids,
  

•   .probe          = davinci_emac_probe,
  

•   .ops            = &davinci_emac_ops,
  

•   .platdata_auto_alloc_size = sizeof(struct eth_pdata),
  

+}; diff --git a/include/netdev.h b/include/netdev.h index 0a1a3a2d8d..a40c4adaad 100644 --- a/include/netdev.h +++ b/include/netdev.h @@ -30,7 +30,6 @@ int bcm_sf2_eth_register(bd_t *bis, u8 dev_num); int bfin_EMAC_initialize(bd_t *bis); int calxedaxgmac_initialize(u32 id, ulong base_addr); int cs8900_initialize(u8 dev_num, int base_addr); -int davinci_emac_initialize(void); int dc21x4x_initialize(bd_t *bis); int designware_initialize(ulong base_addr, u32 interface); int dm9000_initialize(bd_t *bis); -- 2.21.0

śr., 12 cze 2019 o 18:46 Adam Ford aford173@gmail.com napisał(a):

On Mon, Jun 3, 2019 at 10:29 AM Bartosz Golaszewski brgl@bgdev.pl wrote:

...

pon., 3 cze 2019 o 15:03 Adam Ford aford173@gmail.com napisał(a):

...

On Mon, Jun 3, 2019 at 3:12 AM Bartosz Golaszewski brgl@bgdev.pl wrote:

...

sob., 1 cze 2019 o 05:24 Adam Ford aford173@gmail.com napisał(a):

...

On Fri, May 31, 2019 at 8:32 AM Bartosz Golaszewski brgl@bgdev.pl wrote:

...

From: Bartosz Golaszewski bgolaszewski@baylibre.com

Now that we removed all legacy boards selecting TI_EMAC we can completely convert the driver code to using the driver model. This patch also updates all remaining users of davinci_emac.

I took a break from this to come back, and I'm going to give some feedback about how the driver was written. I still do not know why I cannot get an IP address with this patch on the AM3517-evm.

Hi Adam,

thanks for all the testing. Unfortunately I can only test with da850-evm and da850-lcdk.

I was wondering if it is possible that the problem is caused by cpu_eth_init() from ./arch/arm/mach-omap2/omap3/emac. not being called with CONFIG_DM_ETH? The comments say that it brings the module out of reset, so maybe this is what causes the problem you're seeing?

I looked into that nearly right away, but there is a chunk of code in board/logicpd/am3517evm/am3517evm.c which has a function called misc_init_r() which does the same thing. Looking through the debug data, it appears as if the drive is communicating for a bit, but at some point it dies. I'm not going to be able to look at it for a few days. Is there any way you can submit a V2 to use #ifdef's to allow users to use the same driver with and without DM_ETH? That would give the da850/L138 boards the DM_ETH, and give me some time to troubleshoot the am3517-evm. I know Sekhar works for TI and the AM3517 was the official development kit for TI back in the day. If TI has some ideas, I'm open to trying them as well when I can get back to it.

adam

Hi Adam,

I'm trying to find out if I can get my hands on one of these boards somehow.

My priority for this week is the nand driver conversion. I'll also be off next week and the one after. I think that since we already removed a bunch of boards to make it possible to avoid the ifdef hell, it's better to wait a bit more than to merge something that we'll remove soon anyway.

If Sekhar agrees, we can push back merging of this patch until it's fixed.

There's also a big backlog of my other davinci patches on the list anyway, maybe they'll get picked up before we get back to it.

Bart

...

...

...

...

Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com

arch/arm/mach-davinci/cpu.c | 13 ----- arch/arm/mach-omap2/omap3/emac.c | 3 +- board/davinci/da8xxevm/da850evm.c | 6 -- board/davinci/da8xxevm/omapl138_lcdk.c | 14 ----- board/logicpd/am3517evm/am3517evm.c | 1 - board/ti/ti816x/evm.c | 3 +- configs/am3517_evm_defconfig | 1 + configs/da850evm_defconfig | 1 + configs/da850evm_direct_nor_defconfig | 1 + configs/da850evm_nand_defconfig | 1 + configs/omapl138_lcdk_defconfig | 1 + configs/ti816x_evm_defconfig | 1 + drivers/net/ti/davinci_emac.c | 77 ++++++++++++++------------ include/netdev.h | 1 - 14 files changed, 51 insertions(+), 73 deletions(-)

diff --git a/arch/arm/mach-davinci/cpu.c b/arch/arm/mach-davinci/cpu.c index f97ad3fc74..9fd6564d04 100644 --- a/arch/arm/mach-davinci/cpu.c +++ b/arch/arm/mach-davinci/cpu.c @@ -5,7 +5,6 @@ */

#include <common.h> -#include <netdev.h> #include <asm/arch/hardware.h> #include <asm/io.h>

@@ -90,15 +89,3 @@ int set_cpu_clk_info(void) gd->bd->bi_dsp_freq = 0; return 0; }

-/*

• • Initializes on-chip ethernet controllers.
• • to override, implement board_eth_init()
• */

-int cpu_eth_init(bd_t *bis) -{ -#if defined(CONFIG_DRIVER_TI_EMAC)

•   davinci_emac_initialize();
  

-#endif

•   return 0;
  

-} diff --git a/arch/arm/mach-omap2/omap3/emac.c b/arch/arm/mach-omap2/omap3/emac.c index c79e870183..fb0c9188f5 100644 --- a/arch/arm/mach-omap2/omap3/emac.c +++ b/arch/arm/mach-omap2/omap3/emac.c @@ -7,7 +7,6 @@ */

#include <common.h> -#include <netdev.h> #include <asm/io.h> #include <asm/arch/am35x_def.h>

@@ -24,5 +23,5 @@ int cpu_eth_init(bd_t *bis) reset &= ~CPGMACSS_SW_RST; writel(reset, &am35x_scm_general_regs->ip_sw_reset);

•   return davinci_emac_initialize();
  

•   return 0;
  

} diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c index 1bc26828bf..d090c00466 100644 --- a/board/davinci/da8xxevm/da850evm.c +++ b/board/davinci/da8xxevm/da850evm.c @@ -13,7 +13,6 @@ #include <environment.h> #include <i2c.h> #include <net.h> -#include <netdev.h> #include <spi.h> #include <spi_flash.h> #include <asm/arch/hardware.h> @@ -472,11 +471,6 @@ int board_eth_init(bd_t *bis) if (rmii_hw_init()) printf("RMII hardware init failed!!!\n"); #endif

•   if (!davinci_emac_initialize()) {
  

•           printf("Error: Ethernet init failed!\n");
  

•           return -1;
  

•   }
  

•   return 0;
  

} #endif /* CONFIG_DRIVER_TI_EMAC */ diff --git a/board/davinci/da8xxevm/omapl138_lcdk.c b/board/davinci/da8xxevm/omapl138_lcdk.c index 2c2f885d43..ef9656add8 100644 --- a/board/davinci/da8xxevm/omapl138_lcdk.c +++ b/board/davinci/da8xxevm/omapl138_lcdk.c @@ -11,7 +11,6 @@ #include <common.h> #include <i2c.h> #include <net.h> -#include <netdev.h> #include <spi.h> #include <spi_flash.h> #include <asm/arch/hardware.h> @@ -229,19 +228,6 @@ int board_init(void)

#ifdef CONFIG_DRIVER_TI_EMAC

-/*

• • Initializes on-board ethernet controllers.
• */

-int board_eth_init(bd_t *bis) -{

•   if (!davinci_emac_initialize()) {
  

•           printf("Error: Ethernet init failed!\n");
  

•           return -1;
  

•   }
  

•   return 0;
  

-}

#endif /* CONFIG_DRIVER_TI_EMAC */

#define CFG_MAC_ADDR_SPI_BUS 0 diff --git a/board/logicpd/am3517evm/am3517evm.c b/board/logicpd/am3517evm/am3517evm.c index 10031a4801..bfd4e78274 100644 --- a/board/logicpd/am3517evm/am3517evm.c +++ b/board/logicpd/am3517evm/am3517evm.c @@ -28,7 +28,6 @@ #include <linux/usb/gadget.h> #include <linux/usb/musb.h> #include <i2c.h> -#include <netdev.h> #include "am3517evm.h"

DECLARE_GLOBAL_DATA_PTR; diff --git a/board/ti/ti816x/evm.c b/board/ti/ti816x/evm.c index 07a084bab8..240df8cbe1 100644 --- a/board/ti/ti816x/evm.c +++ b/board/ti/ti816x/evm.c @@ -9,7 +9,6 @@ #include <common.h> #include <environment.h> #include <spl.h> -#include <netdev.h> #include <asm/cache.h> #include <asm/io.h> #include <asm/arch/clock.h> @@ -56,7 +55,7 @@ int board_eth_init(bd_t *bis) printf("Unable to read MAC address. Set <ethaddr>\n"); }

•   return davinci_emac_initialize();
  

•   return 0;
  

}

#ifdef CONFIG_SPL_BUILD diff --git a/configs/am3517_evm_defconfig b/configs/am3517_evm_defconfig index b9f59f3291..5cb76322df 100644 --- a/configs/am3517_evm_defconfig +++ b/configs/am3517_evm_defconfig @@ -44,6 +44,7 @@ CONFIG_SYS_NAND_BUSWIDTH_16BIT=y CONFIG_SYS_NAND_U_BOOT_LOCATIONS=y CONFIG_SYS_NAND_U_BOOT_OFFS=0x80000 CONFIG_SPL_NAND_SIMPLE=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_PINCTRL=y diff --git a/configs/da850evm_defconfig b/configs/da850evm_defconfig index 8c16d5c4f5..ca304ed78a 100644 --- a/configs/da850evm_defconfig +++ b/configs/da850evm_defconfig @@ -60,6 +60,7 @@ CONFIG_SF_DEFAULT_SPEED=30000000 CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y CONFIG_SPI_FLASH_MTD=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_PINCTRL=y diff --git a/configs/da850evm_direct_nor_defconfig b/configs/da850evm_direct_nor_defconfig index 166e77b8e3..9b1da07384 100644 --- a/configs/da850evm_direct_nor_defconfig +++ b/configs/da850evm_direct_nor_defconfig @@ -50,6 +50,7 @@ CONFIG_DM_SPI_FLASH=y CONFIG_SPI_FLASH=y CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_PINCTRL=y diff --git a/configs/da850evm_nand_defconfig b/configs/da850evm_nand_defconfig index b8eac0e659..eaf5d73dd8 100644 --- a/configs/da850evm_nand_defconfig +++ b/configs/da850evm_nand_defconfig @@ -60,6 +60,7 @@ CONFIG_SPI_FLASH=y CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y CONFIG_SPI_FLASH_MTD=y +CONFIG_DM_ETH=y CONFIG_PINCTRL=y CONFIG_PINCTRL_SINGLE=y CONFIG_DM_SERIAL=y diff --git a/configs/omapl138_lcdk_defconfig b/configs/omapl138_lcdk_defconfig index e43141844a..f93a06083e 100644 --- a/configs/omapl138_lcdk_defconfig +++ b/configs/omapl138_lcdk_defconfig @@ -50,6 +50,7 @@ CONFIG_SPI_FLASH=y CONFIG_SF_DEFAULT_SPEED=30000000 CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_DM_SERIAL=y diff --git a/configs/ti816x_evm_defconfig b/configs/ti816x_evm_defconfig index bf877f596b..b233ab8cc9 100644 --- a/configs/ti816x_evm_defconfig +++ b/configs/ti816x_evm_defconfig @@ -45,6 +45,7 @@ CONFIG_SYS_I2C_OMAP24XX=y CONFIG_MMC_OMAP_HS=y CONFIG_NAND=y CONFIG_SYS_NAND_BUSWIDTH_16BIT=y +CONFIG_DM_ETH=y CONFIG_MII=y CONFIG_DRIVER_TI_EMAC=y CONFIG_SYS_NS16550=y diff --git a/drivers/net/ti/davinci_emac.c b/drivers/net/ti/davinci_emac.c index 9d53984973..4b0a98a4aa 100644 --- a/drivers/net/ti/davinci_emac.c +++ b/drivers/net/ti/davinci_emac.c @@ -26,7 +26,6 @@ #include <net.h> #include <miiphy.h> #include <malloc.h> -#include <netdev.h> #include <linux/compiler.h> #include <asm/arch/emac_defs.h> #include <asm/io.h> @@ -107,8 +106,9 @@ static u_int8_t num_phy;

phy_t phy[CONFIG_SYS_DAVINCI_EMAC_PHY_COUNT];

-static int davinci_eth_set_mac_addr(struct eth_device *dev) +static int davinci_emac_write_hwaddr(struct udevice *dev) {

•   struct eth_pdata *pdata = dev_get_platdata(dev);
    unsigned long           mac_hi;
    unsigned long           mac_lo;
  
  

@@ -118,12 +118,12 @@ static int davinci_eth_set_mac_addr(struct eth_device *dev) * Using channel 0 only - other channels are disabled * */ writel(0, &adap_emac->MACINDEX);

•   mac_hi = (dev->enetaddr[3] << 24) |
  

•            (dev->enetaddr[2] << 16) |
  

•            (dev->enetaddr[1] << 8)  |
  

•            (dev->enetaddr[0]);
  

•   mac_lo = (dev->enetaddr[5] << 8) |
  

•            (dev->enetaddr[4]);
  

•   mac_hi = (pdata->enetaddr[3] << 24) |
  

•            (pdata->enetaddr[2] << 16) |
  

•            (pdata->enetaddr[1] << 8)  |
  

•            (pdata->enetaddr[0]);
  

•   mac_lo = (pdata->enetaddr[5] << 8) |
  

•            (pdata->enetaddr[4]);
  
    writel(mac_hi, &adap_emac->MACADDRHI);
  

#if defined(DAVINCI_EMAC_VERSION2) @@ -411,7 +411,7 @@ static void __attribute__((unused)) davinci_eth_gigabit_enable(int phy_addr) }

/* Eth device open */ -static int davinci_eth_open(struct eth_device *dev, bd_t *bis) +static int davinci_emac_start(struct udevice *dev) { dv_reg_p addr; u_int32_t clkdiv, cnt, mac_control; @@ -447,7 +447,7 @@ static int davinci_eth_open(struct eth_device *dev, bd_t *bis) writel(1, &adap_emac->TXCONTROL); writel(1, &adap_emac->RXCONTROL);

•   davinci_eth_set_mac_addr(dev);
  

•   davinci_emac_write_hwaddr(dev);
  
    /* Set DMA 8 TX / 8 RX Head pointers to 0 */
    addr = &adap_emac->TX0HDP;
  

@@ -588,7 +588,7 @@ static void davinci_eth_ch_teardown(int ch) }

/* Eth device close */ -static void davinci_eth_close(struct eth_device *dev) +static void davinci_emac_stop(struct udevice *dev) { debug_emac("+ emac_close\n");

@@ -619,8 +619,8 @@ static int tx_send_loop = 0;

• This function sends a single packet on the network and returns
• positive number (number of bytes transmitted) or negative for error

*/ -static int davinci_eth_send_packet (struct eth_device *dev,

•                                   void *packet, int length)
  

+static int davinci_emac_send(struct udevice *dev,

•                        void *packet, int length)
  

{ int ret_status = -1; int index; @@ -672,7 +672,7 @@ static int davinci_eth_send_packet (struct eth_device *dev, /*

• This function handles receipt of a packet from the network

*/ -static int davinci_eth_rcv_packet (struct eth_device *dev) +static int davinci_emac_recv(struct udevice *dev, int flags, uchar **packetp) { volatile emac_desc *rx_curr_desc; volatile emac_desc *curr_desc; @@ -742,6 +742,9 @@ static int davinci_eth_rcv_packet (struct eth_device *dev) }

To fix my crashing on the am3517, I had to add another instance here. *packetp = rx_curr_desc->buffer;

...

...

...

...

...

            return (ret);
    }

I am not familiar enough with the driver to know why it works, so if it's wrong, we'll need to diagnose it further, but I am able to reliably connect with the am3517 now and I tested my da850-evm and it's still working as well.

adam

I have no idea why, but while this tweak works, it also breaks the formatting of the tftp progress bar - it removes the line wrapping after 66 characters.

I'm trying to figure out why.

Bart

...

...

...

...

...

•   *packetp = rx_curr_desc->buffer;
  

•   return (0);
  

}

@@ -750,30 +753,12 @@ static int davinci_eth_rcv_packet (struct eth_device *dev)

• EMAC modules power or pin multiplexors, that is done by board_init()
• much earlier in bootup process. Returns 1 on success, 0 otherwise.

*/ -int davinci_emac_initialize(void) +static int davinci_emac_probe(struct udevice *dev)

From the device tree, we should be able to remove the hard-coded values referencing:

EMAC_BASE_ADDR; EMAC_WRAPPER_BASE_ADDR;

/* EMAC descriptors */ EMAC_WRAPPER_RAM_ADDR + EMAC_RX_DESC_BASE); EMAC_WRAPPER_RAM_ADDR + EMAC_TX_DESC_BASE);

From the device tree, EMAC_BASE_ADDR should point to the address of the emac node. In the case of the AM3517, it would be 5c000000 and this is how the name appears, so we should be able to use that. EMAC_WRAPPER_BASE_ADDR should the also read by the device tree as an offset from the base address by reading "ti,davinci-ctrl-reg-offset" and "ti,davinci-ctrl-mod-reg-offset" in the case of AM3517, 0x5c000000

• 0x10000 + 0 give the same correct address as the

EMAC_WRAPPER_BASE_ADDR currently does.

The EMAC_WRAPPER_RAM_ADDR is read by reading the "ti,davinci-ctrl-ram-offset" which for am3517 is 0x20000 and 5c000000

• 0x20000 matches the value currenty set for 0x5C020000.

I may have some of the math wrong a bit, but the linux driver doc explains how these tree entries are defined, and I would think the emac driver could be modeled after the Linux implementation.

I think we should try to extract these and calculator the addresses this way we can remove the hard-coded values and make the driver more generic.

Definitely and it's on my list, but first I wanted to do the conversion, make sure it works and then gradually improve the code.

Best regards, Bartosz Golaszewski

...

I am going to bed now, but I'll try to beat on the da850 on Sunday.

adam

...

{ u_int32_t phy_id; u_int16_t tmp; int i; int ret;

•   struct eth_device *dev;
  

•   dev = malloc(sizeof *dev);
  

•   if (dev == NULL)
  

•           return -1;
  

•   memset(dev, 0, sizeof *dev);
  

•   strcpy(dev->name, "DaVinci-EMAC");
  

•   dev->iobase = 0;
  

•   dev->init = davinci_eth_open;
  

•   dev->halt = davinci_eth_close;
  

•   dev->send = davinci_eth_send_packet;
  

•   dev->recv = davinci_eth_rcv_packet;
  

•   dev->write_hwaddr = davinci_eth_set_mac_addr;
  

•   eth_register(dev);
  
    davinci_eth_mdio_enable();
  
  

@@ -854,5 +839,29 @@ int davinci_emac_initialize(void) phy[i].auto_negotiate(i); } #endif

•   return(1);
  

•   return 0;
  

}

+static const struct eth_ops davinci_emac_ops = {

•   .start          = davinci_emac_start,
  

•   .send           = davinci_emac_send,
  

•   .recv           = davinci_emac_recv,
  

•   .stop           = davinci_emac_stop,
  

•   .write_hwaddr   = davinci_emac_write_hwaddr,
  

+};

+static const struct udevice_id davinci_emac_ids[] = {

•   { .compatible = "ti,davinci-dm6467-emac" },
  

•   { .compatible = "ti,am3517-emac", },
  

•   { .compatible = "ti,dm816-emac", },
  

•   { }
  

+};

+U_BOOT_DRIVER(davinci_emac) = {

•   .name           = "davinci_emac",
  

•   .id             = UCLASS_ETH,
  

•   .of_match       = davinci_emac_ids,
  

•   .probe          = davinci_emac_probe,
  

•   .ops            = &davinci_emac_ops,
  

•   .platdata_auto_alloc_size = sizeof(struct eth_pdata),
  

+}; diff --git a/include/netdev.h b/include/netdev.h index 0a1a3a2d8d..a40c4adaad 100644 --- a/include/netdev.h +++ b/include/netdev.h @@ -30,7 +30,6 @@ int bcm_sf2_eth_register(bd_t *bis, u8 dev_num); int bfin_EMAC_initialize(bd_t *bis); int calxedaxgmac_initialize(u32 id, ulong base_addr); int cs8900_initialize(u8 dev_num, int base_addr); -int davinci_emac_initialize(void); int dc21x4x_initialize(bd_t *bis); int designware_initialize(ulong base_addr, u32 interface); int dm9000_initialize(bd_t *bis); -- 2.21.0

pon., 24 cze 2019 o 15:39 Adam Ford aford173@gmail.com napisał(a):

On Mon, Jun 24, 2019 at 7:07 AM Bartosz Golaszewski brgl@bgdev.pl wrote:

...

śr., 12 cze 2019 o 18:46 Adam Ford aford173@gmail.com napisał(a):

...

On Mon, Jun 3, 2019 at 10:29 AM Bartosz Golaszewski brgl@bgdev.pl wrote:

...

pon., 3 cze 2019 o 15:03 Adam Ford aford173@gmail.com napisał(a):

...

On Mon, Jun 3, 2019 at 3:12 AM Bartosz Golaszewski brgl@bgdev.pl wrote:

...

sob., 1 cze 2019 o 05:24 Adam Ford aford173@gmail.com napisał(a): > > On Fri, May 31, 2019 at 8:32 AM Bartosz Golaszewski brgl@bgdev.pl wrote: > > > > From: Bartosz Golaszewski bgolaszewski@baylibre.com > > > > Now that we removed all legacy boards selecting TI_EMAC we can > > completely convert the driver code to using the driver model. > > This patch also updates all remaining users of davinci_emac. > > > > I took a break from this to come back, and I'm going to give some > feedback about how the driver was written. I still do not know why I > cannot get an IP address with this patch on the AM3517-evm. >

Hi Adam,

thanks for all the testing. Unfortunately I can only test with da850-evm and da850-lcdk.

I was wondering if it is possible that the problem is caused by cpu_eth_init() from ./arch/arm/mach-omap2/omap3/emac. not being called with CONFIG_DM_ETH? The comments say that it brings the module out of reset, so maybe this is what causes the problem you're seeing?

I looked into that nearly right away, but there is a chunk of code in board/logicpd/am3517evm/am3517evm.c which has a function called misc_init_r() which does the same thing. Looking through the debug data, it appears as if the drive is communicating for a bit, but at some point it dies. I'm not going to be able to look at it for a few days. Is there any way you can submit a V2 to use #ifdef's to allow users to use the same driver with and without DM_ETH? That would give the da850/L138 boards the DM_ETH, and give me some time to troubleshoot the am3517-evm. I know Sekhar works for TI and the AM3517 was the official development kit for TI back in the day. If TI has some ideas, I'm open to trying them as well when I can get back to it.

adam

Hi Adam,

I'm trying to find out if I can get my hands on one of these boards somehow.

My priority for this week is the nand driver conversion. I'll also be off next week and the one after. I think that since we already removed a bunch of boards to make it possible to avoid the ifdef hell, it's better to wait a bit more than to merge something that we'll remove soon anyway.

If Sekhar agrees, we can push back merging of this patch until it's fixed.

There's also a big backlog of my other davinci patches on the list anyway, maybe they'll get picked up before we get back to it.

Bart

...

...

> > Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com > > --- > > arch/arm/mach-davinci/cpu.c | 13 ----- > > arch/arm/mach-omap2/omap3/emac.c | 3 +- > > board/davinci/da8xxevm/da850evm.c | 6 -- > > board/davinci/da8xxevm/omapl138_lcdk.c | 14 ----- > > board/logicpd/am3517evm/am3517evm.c | 1 - > > board/ti/ti816x/evm.c | 3 +- > > configs/am3517_evm_defconfig | 1 + > > configs/da850evm_defconfig | 1 + > > configs/da850evm_direct_nor_defconfig | 1 + > > configs/da850evm_nand_defconfig | 1 + > > configs/omapl138_lcdk_defconfig | 1 + > > configs/ti816x_evm_defconfig | 1 + > > drivers/net/ti/davinci_emac.c | 77 ++++++++++++++------------ > > include/netdev.h | 1 - > > 14 files changed, 51 insertions(+), 73 deletions(-) > > > > diff --git a/arch/arm/mach-davinci/cpu.c b/arch/arm/mach-davinci/cpu.c > > index f97ad3fc74..9fd6564d04 100644 > > --- a/arch/arm/mach-davinci/cpu.c > > +++ b/arch/arm/mach-davinci/cpu.c > > @@ -5,7 +5,6 @@ > > */ > > > > #include <common.h> > > -#include <netdev.h> > > #include <asm/arch/hardware.h> > > #include <asm/io.h> > > > > @@ -90,15 +89,3 @@ int set_cpu_clk_info(void) > > gd->bd->bi_dsp_freq = 0; > > return 0; > > } > > - > > -/* > > - * Initializes on-chip ethernet controllers. > > - * to override, implement board_eth_init() > > - */ > > -int cpu_eth_init(bd_t *bis) > > -{ > > -#if defined(CONFIG_DRIVER_TI_EMAC) > > - davinci_emac_initialize(); > > -#endif > > - return 0; > > -} > > diff --git a/arch/arm/mach-omap2/omap3/emac.c b/arch/arm/mach-omap2/omap3/emac.c > > index c79e870183..fb0c9188f5 100644 > > --- a/arch/arm/mach-omap2/omap3/emac.c > > +++ b/arch/arm/mach-omap2/omap3/emac.c > > @@ -7,7 +7,6 @@ > > */ > > > > #include <common.h> > > -#include <netdev.h> > > #include <asm/io.h> > > #include <asm/arch/am35x_def.h> > > > > @@ -24,5 +23,5 @@ int cpu_eth_init(bd_t *bis) > > reset &= ~CPGMACSS_SW_RST; > > writel(reset, &am35x_scm_general_regs->ip_sw_reset); > > > > - return davinci_emac_initialize(); > > + return 0; > > } > > diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c > > index 1bc26828bf..d090c00466 100644 > > --- a/board/davinci/da8xxevm/da850evm.c > > +++ b/board/davinci/da8xxevm/da850evm.c > > @@ -13,7 +13,6 @@ > > #include <environment.h> > > #include <i2c.h> > > #include <net.h> > > -#include <netdev.h> > > #include <spi.h> > > #include <spi_flash.h> > > #include <asm/arch/hardware.h> > > @@ -472,11 +471,6 @@ int board_eth_init(bd_t *bis) > > if (rmii_hw_init()) > > printf("RMII hardware init failed!!!\n"); > > #endif > > - if (!davinci_emac_initialize()) { > > - printf("Error: Ethernet init failed!\n"); > > - return -1; > > - } > > - > > return 0; > > } > > #endif /* CONFIG_DRIVER_TI_EMAC */ > > diff --git a/board/davinci/da8xxevm/omapl138_lcdk.c b/board/davinci/da8xxevm/omapl138_lcdk.c > > index 2c2f885d43..ef9656add8 100644 > > --- a/board/davinci/da8xxevm/omapl138_lcdk.c > > +++ b/board/davinci/da8xxevm/omapl138_lcdk.c > > @@ -11,7 +11,6 @@ > > #include <common.h> > > #include <i2c.h> > > #include <net.h> > > -#include <netdev.h> > > #include <spi.h> > > #include <spi_flash.h> > > #include <asm/arch/hardware.h> > > @@ -229,19 +228,6 @@ int board_init(void) > > > > #ifdef CONFIG_DRIVER_TI_EMAC > > > > -/* > > - * Initializes on-board ethernet controllers. > > - */ > > -int board_eth_init(bd_t *bis) > > -{ > > - if (!davinci_emac_initialize()) { > > - printf("Error: Ethernet init failed!\n"); > > - return -1; > > - } > > - > > - return 0; > > -} > > - > > #endif /* CONFIG_DRIVER_TI_EMAC */ > > > > #define CFG_MAC_ADDR_SPI_BUS 0 > > diff --git a/board/logicpd/am3517evm/am3517evm.c b/board/logicpd/am3517evm/am3517evm.c > > index 10031a4801..bfd4e78274 100644 > > --- a/board/logicpd/am3517evm/am3517evm.c > > +++ b/board/logicpd/am3517evm/am3517evm.c > > @@ -28,7 +28,6 @@ > > #include <linux/usb/gadget.h> > > #include <linux/usb/musb.h> > > #include <i2c.h> > > -#include <netdev.h> > > #include "am3517evm.h" > > > > DECLARE_GLOBAL_DATA_PTR; > > diff --git a/board/ti/ti816x/evm.c b/board/ti/ti816x/evm.c > > index 07a084bab8..240df8cbe1 100644 > > --- a/board/ti/ti816x/evm.c > > +++ b/board/ti/ti816x/evm.c > > @@ -9,7 +9,6 @@ > > #include <common.h> > > #include <environment.h> > > #include <spl.h> > > -#include <netdev.h> > > #include <asm/cache.h> > > #include <asm/io.h> > > #include <asm/arch/clock.h> > > @@ -56,7 +55,7 @@ int board_eth_init(bd_t *bis) > > printf("Unable to read MAC address. Set <ethaddr>\n"); > > } > > > > - return davinci_emac_initialize(); > > + return 0; > > } > > > > #ifdef CONFIG_SPL_BUILD > > diff --git a/configs/am3517_evm_defconfig b/configs/am3517_evm_defconfig > > index b9f59f3291..5cb76322df 100644 > > --- a/configs/am3517_evm_defconfig > > +++ b/configs/am3517_evm_defconfig > > @@ -44,6 +44,7 @@ CONFIG_SYS_NAND_BUSWIDTH_16BIT=y > > CONFIG_SYS_NAND_U_BOOT_LOCATIONS=y > > CONFIG_SYS_NAND_U_BOOT_OFFS=0x80000 > > CONFIG_SPL_NAND_SIMPLE=y > > +CONFIG_DM_ETH=y > > CONFIG_MII=y > > CONFIG_DRIVER_TI_EMAC=y > > CONFIG_PINCTRL=y > > diff --git a/configs/da850evm_defconfig b/configs/da850evm_defconfig > > index 8c16d5c4f5..ca304ed78a 100644 > > --- a/configs/da850evm_defconfig > > +++ b/configs/da850evm_defconfig > > @@ -60,6 +60,7 @@ CONFIG_SF_DEFAULT_SPEED=30000000 > > CONFIG_SPI_FLASH_STMICRO=y > > CONFIG_SPI_FLASH_WINBOND=y > > CONFIG_SPI_FLASH_MTD=y > > +CONFIG_DM_ETH=y > > CONFIG_MII=y > > CONFIG_DRIVER_TI_EMAC=y > > CONFIG_PINCTRL=y > > diff --git a/configs/da850evm_direct_nor_defconfig b/configs/da850evm_direct_nor_defconfig > > index 166e77b8e3..9b1da07384 100644 > > --- a/configs/da850evm_direct_nor_defconfig > > +++ b/configs/da850evm_direct_nor_defconfig > > @@ -50,6 +50,7 @@ CONFIG_DM_SPI_FLASH=y > > CONFIG_SPI_FLASH=y > > CONFIG_SPI_FLASH_STMICRO=y > > CONFIG_SPI_FLASH_WINBOND=y > > +CONFIG_DM_ETH=y > > CONFIG_MII=y > > CONFIG_DRIVER_TI_EMAC=y > > CONFIG_PINCTRL=y > > diff --git a/configs/da850evm_nand_defconfig b/configs/da850evm_nand_defconfig > > index b8eac0e659..eaf5d73dd8 100644 > > --- a/configs/da850evm_nand_defconfig > > +++ b/configs/da850evm_nand_defconfig > > @@ -60,6 +60,7 @@ CONFIG_SPI_FLASH=y > > CONFIG_SPI_FLASH_STMICRO=y > > CONFIG_SPI_FLASH_WINBOND=y > > CONFIG_SPI_FLASH_MTD=y > > +CONFIG_DM_ETH=y > > CONFIG_PINCTRL=y > > CONFIG_PINCTRL_SINGLE=y > > CONFIG_DM_SERIAL=y > > diff --git a/configs/omapl138_lcdk_defconfig b/configs/omapl138_lcdk_defconfig > > index e43141844a..f93a06083e 100644 > > --- a/configs/omapl138_lcdk_defconfig > > +++ b/configs/omapl138_lcdk_defconfig > > @@ -50,6 +50,7 @@ CONFIG_SPI_FLASH=y > > CONFIG_SF_DEFAULT_SPEED=30000000 > > CONFIG_SPI_FLASH_STMICRO=y > > CONFIG_SPI_FLASH_WINBOND=y > > +CONFIG_DM_ETH=y > > CONFIG_MII=y > > CONFIG_DRIVER_TI_EMAC=y > > CONFIG_DM_SERIAL=y > > diff --git a/configs/ti816x_evm_defconfig b/configs/ti816x_evm_defconfig > > index bf877f596b..b233ab8cc9 100644 > > --- a/configs/ti816x_evm_defconfig > > +++ b/configs/ti816x_evm_defconfig > > @@ -45,6 +45,7 @@ CONFIG_SYS_I2C_OMAP24XX=y > > CONFIG_MMC_OMAP_HS=y > > CONFIG_NAND=y > > CONFIG_SYS_NAND_BUSWIDTH_16BIT=y > > +CONFIG_DM_ETH=y > > CONFIG_MII=y > > CONFIG_DRIVER_TI_EMAC=y > > CONFIG_SYS_NS16550=y > > diff --git a/drivers/net/ti/davinci_emac.c b/drivers/net/ti/davinci_emac.c > > index 9d53984973..4b0a98a4aa 100644 > > --- a/drivers/net/ti/davinci_emac.c > > +++ b/drivers/net/ti/davinci_emac.c > > @@ -26,7 +26,6 @@ > > #include <net.h> > > #include <miiphy.h> > > #include <malloc.h> > > -#include <netdev.h> > > #include <linux/compiler.h> > > #include <asm/arch/emac_defs.h> > > #include <asm/io.h> > > @@ -107,8 +106,9 @@ static u_int8_t num_phy; > > > > phy_t phy[CONFIG_SYS_DAVINCI_EMAC_PHY_COUNT]; > > > > -static int davinci_eth_set_mac_addr(struct eth_device *dev) > > +static int davinci_emac_write_hwaddr(struct udevice *dev) > > { > > + struct eth_pdata *pdata = dev_get_platdata(dev); > > unsigned long mac_hi; > > unsigned long mac_lo; > > > > @@ -118,12 +118,12 @@ static int davinci_eth_set_mac_addr(struct eth_device *dev) > > * Using channel 0 only - other channels are disabled > > * */ > > writel(0, &adap_emac->MACINDEX); > > - mac_hi = (dev->enetaddr[3] << 24) | > > - (dev->enetaddr[2] << 16) | > > - (dev->enetaddr[1] << 8) | > > - (dev->enetaddr[0]); > > - mac_lo = (dev->enetaddr[5] << 8) | > > - (dev->enetaddr[4]); > > + mac_hi = (pdata->enetaddr[3] << 24) | > > + (pdata->enetaddr[2] << 16) | > > + (pdata->enetaddr[1] << 8) | > > + (pdata->enetaddr[0]); > > + mac_lo = (pdata->enetaddr[5] << 8) | > > + (pdata->enetaddr[4]); > > > > writel(mac_hi, &adap_emac->MACADDRHI); > > #if defined(DAVINCI_EMAC_VERSION2) > > @@ -411,7 +411,7 @@ static void __attribute__((unused)) davinci_eth_gigabit_enable(int phy_addr) > > } > > > > /* Eth device open */ > > -static int davinci_eth_open(struct eth_device *dev, bd_t *bis) > > +static int davinci_emac_start(struct udevice *dev) > > { > > dv_reg_p addr; > > u_int32_t clkdiv, cnt, mac_control; > > @@ -447,7 +447,7 @@ static int davinci_eth_open(struct eth_device *dev, bd_t *bis) > > writel(1, &adap_emac->TXCONTROL); > > writel(1, &adap_emac->RXCONTROL); > > > > - davinci_eth_set_mac_addr(dev); > > + davinci_emac_write_hwaddr(dev); > > > > /* Set DMA 8 TX / 8 RX Head pointers to 0 */ > > addr = &adap_emac->TX0HDP; > > @@ -588,7 +588,7 @@ static void davinci_eth_ch_teardown(int ch) > > } > > > > /* Eth device close */ > > -static void davinci_eth_close(struct eth_device *dev) > > +static void davinci_emac_stop(struct udevice *dev) > > { > > debug_emac("+ emac_close\n"); > > > > @@ -619,8 +619,8 @@ static int tx_send_loop = 0; > > * This function sends a single packet on the network and returns > > * positive number (number of bytes transmitted) or negative for error > > */ > > -static int davinci_eth_send_packet (struct eth_device *dev, > > - void *packet, int length) > > +static int davinci_emac_send(struct udevice *dev, > > + void *packet, int length) > > { > > int ret_status = -1; > > int index; > > @@ -672,7 +672,7 @@ static int davinci_eth_send_packet (struct eth_device *dev, > > /* > > * This function handles receipt of a packet from the network > > */ > > -static int davinci_eth_rcv_packet (struct eth_device *dev) > > +static int davinci_emac_recv(struct udevice *dev, int flags, uchar **packetp) > > { > > volatile emac_desc *rx_curr_desc; > > volatile emac_desc *curr_desc; > > @@ -742,6 +742,9 @@ static int davinci_eth_rcv_packet (struct eth_device *dev) > > }

To fix my crashing on the am3517, I had to add another instance here. *packetp = rx_curr_desc->buffer;

...

...

...

> > return (ret); > > }

I am not familiar enough with the driver to know why it works, so if it's wrong, we'll need to diagnose it further, but I am able to reliably connect with the am3517 now and I tested my da850-evm and it's still working as well.

adam

I have no idea why, but while this tweak works, it also breaks the formatting of the tftp progress bar - it removes the line wrapping after 66 characters.

I'm trying to figure out why.

Thanks for looking into that. I am back from my vacation as well, and I should be able to test AM3517 and DA850 patches when you have something. Feel free to send them, and I'll try to test them within a day or two. When they get mainlined, I might add some enhancements. I've already tested some, but I'm holding back until your initial stuff has been accepted. I am also hoping for the DM_MDIO stuff to be accepted, then we can start removing the hard-coded addressing.

adam

I think I found the culprit. The emac driver was calling net_process_received_packet() internally - it shouldn't do it if we're using the uclass driver since it's done in the core eth code.

I'll resend a patch for you to test shortly.

Bart

...

Bart

...

...

...

...

> > + > > + *packetp = rx_curr_desc->buffer; > > + > > return (0); > > } > > > > @@ -750,30 +753,12 @@ static int davinci_eth_rcv_packet (struct eth_device *dev) > > * EMAC modules power or pin multiplexors, that is done by board_init() > > * much earlier in bootup process. Returns 1 on success, 0 otherwise. > > */ > > -int davinci_emac_initialize(void) > > +static int davinci_emac_probe(struct udevice *dev) > > From the device tree, we should be able to remove the hard-coded > values referencing: > > EMAC_BASE_ADDR; > EMAC_WRAPPER_BASE_ADDR; > > /* EMAC descriptors */ > EMAC_WRAPPER_RAM_ADDR + EMAC_RX_DESC_BASE); > EMAC_WRAPPER_RAM_ADDR + EMAC_TX_DESC_BASE); > > From the device tree, EMAC_BASE_ADDR should point to the address of > the emac node. In the case of the AM3517, it would be 5c000000 and > this is how the name appears, so we should be able to use that. > EMAC_WRAPPER_BASE_ADDR should the also read by the device tree as an > offset from the base address by reading "ti,davinci-ctrl-reg-offset" > and "ti,davinci-ctrl-mod-reg-offset" in the case of AM3517, 0x5c000000 > + 0x10000 + 0 give the same correct address as the > EMAC_WRAPPER_BASE_ADDR currently does. > > The EMAC_WRAPPER_RAM_ADDR is read by reading the > "ti,davinci-ctrl-ram-offset" which for am3517 is 0x20000 and 5c000000 > + 0x20000 matches the value currenty set for 0x5C020000. > > I may have some of the math wrong a bit, but the linux driver doc > explains how these tree entries are defined, and I would think the > emac driver could be modeled after the Linux implementation. > > I think we should try to extract these and calculator the addresses > this way we can remove the hard-coded values and make the driver more > generic.

Definitely and it's on my list, but first I wanted to do the conversion, make sure it works and then gradually improve the code.

Best regards, Bartosz Golaszewski

> I am going to bed now, but I'll try to beat on the da850 on Sunday. > > adam > > { > > u_int32_t phy_id; > > u_int16_t tmp; > > int i; > > int ret; > > - struct eth_device *dev; > > - > > - dev = malloc(sizeof *dev); > > - > > - if (dev == NULL) > > - return -1; > > - > > - memset(dev, 0, sizeof *dev); > > - strcpy(dev->name, "DaVinci-EMAC"); > > - > > - dev->iobase = 0; > > - dev->init = davinci_eth_open; > > - dev->halt = davinci_eth_close; > > - dev->send = davinci_eth_send_packet; > > - dev->recv = davinci_eth_rcv_packet; > > - dev->write_hwaddr = davinci_eth_set_mac_addr; > > - > > - eth_register(dev); > > > > davinci_eth_mdio_enable(); > > > > @@ -854,5 +839,29 @@ int davinci_emac_initialize(void) > > phy[i].auto_negotiate(i); > > } > > #endif > > - return(1); > > + return 0; > > } > > + > > +static const struct eth_ops davinci_emac_ops = { > > + .start = davinci_emac_start, > > + .send = davinci_emac_send, > > + .recv = davinci_emac_recv, > > + .stop = davinci_emac_stop, > > + .write_hwaddr = davinci_emac_write_hwaddr, > > +}; > > + > > +static const struct udevice_id davinci_emac_ids[] = { > > + { .compatible = "ti,davinci-dm6467-emac" }, > > + { .compatible = "ti,am3517-emac", }, > > + { .compatible = "ti,dm816-emac", }, > > + { } > > +}; > > + > > +U_BOOT_DRIVER(davinci_emac) = { > > + .name = "davinci_emac", > > + .id = UCLASS_ETH, > > + .of_match = davinci_emac_ids, > > + .probe = davinci_emac_probe, > > + .ops = &davinci_emac_ops, > > + .platdata_auto_alloc_size = sizeof(struct eth_pdata), > > +}; > > diff --git a/include/netdev.h b/include/netdev.h > > index 0a1a3a2d8d..a40c4adaad 100644 > > --- a/include/netdev.h > > +++ b/include/netdev.h > > @@ -30,7 +30,6 @@ int bcm_sf2_eth_register(bd_t *bis, u8 dev_num); > > int bfin_EMAC_initialize(bd_t *bis); > > int calxedaxgmac_initialize(u32 id, ulong base_addr); > > int cs8900_initialize(u8 dev_num, int base_addr); > > -int davinci_emac_initialize(void); > > int dc21x4x_initialize(bd_t *bis); > > int designware_initialize(ulong base_addr, u32 interface); > > int dm9000_initialize(bd_t *bis); > > -- > > 2.21.0 > >