RE: [PATCH] PCI: designware: fix dw_pcie_cfg_write

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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...? 
Do you think it is impossible to have a scenario where: "where = hxxxxxx01" and  "size = 2"? 


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



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux