Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <2a94b361-2dff-e673-ba54-b4890d2bdaf8@linaro.org>
Date:   Thu, 6 Apr 2023 20:43:14 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH 2/3] thermal/drivers/tsens: Describe sensor registers via
 a structure
Content-Language: en-GB
To:     Bryan O'Donoghue <bryan.odonoghue@linaro.org>, amitk@kernel.org,
        thara.gopinath@gmail.com, agross@kernel.org, andersson@kernel.org,
        konrad.dybcio@linaro.org, rafael@kernel.org,
        daniel.lezcano@linaro.org, rui.zhang@intel.com
Cc:     linux-pm@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20230406145850.357296-1-bryan.odonoghue@linaro.org>
 <20230406145850.357296-3-bryan.odonoghue@linaro.org>
From:   Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
In-Reply-To: <20230406145850.357296-3-bryan.odonoghue@linaro.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 06/04/2023 17:58, Bryan O'Donoghue wrote:
> Define sensor identifiers and optional shifts in a single data-structure.
> This facilitates extraction of calibration data from non-contiguous
> addresses.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   drivers/thermal/qcom/tsens-v0_1.c | 56 +++++++++++++++++++++++++++++--
>   drivers/thermal/qcom/tsens.c      |  5 +--
>   drivers/thermal/qcom/tsens.h      | 16 ++++++++-
>   3 files changed, 72 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c
> index e89c6f39a3aea..c20c002d98650 100644
> --- a/drivers/thermal/qcom/tsens-v0_1.c
> +++ b/drivers/thermal/qcom/tsens-v0_1.c
> @@ -319,10 +319,31 @@ static const struct tsens_ops ops_8916 = {
>   	.get_temp	= get_temp_common,
>   };
>   
> +struct tsens_reg_data reg_8916[] = {
> +	{
> +		.id = 0,
> +	},
> +	{
> +		.id = 1,
> +	},
> +	{
> +		.id = 2,
> +	},
> +	{
> +		.id = 3,
> +	},
> +	{
> +		.id = 4,
> +	},
> +	{
> +		.id = 5,
> +	},
> +};
> +
>   struct tsens_plat_data data_8916 = {
>   	.num_sensors	= 5,
>   	.ops		= &ops_8916,
> -	.hw_ids		= (unsigned int []){0, 1, 2, 4, 5 },
> +	.reg		= reg_8916,
>   
>   	.feat		= &tsens_v0_1_feat,
>   	.fields	= tsens_v0_1_regfields,
> @@ -334,10 +355,41 @@ static const struct tsens_ops ops_8939 = {
>   	.get_temp	= get_temp_common,
>   };
>   
> +struct tsens_reg_data reg_8939[] = {
> +	{
> +		.id = 0,
> +	},
> +	{
> +		.id = 1,
> +	},
> +	{
> +		.id = 2,
> +	},
> +	{
> +		.id = 3,
> +	},
> +	{
> +		.id = 5,
> +	},
> +	{
> +		.id = 6,
> +	},
> +	{
> +		.id = 7,
> +	},
> +	{
> +		.id = 8,
> +	},
> +	{
> +		.id = 9,

Sensor 9 is contiguous. It's sensor with hwid=10, who requires two reads.

> +		.p2_shift = 8,
> +	},
> +};
> +
>   struct tsens_plat_data data_8939 = {
>   	.num_sensors	= 9,
>   	.ops		= &ops_8939,
> -	.hw_ids		= (unsigned int []){ 0, 1, 2, 3, 5, 6, 7, 8, 9, /* 10 */ },
> +	.reg		= reg_8939,
>   
>   	.feat		= &tsens_v0_1_feat,
>   	.fields	= tsens_v0_1_regfields,
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index 7165b0bfe8b9f..a260f563b4889 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -1274,13 +1274,14 @@ static int tsens_probe(struct platform_device *pdev)
>   	priv->num_sensors = num_sensors;
>   	priv->ops = data->ops;
>   	for (i = 0;  i < priv->num_sensors; i++) {
> -		if (data->hw_ids)
> -			priv->sensor[i].hw_id = data->hw_ids[i];
> +		if (data->reg)
> +			priv->sensor[i].hw_id = data->reg[i].id;
>   		else
>   			priv->sensor[i].hw_id = i;
>   	}
>   	priv->feat = data->feat;
>   	priv->fields = data->fields;
> +	priv->reg = data->reg;
>   
>   	platform_set_drvdata(pdev, priv);
>   
> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> index dba9cd38f637c..31f67da03bce6 100644
> --- a/drivers/thermal/qcom/tsens.h
> +++ b/drivers/thermal/qcom/tsens.h
> @@ -517,18 +517,31 @@ struct tsens_features {
>   	int trip_max_temp;
>   };
>   
> +/**
> + * struct tsens_reg_data - describes register data retrieved non-contiguously
> + * @id: thermal sensor identifier
> + * @p1_shift: When non-zero is the # of bits to right shift p1 MSB by
> + * @p2_shift: When non-zero is the # of bits to right shift p2 MSB by
> + */
> +struct tsens_reg_data {
> +	unsigned int id;
> +	unsigned int p1_shift;
> +	unsigned int p2_shift;
> +};
> +
>   /**
>    * struct tsens_plat_data - tsens compile-time platform data
>    * @num_sensors: Number of sensors supported by platform
>    * @ops: operations the tsens instance supports
>    * @hw_ids: Subset of sensors ids supported by platform, if not the first n
> + * @reg: Describe sensor id and calibration shifts
>    * @feat: features of the IP
>    * @fields: bitfield locations
>    */
>   struct tsens_plat_data {
>   	const u32		num_sensors;
>   	const struct tsens_ops	*ops;
> -	unsigned int		*hw_ids;
> +	struct tsens_reg_data	*reg;
>   	struct tsens_features	*feat;
>   	const struct reg_field		*fields;
>   };
> @@ -575,6 +588,7 @@ struct tsens_priv {
>   	struct regmap_field		*rf[MAX_REGFIELDS];
>   	struct tsens_context		ctx;
>   	struct tsens_features		*feat;
> +	struct tsens_reg_data		*reg;
>   	const struct reg_field		*fields;
>   	const struct tsens_ops		*ops;
>   

-- 
With best wishes
Dmitry