[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <479105A3.3040508@garzik.org>
Date: Fri, 18 Jan 2008 15:01:39 -0500
From: Jeff Garzik <jeff@...zik.org>
To: Claudio Lanconelli <lanconelli.claudio@...ar.com>
CC: netdev@...r.kernel.org, Jeff Garzik <jgarzik@...ox.com>,
Stephen Hemminger <shemminger@...ux-foundation.org>
Subject: Re: [PATCH][resend] add driver for enc28j60 ethernet chip
Claudio Lanconelli wrote:
> This patch add support for Microchip enc28j60 10Mbps Ethernet chip used
> in embedded systems
> due to its cheap SPI interface.
> This 2nd version include changes from previous comments by Jeff and
> Stephen,
> all but NAPI, see comments below at this regard.
>
> I resend the patch because I didn't receive any feedback.
>
> Changes to 1st version:
> - use mutex instead of semaphore
> - add carrier detect handling
> - add ethtool support
> - set_multicast_list when the interface is up using a workqueue
> - add restart_work to reset the chip in case of tx_timeout
> - removed kmalloc() for spi_transfer_buf (array defined in the priv struct)
>
> Jeff Garzik wrote:
>>
>> comments:
>>
>> * Why do interrupt work in a kernel thread? Your comment says you
>> cannot, but does not explain.
> The enc28j60 is a SPI to Ethernet adapter, so we cannot access register
> with simple in() out() instructions, but we need to use the SPI
> subsystem. The spi_sync() basic operation to read/write a register is a
> blocking operation, so can't be done in interrupt context.
> Since every basic operation like read interrupt flag register call
> spi_sync() we need the work queue for almost everything.
>
>>
>> * should use NAPI
>>
> For the reason I just explained I don't think NAPI is a viable way for
> enc28j60.
> Furthermore enc28j60 is a 10Mb only device and probably don't suffer to
> interrupt overload.
>
> I'm waiting for any comments, please.
applied
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists