Re: ne2000 with 8bit bus width

[Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>]

nonakap%gmail.com@localhost wrote:

> bus_space(9) function is frequently called.
> It is necessary to clear the ED_DCR_WTS bit at 8 bit bus width.

I think Arnaud asked if putting "(useworddma ? FOO : 0)" conditionals
into bus_space(9) functions could cause negative performance.

With a quick glance, those conditionals are only used
to select word/byte transfer in DCR register on
detecting chip types and reading MAC address,
(no additional conditionals in each transfer)
so it seems no problem.

> I did benchmark.
> 
> test machine
> CPU: SH3 SH7706 128MHz
> RAM: 32MB
> NIC: REALTEK RTL8019AS (8bit bus)
> 
> ftp: remote -> test machine /tmp(tmpfs)
> - use word function: 186.91KiB/s
> - use byte function: 169.51KiB/s

ttcp(1) or netperf(1) in pkgsrc/benchmark might be better.

> >> Applied patch are necessary for connecting NE2000(RTL8019AS) by the
> >> 8bit bus width.

- Where should a new sc_dma_witdh member be set? In each backend?
  If so, isn't it enough to set sc_useworddma in each backend?
>> +            useworddma = (nsc->sc_dma_width == NE2000_DMAWIDTH_WORD);
 :
>> +    nsc->sc_useworddma = useworddma;

- It looks now ne2000_detect() also checks if chip is 16 bit DMA capable.
  If so, isn't it better to use the result of ne2000_detect()
  to set sc_useworddma, rather than setting it in backends?

- It might be better to use bool for useworddma and sc_useworddma
  unless it's also used as an integer as current useword.

- In a new patch, useworddma is could be 0 even on NE2000 but
  usewordrw is set per sc_type (i.e. 0 only on NE1000).
  Is the following part still necessary to read MAC address?
  Isn't it enought to use usewordrw for indexes as current code?

>> +            } else if (nsc->sc_type == NE2000_TYPE_DL10019 ||
>> +                       nsc->sc_type == NE2000_TYPE_DL10022 ||
>> +                       nsc->sc_type == NE2000_TYPE_NE2000) {
>> +                    ne2000_readmem(nict, nich, asict, asich, 0, romdata,
>> +                        sizeof(romdata), usewordrw);
>> +                    for (i = 0; i < ETHER_ADDR_LEN; i++)
>> +                            dsc->sc_enaddr[i] = romdata[i << 1];
>>              } else {
>>                      ne2000_readmem(nict, nich, asict, asich, 0, romdata,
>> -                        sizeof(romdata), useword);
>> +                        sizeof(romdata), usewordrw);
>>                      for (i = 0; i < ETHER_ADDR_LEN; i++)
>> -                            dsc->sc_enaddr[i] =
>> -                                romdata[i * (useword ? 2 : 1)];
>> +                            dsc->sc_enaddr[i] = romdata[i << usewordrw];
>>              }

  (maybe we should change it to using switch(sc->sc_type) in this case)
---
Izumi Tsutsui