[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150421212649.GH9455@htj.duckdns.org>
Date: Tue, 21 Apr 2015 17:26:49 -0400
From: Tejun Heo <tj@...nel.org>
To: Matthew Garrett <mjg59@...eos.com>
Cc: linux-kernel@...r.kernel.org, linux-ide@...r.kernel.org,
kristen@...ux.intel.com, Matthew Garrett <mjg59@...f.ucam.org>
Subject: Re: [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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists