diff mbox series

[v5,2/5] soc: qcom: llcc: Add regmap for Broadcast_AND region

Message ID 2c7654492ee436b41acddf2edc65d6722c3ad6aa.1716228054.git.quic_uchalich@quicinc.com (mailing list archive)
State Superseded
Headers show
Series LLCC: Support for Broadcast_AND region | expand

Commit Message

Unnathi Chalicheemala May 20, 2024, 9 p.m. UTC 
Define new regmap structure for Broadcast_AND region and initialize
this regmap when HW block version is greater than 4.1, otherwise
initialize as a NULL pointer for backwards compatibility.

Switch from broadcast_OR to broadcast_AND region (when defined in DT)
for checking status bit 1 as Broadcast_OR region checks only for bit 0.

Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com>
Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 drivers/soc/qcom/llcc-qcom.c       | 16 +++++++++++++++-
 include/linux/soc/qcom/llcc-qcom.h |  4 +++-
 2 files changed, 18 insertions(+), 2 deletions(-)

Comments

Bjorn Andersson May 27, 2024, 3:11 a.m. UTC | #1 
On Mon, May 20, 2024 at 02:00:14PM GMT, Unnathi Chalicheemala wrote:
> Define new regmap structure for Broadcast_AND region and initialize
> this regmap when HW block version is greater than 4.1, otherwise
> initialize as a NULL pointer for backwards compatibility.
> 
> Switch from broadcast_OR to broadcast_AND region (when defined in DT)
> for checking status bit 1 as Broadcast_OR region checks only for bit 0.
> 

This is a good technical description of the change you're making. But
it's been long enough since we discussed this that I've forgotten which
problem it solves, and the commit message doesn't tell me.

Please add a paragraph on the top describing the actual problem this
solves?

Regards,
Bjorn

> Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com>
> Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  drivers/soc/qcom/llcc-qcom.c       | 16 +++++++++++++++-
>  include/linux/soc/qcom/llcc-qcom.h |  4 +++-
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
> index cbef0dea1d5d..5eac6aa567e7 100644
> --- a/drivers/soc/qcom/llcc-qcom.c
> +++ b/drivers/soc/qcom/llcc-qcom.c
> @@ -821,6 +821,7 @@ EXPORT_SYMBOL_GPL(llcc_slice_putd);
>  static int llcc_update_act_ctrl(u32 sid,
>  				u32 act_ctrl_reg_val, u32 status)
>  {
> +	struct regmap *regmap;
>  	u32 act_ctrl_reg;
>  	u32 act_clear_reg;
>  	u32 status_reg;
> @@ -849,7 +850,8 @@ static int llcc_update_act_ctrl(u32 sid,
>  		return ret;
>  
>  	if (drv_data->version >= LLCC_VERSION_4_1_0_0) {
> -		ret = regmap_read_poll_timeout(drv_data->bcast_regmap, status_reg,
> +		regmap = drv_data->bcast_and_regmap ?: drv_data->bcast_regmap;
> +		ret = regmap_read_poll_timeout(regmap, status_reg,
>  				      slice_status, (slice_status & ACT_COMPLETE),
>  				      0, LLCC_STATUS_READ_DELAY);
>  		if (ret)
> @@ -1284,6 +1286,18 @@ static int qcom_llcc_probe(struct platform_device *pdev)
>  
>  	drv_data->version = version;
>  
> +	/* Applicable only when drv_data->version >= 4.1 */
> +	if (drv_data->version >= LLCC_VERSION_4_1_0_0) {
> +		drv_data->bcast_and_regmap = qcom_llcc_init_mmio(pdev, i + 1, "llcc_broadcast_and_base");
> +		if (IS_ERR(drv_data->bcast_and_regmap)) {
> +			ret = PTR_ERR(drv_data->bcast_and_regmap);
> +			if (ret == -EINVAL)
> +				drv_data->bcast_and_regmap = NULL;
> +			else
> +				goto err;
> +		}
> +	}
> +
>  	llcc_cfg = cfg->sct_data;
>  	sz = cfg->size;
>  
> diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h
> index 1a886666bbb6..9e9f528b1370 100644
> --- a/include/linux/soc/qcom/llcc-qcom.h
> +++ b/include/linux/soc/qcom/llcc-qcom.h
> @@ -115,7 +115,8 @@ struct llcc_edac_reg_offset {
>  /**
>   * struct llcc_drv_data - Data associated with the llcc driver
>   * @regmaps: regmaps associated with the llcc device
> - * @bcast_regmap: regmap associated with llcc broadcast offset
> + * @bcast_regmap: regmap associated with llcc broadcast OR offset
> + * @bcast_and_regmap: regmap associated with llcc broadcast AND offset
>   * @cfg: pointer to the data structure for slice configuration
>   * @edac_reg_offset: Offset of the LLCC EDAC registers
>   * @lock: mutex associated with each slice
> @@ -129,6 +130,7 @@ struct llcc_edac_reg_offset {
>  struct llcc_drv_data {
>  	struct regmap **regmaps;
>  	struct regmap *bcast_regmap;
> +	struct regmap *bcast_and_regmap;
>  	const struct llcc_slice_config *cfg;
>  	const struct llcc_edac_reg_offset *edac_reg_offset;
>  	struct mutex lock;
> -- 
> 2.34.1
>
Unnathi Chalicheemala May 29, 2024, 5:09 p.m. UTC | #2 
On 5/26/2024 8:11 PM, Bjorn Andersson wrote:
> On Mon, May 20, 2024 at 02:00:14PM GMT, Unnathi Chalicheemala wrote:
>> Define new regmap structure for Broadcast_AND region and initialize
>> this regmap when HW block version is greater than 4.1, otherwise
>> initialize as a NULL pointer for backwards compatibility.
>>
>> Switch from broadcast_OR to broadcast_AND region (when defined in DT)
>> for checking status bit 1 as Broadcast_OR region checks only for bit 0.
>>
> 
> This is a good technical description of the change you're making. But
> it's been long enough since we discussed this that I've forgotten which
> problem it solves, and the commit message doesn't tell me.
> 
> Please add a paragraph on the top describing the actual problem this
> solves?
>
Yes understood, I'll append this to the commit message:

To support CSR programming, a broadcast interface is used to program all
channels in a single command. Until SM8450 there was only one broadcast
region (Broadcast_OR) used to broadcast write and check for status bit
0.
From SM8450 onwards another broadcast region (Broadcast_AND) has been
added which checks for status bit 1. This hasn't been updated and
Broadcast_OR region was wrongly being used to check for status bit 1 all
along.

Thank you Bjorn.
> Regards,
> Bjorn
> 
>> Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com>
>> Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com>
>> ---
>>  drivers/soc/qcom/llcc-qcom.c       | 16 +++++++++++++++-
>>  include/linux/soc/qcom/llcc-qcom.h |  4 +++-
>>  2 files changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
>> index cbef0dea1d5d..5eac6aa567e7 100644
>> --- a/drivers/soc/qcom/llcc-qcom.c
>> +++ b/drivers/soc/qcom/llcc-qcom.c
>> @@ -821,6 +821,7 @@ EXPORT_SYMBOL_GPL(llcc_slice_putd);
>>  static int llcc_update_act_ctrl(u32 sid,
>>  				u32 act_ctrl_reg_val, u32 status)
>>  {
>> +	struct regmap *regmap;
>>  	u32 act_ctrl_reg;
>>  	u32 act_clear_reg;
>>  	u32 status_reg;
>> @@ -849,7 +850,8 @@ static int llcc_update_act_ctrl(u32 sid,
>>  		return ret;
>>  
>>  	if (drv_data->version >= LLCC_VERSION_4_1_0_0) {
>> -		ret = regmap_read_poll_timeout(drv_data->bcast_regmap, status_reg,
>> +		regmap = drv_data->bcast_and_regmap ?: drv_data->bcast_regmap;
>> +		ret = regmap_read_poll_timeout(regmap, status_reg,
>>  				      slice_status, (slice_status & ACT_COMPLETE),
>>  				      0, LLCC_STATUS_READ_DELAY);
>>  		if (ret)
>> @@ -1284,6 +1286,18 @@ static int qcom_llcc_probe(struct platform_device *pdev)
>>  
>>  	drv_data->version = version;
>>  
>> +	/* Applicable only when drv_data->version >= 4.1 */
>> +	if (drv_data->version >= LLCC_VERSION_4_1_0_0) {
>> +		drv_data->bcast_and_regmap = qcom_llcc_init_mmio(pdev, i + 1, "llcc_broadcast_and_base");
>> +		if (IS_ERR(drv_data->bcast_and_regmap)) {
>> +			ret = PTR_ERR(drv_data->bcast_and_regmap);
>> +			if (ret == -EINVAL)
>> +				drv_data->bcast_and_regmap = NULL;
>> +			else
>> +				goto err;
>> +		}
>> +	}
>> +
>>  	llcc_cfg = cfg->sct_data;
>>  	sz = cfg->size;
>>  
>> diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h
>> index 1a886666bbb6..9e9f528b1370 100644
>> --- a/include/linux/soc/qcom/llcc-qcom.h
>> +++ b/include/linux/soc/qcom/llcc-qcom.h
>> @@ -115,7 +115,8 @@ struct llcc_edac_reg_offset {
>>  /**
>>   * struct llcc_drv_data - Data associated with the llcc driver
>>   * @regmaps: regmaps associated with the llcc device
>> - * @bcast_regmap: regmap associated with llcc broadcast offset
>> + * @bcast_regmap: regmap associated with llcc broadcast OR offset
>> + * @bcast_and_regmap: regmap associated with llcc broadcast AND offset
>>   * @cfg: pointer to the data structure for slice configuration
>>   * @edac_reg_offset: Offset of the LLCC EDAC registers
>>   * @lock: mutex associated with each slice
>> @@ -129,6 +130,7 @@ struct llcc_edac_reg_offset {
>>  struct llcc_drv_data {
>>  	struct regmap **regmaps;
>>  	struct regmap *bcast_regmap;
>> +	struct regmap *bcast_and_regmap;
>>  	const struct llcc_slice_config *cfg;
>>  	const struct llcc_edac_reg_offset *edac_reg_offset;
>>  	struct mutex lock;
>> -- 
>> 2.34.1
>>
diff mbox series

Patch

diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index cbef0dea1d5d..5eac6aa567e7 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -821,6 +821,7 @@  EXPORT_SYMBOL_GPL(llcc_slice_putd);
 static int llcc_update_act_ctrl(u32 sid,
 				u32 act_ctrl_reg_val, u32 status)
 {
+	struct regmap *regmap;
 	u32 act_ctrl_reg;
 	u32 act_clear_reg;
 	u32 status_reg;
@@ -849,7 +850,8 @@  static int llcc_update_act_ctrl(u32 sid,
 		return ret;
 
 	if (drv_data->version >= LLCC_VERSION_4_1_0_0) {
-		ret = regmap_read_poll_timeout(drv_data->bcast_regmap, status_reg,
+		regmap = drv_data->bcast_and_regmap ?: drv_data->bcast_regmap;
+		ret = regmap_read_poll_timeout(regmap, status_reg,
 				      slice_status, (slice_status & ACT_COMPLETE),
 				      0, LLCC_STATUS_READ_DELAY);
 		if (ret)
@@ -1284,6 +1286,18 @@  static int qcom_llcc_probe(struct platform_device *pdev)
 
 	drv_data->version = version;
 
+	/* Applicable only when drv_data->version >= 4.1 */
+	if (drv_data->version >= LLCC_VERSION_4_1_0_0) {
+		drv_data->bcast_and_regmap = qcom_llcc_init_mmio(pdev, i + 1, "llcc_broadcast_and_base");
+		if (IS_ERR(drv_data->bcast_and_regmap)) {
+			ret = PTR_ERR(drv_data->bcast_and_regmap);
+			if (ret == -EINVAL)
+				drv_data->bcast_and_regmap = NULL;
+			else
+				goto err;
+		}
+	}
+
 	llcc_cfg = cfg->sct_data;
 	sz = cfg->size;
 
diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h
index 1a886666bbb6..9e9f528b1370 100644
--- a/include/linux/soc/qcom/llcc-qcom.h
+++ b/include/linux/soc/qcom/llcc-qcom.h
@@ -115,7 +115,8 @@  struct llcc_edac_reg_offset {
 /**
  * struct llcc_drv_data - Data associated with the llcc driver
  * @regmaps: regmaps associated with the llcc device
- * @bcast_regmap: regmap associated with llcc broadcast offset
+ * @bcast_regmap: regmap associated with llcc broadcast OR offset
+ * @bcast_and_regmap: regmap associated with llcc broadcast AND offset
  * @cfg: pointer to the data structure for slice configuration
  * @edac_reg_offset: Offset of the LLCC EDAC registers
  * @lock: mutex associated with each slice
@@ -129,6 +130,7 @@  struct llcc_edac_reg_offset {
 struct llcc_drv_data {
 	struct regmap **regmaps;
 	struct regmap *bcast_regmap;
+	struct regmap *bcast_and_regmap;
 	const struct llcc_slice_config *cfg;
 	const struct llcc_edac_reg_offset *edac_reg_offset;
 	struct mutex lock;