Messages in this thread |  | | Date | Thu, 21 Oct 2021 09:09:40 +0200 | From | Simon Horman <> | Subject | Re: [Intel-wired-lan] [PATCH linux-next] e1000: Remove redundant statement |
| |
On Wed, Oct 20, 2021 at 11:08:11AM -0700, Jesse Brandeburg wrote: > Apologies for the duplicates, mail from my intel account going out > through outlook.com is not being delivered. > > On Wed, Oct 20, 2021 at 7:00 AM Simon Horman <horms@kernel.org> wrote: > > > > Value stored to 'ctrl_reg' is never read. > > > > I agree this does seem to be the case. > > > > > Reported-by: Zeal Robot <zealci@zte.com.cn> > > > Signed-off-by: luo penghao <luo.penghao@zte.com.cn> > > > > Reviewed-by: Simon Horman <horms@kernel.org> > > Thanks for the review, but (davem/kuba) please do not apply.
Thanks, and sorry for misunderstanding the patch.
> > > > @@ -1215,8 +1215,6 @@ static int e1000_integrated_phy_loopback(struct e1000_adapter *adapter) > > > e1000_write_phy_reg(hw, PHY_CTRL, 0x8140); > > > } > > > > > > - ctrl_reg = er32(CTRL); > > Thanks for your patch, but this change is not safe. you're removing a > read that could do two things. The first is that the read "flushes" > the write just above to PCI (it's a PCI barrier), and the second is > that the read can have some side effects. > > If this change must be done, the code should be to remove the > assignment to ctrl_reg, but leave the read, so the line would just > look like: > er32(CTRL); > > This will get rid of the warning and not change the flow from the > hardware perspective. > > > > - > > > /* force 1000, set loopback */ > > > e1000_write_phy_reg(hw, PHY_CTRL, 0x4140); > > > > > Please do not apply this. >
|  |