From: Yijing Wang <wangyijing@huawei.com>
To: Gavin Shan <shangw@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Bjorn Helgaas <bhelgaas@google.com>,
"James E.J. Bottomley" <JBottomley@parallels.com>,
<linux-kernel@vger.kernel.org>, <linux-pci@vger.kernel.org>,
Hanjun Guo <guohanjun@huawei.com>,
Paul Mackerras <paulus@samba.org>,
<linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH v2 3/6] powerpc/pci: use pci_is_pcie() to simplify code
Date: Fri, 11 Oct 2013 16:22:25 +0800 [thread overview]
Message-ID: <5257B541.9050400@huawei.com> (raw)
In-Reply-To: <20131011075642.GA20443@shangw.(null)>
>> In my idea, dev->pcie_cap(here is pci_dev->pcie_cap) will update in set_pcie_port_type() function,
>> and this function always be called after allocate pci device. We get pci_dev by eeh_dev_to_pci_dev(),
>> I think pci_dev has been initialized completely.
>>
>>> This function has possibility to be invoked before that. However,
>>> we don't have the binding (eeh device <-> PCI device) for the case.
>>> So the piece of code shouldn't be running
>>
>> In PCI core, I knew
>>
>> pci_scan_device()
>> pci_setup_device()
>> set_pcie_port_type()
>> pci_dev->pcie_cap = pci_find_capability(pdev, PCI_CAP_ID_EXP);
>>
>> In powerpc, I also found
>>
>> of_scan_pci_dev()
>> of_create_pci_dev()
>> set_pcie_port_type()
>> pci_dev->pcie_cap = pci_find_capability(pdev, PCI_CAP_ID_EXP);
>>>
>>> However, it's a bit safer to have pci_find_capability(dev, PCI_CAP_ID_EXP)
>>> as well even though we needn't it for 99.9% cases if you agree :-)
>>
>> I agree, this function is not the performance bottleneck,
>> safety is more important. :)
>> So if Bjorn and Benjamin think it's not safe, it's ok to drop it. :)
>>
>
> No, it's not what I mean. Anyway, "v3" looks good to me.
> At least, it can save PCI-CFG access cycles find locate
> the PCIe capability position :-)
Thanks! :)
>
> Thanks,
> Gavin
>
>
> .
>
--
Thanks!
Yijing
WARNING: multiple messages have this Message-ID (diff)
From: Yijing Wang <wangyijing@huawei.com>
To: Gavin Shan <shangw@linux.vnet.ibm.com>
Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
"James E.J. Bottomley" <JBottomley@parallels.com>,
Paul Mackerras <paulus@samba.org>,
Hanjun Guo <guohanjun@huawei.com>,
Bjorn Helgaas <bhelgaas@google.com>,
linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v2 3/6] powerpc/pci: use pci_is_pcie() to simplify code
Date: Fri, 11 Oct 2013 16:22:25 +0800 [thread overview]
Message-ID: <5257B541.9050400@huawei.com> (raw)
In-Reply-To: <20131011075642.GA20443@shangw.(null)>
>> In my idea, dev->pcie_cap(here is pci_dev->pcie_cap) will update in set_pcie_port_type() function,
>> and this function always be called after allocate pci device. We get pci_dev by eeh_dev_to_pci_dev(),
>> I think pci_dev has been initialized completely.
>>
>>> This function has possibility to be invoked before that. However,
>>> we don't have the binding (eeh device <-> PCI device) for the case.
>>> So the piece of code shouldn't be running
>>
>> In PCI core, I knew
>>
>> pci_scan_device()
>> pci_setup_device()
>> set_pcie_port_type()
>> pci_dev->pcie_cap = pci_find_capability(pdev, PCI_CAP_ID_EXP);
>>
>> In powerpc, I also found
>>
>> of_scan_pci_dev()
>> of_create_pci_dev()
>> set_pcie_port_type()
>> pci_dev->pcie_cap = pci_find_capability(pdev, PCI_CAP_ID_EXP);
>>>
>>> However, it's a bit safer to have pci_find_capability(dev, PCI_CAP_ID_EXP)
>>> as well even though we needn't it for 99.9% cases if you agree :-)
>>
>> I agree, this function is not the performance bottleneck,
>> safety is more important. :)
>> So if Bjorn and Benjamin think it's not safe, it's ok to drop it. :)
>>
>
> No, it's not what I mean. Anyway, "v3" looks good to me.
> At least, it can save PCI-CFG access cycles find locate
> the PCIe capability position :-)
Thanks! :)
>
> Thanks,
> Gavin
>
>
> .
>
--
Thanks!
Yijing
next prev parent reply other threads:[~2013-10-11 8:23 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-05 7:55 [PATCH v2 1/6] scsi/bfa: use pcie_set/get_readrq to simplify code Yijing Wang
2013-09-05 7:55 ` Yijing Wang
2013-09-05 7:55 ` [PATCH v2 2/6] scsi/csiostor: use pcie_capability_xxx " Yijing Wang
2013-09-05 7:55 ` Yijing Wang
2013-09-05 7:55 ` [PATCH v2 3/6] powerpc/pci: use pci_is_pcie() " Yijing Wang
2013-09-05 7:55 ` Yijing Wang
2013-09-06 20:30 ` Bjorn Helgaas
2013-09-06 20:30 ` Bjorn Helgaas
2013-10-11 5:49 ` Benjamin Herrenschmidt
2013-10-11 5:49 ` Benjamin Herrenschmidt
2013-10-11 6:16 ` Gavin Shan
2013-10-11 6:33 ` Yijing Wang
2013-10-11 6:33 ` Yijing Wang
2013-10-11 6:53 ` Gavin Shan
2013-10-11 7:28 ` Yijing Wang
2013-10-11 7:28 ` Yijing Wang
2013-10-11 7:56 ` Gavin Shan
2013-10-11 8:22 ` Yijing Wang [this message]
2013-10-11 8:22 ` Yijing Wang
2013-10-11 6:28 ` Yijing Wang
2013-10-11 6:28 ` Yijing Wang
2013-09-05 7:55 ` [PATCH v2 4/6] x86/pci: use pcie_cap " Yijing Wang
2013-09-05 7:55 ` [PATCH v2 5/6] PCI: use pci_is_pcie() " Yijing Wang
2013-09-05 7:55 ` [PATCH v2 6/6] scsi/qla2xxx: use pcie_is_pcie() " Yijing Wang
2013-09-05 7:55 ` Yijing Wang
2013-09-06 18:17 ` [PATCH v2 1/6] scsi/bfa: use pcie_set/get_readrq " Jon Mason
2013-09-06 22:14 ` Bjorn Helgaas
2013-09-09 2:41 ` Yijing Wang
2013-09-09 2:41 ` Yijing Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5257B541.9050400@huawei.com \
--to=wangyijing@huawei.com \
--cc=JBottomley@parallels.com \
--cc=benh@kernel.crashing.org \
--cc=bhelgaas@google.com \
--cc=guohanjun@huawei.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=paulus@samba.org \
--cc=shangw@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.