Re: [RESEND PATCH v2 4/9] imx: thermal: Configure trip point from DT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 17, 2022 at 10:40:17AM +0200, Marco Felsch wrote:
> On 22-06-17, Francesco Dolcini wrote:
> > Allow over-writing critical and passive trip point for each
> > temperature grade from the device tree, by default the pre-existing
> > hard-coded trip points are used.
> > 
> > This change enables configuring the system thermal characteristics into
> > the system-specific device tree instead of relying on global hard-coded
> > temperature thresholds that does not take into account the specific
> > system thermal design.
> > 
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@xxxxxxxxxxx>
> > ---
> > v2:
> >  - return immediately if no thermal node present in the dts
> >  - use dev_info instead of dev_dbg if there is an invalid trip
> >  - additional comment in case passive trip point is higher than critical
> > ---
> >  drivers/thermal/imx_thermal.c | 58 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> > 
> > diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> > index 16663373b682..a964baf802fc 100644
> > --- a/drivers/thermal/imx_thermal.c
> > +++ b/drivers/thermal/imx_thermal.c
> > @@ -17,6 +17,8 @@
> >  #include <linux/nvmem-consumer.h>
> >  #include <linux/pm_runtime.h>
> >  
> > +#include "thermal_core.h"
> > +
> >  #define REG_SET		0x4
> >  #define REG_CLR		0x8
> >  #define REG_TOG		0xc
> > @@ -479,36 +481,92 @@ static int imx_init_calib(struct platform_device *pdev, u32 ocotp_ana1)
> >  	return 0;
> >  }
> >  
> > +static void imx_init_temp_from_of(struct platform_device *pdev, const char *name)
> > +{
> > +	struct imx_thermal_data *data = platform_get_drvdata(pdev);
> > +	struct device_node *thermal, *trips, *trip_point;
> > +
> > +	thermal = of_get_child_by_name(pdev->dev.of_node, name);
> > +	if (!thermal)
> > +		return;
> > +
> > +	trips = of_get_child_by_name(thermal, "trips");
> > +
> > +	for_each_child_of_node(trips, trip_point) {
> > +		struct thermal_trip t;
> > +
> > +		if (thermal_of_populate_trip(trip_point, &t))
> > +			continue;
> > +
> > +		switch (t.type) {
> > +		case THERMAL_TRIP_PASSIVE:
> > +			data->temp_passive = t.temperature;
> > +			break;
> > +		case THERMAL_TRIP_CRITICAL:
> 
> Should we check also the temp_critical and temp_passive not exceeding
> the temp_max? Sry. that it came not earlier in my mind. So system damage
> is avoided.

I would not add such kind of restriction in the code. I can think of
multiple situations in which a system designer would prefer to take the
chances of burning a silicon (or more likely just age it a little bit
faster) than to just shut down the system.

In the end whoever is building the system should be empowered to do
something like that and it's no different from what it's already possible
with thermal_of driver for example. 

In addition to that from a system debug prospective all the threshold
(max, passive, critical) are already available in the kernel logs.

Francesco




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux