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