* Re: [PATCH V2 2/4] MIPS: Add board support for Loongson1B [not found] ` <1319192888-21465-2-git-send-email-keguang.zhang@gmail.com> @ 2011-10-21 17:33 ` Wu Zhangjin 2011-10-24 7:19 ` Giuseppe CAVALLARO 0 siblings, 1 reply; 14+ messages in thread From: Wu Zhangjin @ 2011-10-21 17:33 UTC (permalink / raw) To: keguang.zhang; +Cc: linux-mips, linux-kernel, ralf, r0bertz, netdev On Fri, Oct 21, 2011 at 6:28 PM, <keguang.zhang@gmail•com> wrote: > From: Kelvin Cheung <keguang.zhang@gmail•com> > > This patch adds basic platform support for Loongson1B > including serial port, ethernet, and interrupt handler. > > Loongson1B UART is compatible with NS16550A. > Loongson1B GMAC is built around Synopsys IP Core. > Perhaps you'd better split out the GMAC support to its own patch and send it to the net/ maintainer and the authors of the original files. > diff --git a/drivers/net/stmmac/descs.h b/drivers/net/stmmac/descs.h > index 63a03e2..4db27d0 100644 > --- a/drivers/net/stmmac/descs.h > +++ b/drivers/net/stmmac/descs.h > @@ -53,6 +53,38 @@ struct dma_desc { > u32 reserved3:5; > u32 disable_ic:1; > } rx; > +#ifdef CONFIG_MACH_LOONGSON1 > + struct { > + /* RDES0 */ > + u32 payload_csum_error:1; > + u32 crc_error:1; > + u32 dribbling:1; > + u32 error_gmii:1; > + u32 receive_watchdog:1; > + u32 frame_type:1; > + u32 late_collision:1; > + u32 ipc_csum_error:1; > + u32 last_descriptor:1; > + u32 first_descriptor:1; > + u32 vlan_tag:1; > + u32 overflow_error:1; > + u32 length_error:1; > + u32 sa_filter_fail:1; > + u32 descriptor_error:1; > + u32 error_summary:1; > + u32 frame_length:14; > + u32 da_filter_fail:1; > + u32 own:1; > + /* RDES1 */ > + u32 buffer1_size:11; > + u32 buffer2_size:11; > + u32 reserved1:2; > + u32 second_address_chained:1; > + u32 end_ring:1; > + u32 reserved2:5; > + u32 disable_ic:1; > + } erx; /* -- enhanced -- */ > +#else > struct { > /* RDES0 */ > u32 payload_csum_error:1; > @@ -83,6 +115,7 @@ struct dma_desc { > u32 reserved2:2; > u32 disable_ic:1; > } erx; /* -- enhanced -- */ > +#endif > > /* Transmit descriptor */ > struct { > @@ -113,6 +146,40 @@ struct dma_desc { > u32 last_segment:1; > u32 interrupt:1; > } tx; > +#ifdef CONFIG_MACH_LOONGSON1 > + struct { > + /* TDES0 */ > + u32 deferred:1; > + u32 underflow_error:1; > + u32 excessive_deferral:1; > + u32 collision_count:4; > + u32 vlan_frame:1; > + u32 excessive_collisions:1; > + u32 late_collision:1; > + u32 no_carrier:1; > + u32 loss_carrier:1; > + u32 payload_error:1; > + u32 frame_flushed:1; > + u32 jabber_timeout:1; > + u32 error_summary:1; > + 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 time_stamp_enable:1; > + u32 disable_padding:1; > + u32 second_address_chained:1; > + u32 end_ring:1; > + u32 crc_disable:1; > + u32 checksum_insertion:2; > + u32 first_segment:1; > + u32 last_segment:1; > + u32 interrupt:1; > + } etx; /* -- enhanced -- */ > +#else > struct { > /* TDES0 */ > u32 deferred:1; > @@ -148,6 +215,7 @@ struct dma_desc { > u32 buffer2_size:13; > u32 reserved4:3; > } etx; /* -- enhanced -- */ > +#endif > } des01; > unsigned int des2; > unsigned int des3; If the difference is very much, perhaps a new dma_desc struct can be defined instead. > diff --git a/drivers/net/stmmac/enh_desc.c b/drivers/net/stmmac/enh_desc.c > index e5dfb6a..3b5e4f1 100644 > --- a/drivers/net/stmmac/enh_desc.c > +++ b/drivers/net/stmmac/enh_desc.c > @@ -108,6 +108,7 @@ static int enh_desc_get_tx_len(struct dma_desc *p) > static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err) > { > int ret = good_frame; > +#ifndef CONFIG_MACH_LOONGSON1 > u32 status = (type << 2 | ipc_err << 1 | payload_err) & 0x7; > > /* bits 5 7 0 | Frame status > @@ -145,6 +146,7 @@ static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err) > CHIP_DBG(KERN_ERR "RX Des0 status: No IPv4, IPv6 frame.\n"); > ret = discard_frame; > } > +#endif > return ret; > } > > @@ -232,9 +234,17 @@ static void enh_desc_init_rx_desc(struct dma_desc *p, unsigned int ring_size, > int i; > for (i = 0; i < ring_size; i++) { > p->des01.erx.own = 1; > +#ifdef CONFIG_MACH_LOONGSON1 > + p->des01.erx.buffer1_size = BUF_SIZE_2KiB - 1; > +#else > p->des01.erx.buffer1_size = BUF_SIZE_8KiB - 1; > +#endif > /* To support jumbo frames */ > +#ifdef CONFIG_MACH_LOONGSON1 > + p->des01.erx.buffer2_size = BUF_SIZE_2KiB - 1; > +#else > p->des01.erx.buffer2_size = BUF_SIZE_8KiB - 1; > +#endif > if (i == ring_size - 1) > p->des01.erx.end_ring = 1; > if (disable_rx_ic) > @@ -292,9 +302,15 @@ static void enh_desc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len, > int csum_flag) > { > p->des01.etx.first_segment = is_fs; > +#ifdef CONFIG_MACH_LOONGSON1 > + if (unlikely(len > BUF_SIZE_2KiB)) { > + p->des01.etx.buffer1_size = BUF_SIZE_2KiB - 1; > + p->des01.etx.buffer2_size = len - BUF_SIZE_2KiB + 1; > +#else > if (unlikely(len > BUF_SIZE_4KiB)) { > p->des01.etx.buffer1_size = BUF_SIZE_4KiB; > p->des01.etx.buffer2_size = len - BUF_SIZE_4KiB; > +#endif > } else { > p->des01.etx.buffer1_size = len; > } Is it possible to add two new macros RX_BUF_SIZE and TX_BUF_SIZE to .h instead? which may reduce code duplication. Regards, Wu Zhangjin > -- > 1.7.1 > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2 2/4] MIPS: Add board support for Loongson1B 2011-10-21 17:33 ` [PATCH V2 2/4] MIPS: Add board support for Loongson1B Wu Zhangjin @ 2011-10-24 7:19 ` Giuseppe CAVALLARO 2011-10-24 10:36 ` Kelvin Cheung 0 siblings, 1 reply; 14+ messages in thread From: Giuseppe CAVALLARO @ 2011-10-24 7:19 UTC (permalink / raw) To: keguang.zhang Cc: Wu Zhangjin, linux-mips, linux-kernel, ralf, r0bertz, netdev On 10/21/2011 7:33 PM, Wu Zhangjin wrote: > On Fri, Oct 21, 2011 at 6:28 PM, <keguang.zhang@gmail•com> wrote: >> From: Kelvin Cheung <keguang.zhang@gmail•com> >> >> This patch adds basic platform support for Loongson1B >> including serial port, ethernet, and interrupt handler. >> >> Loongson1B UART is compatible with NS16550A. >> Loongson1B GMAC is built around Synopsys IP Core. >> > > Perhaps you'd better split out the GMAC support to its own patch and > send it to the net/ maintainer and the authors of the original files. Also suggest you to provide the stmmac patches for net-next. The stmmac driver has been recently updated and I've also several patches to commit (for example for PCI etc) on it. I'm happy that the support for big endianess arrived. I supported a guy some time ago but he didn't provided me the patches tested on his side :-(. So welcome yours and many thanks Kelvin! Please send the stmmac patches and add me on CC. I'm happy to help you on reviewing them. >> diff --git a/drivers/net/stmmac/descs.h b/drivers/net/stmmac/descs.h >> index 63a03e2..4db27d0 100644 >> --- a/drivers/net/stmmac/descs.h >> +++ b/drivers/net/stmmac/descs.h >> @@ -53,6 +53,38 @@ struct dma_desc { >> u32 reserved3:5; >> u32 disable_ic:1; >> } rx; >> +#ifdef CONFIG_MACH_LOONGSON1 >> + struct { >> + /* RDES0 */ >> + u32 payload_csum_error:1; >> + u32 crc_error:1; >> + u32 dribbling:1; >> + u32 error_gmii:1; >> + u32 receive_watchdog:1; >> + u32 frame_type:1; >> + u32 late_collision:1; >> + u32 ipc_csum_error:1; >> + u32 last_descriptor:1; >> + u32 first_descriptor:1; >> + u32 vlan_tag:1; >> + u32 overflow_error:1; >> + u32 length_error:1; >> + u32 sa_filter_fail:1; >> + u32 descriptor_error:1; >> + u32 error_summary:1; >> + u32 frame_length:14; >> + u32 da_filter_fail:1; >> + u32 own:1; >> + /* RDES1 */ >> + u32 buffer1_size:11; >> + u32 buffer2_size:11; >> + u32 reserved1:2; >> + u32 second_address_chained:1; >> + u32 end_ring:1; >> + u32 reserved2:5; >> + u32 disable_ic:1; >> + } erx; /* -- enhanced -- */ >> +#else >> struct { >> /* RDES0 */ >> u32 payload_csum_error:1; >> @@ -83,6 +115,7 @@ struct dma_desc { >> u32 reserved2:2; >> u32 disable_ic:1; >> } erx; /* -- enhanced -- */ >> +#endif >> >> /* Transmit descriptor */ >> struct { >> @@ -113,6 +146,40 @@ struct dma_desc { >> u32 last_segment:1; >> u32 interrupt:1; >> } tx; >> +#ifdef CONFIG_MACH_LOONGSON1 >> + struct { >> + /* TDES0 */ >> + u32 deferred:1; >> + u32 underflow_error:1; >> + u32 excessive_deferral:1; >> + u32 collision_count:4; >> + u32 vlan_frame:1; >> + u32 excessive_collisions:1; >> + u32 late_collision:1; >> + u32 no_carrier:1; >> + u32 loss_carrier:1; >> + u32 payload_error:1; >> + u32 frame_flushed:1; >> + u32 jabber_timeout:1; >> + u32 error_summary:1; >> + 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 time_stamp_enable:1; >> + u32 disable_padding:1; >> + u32 second_address_chained:1; >> + u32 end_ring:1; >> + u32 crc_disable:1; >> + u32 checksum_insertion:2; >> + u32 first_segment:1; >> + u32 last_segment:1; >> + u32 interrupt:1; >> + } etx; /* -- enhanced -- */ >> +#else >> struct { >> /* TDES0 */ >> u32 deferred:1; >> @@ -148,6 +215,7 @@ struct dma_desc { >> u32 buffer2_size:13; >> u32 reserved4:3; >> } etx; /* -- enhanced -- */ >> +#endif >> } des01; >> unsigned int des2; >> unsigned int des3; > > > If the difference is very much, perhaps a new dma_desc struct can be > defined instead. > Concerning the descriptors, we could have two different files: descs_le.h descs_be.h and select their inclusion inside the common.h. Please use instead of the macro CONFIG_MACH_LOONGSON1 another one more generic e.g. CONFIG_STMMAC_BE (and add it in the driver's Kconfig). On your platform you will have to enable it by default. Or it could be the default on MIPS: LE will be on ARM and SuperH. >> diff --git a/drivers/net/stmmac/enh_desc.c b/drivers/net/stmmac/enh_desc.c >> index e5dfb6a..3b5e4f1 100644 >> --- a/drivers/net/stmmac/enh_desc.c >> +++ b/drivers/net/stmmac/enh_desc.c >> @@ -108,6 +108,7 @@ static int enh_desc_get_tx_len(struct dma_desc *p) >> static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err) >> { >> int ret = good_frame; >> +#ifndef CONFIG_MACH_LOONGSON1 >> u32 status = (type << 2 | ipc_err << 1 | payload_err) & 0x7; >> >> /* bits 5 7 0 | Frame status >> @@ -145,6 +146,7 @@ static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err) >> CHIP_DBG(KERN_ERR "RX Des0 status: No IPv4, IPv6 frame.\n"); >> ret = discard_frame; >> } >> +#endif >> return ret; >> } >> >> @@ -232,9 +234,17 @@ static void enh_desc_init_rx_desc(struct dma_desc *p, unsigned int ring_size, >> int i; >> for (i = 0; i < ring_size; i++) { >> p->des01.erx.own = 1; >> +#ifdef CONFIG_MACH_LOONGSON1 >> + p->des01.erx.buffer1_size = BUF_SIZE_2KiB - 1; >> +#else >> p->des01.erx.buffer1_size = BUF_SIZE_8KiB - 1; >> +#endif >> /* To support jumbo frames */ >> +#ifdef CONFIG_MACH_LOONGSON1 >> + p->des01.erx.buffer2_size = BUF_SIZE_2KiB - 1; >> +#else >> p->des01.erx.buffer2_size = BUF_SIZE_8KiB - 1; >> +#endif >> if (i == ring_size - 1) >> p->des01.erx.end_ring = 1; >> if (disable_rx_ic) >> @@ -292,9 +302,15 @@ static void enh_desc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len, >> int csum_flag) >> { >> p->des01.etx.first_segment = is_fs; >> +#ifdef CONFIG_MACH_LOONGSON1 >> + if (unlikely(len > BUF_SIZE_2KiB)) { >> + p->des01.etx.buffer1_size = BUF_SIZE_2KiB - 1; >> + p->des01.etx.buffer2_size = len - BUF_SIZE_2KiB + 1; >> +#else >> if (unlikely(len > BUF_SIZE_4KiB)) { >> p->des01.etx.buffer1_size = BUF_SIZE_4KiB; >> p->des01.etx.buffer2_size = len - BUF_SIZE_4KiB; >> +#endif >> } else { >> p->des01.etx.buffer1_size = len; >> } No. I do not want to see all these ifdef inside the code. I had to rework some driver's part just last week to avoid this kind of code. I suggest you to re-base the work against the net-next kernel and look at how the ring/chained modes have been managed. I added a new file called descs_com.h that you can re-use adding small inline functions where define the changes for be mode. > Is it possible to add two new macros RX_BUF_SIZE and TX_BUF_SIZE to .h > instead? which may reduce code duplication. This code exists because we have to properly handle the jumbo frames. Note that this code has been reworked to use the ring/chained modes. Take a look at descs_com.h. I expect to see the driver on your platform that uses jumbo and chained/ring modes. Best Regards Giuseppe > > Regards, > Wu Zhangjin > >> -- >> 1.7.1 >> >> > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger•kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2 2/4] MIPS: Add board support for Loongson1B 2011-10-24 7:19 ` Giuseppe CAVALLARO @ 2011-10-24 10:36 ` Kelvin Cheung 2011-10-24 12:18 ` Giuseppe CAVALLARO 0 siblings, 1 reply; 14+ messages in thread From: Kelvin Cheung @ 2011-10-24 10:36 UTC (permalink / raw) To: Giuseppe CAVALLARO Cc: Wu Zhangjin, linux-mips, linux-kernel, ralf, r0bertz, netdev Hi Giuseppe, 2011/10/24, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: > On 10/21/2011 7:33 PM, Wu Zhangjin wrote: >> On Fri, Oct 21, 2011 at 6:28 PM, <keguang.zhang@gmail•com> wrote: >>> From: Kelvin Cheung <keguang.zhang@gmail•com> >>> >>> This patch adds basic platform support for Loongson1B >>> including serial port, ethernet, and interrupt handler. >>> >>> Loongson1B UART is compatible with NS16550A. >>> Loongson1B GMAC is built around Synopsys IP Core. >>> >> >> Perhaps you'd better split out the GMAC support to its own patch and >> send it to the net/ maintainer and the authors of the original files. > > Also suggest you to provide the stmmac patches for net-next. > The stmmac driver has been recently updated and I've also several > patches to commit (for example for PCI etc) on it. > > I'm happy that the support for big endianess arrived. > I supported a guy some time ago but he didn't provided me the patches > tested on his side :-(. So welcome yours and many thanks Kelvin! > > Please send the stmmac patches and add me on CC. I'm happy to help you > on reviewing them. Thanks for your review. >>> diff --git a/drivers/net/stmmac/descs.h b/drivers/net/stmmac/descs.h >>> index 63a03e2..4db27d0 100644 >>> --- a/drivers/net/stmmac/descs.h >>> +++ b/drivers/net/stmmac/descs.h >>> @@ -53,6 +53,38 @@ struct dma_desc { >>> u32 reserved3:5; >>> u32 disable_ic:1; >>> } rx; >>> +#ifdef CONFIG_MACH_LOONGSON1 >>> + struct { >>> + /* RDES0 */ >>> + u32 payload_csum_error:1; >>> + u32 crc_error:1; >>> + u32 dribbling:1; >>> + u32 error_gmii:1; >>> + u32 receive_watchdog:1; >>> + u32 frame_type:1; >>> + u32 late_collision:1; >>> + u32 ipc_csum_error:1; >>> + u32 last_descriptor:1; >>> + u32 first_descriptor:1; >>> + u32 vlan_tag:1; >>> + u32 overflow_error:1; >>> + u32 length_error:1; >>> + u32 sa_filter_fail:1; >>> + u32 descriptor_error:1; >>> + u32 error_summary:1; >>> + u32 frame_length:14; >>> + u32 da_filter_fail:1; >>> + u32 own:1; >>> + /* RDES1 */ >>> + u32 buffer1_size:11; >>> + u32 buffer2_size:11; >>> + u32 reserved1:2; >>> + u32 second_address_chained:1; >>> + u32 end_ring:1; >>> + u32 reserved2:5; >>> + u32 disable_ic:1; >>> + } erx; /* -- enhanced -- */ >>> +#else >>> struct { >>> /* RDES0 */ >>> u32 payload_csum_error:1; >>> @@ -83,6 +115,7 @@ struct dma_desc { >>> u32 reserved2:2; >>> u32 disable_ic:1; >>> } erx; /* -- enhanced -- */ >>> +#endif >>> >>> /* Transmit descriptor */ >>> struct { >>> @@ -113,6 +146,40 @@ struct dma_desc { >>> u32 last_segment:1; >>> u32 interrupt:1; >>> } tx; >>> +#ifdef CONFIG_MACH_LOONGSON1 >>> + struct { >>> + /* TDES0 */ >>> + u32 deferred:1; >>> + u32 underflow_error:1; >>> + u32 excessive_deferral:1; >>> + u32 collision_count:4; >>> + u32 vlan_frame:1; >>> + u32 excessive_collisions:1; >>> + u32 late_collision:1; >>> + u32 no_carrier:1; >>> + u32 loss_carrier:1; >>> + u32 payload_error:1; >>> + u32 frame_flushed:1; >>> + u32 jabber_timeout:1; >>> + u32 error_summary:1; >>> + 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 time_stamp_enable:1; >>> + u32 disable_padding:1; >>> + u32 second_address_chained:1; >>> + u32 end_ring:1; >>> + u32 crc_disable:1; >>> + u32 checksum_insertion:2; >>> + u32 first_segment:1; >>> + u32 last_segment:1; >>> + u32 interrupt:1; >>> + } etx; /* -- enhanced -- */ >>> +#else >>> struct { >>> /* TDES0 */ >>> u32 deferred:1; >>> @@ -148,6 +215,7 @@ struct dma_desc { >>> u32 buffer2_size:13; >>> u32 reserved4:3; >>> } etx; /* -- enhanced -- */ >>> +#endif >>> } des01; >>> unsigned int des2; >>> unsigned int des3; >> >> >> If the difference is very much, perhaps a new dma_desc struct can be >> defined instead. >> > > Concerning the descriptors, we could have two different files: > > descs_le.h > descs_be.h > > and select their inclusion inside the common.h. > > Please use instead of the macro CONFIG_MACH_LOONGSON1 another one more > generic e.g. CONFIG_STMMAC_BE (and add it in the driver's Kconfig). > > On your platform you will have to enable it by default. > Or it could be the default on MIPS: LE will be on ARM and SuperH. Loongson1B(MIPS32 R2 compatible) is little endian. And as you can see, the bitfield of RX/TX descriptor is different from the enhanced descriptor. >>> diff --git a/drivers/net/stmmac/enh_desc.c >>> b/drivers/net/stmmac/enh_desc.c >>> index e5dfb6a..3b5e4f1 100644 >>> --- a/drivers/net/stmmac/enh_desc.c >>> +++ b/drivers/net/stmmac/enh_desc.c >>> @@ -108,6 +108,7 @@ static int enh_desc_get_tx_len(struct dma_desc *p) >>> static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err) >>> { >>> int ret = good_frame; >>> +#ifndef CONFIG_MACH_LOONGSON1 >>> u32 status = (type << 2 | ipc_err << 1 | payload_err) & 0x7; >>> >>> /* bits 5 7 0 | Frame status >>> @@ -145,6 +146,7 @@ static int enh_desc_coe_rdes0(int ipc_err, int type, >>> int payload_err) >>> CHIP_DBG(KERN_ERR "RX Des0 status: No IPv4, IPv6 >>> frame.\n"); >>> ret = discard_frame; >>> } >>> +#endif >>> return ret; >>> } > >>> >>> @@ -232,9 +234,17 @@ static void enh_desc_init_rx_desc(struct dma_desc >>> *p, unsigned int ring_size, >>> int i; >>> for (i = 0; i < ring_size; i++) { >>> p->des01.erx.own = 1; >>> +#ifdef CONFIG_MACH_LOONGSON1 >>> + p->des01.erx.buffer1_size = BUF_SIZE_2KiB - 1; >>> +#else >>> p->des01.erx.buffer1_size = BUF_SIZE_8KiB - 1; >>> +#endif >>> /* To support jumbo frames */ >>> +#ifdef CONFIG_MACH_LOONGSON1 >>> + p->des01.erx.buffer2_size = BUF_SIZE_2KiB - 1; >>> +#else >>> p->des01.erx.buffer2_size = BUF_SIZE_8KiB - 1; >>> +#endif >>> if (i == ring_size - 1) >>> p->des01.erx.end_ring = 1; >>> if (disable_rx_ic) >>> @@ -292,9 +302,15 @@ static void enh_desc_prepare_tx_desc(struct dma_desc >>> *p, int is_fs, int len, >>> int csum_flag) >>> { >>> p->des01.etx.first_segment = is_fs; >>> +#ifdef CONFIG_MACH_LOONGSON1 >>> + if (unlikely(len > BUF_SIZE_2KiB)) { >>> + p->des01.etx.buffer1_size = BUF_SIZE_2KiB - 1; >>> + p->des01.etx.buffer2_size = len - BUF_SIZE_2KiB + 1; >>> +#else >>> if (unlikely(len > BUF_SIZE_4KiB)) { >>> p->des01.etx.buffer1_size = BUF_SIZE_4KiB; >>> p->des01.etx.buffer2_size = len - BUF_SIZE_4KiB; >>> +#endif >>> } else { >>> p->des01.etx.buffer1_size = len; >>> } > > No. I do not want to see all these ifdef inside the code. > I had to rework some driver's part just last week to avoid this kind of > code. I suggest you to re-base the work against the net-next kernel and > look at how the ring/chained modes have been managed. > > I added a new file called descs_com.h that you can re-use adding small > inline functions where define the changes for be mode. 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? >> Is it possible to add two new macros RX_BUF_SIZE and TX_BUF_SIZE to .h >> instead? which may reduce code duplication. > > This code exists because we have to properly handle the jumbo frames. > > Note that this code has been reworked to use the ring/chained modes. > Take a look at descs_com.h. > > I expect to see the driver on your platform that uses jumbo and > chained/ring modes. > > Best Regards > Giuseppe > >> >> Regards, >> Wu Zhangjin >> >>> -- >>> 1.7.1 >>> >>> >> -- >> To unsubscribe from this list: send the line "unsubscribe netdev" in >> the body of a message to majordomo@vger•kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > -- Best Regards! Kelvin ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2 2/4] MIPS: Add board support for Loongson1B 2011-10-24 10:36 ` Kelvin Cheung @ 2011-10-24 12:18 ` Giuseppe CAVALLARO 2011-10-24 14:05 ` Kelvin Cheung 0 siblings, 1 reply; 14+ messages in thread From: Giuseppe CAVALLARO @ 2011-10-24 12:18 UTC (permalink / raw) To: Kelvin Cheung Cc: Wu Zhangjin, linux-mips, linux-kernel, ralf, r0bertz, netdev 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. 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2 2/4] MIPS: Add board support for Loongson1B 2011-10-24 12:18 ` Giuseppe CAVALLARO @ 2011-10-24 14:05 ` Kelvin Cheung 2011-10-24 15:35 ` Giuseppe CAVALLARO 0 siblings, 1 reply; 14+ messages in thread From: Kelvin Cheung @ 2011-10-24 14:05 UTC (permalink / raw) To: Giuseppe CAVALLARO Cc: Wu Zhangjin, linux-mips, linux-kernel, ralf, r0bertz, netdev 2011/10/24, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: > 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. Moreover, I want to enable the TX checksum offload function which is not supported in normal descriptor. Any suggestions? > 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 > > -- Best Regards! Kelvin ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2 2/4] MIPS: Add board support for Loongson1B 2011-10-24 14:05 ` Kelvin Cheung @ 2011-10-24 15:35 ` Giuseppe CAVALLARO 2011-10-25 2:12 ` Kelvin Cheung 0 siblings, 1 reply; 14+ messages in thread From: Giuseppe CAVALLARO @ 2011-10-24 15:35 UTC (permalink / raw) To: Kelvin Cheung Cc: Wu Zhangjin, linux-mips, linux-kernel, ralf, r0bertz, netdev On 10/24/2011 4:05 PM, Kelvin Cheung wrote: > 2011/10/24, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: >> 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. The normal descriptor fields in the stmmac refer to an old synopsys databook. 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. 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? > 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. 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 >> >> > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2 2/4] MIPS: Add board support for Loongson1B 2011-10-24 15:35 ` Giuseppe CAVALLARO @ 2011-10-25 2:12 ` Kelvin Cheung 2011-10-25 7:09 ` Giuseppe CAVALLARO 0 siblings, 1 reply; 14+ messages in thread From: Kelvin Cheung @ 2011-10-25 2:12 UTC (permalink / raw) To: Giuseppe CAVALLARO Cc: Wu Zhangjin, linux-mips, linux-kernel, ralf, r0bertz, netdev 2011/10/24, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: > On 10/24/2011 4:05 PM, Kelvin Cheung wrote: >> 2011/10/24, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: >>> 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. >> 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? > 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 >>> >>> >> >> > > -- Best Regards! Kelvin ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2 2/4] MIPS: Add board support for Loongson1B 2011-10-25 2:12 ` Kelvin Cheung @ 2011-10-25 7:09 ` Giuseppe CAVALLARO 2011-10-25 8:20 ` Giuseppe CAVALLARO 2011-10-25 9:18 ` Kelvin Cheung 0 siblings, 2 replies; 14+ messages in thread From: Giuseppe CAVALLARO @ 2011-10-25 7:09 UTC (permalink / raw) To: Kelvin Cheung Cc: Wu Zhangjin, linux-mips, linux-kernel, ralf, r0bertz, netdev On 10/25/2011 4:12 AM, Kelvin Cheung wrote: > 2011/10/24, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: >> On 10/24/2011 4:05 PM, Kelvin Cheung wrote: >>> 2011/10/24, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: >>>> 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. > >> 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 >>>> >>>> >>> >>> >> >> > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2 2/4] MIPS: Add board support for Loongson1B 2011-10-25 7:09 ` Giuseppe CAVALLARO @ 2011-10-25 8:20 ` Giuseppe CAVALLARO 2011-10-25 9:21 ` Kelvin Cheung 2011-10-26 4:27 ` Kelvin Cheung 2011-10-25 9:18 ` Kelvin Cheung 1 sibling, 2 replies; 14+ messages in thread From: Giuseppe CAVALLARO @ 2011-10-25 8:20 UTC (permalink / raw) To: Kelvin Cheung Cc: Wu Zhangjin, linux-mips, linux-kernel, ralf, r0bertz, netdev [-- Attachment #1: Type: text/plain, Size: 3770 bytes --] 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@st•com>: >>> On 10/24/2011 4:05 PM, Kelvin Cheung wrote: >>>> 2011/10/24, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: >>>>> 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@vger•kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > [-- Attachment #2: 0002-stmmac-update-normal-descriptor-structure.patch --] [-- Type: text/x-patch, Size: 7086 bytes --] >From 85da9c92eace0b56148e9eedad1061ed72271409 Mon Sep 17 00:00:00 2001 From: Giuseppe Cavallaro <peppe.cavallaro@st•com> Date: Tue, 25 Oct 2011 09:46:41 +0200 Subject: [PATCH (sh-2.6.32.y) 2/2] 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. 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@gmail•com> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st•com> --- drivers/net/stmmac/common.h | 8 ++++---- drivers/net/stmmac/descs.h | 31 ++++++++++++++++++------------- drivers/net/stmmac/norm_desc.c | 34 ++++++++++++++++++++-------------- drivers/net/stmmac/stmmac_ethtool.c | 8 ++++---- 4 files changed, 46 insertions(+), 35 deletions(-) diff --git a/drivers/net/stmmac/common.h b/drivers/net/stmmac/common.h index 9100c10..2cc1192 100644 --- a/drivers/net/stmmac/common.h +++ b/drivers/net/stmmac/common.h @@ -49,7 +49,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; @@ -58,9 +58,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 f7e8ba7..c7c1522 100644 --- a/drivers/net/stmmac/norm_desc.c +++ b/drivers/net/stmmac/norm_desc.c @@ -50,11 +50,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++; @@ -86,12 +87,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++; @@ -113,10 +114,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 +187,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 d81cf20..6831cf2 100644 --- a/drivers/net/stmmac/stmmac_ethtool.c +++ b/drivers/net/stmmac/stmmac_ethtool.c @@ -48,7 +48,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), @@ -57,9 +57,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.4.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH V2 2/4] MIPS: Add board support for Loongson1B 2011-10-25 8:20 ` Giuseppe CAVALLARO @ 2011-10-25 9:21 ` Kelvin Cheung 2011-10-26 4:27 ` Kelvin Cheung 1 sibling, 0 replies; 14+ messages in thread From: Kelvin Cheung @ 2011-10-25 9:21 UTC (permalink / raw) To: Giuseppe CAVALLARO Cc: Wu Zhangjin, linux-mips, linux-kernel, ralf, r0bertz, netdev 2011/10/25, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: > 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@st•com>: >>>> On 10/24/2011 4:05 PM, Kelvin Cheung wrote: >>>>> 2011/10/24, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: >>>>>> 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 ;-). Sure. > 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@vger•kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > -- Best Regards! Kelvin ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2 2/4] MIPS: Add board support for Loongson1B 2011-10-25 8:20 ` Giuseppe CAVALLARO 2011-10-25 9:21 ` Kelvin Cheung @ 2011-10-26 4:27 ` Kelvin Cheung 2011-10-26 7:20 ` Giuseppe CAVALLARO 1 sibling, 1 reply; 14+ messages in thread From: Kelvin Cheung @ 2011-10-26 4:27 UTC (permalink / raw) To: Giuseppe CAVALLARO Cc: Wu Zhangjin, linux-mips, linux-kernel, ralf, r0bertz, netdev [-- Attachment #1: Type: text/plain, Size: 4272 bytes --] 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@st•com>: > 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@st•com>: >>>> On 10/24/2011 4:05 PM, Kelvin Cheung wrote: >>>>> 2011/10/24, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: >>>>>> 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@vger•kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > -- Best Regards! Kelvin [-- Attachment #2: 0001-stmmac-update-normal-descriptor-structure.patch --] [-- Type: text/x-patch, Size: 7719 bytes --] From 93ee3effbde61013a886d579fc89c36ddddb35a4 Mon Sep 17 00:00:00 2001 From: Kelvin Cheung <keguang.zhang@gmail•com> 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@gmail•com> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st•com> --- 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH V2 2/4] MIPS: Add board support for Loongson1B 2011-10-26 4:27 ` Kelvin Cheung @ 2011-10-26 7:20 ` Giuseppe CAVALLARO 2011-10-26 7:48 ` Kelvin Cheung 0 siblings, 1 reply; 14+ messages in thread From: Giuseppe CAVALLARO @ 2011-10-26 7:20 UTC (permalink / raw) To: Kelvin Cheung Cc: Wu Zhangjin, linux-mips, linux-kernel, ralf, r0bertz, netdev [-- Attachment #1: Type: text/plain, Size: 4725 bytes --] Hello Kelvin On 10/26/2011 6:27 AM, Kelvin Cheung wrote: > 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? Yes, I had not enabled the rx coe. I'm resending your patch (v3) where I fixed a problem. Old mac10/100 has no rx csum in HW. So I added an extra check. Let me consider it the final version. ;-) Thanks a lot for you effort. I'll send it for net-next now. Regards Peppe > > BTW, tx checksum insertion works now. > > 2011/10/25, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: >> 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@st•com>: >>>>> On 10/24/2011 4:05 PM, Kelvin Cheung wrote: >>>>>> 2011/10/24, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: >>>>>>> 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@vger•kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >> >> > > [-- Attachment #2: 0001-stmmac-update-normal-descriptor-structure-v3.patch --] [-- Type: text/x-patch, Size: 8441 bytes --] >From 324c8e56e728046590bae14d14e8ff09978b71b4 Mon Sep 17 00:00:00 2001 From: Giuseppe Cavallaro <peppe.cavallaro@st•com> Date: Tue, 25 Oct 2011 09:46:41 +0200 Subject: [PATCH (sh-2.6.32.y)] stmmac: update normal descriptor structure (v3) 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@gmail•com> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st•com> --- 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 +++--- drivers/net/stmmac/stmmac_main.c | 4 +- 5 files changed, 51 insertions(+), 40 deletions(-) diff --git a/drivers/net/stmmac/common.h b/drivers/net/stmmac/common.h index 9100c10..2cc1192 100644 --- a/drivers/net/stmmac/common.h +++ b/drivers/net/stmmac/common.h @@ -49,7 +49,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; @@ -58,9 +58,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 f7e8ba7..3955ae7 100644 --- a/drivers/net/stmmac/norm_desc.c +++ b/drivers/net/stmmac/norm_desc.c @@ -50,11 +50,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++; @@ -68,12 +69,12 @@ 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. */ + * In case of success, it returns good_frame because the GMAC device + * is supposed to be able to compute the csum in HW. */ 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)) { @@ -86,12 +87,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++; @@ -113,10 +114,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 +187,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 d81cf20..6831cf2 100644 --- a/drivers/net/stmmac/stmmac_ethtool.c +++ b/drivers/net/stmmac/stmmac_ethtool.c @@ -48,7 +48,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), @@ -57,9 +57,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), diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c index 3a3efb1..18e614b 100644 --- a/drivers/net/stmmac/stmmac_main.c +++ b/drivers/net/stmmac/stmmac_main.c @@ -1318,8 +1318,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit) #endif skb->protocol = eth_type_trans(skb, priv->dev); - if (unlikely(status == csum_none)) { - /* always for the old mac 10/100 */ + if (unlikely(!priv->plat->has_gmac)) { + /* No csum for the old mac 10/100 devices */ skb->ip_summed = CHECKSUM_NONE; netif_receive_skb(skb); } else { -- 1.7.4.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH V2 2/4] MIPS: Add board support for Loongson1B 2011-10-26 7:20 ` Giuseppe CAVALLARO @ 2011-10-26 7:48 ` Kelvin Cheung 0 siblings, 0 replies; 14+ messages in thread From: Kelvin Cheung @ 2011-10-26 7:48 UTC (permalink / raw) To: Giuseppe CAVALLARO Cc: Wu Zhangjin, linux-mips, linux-kernel, ralf, r0bertz, netdev It's perfect now. Please add me to CC list when you send the new patch. Thanks a lot for your help. 2011/10/26, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: > Hello Kelvin > > On 10/26/2011 6:27 AM, Kelvin Cheung wrote: >> 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? > > Yes, I had not enabled the rx coe. I'm resending your patch (v3) where I > fixed a problem. Old mac10/100 has no rx csum in HW. So I added an extra > check. > Let me consider it the final version. ;-) > > Thanks a lot for you effort. > I'll send it for net-next now. > > Regards > Peppe > >> >> BTW, tx checksum insertion works now. >> >> 2011/10/25, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: >>> 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@st•com>: >>>>>> On 10/24/2011 4:05 PM, Kelvin Cheung wrote: >>>>>>> 2011/10/24, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: >>>>>>>> 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@vger•kernel.org >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>> >>> >>> >> >> > > -- Best Regards! Kelvin ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2 2/4] MIPS: Add board support for Loongson1B 2011-10-25 7:09 ` Giuseppe CAVALLARO 2011-10-25 8:20 ` Giuseppe CAVALLARO @ 2011-10-25 9:18 ` Kelvin Cheung 1 sibling, 0 replies; 14+ messages in thread From: Kelvin Cheung @ 2011-10-25 9:18 UTC (permalink / raw) To: Giuseppe CAVALLARO Cc: Wu Zhangjin, linux-mips, linux-kernel, ralf, r0bertz, netdev 2011/10/25, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: > On 10/25/2011 4:12 AM, Kelvin Cheung wrote: >> 2011/10/24, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: >>> On 10/24/2011 4:05 PM, Kelvin Cheung wrote: >>>> 2011/10/24, Giuseppe CAVALLARO <peppe.cavallaro@st•com>: >>>>> 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. Fully agree. > 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. Thanks! >> >>> 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 >>>>> >>>>> >>>> >>>> >>> >>> >> >> > > -- Best Regards! Kelvin ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2011-10-26 7:48 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1319192888-21465-1-git-send-email-keguang.zhang@gmail.com>
[not found] ` <1319192888-21465-2-git-send-email-keguang.zhang@gmail.com>
2011-10-21 17:33 ` [PATCH V2 2/4] MIPS: Add board support for Loongson1B Wu Zhangjin
2011-10-24 7:19 ` Giuseppe CAVALLARO
2011-10-24 10:36 ` Kelvin Cheung
2011-10-24 12:18 ` Giuseppe CAVALLARO
2011-10-24 14:05 ` Kelvin Cheung
2011-10-24 15:35 ` Giuseppe CAVALLARO
2011-10-25 2:12 ` Kelvin Cheung
2011-10-25 7:09 ` Giuseppe CAVALLARO
2011-10-25 8:20 ` Giuseppe CAVALLARO
2011-10-25 9:21 ` Kelvin Cheung
2011-10-26 4:27 ` Kelvin Cheung
2011-10-26 7:20 ` Giuseppe CAVALLARO
2011-10-26 7:48 ` Kelvin Cheung
2011-10-25 9:18 ` Kelvin Cheung
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox