Dear David
On 04/08/2014 02:53 AM, David Miller wrote:
+static void hip04_tx_reclaim(struct net_device *ndev, bool force)
...
+static void hip04_xmit_timer(unsigned long data)
+{
+ struct net_device *ndev = (void *) data;
+
+ hip04_tx_reclaim(ndev, false);
+}
...
+ mod
Dear David,
On 04/08/2014 04:30 PM, David Laight wrote:
From: zhangfei [mailto:zhangfei@linaro.org]
On 04/08/2014 02:53 AM, David Miller wrote:
From: Zhangfei Gao
Date: Sat, 5 Apr 2014 12:35:06 +0800
+struct tx_desc {
+ u32 send_addr;
+ u16 reserved_16;
+ u16 send_siz
On Tuesday 08 April 2014 08:30:37 David Laight wrote:
> From: zhangfei [mailto:zhangfei@linaro.org]
> > On 04/08/2014 02:53 AM, David Miller wrote:
> > > From: Zhangfei Gao
> > > Date: Sat, 5 Apr 2014 12:35:06 +0800
> > >
> > >> +struct tx_desc {
> > >> + u32 send_addr;
> > >> + u16 reserve
From: zhangfei [mailto:zhangfei@linaro.org]
> On 04/08/2014 02:53 AM, David Miller wrote:
> > From: Zhangfei Gao
> > Date: Sat, 5 Apr 2014 12:35:06 +0800
> >
> >> +struct tx_desc {
> >> + u32 send_addr;
> >> + u16 reserved_16;
> >> + u16 send_size;
The above doesn't look right for endiann
Dear David
On 04/08/2014 02:53 AM, David Miller wrote:
From: Zhangfei Gao
Date: Sat, 5 Apr 2014 12:35:06 +0800
+struct tx_desc {
+ u32 send_addr;
+ u16 reserved_16;
+ u16 send_size;
+ u32 reserved_32;
+ u32 cfg;
+ u32 wb_addr;
+} cacheline_aligned;
I
From: Zhangfei Gao
Date: Sat, 5 Apr 2014 12:35:06 +0800
> +#define DESC_DEF_CFG 0x14
You absolutely cannot do this.
You must document what the bits in the TX descriptor config field
mean, all of them.
I bet there is a bit in there somewhere which tells the chip to signal
an in
From: Zhangfei Gao
Date: Sat, 5 Apr 2014 12:35:06 +0800
> +struct tx_desc {
> + u32 send_addr;
> + u16 reserved_16;
> + u16 send_size;
> + u32 reserved_32;
> + u32 cfg;
> + u32 wb_addr;
> +} cacheline_aligned;
I do not think that cacheline_aligned is appropriate
Dear Russell
On Thu, Apr 3, 2014 at 11:27 PM, Russell King - ARM Linux
wrote:
> On Wed, Apr 02, 2014 at 11:21:45AM +0200, Arnd Bergmann wrote:
>> - As David Laight pointed out earlier, you must also ensure that
>> you don't have too much /data/ pending in the descriptor ring
>> when you stop
On Thursday 03 April 2014 16:27:46 Russell King - ARM Linux wrote:
> On Wed, Apr 02, 2014 at 11:21:45AM +0200, Arnd Bergmann wrote:
> > - As David Laight pointed out earlier, you must also ensure that
> > you don't have too much /data/ pending in the descriptor ring
> > when you stop the queue.
On Thu, Apr 03, 2014 at 03:42:00PM +, David Laight wrote:
> From: Russell King - ARM Linux
> > DMA coherent memory is write combining, so multiple writes will be
> > coalesced. This also means that barriers may be required to ensure the
> > descriptors are pushed out in a timely manner if some
From: Russell King - ARM Linux
> DMA coherent memory is write combining, so multiple writes will be
> coalesced. This also means that barriers may be required to ensure the
> descriptors are pushed out in a timely manner if something like writel()
> is not used in the transmit-triggering path.
Yo
Dear David
On 04/02/2014 06:04 PM, David Laight wrote:
From: Arnd Bergmann
On Tuesday 01 April 2014 21:27:12 Zhangfei Gao wrote:
+static int hip04_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
While it looks like there are no serious functionality bugs left, this
function is r
On Wed, Apr 02, 2014 at 11:21:45AM +0200, Arnd Bergmann wrote:
> - As David Laight pointed out earlier, you must also ensure that
> you don't have too much /data/ pending in the descriptor ring
> when you stop the queue. For a 10mbit connection, you have already
> tested (as we discussed on I
From: David Laight
Date: Wed, 2 Apr 2014 10:04:34 +
> From: Arnd Bergmann
>> On Tuesday 01 April 2014 21:27:12 Zhangfei Gao wrote:
>> > +static int hip04_mac_start_xmit(struct sk_buff *skb, struct net_device
>> > *ndev)
>>
>> While it looks like there are no serious functionality bugs left,
On Thursday 03 April 2014 14:24:25 Zhangfei Gao wrote:
> On Wed, Apr 2, 2014 at 11:49 PM, Arnd Bergmann wrote:
> > On Wednesday 02 April 2014 10:04:34 David Laight wrote:
> >> What you need to avoid is reads from uncached memory.
> >> It may well beneficial for the tx reclaim code to first
> >> ch
On Wed, Apr 2, 2014 at 11:49 PM, Arnd Bergmann wrote:
> On Wednesday 02 April 2014 10:04:34 David Laight wrote:
>> From: Arnd Bergmann
>> > On Tuesday 01 April 2014 21:27:12 Zhangfei Gao wrote:
>> > > + phys = dma_map_single(&ndev->dev, skb->data, skb->len,
>> > > DMA_TO_DEVICE);
>> > > +
On Wednesday 02 April 2014 10:04:34 David Laight wrote:
> From: Arnd Bergmann
> > On Tuesday 01 April 2014 21:27:12 Zhangfei Gao wrote:
> > > + phys = dma_map_single(&ndev->dev, skb->data, skb->len,
> > > DMA_TO_DEVICE);
> > > + if (dma_mapping_error(&ndev->dev, phys)) {
> > > +
On Wednesday 02 April 2014 17:51:54 zhangfei wrote:
> Dear Arnd
>
> On 04/02/2014 05:21 PM, Arnd Bergmann wrote:
> > On Tuesday 01 April 2014 21:27:12 Zhangfei Gao wrote:
> >> +static int hip04_mac_start_xmit(struct sk_buff *skb, struct net_device
> >> *ndev)
> >
> > While it looks like there are
From: Arnd Bergmann
> On Tuesday 01 April 2014 21:27:12 Zhangfei Gao wrote:
> > +static int hip04_mac_start_xmit(struct sk_buff *skb, struct net_device
> > *ndev)
>
> While it looks like there are no serious functionality bugs left, this
> function is rather inefficient, as has been pointed out b
Dear Arnd
On 04/02/2014 05:21 PM, Arnd Bergmann wrote:
On Tuesday 01 April 2014 21:27:12 Zhangfei Gao wrote:
+static int hip04_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
While it looks like there are no serious functionality bugs left, this
function is rather inefficient, as
On Tuesday 01 April 2014 21:27:12 Zhangfei Gao wrote:
> +static int hip04_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
While it looks like there are no serious functionality bugs left, this
function is rather inefficient, as has been pointed out before:
> +{
> + struct hip04
On 03/26/2014 01:54 AM, Arnd Bergmann wrote:
On Tuesday 25 March 2014 10:21:42 Eric Dumazet wrote:
On Tue, 2014-03-25 at 18:05 +0100, Arnd Bergmann wrote:
On Tuesday 25 March 2014 10:00:30 Florian Fainelli wrote:
Using a timer to ensure completion of TX packets is a trick that
worked in the
Dear Florian
Thanks for the kind suggestion.
On Tue, Mar 25, 2014 at 12:32 AM, Florian Fainelli wrote:
> 2014-03-24 7:14 GMT-07:00 Zhangfei Gao :
>> Support Hisilicon hip04 ethernet driver, including 100M / 1000M controller
>>
>> Signed-off-by: Zhangfei Gao
>> ---
>> drivers/net/ethernet/hisil
Dear Rob
On Mon, Mar 24, 2014 at 10:17 PM, Rob Herring wrote:
>
+ dma_map_single(&ndev->dev, skb->data,
+ RX_BUF_SIZE, DMA_FROM_DEVICE);
>>>
>>> This is incorrect.
>>>
>>> buf = buffer alloc()
>>> /* CPU owns buffer and can read/write it, device does not
From: Arnd Bergmann
> On Tuesday 25 March 2014 10:16:28 Florian Fainelli wrote:
> >
> > Ok, well that's really unfortunate, to achieve the best of everything,
> > the workaround should probably look like:
> >
> > - keep reclaiming TX buffers in ndo_start_xmit() in case you push more
> > packets to
On Tuesday 25 March 2014 10:21:42 Eric Dumazet wrote:
> On Tue, 2014-03-25 at 18:05 +0100, Arnd Bergmann wrote:
> > On Tuesday 25 March 2014 10:00:30 Florian Fainelli wrote:
> >
> > > Using a timer to ensure completion of TX packets is a trick that
> > > worked in the past, but now that the network
On Tuesday 25 March 2014 10:16:28 Florian Fainelli wrote:
>
> Ok, well that's really unfortunate, to achieve the best of everything,
> the workaround should probably look like:
>
> - keep reclaiming TX buffers in ndo_start_xmit() in case you push more
> packets to the NICs than your timer can fre
2014-03-25 10:05 GMT-07:00 Arnd Bergmann :
> On Tuesday 25 March 2014 10:00:30 Florian Fainelli wrote:
>> 2014-03-25 1:12 GMT-07:00 Arnd Bergmann :
>> > On Tuesday 25 March 2014 12:06:31 Zhangfei Gao wrote:
>> >> Dear Arnd
>> >>
>> >> On Mon, Mar 24, 2014 at 11:18 PM, Arnd Bergmann wrote:
>> >> >
On Tue, 2014-03-25 at 18:05 +0100, Arnd Bergmann wrote:
> On Tuesday 25 March 2014 10:00:30 Florian Fainelli wrote:
> > 2014-03-25 1:12 GMT-07:00 Arnd Bergmann :
> > > On Tuesday 25 March 2014 12:06:31 Zhangfei Gao wrote:
> > >> Dear Arnd
> > >>
> > >> On Mon, Mar 24, 2014 at 11:18 PM, Arnd Bergman
From: Arnd Bergmann
> > Using a timer to ensure completion of TX packets is a trick that
> > worked in the past, but now that the networking stack got smarter,
> > this might artificially increase the processing time of packets in the
> > transmit path, and this will defeat features like TCP small
On Tuesday 25 March 2014 10:00:30 Florian Fainelli wrote:
> 2014-03-25 1:12 GMT-07:00 Arnd Bergmann :
> > On Tuesday 25 March 2014 12:06:31 Zhangfei Gao wrote:
> >> Dear Arnd
> >>
> >> On Mon, Mar 24, 2014 at 11:18 PM, Arnd Bergmann wrote:
> >> > On Monday 24 March 2014 22:14:56 Zhangfei Gao wrote
2014-03-25 1:12 GMT-07:00 Arnd Bergmann :
> On Tuesday 25 March 2014 12:06:31 Zhangfei Gao wrote:
>> Dear Arnd
>>
>> On Mon, Mar 24, 2014 at 11:18 PM, Arnd Bergmann wrote:
>> > On Monday 24 March 2014 22:14:56 Zhangfei Gao wrote:
>> >
>> >> +
>> >> +static void hip04_tx_reclaim(struct net_device *
On Tuesday 25 March 2014 12:06:31 Zhangfei Gao wrote:
> Dear Arnd
>
> On Mon, Mar 24, 2014 at 11:18 PM, Arnd Bergmann wrote:
> > On Monday 24 March 2014 22:14:56 Zhangfei Gao wrote:
> >
> >> +
> >> +static void hip04_tx_reclaim(struct net_device *ndev, bool force)
> >> +{
> >> + struct hip04_
Dear Arnd
On Mon, Mar 24, 2014 at 11:18 PM, Arnd Bergmann wrote:
> On Monday 24 March 2014 22:14:56 Zhangfei Gao wrote:
>
>> +
>> +static void hip04_tx_reclaim(struct net_device *ndev, bool force)
>> +{
>> + struct hip04_priv *priv = netdev_priv(ndev);
>> + unsigned tx_head = priv->tx_hea
2014-03-24 10:23 GMT-07:00 Arnd Bergmann :
> On Monday 24 March 2014 09:32:17 Florian Fainelli wrote:
>> > + priv->tx_skb[tx_head] = skb;
>> > + priv->tx_phys[tx_head] = phys;
>> > + desc->send_addr = cpu_to_be32(phys);
>> > + desc->send_size = cpu_to_be16(skb->len);
>> > +
On Monday 24 March 2014 09:32:17 Florian Fainelli wrote:
> > + priv->tx_skb[tx_head] = skb;
> > + priv->tx_phys[tx_head] = phys;
> > + desc->send_addr = cpu_to_be32(phys);
> > + desc->send_size = cpu_to_be16(skb->len);
> > + desc->cfg = cpu_to_be32(DESC_DEF_CFG);
> > +
2014-03-24 7:14 GMT-07:00 Zhangfei Gao :
> Support Hisilicon hip04 ethernet driver, including 100M / 1000M controller
>
> Signed-off-by: Zhangfei Gao
> ---
> drivers/net/ethernet/hisilicon/Makefile|2 +-
> drivers/net/ethernet/hisilicon/hip04_eth.c | 728
>
>
On Monday 24 March 2014 22:14:56 Zhangfei Gao wrote:
> +
> +static void hip04_tx_reclaim(struct net_device *ndev, bool force)
> +{
> + struct hip04_priv *priv = netdev_priv(ndev);
> + unsigned tx_head = priv->tx_head;
> + unsigned tx_tail = priv->tx_tail;
> + struct tx_desc *desc =
On Thu, Mar 20, 2014 at 4:51 AM, Zhangfei Gao wrote:
> Dear Russell
>
> Thanks for sparing time and giving so many perfect suggestion, really helpful.
>
> On Tue, Mar 18, 2014 at 6:46 PM, Russell King - ARM Linux
> wrote:
>> I was just browsing this patch when I noticed some of these issues - I
>
On Mon, Mar 24, 2014 at 6:02 PM, Arnd Bergmann wrote:
> On Monday 24 March 2014 16:17:42 Zhangfei Gao wrote:
>> On Thu, Mar 20, 2014 at 10:31 PM, Arnd Bergmann wrote:
>> >> >
>> >> >> + if (!ppebase) {
>> >> >> + struct device_node *n;
>> >> >> +
>> >> >> + n = of_find
On Monday 24 March 2014 16:17:42 Zhangfei Gao wrote:
> On Thu, Mar 20, 2014 at 10:31 PM, Arnd Bergmann wrote:
> >> >
> >> >> + if (!ppebase) {
> >> >> + struct device_node *n;
> >> >> +
> >> >> + n = of_find_compatible_node(NULL, NULL,
> >> >> "hisilicon,hip04-ppebase"
Dear Arnd
On Thu, Mar 20, 2014 at 10:31 PM, Arnd Bergmann wrote:
> On Thursday 20 March 2014, Zhangfei Gao wrote:
>> On Tue, Mar 18, 2014 at 7:25 PM, Arnd Bergmann wrote:
>> > On Tuesday 18 March 2014 16:40:17 Zhangfei Gao wrote:
>> >
>> >> +
>> >> +static void __iomem *ppebase;
>> >
>> > The gl
On Saturday 22 March 2014 09:18:35 zhangfei wrote:
> >> +static void hip04_tx_reclaim(struct net_device *ndev, bool force)
> >> +{
> >> + struct hip04_priv *priv = netdev_priv(ndev);
> >> + unsigned tx_head = priv->tx_head;
> >> + unsigned tx_tail = priv->tx_tail;
> >> + struct tx_desc *desc =
Dear Arnd
On 03/21/2014 11:27 PM, Arnd Bergmann wrote:
On Friday 21 March 2014 23:09:30 Zhangfei Gao wrote:
+
+static void __iomem *ppebase;
Any reason why you still have this, rather than using a separate
driver for it as we discussed? If you have comments that you still
plan to address, pl
On Friday 21 March 2014 23:09:30 Zhangfei Gao wrote:
> +
> +static void __iomem *ppebase;
Any reason why you still have this, rather than using a separate
driver for it as we discussed? If you have comments that you still
plan to address, please mention those in the introductory mail,
so you don'
Dear Arnd
On Fri, Mar 21, 2014 at 3:37 PM, Arnd Bergmann wrote:
>> >> >> +static int hip04_alloc_ring(struct net_device *ndev, struct device *d)
>> >> >> +{
>> >> >> + struct hip04_priv *priv = netdev_priv(ndev);
>> >> >> + int i;
>> >> >> +
>> >> >> + priv->rx_buf_size = RX_BUF_SIZE
On Friday 21 March 2014 13:19:07 Zhangfei Gao wrote:
> On Thu, Mar 20, 2014 at 10:31 PM, Arnd Bergmann wrote:
>
> > Yes, this looks better, but where does 'speed' come from now? I assume
> > even in SGMII mode, you should allow autonegotiation and set this correctly
> > from the PHY code. Is that
Dear Arnd
On Thu, Mar 20, 2014 at 10:31 PM, Arnd Bergmann wrote:
>> > This also seems to encode knowledge about a particular implementation
>> > into the driver. Maybe it's better to add a property for the port
>> > mode?
>>
>> After check Documentation/devicetree/bindings/net/ethernet.txt,
>> I
On Thursday 20 March 2014, Zhangfei Gao wrote:
> On Tue, Mar 18, 2014 at 7:25 PM, Arnd Bergmann wrote:
> > On Tuesday 18 March 2014 16:40:17 Zhangfei Gao wrote:
> >
> >> +
> >> +static void __iomem *ppebase;
> >
> > The global 'ppebase' seems hacky. Isn't that a SoC-specific register area,
> > wh
Dear Arnd
On Tue, Mar 18, 2014 at 7:25 PM, Arnd Bergmann wrote:
> On Tuesday 18 March 2014 16:40:17 Zhangfei Gao wrote:
>
>> +
>> +static void __iomem *ppebase;
>
> The global 'ppebase' seems hacky. Isn't that a SoC-specific register area,
> while
> the rest of the driver is reusable across SoCs
Dear Russell
Thanks for sparing time and giving so many perfect suggestion, really helpful.
On Tue, Mar 18, 2014 at 6:46 PM, Russell King - ARM Linux
wrote:
> I was just browsing this patch when I noticed some of these issues - I
> haven't done a full review of this driver, I'm just commenting o
On Tuesday 18 March 2014 16:40:17 Zhangfei Gao wrote:
> +
> +static void __iomem *ppebase;
The global 'ppebase' seems hacky. Isn't that a SoC-specific register area, while
the rest of the driver is reusable across SoCs?
What does 'ppe' stand for?
What if there are multiple instances of this, wh
I was just browsing this patch when I noticed some of these issues - I
haven't done a full review of this driver, I'm just commenting on the
things I've spotted.
On Tue, Mar 18, 2014 at 04:40:17PM +0800, Zhangfei Gao wrote:
> +static int hip04_rx_poll(struct napi_struct *napi, int budget)
> +{
> +
53 matches
Mail list logo
