On 20/06/2022 17:48, Francesco Dolcini wrote: > Hello Krzysztof, > thanks for your comment, let me try to provide you some additional > background to better understand this change. > > On Fri, Jun 17, 2022 at 06:02:39PM -0700, Krzysztof Kozlowski wrote: >> On 17/06/2022 00:08, Francesco Dolcini wrote: >>> Move `trips` definition to `#/$defs/trips-base` and just reference it >>> from the trips node. This allows to easily re-use this binding from >>> another binding file. >>> >>> No functional changes expected. >> >> If you want to re-use trips, they should be rather moved to separate >> YAML file... > > Fine, this should not be a big deal to achieve. Let's agree on the rest > first, however. > >> but anyway this should not be done per-driver bindings, but >> in more general way. Either the problem - using one DTS for different >> temperature grades - looks generic or is wrong at the core. In the first >> option, the generic bindings should be fixed. In the second case - using >> same DTS for different HW is not correct approach and why only thermal >> should be specific? I can imagine that cooling devices might have >> different settings, regulator voltages for DVFS could be a bit different... > > Let me try to explain the problem I am trying to solve here. > > Currently the imx-thermal driver harcode the critical trip threshold, > this trip point is read-only as it is considered a system property that > should not be changed and it is set to a value that is less than the > actual SoC maximum temperature. NO thermal_of driver used. > > Because of that there are systems that cannot work on some valid > temperature range. > > We are currently looking at a solution that would be backward compatible > with old device tree. > > I proposed the following: > 1- just increase the threshold to the actual max value allowed according > to the SoC thermal grade. > > As easy as > > - data->temp_critical = data->temp_max - (1000 * 5); > + data->temp_critical = data->temp_max; > > in drivers/thermal/imx_thermal.c > > https://lore.kernel.org/all/20220420091300.179753-1-francesco.dolcini@xxxxxxxxxxx/ > > It was not considered good enough by Lucas since this is a overall > system design question, therefore should be configurable. > > 2- make the critical trip write-able from userspace/sysfs. > > Daniel is against this since critical trip point is a system > property, not something the user should be allowed to change. > > 3- kernel parameter: https://lore.kernel.org/all/20220516190001.147919-1-francesco.dolcini@xxxxxxxxxxx/ > > Initially proposed by Daniel, but Marco did not like the idea. > > 4- New device tree property, fsl,tempmon-critical-offset, ditched also > by Marco > > 5- The current solution in this patch, with the existing trip points > that are hardcoded in the code exposed in the device tree as trips. > > > Ideally one could just implement the imx6/7 thermal sensor reading and > just make use of the thermal_of driver, however that would break > compatibility with a lot of existing system ... to me this is just a > no-go. > > Adding only one set of thermal trip point in the dts (no thermal-grade > specific set) could work in some specific scenario, however it does not > work for me since I have the same dts files using different temperature > grade SoC. I would need to update this in the firmware before starting > Linux. > > Krzysztof, what do you think? I would not mind to get back to one of > the more simpler approach I proposed. > > Lucas, are you really that against the simple working solution I > proposed initially [1]? I feel like I am running in circles ... BTW, the link [1] was missing in your email, so I understood that you meant this patchset. If [1] refers to something else, then we need to discuss that something else. Best regards, Krzysztof
