Re: [PATCH V2 2/4] MIPS: Add board support for Loongson1B

From: Kelvin Cheung
Date: Wed Oct 26 2011 - 00:28:07 EST


Hi Giuseppe,

This patch works well on Loongson1B platform except one thing.
The rx checksum offload of normal descriptor is disabled by default.
So, I enabled this functon. And one minor tweak is added to your patch.
What about your opinion?

BTW, tx checksum insertion works now.

2011/10/25, Giuseppe CAVALLARO <peppe.cavallaro@xxxxxx>:
> On 10/25/2011 9:09 AM, Giuseppe CAVALLARO wrote:
>> On 10/25/2011 4:12 AM, Kelvin Cheung wrote:
>>> 2011/10/24, Giuseppe CAVALLARO <peppe.cavallaro@xxxxxx>:
>>>> On 10/24/2011 4:05 PM, Kelvin Cheung wrote:
>>>>> 2011/10/24, Giuseppe CAVALLARO <peppe.cavallaro@xxxxxx>:
>>>>>> Hello Kelvin.
>>>>>>
>>>>>> On 10/24/2011 12:36 PM, Kelvin Cheung wrote:
>>>>>>
>>>>>> [snip]
>>>>>>
>>>>>>> According to datasheet of Loongson 1B, the buffer size in RX/TX
>>>>>>> descriptor is only 2KB. So the Loongson1B's GMAC could not handle
>>>>>>> jumbo frames. And the second buffer is useless in this case. Am I
>>>>>>> right? Is there a better way than ifdef CONFIG_MACH_LOONGSON1 to
>>>>>>> avoid duplicate code?
>>>>>>
>>>>>> Sorry for my misunderstanding.
>>>>>>
>>>>>> I think you have to use the normal descriptor and remove the enh_desc
>>>>>> from the platform w/o modifying the driver at all.
>>>>>>
>>>>>> The driver will be able to select/configure all automatically (also
>>>>>> jumbo).
>>>>>>
>>>>>> Let me know.
>>>>>
>>>>> That's the problem.
>>>>> The bitfield definition of Loongson1B is also different from normal
>>>>> descriptor.
>>>>
>>>> The problem is not in the Loongson1B gmac.
>>>
>>> I found that the bit checksum_insertion is not existed in normal
>>> descriptor.
>>>
>>>> The normal descriptor fields in the stmmac refer to an old synopsys
>>>> databook.
>>>
>>> Could you send me the new databook of Synopsys GMAC?
>>>
>>>> New chips have the same structure you have added; so we should fix this
>>>> in the driver w/o breaking the compatibility for old chips.
>>>
>>> Agree.
>>>
>>>> I kindly ask you to confirm if the currently normal descriptor structure
>>>> (w/o your changes) doesn't work on your platform.
>>>> Did you test it?
>>>
>>> Well, the normal descriptor works on my platform except TX checksum
>>> offload.
>>
>> ok! I suspected that.
>>
>>
>>>>> Moreover, I want to enable the TX checksum offload function which is
>>>>> not supported in normal descriptor.
>>>>> Any suggestions?
>>>>
>>>> It is supported but you have to pass from the platform: tx_coe = 1.
>>>
>>> I noticed that the flag csum_insertion is passed to
>>> ndesc_prepare_tx_desc() in stmmac_xmit(). But ndesc_prepare_tx_desc()
>>> just ignores it.
>>> In other words, the TX checksum offload function is disabled in normal
>>> descriptor currently.
>>>
>>> Should we fix this problem for normal descriptor?
>>
>> Yes, we should. If you agree, I'll update the normal descriptor
>> structure to yours. This is the normal descriptor used in newer GMAC.
>> Tx csum will be done for normal descriptors in case of these GMAC
>> devices and not for old MAC10/100. For the MAC10/100 some bits for
>> normal descriptors are reserved and won't be used at all.
>>
>> I'll also verify that the patch doesn't break the back-compatibility
>> with old MAC10/100. I have the HW where doing the tests.
>>
>> After that, I'll prepare the patch for net-next and for your kernel.
>
> Hello Kelvin
>
> attached the patch tested on my development kernel.
> It runs fine on old and new mac devices.
>
> Can you try it on your side? Hmm, it is likely it won't apply fine on
> your tree but you know the changes ;-).
>
> If ok, I'll rework it for net-next and send it to the mailing list.
>
> Thanks
> Peppe
>
>>
>>>
>>>> Peppe
>>>>>
>>>>>> Note:
>>>>>> IIRC, there is a bit difference in case of normal descriptors for
>>>>>> Synopsys databook newer than the 1.91 (I used for testing this mode).
>>>>>> In any case, I remember that, on some platforms, the normal
>>>>>> descriptors
>>>>>> have been used w/o problems also on these new chip generations.
>>>>>>
>>>>>> Peppe
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
>


--
Best Regards!
Kelvin
From 93ee3effbde61013a886d579fc89c36ddddb35a4 Mon Sep 17 00:00:00 2001
From: Kelvin Cheung <keguang.zhang@xxxxxxxxx>
Date: Wed, 26 Oct 2011 11:08:01 +0800
Subject: [PATCH] stmmac: update normal descriptor structure

This patch updates the normal descriptor structure
to work fine on new GMAC Synopsys chips.

Normal descriptors were designed on the old MAC10/100
databook 1.91 where some bits were reserved: for example
the tx checksum insertion and rx checksum offload.

The patch maintains the back-compatibility with old
MAC devices (tested on STx7109 MAC10/100) and adds new
fields that actually new GMAC devices can use.

For example, STx7109 will pass from the platform
tx_coe = 0, enh_desc = 0, has_gmac = 0.
A platform like Loongson1B (Mips) will pass:
tx_coe = 1, enh_desc = 0, has_gmac = 1.

Thanks to Kelvin, he enhanced the normal descriptors for
GMAC on Loongson1B.

Signed-off-by: Kelvin Cheung <keguang.zhang@xxxxxxxxx>
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@xxxxxx>
---
drivers/net/stmmac/common.h | 8 +++---
drivers/net/stmmac/descs.h | 31 +++++++++++++++-----------
drivers/net/stmmac/norm_desc.c | 40 +++++++++++++++++++---------------
drivers/net/stmmac/stmmac_ethtool.c | 8 +++---
4 files changed, 48 insertions(+), 39 deletions(-)

diff --git a/drivers/net/stmmac/common.h b/drivers/net/stmmac/common.h
index 375ea19..6ec4a8c 100644
--- a/drivers/net/stmmac/common.h
+++ b/drivers/net/stmmac/common.h
@@ -48,7 +48,7 @@ struct stmmac_extra_stats {
unsigned long tx_underflow ____cacheline_aligned;
unsigned long tx_carrier;
unsigned long tx_losscarrier;
- unsigned long tx_heartbeat;
+ unsigned long vlan_tag;
unsigned long tx_deferred;
unsigned long tx_vlan;
unsigned long tx_jabber;
@@ -57,9 +57,9 @@ struct stmmac_extra_stats {
unsigned long tx_ip_header_error;
/* Receive errors */
unsigned long rx_desc;
- unsigned long rx_partial;
- unsigned long rx_runt;
- unsigned long rx_toolong;
+ unsigned long sa_filter_fail;
+ unsigned long overflow_error;
+ unsigned long ipc_csum_error;
unsigned long rx_collision;
unsigned long rx_crc;
unsigned long rx_length;
diff --git a/drivers/net/stmmac/descs.h b/drivers/net/stmmac/descs.h
index 63a03e2..9820ec8 100644
--- a/drivers/net/stmmac/descs.h
+++ b/drivers/net/stmmac/descs.h
@@ -25,33 +25,34 @@ struct dma_desc {
union {
struct {
/* RDES0 */
- u32 reserved1:1;
+ u32 payload_csum_error:1;
u32 crc_error:1;
u32 dribbling:1;
u32 mii_error:1;
u32 receive_watchdog:1;
u32 frame_type:1;
u32 collision:1;
- u32 frame_too_long:1;
+ u32 ipc_csum_error:1;
u32 last_descriptor:1;
u32 first_descriptor:1;
- u32 multicast_frame:1;
- u32 run_frame:1;
+ u32 vlan_tag:1;
+ u32 overflow_error:1;
u32 length_error:1;
- u32 partial_frame_error:1;
+ u32 sa_filter_fail:1;
u32 descriptor_error:1;
u32 error_summary:1;
u32 frame_length:14;
- u32 filtering_fail:1;
+ u32 da_filter_fail:1;
u32 own:1;
/* RDES1 */
u32 buffer1_size:11;
u32 buffer2_size:11;
- u32 reserved2:2;
+ u32 reserved1:2;
u32 second_address_chained:1;
u32 end_ring:1;
- u32 reserved3:5;
+ u32 reserved2:5;
u32 disable_ic:1;
+
} rx;
struct {
/* RDES0 */
@@ -91,24 +92,28 @@ struct dma_desc {
u32 underflow_error:1;
u32 excessive_deferral:1;
u32 collision_count:4;
- u32 heartbeat_fail:1;
+ u32 vlan_frame:1;
u32 excessive_collisions:1;
u32 late_collision:1;
u32 no_carrier:1;
u32 loss_carrier:1;
- u32 reserved1:3;
+ u32 payload_error:1;
+ u32 frame_flushed:1;
+ u32 jabber_timeout:1;
u32 error_summary:1;
- u32 reserved2:15;
+ u32 ip_header_error:1;
+ u32 time_stamp_status:1;
+ u32 reserved1:13;
u32 own:1;
/* TDES1 */
u32 buffer1_size:11;
u32 buffer2_size:11;
- u32 reserved3:1;
+ u32 time_stamp_enable:1;
u32 disable_padding:1;
u32 second_address_chained:1;
u32 end_ring:1;
u32 crc_disable:1;
- u32 reserved4:2;
+ u32 checksum_insertion:2;
u32 first_segment:1;
u32 last_segment:1;
u32 interrupt:1;
diff --git a/drivers/net/stmmac/norm_desc.c b/drivers/net/stmmac/norm_desc.c
index 7008c29..fab8f0c 100644
--- a/drivers/net/stmmac/norm_desc.c
+++ b/drivers/net/stmmac/norm_desc.c
@@ -49,11 +49,12 @@ static int ndesc_get_tx_status(void *data, struct stmmac_extra_stats *x,
stats->collisions += p->des01.tx.collision_count;
ret = -1;
}
- if (unlikely(p->des01.tx.heartbeat_fail)) {
- x->tx_heartbeat++;
- stats->tx_heartbeat_errors++;
- ret = -1;
- }
+
+ if (p->des01.etx.vlan_frame) {
+ CHIP_DBG(KERN_INFO "GMAC TX status: VLAN frame\n");
+ x->tx_vlan++;
+ }
+
if (unlikely(p->des01.tx.deferred))
x->tx_deferred++;

@@ -66,13 +67,11 @@ static int ndesc_get_tx_len(struct dma_desc *p)
}

/* This function verifies if each incoming frame has some errors
- * and, if required, updates the multicast statistics.
- * In case of success, it returns csum_none because the device
- * is not able to compute the csum in HW. */
+ * and, if required, updates the multicast statistics. */
static int ndesc_get_rx_status(void *data, struct stmmac_extra_stats *x,
struct dma_desc *p)
{
- int ret = csum_none;
+ int ret = good_frame;
struct net_device_stats *stats = (struct net_device_stats *)data;

if (unlikely(p->des01.rx.last_descriptor == 0)) {
@@ -85,12 +84,12 @@ static int ndesc_get_rx_status(void *data, struct stmmac_extra_stats *x,
if (unlikely(p->des01.rx.error_summary)) {
if (unlikely(p->des01.rx.descriptor_error))
x->rx_desc++;
- if (unlikely(p->des01.rx.partial_frame_error))
- x->rx_partial++;
- if (unlikely(p->des01.rx.run_frame))
- x->rx_runt++;
- if (unlikely(p->des01.rx.frame_too_long))
- x->rx_toolong++;
+ if (unlikely(p->des01.rx.sa_filter_fail))
+ x->sa_filter_fail++;
+ if (unlikely(p->des01.rx.overflow_error))
+ x->overflow_error++;
+ if (unlikely(p->des01.rx.ipc_csum_error))
+ x->ipc_csum_error++;
if (unlikely(p->des01.rx.collision)) {
x->rx_collision++;
stats->collisions++;
@@ -112,10 +111,12 @@ static int ndesc_get_rx_status(void *data, struct stmmac_extra_stats *x,
x->rx_mii++;
ret = discard_frame;
}
- if (p->des01.rx.multicast_frame) {
- x->rx_multicast++;
- stats->multicast++;
+#ifdef STMMAC_VLAN_TAG_USED
+ if (p->des01.rx.vlan_tag) {
+ x->vlan_tag++;
+ stats->vlan_tag++;
}
+#endif
return ret;
}

@@ -184,6 +185,9 @@ static void ndesc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
{
p->des01.tx.first_segment = is_fs;
norm_set_tx_desc_len(p, len);
+
+ if (likely(csum_flag))
+ p->des01.tx.checksum_insertion = cic_full;
}

static void ndesc_clear_tx_ic(struct dma_desc *p)
diff --git a/drivers/net/stmmac/stmmac_ethtool.c b/drivers/net/stmmac/stmmac_ethtool.c
index 7ed8fb6..3d58fcc 100644
--- a/drivers/net/stmmac/stmmac_ethtool.c
+++ b/drivers/net/stmmac/stmmac_ethtool.c
@@ -50,7 +50,7 @@ static const struct stmmac_stats stmmac_gstrings_stats[] = {
STMMAC_STAT(tx_underflow),
STMMAC_STAT(tx_carrier),
STMMAC_STAT(tx_losscarrier),
- STMMAC_STAT(tx_heartbeat),
+ STMMAC_STAT(vlan_tag),
STMMAC_STAT(tx_deferred),
STMMAC_STAT(tx_vlan),
STMMAC_STAT(rx_vlan),
@@ -59,9 +59,9 @@ static const struct stmmac_stats stmmac_gstrings_stats[] = {
STMMAC_STAT(tx_payload_error),
STMMAC_STAT(tx_ip_header_error),
STMMAC_STAT(rx_desc),
- STMMAC_STAT(rx_partial),
- STMMAC_STAT(rx_runt),
- STMMAC_STAT(rx_toolong),
+ STMMAC_STAT(sa_filter_fail),
+ STMMAC_STAT(overflow_error),
+ STMMAC_STAT(ipc_csum_error),
STMMAC_STAT(rx_collision),
STMMAC_STAT(rx_crc),
STMMAC_STAT(rx_length),
--
1.7.1