On Thursday 30 October 2008 21:42:51 Matthew Wilcox wrote:
> On Thu, Oct 30, 2008 at 01:32:17PM +0800, Sheng Yang wrote:
> > Some PCI device implemented PCI Advanced Feature, which is also support
> > Function Level Reset(FLR).
>
> I think patches 3 and 4 go off in the wrong direction. Rather than have
> one megafunction that knows how to reset all devices, we should have
> something more like this:
>
> int pci_execute_reset_function(pdev)
> {
> int res;
>
> res = pcie_flr(pdev);
> if (res != -ENOTTY)
> return res;
>
> res = pci_af_flr(pdev);
> if (res != -ENOTTY)
> return res;
>
> if (pdev->driver && pdev->driver->reset)
> res = pdev->driver->reset(pdev);
>
> return res;
> }
>
> (Assuming we add a reset op to the pci_driver function).
>
> It's a bit tricky in that we'd like to test early if a device supports
> reset or not.
Thanks for reviewing!
Yes, I like this approach much better. But for pci_reset_function(), I think
we still need a capability judgment. So how about like this?
int pci_execute_reset_function(pdev)
{
int res;
res = __pci_check_reset_function_cap(pdev);
switch (res) {
case pcie_flr:
res = pcie_flr(pdev);
break;
case pci_af_flr:
res = pci_af_flr(pdev);
break;
case reset_op:
if (pdev->driver && pdev->driver->reset)
res = pdev->driver->reset(pdev);
else
res = -ENOTTY;
break;
default:
res = -ENOTTY;
break;
}
return res;
}
Of course the capability check is a little duplicate, but I think it's
affordable.
--
regards
Yang, Sheng
>
> > Signed-off-by: Sheng Yang <sheng@xxxxxxxxxxxxxxx>
> > ---
> > drivers/pci/pci.c | 38 ++++++++++++++++++++++++++++++++++++--
> > 1 files changed, 36 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 2077aee..c432473 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -1752,6 +1752,7 @@ EXPORT_SYMBOL(pci_set_dma_seg_boundary);
> > #endif
> >
> > #define PCI_RESET_FUNC_CAP_PCIE_FLR 0
> > +#define PCI_RESET_FUNC_CAP_PCI_AF_FLR 1
> > static int __pci_check_reset_function_cap(struct pci_dev *dev)
> > {
> > int cappos;
> > @@ -1764,6 +1765,13 @@ static int __pci_check_reset_function_cap(struct
> > pci_dev *dev) return PCI_RESET_FUNC_CAP_PCIE_FLR;
> > }
> >
> > + cappos = pci_find_capability(dev, PCI_CAP_ID_AF);
> > + if (cappos) {
> > + pci_read_config_byte(dev, cappos + PCI_AF_CAP, (u8 *)&cap);
> > + if ((cap & PCI_AF_CAP_TP) && (cap & PCI_AF_CAP_FLR))
> > + return PCI_RESET_FUNC_CAP_PCI_AF_FLR;
> > + }
> > +
> > return -ENOTTY;
> > }
> >
> > @@ -1798,7 +1806,8 @@ int pci_execute_reset_function(struct pci_dev *dev)
> > r = 0;
> > pci_block_user_cfg_access(dev);
> >
> > - if (reset_cap == PCI_RESET_FUNC_CAP_PCIE_FLR) {
> > + switch (reset_cap) {
> > + case PCI_RESET_FUNC_CAP_PCIE_FLR:
> > /* Wait for Transaction Pending bit clean */
> > msleep(100);
> > cappos = pci_find_capability(dev, PCI_CAP_ID_EXP);
> > @@ -1816,8 +1825,33 @@ int pci_execute_reset_function(struct pci_dev
> > *dev) pci_write_config_word(dev, cappos + PCI_EXP_DEVCTL,
> > PCI_EXP_DEVCTL_BCR_FLR);
> > mdelay(100);
> > - } else
> > + break;
> > +
> > + case PCI_RESET_FUNC_CAP_PCI_AF_FLR:
> > + /* Wait for Transaction Pending bit clean */
> > + msleep(100);
> > + cappos = pci_find_capability(dev, PCI_CAP_ID_AF);
> > + pci_read_config_byte(dev, cappos + PCI_AF_STATUS,
> > + (u8 *)&status);
> > + if (status & PCI_AF_STATUS_TP) {
> > + dev_info(&dev->dev, "Busy after 100ms while trying to"
> > + " reset; sleeping for 1 second\n");
> > + ssleep(1);
> > + pci_read_config_byte(dev,
> > + cappos + PCI_AF_STATUS, (u8 *)&status);
> > + if (status & PCI_AF_STATUS_TP)
> > + dev_info(&dev->dev, "Still busy after 1s; "
> > + "proceeding with reset anyway\n");
> > + }
> > + pci_write_config_byte(dev, cappos + PCI_AF_CTRL,
> > + PCI_AF_CTRL_FLR);
> > + mdelay(100);
> > + break;
> > +
> > + default:
> > r = -ENOTTY;
> > + break;
> > + }
> >
> > pci_unblock_user_cfg_access(dev);
> > return r;
> > --
> > 1.5.4.5
> >
> > --
> > 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
