Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

Arnd Bergmann wrote: This is basically ok, but then I think you should pass GFP_DMA or GFP_DMA32 to all allocations that the driver does after the 64-bit mask fails, otherwise you get a significant overhead in the bounce buffers. Well, for starters, ZONE_DMA32 is the same as ZONE_NORMAL on

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

On Wednesday, August 3, 2016 4:24:22 PM CEST Timur Tabi wrote: > Arnd Bergmann wrote: > > As I said, this is inherently driver specific. If setting the 64-bit > > mask fails, the driver itself needs to fall back to the 32-bit mask > > so it can allocate buffers from ZONE_DMA instead of

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

Arnd Bergmann wrote: As I said, this is inherently driver specific. If setting the 64-bit mask fails, the driver itself needs to fall back to the 32-bit mask so it can allocate buffers from ZONE_DMA instead of ZONE_NORMAL. I just posted a v7 of my patch, but I forgot to fix the dma_set_mask

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

Timur Tabi wrote: Is there a good way to test my code? ping and iperf appear to send no more than 3 packets at a time, which comes nowhere close to filling the queue (which holds 512 normally). netif_queue_stopped() never returns true, no matter what I do. Never mind, I fixed this problem.

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

Lino Sanfilippo wrote: By looking closer to the code, the lock seems to serve the protection of a list of skbs that are queued to be timestamped. However there is nothing that ever enqueues those skbs, is it? I assume that this is a leftover of a previous version of that driver. Code to be

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

On 28.07.2016 21:12, Timur Tabi wrote: > >>> + if (ret) { >>> + netdev_err(adpt->netdev, >>> + "error:%d on request_irq(%d:%s flags:0)\n", ret, >>> + irq->irq, EMAC_MAC_IRQ_RES); >> >> freeing the irq is missing > > Will fix. > >>> + /*

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

Lino Sanfilippo wrote: + skb = dev_alloc_skb(adpt->rxbuf_size + NET_IP_ALIGN); + if (!skb) + break; + + /* Make buffer alignment 2 beyond a 16 byte boundary +* this will result in a 16 byte aligned IP header after +

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

Hi, some remarks below. > +/* Fill up receive queue's RFD with preallocated receive buffers */ > +static void emac_mac_rx_descs_refill(struct emac_adapter *adpt, > + struct emac_rx_queue *rx_q) > +{ > + struct emac_buffer *curr_rxbuf; > + struct

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

On Wednesday, June 29, 2016 3:16:04 PM CEST Timur Tabi wrote: > Arnd Bergmann wrote: > > >> Sure, but in theory, my for-loop is correct, right? Wouldn't there be > >> some value in setting a 36-bit or 40-bit DMA mask if it works? We have > >> a platform where memory starts at a 40-bit address,

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

Arnd Bergmann wrote: Sure, but in theory, my for-loop is correct, right? Wouldn't there be some value in setting a 36-bit or 40-bit DMA mask if it works? We have a platform where memory starts at a 40-bit address, so some devices have a 44-bit address bus. If a 64-bit mask doesn't work, then

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

On Wednesday, June 29, 2016 10:46:23 AM CEST Timur Tabi wrote: > Arnd Bergmann wrote: > > Usually drivers try 64-bit mask and 32-bit masks, and the 32 bit > > mask is practically guaranteed to succeed. > > Sure, but in theory, my for-loop is correct, right? Wouldn't there be > some value in

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

Arnd Bergmann wrote: Usually drivers try 64-bit mask and 32-bit masks, and the 32 bit mask is practically guaranteed to succeed. Sure, but in theory, my for-loop is correct, right? Wouldn't there be some value in setting a 36-bit or 40-bit DMA mask if it works? We have a platform where

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

On Wednesday, June 29, 2016 10:10:59 AM CEST Timur Tabi wrote: > Arnd Bergmann wrote: > > That's also not how it works: each device starts out with a 32-bit mask, > > because that's what historically all PCI devices can do. If a device > > is 64-bit DMA capable, it can extend the mask by passing

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

Arnd Bergmann wrote: That's also not how it works: each device starts out with a 32-bit mask, because that's what historically all PCI devices can do. If a device is 64-bit DMA capable, it can extend the mask by passing DMA_BIT_MASK(64) (or whatever it can support), and the platform code checks

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

On Wednesday, June 29, 2016 9:33:54 AM CEST Timur Tabi wrote: > Arnd Bergmann wrote: > > If the ranges property lists the bus as dma capable for only the > > lower 32 bits, then dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); > > should fail, otherwise dma_alloc_coherent() will return an invalid

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

Arnd Bergmann wrote: If the ranges property lists the bus as dma capable for only the lower 32 bits, then dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); should fail, otherwise dma_alloc_coherent() will return an invalid memory area. That seems wrong. dma_alloc_coherent() should be smart

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

On Wednesday, June 29, 2016 7:17:29 AM CEST Timur Tabi wrote: > Arnd Bergmann wrote: > > On Friday, June 24, 2016 6:46:48 PM CEST Timur Tabi wrote: > >> >+ /* The EMAC itself is capable of 64-bit DMA. If the SOC limits > >> >that > >> >+* range, then we expect platform code to

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

Arnd Bergmann wrote: On Friday, June 24, 2016 6:46:48 PM CEST Timur Tabi wrote: >+ /* The EMAC itself is capable of 64-bit DMA. If the SOC limits that >+* range, then we expect platform code to adjust the mask accordingly. >+*/ >+ ret =

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

On Friday, June 24, 2016 6:46:48 PM CEST Timur Tabi wrote: > + /* The EMAC itself is capable of 64-bit DMA. If the SOC limits that > +* range, then we expect platform code to adjust the mask accordingly. > +*/ > + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64));

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

From: Timur Tabi Date: Fri, 24 Jun 2016 18:46:48 -0500 > + while (test_and_set_bit(EMAC_STATUS_RESETTING, >status)) > + msleep(20); /* Reset might take few 10s of ms */ ... > + while (test_and_set_bit(EMAC_STATUS_RESETTING, >status)) > +

Re: [PATCH] [v6] net: emac: emac gigabit ethernet controller driver

On Fri, Jun 24, 2016 at 06:46:48PM -0500, Timur Tabi wrote: > Add supports for ethernet controller HW on Qualcomm Technologies, Inc. SoC. > This driver supports the following features: > 1) Checksum offload. > 2) Interrupt coalescing support. > 3) SGMII phy. > 4) phylib interface for external phy

[PATCH] [v6] net: emac: emac gigabit ethernet controller driver

Add supports for ethernet controller HW on Qualcomm Technologies, Inc. SoC. This driver supports the following features: 1) Checksum offload. 2) Interrupt coalescing support. 3) SGMII phy. 4) phylib interface for external phy Based on original work by Niranjana Vishwanathapura