Re: [PATCH] m68k/atari: EtherNEC - rewrite to use mainstream ne.c, take two

Geert Uytterhoeven
2012-04-05T14:21:58Z

Hi Paul,

On Thu, Apr 5, 2012 at 15:24, Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:
> On 12-04-05 05:28 AM, Geert Uytterhoeven wrote:
>> On Wed, Apr 4, 2012 at 22:46, Paul Gortmaker
>> <paul.gortmaker@windriver.com> wrote:
>>> On 12-04-01 04:49 AM, Michael Schmitz wrote:
>>>>> And on re-reading the comments in the other part of the patch, i.e.
>>>>> "...emulates the card interrupt via a timer"  --perhaps the driver
>>>>> should be just fixed to support generic netpoll, instead of adding
>>>>> an arch specific thing that amounts to netpoll.  Then anyone can
>>>>> attempt to limp along and use one of these ancient relics w/o IRQ.
>>>>>
>>>> Here's take two of my patch to convert the m68k Atari ROM port Ethernet
>>>> driver to use mainstream ne.c, with minimal changes to the core NE2000
>>>> code.
>>>>
>>>> In particular:
>>>>
>>>> Changes to core net code:
>>>> * add a platform specific IRQ flag, so ne.c can share a hardware or
>>>> timer interrupt with some other interrupt source.
>>>>
>>>> Changes to arch/m68k code:
>>>> * register the 8390 platform device on Atari only if the hardware is present
>>>> * retain the old driver (atari_ethernec.c in Geert's tree) under a
>>>> different config option, to be removed soon.
>>>>
>>>> Regarding your suggestion that netpoll be used instead of a dedicated
>>>> timer interrupt: no changes to ne.c or 8390p.c are required to use
>>>> netpoll, it all works out of the box. All that is needed to use the
>>>> driver with netpoll is setting the device interrupt to some source that
>>>> can be registered, and enabling CONFIG_NETPOLL. Interrupt rate and hence
>>>> throughput is lower with netpoll though, which is why I still prefer the
>>>> dedicated timer option.
>>>
>>> How much lower?  Enough to matter?  Implicit in that question is
>>> the assumption that this is largely a hobbyist platform and nobody
>>> is using it in a closet to route gigabytes of traffic.
>>
>> One other thing we could do is increase CONFIG_HZ to 250.
>>
>>> Also, the only advantage to modifying ne.c is to allow dumping
>>> the old driver.  What is the "remove soon" plan?  Any reason
>>> for it to not be synchronous?  That would eliminate the Kconfig
>>> churn and the introduction of the _OLD option.  Modifying ne.c
>>> and then deciding to keep the old driver because it is "faster"
>>> would make this change pointless.
>>
>> From my point of view, "remove soon" means it will never hit mainline.
>
> Can you clarify what "it" is?   It isn't clear to me if you
> mean the _removal_ will never hit mainline, or the transient
> renamed "old" driver will never hit mainline.
>
> If the former, then there is no point pursuing this any further
> as I said above.
>
> If the latter, then the commit sent out for review should have
> no instances of this "renaming to old" related changes.

Sorry for being unclear. The latter (i.e. the old driver will
never hit mainline).

And Michael's patch against ne.c should indeed not touch the old
driver, as it doesn't exist in mainline.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds