| Message ID | 1628084828-119542-6-git-send-email-liudongdong3@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | [V7,1/9] PCI: Use cached Device Capabilities Register | expand |
On Wed, Aug 04, 2021 at 09:47:04PM +0800, Dongdong Liu wrote: > Enable VF 10-Bit Tag Requester when it's upstream component support > 10-bit Tag Completer. s/it's/its/ s/support/supports/ I think "upstream component" here means the PF, doesn't it? I don't think the PF is really an *upstream* component; there's no routing like with a switch. > Signed-off-by: Dongdong Liu <liudongdong3@huawei.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> > --- > drivers/pci/iov.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c > index dafdc65..0d0bed1 100644 > --- a/drivers/pci/iov.c > +++ b/drivers/pci/iov.c > @@ -634,6 +634,10 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn) > > pci_iov_set_numvfs(dev, nr_virtfn); > iov->ctrl |= PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE; > + if ((iov->cap & PCI_SRIOV_CAP_VF_10BIT_TAG_REQ) && > + dev->ext_10bit_tag) > + iov->ctrl |= PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN; > + > pci_cfg_access_lock(dev); > pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl); > msleep(100); > @@ -650,6 +654,8 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn) > > err_pcibios: > iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE); > + if (iov->ctrl & PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN) > + iov->ctrl &= ~PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN; > pci_cfg_access_lock(dev); > pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl); > ssleep(1); > @@ -682,6 +688,8 @@ static void sriov_disable(struct pci_dev *dev) > > sriov_del_vfs(dev); > iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE); > + if (iov->ctrl & PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN) > + iov->ctrl &= ~PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN; You can just clear PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN unconditionally, can't you? I know it wouldn't change anything, but removing the "if" makes the code prettier. You could just add it in the existing PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE mask. > pci_cfg_access_lock(dev); > pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl); > ssleep(1); > -- > 2.7.4 >
On 2021/8/5 7:29, Bjorn Helgaas wrote: > On Wed, Aug 04, 2021 at 09:47:04PM +0800, Dongdong Liu wrote: >> Enable VF 10-Bit Tag Requester when it's upstream component support >> 10-bit Tag Completer. > > s/it's/its/ > s/support/supports/ Will fix. > > I think "upstream component" here means the PF, doesn't it? I don't > think the PF is really an *upstream* component; there's no routing > like with a switch. I want to say the switch and root port devices that support 10-Bit Tag Completer. Sure, VF also needs to have 10-bit Tag Requester Supported capability. > >> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com> >> Reviewed-by: Christoph Hellwig <hch@lst.de> >> --- >> drivers/pci/iov.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c >> index dafdc65..0d0bed1 100644 >> --- a/drivers/pci/iov.c >> +++ b/drivers/pci/iov.c >> @@ -634,6 +634,10 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn) >> >> pci_iov_set_numvfs(dev, nr_virtfn); >> iov->ctrl |= PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE; >> + if ((iov->cap & PCI_SRIOV_CAP_VF_10BIT_TAG_REQ) && >> + dev->ext_10bit_tag) >> + iov->ctrl |= PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN; >> + >> pci_cfg_access_lock(dev); >> pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl); >> msleep(100); >> @@ -650,6 +654,8 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn) >> >> err_pcibios: >> iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE); >> + if (iov->ctrl & PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN) >> + iov->ctrl &= ~PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN; >> pci_cfg_access_lock(dev); >> pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl); >> ssleep(1); >> @@ -682,6 +688,8 @@ static void sriov_disable(struct pci_dev *dev) >> >> sriov_del_vfs(dev); >> iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE); >> + if (iov->ctrl & PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN) >> + iov->ctrl &= ~PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN; > > You can just clear PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN unconditionally, > can't you? I know it wouldn't change anything, but removing the "if" > makes the code prettier. You could just add it in the existing > PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE mask. Will do. Thanks, Dongdong > >> pci_cfg_access_lock(dev); >> pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl); >> ssleep(1); >> -- >> 2.7.4 >> > . >
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c index dafdc65..0d0bed1 100644 --- a/drivers/pci/iov.c +++ b/drivers/pci/iov.c @@ -634,6 +634,10 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn) pci_iov_set_numvfs(dev, nr_virtfn); iov->ctrl |= PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE; + if ((iov->cap & PCI_SRIOV_CAP_VF_10BIT_TAG_REQ) && + dev->ext_10bit_tag) + iov->ctrl |= PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN; + pci_cfg_access_lock(dev); pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl); msleep(100); @@ -650,6 +654,8 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn) err_pcibios: iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE); + if (iov->ctrl & PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN) + iov->ctrl &= ~PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN; pci_cfg_access_lock(dev); pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl); ssleep(1); @@ -682,6 +688,8 @@ static void sriov_disable(struct pci_dev *dev) sriov_del_vfs(dev); iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE); + if (iov->ctrl & PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN) + iov->ctrl &= ~PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN; pci_cfg_access_lock(dev); pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl); ssleep(1);