netdev - RE: [PATCH net 1/4] rtase: Refactor the rtase_check_mac_version

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <baeeea41621c4b87a2a5152f72874556@realtek.com>
Date: Fri, 15 Nov 2024 05:57:55 +0000
From: Justin Lai <justinlai0215@...ltek.com>
To: Andrew Lunn <andrew@...n.ch>
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 1/4] rtase: Refactor the rtase_check_mac_version_valid() function

> 
> On Thu, Nov 14, 2024 at 07:14:40PM +0800, Justin Lai wrote:
> > 1. Sets tp->hw_ver.
> > 2. Changes the return type from bool to int.
> >
> > Signed-off-by: Justin Lai <justinlai0215@...ltek.com>
> > ---
> >  drivers/net/ethernet/realtek/rtase/rtase.h    |  2 ++
> >  .../net/ethernet/realtek/rtase/rtase_main.c   | 21 +++++++++++--------
> >  2 files changed, 14 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/realtek/rtase/rtase.h
> > b/drivers/net/ethernet/realtek/rtase/rtase.h
> > index 583c33930f88..547c71937b01 100644
> > --- a/drivers/net/ethernet/realtek/rtase/rtase.h
> > +++ b/drivers/net/ethernet/realtek/rtase/rtase.h
> > @@ -327,6 +327,8 @@ struct rtase_private {
> >       u16 int_nums;
> >       u16 tx_int_mit;
> >       u16 rx_int_mit;
> > +
> > +     u32 hw_ver;
> >  };
> >
> >  #define RTASE_LSO_64K 64000
> > diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > index f8777b7663d3..33808afd588d 100644
> > --- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > +++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > @@ -1972,20 +1972,21 @@ static void rtase_init_software_variable(struct
> pci_dev *pdev,
> >       tp->dev->max_mtu = RTASE_MAX_JUMBO_SIZE;  }
> >
> > -static bool rtase_check_mac_version_valid(struct rtase_private *tp)
> > +static int rtase_check_mac_version_valid(struct rtase_private *tp)
> >  {
> > -     u32 hw_ver = rtase_r32(tp, RTASE_TX_CONFIG_0) &
> RTASE_HW_VER_MASK;
> > -     bool known_ver = false;
> > +     int ret = -ENODEV;
> >
> > -     switch (hw_ver) {
> > +     tp->hw_ver = rtase_r32(tp, RTASE_TX_CONFIG_0) &
> > + RTASE_HW_VER_MASK;
> > +
> > +     switch (tp->hw_ver) {
> >       case 0x00800000:
> >       case 0x04000000:
> >       case 0x04800000:
> 
> Since these magic numbers are being used in more places, please add some
> #define with sensible names.

Ok, I will define these hardware version ID names.
> 
> > -     if (!rtase_check_mac_version_valid(tp))
> > -             return dev_err_probe(&pdev->dev, -ENODEV,
> > -                                  "unknown chip version, contact
> rtase maintainers (see MAINTAINERS file)\n");
> > +     ret = rtase_check_mac_version_valid(tp);
> > +     if (ret != 0) {
> > +             dev_err(&pdev->dev,
> > +                     "unknown chip version, contact rtase maintainers
> (see MAINTAINERS file)\n");
> > +     }
> 
> Since you are changing this, maybe include the hw_ver?

Thank you for your suggestion, I will add hw_ver to the error message.
> 
>         Andrew