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@xxxxxxxxx> >>> --- >>> >>> 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++) > -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html