> 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'.
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
