> -----Original Message----- > From: Bjorn Helgaas [mailto:bhelgaas@xxxxxxxxxx] > Sent: Thursday, September 10, 2015 1:40 PM > To: Gabriele Paoloni > Cc: Jingoo Han; Pratyush Anand Thakur; linux-pci@xxxxxxxxxxxxxxx; > Wangzhou (B); Yuanzhichang; Zhudacai; zhangjukuo; qiuzhenfa; Liguozhu > (Kenneth) > Subject: Re: [PATCH] PCI: designware: change dw_pcie_cfg_write() and > dw_pcie_cfg_read() > > On Thu, Sep 10, 2015 at 6:26 AM, Gabriele Paoloni > <gabriele.paoloni@xxxxxxxxxx> wrote: > >> From: Bjorn Helgaas [mailto:bhelgaas@xxxxxxxxxx] > > >> I think there was a separate patch for the pcie-spear13xx.c bug, > >> wasn't there? At least, I don't see that fix in *this* patch. > >> > > > > From your previous comments: > > ******** > > *2) Pratyush, this one's for you :) spear13xx_pcie_establish_link() > > * calls dw_pcie_cfg_read() and dw_pcie_cfg_write() several times > like > > * this: > > * > > * dw_pcie_cfg_read(pp->dbi_base, exp_cap_off + PCI_EXP_DEVCTL, > 4, ...) > > * dw_pcie_cfg_write(pp->dbi_base, exp_cap_off + PCI_EXP_DEVCTL, > 4, ...) > > ******** > > > > I think that the implementation of dw_pcie_cfg_read() and > > dw_pcie_cfg_write() that I proposed in this patch is compatible with > > the previous calls in pcie-spear13xx.c. So the calls in pcie- > spear13xx.c > > were buggy for the previous implementations of the functions; with > > the reworked functions there is no need to modify the function calls > > in pcie-spear13xx.c...do you agree? > > Oh, I see, I didn't read it closely enough, but you're probably right. > What I would probably do is apply Pratyush's fix first, then apply > your patch on top. That way we can do the spear13xx bug fix by itself > (which should be a fairly obviously-correct patch), then your > interface change (which would be extended to touch all the calls > including spear13xx and be similarly obviously correct). I think that > would make it a bit easier for future changelog archaeologists. Since Pratyush has not pushed the fix yet, maybe I can prepare a v2 as a patchset of 2 patches.... (to benefit the Linux archeologists as you said :-) ) > > Bjorn ��.n��������+%������w��{.n�����{���"�)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
