Hi Tejun,
On Thu, Jan 10, 2013 at 11:52:31AM -0800, Tejun Heo wrote:
> Hello, Aaron.
>
> On Thu, Jan 10, 2013 at 05:24:25PM +0800, Aaron Lu wrote:
> > +/*
> > + * Update the zpodd->zp_ready field. This field will only be set
> > + * if the ODD has stayed in ZP ready state for zpodd_poweroff_delay
> > + * time, and will be used to decide if power off is allowed. If it
> > + * is set, it will be cleared during resume from powered off state.
> > + */
> > +void zpodd_on_suspend(struct ata_device *dev)
> > +{
> > + struct zpodd *zpodd = dev->zpodd;
> > + unsigned long expires;
> > +
> > + if (!zpready(dev)) {
> > + zpodd->zp_sampled = false;
> > + return;
> > + }
> > +
> > + if (!zpodd->zp_sampled) {
> > + zpodd->zp_sampled = true;
> > + zpodd->last_ready = jiffies;
> > + return;
>
> Is the above return necessary? Can't we just fall through and always
> check the actual condition?
The zpready(dev) function will check the actual condition by issuing a
TUR and check the returned sense code. And the zp_sampled serves as an
indication if this is the first time we sensed the ODD in ZP ready
status.
The zpodd_on_suspend works like this:
check ZP ready status by calling zpready(dev)
if not, clear zp_sampled and zp_ready
return
ODD is ZP ready
check if this is the first time we sensed it is ZP ready
if not, set the zp_sampled flag and record timestamp
return
This is not the first time we sensed ZP ready for the ODD
check if the ODD has been in this state long enough
if not, return
The ODD has been in ZP ready state long enough
set the zp_ready flag, the zpodd_zpready will use that flag
Done.
Does this work flow look OK?
BTW, I just realized I mistakenly removed the clear of zp_ready in v12
when zpready(dev) returns false, I thought it is not necessary since
each time we set that flag, the ODD will be powered off and the flag
will always be cleared during resume time in post_poweron. But when
qos_no_poweroff is set, this is not the case and can cause problem when
ODD is not in ZP ready state and user cleared the qos_no_poweroff flag.
I'll add a comment for this in next version to avoid making this mistake
again in the future...
Thanks,
Aaron
>
> > + }
> > +
> > + expires = zpodd->last_ready +
> > + msecs_to_jiffies(zpodd_poweroff_delay * 1000);
> > + if (time_before(jiffies, expires))
> > + return;
> > +
> > + zpodd->zp_ready = true;
> > +}
>
> Thanks.
>
> --
> tejun
--
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
