Message ID | 20120925081825.GA1666@mint-spring.sh.intel.com |
---|---|
State | Not Applicable |
Delegated to: | David Miller |
Headers | show |
On Tue, 2012-09-25 at 16:18 +0800, Aaron Lu wrote: > A example patch would be something like the following, I didn't seperate > these ACPI calls in sr.c as this is just a concept proof, if this is the > right thing to do, I will separate them into another file sr-acpi.c and > make empty stubs for them in sr.h for systems do not have ACPI configured. Apart from the needed separation to compile in the !ACPI case > diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c > index ef72682..94d17f1 100644 > --- a/drivers/scsi/sr.c > +++ b/drivers/scsi/sr.c > @@ -46,6 +46,7 @@ > #include <linux/mutex.h> > #include <linux/slab.h> > #include <linux/pm_runtime.h> > +#include <linux/acpi.h> > #include <asm/uaccess.h> > > #include <scsi/scsi.h> > @@ -57,6 +58,8 @@ > #include <scsi/scsi_host.h> > #include <scsi/scsi_ioctl.h> /* For the door lock/unlock commands */ > > +#include <acpi/acpi_bus.h> > + > #include "scsi_logging.h" > #include "sr.h" > > @@ -212,8 +220,8 @@ static int sr_resume(struct device *dev) > scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES, &sshdr); > > /* If user wakes up the ODD, eject the tray */ > - if (cd->device->need_eject) { > - cd->device->need_eject = 0; > + if (cd->need_eject) { > + cd->need_eject = false; > /* But only for tray type ODD when door is not locked */ > if (!(cd->cdi.mask & CDC_CLOSE_TRAY) && !cd->door_locked) > sr_tray_move(&cd->cdi, 1); > @@ -704,6 +711,58 @@ static void sr_release(struct cdrom_device_info *cdi) > > } > > +static void sr_acpi_wake_dev(acpi_handle handle, u32 event, void *context) > +{ > + struct device *dev = context; > + struct scsi_cd *cd = dev_get_drvdata(dev); > + > + if (event == ACPI_NOTIFY_DEVICE_WAKE && pm_runtime_suspended(dev)) { > + cd->need_eject = true; > + pm_runtime_resume(dev); > + } > +} > + > +static void sr_acpi_add_pm_notifier(struct device *dev) > +{ > + struct acpi_device *acpi_dev; > + acpi_handle handle; > + acpi_status status; > + > + handle = dev->archdata.acpi_handle; This is a complete no-no. archdata is defined to be specific to the architecture it's supposed to be opaque to non-arch code. You'll find that only x86 and ia64 defines an acpi_handle there. This will instantly fail to compile on non intel. If you need the handle, it should be obtained via some accessor like dev_to_acpi_handle() which will allow this to continue to function when, say, arm acquires ACPI. I've got to say, this looks like a fault in ACPI itself. If the handles are 1:1 with struct device, then why not have all the functions taking handles take the device instead and leave the embedded handle safely in the architecture specific code? James -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Sep 25, 2012 at 03:02:29PM +0400, James Bottomley wrote: > On Tue, 2012-09-25 at 16:18 +0800, Aaron Lu wrote: > > A example patch would be something like the following, I didn't seperate > > these ACPI calls in sr.c as this is just a concept proof, if this is the > > right thing to do, I will separate them into another file sr-acpi.c and > > make empty stubs for them in sr.h for systems do not have ACPI configured. > > Apart from the needed separation to compile in the !ACPI case > > > diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c > > index ef72682..94d17f1 100644 > > --- a/drivers/scsi/sr.c > > +++ b/drivers/scsi/sr.c > > @@ -46,6 +46,7 @@ > > #include <linux/mutex.h> > > #include <linux/slab.h> > > #include <linux/pm_runtime.h> > > +#include <linux/acpi.h> > > #include <asm/uaccess.h> > > > > #include <scsi/scsi.h> > > @@ -57,6 +58,8 @@ > > #include <scsi/scsi_host.h> > > #include <scsi/scsi_ioctl.h> /* For the door lock/unlock commands */ > > > > +#include <acpi/acpi_bus.h> > > + > > #include "scsi_logging.h" > > #include "sr.h" > > > > @@ -212,8 +220,8 @@ static int sr_resume(struct device *dev) > > scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES, &sshdr); > > > > /* If user wakes up the ODD, eject the tray */ > > - if (cd->device->need_eject) { > > - cd->device->need_eject = 0; > > + if (cd->need_eject) { > > + cd->need_eject = false; > > /* But only for tray type ODD when door is not locked */ > > if (!(cd->cdi.mask & CDC_CLOSE_TRAY) && !cd->door_locked) > > sr_tray_move(&cd->cdi, 1); > > @@ -704,6 +711,58 @@ static void sr_release(struct cdrom_device_info *cdi) > > > > } > > > > +static void sr_acpi_wake_dev(acpi_handle handle, u32 event, void *context) > > +{ > > + struct device *dev = context; > > + struct scsi_cd *cd = dev_get_drvdata(dev); > > + > > + if (event == ACPI_NOTIFY_DEVICE_WAKE && pm_runtime_suspended(dev)) { > > + cd->need_eject = true; > > + pm_runtime_resume(dev); > > + } > > +} > > + > > +static void sr_acpi_add_pm_notifier(struct device *dev) > > +{ > > + struct acpi_device *acpi_dev; > > + acpi_handle handle; > > + acpi_status status; > > + > > + handle = dev->archdata.acpi_handle; > > This is a complete no-no. archdata is defined to be specific to the > architecture it's supposed to be opaque to non-arch code. You'll find > that only x86 and ia64 defines an acpi_handle there. This will > instantly fail to compile on non intel. If you need the handle, it If you are OK with this change to solve the need_eject flag, I'll prepare a formal patch, in which, all of the newly added function will be within the range of #ifdef CONFIG_ACPI ... ... #endif And for the CONFIG_ACPI not defined case, they will be static inline empty functions. Then there should be no compile errors. > should be obtained via some accessor like dev_to_acpi_handle() which > will allow this to continue to function when, say, arm acquires ACPI. There is a DEVICE_ACPI_HANDLE macro that I'll use when preparing the formal patch. I'm rushing out these code to show the idea. Sorry for not considering these things. Thanks, Aaron -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c index ef72682..94d17f1 100644 --- a/drivers/scsi/sr.c +++ b/drivers/scsi/sr.c @@ -46,6 +46,7 @@ #include <linux/mutex.h> #include <linux/slab.h> #include <linux/pm_runtime.h> +#include <linux/acpi.h> #include <asm/uaccess.h> #include <scsi/scsi.h> @@ -57,6 +58,8 @@ #include <scsi/scsi_host.h> #include <scsi/scsi_ioctl.h> /* For the door lock/unlock commands */ +#include <acpi/acpi_bus.h> + #include "scsi_logging.h" #include "sr.h" @@ -212,8 +220,8 @@ static int sr_resume(struct device *dev) scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES, &sshdr); /* If user wakes up the ODD, eject the tray */ - if (cd->device->need_eject) { - cd->device->need_eject = 0; + if (cd->need_eject) { + cd->need_eject = false; /* But only for tray type ODD when door is not locked */ if (!(cd->cdi.mask & CDC_CLOSE_TRAY) && !cd->door_locked) sr_tray_move(&cd->cdi, 1); @@ -704,6 +711,58 @@ static void sr_release(struct cdrom_device_info *cdi) } +static void sr_acpi_wake_dev(acpi_handle handle, u32 event, void *context) +{ + struct device *dev = context; + struct scsi_cd *cd = dev_get_drvdata(dev); + + if (event == ACPI_NOTIFY_DEVICE_WAKE && pm_runtime_suspended(dev)) { + cd->need_eject = true; + pm_runtime_resume(dev); + } +} + +static void sr_acpi_add_pm_notifier(struct device *dev) +{ + struct acpi_device *acpi_dev; + acpi_handle handle; + acpi_status status; + + handle = dev->archdata.acpi_handle; + if (!handle) + return; + + status = acpi_bus_get_device(handle, &acpi_dev); + if (ACPI_FAILURE(status)) + return; + + acpi_power_resource_register_device(dev, handle); + acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY, + sr_acpi_wake_dev, dev); + device_set_run_wake(dev, true); +} + +static void sr_acpi_remove_pm_notifier(struct device *dev) +{ + struct acpi_device *acpi_dev; + acpi_handle handle; + acpi_status status; + + handle = dev->archdata.acpi_handle; + if (!handle) + return; + + status = acpi_bus_get_device(handle, &acpi_dev); + if (ACPI_FAILURE(status)) + return; + + acpi_power_resource_unregister_device(dev, handle); + device_set_run_wake(dev, false); + acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY, sr_acpi_wake_dev); +} + static int sr_probe(struct device *dev) { struct scsi_device *sdev = to_scsi_device(dev); @@ -786,7 +845,9 @@ static int sr_probe(struct device *dev) sdev_printk(KERN_DEBUG, sdev,