On Wed, Nov 28, 2012 at 08:56:09AM +0000, James Bottomley wrote:
> On Tue, 2012-11-27 at 17:39 -0800, Tejun Heo wrote:
> > Hey, Rafael.
> >
> > On Wed, Nov 28, 2012 at 01:51:00AM +0100, Rafael J. Wysocki wrote:
> > > Having considered that a bit more I'm now thinking that in fact the power state
> > > the device is in at the moment doesn't really matter, so the polling code need
> > > not really know what PM is doing. What it needs to know is that the device
> > > will generate a hardware event when something interesting happens, so it is not
> > > necessary to poll it.
> > >
> > > In this particular case it is related to power management (apparently, hardware
> > > events will only be generated after the device has been put into ACPI D3cold,
> > > or so Aaron seems to be claiming), but it need not be in general, at least in
> > > principle.
> > >
> > > It looks like we need an "event driven" flag that the can be set in the lower
> > > layers and read by the upper layers. I suppose this means it would need to be
> > > in struct device, but not necessarily in the PM-specific part of it.
> >
> > We already have that. That's what gendisk->async_events is for (as
> > opposed to gendisk->events). If all events are async_events, block
> > won't poll for events, but I'm not sure that's the golden bullet.
> >
> > * None implements async_events yet and an API is missing -
> > disk_check_events() - which is trivial to add, but it's the same
> > story. We'll need a mechanism to shoot up notification from libata
> > to block layer. It's admittedly easier to justify routing through
> > SCSI tho. So, we're mostly shifting the problem. Given that async
> > events is nice to have, so it isn't a bad idea.
>
> Could we drive it in the polling code? As in, if we set a flag to say
> we're event driven and please don't bother us, we could just respond to
> the poll with the last known state (this would probably have to be in
> SCSI somewhere since most polls are Test Unit Readys). That way ZPODD
> sets this flag when the device powers down and unsets it when it powers
> up.
Does it mean I can do something like this:
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 5fc97d2..219820c 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -578,7 +578,10 @@ static unsigned int sr_block_check_events(struct gendisk *disk,
unsigned int clearing)
{
struct scsi_cd *cd = scsi_cd(disk);
- return cdrom_check_events(&cd->cdi, clearing);
+ if (!cd->device->event_driven)
+ return cdrom_check_events(&cd->cdi, clearing);
+ else
+ return 0;
}
static int sr_block_revalidate_disk(struct gendisk *disk)
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index e65c62e..1756151 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -160,6 +160,7 @@ struct scsi_device {
unsigned can_power_off:1; /* Device supports runtime power off */
unsigned wce_default_on:1; /* Cache is ON by default */
unsigned no_dif:1; /* T10 PI (DIF) should be disabled */
+ unsigned event_driven:1; /* No need to poll the device */
DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
struct list_head event_list; /* asserted events */
Then when ZPODD is powered off, it will set this flag; and unset it when
it is powered up.
Thanks,
Aaron
>
> James
>
> > * Still dunno much about zpodd but IIUC the notification from
> > zero-power is via ACPI. To advertise that the device doesn't need
> > polling, it should also be able to do async notification while
> > powered up, which isn't covered by zpodd but ATA async notification.
> > So, ummm... that's another obstacle. If zpodd requires the device
> > to support ATA async notification, it might not be too bad tho.
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
