SiS 190 NIC driver
Pyun YongHyeon
pyunyh at gmail.com
Tue Dec 11 19:45:10 PST 2007
On Tue, Dec 11, 2007 at 08:45:09AM +0100, Alexander Pohoyda wrote:
> > > Attached is a driver for the SiS 190 NIC found in some Shuttle XPS
>
> > I've looked over the driver code and I guess the driver is not
> > for RELENG_6/RELENG_7/CURRENT. Would you make it work for CURRENT?
>
> I would like to, but neither RELENG_6, nor RELENG_7 boots on my Shuttle box. I have reported this in -current mailing list some time ago, but got no usable suggestion. Maybe I should try a recent CURRENT, but I don't have much time to play around with it.
Maybe sumbit a PR for this?
>
>
> > - I guess SiS 190 is a gigabit controller but the probe message just
> > shows 10/100TX message. Does this driver supports 1Gbps?
>
> I don't know for sure, but SiS website <http://www.sis.com/download/agreement.php?id=155880> mentions: SiS191 Gigabit LAN & SiS190 LAN Driver(v2.03)
>
> Other websites have it wrong, I guess, e.g.: <http://www.dirfile.com/sis_190_gigabit_lan__sis_191_lan_driver_v_1_03b.htm> SiS 190 Gigabit LAN & SiS 191 LAN Driver v 1.03b
>
> However, I will have a look at this.
Thanks.
>
>
> > - The usage of bus_dma(9) KPI seems to be incorrect. For example
> > there should be no reason to set BUS_DMA_ALLOCNOW flag in
> > bus_dma_tag_create.
>
> This code is left from the original sis driver and it works. Moreover, the Architecture Handbook <http://www.freebsd.org/doc/en_US.ISO8859-1/books/arch-handbook/isa-driver-busmem.html> does not mention bus_dmamap_load_mbuf_sg(9) and once again, newer FreeBSD releases do not start on this box at all, so if this new bus_dma API is not available in RELENG_5, I cannot use it.
I don't think sis(4) is the best example for bus_dma(9) usage. If it
was it would have worked on sparc64. Check otherdrivers that runs
without problems on !i386 architecture.
And yes, the Handbook needs updating. See bus_dma(9) man page.
>
>
> > needed to pass/get to/from callback. Please check other network
> > drivers for correct usage of bus_dma(9).
>
> The existing sis driver does no use bus_dmamap_load_mbuf_sg either.
>
>
> > - It seems that Tx/Rx ring have alignment restrictions, probably
> > 16 bytes or larger. Would you please check it?
>
> I do not understand, please elaborate.
>
For example, you seem to use alignemnet 1 in dma tags for Tx ring.
796 /* TX */
797 error = bus_dma_tag_create(sc->sis_parent_tag, /* parent */
798 1, 0, /* alignment, boundary */
^^^
799 BUS_SPACE_MAXADDR, /* lowaddr */
800 BUS_SPACE_MAXADDR, /* highaddr */
801 NULL, NULL, /* filter, filterarg */
802 SIS_TX_RING_SZ, 1, /* maxsize,nsegments */
803 BUS_SPACE_MAXSIZE_32BIT, /* maxsegsize */
804 0, /* flags */
805 busdma_lock_mutex, /* lockfunc */
806 &Giant, /* lockarg */
807 &ld->sis_tx_tag);
I guess the the alignment would be sizeof(struct sis19x_desc) at
least, i.e. 16 bytes. Normally ethernet controllers may have more
larger alignment limitation than its single descriptor size(e.g.
256 bytes). You seems to use m_devget(9) for !386 to align recevied
frames in sis_rxeof(). This clearly indicates it needs Rx buffer
alignment limitation. In addition, you seem to use the same dma tag
for Tx/Rx dma maps. Becasue most hardwares supports multi-fragmented
frames in Tx side, total Tx DMA segment size would be larger than
Rx DMA segment size. For Rx side, most hardwares just support 1 DMA
segment, so it's maximum DMA segment size would be MCLBYTES.
>
> > - Descriptor counter 64 seems to be small for gigabit controllers.
> > I guess 256 or higher would be more reasonable one.
>
> Do you mean that:
> int sis_tx_cnt;
> should rather be:
> u_int256_t sis_tx_cnt;
>
> If not, please elaborate.
>
Sorry, I meant the following descriptor counters for Rx/Tx ring.
77 #define SIS_RX_RING_CNT 64
^^^
78 #define SIS_TX_RING_CNT 64
^^^
>
> > - I'm not sure SiS 190 can't handle DMA gathering in Tx path but
> > it seems that Linux doesn't use multiple Tx descriptors at all
> > in Tx path(NETIF_F_SG flag is absent in Linux) So I guess you can
> > simplify sis_encap() function.
>
> Please offer your version of a function and I will test it. Or explain how you mean to simplify it. Is there an existing driver with a simplified variant?
>
Check vr(4)'s vr_encap, it simply calls m_defrag(9) as Rhine I
hardware doesn't support multi-segment dma.
>
> > - It seems the driver name sis19x looks odd. How about sge(SiS
> > Gigabit Ethernet)?
>
> Yes, I would like sge better, but see above.
>
>
> > - Missing Makefile and man page.
>
> Sorry, cannot provide a man page other than a standard sis(4), but a Makefile is not a problem, will do.
>
>
> > - style(9) cleanup.
>
> I tried to stick to style(9), do you have any specific complaints?
>
198 Reserved0 = 0x08, // unused
^^^
199 /* 2007-04-25, Alexander Pohoyda: this register is
^^^^
200 * automatically set by NIC to the value of TxDescStartAddr
201 * register plus 8 (which, I suppose, is the offset of the
202 * sis_ptr field in sis19x_desc structure). */
^^^
203 TxNextDescAddr = 0x0c, // unused
// comment, multi-line comment, missing () in return statement etc.
>
> > I don't have SiS 190 hardware to test the driver. So it's hard for me
> > to make it work on CURRENT. Feel free to contact to me if you have
> > any questions.
>
> Thank you very much for useful comments! Please send me code snippets and I will test them!
I'm somewhat overloaded for other work so I'm afraid I can't write a
code for not-having hardwares. It would be even better if you can
check other drivers(bge, em, fxp, gem, msk, nfe, sk, stge etc) in CURRENT.
--
Regards,
Pyun YongHyeon
More information about the freebsd-drivers
mailing list