diff mbox series

[v4,12/12] interconnect: qcom: icc-rpm: Allow negative QoS offset

Message ID 20230214143720.2416762-13-konrad.dybcio@linaro.org
State New
Headers
Series [v4,01/12] interconnect: qcom: rpm: make QoS INVALID default, separate out driver data |

Commit Message

Konrad Dybcio Feb. 14, 2023, 2:37 p.m. UTC 
  In some very very very very unfortunate cases, the correct offset of
the QoS registers will be.. negative. One such case is MSM8998, where
The DDR BWMON occupies what-would-be-the-BIMC-base which we usually
take into account with the register calculation, making the actual
BIMC node start at what-would-be-the-BIMC-base+0x300.

In order to keep the calculation code sane, the simplest - however
ugly it may be - solution is to allow the offset to be negative.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/interconnect/qcom/icc-rpm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Dmitry Baryshkov Feb. 15, 2023, 1:16 a.m. UTC | #1 
On 14/02/2023 16:37, Konrad Dybcio wrote:
> In some very very very very unfortunate cases, the correct offset of
> the QoS registers will be.. negative. One such case is MSM8998, where
> The DDR BWMON occupies what-would-be-the-BIMC-base which we usually
> take into account with the register calculation, making the actual
> BIMC node start at what-would-be-the-BIMC-base+0x300.
> 
> In order to keep the calculation code sane, the simplest - however
> ugly it may be - solution is to allow the offset to be negative.

I'm not sure how does this work, as e.g. qcom_icc_set_qnoc_qos() will 
try to access an address before the first register. Most probably this 
patch should go together with the rest of msm8998 patches.

A slightly better solution to making qos_offset negative might be to 
make bwmon a child node of the icc.

> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>   drivers/interconnect/qcom/icc-rpm.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/interconnect/qcom/icc-rpm.h b/drivers/interconnect/qcom/icc-rpm.h
> index 77e263b93c27..5e4215f25c2e 100644
> --- a/drivers/interconnect/qcom/icc-rpm.h
> +++ b/drivers/interconnect/qcom/icc-rpm.h
> @@ -39,7 +39,7 @@ struct qcom_icc_provider {
>   	int num_intf_clks;
>   	enum qcom_icc_type type;
>   	struct regmap *regmap;
> -	unsigned int qos_offset;
> +	int qos_offset;
>   	u64 bus_clk_rate[2];
>   	bool keep_alive;
>   	struct clk_bulk_data bus_clks[2];
> @@ -105,7 +105,7 @@ struct qcom_icc_desc {
>   	bool keep_alive;
>   	enum qcom_icc_type type;
>   	const struct regmap_config *regmap_cfg;
> -	unsigned int qos_offset;
> +	int qos_offset;
>   };
>   
>   /* Valid for all bus types */
Konrad Dybcio Feb. 15, 2023, 1:31 a.m. UTC | #2 
On 15.02.2023 02:16, Dmitry Baryshkov wrote:
> On 14/02/2023 16:37, Konrad Dybcio wrote:
>> In some very very very very unfortunate cases, the correct offset of
>> the QoS registers will be.. negative. One such case is MSM8998, where
>> The DDR BWMON occupies what-would-be-the-BIMC-base which we usually
>> take into account with the register calculation, making the actual
>> BIMC node start at what-would-be-the-BIMC-base+0x300.
>>
>> In order to keep the calculation code sane, the simplest - however
>> ugly it may be - solution is to allow the offset to be negative.
> 
> I'm not sure how does this work, as e.g. qcom_icc_set_qnoc_qos() will try to access an address before the first register. Most probably this patch should go together with the rest of msm8998 patches.
In our case BIMC is the offender and its defines all base on
M_BKE_REG_BASE(n) (0x300 + (0x4000 * n)), so removing 0x300
is perfectly ok.

> 
> A slightly better solution to making qos_offset negative might be to make bwmon a child node of the icc.
That sounds like a lot of complex changes and accounting for old
/ different DTs who don't have the bwmon as a child of icc :/

Konrad
> 
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>>   drivers/interconnect/qcom/icc-rpm.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/interconnect/qcom/icc-rpm.h b/drivers/interconnect/qcom/icc-rpm.h
>> index 77e263b93c27..5e4215f25c2e 100644
>> --- a/drivers/interconnect/qcom/icc-rpm.h
>> +++ b/drivers/interconnect/qcom/icc-rpm.h
>> @@ -39,7 +39,7 @@ struct qcom_icc_provider {
>>       int num_intf_clks;
>>       enum qcom_icc_type type;
>>       struct regmap *regmap;
>> -    unsigned int qos_offset;
>> +    int qos_offset;
>>       u64 bus_clk_rate[2];
>>       bool keep_alive;
>>       struct clk_bulk_data bus_clks[2];
>> @@ -105,7 +105,7 @@ struct qcom_icc_desc {
>>       bool keep_alive;
>>       enum qcom_icc_type type;
>>       const struct regmap_config *regmap_cfg;
>> -    unsigned int qos_offset;
>> +    int qos_offset;
>>   };
>>     /* Valid for all bus types */
>
diff mbox series

Patch

diff --git a/drivers/interconnect/qcom/icc-rpm.h b/drivers/interconnect/qcom/icc-rpm.h
index 77e263b93c27..5e4215f25c2e 100644
--- a/drivers/interconnect/qcom/icc-rpm.h
+++ b/drivers/interconnect/qcom/icc-rpm.h
@@ -39,7 +39,7 @@  struct qcom_icc_provider {
 	int num_intf_clks;
 	enum qcom_icc_type type;
 	struct regmap *regmap;
-	unsigned int qos_offset;
+	int qos_offset;
 	u64 bus_clk_rate[2];
 	bool keep_alive;
 	struct clk_bulk_data bus_clks[2];
@@ -105,7 +105,7 @@  struct qcom_icc_desc {
 	bool keep_alive;
 	enum qcom_icc_type type;
 	const struct regmap_config *regmap_cfg;
-	unsigned int qos_offset;
+	int qos_offset;
 };
 
 /* Valid for all bus types */