LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: 許恆嘉 <edward_hsu@realtek.com.tw>
To: Francois Romieu <romieu@fr.zoreil.com>
Cc: jeff@garzik.org, linux-kernel@vger.kernel.org, hiwu@realtek.com.tw
Subject: Re: [PATCH 2.6.19.2] r8169: support RTL8169SC/8110SC
Date: Tue, 06 Feb 2007 10:44:26 +0800	[thread overview]
Message-ID: <45C7EB8A.1090700@realtek.com.tw> (raw)
In-Reply-To: <20070206003130.GA18236@electric-eye.fr.zoreil.com>

Dear Francois:

Thanks for your reply. I gave my idea about your questions with the the
key word "ANS_2".

If you have further questions, please raise them.

Best Regards,
Edward 2007/02/06

Francois Romieu 提到:

>許恆嘉 <edward_hsu@realtek.com.tw> :
>[...]
>  
>
>>>>static struct pci_device_id rtl8169_pci_tbl[] = {
>>>>- { PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8129), 0, 0, RTL_CFG_0 },
>>>>- { PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8136), 0, 0, RTL_CFG_2 },
>>>>- { PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8167), 0, 0, RTL_CFG_0 },
>>>>- { PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8168), 0, 0, RTL_CFG_2 },
>>>>- { PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8169), 0, 0, RTL_CFG_0 },
>>>>- { PCI_DEVICE(PCI_VENDOR_ID_DLINK, 0x4300), 0, 0, RTL_CFG_0 },
>>>>- { PCI_DEVICE(0x1259, 0xc107), 0, 0, RTL_CFG_0 },
>>>>- { PCI_DEVICE(0x16ec, 0x0116), 0, 0, RTL_CFG_0 },
>>>>- { PCI_VENDOR_ID_LINKSYS, 0x1032,
>>>>- PCI_ANY_ID, 0x0024, 0, 0, RTL_CFG_0 },
>>>>+ { PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8169), },
>>>>+ { PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8167), },
>>>>  
>>>>
>>>>        
>>>>
>>>The current driver is intended to handle the whole set of PCI IDs
>>>which would be removed by the patch. Thoug some combination of
>>>chipset and motherboard do not work as expected, the gigabit
>>>chipsets have been reported to work.
>>>
>>>Please elaborate if there is a good reason to remove any ID.
>>>
>>>
>>>      
>>>
>>ANS:
>>I have explained my point about this in last question. This driver is 
>>developed for Realtek PCI gigabit ethernet controllers. Although some 
>>vendors may use Realtek solutions with their own PCI DIDs and VIDs, they 
>>should customize this driver and maintained the customized driver on 
>>their own.
>>    
>>
>
>Vendors will change the PCI ID because they like to see their name in
>the nifty hardware GUI under Windows. The brave ones will mess with the
>VPD (before or after they vandalize the ACPI tables, at their option).
>Eventually they will copy an outdated version of your driver on their site
>with the updated ID. Given the tight margin on these kind of mass-volume
>product, I would not expect vendors to do more for their customers who
>use Linux.
>  
>
ANS_2:
So, do you think that it is a good idea to keep other vendos's PID and
DID in the part?

>If your hardware changes significantly, it may make sense to use a new,
>different driver. Otherwise, as long as the changes are minor, a single
>in-kernel driver which works out of the box for most users/vendors offers
>the best coverage. It should not be neglected.
>
>So far, I have only seen minor differences between r8101-1.001.00,
>r8168-8.001.00 and r8169-6.001.00. The mac init sequence is not pretty
>to unify but the drivers stay mostly the same. There even is a floating
>patch for MSI support which seems manageable within a single driver.
>
>Of course it depends a lot on the kind of changes that you envision for
>the driver(s).
>  
>

ANS_2:

Sure! You are right. RTL8110SC, RTL8111B and RTL8101E have modest
differences, now. However, RTL8101E is a PCI-E fast ethernet controller.
I don't think is a good idea to merge its Linux driver into r8168.c or
r8169.c. RTL8110SC is the final version of Realtek PCI gigabit ethernet
controller. Moreover, due to the increasing popularity of PCI-E, Realtek
is going to design several generations of PCI-E ethernet controllers to
satisfy customer requests. I have discussed this issue with my hardware
colleagues. They believe that both MAC register layout and tx/rx
descriptor layout will be changed a lot in new PCI-E ICs. Actually, they
already did. Therefore, the hardwares of RTL8111B(PCI-E gigabit
ethernet) and RTL8101E(PCI-E fast ethernet) will have frequent and
drastic changes. So, I think that it's a good moment to separate their
Linux drivers, and r8169.c can become stable.

>[...]
>  
>
>>>>RxMaxSize = 0xDA,
>>>>CPlusCmd = 0xE0,
>>>>IntrMitigate = 0xE2,
>>>>@@ -287,11 +291,10 @@ enum RTL8169_register_content {
>>>>RxOK = 0x01,
>>>>
>>>>/* RxStatusDesc */
>>>>- RxFOVF = (1 << 23),
>>>>- RxRWT = (1 << 22),
>>>>- RxRES = (1 << 21),
>>>>- RxRUNT = (1 << 20),
>>>>- RxCRC = (1 << 19),
>>>>+ RxRES = 0x00200000,
>>>>+ RxCRC = 0x00080000,
>>>>+ RxRUNT = 0x00100000,
>>>>+ RxRWT = 0x00400000,
>>>>  
>>>>
>>>>        
>>>>
>>>This part removes RxFOVF. Please elaborate if there is a reason
>>>to do so.
>>>
>>>
>>>      
>>>
>>ANS:
>>According to the spec of RTL8110SC, bit 23 and bit 24 are reserved in 
>>the 1st double word of rx status descriptor. Therefore, I delete it.
>>    
>>
>
>  
>

ANS_2:
I have the specs of RTL8110S/SB/SC. According those specs, the two bits are reserved. Since I didn't create this driver, I don't know who wrote it. I think they are not used.

>The bits are fine for the old Gigabit 8169 though, aren't they ?
>
>[...]
>  
>
>>>Two points here:
>>>1. the driver uniformly uses u{8/16/32} types. Please follow the current
>>> style and avoid to add uint{8/16/32}_t things.
>>>2. does this new field bring something that struct net_device.dev_addr
>>> does not ?
>>>
>>>
>>>
>>>      
>>>
>>ANS:
>>I reference the source code of e1000.c, which is currently existing in 
>>Linux kernel. I think it uses u{8/16/32} and the new field.
>>    
>>
>
>The e1000 driver has an interesting history. It is strongly suggested
>to ponder several drivers to figure some good practices. Please see for
>instance tg3.c, b44.c, sky2.c or any recent driver in drivers/net.
>
>  
>

ANS_2:
Thanks for your suggestion. I will see those drivers that you suggest. And revise my driver.


  reply	other threads:[~2007-02-06  2:44 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-02  8:17 許恆嘉
2007-02-03  0:39 ` Francois Romieu
2007-02-05  3:47   ` 許恆嘉
2007-02-06  0:31     ` Francois Romieu
2007-02-06  2:44       ` 許恆嘉 [this message]
2007-02-06 21:38         ` Francois Romieu
2007-02-07  2:56           ` 許恆嘉
2007-02-07  3:25           ` 許恆嘉
2007-02-07 23:49             ` Francois Romieu
2007-02-08  1:18               ` 許恆嘉
  -- strict thread matches above, loose matches on Subject: below --
2007-02-02  7:08 許恆嘉
2007-02-02  8:03 ` Andrey Panin
2007-02-02  8:06 ` Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=45C7EB8A.1090700@realtek.com.tw \
    --to=edward_hsu@realtek.com.tw \
    --cc=hiwu@realtek.com.tw \
    --cc=jeff@garzik.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=romieu@fr.zoreil.com \
    --subject='Re: [PATCH 2.6.19.2] r8169: support RTL8169SC/8110SC' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).