netdev
[Top] [All Lists]

Re: [PATCH] r8169: support restricting speed+duplex in autonegotiation

To: Richard Dawe <rich@xxxxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH] r8169: support restricting speed+duplex in autonegotiation
From: Francois Romieu <romieu@xxxxxxxxxxxxx>
Date: Mon, 30 May 2005 00:54:33 +0200
Cc: Ben Greear <greearb@xxxxxxxxxxxxxxx>, Linux netdev <netdev@xxxxxxxxxxx>
In-reply-to: <4298D6F9.7030608@xxxxxxxxxxxxxxxxxxxx>
References: <4297A0F7.5070209@xxxxxxxxxxxxxxxxxxxx> <4297B479.1080404@xxxxxxxxxxxxxxx> <429871F9.6040305@xxxxxxxxxxxxxxxxxxxx> <20050528144207.GA17021@xxxxxxxxxxxxxxxxxxxxxxxxxx> <4298D6F9.7030608@xxxxxxxxxxxxxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mutt/1.4.1i
Richard Dawe <rich@xxxxxxxxxxxxxxxxxxxx> :
[...]
> Is the "media" part of the patch confusing?

No (it could have stored its data in link_settings[] though).

I do not understand what the "ret" variable in rtl8169_set_speed_xmii()
is supposed to do but it is not a big issue.

Style aside, Ben has imho a point. The patch adds a special case:
- is there a real need ?
- is it a 8169-specific thing or not ?

The code is added in an area where the driver is currently not pretty.
The driver should probably replace its own defines with the ones in
<linux/mii.h> then really disable autonegotiation when it is asked to
(hint, hint).

--
Ueimor

<Prev in Thread] Current Thread [Next in Thread>