* [PATCH 0/2] opp: ti-opp-supply: Fixes
@ 2018-11-07 4:34 ` Keerthy
0 siblings, 0 replies; 15+ messages in thread
From: Keerthy @ 2018-11-07 4:34 UTC (permalink / raw)
To: vireshk, nm, sboyd; +Cc: linux-pm, linux-kernel, d-gerlach, t-kristo, j-keerthy
The series brings in couple of fixes to the ti-opp-supply driver.
One of them updates u_volt_min dynamically and avoids hang due
to lesser static u_volt_min and the other fixes the supply in
_get_optimal_vdd_voltage.
Keerthy (2):
power: opp: ti-opp-supply: Dynamically update u_volt_min
power: opp: ti-opp-supply: Correct the supply in
_get_optimal_vdd_voltage call
drivers/opp/ti-opp-supply.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
--
1.9.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/2] opp: ti-opp-supply: Dynamically update u_volt_min
2018-11-07 4:34 ` Keerthy
@ 2018-11-07 4:34 ` Keerthy
-1 siblings, 0 replies; 15+ messages in thread
From: Keerthy @ 2018-11-07 4:34 UTC (permalink / raw)
To: vireshk, nm, sboyd; +Cc: linux-pm, linux-kernel, d-gerlach, t-kristo, j-keerthy
The voltage range (min, max) provided in the device tree is from
the data manual and is pretty big, catering to a wide range of devices.
On a i2c read/write failure the regulator_set_voltage_triplet function
falls back to set voltage between min and max. The min value from Device
Tree can be lesser than the optimal value and in that case that can lead
to a hang or crash. Hence set the u_volt_min dynamically to the optimal
voltage value.
Fixes: 9a835fa6e47 ("PM / OPP: Add ti-opp-supply driver")
Signed-off-by: Keerthy <j-keerthy@ti.com>
---
drivers/opp/ti-opp-supply.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/opp/ti-opp-supply.c b/drivers/opp/ti-opp-supply.c
index 9e5a9a3..29e08a4 100644
--- a/drivers/opp/ti-opp-supply.c
+++ b/drivers/opp/ti-opp-supply.c
@@ -290,6 +290,9 @@ static int ti_opp_supply_set_opp(struct dev_pm_set_opp_data *data)
vdd_uv = _get_optimal_vdd_voltage(dev, &opp_data,
new_supply_vbb->u_volt);
+ if (new_supply_vdd->u_volt_min < vdd_uv)
+ new_supply_vdd->u_volt_min = vdd_uv;
+
/* Scaling up? Scale voltage before frequency */
if (freq > old_freq) {
ret = _opp_set_voltage(dev, new_supply_vdd, vdd_uv, vdd_reg,
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 1/2] opp: ti-opp-supply: Dynamically update u_volt_min
@ 2018-11-07 4:34 ` Keerthy
0 siblings, 0 replies; 15+ messages in thread
From: Keerthy @ 2018-11-07 4:34 UTC (permalink / raw)
To: vireshk, nm, sboyd; +Cc: linux-pm, linux-kernel, d-gerlach, t-kristo, j-keerthy
The voltage range (min, max) provided in the device tree is from
the data manual and is pretty big, catering to a wide range of devices.
On a i2c read/write failure the regulator_set_voltage_triplet function
falls back to set voltage between min and max. The min value from Device
Tree can be lesser than the optimal value and in that case that can lead
to a hang or crash. Hence set the u_volt_min dynamically to the optimal
voltage value.
Fixes: 9a835fa6e47 ("PM / OPP: Add ti-opp-supply driver")
Signed-off-by: Keerthy <j-keerthy@ti.com>
---
drivers/opp/ti-opp-supply.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/opp/ti-opp-supply.c b/drivers/opp/ti-opp-supply.c
index 9e5a9a3..29e08a4 100644
--- a/drivers/opp/ti-opp-supply.c
+++ b/drivers/opp/ti-opp-supply.c
@@ -290,6 +290,9 @@ static int ti_opp_supply_set_opp(struct dev_pm_set_opp_data *data)
vdd_uv = _get_optimal_vdd_voltage(dev, &opp_data,
new_supply_vbb->u_volt);
+ if (new_supply_vdd->u_volt_min < vdd_uv)
+ new_supply_vdd->u_volt_min = vdd_uv;
+
/* Scaling up? Scale voltage before frequency */
if (freq > old_freq) {
ret = _opp_set_voltage(dev, new_supply_vdd, vdd_uv, vdd_reg,
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 1/2] opp: ti-opp-supply: Dynamically update u_volt_min
2018-11-07 4:34 ` Keerthy
(?)
@ 2018-11-08 5:54 ` Viresh Kumar
2018-11-12 3:15 ` J, KEERTHY
-1 siblings, 1 reply; 15+ messages in thread
From: Viresh Kumar @ 2018-11-08 5:54 UTC (permalink / raw)
To: Keerthy; +Cc: vireshk, nm, sboyd, linux-pm, linux-kernel, d-gerlach, t-kristo
On 07-11-18, 10:04, Keerthy wrote:
> The voltage range (min, max) provided in the device tree is from
> the data manual and is pretty big, catering to a wide range of devices.
> On a i2c read/write failure the regulator_set_voltage_triplet function
> falls back to set voltage between min and max. The min value from Device
> Tree can be lesser than the optimal value and in that case that can lead
> to a hang or crash. Hence set the u_volt_min dynamically to the optimal
> voltage value.
And why shouldn't we fix the DT for this ?
--
viresh
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] opp: ti-opp-supply: Dynamically update u_volt_min
2018-11-08 5:54 ` Viresh Kumar
@ 2018-11-12 3:15 ` J, KEERTHY
0 siblings, 0 replies; 15+ messages in thread
From: J, KEERTHY @ 2018-11-12 3:15 UTC (permalink / raw)
To: Viresh Kumar
Cc: vireshk, nm, sboyd, linux-pm, linux-kernel, d-gerlach, t-kristo
On 11/8/2018 11:24 AM, Viresh Kumar wrote:
> On 07-11-18, 10:04, Keerthy wrote:
>> The voltage range (min, max) provided in the device tree is from
>> the data manual and is pretty big, catering to a wide range of devices.
>> On a i2c read/write failure the regulator_set_voltage_triplet function
>> falls back to set voltage between min and max. The min value from Device
>> Tree can be lesser than the optimal value and in that case that can lead
>> to a hang or crash. Hence set the u_volt_min dynamically to the optimal
>> voltage value.
>
> And why shouldn't we fix the DT for this ?
The DT voltages do not cater to the broad range of devices. In some
particular cases the DT min voltage is slightly lower the voltage at
which the device cannot sustain a particular frequency in which case the
device just silently hangs. So best thing to do is to actually read the
device specific voltages dynamically which will guarantee a particular
device sustaining a particular frequency at the optimal voltage.
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] opp: ti-opp-supply: Dynamically update u_volt_min
@ 2018-11-12 3:15 ` J, KEERTHY
0 siblings, 0 replies; 15+ messages in thread
From: J, KEERTHY @ 2018-11-12 3:15 UTC (permalink / raw)
To: Viresh Kumar
Cc: vireshk, nm, sboyd, linux-pm, linux-kernel, d-gerlach, t-kristo
On 11/8/2018 11:24 AM, Viresh Kumar wrote:
> On 07-11-18, 10:04, Keerthy wrote:
>> The voltage range (min, max) provided in the device tree is from
>> the data manual and is pretty big, catering to a wide range of devices.
>> On a i2c read/write failure the regulator_set_voltage_triplet function
>> falls back to set voltage between min and max. The min value from Device
>> Tree can be lesser than the optimal value and in that case that can lead
>> to a hang or crash. Hence set the u_volt_min dynamically to the optimal
>> voltage value.
>
> And why shouldn't we fix the DT for this ?
The DT voltages do not cater to the broad range of devices. In some
particular cases the DT min voltage is slightly lower the voltage at
which the device cannot sustain a particular frequency in which case the
device just silently hangs. So best thing to do is to actually read the
device specific voltages dynamically which will guarantee a particular
device sustaining a particular frequency at the optimal voltage.
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] opp: ti-opp-supply: Dynamically update u_volt_min
2018-11-12 3:15 ` J, KEERTHY
@ 2018-11-12 22:05 ` Dave Gerlach
-1 siblings, 0 replies; 15+ messages in thread
From: Dave Gerlach @ 2018-11-12 22:05 UTC (permalink / raw)
To: J, KEERTHY, Viresh Kumar
Cc: vireshk, nm, sboyd, linux-pm, linux-kernel, t-kristo
On 11/11/2018 09:15 PM, J, KEERTHY wrote:
>
>
> On 11/8/2018 11:24 AM, Viresh Kumar wrote:
>> On 07-11-18, 10:04, Keerthy wrote:
>>> The voltage range (min, max) provided in the device tree is from
>>> the data manual and is pretty big, catering to a wide range of devices.
>>> On a i2c read/write failure the regulator_set_voltage_triplet function
>>> falls back to set voltage between min and max. The min value from Device
>>> Tree can be lesser than the optimal value and in that case that can lead
>>> to a hang or crash. Hence set the u_volt_min dynamically to the optimal
>>> voltage value.
>>
>> And why shouldn't we fix the DT for this ?
>
> The DT voltages do not cater to the broad range of devices. In some
> particular cases the DT min voltage is slightly lower the voltage at
> which the device cannot sustain a particular frequency in which case the
> device just silently hangs. So best thing to do is to actually read the
> device specific voltages dynamically which will guarantee a particular
> device sustaining a particular frequency at the optimal voltage.
>
Acked-by: Dave Gerlach <d-gerlach@ti.com>
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] opp: ti-opp-supply: Dynamically update u_volt_min
@ 2018-11-12 22:05 ` Dave Gerlach
0 siblings, 0 replies; 15+ messages in thread
From: Dave Gerlach @ 2018-11-12 22:05 UTC (permalink / raw)
To: J, KEERTHY, Viresh Kumar
Cc: vireshk, nm, sboyd, linux-pm, linux-kernel, t-kristo
On 11/11/2018 09:15 PM, J, KEERTHY wrote:
>
>
> On 11/8/2018 11:24 AM, Viresh Kumar wrote:
>> On 07-11-18, 10:04, Keerthy wrote:
>>> The voltage range (min, max) provided in the device tree is from
>>> the data manual and is pretty big, catering to a wide range of devices.
>>> On a i2c read/write failure the regulator_set_voltage_triplet function
>>> falls back to set voltage between min and max. The min value from Device
>>> Tree can be lesser than the optimal value and in that case that can lead
>>> to a hang or crash. Hence set the u_volt_min dynamically to the optimal
>>> voltage value.
>>
>> And why shouldn't we fix the DT for this ?
>
> The DT voltages do not cater to the broad range of devices. In some
> particular cases the DT min voltage is slightly lower the voltage at
> which the device cannot sustain a particular frequency in which case the
> device just silently hangs. So best thing to do is to actually read the
> device specific voltages dynamically which will guarantee a particular
> device sustaining a particular frequency at the optimal voltage.
>
Acked-by: Dave Gerlach <d-gerlach@ti.com>
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/2] opp: ti-opp-supply: Correct the supply in _get_optimal_vdd_voltage call
2018-11-07 4:34 ` Keerthy
@ 2018-11-07 4:34 ` Keerthy
-1 siblings, 0 replies; 15+ messages in thread
From: Keerthy @ 2018-11-07 4:34 UTC (permalink / raw)
To: vireshk, nm, sboyd; +Cc: linux-pm, linux-kernel, d-gerlach, t-kristo, j-keerthy
_get_optimal_vdd_voltage call provides new_supply_vbb->u_volt
as the reference voltage while it should be really new_supply_vdd->u_volt.
Fixes: 9a835fa6e47 ("PM / OPP: Add ti-opp-supply driver")
Signed-off-by: Keerthy <j-keerthy@ti.com>
---
drivers/opp/ti-opp-supply.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/opp/ti-opp-supply.c b/drivers/opp/ti-opp-supply.c
index 29e08a4..3f4fb4d 100644
--- a/drivers/opp/ti-opp-supply.c
+++ b/drivers/opp/ti-opp-supply.c
@@ -288,7 +288,7 @@ static int ti_opp_supply_set_opp(struct dev_pm_set_opp_data *data)
int ret;
vdd_uv = _get_optimal_vdd_voltage(dev, &opp_data,
- new_supply_vbb->u_volt);
+ new_supply_vdd->u_volt);
if (new_supply_vdd->u_volt_min < vdd_uv)
new_supply_vdd->u_volt_min = vdd_uv;
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/2] opp: ti-opp-supply: Correct the supply in _get_optimal_vdd_voltage call
@ 2018-11-07 4:34 ` Keerthy
0 siblings, 0 replies; 15+ messages in thread
From: Keerthy @ 2018-11-07 4:34 UTC (permalink / raw)
To: vireshk, nm, sboyd; +Cc: linux-pm, linux-kernel, d-gerlach, t-kristo, j-keerthy
_get_optimal_vdd_voltage call provides new_supply_vbb->u_volt
as the reference voltage while it should be really new_supply_vdd->u_volt.
Fixes: 9a835fa6e47 ("PM / OPP: Add ti-opp-supply driver")
Signed-off-by: Keerthy <j-keerthy@ti.com>
---
drivers/opp/ti-opp-supply.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/opp/ti-opp-supply.c b/drivers/opp/ti-opp-supply.c
index 29e08a4..3f4fb4d 100644
--- a/drivers/opp/ti-opp-supply.c
+++ b/drivers/opp/ti-opp-supply.c
@@ -288,7 +288,7 @@ static int ti_opp_supply_set_opp(struct dev_pm_set_opp_data *data)
int ret;
vdd_uv = _get_optimal_vdd_voltage(dev, &opp_data,
- new_supply_vbb->u_volt);
+ new_supply_vdd->u_volt);
if (new_supply_vdd->u_volt_min < vdd_uv)
new_supply_vdd->u_volt_min = vdd_uv;
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 2/2] opp: ti-opp-supply: Correct the supply in _get_optimal_vdd_voltage call
2018-11-07 4:34 ` Keerthy
@ 2018-11-12 21:49 ` Dave Gerlach
-1 siblings, 0 replies; 15+ messages in thread
From: Dave Gerlach @ 2018-11-12 21:49 UTC (permalink / raw)
To: Keerthy, vireshk, nm, sboyd; +Cc: linux-pm, linux-kernel, t-kristo
On 11/06/2018 10:34 PM, Keerthy wrote:
> _get_optimal_vdd_voltage call provides new_supply_vbb->u_volt
> as the reference voltage while it should be really new_supply_vdd->u_volt.
>
> Fixes: 9a835fa6e47 ("PM / OPP: Add ti-opp-supply driver")
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
Acked-by: Dave Gerlach <d-gerlach@ti.com>
Currently all corresponding vbb and vdd values provided by the device tree match
which is why this does not fail, but this was a typo by me and vdd is the
correct value to actually use.
Regards,
Dave
> drivers/opp/ti-opp-supply.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/opp/ti-opp-supply.c b/drivers/opp/ti-opp-supply.c
> index 29e08a4..3f4fb4d 100644
> --- a/drivers/opp/ti-opp-supply.c
> +++ b/drivers/opp/ti-opp-supply.c
> @@ -288,7 +288,7 @@ static int ti_opp_supply_set_opp(struct dev_pm_set_opp_data *data)
> int ret;
>
> vdd_uv = _get_optimal_vdd_voltage(dev, &opp_data,
> - new_supply_vbb->u_volt);
> + new_supply_vdd->u_volt);
>
> if (new_supply_vdd->u_volt_min < vdd_uv)
> new_supply_vdd->u_volt_min = vdd_uv;
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 2/2] opp: ti-opp-supply: Correct the supply in _get_optimal_vdd_voltage call
@ 2018-11-12 21:49 ` Dave Gerlach
0 siblings, 0 replies; 15+ messages in thread
From: Dave Gerlach @ 2018-11-12 21:49 UTC (permalink / raw)
To: Keerthy, vireshk, nm, sboyd; +Cc: linux-pm, linux-kernel, t-kristo
On 11/06/2018 10:34 PM, Keerthy wrote:
> _get_optimal_vdd_voltage call provides new_supply_vbb->u_volt
> as the reference voltage while it should be really new_supply_vdd->u_volt.
>
> Fixes: 9a835fa6e47 ("PM / OPP: Add ti-opp-supply driver")
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
Acked-by: Dave Gerlach <d-gerlach@ti.com>
Currently all corresponding vbb and vdd values provided by the device tree match
which is why this does not fail, but this was a typo by me and vdd is the
correct value to actually use.
Regards,
Dave
> drivers/opp/ti-opp-supply.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/opp/ti-opp-supply.c b/drivers/opp/ti-opp-supply.c
> index 29e08a4..3f4fb4d 100644
> --- a/drivers/opp/ti-opp-supply.c
> +++ b/drivers/opp/ti-opp-supply.c
> @@ -288,7 +288,7 @@ static int ti_opp_supply_set_opp(struct dev_pm_set_opp_data *data)
> int ret;
>
> vdd_uv = _get_optimal_vdd_voltage(dev, &opp_data,
> - new_supply_vbb->u_volt);
> + new_supply_vdd->u_volt);
>
> if (new_supply_vdd->u_volt_min < vdd_uv)
> new_supply_vdd->u_volt_min = vdd_uv;
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] opp: ti-opp-supply: Fixes
2018-11-07 4:34 ` Keerthy
` (2 preceding siblings ...)
(?)
@ 2018-11-08 5:53 ` Viresh Kumar
-1 siblings, 0 replies; 15+ messages in thread
From: Viresh Kumar @ 2018-11-08 5:53 UTC (permalink / raw)
To: d-gerlach, Keerthy; +Cc: vireshk, nm, sboyd, linux-pm, linux-kernel, t-kristo
On 07-11-18, 10:04, Keerthy wrote:
> The series brings in couple of fixes to the ti-opp-supply driver.
> One of them updates u_volt_min dynamically and avoids hang due
> to lesser static u_volt_min and the other fixes the supply in
> _get_optimal_vdd_voltage.
>
> Keerthy (2):
> power: opp: ti-opp-supply: Dynamically update u_volt_min
> power: opp: ti-opp-supply: Correct the supply in
> _get_optimal_vdd_voltage call
@Dave: I would like to get an Ack from you on these before applying.
--
viresh
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 0/2] opp: ti-opp-supply: Fixes
2018-11-07 4:34 ` Keerthy
` (3 preceding siblings ...)
(?)
@ 2018-11-13 4:12 ` Viresh Kumar
-1 siblings, 0 replies; 15+ messages in thread
From: Viresh Kumar @ 2018-11-13 4:12 UTC (permalink / raw)
To: Keerthy; +Cc: vireshk, nm, sboyd, linux-pm, linux-kernel, d-gerlach, t-kristo
On 07-11-18, 10:04, Keerthy wrote:
> The series brings in couple of fixes to the ti-opp-supply driver.
> One of them updates u_volt_min dynamically and avoids hang due
> to lesser static u_volt_min and the other fixes the supply in
> _get_optimal_vdd_voltage.
Applied both patches after manually adding below:
Cc: 4.16+ <stable@vger.kernel.org> # v4.16+
Thanks.
--
viresh
^ permalink raw reply [flat|nested] 15+ messages in thread