netdev - Re: [PATCH net v3 2/4] rtase: Correct the speed for RTL907XD-V1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Zzx5t24M0DG0yUrt@localhost.localdomain>
Date: Tue, 19 Nov 2024 12:42:47 +0100
From: Michal Kubiak <michal.kubiak@...el.com>
To: Justin Lai <justinlai0215@...ltek.com>
CC: "kuba@...nel.org" <kuba@...nel.org>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "andrew+netdev@...n.ch"
	<andrew+netdev@...n.ch>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "horms@...nel.org" <horms@...nel.org>, Ping-Ke Shih
	<pkshih@...ltek.com>, Larry Chiu <larry.chiu@...ltek.com>
Subject: Re: [PATCH net v3 2/4] rtase: Correct the speed for RTL907XD-V1

On Tue, Nov 19, 2024 at 07:23:12AM +0000, Justin Lai wrote:
> > 
> > On Mon, Nov 18, 2024 at 12:08:26PM +0800, Justin Lai wrote:

[...]

> > >
> > >
> > ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
> > >                                               supported);
> > > -     cmd->base.speed = SPEED_5000;
> > > +
> > > +     switch (tp->hw_ver) {
> > > +     case 0x00800000:
> > > +     case 0x04000000:
> > > +             cmd->base.speed = SPEED_5000;
> > > +             break;
> > > +     case 0x04800000:
> > > +             cmd->base.speed = SPEED_10000;
> > > +             break;
> > > +     }
> > > +
> > 
> > Above you are adding the code introducing some magic numbers and in your
> > last patch you are refactoring that newly added code.
> > Would it be possible to avoid those intermediate results and prepare the final
> > version of the fix in the series?
> 
> We would still prefer to follow the "single patch, single purpose"
> approach for this part. Other reviewers have also provided similar
> feedback, so we would like to maintain the current approach.
> 

I understand other reviewers' feedback because it's simply hard to
review the series with many intermediate changes in the same code.
Moreover, in this case, those intermediate changes can be easily avoided
by moving the patch #4 to the beginning of the series.
But still - I have doubts if the patch #4 can go into the "net" tree
since it doesn't have any functional fixes.

Thanks,
Michal