> -----Original Message----- > From: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> > Sent: Tuesday, March 14, 2023 4:12 AM > To: Ping-Ke Shih <pkshih@xxxxxxxxxxx> > Cc: linux-wireless@xxxxxxxxxxxxxxx; Yan-Hsuan Chuang <tony0620emma@xxxxxxxxx>; Kalle Valo > <kvalo@xxxxxxxxxx>; Ulf Hansson <ulf.hansson@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; > netdev@xxxxxxxxxxxxxxx; linux-mmc@xxxxxxxxxxxxxxx; Chris Morgan <macroalpha82@xxxxxxxxx>; Nitin Gupta > <nitin.gupta981@xxxxxxxxx>; Neo Jou <neojou@xxxxxxxxx>; Jernej Skrabec <jernej.skrabec@xxxxxxxxx> > Subject: Re: [PATCH v2 RFC 3/9] wifi: rtw88: mac: Support SDIO specific bits in the power on sequence > > Hello Ping-Ke, > > On Mon, Mar 13, 2023 at 10:05 AM Ping-Ke Shih <pkshih@xxxxxxxxxxx> wrote: > [...] > > > pwr_seq = pwr_on ? chip->pwr_on_seq : chip->pwr_off_seq; > > > ret = rtw_pwr_seq_parser(rtwdev, pwr_seq); > > > - if (ret) > > > - return ret; > > > + > > > + if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_SDIO) > > > + rtw_write32(rtwdev, REG_SDIO_HIMR, imr); > > > > > > if (pwr_on) > > > set_bit(RTW_FLAG_POWERON, rtwdev->flags); > > > > If failed to power on, it still set RTW_FLAG_POWERON. Is it reasonable? > That sounds very reasonable to me! Let me clear here more. Consider a use case: 1. Initially, it enters this function with pwr_on = true, and RTW_FLAG_POWERON is unset. 2. rtw_pwr_seq_parser() return error, so power state is uncertain. 3. Unconditionally, set RTW_FLAG_POWERON. rtw_mac_power_on() will try to power off/on once again if it fails, so in step 3 setting RTW_FLAG_POWERON only if rtw_pwr_seq_parser() returns 0 can have the same values as step 1, when it retries to power on. Honestly, we don't have perfect error handle for error of rtw_pwr_seq_parser(), but still want to make things easier understand. > > > Did you meet real problem here? > > > > Maybe, here can be > > > > if (pwr_on && !ret) > > set_bit(RTW_FLAG_POWERON, rtwdev->flags); > I can't remember any issue that I've seen. I'll verify this at the end > of the week (until then I am pretty busy with my daytime job) and then > go with your suggestion. Thanks. Wait for you. > Thanks again as always - your feedback is really appreciated! > > Also thank you for commenting on the other patches. I'll take a closer > look at your feedback at the end of the week and send another version > of this series. > I also thank you for cooking these patches voluntarily for people who can use their own wifi happily. :-) Ping-Ke
