diff mbox series

ARM: dts: stm32: Enable thermal sensor support on stm32mp15xx-dhcor

Message ID 20200923232535.241437-1-marex@denx.de
State New, archived
Headers show
Series ARM: dts: stm32: Enable thermal sensor support on stm32mp15xx-dhcor | expand

Commit Message

Marek Vasut Sept. 23, 2020, 11:25 p.m. UTC
Enable STM32 Digital Thermal Sensor driver for stm32mp15xx-dhcor SoMs.

Fixes: 94cafe1b6482 ("ARM: dts: stm32: Add Avenger96 devicetree support based on STM32MP157A")
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: linux-stm32@st-md-mailman.stormreply.com
To: linux-arm-kernel@lists.infradead.org
---
 arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Manivannan Sadhasivam Sept. 24, 2020, 5:16 a.m. UTC | #1
On 24 September 2020 4:55:35 AM IST, Marek Vasut <marex@denx.de> wrote:
>Enable STM32 Digital Thermal Sensor driver for stm32mp15xx-dhcor SoMs.
>
>Fixes: 94cafe1b6482 ("ARM: dts: stm32: Add Avenger96 devicetree support
>based on STM32MP157A")

The change looks good but what does this patch fixes? 

Thanks, 
Mani

>Signed-off-by: Marek Vasut <marex@denx.de>
>Cc: Alexandre Torgue <alexandre.torgue@st.com>
>Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>Cc: linux-stm32@st-md-mailman.stormreply.com
>To: linux-arm-kernel@lists.infradead.org
>---
> arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>index 04fbb324a541..803eb8bc9c85 100644
>--- a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>+++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>@@ -21,6 +21,10 @@ memory@c0000000 {
> 	};
> };
> 
>+&dts {
>+	status = "okay";
>+};
>+
> &i2c4 {
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&i2c4_pins_a>;
Marek Vasut Sept. 24, 2020, 10:41 a.m. UTC | #2
On 9/24/20 7:16 AM, Manivannan Sadhasivam wrote:
> 
> 
> On 24 September 2020 4:55:35 AM IST, Marek Vasut <marex@denx.de> wrote:
>> Enable STM32 Digital Thermal Sensor driver for stm32mp15xx-dhcor SoMs.
>>
>> Fixes: 94cafe1b6482 ("ARM: dts: stm32: Add Avenger96 devicetree support
>> based on STM32MP157A")
> 
> The change looks good but what does this patch fixes? 

The missing temp sensor, which helps you detect overheat of the SoC.
That is esp. important on the 800 MHz AV96.
Manivannan Sadhasivam Sept. 25, 2020, 2:21 a.m. UTC | #3
On 24 September 2020 4:11:11 PM IST, Marek Vasut <marex@denx.de> wrote:
>On 9/24/20 7:16 AM, Manivannan Sadhasivam wrote:
>> 
>> 
>> On 24 September 2020 4:55:35 AM IST, Marek Vasut <marex@denx.de>
>wrote:
>>> Enable STM32 Digital Thermal Sensor driver for stm32mp15xx-dhcor
>SoMs.
>>>
>>> Fixes: 94cafe1b6482 ("ARM: dts: stm32: Add Avenger96 devicetree
>support
>>> based on STM32MP157A")
>> 
>> The change looks good but what does this patch fixes? 
>
>The missing temp sensor, which helps you detect overheat of the SoC.
>That is esp. important on the 800 MHz AV96.

This doesn't quality as a "fix". Essentially you're just adding a missing feature and not fixing any issues. So please remove the fixes tag and resubmit. 

Thanks, 
Mani
Marek Vasut Sept. 25, 2020, 11:12 a.m. UTC | #4
On 9/25/20 4:21 AM, Manivannan Sadhasivam wrote:
> 
> 
> On 24 September 2020 4:11:11 PM IST, Marek Vasut <marex@denx.de> wrote:
>> On 9/24/20 7:16 AM, Manivannan Sadhasivam wrote:
>>>
>>>
>>> On 24 September 2020 4:55:35 AM IST, Marek Vasut <marex@denx.de>
>> wrote:
>>>> Enable STM32 Digital Thermal Sensor driver for stm32mp15xx-dhcor
>> SoMs.
>>>>
>>>> Fixes: 94cafe1b6482 ("ARM: dts: stm32: Add Avenger96 devicetree
>> support
>>>> based on STM32MP157A")
>>>
>>> The change looks good but what does this patch fixes? 
>>
>> The missing temp sensor, which helps you detect overheat of the SoC.
>> That is esp. important on the 800 MHz AV96.
> 
> This doesn't quality as a "fix". Essentially you're just adding a missing feature and not fixing any issues. So please remove the fixes tag and resubmit. 

I would argue that if the system overheats and crashes, we want to know
about that, possibly in advance so thermal throttling can be applied.
Currently this is not possible and I think that is a bug.
Manivannan Sadhasivam Sept. 25, 2020, 4:20 p.m. UTC | #5
On Fri, Sep 25, 2020 at 01:12:12PM +0200, Marek Vasut wrote:
> On 9/25/20 4:21 AM, Manivannan Sadhasivam wrote:
> > 
> > 
> > On 24 September 2020 4:11:11 PM IST, Marek Vasut <marex@denx.de> wrote:
> >> On 9/24/20 7:16 AM, Manivannan Sadhasivam wrote:
> >>>
> >>>
> >>> On 24 September 2020 4:55:35 AM IST, Marek Vasut <marex@denx.de>
> >> wrote:
> >>>> Enable STM32 Digital Thermal Sensor driver for stm32mp15xx-dhcor
> >> SoMs.
> >>>>
> >>>> Fixes: 94cafe1b6482 ("ARM: dts: stm32: Add Avenger96 devicetree
> >> support
> >>>> based on STM32MP157A")
> >>>
> >>> The change looks good but what does this patch fixes? 
> >>
> >> The missing temp sensor, which helps you detect overheat of the SoC.
> >> That is esp. important on the 800 MHz AV96.
> > 
> > This doesn't quality as a "fix". Essentially you're just adding a missing feature and not fixing any issues. So please remove the fixes tag and resubmit. 
> 
> I would argue that if the system overheats and crashes, we want to know
> about that, possibly in advance so thermal throttling can be applied.
> Currently this is not possible and I think that is a bug.

No, this is not a _bug_. This is a missing feature that the current kernel
doesn't support and you know about that! The fact that you can trigger a crash
due to hw limitation doesn't qualify as a bug IMO. And you can do that by other
means also (CPU throttling without CPUFreq support etc...)

Anyway, I'll stop here and let Alex to make a call.

Either case, feel free to add:

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani
Marek Vasut Sept. 25, 2020, 6:06 p.m. UTC | #6
On 9/25/20 6:20 PM, Manivannan Sadhasivam wrote:
> On Fri, Sep 25, 2020 at 01:12:12PM +0200, Marek Vasut wrote:
>> On 9/25/20 4:21 AM, Manivannan Sadhasivam wrote:
>>>
>>>
>>> On 24 September 2020 4:11:11 PM IST, Marek Vasut <marex@denx.de> wrote:
>>>> On 9/24/20 7:16 AM, Manivannan Sadhasivam wrote:
>>>>>
>>>>>
>>>>> On 24 September 2020 4:55:35 AM IST, Marek Vasut <marex@denx.de>
>>>> wrote:
>>>>>> Enable STM32 Digital Thermal Sensor driver for stm32mp15xx-dhcor
>>>> SoMs.
>>>>>>
>>>>>> Fixes: 94cafe1b6482 ("ARM: dts: stm32: Add Avenger96 devicetree
>>>> support
>>>>>> based on STM32MP157A")
>>>>>
>>>>> The change looks good but what does this patch fixes? 
>>>>
>>>> The missing temp sensor, which helps you detect overheat of the SoC.
>>>> That is esp. important on the 800 MHz AV96.
>>>
>>> This doesn't quality as a "fix". Essentially you're just adding a missing feature and not fixing any issues. So please remove the fixes tag and resubmit. 
>>
>> I would argue that if the system overheats and crashes, we want to know
>> about that, possibly in advance so thermal throttling can be applied.
>> Currently this is not possible and I think that is a bug.
> 
> No, this is not a _bug_. This is a missing feature that the current kernel
> doesn't support and you know about that! The fact that you can trigger a crash
> due to hw limitation doesn't qualify as a bug IMO. And you can do that by other
> means also (CPU throttling without CPUFreq support etc...)

I would say that makes the hardware pretty useless, which is not the
expectation I would have of a system I would like to use in production.
Just assume your laptop would randomly overheat and crash, would you
consider that a missing feature or a bug that should be fixed ?

> Anyway, I'll stop here and let Alex to make a call.

I agree.
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
index 04fbb324a541..803eb8bc9c85 100644
--- a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
+++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
@@ -21,6 +21,10 @@  memory@c0000000 {
 	};
 };
 
+&dts {
+	status = "okay";
+};
+
 &i2c4 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c4_pins_a>;