Received-SPF: pass (google.com: domain of linux-wireless+bounces-4219-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: Ping-Ke Shih <pkshih@realtek.com>
To: Sascha Hauer <sha@pengutronix.de>,
        Bitterblue Smith
	<rtl8821cerfe2@gmail.com>
CC: "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        "Sean
 Mollet" <sean@malmoset.com>
Subject: RE: [PATCH 1/4] wifi: rtw88: 8821cu: Fix firmware upload fail
Thread-Topic: [PATCH 1/4] wifi: rtw88: 8821cu: Fix firmware upload fail
Thread-Index: AQHaaXcR3HkprUxyPEepnjoj4RB95rEdn6yAgAA7GoCAARQJgIABmkjQ
Date: Thu, 29 Feb 2024 01:37:08 +0000
Message-ID: <d03453973370468aaf5213d61e8e3089@realtek.com>
References: <731ea688-04ef-4f02-9d01-3e9026981057@gmail.com>
 <Zd3b81m3_Hh47lww@pengutronix.de>
 <aacc9bf5-90e3-4e7a-af44-33d11fe89c18@gmail.com>
 <Zd71FWoTDn5plI5R@pengutronix.de>
In-Reply-To: <Zd71FWoTDn5plI5R@pengutronix.de>
Accept-Language: en-US, zh-TW
Content-Language: zh-TW
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
MIME-Version: 1.0


> -----Original Message-----
> From: Sascha Hauer <sha@pengutronix.de>
> Sent: Wednesday, February 28, 2024 4:56 PM
> To: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Cc: linux-wireless@vger.kernel.org; Ping-Ke Shih <pkshih@realtek.com>; Se=
an Mollet <sean@malmoset.com>
> Subject: Re: [PATCH 1/4] wifi: rtw88: 8821cu: Fix firmware upload fail
>=20
> On Tue, Feb 27, 2024 at 06:27:51PM +0200, Bitterblue Smith wrote:
> > Adding Sean Mollet because I forgot earlier.
> >
> > On 27/02/2024 14:56, Sascha Hauer wrote:
> > > On Tue, Feb 27, 2024 at 02:18:20PM +0200, Bitterblue Smith wrote:
> > >> +  if (addr < 0xFE00) {
> > >> +          if (addr <=3D 0xff)
> > >> +                  current_reg_sec =3D REG_ON_SEC;
> > >> +          else if (0x1000 <=3D addr && addr <=3D 0x10ff)
> > >> +                  current_reg_sec =3D REG_ON_SEC;
> > >> +          else
> > >> +                  current_reg_sec =3D REG_OFF_SEC;
> > >> +  } else {
> > >> +          current_reg_sec =3D REG_LOCAL_SEC;
> > >> +  }
> > >> +
> > >> +  if (current_reg_sec !=3D REG_ON_SEC)
> > >> +          return;
> > >
> > > Is there something we want to do with current_reg_sec =3D=3D REG_LOCA=
L_SEC
> > > or current_reg_sec =3D=3D REG_OFF_SEC later? If not the above could b=
e
> > > rewritten as:
> > >
> > >     if (addr > 0xff && addr < 0x1000)
> > >             return;
> > >     if (addr > 0x10ff)
> > >             return;
> > >
> > >     ...
> >
> > Dunno, I just copied the code from the other drivers:
> >
> >
> https://github.com/morrownr/8821cu-20210916/blob/5b39398e2de146edeb767164=
20f3288f508bea61/os_dep/linux
> /usb_ops_linux.c#L171
>=20
> Ok, nothing is done with current_reg_sec here as well, so I suggest
> rewriting the check like I suggested.

I also prefer rewriting the code, but we can add comments to describe there
are three sections:=20
  1. on (0x00~0xFF;0x1000~0x10FF): this section is always powered on
  2. off (< 0xFE00; but not on section): this section could be powered off
  3. local (>=3D 0xFE00): usb specific registers section=20

Since only on-section needs special deal, maybe positively listing register
ranges would be clear, like

	bool reg_on_sec =3D false;

	if ((addr >=3D 0x00 && addr <=3D 0xFF) ||
	    (addr >=3D 0x1000 && addr <=3D 0x10FF))
		reg_on_sec =3D true;

	if (!reg_on_sec)
		return;

Ping-Ke