Re: [PATCH] OPP: debugfs: Use performance level to distinguish between rates

[Viresh Kumar]

On 29-01-26, 23:03, Manivannan Sadhasivam wrote:
> Some OPP tables have entries with same rate and different performance
> level. For these entries, using only the rate as the debugfs directory name
> causes below error:
> 
> debugfs: 'opp:5000000' already exists in 'soc@xxxxxxxxxxxxx'
> 
> Fix it by appending the performance level to the dir name.
> 
> Reported-by: Bjorn Andersson <andersson@xxxxxxxxxx>
> Closes: https://lore.kernel.org/linux-arm-msm/75lzykd37zdvrks5i2bb4zb2yzjtm25kv3hegmikndkbr772mz@w2ykff3ny45u/
> Fixes: 05db35963eef ("OPP: Add support to find OPP for a set of keys")
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
> ---
>  drivers/opp/debugfs.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/opp/debugfs.c b/drivers/opp/debugfs.c
> index 8fc6238b1728..7c912bae2a5a 100644
> --- a/drivers/opp/debugfs.c
> +++ b/drivers/opp/debugfs.c
> @@ -131,21 +131,24 @@ void opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table)
>  	struct dentry *pdentry = opp_table->dentry;
>  	struct dentry *d;
>  	unsigned long id;
> -	char name[25];	/* 20 chars for 64 bit value + 5 (opp:\0) */
> +	char name[36];	/* "opp:"(4) + u64(20) + "-" (1) + u32(10) + NULL(1) */
>  
>  	/*
>  	 * Get directory name for OPP.
>  	 *
> -	 * - Normally rate is unique to each OPP, use it to get unique opp-name.
> +	 * - Normally rate is unique to each OPP, use it to get unique opp-name,
> +	 *   together with performance level.
>  	 * - For some devices rate isn't available or there are multiple, use
>  	 *   index instead for them.
>  	 */
> -	if (likely(opp_table->clk_count == 1 && opp->rates[0]))
> +	if (likely(opp_table->clk_count == 1 && opp->rates[0])) {
>  		id = opp->rates[0];
> -	else
> +		snprintf(name, sizeof(name), "opp:%lu-%u", id, opp->level);

This would be confusing for devices that don't have the level
property. Maybe add the second part only if level is used.

-- 
viresh