Re: [RFC] use pci_find_ext_capability everywhere

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

 



You're applying this one?  Would be best to try with the linux-next branch of 
my pci-2.6 tree; I don't have a backport to 2.6.26.5 handy...

Jesse

On Monday, October 6, 2008 2:11 pm Tomasz Czernecki wrote:
> Hi Yanmin,
>
> When I attempt to apply the patch to a freshly extracted, uncompiled,
> clean 2.6.26.5 kernel using cat <file> | patch -p1, I get errors.
> Please see the attached screen capture.  Can you comment?  Is this
> patch only for newer kernels, or is there another version available
> that works for mine?
>
> Thanks.
> Tomasz
>
> On Sun, Oct 5, 2008 at 9:12 PM, Tomasz Czernecki <czernecki@xxxxxxxxx> 
wrote:
> > I'll try to give this a run tomorrow - thanks Yanmin & others.
> >
> > Tomasz
> >
> > On 10/5/08, Zhang, Yanmin <yanmin_zhang@xxxxxxxxxxxxxxx> wrote:
> >> Tomasz,
> >>
> >>  Matthew and Jesse have a better patch to fix the AER issue on your
> >> machine. Would you like to help test the new patch? You could get the
> >> new patch at:
> >>  http://article.gmane.org/gmane.linux.kernel.pci/1115.
> >>
> >>  Thanks,
> >>  Yanmin
> >>
> >>  On Fri, 2008-10-03 at 09:24 -0700, Jesse Barnes wrote:
> >>  > On Friday, October 3, 2008 8:39 am Jesse Barnes wrote:
> >>  > > On Friday, October 3, 2008 7:50 am Matthew Wilcox wrote:
> >>  > > > On Thu, Oct 02, 2008 at 11:50:24AM -0700, Jesse Barnes wrote:
> >>  > > > > +++ b/drivers/pci/pcie/aer/aerdrv.c
> >>  > > > > @@ -105,7 +105,7 @@ static irqreturn_t aer_irq(int irq, void
> >>  > > > > *context) unsigned long flags;
> >>  > > > >         int pos;
> >>  > > > >
> >>  > > > > -       pos = pci_find_aer_capability(pdev->port);
> >>  > > > > +       pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
> >>  > > > >         /*
> >>  > > > >          * Must lock access to Root Error Status Reg, Root
> >>  > > > > Error ID Reg, * and Root error producer/consumer index
> >>  > > >
> >>  > > > Waitaminute ... you didn't actually compile this, did you?  pdev
> >>  > > > changes
> >>  > > >
> >>  > > > to dev in three other hunks:
> >>  > >
> >>  > > Doh!  You caught me. :)  That's why I was careful to put [RFC]
> >>  > > rather than [PATCH] on the subject.
> >>  > >
> >>  > > But since you're looking... I'll respin and include a change to do
> >>  > > something similar to get_port_device_capability(), please check it
> >>  > > out when you get a chance.
> >>  >
> >>  > Ok, here's an updated version that fixes the qla_os.c change (Andrew,
> >>  > how does this look?) and also fixes up the portdrv_core.c stuff
> >>  > (Yanmin can you take a look at that to see if it'll fix your
> >>  > problem?).
> >>  >
> >>  > Thanks,
> >>  > --
> >>  > Jesse Barnes, Intel Open Source Technology Center
> >>  >
> >>  >
> >>  > diff --git a/drivers/pci/pcie/aer/aerdrv.c
> >>  > b/drivers/pci/pcie/aer/aerdrv.c index 77036f4..e390707 100644
> >>  > --- a/drivers/pci/pcie/aer/aerdrv.c
> >>  > +++ b/drivers/pci/pcie/aer/aerdrv.c
> >>  > @@ -105,7 +105,7 @@ static irqreturn_t aer_irq(int irq, void
> >>  > *context) unsigned long flags;
> >>  >       int pos;
> >>  >
> >>  > -     pos = pci_find_aer_capability(pdev->port);
> >>  > +     pos = pci_find_ext_capability(pdev->port, PCI_EXT_CAP_ID_ERR);
> >>  >       /*
> >>  >        * Must lock access to Root Error Status Reg, Root Error ID
> >>  > Reg, * and Root error producer/consumer index
> >>  > @@ -252,7 +252,7 @@ static pci_ers_result_t aer_root_reset(struct
> >>  > pci_dev *dev)
> >>  >       u32 status;
> >>  >       int pos;
> >>  >
> >>  > -     pos = pci_find_aer_capability(dev);
> >>  > +     pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
> >>  >
> >>  >       /* Disable Root's interrupt in response to error messages */
> >>  >       pci_write_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, 0);
> >>  > @@ -316,7 +316,7 @@ static void aer_error_resume(struct pci_dev *dev)
> >>  >       pci_write_config_word(dev, pos + PCI_EXP_DEVSTA, reg16);
> >>  >
> >>  >       /* Clean AER Root Error Status */
> >>  > -     pos = pci_find_aer_capability(dev);
> >>  > +     pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
> >>  >       pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS,
> >>  > &status); pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER,
> >>  > &mask); if (dev->error_state == pci_channel_io_normal)
> >>  > diff --git a/drivers/pci/pcie/aer/aerdrv_core.c
> >>  > b/drivers/pci/pcie/aer/aerdrv_core.c
> >>  > index 6f5244b..2ef8bbd 100644
> >>  > --- a/drivers/pci/pcie/aer/aerdrv_core.c
> >>  > +++ b/drivers/pci/pcie/aer/aerdrv_core.c
> >>  > @@ -28,35 +28,6 @@
> >>  >  static int forceload;
> >>  >  module_param(forceload, bool, 0);
> >>  >
> >>  > -int pci_find_aer_capability(struct pci_dev *dev)
> >>  > -{
> >>  > -     int pos;
> >>  > -     u32 reg32 = 0;
> >>  > -
> >>  > -     /* Check if it's a pci-express device */
> >>  > -     pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
> >>  > -     if (!pos)
> >>  > -             return 0;
> >>  > -
> >>  > -     /* Check if it supports pci-express AER */
> >>  > -     pos = PCI_CFG_SPACE_SIZE;
> >>  > -     while (pos) {
> >>  > -             if (pci_read_config_dword(dev, pos, &reg32))
> >>  > -                     return 0;
> >>  > -
> >>  > -             /* some broken boards return ~0 */
> >>  > -             if (reg32 == 0xffffffff)
> >>  > -                     return 0;
> >>  > -
> >>  > -             if (PCI_EXT_CAP_ID(reg32) == PCI_EXT_CAP_ID_ERR)
> >>  > -                     break;
> >>  > -
> >>  > -             pos = reg32 >> 20;
> >>  > -     }
> >>  > -
> >>  > -     return pos;
> >>  > -}
> >>  > -
> >>  >  int pci_enable_pcie_error_reporting(struct pci_dev *dev)
> >>  >  {
> >>  >       u16 reg16 = 0;
> >>  > @@ -66,6 +37,10 @@ int pci_enable_pcie_error_reporting(struct pci_dev
> >>  > *dev) if (!pos)
> >>  >               return -EIO;
> >>  >
> >>  > +     pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
> >>  > +     if (!pos)
> >>  > +             return -EIO;
> >>  > +
> >>  >       pci_read_config_word(dev, pos+PCI_EXP_DEVCTL, &reg16);
> >>  >       reg16 = reg16 |
> >>  >               PCI_EXP_DEVCTL_CERE |
> >>  > @@ -101,7 +76,7 @@ int pci_cleanup_aer_uncorrect_error_status(struct
> >>  > pci_dev *dev)
> >>  >       int pos;
> >>  >       u32 status, mask;
> >>  >
> >>  > -     pos = pci_find_aer_capability(dev);
> >>  > +     pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
> >>  >       if (!pos)
> >>  >               return -EIO;
> >>  >
> >>  > @@ -122,7 +97,7 @@ int pci_cleanup_aer_correct_error_status(struct
> >>  > pci_dev *dev)
> >>  >       int pos;
> >>  >       u32 status;
> >>  >
> >>  > -     pos = pci_find_aer_capability(dev);
> >>  > +     pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
> >>  >       if (!pos)
> >>  >               return -EIO;
> >>  >
> >>  > @@ -501,7 +476,7 @@ static void handle_error_source(struct
> >>  > pcie_device * aerdev,
> >>  >                * Correctable error does not need software
> >>  > intevention. * No need to go through error recovery process. */
> >>  > -             pos = pci_find_aer_capability(dev);
> >>  > +             pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
> >>  >               if (pos)
> >>  >                       pci_write_config_dword(dev, pos +
> >>  > PCI_ERR_COR_STATUS, info.status);
> >>  > @@ -541,7 +516,7 @@ void aer_enable_rootport(struct aer_rpc *rpc)
> >>  >       reg16 &= ~(SYSTEM_ERROR_INTR_ON_MESG_MASK);
> >>  >       pci_write_config_word(pdev, pos + PCI_EXP_RTCTL, reg16);
> >>  >
> >>  > -     aer_pos = pci_find_aer_capability(pdev);
> >>  > +     aer_pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_ERR);
> >>  >       /* Clear error status */
> >>  >       pci_read_config_dword(pdev, aer_pos + PCI_ERR_ROOT_STATUS,
> >>  > &reg32); pci_write_config_dword(pdev, aer_pos + PCI_ERR_ROOT_STATUS,
> >>  > reg32); @@ -578,7 +553,7 @@ static void disable_root_aer(struct
> >>  > aer_rpc *rpc) u32 reg32;
> >>  >       int pos;
> >>  >
> >>  > -     pos = pci_find_aer_capability(pdev);
> >>  > +     pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_ERR);
> >>  >       /* Disable Root's interrupt in response to error messages */
> >>  >       pci_write_config_dword(pdev, pos + PCI_ERR_ROOT_COMMAND, 0);
> >>  >
> >>  > @@ -617,7 +592,7 @@ static int get_device_error_info(struct pci_dev
> >>  > *dev, struct aer_err_info *info)
> >>  >  {
> >>  >       int pos;
> >>  >
> >>  > -     pos = pci_find_aer_capability(dev);
> >>  > +     pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
> >>  >
> >>  >       /* The device might not support AER */
> >>  >       if (!pos)
> >>  > diff --git a/drivers/pci/pcie/portdrv_core.c
> >>  > b/drivers/pci/pcie/portdrv_core.c index 18bb811..6cdd41f 100644
> >>  > --- a/drivers/pci/pcie/portdrv_core.c
> >>  > +++ b/drivers/pci/pcie/portdrv_core.c
> >>  > @@ -196,23 +196,10 @@ static int get_port_device_capability(struct
> >>  > pci_dev *dev)
> >>  >       if (((reg16 >> 4) & PORT_TYPE_MASK) == PCIE_RC_PORT)
> >>  >               services |= PCIE_PORT_SERVICE_PME;
> >>  >
> >>  > -     pos = PCI_CFG_SPACE_SIZE;
> >>  > -     while (pos) {
> >>  > -             pci_read_config_dword(dev, pos, &reg32);
> >>  > -             switch (reg32 & 0xffff) {
> >>  > -             case PCI_EXT_CAP_ID_ERR:
> >>  > -                     services |= PCIE_PORT_SERVICE_AER;
> >>  > -                     pos = reg32 >> 20;
> >>  > -                     break;
> >>  > -             case PCI_EXT_CAP_ID_VC:
> >>  > -                     services |= PCIE_PORT_SERVICE_VC;
> >>  > -                     pos = reg32 >> 20;
> >>  > -                     break;
> >>  > -             default:
> >>  > -                     pos = 0;
> >>  > -                     break;
> >>  > -             }
> >>  > -     }
> >>  > +     if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR))
> >>  > +             services |= PCIE_PORT_SERVICE_AER;
> >>  > +     if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VC))
> >>  > +             services |= PCIE_PORT_SERVICE_VC;
> >>  >
> >>  >       return services;
> >>  >  }
> >>  > diff --git a/drivers/scsi/qla2xxx/qla_os.c
> >>  > b/drivers/scsi/qla2xxx/qla_os.c index 26afe44..7e79835 100644
> >>  > --- a/drivers/scsi/qla2xxx/qla_os.c
> >>  > +++ b/drivers/scsi/qla2xxx/qla_os.c
> >>  > @@ -1567,9 +1567,8 @@ qla2x00_probe_one(struct pci_dev *pdev, const
> >>  > struct pci_device_id *id)
> >>  >                       goto probe_out;
> >>  >       }
> >>  >
> >>  > -     if (pci_find_aer_capability(pdev))
> >>  > -             if (pci_enable_pcie_error_reporting(pdev))
> >>  > -                     goto probe_out;
> >>  > +     if (!pci_enable_pcie_error_reporting(pdev))
> >>  > +             printk(KERN_INFO "qla2xxx: Enabled error reporting\n");
> >>  >
> >>  >       host = scsi_host_alloc(sht, sizeof(scsi_qla_host_t));
> >>  >       if (host == NULL) {



-- 
Jesse Barnes, Intel Open Source Technology Center
diff --git a/drivers/pci/pcie/aer/aerdrv.c b/drivers/pci/pcie/aer/aerdrv.c
index 77036f4..e390707 100644
--- a/drivers/pci/pcie/aer/aerdrv.c
+++ b/drivers/pci/pcie/aer/aerdrv.c
@@ -105,7 +105,7 @@ static irqreturn_t aer_irq(int irq, void *context)
 	unsigned long flags;
 	int pos;
 
-	pos = pci_find_aer_capability(pdev->port);
+	pos = pci_find_ext_capability(pdev->port, PCI_EXT_CAP_ID_ERR);
 	/*
 	 * Must lock access to Root Error Status Reg, Root Error ID Reg,
 	 * and Root error producer/consumer index
@@ -252,7 +252,7 @@ static pci_ers_result_t aer_root_reset(struct pci_dev *dev)
 	u32 status;
 	int pos;
 
-	pos = pci_find_aer_capability(dev);
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
 
 	/* Disable Root's interrupt in response to error messages */
 	pci_write_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, 0);
@@ -316,7 +316,7 @@ static void aer_error_resume(struct pci_dev *dev)
 	pci_write_config_word(dev, pos + PCI_EXP_DEVSTA, reg16);
 
 	/* Clean AER Root Error Status */
-	pos = pci_find_aer_capability(dev);
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
 	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, &status);
 	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, &mask);
 	if (dev->error_state == pci_channel_io_normal)
diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c
index 6f5244b..2ef8bbd 100644
--- a/drivers/pci/pcie/aer/aerdrv_core.c
+++ b/drivers/pci/pcie/aer/aerdrv_core.c
@@ -28,35 +28,6 @@
 static int forceload;
 module_param(forceload, bool, 0);
 
-int pci_find_aer_capability(struct pci_dev *dev)
-{
-	int pos;
-	u32 reg32 = 0;
-
-	/* Check if it's a pci-express device */
-	pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
-	if (!pos)
-		return 0;
-
-	/* Check if it supports pci-express AER */
-	pos = PCI_CFG_SPACE_SIZE;
-	while (pos) {
-		if (pci_read_config_dword(dev, pos, &reg32))
-			return 0;
-
-		/* some broken boards return ~0 */
-		if (reg32 == 0xffffffff)
-			return 0;
-
-		if (PCI_EXT_CAP_ID(reg32) == PCI_EXT_CAP_ID_ERR)
-			break;
-
-		pos = reg32 >> 20;
-	}
-
-	return pos;
-}
-
 int pci_enable_pcie_error_reporting(struct pci_dev *dev)
 {
 	u16 reg16 = 0;
@@ -66,6 +37,10 @@ int pci_enable_pcie_error_reporting(struct pci_dev *dev)
 	if (!pos)
 		return -EIO;
 
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
+	if (!pos)
+		return -EIO;
+
 	pci_read_config_word(dev, pos+PCI_EXP_DEVCTL, &reg16);
 	reg16 = reg16 |
 		PCI_EXP_DEVCTL_CERE |
@@ -101,7 +76,7 @@ int pci_cleanup_aer_uncorrect_error_status(struct pci_dev *dev)
 	int pos;
 	u32 status, mask;
 
-	pos = pci_find_aer_capability(dev);
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
 	if (!pos)
 		return -EIO;
 
@@ -122,7 +97,7 @@ int pci_cleanup_aer_correct_error_status(struct pci_dev *dev)
 	int pos;
 	u32 status;
 
-	pos = pci_find_aer_capability(dev);
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
 	if (!pos)
 		return -EIO;
 
@@ -501,7 +476,7 @@ static void handle_error_source(struct pcie_device * aerdev,
 		 * Correctable error does not need software intevention.
 		 * No need to go through error recovery process.
 		 */
-		pos = pci_find_aer_capability(dev);
+		pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
 		if (pos)
 			pci_write_config_dword(dev, pos + PCI_ERR_COR_STATUS,
 					info.status);
@@ -541,7 +516,7 @@ void aer_enable_rootport(struct aer_rpc *rpc)
 	reg16 &= ~(SYSTEM_ERROR_INTR_ON_MESG_MASK);
 	pci_write_config_word(pdev, pos + PCI_EXP_RTCTL, reg16);
 
-	aer_pos = pci_find_aer_capability(pdev);
+	aer_pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_ERR);
 	/* Clear error status */
 	pci_read_config_dword(pdev, aer_pos + PCI_ERR_ROOT_STATUS, &reg32);
 	pci_write_config_dword(pdev, aer_pos + PCI_ERR_ROOT_STATUS, reg32);
@@ -578,7 +553,7 @@ static void disable_root_aer(struct aer_rpc *rpc)
 	u32 reg32;
 	int pos;
 
-	pos = pci_find_aer_capability(pdev);
+	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_ERR);
 	/* Disable Root's interrupt in response to error messages */
 	pci_write_config_dword(pdev, pos + PCI_ERR_ROOT_COMMAND, 0);
 
@@ -617,7 +592,7 @@ static int get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
 {
 	int pos;
 
-	pos = pci_find_aer_capability(dev);
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
 
 	/* The device might not support AER */
 	if (!pos)
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 18bb811..6cdd41f 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -196,23 +196,10 @@ static int get_port_device_capability(struct pci_dev *dev)
 	if (((reg16 >> 4) & PORT_TYPE_MASK) == PCIE_RC_PORT)
 		services |= PCIE_PORT_SERVICE_PME;
 
-	pos = PCI_CFG_SPACE_SIZE;
-	while (pos) {
-		pci_read_config_dword(dev, pos, &reg32);
-		switch (reg32 & 0xffff) {
-		case PCI_EXT_CAP_ID_ERR:
-			services |= PCIE_PORT_SERVICE_AER;
-			pos = reg32 >> 20;
-			break;
-		case PCI_EXT_CAP_ID_VC:
-			services |= PCIE_PORT_SERVICE_VC;
-			pos = reg32 >> 20;
-			break;
-		default:
-			pos = 0;
-			break;
-		}
-	}
+	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR))
+		services |= PCIE_PORT_SERVICE_AER;
+	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VC))
+		services |= PCIE_PORT_SERVICE_VC;
 
 	return services;
 }
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 26afe44..28ea570 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -1567,9 +1567,8 @@ qla2x00_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
 			goto probe_out;
 	}
 
-	if (pci_find_aer_capability(pdev))
-		if (pci_enable_pcie_error_reporting(pdev))
-			goto probe_out;
+	/* This may fail but that's ok */
+	pci_enable_pcie_error_reporting(pdev);
 
 	host = scsi_host_alloc(sht, sizeof(scsi_qla_host_t));
 	if (host == NULL) {
diff --git a/include/linux/aer.h b/include/linux/aer.h
index f251814..a2383a7 100644
--- a/include/linux/aer.h
+++ b/include/linux/aer.h
@@ -18,10 +18,6 @@ static inline int pci_enable_pcie_error_reporting(struct pci_dev *dev)
 {
 	return -EINVAL;
 }
-static inline int pci_find_aer_capability(struct pci_dev *dev)
-{
-	return 0;
-}
 static inline int pci_disable_pcie_error_reporting(struct pci_dev *dev)
 {
 	return -EINVAL;

[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