lkml.org 
[lkml]   [2015]   [Apr]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/3] libata: Stash initial power management configuration
Hello, Matthew.

On Sat, Apr 18, 2015 at 08:26:34AM -0700, Matthew Garrett wrote:
> @@ -313,6 +313,12 @@ struct ahci_port_priv {
> /* enclosure management info per PM slot */
> struct ahci_em_priv em_priv[EM_MAX_SLOTS];
> char *irq_desc; /* desc in /proc/interrupts */

Can you please add a comment here referring to
ahci_setup_port_privdata()?

> + bool init_alpe; /* alpe enabled by default */
> + bool init_asp; /* asp enabled by default */
> + bool init_devslp; /* devslp enabled by default */
> + u32 init_dito; /* initial dito configuration */
> + u32 init_deto; /* initial deto configuration */
> + u32 init_mdat; /* initial mdat configuration */
> };
...
> +/*
> + * Allocate port privdata and read back initial power management configuration
> + */

And convert this to proper function comment which explains where it's
supposed to be called from and why it does what it does there?

> +int ahci_setup_port_privdata(struct ata_port *ap)
> +{
...
> @@ -5583,11 +5586,11 @@ void ata_link_init(struct ata_port *ap, struct ata_link *link, int pmp)
> }
>
> /**
> - * sata_link_init_spd - Initialize link->sata_spd_limit
> - * @link: Link to configure sata_spd_limit for
> + * sata_link_init_config - Initialize link->sata_spd_limit and init_lpm
> + * @link: Link to configure sata_spd_limit and init_lpm for
> *
> - * Initialize @link->[hw_]sata_spd_limit to the currently
> - * configured value.
> + * Initialize @link->[hw_]sata_spd_limit and @link->init_lpm to the
> + * currently configured value.

And I think it probably would be a better idea to make the libata core
dipm part a separate patch.

Other than that, looks good to me.

Thanks.

--
tejun


\
 
 \ /
  Last update: 2015-04-21 23:41    [W:0.122 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site