Tony Chuang <yhchuang@xxxxxxxxxxx> 於 2019年11月13日 下午3:16 寫道：
>>> On Thu, Nov 7, 2019 at 7:16 PM <yhchuang@xxxxxxxxxxx> wrote:
>>>
>>> From: Yan-Hsuan Chuang <yhchuang@xxxxxxxxxxx>
>>>
>>> By Realtek's design, there are two HW modules associated for CLKREQ,
>>> one is responsible to follow the PCIE host settings, and another
>>> is to actually working on it. But the module that is actually working
>>> on it is default disabled, and driver should enable that module if
>>> host and device have successfully sync'ed with each other.
>>>
>>> The module is default disabled because sometimes the host does not
>>> support it, and if there is any incorrect settings (ex. CLKREQ# is
>>> not Bi-Direction), device can be lost and disconnected to the host.
>>>
>>> So driver should first check after host and device are sync'ed, and
>>> the host does support the function and set it in configuration
>>> space, then driver can turn on the HW module to working on it.
>>>
>>> Signed-off-by: Yan-Hsuan Chuang <yhchuang@xxxxxxxxxxx>
>>> ---
>>> drivers/net/wireless/realtek/rtw88/pci.c | 83
>> ++++++++++++++++++++++++
>>> drivers/net/wireless/realtek/rtw88/pci.h | 5 ++
>>> 2 files changed, 88 insertions(+)
>>>
>>> diff --git a/drivers/net/wireless/realtek/rtw88/pci.c
>> b/drivers/net/wireless/realtek/rtw88/pci.c
>>> index 6d1aa6f41e84..4fcef8a6fc42 100644
>>> --- a/drivers/net/wireless/realtek/rtw88/pci.c
>>> +++ b/drivers/net/wireless/realtek/rtw88/pci.c
>>> @@ -1081,6 +1081,33 @@ static void rtw_dbi_write8(struct rtw_dev
>> *rtwdev, u16 addr, u8 data)
>>> WARN(flag, "failed to write to DBI register, addr=0x%04x\n",
>> addr);
>>> }
>>>
>>> +static int rtw_dbi_read8(struct rtw_dev *rtwdev, u16 addr, u8 *value)
>>> +{
>>> + u16 read_addr = addr & BITS_DBI_ADDR_MASK;
>>> + u8 flag;
>>> + u8 cnt;
>>> +
>>> + rtw_write16(rtwdev, REG_DBI_FLAG_V1, read_addr);
>>> + rtw_write8(rtwdev, REG_DBI_FLAG_V1 + 2, BIT_DBI_RFLAG >>
>> 16);
>>> +
>>> + for (cnt = 0; cnt < RTW_PCI_WR_RETRY_CNT; cnt++) {
>>> + flag = rtw_read8(rtwdev, REG_DBI_FLAG_V1 + 2);
>>> + if (flag == 0)
>>> + break;
>> Why not just doing ' rtw_read8(rtwdev, read_addr);' and return here?
>> Then you don't need to check the flag != 0 at the following. It would
>> make the code cleaner and same expressive.
>
> Maybe it would look cleaner, but you need to add statements when
> 'if (flag == 0)', then the indents will be deeper.
> Also you will need to return 0 at the end if 'flag == 0'.
>
Yup. And I believe it’s still under 80 characters so I think I’m cool with it.
> So you still think it's cleaner to write it that way? If so, I can try to send
> v2 for it.
>>
>>> +
>>> + udelay(10);
>>> + }
>>> +
>>> + if (flag != 0) {
>>> + WARN(1, "failed to read DBI register, addr=0x%04x\n",
>> addr);
>>> + return -EIO;
>>> + }
>>> +
>>> + read_addr = REG_DBI_RDATA_V1 + (addr & 3);
>>> + *value = rtw_read8(rtwdev, read_addr);
>>> + return 0;
>>> +}
>>> +
>>> --
>>> 2.17.1
>>>
>>
>
> Yan-Hsuan
