| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 24 Oct 2011 09:19:24 +0200
From: Giuseppe CAVALLARO <peppe.cavallaro@...com>
To: keguang.zhang@...il.com
Cc: Wu Zhangjin <wuzhangjin@...il.com>, linux-mips@...ux-mips.org,
linux-kernel@...r.kernel.org, ralf@...ux-mips.org,
r0bertz@...too.org, netdev@...r.kernel.org
Subject: Re: [PATCH V2 2/4] MIPS: Add board support for Loongson1B
On 10/21/2011 7:33 PM, Wu Zhangjin wrote:
> On Fri, Oct 21, 2011 at 6:28 PM, <keguang.zhang@...il.com> wrote:
>> From: Kelvin Cheung <keguang.zhang@...il.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@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists