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

From: Justin Lai <[email-masked]>
Date: Wed, 20 Nov 2024 06:40:11 +0000

> 
> 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

Hi Michal,

Thank you for your response. I will integrate the addition of the hardware
version ID definitions into patch #1, as this will address both of your
concerns at once.

Justin