* [PATCH] correct pciehp init recovery
@ 2006-04-28 14:59 Jan Beulich
2006-04-28 17:38 ` [Pcihpd-discuss] " Kristen Accardi
0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2006-04-28 14:59 UTC (permalink / raw)
To: pcihpd-discuss; +Cc: linux-kernel
Clean up the recovery path from errors during pcie_init().
Signed-off-by: Jan Beulich <jbeulich@novell.com>
diff -Npru /home/jbeulich/tmp/linux-2.6.17-rc3/drivers/pci/hotplug/pciehp_hpc.c
2.6.17-rc3-pciehp-init-recovery/drivers/pci/hotplug/pciehp_hpc.c
--- /home/jbeulich/tmp/linux-2.6.17-rc3/drivers/pci/hotplug/pciehp_hpc.c 2006-04-27 17:49:41.000000000 +0200
+++ 2.6.17-rc3-pciehp-init-recovery/drivers/pci/hotplug/pciehp_hpc.c 2006-04-28 09:20:55.000000000 +0200
@@ -1474,7 +1474,7 @@ int pcie_init(struct controller * ctrl,
rc = hp_register_read_word(pdev, SLOT_CTRL(ctrl->cap_base), temp_word);
if (rc) {
err("%s : hp_register_read_word SLOT_CTRL failed\n", __FUNCTION__);
- goto abort_free_ctlr;
+ goto abort_free_irq;
}
intr_enable = intr_enable | PRSN_DETECT_ENABLE;
@@ -1500,19 +1500,19 @@ int pcie_init(struct controller * ctrl,
rc = hp_register_write_word(pdev, SLOT_CTRL(ctrl->cap_base), temp_word);
if (rc) {
err("%s : hp_register_write_word SLOT_CTRL failed\n", __FUNCTION__);
- goto abort_free_ctlr;
+ goto abort_free_irq;
}
rc = hp_register_read_word(php_ctlr->pci_dev, SLOT_STATUS(ctrl->cap_base), slot_status);
if (rc) {
err("%s : hp_register_read_word SLOT_STATUS failed\n", __FUNCTION__);
- goto abort_free_ctlr;
+ goto abort_disable_intr;
}
temp_word = 0x1F; /* Clear all events */
rc = hp_register_write_word(php_ctlr->pci_dev, SLOT_STATUS(ctrl->cap_base), temp_word);
if (rc) {
err("%s : hp_register_write_word SLOT_STATUS failed\n", __FUNCTION__);
- goto abort_free_ctlr;
+ goto abort_disable_intr;
}
if (pciehp_force) {
@@ -1521,7 +1521,7 @@ int pcie_init(struct controller * ctrl,
} else {
rc = pciehp_get_hp_hw_control_from_firmware(ctrl->pci_dev);
if (rc)
- goto abort_free_ctlr;
+ goto abort_disable_intr;
}
/* Add this HPC instance into the HPC list */
@@ -1548,6 +1548,18 @@ int pcie_init(struct controller * ctrl,
return 0;
/* We end up here for the many possible ways to fail this API. */
+abort_disable_intr:
+ rc = hp_register_read_word(pdev, SLOT_CTRL(ctrl->cap_base), temp_word);
+ if (!rc) {
+ temp_word &= ~(intr_enable | HP_INTR_ENABLE);
+ rc = hp_register_write_word(pdev, SLOT_CTRL(ctrl->cap_base), temp_word);
+ }
+ if (rc)
+ err("%s : disabling interrupts failed\n", __FUNCTION__);
+
+abort_free_irq:
+ free_irq(php_ctlr->irq, ctrl);
+
abort_free_ctlr:
pcie_cap_base = saved_cap_base;
kfree(php_ctlr);
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Pcihpd-discuss] [PATCH] correct pciehp init recovery
2006-04-28 14:59 [PATCH] correct pciehp init recovery Jan Beulich
@ 2006-04-28 17:38 ` Kristen Accardi
2006-05-02 10:32 ` Jan Beulich
0 siblings, 1 reply; 5+ messages in thread
From: Kristen Accardi @ 2006-04-28 17:38 UTC (permalink / raw)
To: Jan Beulich; +Cc: pcihpd-discuss, linux-kernel
On Fri, 2006-04-28 at 16:59 +0200, Jan Beulich wrote:
> Clean up the recovery path from errors during pcie_init().
>
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>
> diff -Npru /home/jbeulich/tmp/linux-2.6.17-rc3/drivers/pci/hotplug/pciehp_hpc.c
> 2.6.17-rc3-pciehp-init-recovery/drivers/pci/hotplug/pciehp_hpc.c
> --- /home/jbeulich/tmp/linux-2.6.17-rc3/drivers/pci/hotplug/pciehp_hpc.c 2006-04-27 17:49:41.000000000 +0200
> +++ 2.6.17-rc3-pciehp-init-recovery/drivers/pci/hotplug/pciehp_hpc.c 2006-04-28 09:20:55.000000000 +0200
> @@ -1474,7 +1474,7 @@ int pcie_init(struct controller * ctrl,
> rc = hp_register_read_word(pdev, SLOT_CTRL(ctrl->cap_base), temp_word);
> if (rc) {
> err("%s : hp_register_read_word SLOT_CTRL failed\n", __FUNCTION__);
> - goto abort_free_ctlr;
> + goto abort_free_irq;
> }
>
It's possible that this driver never actually requested an irq if was in
poll mode. Then you will call free_irq, when what you really want to do
is kill the timer that may have been started.
> intr_enable = intr_enable | PRSN_DETECT_ENABLE;
> @@ -1500,19 +1500,19 @@ int pcie_init(struct controller * ctrl,
> rc = hp_register_write_word(pdev, SLOT_CTRL(ctrl->cap_base), temp_word);
> if (rc) {
> err("%s : hp_register_write_word SLOT_CTRL failed\n", __FUNCTION__);
> - goto abort_free_ctlr;
> + goto abort_free_irq;
> }
> rc = hp_register_read_word(php_ctlr->pci_dev, SLOT_STATUS(ctrl->cap_base), slot_status);
> if (rc) {
> err("%s : hp_register_read_word SLOT_STATUS failed\n", __FUNCTION__);
> - goto abort_free_ctlr;
> + goto abort_disable_intr;
> }
>
> temp_word = 0x1F; /* Clear all events */
> rc = hp_register_write_word(php_ctlr->pci_dev, SLOT_STATUS(ctrl->cap_base), temp_word);
> if (rc) {
> err("%s : hp_register_write_word SLOT_STATUS failed\n", __FUNCTION__);
> - goto abort_free_ctlr;
> + goto abort_disable_intr;
> }
>
> if (pciehp_force) {
> @@ -1521,7 +1521,7 @@ int pcie_init(struct controller * ctrl,
> } else {
> rc = pciehp_get_hp_hw_control_from_firmware(ctrl->pci_dev);
> if (rc)
> - goto abort_free_ctlr;
> + goto abort_disable_intr;
> }
>
> /* Add this HPC instance into the HPC list */
> @@ -1548,6 +1548,18 @@ int pcie_init(struct controller * ctrl,
> return 0;
>
> /* We end up here for the many possible ways to fail this API. */
> +abort_disable_intr:
> + rc = hp_register_read_word(pdev, SLOT_CTRL(ctrl->cap_base), temp_word);
> + if (!rc) {
> + temp_word &= ~(intr_enable | HP_INTR_ENABLE);
> + rc = hp_register_write_word(pdev, SLOT_CTRL(ctrl->cap_base), temp_word);
> + }
> + if (rc)
> + err("%s : disabling interrupts failed\n", __FUNCTION__);
> +
> +abort_free_irq:
> + free_irq(php_ctlr->irq, ctrl);
> +
> abort_free_ctlr:
> pcie_cap_base = saved_cap_base;
> kfree(php_ctlr);
>
>
>
>
> -------------------------------------------------------
> Using Tomcat but need to do more? Need to support web services, security?
> Get stuff done quickly with pre-integrated technology to make your job easier
> Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
> _______________________________________________
> Pcihpd-discuss mailing list
> Pcihpd-discuss@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/pcihpd-discuss
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Pcihpd-discuss] [PATCH] correct pciehp init recovery
2006-04-28 17:38 ` [Pcihpd-discuss] " Kristen Accardi
@ 2006-05-02 10:32 ` Jan Beulich
2006-05-09 7:49 ` Andrew Morton
0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2006-05-02 10:32 UTC (permalink / raw)
To: Kristen Accardi; +Cc: pcihpd-discuss, linux-kernel
>>> Kristen Accardi <kristen.c.accardi@intel.com> 28.04.06 19:38 >>>
>On Fri, 2006-04-28 at 16:59 +0200, Jan Beulich wrote:
>> Clean up the recovery path from errors during pcie_init().
>>
>It's possible that this driver never actually requested an irq if was in
>poll mode. Then you will call free_irq, when what you really want to do
>is kill the timer that may have been started.
Thanks for pointing this out, here's the updated patch:
Clean up the recovery path from errors during pcie_init().
Signed-off-by: Jan Beulich <jbeulich@novell.com>
--- /home/jbeulich/tmp/linux-2.6.17-rc3/drivers/pci/hotplug/pciehp_hpc.c 2006-04-27 17:49:41.000000000 +0200
+++ 2.6.17-rc3-pciehp-init-recovery/drivers/pci/hotplug/pciehp_hpc.c 2006-04-28 09:20:55.000000000 +0200
@@ -1474,7 +1474,7 @@ int pcie_init(struct controller * ctrl,
rc = hp_register_read_word(pdev, SLOT_CTRL(ctrl->cap_base), temp_word);
if (rc) {
err("%s : hp_register_read_word SLOT_CTRL failed\n", __FUNCTION__);
- goto abort_free_ctlr;
+ goto abort_free_irq;
}
intr_enable = intr_enable | PRSN_DETECT_ENABLE;
@@ -1500,19 +1500,19 @@ int pcie_init(struct controller * ctrl,
rc = hp_register_write_word(pdev, SLOT_CTRL(ctrl->cap_base), temp_word);
if (rc) {
err("%s : hp_register_write_word SLOT_CTRL failed\n", __FUNCTION__);
- goto abort_free_ctlr;
+ goto abort_free_irq;
}
rc = hp_register_read_word(php_ctlr->pci_dev, SLOT_STATUS(ctrl->cap_base), slot_status);
if (rc) {
err("%s : hp_register_read_word SLOT_STATUS failed\n", __FUNCTION__);
- goto abort_free_ctlr;
+ goto abort_disable_intr;
}
temp_word = 0x1F; /* Clear all events */
rc = hp_register_write_word(php_ctlr->pci_dev, SLOT_STATUS(ctrl->cap_base), temp_word);
if (rc) {
err("%s : hp_register_write_word SLOT_STATUS failed\n", __FUNCTION__);
- goto abort_free_ctlr;
+ goto abort_disable_intr;
}
if (pciehp_force) {
@@ -1521,7 +1521,7 @@ int pcie_init(struct controller * ctrl,
} else {
rc = pciehp_get_hp_hw_control_from_firmware(ctrl->pci_dev);
if (rc)
- goto abort_free_ctlr;
+ goto abort_disable_intr;
}
/* Add this HPC instance into the HPC list */
@@ -1548,6 +1548,21 @@ int pcie_init(struct controller * ctrl,
return 0;
/* We end up here for the many possible ways to fail this API. */
+abort_disable_intr:
+ rc = hp_register_read_word(pdev, SLOT_CTRL(ctrl->cap_base), temp_word);
+ if (!rc) {
+ temp_word &= ~(intr_enable | HP_INTR_ENABLE);
+ rc = hp_register_write_word(pdev, SLOT_CTRL(ctrl->cap_base), temp_word);
+ }
+ if (rc)
+ err("%s : disabling interrupts failed\n", __FUNCTION__);
+
+abort_free_irq:
+ if (pciehp_poll_mode)
+ del_timer_sync(&php_ctlr->int_poll_timer);
+ else
+ free_irq(php_ctlr->irq, ctrl);
+
abort_free_ctlr:
pcie_cap_base = saved_cap_base;
kfree(php_ctlr);
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Pcihpd-discuss] [PATCH] correct pciehp init recovery
2006-05-02 10:32 ` Jan Beulich
@ 2006-05-09 7:49 ` Andrew Morton
2006-05-09 8:29 ` Jan Beulich
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2006-05-09 7:49 UTC (permalink / raw)
To: Jan Beulich; +Cc: kristen.c.accardi, pcihpd-discuss, linux-kernel
"Jan Beulich" <jbeulich@novell.com> wrote:
>
> >>> Kristen Accardi <kristen.c.accardi@intel.com> 28.04.06 19:38 >>>
> >On Fri, 2006-04-28 at 16:59 +0200, Jan Beulich wrote:
> >> Clean up the recovery path from errors during pcie_init().
> >>
> >It's possible that this driver never actually requested an irq if was in
> >poll mode. Then you will call free_irq, when what you really want to do
> >is kill the timer that may have been started.
>
> Thanks for pointing this out, here's the updated patch:
>
> Clean up the recovery path from errors during pcie_init().
>
Well, it does more than clean things up. It fixes bugs.
Could we have a more accurate and complete changelog please?
>
> --- /home/jbeulich/tmp/linux-2.6.17-rc3/drivers/pci/hotplug/pciehp_hpc.c 2006-04-27 17:49:41.000000000 +0200
> +++ 2.6.17-rc3-pciehp-init-recovery/drivers/pci/hotplug/pciehp_hpc.c 2006-04-28 09:20:55.000000000 +0200
> @@ -1474,7 +1474,7 @@ int pcie_init(struct controller * ctrl,
> rc = hp_register_read_word(pdev, SLOT_CTRL(ctrl->cap_base), temp_word);
> if (rc) {
> err("%s : hp_register_read_word SLOT_CTRL failed\n", __FUNCTION__);
> - goto abort_free_ctlr;
> + goto abort_free_irq;
> }
>
> intr_enable = intr_enable | PRSN_DETECT_ENABLE;
> @@ -1500,19 +1500,19 @@ int pcie_init(struct controller * ctrl,
> rc = hp_register_write_word(pdev, SLOT_CTRL(ctrl->cap_base), temp_word);
> if (rc) {
> err("%s : hp_register_write_word SLOT_CTRL failed\n", __FUNCTION__);
> - goto abort_free_ctlr;
> + goto abort_free_irq;
> }
> rc = hp_register_read_word(php_ctlr->pci_dev, SLOT_STATUS(ctrl->cap_base), slot_status);
> if (rc) {
> err("%s : hp_register_read_word SLOT_STATUS failed\n", __FUNCTION__);
> - goto abort_free_ctlr;
> + goto abort_disable_intr;
> }
>
> temp_word = 0x1F; /* Clear all events */
> rc = hp_register_write_word(php_ctlr->pci_dev, SLOT_STATUS(ctrl->cap_base), temp_word);
> if (rc) {
> err("%s : hp_register_write_word SLOT_STATUS failed\n", __FUNCTION__);
> - goto abort_free_ctlr;
> + goto abort_disable_intr;
> }
>
> if (pciehp_force) {
> @@ -1521,7 +1521,7 @@ int pcie_init(struct controller * ctrl,
> } else {
> rc = pciehp_get_hp_hw_control_from_firmware(ctrl->pci_dev);
> if (rc)
> - goto abort_free_ctlr;
> + goto abort_disable_intr;
> }
>
> /* Add this HPC instance into the HPC list */
> @@ -1548,6 +1548,21 @@ int pcie_init(struct controller * ctrl,
> return 0;
>
> /* We end up here for the many possible ways to fail this API. */
> +abort_disable_intr:
> + rc = hp_register_read_word(pdev, SLOT_CTRL(ctrl->cap_base), temp_word);
> + if (!rc) {
> + temp_word &= ~(intr_enable | HP_INTR_ENABLE);
> + rc = hp_register_write_word(pdev, SLOT_CTRL(ctrl->cap_base), temp_word);
> + }
> + if (rc)
> + err("%s : disabling interrupts failed\n", __FUNCTION__);
> +
> +abort_free_irq:
> + if (pciehp_poll_mode)
> + del_timer_sync(&php_ctlr->int_poll_timer);
> + else
> + free_irq(php_ctlr->irq, ctrl);
> +
> abort_free_ctlr:
> pcie_cap_base = saved_cap_base;
> kfree(php_ctlr);
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Pcihpd-discuss] [PATCH] correct pciehp init recovery
2006-05-09 7:49 ` Andrew Morton
@ 2006-05-09 8:29 ` Jan Beulich
0 siblings, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2006-05-09 8:29 UTC (permalink / raw)
To: Andrew Morton; +Cc: kristen.c.accardi, pcihpd-discuss, linux-kernel
>>> Andrew Morton <akpm@osdl.org> 09.05.06 09:49 >>>
>"Jan Beulich" <jbeulich@novell.com> wrote:
>>
>> >>> Kristen Accardi <kristen.c.accardi@intel.com> 28.04.06 19:38 >>>
>> >On Fri, 2006-04-28 at 16:59 +0200, Jan Beulich wrote:
>> >> Clean up the recovery path from errors during pcie_init().
>> >>
>> >It's possible that this driver never actually requested an irq if was in
>> >poll mode. Then you will call free_irq, when what you really want to do
>> >is kill the timer that may have been started.
>>
>> Thanks for pointing this out, here's the updated patch:
>>
>> Clean up the recovery path from errors during pcie_init().
>>
>
>Well, it does more than clean things up. It fixes bugs.
>
>Could we have a more accurate and complete changelog please?
Clean up (fix) the recovery path from errors during pcie_init() by properly
undoing any initialization steps already carried out.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2006-05-09 8:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-04-28 14:59 [PATCH] correct pciehp init recovery Jan Beulich
2006-04-28 17:38 ` [Pcihpd-discuss] " Kristen Accardi
2006-05-02 10:32 ` Jan Beulich
2006-05-09 7:49 ` Andrew Morton
2006-05-09 8:29 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox