[PATCH v2 0/3] nvme: Add sysfs interface for APST configuration management

[PATCH v2 0/3] nvme: Add sysfs interface for APST configuration management

[Yaxiong Tian]

From: Yaxiong Tian <tianyaxiong@kylinos.cn>

This series enhances NVMe APST (Autonomous Power State Transition) support by:
1. Adding warnings for PST table allocation failures
2. Exposing APST tables via sysfs for runtime inspection
3. Providing per-controller sysfs interface for APST configuration

The changes allow better visibility and control of power management settings
through userspace tools while maintaining the existing functionality.

Yaxiong Tian (3):
  nvme: Add warning for PST table memory allocation failure in
    nvme_configure_apst
  nvme: add sysfs interface for APST table updates
  nvme: add per-controller sysfs interface for APST configuration

Changes Since v1

Add mutex_lock in nvme_set_latency_tolerance() for Potential competition 
between nvme_set_latency_tolerance() and apst_update_store().


 drivers/nvme/host/core.c  | 24 ++++++++++------
 drivers/nvme/host/nvme.h  |  6 ++++
 drivers/nvme/host/sysfs.c | 59 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 8 deletions(-)

-- 
2.25.1

[PATCH v2 1/3] nvme: Add warning for PST table memory allocation failure in nvme_configure_apst

[Yaxiong Tian]

From: Yaxiong Tian <tianyaxiong@kylinos.cn>

Currently the function fails silently on PST table memory allocation failure.
Add warning messages to improve error visibility.

Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
---
 drivers/nvme/host/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index c2d89fac86c5..fb0404fee551 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2678,8 +2678,10 @@ static int nvme_configure_apst(struct nvme_ctrl *ctrl)
 	}
 
 	table = kzalloc(sizeof(*table), GFP_KERNEL);
-	if (!table)
+	if (!table) {
+		dev_warn(ctrl->device, "Failed to allocate pst table; not using APST\n");
 		return 0;
+	}
 
 	if (!ctrl->apst_enabled || ctrl->ps_max_latency_us == 0) {
 		/* Turn off APST. */
-- 
2.25.1

[PATCH v2 2/3] nvme: add sysfs interface for APST table updates

[Yaxiong Tian]

From: Yaxiong Tian <tianyaxiong@kylinos.cn>

Currently, the APST (Autonomous Power State Transition) table can only be
updated during module initialization via module parameters or indirectly
by setting QoS latency requirements. This patch adds a direct sysfs
interface to allow dynamic updates to the APST table at runtime.

The new sysfs entry is created at:
/sys/class/nvme/<controller>/apst_update

This provides more flexibility in power management tuning without
requiring module reload or QoS latency changes.

Example usage:
update nvme module parameters.
echo 1  > /sys/class/nvme/nvme0/apst_update

Signed-off-by: Yaxiong Tian  <tianyaxiong@kylinos.cn>
---
 drivers/nvme/host/core.c  |  9 +++++++--
 drivers/nvme/host/nvme.h  |  2 ++
 drivers/nvme/host/sysfs.c | 23 +++++++++++++++++++++++
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index fb0404fee551..9dea1046b8b4 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2654,7 +2654,7 @@ static bool nvme_apst_get_transition_time(u64 total_latency,
  *
  * Users can set ps_max_latency_us to zero to turn off APST.
  */
-static int nvme_configure_apst(struct nvme_ctrl *ctrl)
+int nvme_configure_apst(struct nvme_ctrl *ctrl)
 {
 	struct nvme_feat_auto_pst *table;
 	unsigned apste = 0;
@@ -2778,8 +2778,11 @@ static void nvme_set_latency_tolerance(struct device *dev, s32 val)
 
 	if (ctrl->ps_max_latency_us != latency) {
 		ctrl->ps_max_latency_us = latency;
-		if (nvme_ctrl_state(ctrl) == NVME_CTRL_LIVE)
+		if (nvme_ctrl_state(ctrl) == NVME_CTRL_LIVE) {
+			mutex_lock(&ctrl->apst_lock);
 			nvme_configure_apst(ctrl);
+			mutex_unlock(&ctrl->apst_lock);
+		}
 	}
 }
 
@@ -4852,6 +4855,8 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
 	ctrl->ka_cmd.common.opcode = nvme_admin_keep_alive;
 	ctrl->ka_last_check_time = jiffies;
 
+	mutex_init(&ctrl->apst_lock);
+
 	BUILD_BUG_ON(NVME_DSM_MAX_RANGES * sizeof(struct nvme_dsm_range) >
 			PAGE_SIZE);
 	ctrl->discard_page = alloc_page(GFP_KERNEL);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 51e078642127..7f8e10f5bf7a 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -385,6 +385,7 @@ struct nvme_ctrl {
 	key_serial_t tls_pskid;
 
 	/* Power saving configuration */
+	struct mutex apst_lock;
 	u64 ps_max_latency_us;
 	bool apst_enabled;
 
@@ -828,6 +829,7 @@ void nvme_unfreeze(struct nvme_ctrl *ctrl);
 void nvme_wait_freeze(struct nvme_ctrl *ctrl);
 int nvme_wait_freeze_timeout(struct nvme_ctrl *ctrl, long timeout);
 void nvme_start_freeze(struct nvme_ctrl *ctrl);
+int nvme_configure_apst(struct nvme_ctrl *ctrl);
 
 static inline enum req_op nvme_req_op(struct nvme_command *cmd)
 {
diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index 6d31226f7a4f..5003cb294d65 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -684,6 +684,28 @@ static DEVICE_ATTR(dhchap_ctrl_secret, S_IRUGO | S_IWUSR,
 	nvme_ctrl_dhchap_ctrl_secret_show, nvme_ctrl_dhchap_ctrl_secret_store);
 #endif
 
+static ssize_t apst_update_store(struct device *dev,
+					      struct device_attribute *attr,
+					      const char *buf, size_t size)
+{
+	int err;
+	bool bool_data = false;
+	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
+	err = kstrtobool(buf, &bool_data);
+
+	if (err)
+		return err;
+
+	if (bool_data && nvme_ctrl_state(ctrl) == NVME_CTRL_LIVE) {
+		mutex_lock(&ctrl->apst_lock);
+		nvme_configure_apst(ctrl);
+		mutex_unlock(&ctrl->apst_lock);
+	}
+
+	return size;
+}
+static DEVICE_ATTR_WO(apst_update);
+
 static struct attribute *nvme_dev_attrs[] = {
 	&dev_attr_reset_controller.attr,
 	&dev_attr_rescan_controller.attr,
@@ -712,6 +734,7 @@ static struct attribute *nvme_dev_attrs[] = {
 	&dev_attr_dhchap_ctrl_secret.attr,
 #endif
 	&dev_attr_adm_passthru_err_log_enabled.attr,
+	&dev_attr_apst_update.attr,
 	NULL
 };
 
-- 
2.25.1

[PATCH v2 3/3] nvme: add per-controller sysfs interface for APST configuration

[Yaxiong Tian]

From: Yaxiong Tian <tianyaxiong@kylinos.cn>

Currently, APST (Autonomous Power State Transition) parameters are
configured as module parameters affecting all controllers uniformly,
 This lacks flexibility for heterogeneous systems.

This patch introduces per-controller sysfs attributes under each NVMe
controller's sysfs directory:
- apst_primary_timeout_ms
- apst_secondary_timeout_ms
- apst_primary_latency_tol_us
- apst_secondary_latency_tol_us

The attributes allow runtime configuration of: Timeout values for
primary/secondary states.

The existing module parameters are retained for backward compatibility
but now only serve as default values for new controllers.

Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
---
 drivers/nvme/host/core.c  | 16 ++++++++++------
 drivers/nvme/host/nvme.h  |  4 ++++
 drivers/nvme/host/sysfs.c | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 9dea1046b8b4..c999bd905d10 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2608,22 +2608,22 @@ static int nvme_configure_host_options(struct nvme_ctrl *ctrl)
  * timeout value is returned and the matching tolerance index (1 or 2) is
  * reported.
  */
-static bool nvme_apst_get_transition_time(u64 total_latency,
+static bool nvme_apst_get_transition_time(struct nvme_ctrl *ctrl, u64 total_latency,
 		u64 *transition_time, unsigned *last_index)
 {
-	if (total_latency <= apst_primary_latency_tol_us) {
+	if (total_latency <= ctrl->apst_primary_latency_tol_us) {
 		if (*last_index == 1)
 			return false;
 		*last_index = 1;
-		*transition_time = apst_primary_timeout_ms;
+		*transition_time = ctrl->apst_primary_timeout_ms;
 		return true;
 	}
 	if (apst_secondary_timeout_ms &&
-		total_latency <= apst_secondary_latency_tol_us) {
+		total_latency <= ctrl->apst_secondary_latency_tol_us) {
 		if (*last_index <= 2)
 			return false;
 		*last_index = 2;
-		*transition_time = apst_secondary_timeout_ms;
+		*transition_time = ctrl->apst_secondary_timeout_ms;
 		return true;
 	}
 	return false;
@@ -2728,7 +2728,7 @@ int nvme_configure_apst(struct nvme_ctrl *ctrl)
 		 * for higher power states.
 		 */
 		if (apst_primary_timeout_ms && apst_primary_latency_tol_us) {
-			if (!nvme_apst_get_transition_time(total_latency_us,
+			if (!nvme_apst_get_transition_time(ctrl, total_latency_us,
 					&transition_ms, &last_lt_index))
 				continue;
 		} else {
@@ -4856,6 +4856,10 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
 	ctrl->ka_last_check_time = jiffies;
 
 	mutex_init(&ctrl->apst_lock);
+	ctrl->apst_primary_timeout_ms = apst_primary_timeout_ms;
+	ctrl->apst_secondary_timeout_ms = apst_secondary_timeout_ms;
+	ctrl->apst_primary_latency_tol_us = apst_primary_latency_tol_us;
+	ctrl->apst_secondary_latency_tol_us = apst_secondary_latency_tol_us;
 
 	BUILD_BUG_ON(NVME_DSM_MAX_RANGES * sizeof(struct nvme_dsm_range) >
 			PAGE_SIZE);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 7f8e10f5bf7a..ed9afc3c6781 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -387,6 +387,10 @@ struct nvme_ctrl {
 	/* Power saving configuration */
 	struct mutex apst_lock;
 	u64 ps_max_latency_us;
+	u64 apst_primary_timeout_ms;
+	u64 apst_secondary_timeout_ms;
+	u64 apst_primary_latency_tol_us;
+	u64 apst_secondary_latency_tol_us;
 	bool apst_enabled;
 
 	/* PCIe only: */
diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index 5003cb294d65..afa61e9c1366 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -684,6 +684,37 @@ static DEVICE_ATTR(dhchap_ctrl_secret, S_IRUGO | S_IWUSR,
 	nvme_ctrl_dhchap_ctrl_secret_show, nvme_ctrl_dhchap_ctrl_secret_store);
 #endif
 
+#define nvme_apst_show_and_store_function(field)				\
+static ssize_t field##_show(struct device *dev,		\
+			    struct device_attribute *attr, char *buf)	\
+{									\
+	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);			\
+	return sysfs_emit(buf, "%llu\n", ctrl->field);	\
+}									\
+									\
+static ssize_t field##_store(struct device *dev,		\
+			     struct device_attribute *attr,		\
+			     const char *buf, size_t size)		\
+{									\
+	int err;						\
+	u64 data = 0;					\
+	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);			\
+	err = kstrtou64(buf, 0, &data);		\
+	if (err < 0)						\
+		return err;						\
+							\
+	mutex_lock(&ctrl->apst_lock);		\
+	ctrl->field = data; 	\
+	mutex_unlock(&ctrl->apst_lock);		\
+	return size;			\
+}									\
+static DEVICE_ATTR_RW(field);
+
+nvme_apst_show_and_store_function(apst_primary_timeout_ms);
+nvme_apst_show_and_store_function(apst_secondary_timeout_ms);
+nvme_apst_show_and_store_function(apst_primary_latency_tol_us);
+nvme_apst_show_and_store_function(apst_secondary_latency_tol_us);
+
 static ssize_t apst_update_store(struct device *dev,
 					      struct device_attribute *attr,
 					      const char *buf, size_t size)
@@ -734,6 +765,11 @@ static struct attribute *nvme_dev_attrs[] = {
 	&dev_attr_dhchap_ctrl_secret.attr,
 #endif
 	&dev_attr_adm_passthru_err_log_enabled.attr,
+
+	&dev_attr_apst_primary_timeout_ms.attr,
+	&dev_attr_apst_secondary_timeout_ms.attr,
+	&dev_attr_apst_primary_latency_tol_us.attr,
+	&dev_attr_apst_secondary_latency_tol_us.attr,
 	&dev_attr_apst_update.attr,
 	NULL
 };
-- 
2.25.1

Re: [PATCH v2 1/3] nvme: Add warning for PST table memory allocation failure in nvme_configure_apst

[Chaitanya Kulkarni]

On 3/27/25 18:40, Yaxiong Tian wrote:
> From: Yaxiong Tian<tianyaxiong@kylinos.cn>
>
> Currently the function fails silently on PST table memory allocation failure.
> Add warning messages to improve error visibility.
>
> Signed-off-by: Yaxiong Tian<tianyaxiong@kylinos.cn>

seems useful while debugging, looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH v2 2/3] nvme: add sysfs interface for APST table updates

[Chaitanya Kulkarni]

On 3/27/25 18:40, Yaxiong Tian wrote:
> From: Yaxiong Tian <tianyaxiong@kylinos.cn>
>
> Currently, the APST (Autonomous Power State Transition) table can only be
> updated during module initialization via module parameters or indirectly
> by setting QoS latency requirements. This patch adds a direct sysfs
> interface to allow dynamic updates to the APST table at runtime.
>
> The new sysfs entry is created at:
> /sys/class/nvme/<controller>/apst_update
>
> This provides more flexibility in power management tuning without
> requiring module reload or QoS latency changes.
>
> Example usage:
> update nvme module parameters.
> echo 1  > /sys/class/nvme/nvme0/apst_update
>
> Signed-off-by: Yaxiong Tian  <tianyaxiong@kylinos.cn>

by any chance can you please provide a use-case in which scenario
you need to dynamically updating APST ?

> ---
>   drivers/nvme/host/core.c  |  9 +++++++--
>   drivers/nvme/host/nvme.h  |  2 ++
>   drivers/nvme/host/sysfs.c | 23 +++++++++++++++++++++++
>   3 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index fb0404fee551..9dea1046b8b4 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2654,7 +2654,7 @@ static bool nvme_apst_get_transition_time(u64 total_latency,
>    *
>    * Users can set ps_max_latency_us to zero to turn off APST.
>    */
> -static int nvme_configure_apst(struct nvme_ctrl *ctrl)
> +int nvme_configure_apst(struct nvme_ctrl *ctrl)
>   {
>   	struct nvme_feat_auto_pst *table;
>   	unsigned apste = 0;
> @@ -2778,8 +2778,11 @@ static void nvme_set_latency_tolerance(struct device *dev, s32 val)
>   
>   	if (ctrl->ps_max_latency_us != latency) {
>   		ctrl->ps_max_latency_us = latency;
> -		if (nvme_ctrl_state(ctrl) == NVME_CTRL_LIVE)
> +		if (nvme_ctrl_state(ctrl) == NVME_CTRL_LIVE) {
> +			mutex_lock(&ctrl->apst_lock);
>   			nvme_configure_apst(ctrl);
> +			mutex_unlock(&ctrl->apst_lock);
> +		}
>   	}
>   }
>   
> @@ -4852,6 +4855,8 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
>   	ctrl->ka_cmd.common.opcode = nvme_admin_keep_alive;
>   	ctrl->ka_last_check_time = jiffies;
>   
> +	mutex_init(&ctrl->apst_lock);
> +
>   	BUILD_BUG_ON(NVME_DSM_MAX_RANGES * sizeof(struct nvme_dsm_range) >
>   			PAGE_SIZE);
>   	ctrl->discard_page = alloc_page(GFP_KERNEL);
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 51e078642127..7f8e10f5bf7a 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -385,6 +385,7 @@ struct nvme_ctrl {
>   	key_serial_t tls_pskid;
>   
>   	/* Power saving configuration */
> +	struct mutex apst_lock;
>   	u64 ps_max_latency_us;
>   	bool apst_enabled;
>   
> @@ -828,6 +829,7 @@ void nvme_unfreeze(struct nvme_ctrl *ctrl);
>   void nvme_wait_freeze(struct nvme_ctrl *ctrl);
>   int nvme_wait_freeze_timeout(struct nvme_ctrl *ctrl, long timeout);
>   void nvme_start_freeze(struct nvme_ctrl *ctrl);
> +int nvme_configure_apst(struct nvme_ctrl *ctrl);
>   
>   static inline enum req_op nvme_req_op(struct nvme_command *cmd)
>   {
> diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
> index 6d31226f7a4f..5003cb294d65 100644
> --- a/drivers/nvme/host/sysfs.c
> +++ b/drivers/nvme/host/sysfs.c
> @@ -684,6 +684,28 @@ static DEVICE_ATTR(dhchap_ctrl_secret, S_IRUGO | S_IWUSR,
>   	nvme_ctrl_dhchap_ctrl_secret_show, nvme_ctrl_dhchap_ctrl_secret_store);
>   #endif
>   
> +static ssize_t apst_update_store(struct device *dev,
> +					      struct device_attribute *attr,
> +					      const char *buf, size_t size)
> +{
> +	int err;
> +	bool bool_data = false;
> +	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);

reverse tree ?

+	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
+	bool bool_data = false;
+	int err;

> +	err = kstrtobool(buf, &bool_data);

add new line above after all the declarations ?

-ck

Re: [PATCH v2 2/3] nvme: add sysfs interface for APST table updates

[Yaxiong Tian]

On 2025/4/1 10:55, Chaitanya Kulkarni wrote:
> On 3/27/25 18:40, Yaxiong Tian wrote:
>> From: Yaxiong Tian <tianyaxiong@kylinos.cn>
>>
>> Currently, the APST (Autonomous Power State Transition) table can only be
>> updated during module initialization via module parameters or indirectly
>> by setting QoS latency requirements. This patch adds a direct sysfs
>> interface to allow dynamic updates to the APST table at runtime.
>>
>> The new sysfs entry is created at:
>> /sys/class/nvme/<controller>/apst_update
>>
>> This provides more flexibility in power management tuning without
>> requiring module reload or QoS latency changes.
>>
>> Example usage:
>> update nvme module parameters.
>> echo 1  > /sys/class/nvme/nvme0/apst_update
>>
>> Signed-off-by: Yaxiong Tian  <tianyaxiong@kylinos.cn>
> 
> by any chance can you please provide a use-case in which scenario
> you need to dynamically updating APST ?
Yes,In desktop systems, users can choose between power-saving mode and 
performance mode. These two modes involve different trade-offs between 
NVMe performance and power efficiency, thus requiring dynamic updates to 
APST. I will update the commit message.
> 
>> ---
>>    drivers/nvme/host/core.c  |  9 +++++++--
>>    drivers/nvme/host/nvme.h  |  2 ++
>>    drivers/nvme/host/sysfs.c | 23 +++++++++++++++++++++++
>>    3 files changed, 32 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index fb0404fee551..9dea1046b8b4 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -2654,7 +2654,7 @@ static bool nvme_apst_get_transition_time(u64 total_latency,
>>     *
>>     * Users can set ps_max_latency_us to zero to turn off APST.
>>     */
>> -static int nvme_configure_apst(struct nvme_ctrl *ctrl)
>> +int nvme_configure_apst(struct nvme_ctrl *ctrl)
>>    {
>>    	struct nvme_feat_auto_pst *table;
>>    	unsigned apste = 0;
>> @@ -2778,8 +2778,11 @@ static void nvme_set_latency_tolerance(struct device *dev, s32 val)
>>    
>>    	if (ctrl->ps_max_latency_us != latency) {
>>    		ctrl->ps_max_latency_us = latency;
>> -		if (nvme_ctrl_state(ctrl) == NVME_CTRL_LIVE)
>> +		if (nvme_ctrl_state(ctrl) == NVME_CTRL_LIVE) {
>> +			mutex_lock(&ctrl->apst_lock);
>>    			nvme_configure_apst(ctrl);
>> +			mutex_unlock(&ctrl->apst_lock);
>> +		}
>>    	}
>>    }
>>    
>> @@ -4852,6 +4855,8 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
>>    	ctrl->ka_cmd.common.opcode = nvme_admin_keep_alive;
>>    	ctrl->ka_last_check_time = jiffies;
>>    
>> +	mutex_init(&ctrl->apst_lock);
>> +
>>    	BUILD_BUG_ON(NVME_DSM_MAX_RANGES * sizeof(struct nvme_dsm_range) >
>>    			PAGE_SIZE);
>>    	ctrl->discard_page = alloc_page(GFP_KERNEL);
>> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
>> index 51e078642127..7f8e10f5bf7a 100644
>> --- a/drivers/nvme/host/nvme.h
>> +++ b/drivers/nvme/host/nvme.h
>> @@ -385,6 +385,7 @@ struct nvme_ctrl {
>>    	key_serial_t tls_pskid;
>>    
>>    	/* Power saving configuration */
>> +	struct mutex apst_lock;
>>    	u64 ps_max_latency_us;
>>    	bool apst_enabled;
>>    
>> @@ -828,6 +829,7 @@ void nvme_unfreeze(struct nvme_ctrl *ctrl);
>>    void nvme_wait_freeze(struct nvme_ctrl *ctrl);
>>    int nvme_wait_freeze_timeout(struct nvme_ctrl *ctrl, long timeout);
>>    void nvme_start_freeze(struct nvme_ctrl *ctrl);
>> +int nvme_configure_apst(struct nvme_ctrl *ctrl);
>>    
>>    static inline enum req_op nvme_req_op(struct nvme_command *cmd)
>>    {
>> diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
>> index 6d31226f7a4f..5003cb294d65 100644
>> --- a/drivers/nvme/host/sysfs.c
>> +++ b/drivers/nvme/host/sysfs.c
>> @@ -684,6 +684,28 @@ static DEVICE_ATTR(dhchap_ctrl_secret, S_IRUGO | S_IWUSR,
>>    	nvme_ctrl_dhchap_ctrl_secret_show, nvme_ctrl_dhchap_ctrl_secret_store);
>>    #endif
>>    
>> +static ssize_t apst_update_store(struct device *dev,
>> +					      struct device_attribute *attr,
>> +					      const char *buf, size_t size)
>> +{
>> +	int err;
>> +	bool bool_data = false;
>> +	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> 
> reverse tree ?
> 
> +	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> +	bool bool_data = false;
> +	int err;
> 
>> +	err = kstrtobool(buf, &bool_data);
> 
> add new line above after all the declarations ?
> 
> -ck
> 
> 
Thanks, I'll update this in the V3 patch.