* [PATCH] ahci: unregister acpi notify handler when a ZPODD is unbound
@ 2014-04-30 16:04 Levente Kurusa
2014-04-30 18:22 ` Bartlomiej Zolnierkiewicz
2014-05-06 3:16 ` Aaron Lu
0 siblings, 2 replies; 7+ messages in thread
From: Levente Kurusa @ 2014-04-30 16:04 UTC (permalink / raw)
To: Tejun Heo; +Cc: LKML, Linux ATA/IDE, Joe Thomas, Levente Kurusa
When a ZPODD device is unbound via sysfs, the acpi notify handler
is not removed. This causes panics as observed in Bug #74601. The
panic only happens when the wake happens from outside the kernel
(i.e. inserting media or pressing a button). Implement a new
ahci_remove_one function which causes zpodd_exit to be called for all
ZPODD devices on the unbound PCI device.
Signed-off-by: Levente Kurusa <levex@linux.com>
---
Hi,
I am not sure if the loop below is correct. Maybe there is a better
solution to loop through all the devices which might use ZPODD?
Thanks, Lev.
drivers/ata/ahci.c | 21 +++++++++++++++++++++
drivers/ata/ahci.h | 4 ++++
drivers/ata/libata-zpodd.c | 1 +
3 files changed, 26 insertions(+)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 5a0bf8e..6d92bc9 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -475,12 +475,33 @@ static const struct pci_device_id ahci_pci_tbl[] = {
{ } /* terminate list */
};
+#ifdef CONFIG_SATA_ZPODD
+void ahci_remove_one(struct pci_dev *pdev)
+{
+ struct ata_host *host = pci_get_drvdata(pdev);
+ struct ata_link *link;
+ struct ata_device *dev;
+ int i;
+
+ for (i = 0; i < host->n_ports; i++)
+ ata_for_each_link(link, host->ports[i], HOST_FIRST)
+ ata_for_each_dev(dev, link, ALL)
+ if (dev->zpodd)
+ zpodd_exit(dev);
+
+ ata_pci_remove_one(pdev);
+}
+#endif
static struct pci_driver ahci_pci_driver = {
.name = DRV_NAME,
.id_table = ahci_pci_tbl,
.probe = ahci_init_one,
+#ifdef CONFIG_SATA_ZPODD
+ .remove = ahci_remove_one,
+#else
.remove = ata_pci_remove_one,
+#endif
#ifdef CONFIG_PM
.suspend = ahci_pci_device_suspend,
.resume = ahci_pci_device_resume,
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 51af275..87e4e6d 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -383,6 +383,10 @@ void ahci_print_info(struct ata_host *host, const char *scc_s);
int ahci_host_activate(struct ata_host *host, int irq, unsigned int n_msis);
void ahci_error_handler(struct ata_port *ap);
+#ifdef CONFIG_SATA_ZPODD
+extern void zpodd_exit(struct ata_device *dev);
+#endif /* CONFIG_SATA_ZPODD */
+
static inline void __iomem *__ahci_port_base(struct ata_host *host,
unsigned int port_no)
{
diff --git a/drivers/ata/libata-zpodd.c b/drivers/ata/libata-zpodd.c
index f3a65a3..fe66949 100644
--- a/drivers/ata/libata-zpodd.c
+++ b/drivers/ata/libata-zpodd.c
@@ -281,3 +281,4 @@ void zpodd_exit(struct ata_device *dev)
kfree(dev->zpodd);
dev->zpodd = NULL;
}
+EXPORT_SYMBOL_GPL(zpodd_exit);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] ahci: unregister acpi notify handler when a ZPODD is unbound
2014-04-30 16:04 [PATCH] ahci: unregister acpi notify handler when a ZPODD is unbound Levente Kurusa
@ 2014-04-30 18:22 ` Bartlomiej Zolnierkiewicz
2014-05-06 3:16 ` Aaron Lu
1 sibling, 0 replies; 7+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-04-30 18:22 UTC (permalink / raw)
To: Levente Kurusa; +Cc: Tejun Heo, LKML, Linux ATA/IDE, Joe Thomas
Hi,
On Wednesday, April 30, 2014 06:04:47 PM Levente Kurusa wrote:
> When a ZPODD device is unbound via sysfs, the acpi notify handler
> is not removed. This causes panics as observed in Bug #74601. The
> panic only happens when the wake happens from outside the kernel
> (i.e. inserting media or pressing a button). Implement a new
> ahci_remove_one function which causes zpodd_exit to be called for all
> ZPODD devices on the unbound PCI device.
>
> Signed-off-by: Levente Kurusa <levex@linux.com>
> ---
>
> Hi,
>
> I am not sure if the loop below is correct. Maybe there is a better
> solution to loop through all the devices which might use ZPODD?
>
> Thanks, Lev.
>
> drivers/ata/ahci.c | 21 +++++++++++++++++++++
> drivers/ata/ahci.h | 4 ++++
> drivers/ata/libata-zpodd.c | 1 +
> 3 files changed, 26 insertions(+)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 5a0bf8e..6d92bc9 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -475,12 +475,33 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { } /* terminate list */
> };
>
> +#ifdef CONFIG_SATA_ZPODD
> +void ahci_remove_one(struct pci_dev *pdev)
> +{
> + struct ata_host *host = pci_get_drvdata(pdev);
> + struct ata_link *link;
> + struct ata_device *dev;
> + int i;
> +
> + for (i = 0; i < host->n_ports; i++)
> + ata_for_each_link(link, host->ports[i], HOST_FIRST)
> + ata_for_each_dev(dev, link, ALL)
> + if (dev->zpodd)
> + zpodd_exit(dev);
zpodd_init() is used in generic libata library code, same should be true
for zpodd_exit(). Ideally, you should find some libata library function
called from ata_pci_remove_one() suitable for this purpose.
Hmm, actually zpodd_exit() is already used in ata_scsi_handle_link_detach()
so this should also be taken into consideration..
> + ata_pci_remove_one(pdev);
> +}
> +#endif
>
> static struct pci_driver ahci_pci_driver = {
> .name = DRV_NAME,
> .id_table = ahci_pci_tbl,
> .probe = ahci_init_one,
> +#ifdef CONFIG_SATA_ZPODD
> + .remove = ahci_remove_one,
> +#else
> .remove = ata_pci_remove_one,
> +#endif
> #ifdef CONFIG_PM
> .suspend = ahci_pci_device_suspend,
> .resume = ahci_pci_device_resume,
> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> index 51af275..87e4e6d 100644
> --- a/drivers/ata/ahci.h
> +++ b/drivers/ata/ahci.h
> @@ -383,6 +383,10 @@ void ahci_print_info(struct ata_host *host, const char *scc_s);
> int ahci_host_activate(struct ata_host *host, int irq, unsigned int n_msis);
> void ahci_error_handler(struct ata_port *ap);
>
> +#ifdef CONFIG_SATA_ZPODD
> +extern void zpodd_exit(struct ata_device *dev);
> +#endif /* CONFIG_SATA_ZPODD */
> +
> static inline void __iomem *__ahci_port_base(struct ata_host *host,
> unsigned int port_no)
> {
> diff --git a/drivers/ata/libata-zpodd.c b/drivers/ata/libata-zpodd.c
> index f3a65a3..fe66949 100644
> --- a/drivers/ata/libata-zpodd.c
> +++ b/drivers/ata/libata-zpodd.c
> @@ -281,3 +281,4 @@ void zpodd_exit(struct ata_device *dev)
> kfree(dev->zpodd);
> dev->zpodd = NULL;
> }
> +EXPORT_SYMBOL_GPL(zpodd_exit);
Then you would also not need to export zpodd_exit()..
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ahci: unregister acpi notify handler when a ZPODD is unbound
2014-04-30 16:04 [PATCH] ahci: unregister acpi notify handler when a ZPODD is unbound Levente Kurusa
2014-04-30 18:22 ` Bartlomiej Zolnierkiewicz
@ 2014-05-06 3:16 ` Aaron Lu
2014-05-06 6:02 ` Levente Kurusa
1 sibling, 1 reply; 7+ messages in thread
From: Aaron Lu @ 2014-05-06 3:16 UTC (permalink / raw)
To: Levente Kurusa, Tejun Heo; +Cc: LKML, Linux ATA/IDE, Joe Thomas
On 05/01/2014 12:04 AM, Levente Kurusa wrote:
> When a ZPODD device is unbound via sysfs, the acpi notify handler
> is not removed. This causes panics as observed in Bug #74601. The
Ah...too bad, I forgot to consider this situation, thanks for tracking
this.
> panic only happens when the wake happens from outside the kernel
> (i.e. inserting media or pressing a button). Implement a new
> ahci_remove_one function which causes zpodd_exit to be called for all
> ZPODD devices on the unbound PCI device.
>
> Signed-off-by: Levente Kurusa <levex@linux.com>
> ---
>
> Hi,
>
> I am not sure if the loop below is correct. Maybe there is a better
> solution to loop through all the devices which might use ZPODD?
I didn't find a proper place either. For hotplug, we did the zpodd_exit
at ata_scsi_handle_link_detach. But for host controller pci device
removal, we used scsi_remove_host in ata_port_detach and there is no
place to add the zpodd_exit for a to-be-removed scsi device...
Looks like we can only iterate the ata devices and call zpodd_exit
explicitly for them if they are zpodd devices. Instead of adding a new
remove callback, what about just embed that into the ata_port_detach
like the following example?
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 943cc8b83e59..43652da6fea6 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6314,6 +6314,8 @@ int ata_host_activate(struct ata_host *host, int irq,
static void ata_port_detach(struct ata_port *ap)
{
unsigned long flags;
+ struct ata_link *link;
+ struct ata_device *dev;
if (!ap->ops->error_handler)
goto skip_eh;
@@ -6333,6 +6335,13 @@ static void ata_port_detach(struct ata_port *ap)
cancel_delayed_work_sync(&ap->hotplug_task);
skip_eh:
+ /* clean up zpodd related stuffs on port removal */
+ ata_for_each_link(link, ap, HOST_FIRST) {
+ ata_for_each_dev(dev, link, ALL) {
+ if (zpodd_dev_enabled(dev))
+ zpodd_exit(dev);
+ }
+ }
if (ap->pmp_link) {
int i;
for (i = 0; i < SATA_PMP_MAX_PORTS; i++)
Thanks,
Aaron
>
> Thanks, Lev.
>
> drivers/ata/ahci.c | 21 +++++++++++++++++++++
> drivers/ata/ahci.h | 4 ++++
> drivers/ata/libata-zpodd.c | 1 +
> 3 files changed, 26 insertions(+)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 5a0bf8e..6d92bc9 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -475,12 +475,33 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { } /* terminate list */
> };
>
> +#ifdef CONFIG_SATA_ZPODD
> +void ahci_remove_one(struct pci_dev *pdev)
> +{
> + struct ata_host *host = pci_get_drvdata(pdev);
> + struct ata_link *link;
> + struct ata_device *dev;
> + int i;
> +
> + for (i = 0; i < host->n_ports; i++)
> + ata_for_each_link(link, host->ports[i], HOST_FIRST)
> + ata_for_each_dev(dev, link, ALL)
> + if (dev->zpodd)
> + zpodd_exit(dev);
> +
> + ata_pci_remove_one(pdev);
> +}
> +#endif
>
> static struct pci_driver ahci_pci_driver = {
> .name = DRV_NAME,
> .id_table = ahci_pci_tbl,
> .probe = ahci_init_one,
> +#ifdef CONFIG_SATA_ZPODD
> + .remove = ahci_remove_one,
> +#else
> .remove = ata_pci_remove_one,
> +#endif
> #ifdef CONFIG_PM
> .suspend = ahci_pci_device_suspend,
> .resume = ahci_pci_device_resume,
> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> index 51af275..87e4e6d 100644
> --- a/drivers/ata/ahci.h
> +++ b/drivers/ata/ahci.h
> @@ -383,6 +383,10 @@ void ahci_print_info(struct ata_host *host, const char *scc_s);
> int ahci_host_activate(struct ata_host *host, int irq, unsigned int n_msis);
> void ahci_error_handler(struct ata_port *ap);
>
> +#ifdef CONFIG_SATA_ZPODD
> +extern void zpodd_exit(struct ata_device *dev);
> +#endif /* CONFIG_SATA_ZPODD */
> +
> static inline void __iomem *__ahci_port_base(struct ata_host *host,
> unsigned int port_no)
> {
> diff --git a/drivers/ata/libata-zpodd.c b/drivers/ata/libata-zpodd.c
> index f3a65a3..fe66949 100644
> --- a/drivers/ata/libata-zpodd.c
> +++ b/drivers/ata/libata-zpodd.c
> @@ -281,3 +281,4 @@ void zpodd_exit(struct ata_device *dev)
> kfree(dev->zpodd);
> dev->zpodd = NULL;
> }
> +EXPORT_SYMBOL_GPL(zpodd_exit);
>
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] ahci: unregister acpi notify handler when a ZPODD is unbound
2014-05-06 3:16 ` Aaron Lu
@ 2014-05-06 6:02 ` Levente Kurusa
2014-05-06 6:07 ` Aaron Lu
0 siblings, 1 reply; 7+ messages in thread
From: Levente Kurusa @ 2014-05-06 6:02 UTC (permalink / raw)
To: Aaron Lu, Tejun Heo; +Cc: LKML, Linux ATA/IDE, Joe Thomas
[-- Attachment #1: Type: text/plain, Size: 2528 bytes --]
Hi,
On 05/06/2014 05:16 AM, Aaron Lu wrote:
> On 05/01/2014 12:04 AM, Levente Kurusa wrote:
>> When a ZPODD device is unbound via sysfs, the acpi notify handler
>> is not removed. This causes panics as observed in Bug #74601. The
>
> Ah...too bad, I forgot to consider this situation, thanks for tracking
> this.
>
>> panic only happens when the wake happens from outside the kernel
>> (i.e. inserting media or pressing a button). Implement a new
>> ahci_remove_one function which causes zpodd_exit to be called for all
>> ZPODD devices on the unbound PCI device.
>>
>> Signed-off-by: Levente Kurusa <levex@linux.com>
>> ---
>>
>> Hi,
>>
>> I am not sure if the loop below is correct. Maybe there is a better
>> solution to loop through all the devices which might use ZPODD?
>
> I didn't find a proper place either. For hotplug, we did the zpodd_exit
> at ata_scsi_handle_link_detach. But for host controller pci device
> removal, we used scsi_remove_host in ata_port_detach and there is no
> place to add the zpodd_exit for a to-be-removed scsi device...
>
> Looks like we can only iterate the ata devices and call zpodd_exit
> explicitly for them if they are zpodd devices. Instead of adding a new
> remove callback, what about just embed that into the ata_port_detach
> like the following example?
Yes, this makes more sense as this doesn't tinker with exports and such...
However this will throw unused variable compiler warnings if we add the
required #ifdefs... Maybe a new function? ata_zpodd_detach_port?
>
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 943cc8b83e59..43652da6fea6 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -6314,6 +6314,8 @@ int ata_host_activate(struct ata_host *host, int irq,
> static void ata_port_detach(struct ata_port *ap)
> {
> unsigned long flags;
> + struct ata_link *link;
> + struct ata_device *dev;
>
> if (!ap->ops->error_handler)
> goto skip_eh;
> @@ -6333,6 +6335,13 @@ static void ata_port_detach(struct ata_port *ap)
> cancel_delayed_work_sync(&ap->hotplug_task);
>
> skip_eh:
> + /* clean up zpodd related stuffs on port removal */
> + ata_for_each_link(link, ap, HOST_FIRST) {
> + ata_for_each_dev(dev, link, ALL) {
> + if (zpodd_dev_enabled(dev))
> + zpodd_exit(dev);
> + }
> + }
> if (ap->pmp_link) {
> int i;
> for (i = 0; i < SATA_PMP_MAX_PORTS; i++)
--
Regards,
Levente Kurusa
PGP: 4EF5D641
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 555 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ahci: unregister acpi notify handler when a ZPODD is unbound
2014-05-06 6:02 ` Levente Kurusa
@ 2014-05-06 6:07 ` Aaron Lu
2014-05-06 7:14 ` Levente Kurusa
0 siblings, 1 reply; 7+ messages in thread
From: Aaron Lu @ 2014-05-06 6:07 UTC (permalink / raw)
To: Levente Kurusa, Tejun Heo; +Cc: LKML, Linux ATA/IDE, Joe Thomas
On 05/06/2014 02:02 PM, Levente Kurusa wrote:
> Hi,
>
> On 05/06/2014 05:16 AM, Aaron Lu wrote:
>> On 05/01/2014 12:04 AM, Levente Kurusa wrote:
>>> When a ZPODD device is unbound via sysfs, the acpi notify handler
>>> is not removed. This causes panics as observed in Bug #74601. The
>>
>> Ah...too bad, I forgot to consider this situation, thanks for tracking
>> this.
>>
>>> panic only happens when the wake happens from outside the kernel
>>> (i.e. inserting media or pressing a button). Implement a new
>>> ahci_remove_one function which causes zpodd_exit to be called for all
>>> ZPODD devices on the unbound PCI device.
>>>
>>> Signed-off-by: Levente Kurusa <levex@linux.com>
>>> ---
>>>
>>> Hi,
>>>
>>> I am not sure if the loop below is correct. Maybe there is a better
>>> solution to loop through all the devices which might use ZPODD?
>>
>> I didn't find a proper place either. For hotplug, we did the zpodd_exit
>> at ata_scsi_handle_link_detach. But for host controller pci device
>> removal, we used scsi_remove_host in ata_port_detach and there is no
>> place to add the zpodd_exit for a to-be-removed scsi device...
>>
>> Looks like we can only iterate the ata devices and call zpodd_exit
>> explicitly for them if they are zpodd devices. Instead of adding a new
>> remove callback, what about just embed that into the ata_port_detach
>> like the following example?
>
> Yes, this makes more sense as this doesn't tinker with exports and such...
> However this will throw unused variable compiler warnings if we add the
> required #ifdefs... Maybe a new function? ata_zpodd_detach_port?
I think we can omit the #ifdefs as the loop is not called frequently and
thus doesn't cost much. We already have stubs for zpodd_dev_enabled and
zpodd_exit.
Thanks,
Aaron
>
>>
>>
>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>> index 943cc8b83e59..43652da6fea6 100644
>> --- a/drivers/ata/libata-core.c
>> +++ b/drivers/ata/libata-core.c
>> @@ -6314,6 +6314,8 @@ int ata_host_activate(struct ata_host *host, int irq,
>> static void ata_port_detach(struct ata_port *ap)
>> {
>> unsigned long flags;
>> + struct ata_link *link;
>> + struct ata_device *dev;
>>
>> if (!ap->ops->error_handler)
>> goto skip_eh;
>> @@ -6333,6 +6335,13 @@ static void ata_port_detach(struct ata_port *ap)
>> cancel_delayed_work_sync(&ap->hotplug_task);
>>
>> skip_eh:
>> + /* clean up zpodd related stuffs on port removal */
>> + ata_for_each_link(link, ap, HOST_FIRST) {
>> + ata_for_each_dev(dev, link, ALL) {
>> + if (zpodd_dev_enabled(dev))
>> + zpodd_exit(dev);
>> + }
>> + }
>> if (ap->pmp_link) {
>> int i;
>> for (i = 0; i < SATA_PMP_MAX_PORTS; i++)
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ahci: unregister acpi notify handler when a ZPODD is unbound
2014-05-06 6:07 ` Aaron Lu
@ 2014-05-06 7:14 ` Levente Kurusa
2014-05-06 7:32 ` Aaron Lu
0 siblings, 1 reply; 7+ messages in thread
From: Levente Kurusa @ 2014-05-06 7:14 UTC (permalink / raw)
To: Aaron Lu, Tejun Heo; +Cc: LKML, Linux ATA/IDE, Joe Thomas
[-- Attachment #1: Type: text/plain, Size: 2012 bytes --]
Hi,
On 05/06/2014 08:07 AM, Aaron Lu wrote:
> On 05/06/2014 02:02 PM, Levente Kurusa wrote:
>> Hi,
>>
>> On 05/06/2014 05:16 AM, Aaron Lu wrote:
>>> On 05/01/2014 12:04 AM, Levente Kurusa wrote:
>>>> When a ZPODD device is unbound via sysfs, the acpi notify handler
>>>> is not removed. This causes panics as observed in Bug #74601. The
>>>
>>> Ah...too bad, I forgot to consider this situation, thanks for tracking
>>> this.
>>>
>>>> panic only happens when the wake happens from outside the kernel
>>>> (i.e. inserting media or pressing a button). Implement a new
>>>> ahci_remove_one function which causes zpodd_exit to be called for all
>>>> ZPODD devices on the unbound PCI device.
>>>>
>>>> Signed-off-by: Levente Kurusa <levex@linux.com>
>>>> ---
>>>>
>>>> Hi,
>>>>
>>>> I am not sure if the loop below is correct. Maybe there is a better
>>>> solution to loop through all the devices which might use ZPODD?
>>>
>>> I didn't find a proper place either. For hotplug, we did the zpodd_exit
>>> at ata_scsi_handle_link_detach. But for host controller pci device
>>> removal, we used scsi_remove_host in ata_port_detach and there is no
>>> place to add the zpodd_exit for a to-be-removed scsi device...
>>>
>>> Looks like we can only iterate the ata devices and call zpodd_exit
>>> explicitly for them if they are zpodd devices. Instead of adding a new
>>> remove callback, what about just embed that into the ata_port_detach
>>> like the following example?
>>
>> Yes, this makes more sense as this doesn't tinker with exports and such...
>> However this will throw unused variable compiler warnings if we add the
>> required #ifdefs... Maybe a new function? ata_zpodd_detach_port?
>
> I think we can omit the #ifdefs as the loop is not called frequently and
> thus doesn't cost much. We already have stubs for zpodd_dev_enabled and
> zpodd_exit.
>
Ah, I see. Shall I send V2? Any tags I should add for you?
--
Regards,
Levente Kurusa
PGP: 4EF5D641
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 555 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ahci: unregister acpi notify handler when a ZPODD is unbound
2014-05-06 7:14 ` Levente Kurusa
@ 2014-05-06 7:32 ` Aaron Lu
0 siblings, 0 replies; 7+ messages in thread
From: Aaron Lu @ 2014-05-06 7:32 UTC (permalink / raw)
To: Levente Kurusa, Tejun Heo; +Cc: LKML, Linux ATA/IDE, Joe Thomas
On 05/06/2014 03:14 PM, Levente Kurusa wrote:
> Hi,
>
> On 05/06/2014 08:07 AM, Aaron Lu wrote:
>> On 05/06/2014 02:02 PM, Levente Kurusa wrote:
>>> Hi,
>>>
>>> On 05/06/2014 05:16 AM, Aaron Lu wrote:
>>>> On 05/01/2014 12:04 AM, Levente Kurusa wrote:
>>>>> When a ZPODD device is unbound via sysfs, the acpi notify handler
>>>>> is not removed. This causes panics as observed in Bug #74601. The
>>>>
>>>> Ah...too bad, I forgot to consider this situation, thanks for tracking
>>>> this.
>>>>
>>>>> panic only happens when the wake happens from outside the kernel
>>>>> (i.e. inserting media or pressing a button). Implement a new
>>>>> ahci_remove_one function which causes zpodd_exit to be called for all
>>>>> ZPODD devices on the unbound PCI device.
>>>>>
>>>>> Signed-off-by: Levente Kurusa <levex@linux.com>
>>>>> ---
>>>>>
>>>>> Hi,
>>>>>
>>>>> I am not sure if the loop below is correct. Maybe there is a better
>>>>> solution to loop through all the devices which might use ZPODD?
>>>>
>>>> I didn't find a proper place either. For hotplug, we did the zpodd_exit
>>>> at ata_scsi_handle_link_detach. But for host controller pci device
>>>> removal, we used scsi_remove_host in ata_port_detach and there is no
>>>> place to add the zpodd_exit for a to-be-removed scsi device...
>>>>
>>>> Looks like we can only iterate the ata devices and call zpodd_exit
>>>> explicitly for them if they are zpodd devices. Instead of adding a new
>>>> remove callback, what about just embed that into the ata_port_detach
>>>> like the following example?
>>>
>>> Yes, this makes more sense as this doesn't tinker with exports and such...
>>> However this will throw unused variable compiler warnings if we add the
>>> required #ifdefs... Maybe a new function? ata_zpodd_detach_port?
>>
>> I think we can omit the #ifdefs as the loop is not called frequently and
>> thus doesn't cost much. We already have stubs for zpodd_dev_enabled and
>> zpodd_exit.
>>
>
> Ah, I see. Shall I send V2? Any tags I should add for you?
Yes please. Also, this seems to be stable candidate.
You can add my reviewed-by tag, thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-05-06 7:32 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-30 16:04 [PATCH] ahci: unregister acpi notify handler when a ZPODD is unbound Levente Kurusa
2014-04-30 18:22 ` Bartlomiej Zolnierkiewicz
2014-05-06 3:16 ` Aaron Lu
2014-05-06 6:02 ` Levente Kurusa
2014-05-06 6:07 ` Aaron Lu
2014-05-06 7:14 ` Levente Kurusa
2014-05-06 7:32 ` Aaron Lu
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.