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 22-06-17, Francesco Dolcini wrote:
> 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.

Okay, fine with me since you provided dt-snippets with the correct
temperature. But we should really print a warning since this is a
abnormal usage and the user should be warned.

Regards,
  Marco
> 
> 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