From: Michal Simek <monstr@monstr.eu> Date: 2012-10-04 18:15:18
Warning message:
eth0: Dropping NETIF_F_SG since no checksum feature.
Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <redacted>
CC: John Linn <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <redacted>
CC: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
@@ -1519,14 +1519,14 @@ static int __devinit axienet_of_probe(struct platform_device *op)XAE_FEATURE_PARTIAL_TX_CSUM;lp->features|=XAE_FEATURE_PARTIAL_TX_CSUM;/* Can checksum TCP/UDP over IPv4. */-ndev->features|=NETIF_F_IP_CSUM;+ndev->features|=NETIF_F_IP_CSUM|NETIF_F_SG;break;case2:lp->csum_offload_on_tx_path=XAE_FEATURE_FULL_TX_CSUM;lp->features|=XAE_FEATURE_FULL_TX_CSUM;/* Can checksum TCP/UDP over IPv4. */-ndev->features|=NETIF_F_IP_CSUM;+ndev->features|=NETIF_F_IP_CSUM|NETIF_F_SG;break;default:lp->csum_offload_on_tx_path=XAE_NO_CSUM_OFFLOAD;
From: Michal Simek <monstr@monstr.eu> Date: 2012-10-04 18:15:12
NO_IRQ is not longer used by Microblaze.
Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <redacted>
CC: John Linn <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <redacted>
CC: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
From: Michal Simek <monstr@monstr.eu> Date: 2012-10-04 18:15:15
Allow user to access the MDIO from userspace.
Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <redacted>
CC: John Linn <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <redacted>
CC: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
From: Michal Simek <monstr@monstr.eu> Date: 2012-10-04 18:15:21
DMA allocates skb->len instead of headlen
which is used for DMA.
The same fix was applied to the axienet driver.
Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <redacted>
CC: John Linn <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <redacted>
CC: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/xilinx/ll_temac_main.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
From: Michal Simek <monstr@monstr.eu> Date: 2012-10-04 18:15:47
Just show current setting in bootlog.
Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <redacted>
CC: John Linn <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <redacted>
CC: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/xilinx/ll_temac_main.c | 2 ++
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 2 ++
2 files changed, 4 insertions(+), 0 deletions(-)
@@ -1052,12 +1052,14 @@ static int __devinit temac_of_probe(struct platform_device *op)/* Setup checksum offload, but default to off if not specified */lp->temac_features=0;p=(__be32*)of_get_property(op->dev.of_node,"xlnx,txcsum",NULL);+dev_info(&op->dev,"TX_CSUM %d\n",be32_to_cpup(p));if(p&&be32_to_cpu(*p)){lp->temac_features|=TEMAC_FEATURE_TX_CSUM;/* Can checksum TCP/UDP over IPv4. */ndev->features|=NETIF_F_IP_CSUM;}p=(__be32*)of_get_property(op->dev.of_node,"xlnx,rxcsum",NULL);+dev_info(&op->dev,"RX_CSUM %d\n",be32_to_cpup(p));if(p&&be32_to_cpu(*p))lp->temac_features|=TEMAC_FEATURE_RX_CSUM;
From: Michal Simek <monstr@monstr.eu> Date: 2012-10-04 18:16:06
Use one return statement instead of two.
Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <redacted>
CC: John Linn <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <redacted>
CC: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/xilinx/ll_temac_main.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
From: Michal Simek <monstr@monstr.eu> Date: 2012-10-04 18:16:31
Load frag value when necessary.
Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <redacted>
CC: John Linn <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <redacted>
CC: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/xilinx/ll_temac_main.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
From: Michal Simek <monstr@monstr.eu> Date: 2012-10-04 18:16:52
Use max mtu instead.
Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <redacted>
CC: John Linn <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <redacted>
CC: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/xilinx/ll_temac_main.c | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
From: Michal Simek <monstr@monstr.eu> Date: 2012-10-04 18:17:24
Current driver required to have phy-node directly in
the driver. After this fixed we can use standard structure.
DTS fragment:
phy-handle = <&phy0>;
mdio {
#address-cells = <1>;
#size-cells = <0>;
phy0: phy@7 {
compatible = "marvell,88e1111";
device_type = "ethernet-phy";
reg = <7>;
} ;
} ;
Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <redacted>
CC: John Linn <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <redacted>
CC: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/xilinx/ll_temac_main.c | 11 ++++++-----
drivers/net/ethernet/xilinx/ll_temac_mdio.c | 5 +++--
2 files changed, 9 insertions(+), 7 deletions(-)
From: Michal Simek <monstr@monstr.eu> Date: 2012-10-04 18:18:00
The driver is using frame size for VLAN packets
but does not enable VLAN IP option.
Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <redacted>
CC: John Linn <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <redacted>
CC: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
From: Michal Simek <monstr@monstr.eu> Date: 2012-10-04 18:18:22
The MDIO clock was previously hard coded and it is now
calculated thanks to a patch from the community.
Modify the Xilinx patch to get the clock frequency from
the connected AXI bus instead of the CPU.
Currently, the AXI ethernet mdio bus id is set as the mdio device node start
address, but the mdio node don't have a start address. Use the start address
of the AXI ethernet controller which connects to the MDIO bus instead.
Signed-off-by: Wendy Liang <redacted>
Signed-off-by: Nico Augustijn <redacted>
CC: Anirudha Sarangi <redacted>
CC: John Linn <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <redacted>
CC: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c | 76 +++++++++++----------
1 files changed, 40 insertions(+), 36 deletions(-)
@@ -128,11 +128,11 @@ static int axienet_mdio_write(struct mii_bus *bus, int phy_id, int reg,intaxienet_mdio_setup(structaxienet_local*lp,structdevice_node*np){intret;-u32clk_div,host_clock;-u32*property_p;+u32clk_div;structmii_bus*bus;structresourceres;structdevice_node*np1;+structdevice_node*npp=0;/* the ethernet controller device node *//* clk_div can be calculated by deriving it from the equation:*fMDIO=fHOST/((1+clk_div)*2)
@@ -158,41 +158,46 @@ int axienet_mdio_setup(struct axienet_local *lp, struct device_node *np)*fHOSTcanbereadfromtheflatteneddevicetreeasproperty*"clock-frequency"fromtheCPU*/--np1=of_find_node_by_name(NULL,"cpu");-if(!np1){-printk(KERN_WARNING"%s(): Could not find CPU device node.",-__func__);-printk(KERN_WARNING"Setting MDIO clock divisor to "-"default %d\n",DEFAULT_CLOCK_DIVISOR);-clk_div=DEFAULT_CLOCK_DIVISOR;-gotoissue;-}-property_p=(u32*)of_get_property(np1,"clock-frequency",NULL);-if(!property_p){-printk(KERN_WARNING"%s(): Could not find CPU property: "-"clock-frequency.",__func__);-printk(KERN_WARNING"Setting MDIO clock divisor to "-"default %d\n",DEFAULT_CLOCK_DIVISOR);+np1=of_get_parent(lp->phy_node);+if(np1)+npp=of_get_parent(np1);+if(!npp){+dev_warn(lp->dev,+"Could not find ethernet controller device node.");+dev_warn(lp->dev,"Setting MDIO clock divisor to default %d\n",+DEFAULT_CLOCK_DIVISOR);clk_div=DEFAULT_CLOCK_DIVISOR;-gotoissue;+}else{+u32*property_p;++property_p=(uint32_t*)of_get_property(npp,+"clock-frequency",NULL);+if(!property_p){+dev_warn(lp->dev,"Could not find clock ethernet "\+"controller property.");+dev_warn(lp->dev,+"Setting MDIO clock divisor to default %d\n",+DEFAULT_CLOCK_DIVISOR);+clk_div=DEFAULT_CLOCK_DIVISOR;+}else{+u32host_clock=be32_to_cpup(property_p);++clk_div=(host_clock/(MAX_MDIO_FREQ*2))-1;++/* If there is any remainder from the division of+*fHOST/(MAX_MDIO_FREQ*2),thenweneedtoadd1+*totheclockdivisororwewillsurelybe+*above2.5MHz*/+if(host_clock%(MAX_MDIO_FREQ*2))+clk_div++;+dev_dbg(lp->dev,"Setting MDIO clock divisor to %u "\+"based on %u Hz host clock.\n",+clk_div,host_clock);+}}-host_clock=be32_to_cpup(property_p);-clk_div=(host_clock/(MAX_MDIO_FREQ*2))-1;-/* If there is any remainder from the division of-*fHOST/(MAX_MDIO_FREQ*2),thenweneedtoadd-*1totheclockdivisororwewillsurelybeabove2.5MHz*/-if(host_clock%(MAX_MDIO_FREQ*2))-clk_div++;--printk(KERN_DEBUG"%s(): Setting MDIO clock divisor to %u based "-"on %u Hz host clock.\n",__func__,clk_div,host_clock);--of_node_put(np1);-issue:-axienet_iow(lp,XAE_MDIO_MC_OFFSET,-(((u32)clk_div)|XAE_MDIO_MC_MDIOEN_MASK));+axienet_iow(lp,XAE_MDIO_MC_OFFSET,(((u32)clk_div)|+XAE_MDIO_MC_MDIOEN_MASK));ret=axienet_mdio_wait_until_ready(lp);if(ret<0)
From: David Miller <davem@davemloft.net> Date: 2012-10-04 18:26:18
Sorry, no.
I've announced on netdev very clearly that net-next submissions are not
appropriate at this time and that only pure bug fixes should be submitted.
Watch for the announcement on netdev of net-next openning up after the
merge window closes, that's when you should resubmit this series.
@@ -1052,12 +1052,14 @@ static int __devinit temac_of_probe(struct platform_device *op)/* Setup checksum offload, but default to off if not specified */lp->temac_features=0;p=(__be32*)of_get_property(op->dev.of_node,"xlnx,txcsum",NULL);+dev_info(&op->dev,"TX_CSUM %d\n",be32_to_cpup(p));if(p&&be32_to_cpu(*p)){lp->temac_features|=TEMAC_FEATURE_TX_CSUM;/* Can checksum TCP/UDP over IPv4. */ndev->features|=NETIF_F_IP_CSUM;}p=(__be32*)of_get_property(op->dev.of_node,"xlnx,rxcsum",NULL);+dev_info(&op->dev,"RX_CSUM %d\n",be32_to_cpup(p));
[...]
Is there any particular reason you think this needs to be logged by
default, rather than letting users run ethtool -k? I suggest using
dev_dbg() instead.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
From: Ben Hutchings <hidden> Date: 2012-10-04 19:17:47
On Thu, 2012-10-04 at 20:14 +0200, Michal Simek wrote:
quoted hunk
Allow user to access the MDIO from userspace.
Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <redacted>
CC: John Linn <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <redacted>
CC: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
From: Ben Hutchings <hidden> Date: 2012-10-04 19:22:48
On Thu, 2012-10-04 at 20:14 +0200, Michal Simek wrote:
Use max mtu instead.
[...]
MTU does not include the Ethernet header so I have no idea how this is
expected to work...
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
From: Michal Simek <monstr@monstr.eu> Date: 2012-10-05 06:50:36
On 10/04/2012 09:22 PM, Ben Hutchings wrote:
On Thu, 2012-10-04 at 20:14 +0200, Michal Simek wrote:
quoted
Use max mtu instead.
[...]
MTU does not include the Ethernet header so I have no idea how this is
expected to work...
Right. This is wrong fix. It should be the same as is in axienet.
There is max_frm_size used which is mtu+header+tailer.
I will fix it.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
@@ -1052,12 +1052,14 @@ static int __devinit temac_of_probe(struct platform_device *op)/* Setup checksum offload, but default to off if not specified */lp->temac_features=0;p=(__be32*)of_get_property(op->dev.of_node,"xlnx,txcsum",NULL);+dev_info(&op->dev,"TX_CSUM %d\n",be32_to_cpup(p));if(p&&be32_to_cpu(*p)){lp->temac_features|=TEMAC_FEATURE_TX_CSUM;/* Can checksum TCP/UDP over IPv4. */ndev->features|=NETIF_F_IP_CSUM;}p=(__be32*)of_get_property(op->dev.of_node,"xlnx,rxcsum",NULL);+dev_info(&op->dev,"RX_CSUM %d\n",be32_to_cpup(p));
[...]
Is there any particular reason you think this needs to be logged by
default, rather than letting users run ethtool -k? I suggest using
dev_dbg() instead.
The reason was just to show it in the bootlog.
I will check ethtool support for these drivers.
Thanks for your comments,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
From: Michal Simek <monstr@monstr.eu> Date: 2012-10-05 07:00:12
On 10/04/2012 09:17 PM, Ben Hutchings wrote:
On Thu, 2012-10-04 at 20:14 +0200, Michal Simek wrote:
quoted
Allow user to access the MDIO from userspace.
Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <redacted>
CC: John Linn <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <redacted>
CC: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
Error code should be EOPNOTSUPP - the device is present but just doesn't
support MDIO.
ok. Thanks will fix it.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
From: Michal Simek <monstr@monstr.eu> Date: 2012-10-05 07:10:23
Hi David,
On 10/04/2012 08:26 PM, David Miller wrote:
Sorry, no.
I've announced on netdev very clearly that net-next submissions are not
appropriate at this time and that only pure bug fixes should be submitted.
Watch for the announcement on netdev of net-next openning up after the
merge window closes, that's when you should resubmit this series.
Sorry I should label it as RFC.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
@@ -1052,12 +1052,14 @@ static int __devinit temac_of_probe(struct platform_device *op)/* Setup checksum offload, but default to off if not specified */lp->temac_features=0;p=(__be32*)of_get_property(op->dev.of_node,"xlnx,txcsum",NULL);+dev_info(&op->dev,"TX_CSUM %d\n",be32_to_cpup(p));if(p&&be32_to_cpu(*p)){lp->temac_features|=TEMAC_FEATURE_TX_CSUM;/* Can checksum TCP/UDP over IPv4. */ndev->features|=NETIF_F_IP_CSUM;}p=(__be32*)of_get_property(op->dev.of_node,"xlnx,rxcsum",NULL);+dev_info(&op->dev,"RX_CSUM %d\n",be32_to_cpup(p));
[...]
Is there any particular reason you think this needs to be logged by
default, rather than letting users run ethtool -k? I suggest using
dev_dbg() instead.
Ok. I have looked at it and there are missing some bits in ndev->features.
Can you please check that my setting is correct?
It is SG DMA ip/driver.
ndev->features = NETIF_F_FRAGLIST | NETIF_F_SG
With two options for csum on RX/TX. They can be selected independently.
tx Partial csum over IPv4. -> NETIF_F_IP_CSUM
tx Full csum. -> NETIF_F_HW_CSUM
rx Full csum -> NETIF_F_RXCSUM
Is there any option to support partial csum?
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
@@ -1052,12 +1052,14 @@ static int __devinit temac_of_probe(struct platform_device *op)/* Setup checksum offload, but default to off if not specified */lp->temac_features=0;p=(__be32*)of_get_property(op->dev.of_node,"xlnx,txcsum",NULL);+dev_info(&op->dev,"TX_CSUM %d\n",be32_to_cpup(p));if(p&&be32_to_cpu(*p)){lp->temac_features|=TEMAC_FEATURE_TX_CSUM;/* Can checksum TCP/UDP over IPv4. */ndev->features|=NETIF_F_IP_CSUM;}p=(__be32*)of_get_property(op->dev.of_node,"xlnx,rxcsum",NULL);+dev_info(&op->dev,"RX_CSUM %d\n",be32_to_cpup(p));
[...]
Is there any particular reason you think this needs to be logged by
default, rather than letting users run ethtool -k? I suggest using
dev_dbg() instead.
The reason was just to show it in the bootlog.
I will check ethtool support for these drivers.
'ethtool -k' should just work so long as the driver sets the right bits
in ndev->features.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
@@ -1052,12 +1052,14 @@ static int __devinit temac_of_probe(struct platform_device *op)/* Setup checksum offload, but default to off if not specified */lp->temac_features=0;p=(__be32*)of_get_property(op->dev.of_node,"xlnx,txcsum",NULL);+dev_info(&op->dev,"TX_CSUM %d\n",be32_to_cpup(p));if(p&&be32_to_cpu(*p)){lp->temac_features|=TEMAC_FEATURE_TX_CSUM;/* Can checksum TCP/UDP over IPv4. */ndev->features|=NETIF_F_IP_CSUM;}p=(__be32*)of_get_property(op->dev.of_node,"xlnx,rxcsum",NULL);+dev_info(&op->dev,"RX_CSUM %d\n",be32_to_cpup(p));
[...]
Is there any particular reason you think this needs to be logged by
default, rather than letting users run ethtool -k? I suggest using
dev_dbg() instead.
Ok. I have looked at it and there are missing some bits in ndev->features.
Can you please check that my setting is correct?
It is SG DMA ip/driver.
ndev->features = NETIF_F_FRAGLIST | NETIF_F_SG
NETIF_F_SG only; NETIF_F_FRAGLIST means you can handle skbs chained
through the frag_list pointer.
With two options for csum on RX/TX. They can be selected independently.
tx Partial csum over IPv4. -> NETIF_F_IP_CSUM
tx Full csum. -> NETIF_F_HW_CSUM
NETIF_F_IP_CSUM means you have hardware checksum generation for TCP/IPv4
and UDP/IPv4 only (XAE_FEATURE_FULL_TX_CSUM).
NETIF_F_HW_CSUM means you have generic TCP-style checksum generation
using the csum_start and csum_offset fields of the skb
(XAE_FEATURE_PARTIAL_TX_CSUM). By the way, you're actually testing
XAE_FEATURE_PARTIAL_RX_CSUM in axienet_start_xmit()...
rx Full csum -> NETIF_F_RXCSUM
Is there any option to support partial csum?
There is no need to differentiate these in the device features. For TX
the stack needs to know whether to use a software fallback before
passing the skb to you, but on RX it looks at the ip_summed field of
each skb you pass up.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
@@ -1052,12 +1052,14 @@ static int __devinit temac_of_probe(struct platform_device *op)/* Setup checksum offload, but default to off if not specified */lp->temac_features=0;p=(__be32*)of_get_property(op->dev.of_node,"xlnx,txcsum",NULL);+dev_info(&op->dev,"TX_CSUM %d\n",be32_to_cpup(p));if(p&&be32_to_cpu(*p)){lp->temac_features|=TEMAC_FEATURE_TX_CSUM;/* Can checksum TCP/UDP over IPv4. */ndev->features|=NETIF_F_IP_CSUM;}p=(__be32*)of_get_property(op->dev.of_node,"xlnx,rxcsum",NULL);+dev_info(&op->dev,"RX_CSUM %d\n",be32_to_cpup(p));
[...]
Is there any particular reason you think this needs to be logged by
default, rather than letting users run ethtool -k? I suggest using
dev_dbg() instead.
Ok. I have looked at it and there are missing some bits in ndev->features.
Can you please check that my setting is correct?
It is SG DMA ip/driver.
ndev->features = NETIF_F_FRAGLIST | NETIF_F_SG
NETIF_F_SG only; NETIF_F_FRAGLIST means you can handle skbs chained
through the frag_list pointer.
The driver is able to handle skb fragments too. temac_start_xmit
quoted
With two options for csum on RX/TX. They can be selected independently.
tx Partial csum over IPv4. -> NETIF_F_IP_CSUM
tx Full csum. -> NETIF_F_HW_CSUM
NETIF_F_IP_CSUM means you have hardware checksum generation for TCP/IPv4
and UDP/IPv4 only (XAE_FEATURE_FULL_TX_CSUM).
NETIF_F_HW_CSUM means you have generic TCP-style checksum generation
using the csum_start and csum_offset fields of the skb
(XAE_FEATURE_PARTIAL_TX_CSUM). By the way, you're actually testing
XAE_FEATURE_PARTIAL_RX_CSUM in axienet_start_xmit()...
We have used non mainline ll_temac driver for a long time but we will
move to this mainline version soon. It is on my todo list to clean this
driver and test all these options.
Also performance tests will be necessary to do.
quoted
rx Full csum -> NETIF_F_RXCSUM
Is there any option to support partial csum?
There is no need to differentiate these in the device features. For TX
the stack needs to know whether to use a software fallback before
passing the skb to you, but on RX it looks at the ip_summed field of
each skb you pass up.
Hardware can be setup asymmetrically. It means enable CSUM only on RX or TX.
All combination are valid.
The point here is if Linux is not able to handle this then we have to
create logic in the driver to support these options too.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
@@ -1052,12 +1052,14 @@ static int __devinit temac_of_probe(struct platform_device *op)/* Setup checksum offload, but default to off if not specified */lp->temac_features=0;p=(__be32*)of_get_property(op->dev.of_node,"xlnx,txcsum",NULL);+dev_info(&op->dev,"TX_CSUM %d\n",be32_to_cpup(p));if(p&&be32_to_cpu(*p)){lp->temac_features|=TEMAC_FEATURE_TX_CSUM;/* Can checksum TCP/UDP over IPv4. */ndev->features|=NETIF_F_IP_CSUM;}p=(__be32*)of_get_property(op->dev.of_node,"xlnx,rxcsum",NULL);+dev_info(&op->dev,"RX_CSUM %d\n",be32_to_cpup(p));
[...]
Is there any particular reason you think this needs to be logged by
default, rather than letting users run ethtool -k? I suggest using
dev_dbg() instead.
Ok. I have looked at it and there are missing some bits in ndev->features.
Can you please check that my setting is correct?
It is SG DMA ip/driver.
ndev->features = NETIF_F_FRAGLIST | NETIF_F_SG
NETIF_F_SG only; NETIF_F_FRAGLIST means you can handle skbs chained
through the frag_list pointer.
The driver is able to handle skb fragments too. temac_start_xmit
rx Full csum -> NETIF_F_RXCSUM
Is there any option to support partial csum?
There is no need to differentiate these in the device features. For TX
the stack needs to know whether to use a software fallback before
passing the skb to you, but on RX it looks at the ip_summed field of
each skb you pass up.
Hardware can be setup asymmetrically. It means enable CSUM only on RX or TX.
All combination are valid.
The point here is if Linux is not able to handle this then we have to
create logic in the driver to support these options too.
Linux handles this just fine. The point is you don't have to tell the
stack in advance whether or what kind of RX checksum validation your
devices will do. (In fact the only reason that feature flag exists at
all is so that it can be generically exposed through the ethtool API.)
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
@@ -1052,12 +1052,14 @@ static int __devinit temac_of_probe(struct platform_device *op)/* Setup checksum offload, but default to off if not specified */lp->temac_features=0;p=(__be32*)of_get_property(op->dev.of_node,"xlnx,txcsum",NULL);+dev_info(&op->dev,"TX_CSUM %d\n",be32_to_cpup(p));if(p&&be32_to_cpu(*p)){lp->temac_features|=TEMAC_FEATURE_TX_CSUM;/* Can checksum TCP/UDP over IPv4. */ndev->features|=NETIF_F_IP_CSUM;}p=(__be32*)of_get_property(op->dev.of_node,"xlnx,rxcsum",NULL);+dev_info(&op->dev,"RX_CSUM %d\n",be32_to_cpup(p));
[...]
Is there any particular reason you think this needs to be logged by
default, rather than letting users run ethtool -k? I suggest using
dev_dbg() instead.
Ok. I have looked at it and there are missing some bits in ndev->features.
Can you please check that my setting is correct?
It is SG DMA ip/driver.
ndev->features = NETIF_F_FRAGLIST | NETIF_F_SG
NETIF_F_SG only; NETIF_F_FRAGLIST means you can handle skbs chained
through the frag_list pointer.
The driver is able to handle skb fragments too. temac_start_xmit
"git grep -w -E 'frag_list|skb_walk_frags|skb_to_sgvec' drivers/net/ethernet/xilinx"
returns nothing in net-next.
What about this? Maybe it is different fragmentation.
drivers/net/ethernet/xilinx/ll_temac_main.c
688 num_frag = skb_shinfo(skb)->nr_frags;
689 frag = &skb_shinfo(skb)->frags[0];
[...]
quoted
quoted
quoted
rx Full csum -> NETIF_F_RXCSUM
Is there any option to support partial csum?
There is no need to differentiate these in the device features. For TX
the stack needs to know whether to use a software fallback before
passing the skb to you, but on RX it looks at the ip_summed field of
each skb you pass up.
Hardware can be setup asymmetrically. It means enable CSUM only on RX or TX.
All combination are valid.
The point here is if Linux is not able to handle this then we have to
create logic in the driver to support these options too.
Linux handles this just fine. The point is you don't have to tell the
stack in advance whether or what kind of RX checksum validation your
devices will do. (In fact the only reason that feature flag exists at
all is so that it can be generically exposed through the ethtool API.)
Ok.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
@@ -1052,12 +1052,14 @@ static int __devinit temac_of_probe(struct platform_device *op)/* Setup checksum offload, but default to off if not specified */lp->temac_features=0;p=(__be32*)of_get_property(op->dev.of_node,"xlnx,txcsum",NULL);+dev_info(&op->dev,"TX_CSUM %d\n",be32_to_cpup(p));if(p&&be32_to_cpu(*p)){lp->temac_features|=TEMAC_FEATURE_TX_CSUM;/* Can checksum TCP/UDP over IPv4. */ndev->features|=NETIF_F_IP_CSUM;}p=(__be32*)of_get_property(op->dev.of_node,"xlnx,rxcsum",NULL);+dev_info(&op->dev,"RX_CSUM %d\n",be32_to_cpup(p));
[...]
Is there any particular reason you think this needs to be logged by
default, rather than letting users run ethtool -k? I suggest using
dev_dbg() instead.
Ok. I have looked at it and there are missing some bits in ndev->features.
Can you please check that my setting is correct?
It is SG DMA ip/driver.
ndev->features = NETIF_F_FRAGLIST | NETIF_F_SG
NETIF_F_SG only; NETIF_F_FRAGLIST means you can handle skbs chained
through the frag_list pointer.
The driver is able to handle skb fragments too. temac_start_xmit
"git grep -w -E 'frag_list|skb_walk_frags|skb_to_sgvec' drivers/net/ethernet/xilinx"
returns nothing in net-next.
What about this? Maybe it is different fragmentation.
drivers/net/ethernet/xilinx/ll_temac_main.c
688 num_frag = skb_shinfo(skb)->nr_frags;
689 frag = &skb_shinfo(skb)->frags[0];
[...]
Yes, the fact that you handle the frags array is indicated by feature
flag NETIF_F_SG.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.