Re: [PATCH] ahci: unregister acpi notify handler when a ZPODD is unbound

[Levente Kurusa]

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?

> 
> 
> 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: signature.asc
Description: OpenPGP digital signature