Re: [patch 2/2] libata: power off unused ports

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 12 Apr 2008 00:28:13 -0400
Jeff Garzik <jeff@xxxxxxxxxx> wrote:

> Kristen Carlson Accardi wrote:
> > If a port doesn't support hot plug, there's no reason to keep the phy powered
> > on unoccupied ports.  
> > 
> > Signed-off-by:  Kristen Carlson Accardi <kristen.c.accardi@xxxxxxxxx>
> > 
> > Index: linux-2.6.25-rc3/drivers/ata/ahci.c
> > ===================================================================
> > --- linux-2.6.25-rc3.orig/drivers/ata/ahci.c
> > +++ linux-2.6.25-rc3/drivers/ata/ahci.c
> > @@ -52,6 +52,7 @@
> >  static int ahci_enable_alpm(struct ata_port *ap,
> >  		enum link_pm policy);
> >  static void ahci_disable_alpm(struct ata_port *ap);
> > +static int ahci_is_hotplug_capable(struct ata_port *ap);
> >  
> >  enum {
> >  	AHCI_PCI_BAR		= 5,
> > @@ -163,6 +164,7 @@ enum {
> >  	PORT_CMD_ASP		= (1 << 27), /* Aggressive Slumber/Partial */
> >  	PORT_CMD_ALPE		= (1 << 26), /* Aggressive Link PM enable */
> >  	PORT_CMD_ATAPI		= (1 << 24), /* Device is ATAPI */
> > +	PORT_CMD_HPCP		= (1 << 18), /* port is hot plug capable */
> 
> Under which conditions is this bit set?
> 
> The conclusion reached by this patch seems correct, but I am not sure 
> about the premise...
> 
> I was under the impression that AHCI ports were hotplug capable, from 
> libata's point of view, simply due to the fundamentals of SATA.

The HPCP bit is set by the BIOS to indicate policy for that port.
For example, a server may set this bit to indicate that there are
removable drive bays on it.  A laptop may not set this bit because nobody
is going to open it up and yank out the cable while it's running.  It is
safe to use this bit to figure out whether or not it's worth checking
for hot plug events on that port in practice, although you are right it
is physically possible for us to still do hot plug if we just pop open
the machine and go for it.

> 
> 
> 
> Thinking about the bigger pictures, powering off the phy is something we 
> want to do in a lot more cases than this, but there is a stumbling 
> block:  we wander into the realm of policy.
> 
> For most users most of the time, empty SATA ports are needlessly 
> powered.  The problem is that, at any given moment, a device may be 
> hot-plugged, so we must be ready for that.  We need some way for the 
> user to let the driver know that they will not be hotplugging anything 
> anytime soon, permitting power savings to be enabled.
> 
> A compromise solution that avoids adding a userspace "knob" has also 
> been proposed (by Tejun, I think?):  power up the phy every N seconds, 
> check for device, power down phy if nothing.  That should provide some 
> power savings, though not as much as with a "knob" switched to "hotplug: 
> off"
> 
> 	Jeff
> 
>

I think that for the common case - we should use HPCP to determine if
the suggested use of the port is for hot plug.  I can see your point
about wanting to give the user the option to just disregard the intended
use of the port and do what they want, but I say we don't make that
the default behavior.  And, I don't like the idea of adding another
wakeup in the driver to do polling - seems like 99% of users are
going to be just fine with a knob - and that they should only
have to use the knob to override the default (or how bout a 
module param?).  I don't think we should compromise power for a 
feature that most people are unlikely to use (if HPCP is not set).
 
--
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

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux