[U-Boot] [PATCH 07/10] net: axi_emac: Move driver to DM
Joe Hershberger
joe.hershberger at gmail.com
Wed Dec 16 09:26:15 CET 2015
On Wed, Dec 16, 2015 at 2:14 AM, Michal Simek <michal.simek at xilinx.com> wrote:
> On 15.12.2015 23:34, Simon Glass wrote:
>> Hi Joe,
>>
>> On 15 December 2015 at 13:52, Joe Hershberger <joe.hershberger at gmail.com> wrote:
>>>
>>> Hi Simon,
>>>
>>> On Tue, Dec 15, 2015 at 12:57 PM, Simon Glass <sjg at chromium.org> wrote:
>>>> Hi Michal,
>>>>
>>>> On 11 December 2015 at 04:59, Michal Simek <michal.simek at xilinx.com> wrote:
>>>>> Move driver to DM.
>>>>>
>>>>> Signed-off-by: Michal Simek <michal.simek at 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 <sjg at 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?
Yes,
Acked-by: Joe Hershberger <joe.hershberger at ni.com>
More information about the U-Boot
mailing list