diff mbox series

[06/20] interconnect: qcom: icc-rpm: Allow negative QoS offset

Message ID 20230526-topic-smd_icc-v1-6-1bf8e6663c4e@linaro.org
State New
Headers
Series Restructure RPM SMD ICC |

Commit Message

Konrad Dybcio May 30, 2023, 10:20 a.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 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Dmitry Baryshkov June 1, 2023, 9:56 a.m. UTC | #1 
On 30/05/2023 13:20, 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.

Can we turn one of devices into a child of another device? This way we 
won't have to cope with negative offsets.

> 
> 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 | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/interconnect/qcom/icc-rpm.h b/drivers/interconnect/qcom/icc-rpm.h
> index d2c04c400cad..ba840a436cc0 100644
> --- a/drivers/interconnect/qcom/icc-rpm.h
> +++ b/drivers/interconnect/qcom/icc-rpm.h
> @@ -29,10 +29,10 @@ enum qcom_icc_type {
>    * @num_intf_clks: the total number of intf_clks clk_bulk_data entries
>    * @type: the ICC provider type
>    * @regmap: regmap for QoS registers read/write access
> - * @qos_offset: offset to QoS registers
>    * @bus_clk_rate: bus clock rate in Hz
>    * @bus_clks: the clk_bulk_data table of bus clocks
>    * @intf_clks: a clk_bulk_data array of interface clocks
> + * @qos_offset: offset to QoS registers

This can be kept in place.

>    * @keep_alive: whether to always keep a minimum vote on the bus clocks
>    * @is_on: whether the bus is powered on
>    */
> @@ -42,7 +42,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[NUM_BUS_CLKS];
>   	struct clk_bulk_data bus_clks[NUM_BUS_CLKS];
>   	struct clk_bulk_data *intf_clks;
> @@ -108,7 +108,7 @@ struct qcom_icc_desc {
>   	bool no_clk_scaling;
>   	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 June 1, 2023, 9:59 a.m. UTC | #2 
On 1.06.2023 11:56, Dmitry Baryshkov wrote:
> On 30/05/2023 13:20, 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.
> 
> Can we turn one of devices into a child of another device? This way we won't have to cope with negative offsets.
Let's try to get more insight how they're correlated, but that sounds
like an option.. 8996 will need this as well..

Konrad
> 
>>
>> 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 | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/interconnect/qcom/icc-rpm.h b/drivers/interconnect/qcom/icc-rpm.h
>> index d2c04c400cad..ba840a436cc0 100644
>> --- a/drivers/interconnect/qcom/icc-rpm.h
>> +++ b/drivers/interconnect/qcom/icc-rpm.h
>> @@ -29,10 +29,10 @@ enum qcom_icc_type {
>>    * @num_intf_clks: the total number of intf_clks clk_bulk_data entries
>>    * @type: the ICC provider type
>>    * @regmap: regmap for QoS registers read/write access
>> - * @qos_offset: offset to QoS registers
>>    * @bus_clk_rate: bus clock rate in Hz
>>    * @bus_clks: the clk_bulk_data table of bus clocks
>>    * @intf_clks: a clk_bulk_data array of interface clocks
>> + * @qos_offset: offset to QoS registers
> 
> This can be kept in place.
> 
>>    * @keep_alive: whether to always keep a minimum vote on the bus clocks
>>    * @is_on: whether the bus is powered on
>>    */
>> @@ -42,7 +42,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[NUM_BUS_CLKS];
>>       struct clk_bulk_data bus_clks[NUM_BUS_CLKS];
>>       struct clk_bulk_data *intf_clks;
>> @@ -108,7 +108,7 @@ struct qcom_icc_desc {
>>       bool no_clk_scaling;
>>       enum qcom_icc_type type;
>>       const struct regmap_config *regmap_cfg;
>> -    unsigned int qos_offset;
>> +    int qos_offset;
>>   };
>>     /* Valid for all bus types */
>>
>
Dmitry Baryshkov June 1, 2023, 10:01 a.m. UTC | #3 
On 01/06/2023 12:59, Konrad Dybcio wrote:
> 
> 
> On 1.06.2023 11:56, Dmitry Baryshkov wrote:
>> On 30/05/2023 13:20, 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.
>>
>> Can we turn one of devices into a child of another device? This way we won't have to cope with negative offsets.
> Let's try to get more insight how they're correlated, but that sounds
> like an option.. 8996 will need this as well..

Anyway, I think this commit should be moved closer to the actual use.

> 
> Konrad
>>
>>>
>>> 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 | 6 +++---
>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/interconnect/qcom/icc-rpm.h b/drivers/interconnect/qcom/icc-rpm.h
>>> index d2c04c400cad..ba840a436cc0 100644
>>> --- a/drivers/interconnect/qcom/icc-rpm.h
>>> +++ b/drivers/interconnect/qcom/icc-rpm.h
>>> @@ -29,10 +29,10 @@ enum qcom_icc_type {
>>>     * @num_intf_clks: the total number of intf_clks clk_bulk_data entries
>>>     * @type: the ICC provider type
>>>     * @regmap: regmap for QoS registers read/write access
>>> - * @qos_offset: offset to QoS registers
>>>     * @bus_clk_rate: bus clock rate in Hz
>>>     * @bus_clks: the clk_bulk_data table of bus clocks
>>>     * @intf_clks: a clk_bulk_data array of interface clocks
>>> + * @qos_offset: offset to QoS registers
>>
>> This can be kept in place.
>>
>>>     * @keep_alive: whether to always keep a minimum vote on the bus clocks
>>>     * @is_on: whether the bus is powered on
>>>     */
>>> @@ -42,7 +42,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[NUM_BUS_CLKS];
>>>        struct clk_bulk_data bus_clks[NUM_BUS_CLKS];
>>>        struct clk_bulk_data *intf_clks;
>>> @@ -108,7 +108,7 @@ struct qcom_icc_desc {
>>>        bool no_clk_scaling;
>>>        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 d2c04c400cad..ba840a436cc0 100644
--- a/drivers/interconnect/qcom/icc-rpm.h
+++ b/drivers/interconnect/qcom/icc-rpm.h
@@ -29,10 +29,10 @@  enum qcom_icc_type {
  * @num_intf_clks: the total number of intf_clks clk_bulk_data entries
  * @type: the ICC provider type
  * @regmap: regmap for QoS registers read/write access
- * @qos_offset: offset to QoS registers
  * @bus_clk_rate: bus clock rate in Hz
  * @bus_clks: the clk_bulk_data table of bus clocks
  * @intf_clks: a clk_bulk_data array of interface clocks
+ * @qos_offset: offset to QoS registers
  * @keep_alive: whether to always keep a minimum vote on the bus clocks
  * @is_on: whether the bus is powered on
  */
@@ -42,7 +42,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[NUM_BUS_CLKS];
 	struct clk_bulk_data bus_clks[NUM_BUS_CLKS];
 	struct clk_bulk_data *intf_clks;
@@ -108,7 +108,7 @@  struct qcom_icc_desc {
 	bool no_clk_scaling;
 	enum qcom_icc_type type;
 	const struct regmap_config *regmap_cfg;
-	unsigned int qos_offset;
+	int qos_offset;
 };
 
 /* Valid for all bus types */