Hi David,
2016-02-18 5:43 GMT+01:00 David Miller <da...@davemloft.net>:
> From: Gregory CLEMENT <gregory.clem...@free-electrons.com>
> Date: Tue, 16 Feb 2016 16:33:41 +0100
>
>> pp->dev = dev;
>> SET_NETDEV_DEV(dev, &pdev->dev);
>>
>> + dev->features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_TSO;
>> + dev->hw_features |= dev->features;
>> + dev->vlan_features |= dev->features;
>> + dev->priv_flags |= IFF_UNICAST_FLT;
>> + dev->gso_max_segs = MVNETA_MAX_TSO_SEGS;
>> +
>> + err = register_netdev(dev);
>> + if (err < 0) {
>> + dev_err(&pdev->dev, "failed to register\n");
>> + goto err_free_stats;
>> + }
>> +
>> + pp->id = dev->ifindex;
>> +
>> + /* Obtain access to BM resources if enabled and already initialized */
>> + bm_node = of_parse_phandle(dn, "buffer-manager", 0);
>> + if (bm_node && bm_node->data) {
>
> This set of changes has a lot of problems.
>
> First, the exact moment you call register_netdev() your device must be
> fully initialized because ->open() can be invoked immediately. This
> means you must take care of all of this buffer manager stuff before
> calling register_netdev().
>
> It must precisely be the last thing you invoke in your probe function
> for this reason.
Ok. I shifted register_netdev in order to obtain port id dynamically
from netdev's ifindex (needed to control port <-> pool mapping). If
this order of registration is problematic, I will add an ID property
to DT.
>
> Also you are now adding conditionalized code to every fastpath in your
> driver, that is rediculous and is going to hurt performance.
>
> Add seperate code paths for the HWBM vs SWBM, and register a unique
> set of netdev_ops as appropriate.
TX is untouched and BM support affects only open, stop and change_mtu
- whose execution is not problematic in terms of performance. However
there are a couple new conditions in mvneta_rx(). It can be reduced to
a single condition check, moved to NAPI callback. I'll try to refactor
code in a way to avoid code duplication. Please bear in mind I don't
want to register to different NAPI functions (exactly the same apart
from one line), as the driver can fall back to SWBM after e.g.
unsuccessful mtu change.
Best regards,
Marcin
