RE: [PATCH net v3 2/4] rtase: Correct the speed for RTL907XD-V1
From: Justin Lai
Date: Wed Nov 20 2024 - 01:40:52 EST
>
> 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