> -----Original Message----- > From: Jingoo Han [mailto:jingoohan1@xxxxxxxxx] > Sent: Friday, August 21, 2015 10:25 AM > To: Gabriele Paoloni > Cc: Wangzhou (B); <bhelgaas@xxxxxxxxxx>; <linux-pci@xxxxxxxxxxxxxxx>; > <pratyush.anand@xxxxxxxxx>; Yuanzhichang; Zhudacai; zhangjukuo; > qiuzhenfa; Liguozhu (Kenneth); jingoohan1@xxxxxxxxx > Subject: Re: [PATCH] PCI: designware: fix dw_pcie_cfg_write > > On 2015. 8. 21., at PM 5:58, Gabriele Paoloni > <gabriele.paoloni@xxxxxxxxxx> wrote: > > > > Hi Jingoo > > > >> -----Original Message----- > >> From: linux-pci-owner@xxxxxxxxxxxxxxx [mailto:linux-pci- > >> owner@xxxxxxxxxxxxxxx] On Behalf Of Jingoo Han > >> Sent: Friday, August 21, 2015 5:48 AM > >> To: Wangzhou (B) > >> Cc: Gabriele Paoloni; <bhelgaas@xxxxxxxxxx>; <linux- > >> pci@xxxxxxxxxxxxxxx>; <pratyush.anand@xxxxxxxxx>; Yuanzhichang; > >> Zhudacai; zhangjukuo; qiuzhenfa; Liguozhu (Kenneth); > >> jingoohan1@xxxxxxxxx > >> Subject: Re: [PATCH] PCI: designware: fix dw_pcie_cfg_write > >> > >>> On 2015. 8. 21., at AM 10:51, Zhou Wang <wangzhou1@xxxxxxxxxxxxx> > wrote: > >>> > >>>> On 2015/8/20 21:58, Gabriele Paoloni wrote: > >>>> From: gabriele paoloni <gabriele.paoloni@xxxxxxxxxx> > >>>> > >>>> Currently in dw_pcie_cfg_write() if the input size is 2 bytes > >>>> the address offset is wrongly calculated as we mask also bit0 > >>>> of "where" input parameter. Instead we should mask all bits of > >>>> "where" except bit0 and bit1. > >>>> > >>>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@xxxxxxxxxx> > >>>> --- > >>>> drivers/pci/host/pcie-designware.c | 2 +- > >>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>> > >>>> diff --git a/drivers/pci/host/pcie-designware.c > >> b/drivers/pci/host/pcie-designware.c > >>>> index 69486be..a27f536 100644 > >>>> --- a/drivers/pci/host/pcie-designware.c > >>>> +++ b/drivers/pci/host/pcie-designware.c > >>>> @@ -99,7 +99,7 @@ int dw_pcie_cfg_write(void __iomem *addr, int > >> where, int size, u32 val) > >>>> if (size == 4) > >>>> writel(val, addr); > >>>> else if (size == 2) > >>>> - writew(val, addr + (where & 2)); > >>>> + writew(val, addr + (where & 3)); > >>>> else if (size == 1) > >>>> writeb(val, addr + (where & 3)); > >>>> else > >>> > >>> Hi Jingoo, > >>> > >>> Is there some special consideration? If we miss something, please > let > >> us know. > >> > >> This patch is unnecessary. > >> > >> In the case of 'size == 2', the writew() in dw_pcie_cfg_write() > should > >> handle the following values of 'where'. > >> > >> h00 b0000 > >> h02 b0010 > >> > >> Thus, there is no need to keep 0th bit. > > > > What if the last bits of "where" are h01(b0001), then the current > implementation will clear the last bit...? > > Yep. > > > Do you think it is impossible to have a scenario where: "where = > hxxxxxx01" and "size = 2"? > > Is there any function using "where = h01" and "size =2"? > It might be wrong usage. Please correct me if I am wrong... For example if a designware based controller needs to modify PCI_CLASS_PROG, then it will end up writing PCI_REVISION_ID. Now this is maybe not happening now, but, since it is a quick fix and since dw_pcie_cfg_write() is a base access function, I just thought better to have it fixed now to avoid any future bug... However since you re the maintainer it is up to you... > > Best regards, > Jingoo Han > > >> > >> One more thing, is there any reason to urge me to review this patch > >> within a few hours? > >> Please wait for reviews for enough hours. > >> > >> Maybe, your company looks to support you and other engineers for > >> mainline kernel. > >> However, I am not supported at all, so it is not easy to review the > >> patches quickly. > > > > Jingoo I am sorry for this. I am sure Zhou Wang did not mean to push > you. > > > > I agree that this patch is low prio...for us I think is much more > important > > to have "PCI: hisi: Add PCIe host support for HiSilicon SoC Hip05" > patchset > > pushed in mainline. > > > >> Best regards, > >> Jingoo Han > >> > >>> Thanks, > >>> Zhou > >> -- > >> To unsubscribe from this list: send the line "unsubscribe linux-pci" > in > >> the body of a message to majordomo@xxxxxxxxxxxxxxx > >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
