[PATCH V1 0/2] Address race condition in MCQ mode and enhance

[PATCH V1 0/2] Address race condition in MCQ mode and enhance

[palash.kambar]

From: Palash Kambar <palash.kambar@oss.qualcomm.com>

This patch series addresses a race condition in MCQ mode of the QCOM 
UFS host controller for hardware version 6.

The second patch integrates the logic into the UFS core layer by
invoking appropriate vops.

Can Guo (1):
  ufs: ufs-qcom: Disable AHIT before SQ tail update to prevent race in
    MCQ mode

Palash Kambar (1):
  ufs: core:Add vendor-specific callbacks and update setup_xfer_req
    interface

 drivers/ufs/core/ufshcd.c     |  6 ++++--
 drivers/ufs/host/ufs-exynos.c |  6 +++++-
 drivers/ufs/host/ufs-qcom.c   | 35 +++++++++++++++++++++++++++++++++++
 drivers/ufs/host/ufs-qcom.h   |  1 +
 include/ufs/ufshcd.h          |  5 +++--
 5 files changed, 48 insertions(+), 5 deletions(-)

-- 
2.34.1

[PATCH V1 1/2] ufs: core:Add vendor-specific callbacks and update setup_xfer_req interface

[palash.kambar]

From: Palash Kambar <palash.kambar@oss.qualcomm.com>

On QCOM UFSHC V6 in MCQ mode, a race condition exists where simultaneous
data and hibernate commands can cause data commands to be dropped when
the Auto-Hibernate Idle Timer (AHIT) is near expiration.

To mitigate this, AHIT is disabled before updating the SQ tail pointer,
and re-enabled only when no active commands remain. This prevents
conflicting command sequences from reaching the UniPro layer during
critical timing windows.

To support this:
- Introduce a new vendor operation `compl_command` to allow vendors to
  handle command completion in a customized manner.
- Update the argument list for the existing `setup_xfer_req` vendor
  operation to align with the updated UFS core interface.
- Modify the Exynos-specific `setup_xfer_req` implementation to match
  the new interface and support the AHIT handling logic.

Signed-off-by: Palash Kambar <palash.kambar@oss.qualcomm.com>
---
 drivers/ufs/core/ufshcd.c     | 6 ++++--
 drivers/ufs/host/ufs-exynos.c | 6 +++++-
 include/ufs/ufshcd.h          | 5 +++--
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 568a449e7331..fd771d6c315e 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2383,11 +2383,11 @@ void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag,
 		memcpy(dest, src, utrd_size);
 		ufshcd_inc_sq_tail(hwq);
 		spin_unlock(&hwq->sq_lock);
+		hba->vops->setup_xfer_req(hba, lrbp);
 	} else {
 		spin_lock_irqsave(&hba->outstanding_lock, flags);
 		if (hba->vops && hba->vops->setup_xfer_req)
-			hba->vops->setup_xfer_req(hba, lrbp->task_tag,
-						  !!lrbp->cmd);
+			hba->vops->setup_xfer_req(hba, lrbp);
 		__set_bit(lrbp->task_tag, &hba->outstanding_reqs);
 		ufshcd_writel(hba, 1 << lrbp->task_tag,
 			      REG_UTP_TRANSFER_REQ_DOOR_BELL);
@@ -5637,6 +5637,7 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag,
 	}
 	cmd = lrbp->cmd;
 	if (cmd) {
+		hba->vops->compl_command(hba, lrbp);
 		if (unlikely(ufshcd_should_inform_monitor(hba, lrbp)))
 			ufshcd_update_monitor(hba, lrbp);
 		ufshcd_add_command_trace(hba, task_tag, UFS_CMD_COMP);
@@ -5645,6 +5646,7 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag,
 		/* Do not touch lrbp after scsi done */
 		scsi_done(cmd);
 	} else {
+		hba->vops->compl_command(hba, lrbp);
 		if (cqe) {
 			ocs = le32_to_cpu(cqe->status) & MASK_OCS;
 			lrbp->utr_descriptor_ptr->header.ocs = ocs;
diff --git a/drivers/ufs/host/ufs-exynos.c b/drivers/ufs/host/ufs-exynos.c
index 70d195179eba..d87276f45e01 100644
--- a/drivers/ufs/host/ufs-exynos.c
+++ b/drivers/ufs/host/ufs-exynos.c
@@ -910,11 +910,15 @@ static int exynos_ufs_post_pwr_mode(struct ufs_hba *hba,
 }
 
 static void exynos_ufs_specify_nexus_t_xfer_req(struct ufs_hba *hba,
-						int tag, bool is_scsi_cmd)
+						struct ufshcd_lrb *lrbp)
 {
 	struct exynos_ufs *ufs = ufshcd_get_variant(hba);
 	u32 type;
+	int tag;
+	bool is_scsi_cmd;
 
+	tag = lrbp->task_tag;
+	is_scsi_cmd = !!lrbp->cmd;
 	type =  hci_readl(ufs, HCI_UTRL_NEXUS_TYPE);
 
 	if (is_scsi_cmd)
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index ea0021f067c9..845f0df34281 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -356,8 +356,9 @@ struct ufs_hba_variant_ops {
 			enum ufs_notify_change_status status,
 			const struct ufs_pa_layer_attr *desired_pwr_mode,
 			struct ufs_pa_layer_attr *final_params);
-	void	(*setup_xfer_req)(struct ufs_hba *hba, int tag,
-				  bool is_scsi_cmd);
+	void	(*setup_xfer_req)(struct ufs_hba *hba, struct ufshcd_lrb *lrbp);
+	void	(*compl_command)(struct ufs_hba *hba,
+				 struct ufshcd_lrb *lrbp);
 	void	(*setup_task_mgmt)(struct ufs_hba *, int, u8);
 	void    (*hibern8_notify)(struct ufs_hba *, enum uic_cmd_dme,
 					enum ufs_notify_change_status);
-- 
2.34.1

Re: [PATCH V1 1/2] ufs: core:Add vendor-specific callbacks and update setup_xfer_req interface

[Bart Van Assche]

On 10/13/25 11:04 PM, palash.kambar@oss.qualcomm.com wrote:
> On QCOM UFSHC V6 in MCQ mode, a race condition exists where simultaneous
> data and hibernate commands can cause data commands to be dropped when
> the Auto-Hibernate Idle Timer (AHIT) is near expiration.
> 
> To mitigate this, AHIT is disabled before updating the SQ tail pointer,
> and re-enabled only when no active commands remain. This prevents
> conflicting command sequences from reaching the UniPro layer during
> critical timing windows.
> 
> To support this:
> - Introduce a new vendor operation `compl_command` to allow vendors to
>    handle command completion in a customized manner.
> - Update the argument list for the existing `setup_xfer_req` vendor
>    operation to align with the updated UFS core interface.
> - Modify the Exynos-specific `setup_xfer_req` implementation to match
>    the new interface and support the AHIT handling logic.

Yikes. Please disable AHIT entirely or disable/enable AHIT from inside
the runtime power management callbacks rather than inventing a new
mechanism for tracking whether any commands are outstanding.

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 568a449e7331..fd771d6c315e 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -2383,11 +2383,11 @@ void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag,
>   		memcpy(dest, src, utrd_size);
>   		ufshcd_inc_sq_tail(hwq);
>   		spin_unlock(&hwq->sq_lock);
> +		hba->vops->setup_xfer_req(hba, lrbp);

What will happen if hba->vops->setup_xfer_req == NULL? Will the above
code trigger a kernel crash?

> @@ -5637,6 +5637,7 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag,
>   	}
>   	cmd = lrbp->cmd;
>   	if (cmd) {
> +		hba->vops->compl_command(hba, lrbp);
>   		if (unlikely(ufshcd_should_inform_monitor(hba, lrbp)))
>   			ufshcd_update_monitor(hba, lrbp);
>   		ufshcd_add_command_trace(hba, task_tag, UFS_CMD_COMP);

Yikes. New unconditional indirect function calls in the hot path are not
acceptable because these have a negative performance impact.

> @@ -5645,6 +5646,7 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag,
>   		/* Do not touch lrbp after scsi done */
>   		scsi_done(cmd);
>   	} else {
> +		hba->vops->compl_command(hba, lrbp);
>   		if (cqe) {
>   			ocs = le32_to_cpu(cqe->status) & MASK_OCS;
>   			lrbp->utr_descriptor_ptr->header.ocs = ocs;

Same comment here.

> diff --git a/drivers/ufs/host/ufs-exynos.c b/drivers/ufs/host/ufs-exynos.c
> index 70d195179eba..d87276f45e01 100644
> --- a/drivers/ufs/host/ufs-exynos.c
> +++ b/drivers/ufs/host/ufs-exynos.c
> @@ -910,11 +910,15 @@ static int exynos_ufs_post_pwr_mode(struct ufs_hba *hba,
>   }
>   
>   static void exynos_ufs_specify_nexus_t_xfer_req(struct ufs_hba *hba,
> -						int tag, bool is_scsi_cmd)
> +						struct ufshcd_lrb *lrbp)
>   {
>   	struct exynos_ufs *ufs = ufshcd_get_variant(hba);
>   	u32 type;
> +	int tag;
> +	bool is_scsi_cmd;
>   
> +	tag = lrbp->task_tag;
> +	is_scsi_cmd = !!lrbp->cmd;
>   	type =  hci_readl(ufs, HCI_UTRL_NEXUS_TYPE);
>   
>   	if (is_scsi_cmd)

I'm about to remove lrbp->cmd so please don't introduce any new users of
this structure member.

Bart.

Re: [PATCH V1 1/2] ufs: core:Add vendor-specific callbacks and update setup_xfer_req interface

[Palash Kambar]

On 10/14/2025 9:40 PM, Bart Van Assche wrote:
> On 10/13/25 11:04 PM, palash.kambar@oss.qualcomm.com wrote:
>> On QCOM UFSHC V6 in MCQ mode, a race condition exists where simultaneous
>> data and hibernate commands can cause data commands to be dropped when
>> the Auto-Hibernate Idle Timer (AHIT) is near expiration.
>>
>> To mitigate this, AHIT is disabled before updating the SQ tail pointer,
>> and re-enabled only when no active commands remain. This prevents
>> conflicting command sequences from reaching the UniPro layer during
>> critical timing windows.
>>
>> To support this:
>> - Introduce a new vendor operation `compl_command` to allow vendors to
>>    handle command completion in a customized manner.
>> - Update the argument list for the existing `setup_xfer_req` vendor
>>    operation to align with the updated UFS core interface.
>> - Modify the Exynos-specific `setup_xfer_req` implementation to match
>>    the new interface and support the AHIT handling logic.
>
> Yikes. Please disable AHIT entirely or disable/enable AHIT from inside
> the runtime power management callbacks rather than inventing a new
> mechanism for tracking whether any commands are outstanding.
>
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index 568a449e7331..fd771d6c315e 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -2383,11 +2383,11 @@ void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag,
>>           memcpy(dest, src, utrd_size);
>>           ufshcd_inc_sq_tail(hwq);
>>           spin_unlock(&hwq->sq_lock);
>> +        hba->vops->setup_xfer_req(hba, lrbp);
>
> What will happen if hba->vops->setup_xfer_req == NULL? Will the above
> code trigger a kernel crash? 


Thanks for catching this, will take care of this in next patch set.

>
>> @@ -5637,6 +5637,7 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag,
>>       }
>>       cmd = lrbp->cmd;
>>       if (cmd) {
>> +        hba->vops->compl_command(hba, lrbp);
>>           if (unlikely(ufshcd_should_inform_monitor(hba, lrbp)))
>>               ufshcd_update_monitor(hba, lrbp);
>>           ufshcd_add_command_trace(hba, task_tag, UFS_CMD_COMP);
>
> Yikes. New unconditional indirect function calls in the hot path are not
> acceptable because these have a negative performance impact. 
>
Sure, will take care of this in next patch set.


>> @@ -5645,6 +5646,7 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag,
>>           /* Do not touch lrbp after scsi done */
>>           scsi_done(cmd);
>>       } else {
>> +        hba->vops->compl_command(hba, lrbp);
>>           if (cqe) {
>>               ocs = le32_to_cpu(cqe->status) & MASK_OCS;
>>               lrbp->utr_descriptor_ptr->header.ocs = ocs;
>
> Same comment here. 

Sure, will take care of this in next patch set.


>
>> diff --git a/drivers/ufs/host/ufs-exynos.c b/drivers/ufs/host/ufs-exynos.c
>> index 70d195179eba..d87276f45e01 100644
>> --- a/drivers/ufs/host/ufs-exynos.c
>> +++ b/drivers/ufs/host/ufs-exynos.c
>> @@ -910,11 +910,15 @@ static int exynos_ufs_post_pwr_mode(struct ufs_hba *hba,
>>   }
>>     static void exynos_ufs_specify_nexus_t_xfer_req(struct ufs_hba *hba,
>> -                        int tag, bool is_scsi_cmd)
>> +                        struct ufshcd_lrb *lrbp)
>>   {
>>       struct exynos_ufs *ufs = ufshcd_get_variant(hba);
>>       u32 type;
>> +    int tag;
>> +    bool is_scsi_cmd;
>>   +    tag = lrbp->task_tag;
>> +    is_scsi_cmd = !!lrbp->cmd;
>>       type =  hci_readl(ufs, HCI_UTRL_NEXUS_TYPE);
>>         if (is_scsi_cmd)
>
> I'm about to remove lrbp->cmd so please don't introduce any new users of
> this structure member.
>
> Bart. 

Sure will update this as per the new change.


Regards,

Palash K

[PATCH V1 2/2] ufs: ufs-qcom: Disable AHIT before SQ tail update to prevent race in MCQ mode

[palash.kambar]

From: Can Guo <quic_cang@quicinc.com>

In MCQ mode, a race condition can occur on QCOM UFSHC V6 when the
Auto-Hibernate Idle Timer (AHIT) is close to expiring. If a data
command and a hibernate command are issued simultaneously to the
UniPro layer, the data command may be dropped.

To prevent this, AHIT is disabled by reprogramming it to 0 before
updating the SQ tail pointer. Once there are no active commands in
the UFS host controller, the timer is re-enabled.

This change ensures reliable command delivery in MCQ mode by
avoiding timing conflicts between data and hibernate operations.

Signed-off-by: Can Guo <quic_cang@quicinc.com>
Co-developed-by: Palash Kambar <palash.kambar@oss.qualcomm.com>
Signed-off-by: Palash Kambar <palash.kambar@oss.qualcomm.com>
---
 drivers/ufs/host/ufs-qcom.c | 35 +++++++++++++++++++++++++++++++++++
 drivers/ufs/host/ufs-qcom.h |  1 +
 2 files changed, 36 insertions(+)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 89a3328a7a75..f31239f4fc50 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1286,6 +1286,39 @@ static int ufs_qcom_icc_init(struct ufs_qcom_host *host)
 	return 0;
 }
 
+static void ufs_qcom_send_command(struct ufs_hba *hba,
+				  struct ufshcd_lrb *lrbp)
+{
+	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+	unsigned long flags;
+
+	if ((host->hw_ver.major == 6 && host->hw_ver.minor == 0 &&
+	     host->hw_ver.step == 0) && hba->mcq_enabled) {
+		spin_lock_irqsave(hba->host->host_lock, flags);
+		if ((++host->active_cmds) == 1)
+			/* Stop the auto-hiberate idle timer */
+			ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER);
+		spin_unlock_irqrestore(hba->host->host_lock, flags);
+	}
+}
+
+static void ufs_qcom_compl_command(struct ufs_hba *hba,
+				   struct ufshcd_lrb *lrbp)
+{
+	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+	unsigned long flags;
+
+	if ((host->hw_ver.major == 6 && host->hw_ver.minor == 0 &&
+	     host->hw_ver.step == 0) && hba->mcq_enabled) {
+		spin_lock_irqsave(hba->host->host_lock, flags);
+		if ((--host->active_cmds) == 0)
+			/* Activate the auto-hiberate idle timer */
+			ufshcd_writel(hba, hba->ahit,
+				      REG_AUTO_HIBERNATE_IDLE_TIMER);
+		spin_unlock_irqrestore(hba->host->host_lock, flags);
+	}
+}
+
 /**
  * ufs_qcom_init - bind phy with controller
  * @hba: host controller instance
@@ -2194,6 +2227,8 @@ static const struct ufs_hba_variant_ops ufs_hba_qcom_vops = {
 	.get_ufs_hci_version	= ufs_qcom_get_ufs_hci_version,
 	.clk_scale_notify	= ufs_qcom_clk_scale_notify,
 	.setup_clocks           = ufs_qcom_setup_clocks,
+	.setup_xfer_req         = ufs_qcom_send_command,
+	.compl_command          = ufs_qcom_compl_command,
 	.hce_enable_notify      = ufs_qcom_hce_enable_notify,
 	.link_startup_notify    = ufs_qcom_link_startup_notify,
 	.pwr_change_notify	= ufs_qcom_pwr_change_notify,
diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
index 380d02333d38..a97da99361a8 100644
--- a/drivers/ufs/host/ufs-qcom.h
+++ b/drivers/ufs/host/ufs-qcom.h
@@ -308,6 +308,7 @@ struct ufs_qcom_host {
 	u32 phy_gear;
 
 	bool esi_enabled;
+	unsigned long active_cmds;
 };
 
 struct ufs_qcom_drvdata {
-- 
2.34.1

Re: [PATCH V1 2/2] ufs: ufs-qcom: Disable AHIT before SQ tail update to prevent race in MCQ mode

[Bart Van Assche]

On 10/13/25 11:04 PM, palash.kambar@oss.qualcomm.com wrote:
> +static void ufs_qcom_send_command(struct ufs_hba *hba,
> +				  struct ufshcd_lrb *lrbp)
> +{
> +	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> +	unsigned long flags;
> +
> +	if ((host->hw_ver.major == 6 && host->hw_ver.minor == 0 &&
> +	     host->hw_ver.step == 0) && hba->mcq_enabled) {
> +		spin_lock_irqsave(hba->host->host_lock, flags);
> +		if ((++host->active_cmds) == 1)
> +			/* Stop the auto-hiberate idle timer */
> +			ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER);
> +		spin_unlock_irqrestore(hba->host->host_lock, flags);
> +	}
> +}

Do you realize that obtaining the host lock from the command submission
path introduces a contention point and hence cancels one of the benefits
of MCQ (different CPU cores can submit commands independently)? This
argument still applies if host->active_cmds would be changed into an
atomic counter because making that change would still trigger cache line
ping-pong if UFS commands are submitted from multiple CPU cores
simultaneously. I think this is a strong argument against this patch
series.

An example of how to implement this kind of counter without killing 
performance is available in the block layer (&q->q_usage_counter).
However, implementing a per-cpu counter and checking in an efficient
way whether it has reached zero is much more complicated than the
approach of this patch series.

Is AHIT really that valuable that you want to sacrifice MCQ performance
to enable AHIT? Why not to enable/disable AHIT from the RPM callbacks?

Bart.

Re: [PATCH V1 2/2] ufs: ufs-qcom: Disable AHIT before SQ tail update to prevent race in MCQ mode

[Bart Van Assche]

On 10/15/25 7:08 AM, Palash Kambar wrote:
> Since AHIT is a hardware-based power-saving feature, disabling it 
> entirely
> could lead to significant power penalties. Therefore, this patch aims to 
> preserve
> power efficiency while resolving the race condition.
> We have tested this change and observed no noticeable performance 
> degradation.
> Also, adding in RPM callbacks will not solve the power penalty as it 
> autosuspend timer is
> 3 secs in comparision to AHIT timer which is 5ms.

The runtime power management timeout can be modified. Please verify
whether the power consumption with AHIT disabled and the runtime power
management timeout set to 5 ms is acceptable.

Thanks,

Bart.

Re: [PATCH V1 2/2] ufs: ufs-qcom: Disable AHIT before SQ tail update to prevent race in MCQ mode

[Palash Kambar]

On 10/15/2025 9:15 PM, Bart Van Assche wrote:
> On 10/15/25 7:08 AM, Palash Kambar wrote:
>> Since AHIT is a hardware-based power-saving feature, disabling it entirely
>> could lead to significant power penalties. Therefore, this patch aims to preserve
>> power efficiency while resolving the race condition.
>> We have tested this change and observed no noticeable performance degradation.
>> Also, adding in RPM callbacks will not solve the power penalty as it autosuspend timer is
>> 3 secs in comparision to AHIT timer which is 5ms.
>
> The runtime power management timeout can be modified. Please verify
> whether the power consumption with AHIT disabled and the runtime power
> management timeout set to 5 ms is acceptable.
>
> Thanks,
>
> Bart.

Thanks for the feedback, Bart. However, I believe setting the runtime suspend delay to 5ms
 might be overly aggressive for the system and may have below side effects:

1. Short autosuspend timeouts can cause the UFS device to enter low-power states even 
during brief idle periods. This results in resume latency, introducing delays when the 
device needs to wake up for subsequent operations.
2. Frequent suspend and resume cycles may disrupt data flow, particularly in workloads
with bursty or intermittent I/O, leading to performance degradation.
3. When the autosuspend timer is overly aggressive, the UFS device may repeatedly 
transition between active and low-power states. These transitions themselves consume power, 
and if they occur too often, they can offset or even negate the intended power savings.

Please let me know your thoughts on this.

Regards,


Palash K

>  

Re: [PATCH V1 2/2] ufs: ufs-qcom: Disable AHIT before SQ tail update to prevent race in MCQ mode

[Peter Wang (王信友)]

On Sat, 2025-10-18 at 12:14 +0530, Palash Kambar wrote:
> Thanks for the feedback, Bart. However, I believe setting the runtime
> suspend delay to 5ms
>  might be overly aggressive for the system and may have below side
> effects:
> 
> 1. Short autosuspend timeouts can cause the UFS device to enter low-
> power states even
> during brief idle periods. This results in resume latency,
> introducing delays when the
> device needs to wake up for subsequent operations.
> 2. Frequent suspend and resume cycles may disrupt data flow,
> particularly in workloads
> with bursty or intermittent I/O, leading to performance degradation.
> 3. When the autosuspend timer is overly aggressive, the UFS device
> may repeatedly
> transition between active and low-power states. These transitions
> themselves consume power,
> and if they occur too often, they can offset or even negate the
> intended power savings.
> 
> Please let me know your thoughts on this.
> 
> Regards,
> 
> 
> Palash K
> 

Hi Palash,

Maybe disabling AH8 and enabling both UFSHCD_CAP_CLK_SCALING
and UFSHCD_CAP_HIBERN8_WITH_CLK_GATING is sufficient?

Thanks
Peter

>