lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 19 Feb 2014 17:40:41 +0100
From:	"Rafael J. Wysocki" <rjw@...ysocki.net>
To:	Aaron Lu <aaron.lu@...el.com>
Cc:	ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
	Linux PCI <linux-pci@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>, Tejun Heo <tj@...nel.org>
Subject: Re: [PATCH 6/9] ACPI / ATA: Add hotplug contexts to ACPI companions of SATA devices

On Wednesday, February 19, 2014 10:42:20 AM Aaron Lu wrote:
> On 02/19/2014 09:30 AM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > 
> > Modify the SATA subsystem to add hotplug contexts to ACPI companions
> > of SATA devices and ports instead of registering special ACPI dock
> > operations using register_hotplug_dock_device().
> > 
> > That change will allow the entire code handling those special ACPI
> > dock operations to be dropped in the next commit.
> > 
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > ---
> >  drivers/ata/libata-acpi.c |   76 +++++++++++++++++++++++++++++-----------------
> >  1 file changed, 49 insertions(+), 27 deletions(-)
> > 
> > Index: linux-pm/drivers/ata/libata-acpi.c
> > ===================================================================
> > --- linux-pm.orig/drivers/ata/libata-acpi.c
> > +++ linux-pm/drivers/ata/libata-acpi.c
> > @@ -38,6 +38,16 @@ static void ata_acpi_clear_gtf(struct at
> >  	dev->gtf_cache = NULL;
> >  }
> >  
> > +struct ata_acpi_hotplug_context {
> > +	struct acpi_hotplug_context hp;
> > +	union {
> > +		struct ata_port *ap;
> > +		struct ata_device *dev;
> > +	} data;
> > +};
> > +
> > +#define ata_hotplug_data(context) (container_of((context), struct ata_acpi_hotplug_context, hp)->data)
> > +
> >  /**
> >   * ata_dev_acpi_handle - provide the acpi_handle for an ata_device
> >   * @dev: the acpi_handle returned will correspond to this device
> > @@ -121,18 +131,17 @@ static void ata_acpi_handle_hotplug(stru
> >  		ata_port_wait_eh(ap);
> >  }
> >  
> > -static void ata_acpi_dev_notify_dock(acpi_handle handle, u32 event, void *data)
> > +static int ata_acpi_dev_notify_dock(struct acpi_device *adev, u32 event)
> >  {
> > -	struct ata_device *dev = data;
> > -
> > +	struct ata_device *dev = ata_hotplug_data(adev->hp).dev;
> >  	ata_acpi_handle_hotplug(dev->link->ap, dev, event);
> > +	return 0;
> >  }
> >  
> > -static void ata_acpi_ap_notify_dock(acpi_handle handle, u32 event, void *data)
> > +static int ata_acpi_ap_notify_dock(struct acpi_device *adev, u32 event)
> >  {
> > -	struct ata_port *ap = data;
> > -
> > -	ata_acpi_handle_hotplug(ap, NULL, event);
> > +	ata_acpi_handle_hotplug(ata_hotplug_data(adev->hp).ap, NULL, event);
> > +	return 0;
> >  }
> >  
> >  static void ata_acpi_uevent(struct ata_port *ap, struct ata_device *dev,
> > @@ -154,31 +163,23 @@ static void ata_acpi_uevent(struct ata_p
> >  	}
> >  }
> >  
> > -static void ata_acpi_ap_uevent(acpi_handle handle, u32 event, void *data)
> > +static void ata_acpi_ap_uevent(struct acpi_device *adev, u32 event)
> >  {
> > -	ata_acpi_uevent(data, NULL, event);
> > +	ata_acpi_uevent(ata_hotplug_data(adev->hp).ap, NULL, event);
> >  }
> >  
> > -static void ata_acpi_dev_uevent(acpi_handle handle, u32 event, void *data)
> > +static void ata_acpi_dev_uevent(struct acpi_device *adev, u32 event)
> >  {
> > -	struct ata_device *dev = data;
> > +	struct ata_device *dev = ata_hotplug_data(adev->hp).dev;
> >  	ata_acpi_uevent(dev->link->ap, dev, event);
> >  }
> >  
> > -static const struct acpi_dock_ops ata_acpi_dev_dock_ops = {
> > -	.handler = ata_acpi_dev_notify_dock,
> > -	.uevent = ata_acpi_dev_uevent,
> > -};
> > -
> > -static const struct acpi_dock_ops ata_acpi_ap_dock_ops = {
> > -	.handler = ata_acpi_ap_notify_dock,
> > -	.uevent = ata_acpi_ap_uevent,
> > -};
> > -
> >  /* bind acpi handle to pata port */
> >  void ata_acpi_bind_port(struct ata_port *ap)
> >  {
> >  	struct acpi_device *host_companion = ACPI_COMPANION(ap->host->dev);
> > +	struct acpi_device *adev;
> > +	struct ata_acpi_hotplug_context *context;
> >  
> >  	if (libata_noacpi || ap->flags & ATA_FLAG_ACPI_SATA || !host_companion)
> >  		return;
> > @@ -188,9 +189,19 @@ void ata_acpi_bind_port(struct ata_port
> >  	if (ata_acpi_gtm(ap, &ap->__acpi_init_gtm) == 0)
> >  		ap->pflags |= ATA_PFLAG_INIT_GTM_VALID;
> >  
> > -	/* we might be on a docking station */
> > -	register_hotplug_dock_device(ACPI_HANDLE(&ap->tdev),
> > -				     &ata_acpi_ap_dock_ops, ap, NULL, NULL);
> > +	adev = ACPI_COMPANION(&ap->tdev);
> > +	if (!adev)
> > +		return;
> > +
> > +	context = kzalloc(sizeof(*context), GFP_KERNEL);
> > +	if (!context)
> > +		return;
> 
> The context isn't freed on ATA driver detach, which doesn't normally
> happen though.

Yes, but it will happen for ATA devices in docks and bays.  However,
the struct acpi_device objects won't go away then and we only need to initialize
the contexts once, so it should be sufficient to simply return from here if
the context is already there.

I'll send an updated patch with that change later today.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ