Re: kern/46961: Please support BCM57762 Ethernet arapter (Apple'sThunderbolt Ethernet Adapter)

[Ryo ONODERA <ryo_on%yk.rim.or.jp@localhost>]

The following reply was made to PR kern/46961; it has been noted by GNATS.

From: Ryo ONODERA <ryo_on%yk.rim.or.jp@localhost>
To: gnats-bugs%NetBSD.org@localhost, tsutsui%ceres.dti.ne.jp@localhost
Cc: 
Subject: Re: kern/46961: Please support BCM57762 Ethernet arapter
 (Apple'sThunderbolt Ethernet Adapter)
Date: Mon, 17 Sep 2012 04:41:25 +0900 (JST)

 Hi,
 
 Thank you.
 
 I have updated the patches.
 http://www.netbsd.org/~ryoon/120916b-bcm57762.diff
 
 From: Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>, Date: Sun, 16 Sep 
2012 15:15:05 +0000 (UTC)
 
 > The following reply was made to PR kern/46961; it has been noted by GNATS.
 > 
 > From: Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>
 > To: gnats-bugs%NetBSD.org@localhost
 > Cc: kern-bug-people%NetBSD.org@localhost, tsutsui%ceres.dti.ne.jp@localhost
 > Subject: Re: kern/46961: Please support BCM57762 Ethernet arapter 
 > (Apple'sThunderbolt
 >       Ethernet Adapter)
 > Date: Mon, 17 Sep 2012 00:13:24 +0900
 > 
 >  >  Please see new patches.
 >  >  http://www.netbsd.org/~ryoon/120916a-bcm57762.diff
 >   :
 >  >  I think DIX Ethernet's max length is 1526.
 >  >  I have no idea about 1536.
 >  >  
 >  >  Anyway I will make ETHER_MAX_DIX_LEN bge local.
 >  
 >  I don't think we support any DIX Ethernet specific features
 >  and the possible maximum packet length is VLAN one
 >  (i.e. ETHER_MAX_LEN + ETHER_VLAN_ENCAP_LEN (1518 + 4) bytes),
 >  so it would be better to define and use ETHER_VLAN_MAX_LEN (or so),
 >  even if it's defined in if_bgereg.h locally.
 >  
 >  >> @@ -2383,3 +2385,10 @@
 >    :
 >  >> +
 >  >> +/*
 >  >> + * Maximum DIX frame length
 >  >> + * In OpenBSD's sys/netinet/if_ether.h, this is 1536,
 >  >> + * but it should be 1526.
 >  >> + */
 >  >> +#define ETHER_MAX_DIX_LEN       1526
 >  
 >  if_bgereg.h already has local packet lenghth definitions
 >  around lines 2330-2335, so it's also better to put a new local
 >  value in the same place.
 
 I see.
 I have defined ETHER_VLAN_MAX_LEN in the suggested place of if_bgereg.h.
 It works.
 
 >  >> +            BGE_ASICREV(sc->bge_chipid) == BGE_ASICREV_BCM57766)
 >  >> +                rcb->bge_maxlen_flags = (BGE_RCB_MAXLEN_FLAGS(512, 0) |
 >  >> +                                        (ETHER_MAX_DIX_LEN << 2));
 >  >> +        else if (BGE_IS_5705_PLUS(sc))
 >  
 >  BGE_RCB_MAXLEN_FLAGS() macro is defined as following in if_bgereg.h:
 >  
 >  >> #define BGE_RCB_MAXLEN_FLAGS(maxlen, flags)      ((maxlen) << 16 | 
 > (flags))
 >  
 >  so it's consistent to put the packet length value into the macro, i.e.
 >      rcb->bge_maxlen_flags =
 >          BGE_RCB_MAXLEN_FLAGS(512, ETHER_VLAN_MAX_LEN << 2);
 
 I understand.
 I have used this.
 
 
 >  >  Linux's tg3 identifies 0x57766 as ASIC_REV_57766.
 >  >  See 
 > http://lxr.free-electrons.com/source/drivers/net/ethernet/broadcom/tg3.c#L13653
 >  .
 >  >  
 >  >  >  (existing "BGE_ASICREV_BCM57765" is "0x57785" anyway)
 >  >  
 >  >  In Linux's tg3 0x57785 is ASIC_REV_57765.
 >  >  
 >  >  In my opinion, 0x57766 should be BGE_ASICREV_57766, due to consistency
 >  >  between OSes.
 >  
 >  I see, then it's fine.
 >  
 >  ---
 >  Izumi Tsutsui
 >  
 > 
 > 
 
 --
 Ryo ONODERA // ryo_on%yk.rim.or.jp@localhost
 PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3