| Message ID | 20241018104839.13296-1-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Daniel Lezcano |
| Headers | show |
| Series | [v3,1/3] dt-bindings: thermal: Add support for Airoha EN7581 thermal sensor | expand |
On Fri, Oct 18, 2024 at 12:48:04PM +0200, Christian Marangi wrote: > Add support for Airoha EN7581 thermal sensor and monitor. This is a > simple sensor for the CPU or SoC Package that provide thermal sensor and > trip point for hot low and critical condition to fire interrupt and > react on the abnormal state. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > Reviewed-by: Rob Herring (Arm) <robh@kernel.org> Any news with this series? Everything wrong with the thermal core small patch? Ansuel
Hi Ansuel, On 13/11/2024 16:56, Christian Marangi wrote: > On Fri, Oct 18, 2024 at 12:48:04PM +0200, Christian Marangi wrote: >> Add support for Airoha EN7581 thermal sensor and monitor. This is a >> simple sensor for the CPU or SoC Package that provide thermal sensor and >> trip point for hot low and critical condition to fire interrupt and >> react on the abnormal state. >> >> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> >> Reviewed-by: Rob Herring (Arm) <robh@kernel.org> > > Any news with this series? Everything wrong with the thermal core small > patch? I understand why you are trying to achieve this but usually it is the kernel which overloads the firmware description, not the opposite, no? Either way, we ignore the offset/slope from tzp and use a couple of private variables offset/slope in the driver (iow do not call thermal_zone_get_offset() thermal_zone_get_slope()). Or add the thermal_zone_set_offset() and thermal_zone_set_slope() helpers. I would prefer the first solution as for today I can not see any DT for ARM64 with the coefficients set. So may be we can consider the slope and the offset as a legacy which should be removed from sysfs and the thermal zone device parameters in a near future.
On Wed, Nov 13, 2024 at 07:18:04PM +0100, Daniel Lezcano wrote: > > Hi Ansuel, > > On 13/11/2024 16:56, Christian Marangi wrote: > > On Fri, Oct 18, 2024 at 12:48:04PM +0200, Christian Marangi wrote: > > > Add support for Airoha EN7581 thermal sensor and monitor. This is a > > > simple sensor for the CPU or SoC Package that provide thermal sensor and > > > trip point for hot low and critical condition to fire interrupt and > > > react on the abnormal state. > > > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > > Reviewed-by: Rob Herring (Arm) <robh@kernel.org> > > > > Any news with this series? Everything wrong with the thermal core small > > patch? > > I understand why you are trying to achieve this but usually it is the kernel > which overloads the firmware description, not the opposite, no? > > Either way, we ignore the offset/slope from tzp and use a couple of private > variables offset/slope in the driver (iow do not call > thermal_zone_get_offset() thermal_zone_get_slope()). Or add the > thermal_zone_set_offset() and thermal_zone_set_slope() helpers. > > I would prefer the first solution as for today I can not see any DT for > ARM64 with the coefficients set. So may be we can consider the slope and the > offset as a legacy which should be removed from sysfs and the thermal zone > device parameters in a near future. > Hi Daniel, Having set OPs is problematic as that would diverge from what is set in DT that should always have priority. Well yes my idea was trying to make use of them as currently there are many driver that set these values but have the slope and offset in thermal core always set to 0 and 1. Thing is that reading temp with ADC is very common and in some way or another you always have a slope and an offset so it makese sense to permit to have those values preallocated instead of handling them in priv struct. Anyway if the idea is to drop that, I will happly move those values handling back in the driver. Just need a confirm on that. Also thanks for the feedback!
On 13/11/2024 21:19, Christian Marangi wrote: > On Wed, Nov 13, 2024 at 07:18:04PM +0100, Daniel Lezcano wrote: >> >> Hi Ansuel, >> >> On 13/11/2024 16:56, Christian Marangi wrote: >>> On Fri, Oct 18, 2024 at 12:48:04PM +0200, Christian Marangi wrote: >>>> Add support for Airoha EN7581 thermal sensor and monitor. This is a >>>> simple sensor for the CPU or SoC Package that provide thermal sensor and >>>> trip point for hot low and critical condition to fire interrupt and >>>> react on the abnormal state. >>>> >>>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> >>>> Reviewed-by: Rob Herring (Arm) <robh@kernel.org> >>> >>> Any news with this series? Everything wrong with the thermal core small >>> patch? >> >> I understand why you are trying to achieve this but usually it is the kernel >> which overloads the firmware description, not the opposite, no? >> >> Either way, we ignore the offset/slope from tzp and use a couple of private >> variables offset/slope in the driver (iow do not call >> thermal_zone_get_offset() thermal_zone_get_slope()). Or add the >> thermal_zone_set_offset() and thermal_zone_set_slope() helpers. >> >> I would prefer the first solution as for today I can not see any DT for >> ARM64 with the coefficients set. So may be we can consider the slope and the >> offset as a legacy which should be removed from sysfs and the thermal zone >> device parameters in a near future. >> > > Hi Daniel, > > Having set OPs is problematic as that would diverge from what is set in > DT that should always have priority. > > Well yes my idea was trying to make use of them as currently there are > many driver that set these values but have the slope and offset in > thermal core always set to 0 and 1. > > Thing is that reading temp with ADC is very common and in some way or > another you always have a slope and an offset so it makese sense to > permit to have those values preallocated instead of handling them in > priv struct. Right but if we look at the coefficients description, it is a bit fuzzy how they are used today. > Anyway if the idea is to drop that, I will happly move those values > handling back in the driver. Just need a confirm on that. Yes, it is probably better to put them private to the driver while we sort out how to deal with the coefficients properly. Especially that we want to introduce thermal zones with multiple sensors support. Thanks
diff --git a/Documentation/devicetree/bindings/thermal/airoha,en7581-thermal.yaml b/Documentation/devicetree/bindings/thermal/airoha,en7581-thermal.yaml new file mode 100644 index 000000000000..ca0242ef0378 --- /dev/null +++ b/Documentation/devicetree/bindings/thermal/airoha,en7581-thermal.yaml @@ -0,0 +1,48 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/thermal/airoha,en7581-thermal.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Airoha EN7581 Thermal Sensor and Monitor + +maintainers: + - Christian Marangi <ansuelsmth@gmail.com> + +properties: + compatible: + const: airoha,en7581-thermal + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + airoha,chip-scu: + description: phandle to the chip SCU syscon + $ref: /schemas/types.yaml#/definitions/phandle + + '#thermal-sensor-cells': + const: 0 + +required: + - compatible + - reg + - interrupts + - airoha,chip-scu + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + + thermal-sensor@1efbd800 { + compatible = "airoha,en7581-thermal"; + reg = <0x1efbd000 0xd5c>; + interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>; + airoha,chip-scu = <&chip_scu>; + + #thermal-sensor-cells = <0>; + };