On 15.12.2015 23:34, Simon Glass wrote:
> Hi Joe,
>
> On 15 December 2015 at 13:52, Joe Hershberger <joe.hershber...@gmail.com>
> wrote:
>>
>> Hi Simon,
>>
>> On Tue, Dec 15, 2015 at 12:57 PM, Simon Glass <s...@chromium.org> wrote:
>>> Hi Michal,
>>>
>>> On 11 December 2015 at 04:59, Michal Simek <michal.si...@xilinx.com> wrote:
>>>> Move driver to DM.
>>>>
>>>> Signed-off-by: Michal Simek <michal.si...@xilinx.com>
>>>> ---
>>>>
>>>> .../xilinx/microblaze-generic/microblaze-generic.c | 5 -
>>>> board/xilinx/zynq/board.c | 4 -
>>>> drivers/net/xilinx_axi_emac.c | 190
>>>> +++++++++++++--------
>>>> include/netdev.h | 2 -
>>>> 4 files changed, 122 insertions(+), 79 deletions(-)
>>>
>>> Reviewed-by: Simon Glass <s...@chromium.org>
>>>
>>> See a few things below.
>>>
>>>>
>>>> diff --git a/board/xilinx/microblaze-generic/microblaze-generic.c
>>>> b/board/xilinx/microblaze-generic/microblaze-generic.c
>>>> index dfa629322223..a3122da9acaa 100644
>>>> --- a/board/xilinx/microblaze-generic/microblaze-generic.c
>>>> +++ b/board/xilinx/microblaze-generic/microblaze-generic.c
>>>> @@ -105,11 +105,6 @@ int board_eth_init(bd_t *bis)
>>>> {
>>>> int ret = 0;
>>>>
>>>> -#ifdef CONFIG_XILINX_AXIEMAC
>>>> - ret |= xilinx_axiemac_initialize(bis, XILINX_AXIEMAC_BASEADDR,
>>>> - XILINX_AXIDMA_BASEADDR);
>>>> -#endif
>>>> -
>>>> #if defined(CONFIG_XILINX_EMACLITE) && defined(XILINX_EMACLITE_BASEADDR)
>>>> u32 txpp = 0;
>>>> u32 rxpp = 0;
>>>> diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
>>>> index 414f5302a066..427e75485deb 100644
>>>> --- a/board/xilinx/zynq/board.c
>>>> +++ b/board/xilinx/zynq/board.c
>>>> @@ -103,10 +103,6 @@ int board_eth_init(bd_t *bis)
>>>> {
>>>> u32 ret = 0;
>>>>
>>>> -#ifdef CONFIG_XILINX_AXIEMAC
>>>> - ret |= xilinx_axiemac_initialize(bis, XILINX_AXIEMAC_BASEADDR,
>>>> - XILINX_AXIDMA_BASEADDR);
>>>> -#endif
>>>> #ifdef CONFIG_XILINX_EMACLITE
>>>> u32 txpp = 0;
>>>> u32 rxpp = 0;
>>>> diff --git a/drivers/net/xilinx_axi_emac.c b/drivers/net/xilinx_axi_emac.c
>>>> index 77b1869dc9dc..c03f8f730d3a 100644
>>>> --- a/drivers/net/xilinx_axi_emac.c
>>>> +++ b/drivers/net/xilinx_axi_emac.c
>>>> @@ -8,12 +8,15 @@
>>>>
>>>> #include <config.h>
>>>> #include <common.h>
>>>> +#include <dm.h>
>>>> #include <net.h>
>>>> #include <malloc.h>
>>>> #include <asm/io.h>
>>>> #include <phy.h>
>>>> #include <miiphy.h>
>>>>
>>>> +DECLARE_GLOBAL_DATA_PTR;
>>>> +
>>>> #if !defined(CONFIG_PHYLIB)
>>>> # error AXI_ETHERNET requires PHYLIB
>>>> #endif
>>>> @@ -87,6 +90,7 @@ struct axidma_priv {
>>>> struct axidma_reg *dmarx;
>>>> int phyaddr;
>>>> struct axi_regs *iobase;
>>>> + phy_interface_t interface;
>>>> struct phy_device *phydev;
>>>> struct mii_dev *bus;
>>>> };
>>>> @@ -218,11 +222,11 @@ static u32 phywrite(struct axidma_priv *priv, u32
>>>> phyaddress, u32 registernum,
>>>> }
>>>>
>>>> /* Setting axi emac and phy to proper setting */
>>>> -static int setup_phy(struct eth_device *dev)
>>>> +static int setup_phy(struct udevice *dev)
>>>> {
>>>> u16 phyreg;
>>>> u32 i, speed, emmc_reg, ret;
>>>> - struct axidma_priv *priv = dev->priv;
>>>> + struct axidma_priv *priv = dev_get_priv(dev);
>>>> struct axi_regs *regs = priv->iobase;
>>>> struct phy_device *phydev;
>>>>
>>>> @@ -298,9 +302,9 @@ static int setup_phy(struct eth_device *dev)
>>>> }
>>>>
>>>> /* STOP DMA transfers */
>>>> -static void axiemac_halt(struct eth_device *dev)
>>>> +static void axiemac_halt(struct udevice *dev)
>>>> {
>>>> - struct axidma_priv *priv = dev->priv;
>>>> + struct axidma_priv *priv = dev_get_priv(dev);
>>>> u32 temp;
>>>>
>>>> /* Stop the hardware */
>>>> @@ -358,16 +362,18 @@ static int axi_ethernet_init(struct axidma_priv
>>>> *priv)
>>>> return 0;
>>>> }
>>>>
>>>> -static int axiemac_setup_mac(struct eth_device *dev)
>>>> +static int axiemac_setup_mac(struct udevice *dev)
>>>> {
>>>> - struct axi_regs *regs = (struct axi_regs *)dev->iobase;
>>>> + struct eth_pdata *pdata = dev_get_platdata(dev);
>>>> + struct axidma_priv *priv = dev_get_priv(dev);
>>>> + struct axi_regs *regs = priv->iobase;
>>>>
>>>> /* Set the MAC address */
>>>> - int val = ((dev->enetaddr[3] << 24) | (dev->enetaddr[2] << 16) |
>>>> - (dev->enetaddr[1] << 8) | (dev->enetaddr[0]));
>>>> + int val = ((pdata->enetaddr[3] << 24) | (pdata->enetaddr[2] << 16)
>>>> |
>>>> + (pdata->enetaddr[1] << 8) | (pdata->enetaddr[0]));
>>>> out_be32(®s->uaw0, val);
>>>>
>>>> - val = (dev->enetaddr[5] << 8) | dev->enetaddr[4] ;
>>>> + val = (pdata->enetaddr[5] << 8) | pdata->enetaddr[4];
>>>> val |= in_be32(®s->uaw1) & ~XAE_UAW1_UNICASTADDR_MASK;
>>>> out_be32(®s->uaw1, val);
>>>> return 0;
>>>> @@ -396,10 +402,10 @@ static void axi_dma_init(struct axidma_priv *priv)
>>>> printf("%s: Timeout\n", __func__);
>>>> }
>>>>
>>>> -static int axiemac_init(struct eth_device *dev, bd_t * bis)
>>>> +static int axiemac_init(struct udevice *dev)
>>>> {
>>>> - struct axidma_priv *priv = dev->priv;
>>>> - struct axi_regs *regs = (struct axi_regs *)dev->iobase;
>>>> + struct axidma_priv *priv = dev_get_priv(dev);
>>>> + struct axi_regs *regs = priv->iobase;
>>>> u32 temp;
>>>>
>>>> debug("axiemac: Init started\n");
>>>> @@ -458,9 +464,9 @@ static int axiemac_init(struct eth_device *dev, bd_t *
>>>> bis)
>>>> return 0;
>>>> }
>>>>
>>>> -static int axiemac_send(struct eth_device *dev, void *ptr, int len)
>>>> +static int axiemac_send(struct udevice *dev, void *ptr, int len)
>>>> {
>>>> - struct axidma_priv *priv = dev->priv;
>>>> + struct axidma_priv *priv = dev_get_priv(dev);
>>>> u32 timeout;
>>>>
>>>> if (len > PKTSIZE_ALIGN)
>>>> @@ -530,15 +536,15 @@ static int isrxready(struct axidma_priv *priv)
>>>> return 0;
>>>> }
>>>>
>>>> -static int axiemac_recv(struct eth_device *dev)
>>>> +static int axiemac_recv(struct udevice *dev, int flags, uchar **packetp)
>>>> {
>>>> u32 length;
>>>> - struct axidma_priv *priv = dev->priv;
>>>> + struct axidma_priv *priv = dev_get_priv(dev);
>>>> u32 temp;
>>>>
>>>> /* Wait for an incoming packet */
>>>> if (!isrxready(priv))
>>>> - return 0;
>>>> + return -1;
>>>
>>> I suggest -EAGAIN
>>>
>>>>
>>>> debug("axiemac: RX data ready\n");
>>>>
>>>> @@ -578,77 +584,125 @@ static int axiemac_recv(struct eth_device *dev)
>>>>
>>>> debug("axiemac: RX completed, framelength = %d\n", length);
>>>>
>>>> - return length;
>>>> + return 0;
>>>
>>> You do need to return the length here. You could update net.h to make
>>> this clearer.
>>
>> I think I figured out what Michal is doing here. What's not clear from
>> this patch is that he doesn't want net_process_received_packet() to
>> run on the packet since this function already calls it directly above.
>>
>> He has a follow-on patch that fixes that and resumes returning the
>> length from recv().
>
> OK I see, thanks.
That's what I meant by word "already" below.
"recv part is calling net_process_received_packet(rxframe, length);
already and then the core is checking for returning value. which should
be 0 or less to quit this loop."
I wanted to do separation by two patches to make it clear but it looks
like I have confused you a little bit. :-)
Does that mean that I can add Simon's Reviewed-by tag?
Joe: What about you?
Thanks,
Michal
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
