[PATCH 1/3] ACPI: CPPC: Refactor register get and set ABIs

Lifeng Zheng posted 3 patches 1 year, 2 months ago
There is a newer version of this series
[PATCH 1/3] ACPI: CPPC: Refactor register get and set ABIs
Posted by Lifeng Zheng 1 year, 2 months ago
Refactor register get and set ABIs using cppc_get_reg() and cppc_set_reg().

Rename cppc_get_perf() to cppc_get_reg() as a generic function to read cppc
registers, with two changes:

1. Change the error kind to "no such device" when pcc_ss_id < 0, which
means that this cpu cannot get a valid pcc_ss_id.

2. Add a check to verify if the register is a cpc supported one before
using it.

Add cppc_set_reg() as a generic function for setting cppc registers. Unlike
other set reg ABIs, this function checks CPC_SUPPORTED right after getting
the register, because the rest of the operations are meaningless if this
register is not a cpc supported one.

Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
---
 drivers/acpi/cppc_acpi.c | 191 +++++++++++++++------------------------
 1 file changed, 72 insertions(+), 119 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index c1f3568d0c50..306ced9c3376 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1179,10 +1179,13 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
 	return ret_val;
 }
 
-static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
+static int cppc_get_reg(int cpunum, enum cppc_regs reg_idx, u64 *val)
 {
 	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
+	struct cppc_pcc_data *pcc_ss_data = NULL;
 	struct cpc_register_resource *reg;
+	int pcc_ss_id;
+	int ret = 0;
 
 	if (!cpc_desc) {
 		pr_debug("No CPC descriptor for CPU:%d\n", cpunum);
@@ -1191,20 +1194,23 @@ static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
 
 	reg = &cpc_desc->cpc_regs[reg_idx];
 
+	if (!CPC_SUPPORTED(reg)) {
+		pr_debug("CPC register (reg_idx=%u) is not supported\n", reg_idx);
+		return -EOPNOTSUPP;
+	}
+
 	if (CPC_IN_PCC(reg)) {
-		int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
-		struct cppc_pcc_data *pcc_ss_data = NULL;
-		int ret = 0;
+		pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
 
 		if (pcc_ss_id < 0)
-			return -EIO;
+			return -ENODEV;
 
 		pcc_ss_data = pcc_data[pcc_ss_id];
 
 		down_write(&pcc_ss_data->pcc_lock);
 
 		if (send_pcc_cmd(pcc_ss_id, CMD_READ) >= 0)
-			cpc_read(cpunum, reg, perf);
+			cpc_read(cpunum, reg, val);
 		else
 			ret = -EIO;
 
@@ -1213,21 +1219,65 @@ static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
 		return ret;
 	}
 
-	cpc_read(cpunum, reg, perf);
+	cpc_read(cpunum, reg, val);
 
 	return 0;
 }
 
+static int cppc_set_reg(int cpu, enum cppc_regs reg_idx, u64 val)
+{
+	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
+	struct cppc_pcc_data *pcc_ss_data = NULL;
+	struct cpc_register_resource *reg;
+	int pcc_ss_id;
+	int ret;
+
+	if (!cpc_desc) {
+		pr_debug("No CPC descriptor for CPU:%d\n", cpu);
+		return -ENODEV;
+	}
+
+	reg = &cpc_desc->cpc_regs[reg_idx];
+
+	if (!CPC_SUPPORTED(reg)) {
+		pr_debug("CPC register (reg_idx=%u) is not supported\n", reg_idx);
+		return -EOPNOTSUPP;
+	}
+
+	if (CPC_IN_PCC(reg)) {
+		pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
+
+		if (pcc_ss_id < 0) {
+			pr_debug("Invalid pcc_ss_id\n");
+			return -ENODEV;
+		}
+
+		ret = cpc_write(cpu, reg, val);
+		if (ret)
+			return ret;
+
+		pcc_ss_data = pcc_data[pcc_ss_id];
+
+		down_write(&pcc_ss_data->pcc_lock);
+		/* after writing CPC, transfer the ownership of PCC to platform */
+		ret = send_pcc_cmd(pcc_ss_id, CMD_WRITE);
+		up_write(&pcc_ss_data->pcc_lock);
+		return ret;
+	}
+
+	return cpc_write(cpu, reg, val);
+}
+
 /**
  * cppc_get_desired_perf - Get the desired performance register value.
  * @cpunum: CPU from which to get desired performance.
  * @desired_perf: Return address.
  *
- * Return: 0 for success, -EIO otherwise.
+ * Return: 0 for success, -ERRNO otherwise.
  */
 int cppc_get_desired_perf(int cpunum, u64 *desired_perf)
 {
-	return cppc_get_perf(cpunum, DESIRED_PERF, desired_perf);
+	return cppc_get_reg(cpunum, DESIRED_PERF, desired_perf);
 }
 EXPORT_SYMBOL_GPL(cppc_get_desired_perf);
 
@@ -1236,11 +1286,11 @@ EXPORT_SYMBOL_GPL(cppc_get_desired_perf);
  * @cpunum: CPU from which to get nominal performance.
  * @nominal_perf: Return address.
  *
- * Return: 0 for success, -EIO otherwise.
+ * Return: 0 for success, -ERRNO otherwise.
  */
 int cppc_get_nominal_perf(int cpunum, u64 *nominal_perf)
 {
-	return cppc_get_perf(cpunum, NOMINAL_PERF, nominal_perf);
+	return cppc_get_reg(cpunum, NOMINAL_PERF, nominal_perf);
 }
 
 /**
@@ -1248,11 +1298,11 @@ int cppc_get_nominal_perf(int cpunum, u64 *nominal_perf)
  * @cpunum: CPU from which to get highest performance.
  * @highest_perf: Return address.
  *
- * Return: 0 for success, -EIO otherwise.
+ * Return: 0 for success, -ERRNO otherwise.
  */
 int cppc_get_highest_perf(int cpunum, u64 *highest_perf)
 {
-	return cppc_get_perf(cpunum, HIGHEST_PERF, highest_perf);
+	return cppc_get_reg(cpunum, HIGHEST_PERF, highest_perf);
 }
 EXPORT_SYMBOL_GPL(cppc_get_highest_perf);
 
@@ -1261,11 +1311,11 @@ EXPORT_SYMBOL_GPL(cppc_get_highest_perf);
  * @cpunum: CPU from which to get epp preference value.
  * @epp_perf: Return address.
  *
- * Return: 0 for success, -EIO otherwise.
+ * Return: 0 for success, -ERRNO otherwise.
  */
 int cppc_get_epp_perf(int cpunum, u64 *epp_perf)
 {
-	return cppc_get_perf(cpunum, ENERGY_PERF, epp_perf);
+	return cppc_get_reg(cpunum, ENERGY_PERF, epp_perf);
 }
 EXPORT_SYMBOL_GPL(cppc_get_epp_perf);
 
@@ -1545,44 +1595,14 @@ EXPORT_SYMBOL_GPL(cppc_set_epp_perf);
  */
 int cppc_get_auto_sel_caps(int cpunum, struct cppc_perf_caps *perf_caps)
 {
-	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
-	struct cpc_register_resource *auto_sel_reg;
-	u64  auto_sel;
-
-	if (!cpc_desc) {
-		pr_debug("No CPC descriptor for CPU:%d\n", cpunum);
-		return -ENODEV;
-	}
-
-	auto_sel_reg = &cpc_desc->cpc_regs[AUTO_SEL_ENABLE];
-
-	if (!CPC_SUPPORTED(auto_sel_reg))
-		pr_warn_once("Autonomous mode is not unsupported!\n");
-
-	if (CPC_IN_PCC(auto_sel_reg)) {
-		int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
-		struct cppc_pcc_data *pcc_ss_data = NULL;
-		int ret = 0;
-
-		if (pcc_ss_id < 0)
-			return -ENODEV;
-
-		pcc_ss_data = pcc_data[pcc_ss_id];
-
-		down_write(&pcc_ss_data->pcc_lock);
-
-		if (send_pcc_cmd(pcc_ss_id, CMD_READ) >= 0) {
-			cpc_read(cpunum, auto_sel_reg, &auto_sel);
-			perf_caps->auto_sel = (bool)auto_sel;
-		} else {
-			ret = -EIO;
-		}
-
-		up_write(&pcc_ss_data->pcc_lock);
+	u64 auto_sel;
+	int ret;
 
+	ret = cppc_get_reg(cpunum, AUTO_SEL_ENABLE, &auto_sel);
+	if (ret)
 		return ret;
-	}
 
+	perf_caps->auto_sel = (bool)auto_sel;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(cppc_get_auto_sel_caps);
@@ -1594,43 +1614,7 @@ EXPORT_SYMBOL_GPL(cppc_get_auto_sel_caps);
  */
 int cppc_set_auto_sel(int cpu, bool enable)
 {
-	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
-	struct cpc_register_resource *auto_sel_reg;
-	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
-	struct cppc_pcc_data *pcc_ss_data = NULL;
-	int ret = -EINVAL;
-
-	if (!cpc_desc) {
-		pr_debug("No CPC descriptor for CPU:%d\n", cpu);
-		return -ENODEV;
-	}
-
-	auto_sel_reg = &cpc_desc->cpc_regs[AUTO_SEL_ENABLE];
-
-	if (CPC_IN_PCC(auto_sel_reg)) {
-		if (pcc_ss_id < 0) {
-			pr_debug("Invalid pcc_ss_id\n");
-			return -ENODEV;
-		}
-
-		if (CPC_SUPPORTED(auto_sel_reg)) {
-			ret = cpc_write(cpu, auto_sel_reg, enable);
-			if (ret)
-				return ret;
-		}
-
-		pcc_ss_data = pcc_data[pcc_ss_id];
-
-		down_write(&pcc_ss_data->pcc_lock);
-		/* after writing CPC, transfer the ownership of PCC to platform */
-		ret = send_pcc_cmd(pcc_ss_id, CMD_WRITE);
-		up_write(&pcc_ss_data->pcc_lock);
-	} else {
-		ret = -ENOTSUPP;
-		pr_debug("_CPC in PCC is not supported\n");
-	}
-
-	return ret;
+	return cppc_set_reg(cpu, AUTO_SEL_ENABLE, enable);
 }
 EXPORT_SYMBOL_GPL(cppc_set_auto_sel);
 
@@ -1644,38 +1628,7 @@ EXPORT_SYMBOL_GPL(cppc_set_auto_sel);
  */
 int cppc_set_enable(int cpu, bool enable)
 {
-	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
-	struct cpc_register_resource *enable_reg;
-	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
-	struct cppc_pcc_data *pcc_ss_data = NULL;
-	int ret = -EINVAL;
-
-	if (!cpc_desc) {
-		pr_debug("No CPC descriptor for CPU:%d\n", cpu);
-		return -EINVAL;
-	}
-
-	enable_reg = &cpc_desc->cpc_regs[ENABLE];
-
-	if (CPC_IN_PCC(enable_reg)) {
-
-		if (pcc_ss_id < 0)
-			return -EIO;
-
-		ret = cpc_write(cpu, enable_reg, enable);
-		if (ret)
-			return ret;
-
-		pcc_ss_data = pcc_data[pcc_ss_id];
-
-		down_write(&pcc_ss_data->pcc_lock);
-		/* after writing CPC, transfer the ownership of PCC to platfrom */
-		ret = send_pcc_cmd(pcc_ss_id, CMD_WRITE);
-		up_write(&pcc_ss_data->pcc_lock);
-		return ret;
-	}
-
-	return cpc_write(cpu, enable_reg, enable);
+	return cppc_set_reg(cpu, ENABLE, enable);
 }
 EXPORT_SYMBOL_GPL(cppc_set_enable);
 
-- 
2.33.0
Re: [PATCH 1/3] ACPI: CPPC: Refactor register get and set ABIs
Posted by Pierre Gondois 1 year, 2 months ago
Hello Lifeng,

On 11/14/24 09:48, Lifeng Zheng wrote:
> Refactor register get and set ABIs using cppc_get_reg() and cppc_set_reg().
> 
> Rename cppc_get_perf() to cppc_get_reg() as a generic function to read cppc
> registers, with two changes:
> 
> 1. Change the error kind to "no such device" when pcc_ss_id < 0, which
> means that this cpu cannot get a valid pcc_ss_id.
> 
> 2. Add a check to verify if the register is a cpc supported one before
> using it.
> 
> Add cppc_set_reg() as a generic function for setting cppc registers. Unlike
> other set reg ABIs, this function checks CPC_SUPPORTED right after getting
> the register, because the rest of the operations are meaningless if this
> register is not a cpc supported one.
> 
> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> ---
>   drivers/acpi/cppc_acpi.c | 191 +++++++++++++++------------------------
>   1 file changed, 72 insertions(+), 119 deletions(-)
> 
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index c1f3568d0c50..306ced9c3376 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1179,10 +1179,13 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
>   	return ret_val;
>   }
>   
> -static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
> +static int cppc_get_reg(int cpunum, enum cppc_regs reg_idx, u64 *val)
>   {
>   	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
> +	struct cppc_pcc_data *pcc_ss_data = NULL;
>   	struct cpc_register_resource *reg;
> +	int pcc_ss_id;
> +	int ret = 0;

NIT: Might not be necessary if we save the value returned by cpc_read(),
cf. other comment below.

>   
>   	if (!cpc_desc) {
>   		pr_debug("No CPC descriptor for CPU:%d\n", cpunum);
> @@ -1191,20 +1194,23 @@ static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
>   
>   	reg = &cpc_desc->cpc_regs[reg_idx];
>   
> +	if (!CPC_SUPPORTED(reg)) {
> +		pr_debug("CPC register (reg_idx=%u) is not supported\n", reg_idx);
> +		return -EOPNOTSUPP;
> +	}
> +
>   	if (CPC_IN_PCC(reg)) {
> -		int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
> -		struct cppc_pcc_data *pcc_ss_data = NULL;
> -		int ret = 0;
> +		pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
>   
>   		if (pcc_ss_id < 0)
> -			return -EIO;
> +			return -ENODEV;

NIT: Could add here:
   pr_debug("Invalid pcc_ss_id\n");
just as you did in cppc_set_reg()

>   
>   		pcc_ss_data = pcc_data[pcc_ss_id];
>   
>   		down_write(&pcc_ss_data->pcc_lock);
>   
>   		if (send_pcc_cmd(pcc_ss_id, CMD_READ) >= 0)
> -			cpc_read(cpunum, reg, perf);
> +			cpc_read(cpunum, reg, val);

This was not introduced by your patch, but cpc_read() return a value.
Shouldn't we return it instead of 0 ?

>   		else
>   			ret = -EIO;
>   
> @@ -1213,21 +1219,65 @@ static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
>   		return ret;
>   	}
>   
> -	cpc_read(cpunum, reg, perf);
> +	cpc_read(cpunum, reg, val);

Same comment as above

>   
>   	return 0;
>   }
>   
> +static int cppc_set_reg(int cpu, enum cppc_regs reg_idx, u64 val)

Just to have similar functions, maybe 'cpu' should be renamed to 'cpunum' ?
Or the other way around.

> +{
> +	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
> +	struct cppc_pcc_data *pcc_ss_data = NULL;
> +	struct cpc_register_resource *reg;
> +	int pcc_ss_id;
> +	int ret;
> +
> +	if (!cpc_desc) {
> +		pr_debug("No CPC descriptor for CPU:%d\n", cpu);
> +		return -ENODEV;
> +	}
> +
> +	reg = &cpc_desc->cpc_regs[reg_idx];
> +
> +	if (!CPC_SUPPORTED(reg)) {
> +		pr_debug("CPC register (reg_idx=%u) is not supported\n", reg_idx);
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (CPC_IN_PCC(reg)) {
> +		pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
> +
> +		if (pcc_ss_id < 0) {
> +			pr_debug("Invalid pcc_ss_id\n");
> +			return -ENODEV;
> +		}
> +
> +		ret = cpc_write(cpu, reg, val);
> +		if (ret)
> +			return ret;
> +
> +		pcc_ss_data = pcc_data[pcc_ss_id];
> +
> +		down_write(&pcc_ss_data->pcc_lock);
> +		/* after writing CPC, transfer the ownership of PCC to platform */
> +		ret = send_pcc_cmd(pcc_ss_id, CMD_WRITE);
> +		up_write(&pcc_ss_data->pcc_lock);
> +		return ret;
> +	}
> +
> +	return cpc_write(cpu, reg, val);
> +}
> +

[snip]
Re: [PATCH 1/3] ACPI: CPPC: Refactor register get and set ABIs
Posted by zhenglifeng (A) 1 year, 2 months ago
Hello Pierre,

On 2024/12/6 22:23, Pierre Gondois wrote:
> Hello Lifeng,
> 
> On 11/14/24 09:48, Lifeng Zheng wrote:
>> Refactor register get and set ABIs using cppc_get_reg() and cppc_set_reg().
>>
>> Rename cppc_get_perf() to cppc_get_reg() as a generic function to read cppc
>> registers, with two changes:
>>
>> 1. Change the error kind to "no such device" when pcc_ss_id < 0, which
>> means that this cpu cannot get a valid pcc_ss_id.
>>
>> 2. Add a check to verify if the register is a cpc supported one before
>> using it.
>>
>> Add cppc_set_reg() as a generic function for setting cppc registers. Unlike
>> other set reg ABIs, this function checks CPC_SUPPORTED right after getting
>> the register, because the rest of the operations are meaningless if this
>> register is not a cpc supported one.
>>
>> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
>> ---
>>   drivers/acpi/cppc_acpi.c | 191 +++++++++++++++------------------------
>>   1 file changed, 72 insertions(+), 119 deletions(-)
>>
>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>> index c1f3568d0c50..306ced9c3376 100644
>> --- a/drivers/acpi/cppc_acpi.c
>> +++ b/drivers/acpi/cppc_acpi.c
>> @@ -1179,10 +1179,13 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
>>       return ret_val;
>>   }
>>   -static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
>> +static int cppc_get_reg(int cpunum, enum cppc_regs reg_idx, u64 *val)
>>   {
>>       struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
>> +    struct cppc_pcc_data *pcc_ss_data = NULL;
>>       struct cpc_register_resource *reg;
>> +    int pcc_ss_id;
>> +    int ret = 0;
> 
> NIT: Might not be necessary if we save the value returned by cpc_read(),
> cf. other comment below.
> 
>>         if (!cpc_desc) {
>>           pr_debug("No CPC descriptor for CPU:%d\n", cpunum);
>> @@ -1191,20 +1194,23 @@ static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
>>         reg = &cpc_desc->cpc_regs[reg_idx];
>>   +    if (!CPC_SUPPORTED(reg)) {
>> +        pr_debug("CPC register (reg_idx=%u) is not supported\n", reg_idx);
>> +        return -EOPNOTSUPP;
>> +    }
>> +
>>       if (CPC_IN_PCC(reg)) {
>> -        int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
>> -        struct cppc_pcc_data *pcc_ss_data = NULL;
>> -        int ret = 0;
>> +        pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
>>             if (pcc_ss_id < 0)
>> -            return -EIO;
>> +            return -ENODEV;
> 
> NIT: Could add here:
>   pr_debug("Invalid pcc_ss_id\n");
> just as you did in cppc_set_reg()

Will add it in next version, Thanks.

> 
>>             pcc_ss_data = pcc_data[pcc_ss_id];
>>             down_write(&pcc_ss_data->pcc_lock);
>>             if (send_pcc_cmd(pcc_ss_id, CMD_READ) >= 0)
>> -            cpc_read(cpunum, reg, perf);
>> +            cpc_read(cpunum, reg, val);
> 
> This was not introduced by your patch, but cpc_read() return a value.
> Shouldn't we return it instead of 0 ?

Indeed. Will optimize it, Thanks.

> 
>>           else
>>               ret = -EIO;
>>   @@ -1213,21 +1219,65 @@ static int cppc_get_perf(int cpunum, enum cppc_regs reg_idx, u64 *perf)
>>           return ret;
>>       }
>>   -    cpc_read(cpunum, reg, perf);
>> +    cpc_read(cpunum, reg, val);
> 
> Same comment as above
> 
>>         return 0;
>>   }
>>   +static int cppc_set_reg(int cpu, enum cppc_regs reg_idx, u64 val)
> 
> Just to have similar functions, maybe 'cpu' should be renamed to 'cpunum' ?
> Or the other way around.

I prefer 'cpu', 'cpunum' looks like the number of cpus to me.
Will rename 'cpunum' to 'cpu' in cppc_get_reg(). Thanks.

> 
>> +{
>> +    struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
>> +    struct cppc_pcc_data *pcc_ss_data = NULL;
>> +    struct cpc_register_resource *reg;
>> +    int pcc_ss_id;
>> +    int ret;
>> +
>> +    if (!cpc_desc) {
>> +        pr_debug("No CPC descriptor for CPU:%d\n", cpu);
>> +        return -ENODEV;
>> +    }
>> +
>> +    reg = &cpc_desc->cpc_regs[reg_idx];
>> +
>> +    if (!CPC_SUPPORTED(reg)) {
>> +        pr_debug("CPC register (reg_idx=%u) is not supported\n", reg_idx);
>> +        return -EOPNOTSUPP;
>> +    }
>> +
>> +    if (CPC_IN_PCC(reg)) {
>> +        pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
>> +
>> +        if (pcc_ss_id < 0) {
>> +            pr_debug("Invalid pcc_ss_id\n");
>> +            return -ENODEV;
>> +        }
>> +
>> +        ret = cpc_write(cpu, reg, val);
>> +        if (ret)
>> +            return ret;
>> +
>> +        pcc_ss_data = pcc_data[pcc_ss_id];
>> +
>> +        down_write(&pcc_ss_data->pcc_lock);
>> +        /* after writing CPC, transfer the ownership of PCC to platform */
>> +        ret = send_pcc_cmd(pcc_ss_id, CMD_WRITE);
>> +        up_write(&pcc_ss_data->pcc_lock);
>> +        return ret;
>> +    }
>> +
>> +    return cpc_write(cpu, reg, val);
>> +}
>> +
> 
> [snip]
>