| Message ID | 20171005084206.2914-1-kukabu@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 05/10/2017 10:42, Michael Tatarinov wrote: > Introduce an optional property called "linux,hwmon", which enable > registration in hwmon subsystems. > > Cc: Zhang Rui <rui.zhang@intel.com> > Cc: Eduardo Valentin <edubezval@gmail.com> > Signed-off-by: Michael Tatarinov <kukabu@gmail.com> > --- > Documentation/devicetree/bindings/thermal/thermal.txt | 6 ++++++ > drivers/thermal/of-thermal.c | 7 +++++-- > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt > index 88b6ea1ad290..0dad55e77c7e 100644 > --- a/Documentation/devicetree/bindings/thermal/thermal.txt > +++ b/Documentation/devicetree/bindings/thermal/thermal.txt > @@ -175,6 +175,10 @@ Optional property: > 2000mW, while on a 10'' tablet is around > 4500mW. > > +- linux,hwmon: Register the thermal zone in hwmon subsystems > + Type: boolean (requires CONFIG_THERMAL_HWMON). > + Size: one cell > + > Note: The delay properties are bound to the maximum dT/dt (temperature > derivative over time) in two situations for a thermal zone: > (i) - when passive cooling is activated (polling-delay-passive); and > @@ -556,6 +560,8 @@ thermal-zones { > > sustainable-power = <2500>; > > + linux,hwmon; > + > trips { > /* Trips are based on resulting linear equation */ > cpu_trip: cpu-trip { > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c > index d04ec3b9e5ff..037ef6bb9420 100644 > --- a/drivers/thermal/of-thermal.c > +++ b/drivers/thermal/of-thermal.c > @@ -994,8 +994,11 @@ int __init of_parse_thermal_zones(void) > goto exit_free; > } > > - /* No hwmon because there might be hwmon drivers registering */ > - tzp->no_hwmon = true; > + /** Nit: this is kerneldoc comment format "/**" > + * No hwmon by default because there might be hwmon drivers > + * registering > + */ > + tzp->no_hwmon = !of_property_read_bool(child, "linux,hwmon"); > > if (!of_property_read_u32(child, "sustainable-power", &prop)) > tzp->sustainable_power = prop; >
Please add devicetree bindings mailing list when you are add such generic and linux specific bindings. For now, I disagree on the approach unless you convince that's the only way on devicetree list and get ACK from DT binding maintainers. On 05/10/17 09:42, Michael Tatarinov wrote: > Introduce an optional property called "linux,hwmon", which enable > registration in hwmon subsystems. > Both the commit and the property binding seem to conflict with the comment in Documentation/thermal/sysfs-api.txt It states "when no_hwmon == false, a hwmon sysfs interface will be created" while you seem to indicate that this is required to register to hwmon subsystems. If it's just to enable this sysfs access, then binding seems a bit of misuse.
Hello 2017-10-05 18:57 GMT+04:00, Sudeep Holla <sudeep.holla@arm.com>: > Please add devicetree bindings mailing list when you are add such > generic and linux specific bindings. For now, I disagree on the approach > unless you convince that's the only way on devicetree list and get > ACK from DT binding maintainers. OK, I will do it after deciding your next issue > On 05/10/17 09:42, Michael Tatarinov wrote: >> Introduce an optional property called "linux,hwmon", which enable >> registration in hwmon subsystems. >> > > Both the commit and the property binding seem to conflict with the > comment in Documentation/thermal/sysfs-api.txt > > It states "when no_hwmon == false, a hwmon sysfs interface will be > created" while you seem to indicate that this is required to register > to hwmon subsystems. > > If it's just to enable this sysfs access, then binding seems a bit of > misuse. You're right but if the property is "linux,no_hwmon" we've enabled this for all drivers by default. I do not want to change the default behavior. > > -- > Regards, > Sudeep >
On 06/10/17 03:44, Michael Tatarinov wrote: > Hello > > 2017-10-05 18:57 GMT+04:00, Sudeep Holla <sudeep.holla@arm.com>: >> Please add devicetree bindings mailing list when you are add such >> generic and linux specific bindings. For now, I disagree on the approach >> unless you convince that's the only way on devicetree list and get >> ACK from DT binding maintainers. > > OK, I will do it after deciding your next issue > >> On 05/10/17 09:42, Michael Tatarinov wrote: >>> Introduce an optional property called "linux,hwmon", which enable >>> registration in hwmon subsystems. >>> >> >> Both the commit and the property binding seem to conflict with the >> comment in Documentation/thermal/sysfs-api.txt >> >> It states "when no_hwmon == false, a hwmon sysfs interface will be >> created" while you seem to indicate that this is required to register >> to hwmon subsystems. >> >> If it's just to enable this sysfs access, then binding seems a bit of >> misuse. > > You're right but if the property is "linux,no_hwmon" we've enabled > this for all drivers by default. > I do not want to change the default behavior. > But you can't just create Linux specific bindings to influence the way you want drivers to behave. In certain cases, we do need linux specific bindings for some configurations. But I am not convinced this one falls in that category. You can always check what DT maintainers think especially when you creating some linux specific bindings which looks so generic.
diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt index 88b6ea1ad290..0dad55e77c7e 100644 --- a/Documentation/devicetree/bindings/thermal/thermal.txt +++ b/Documentation/devicetree/bindings/thermal/thermal.txt @@ -175,6 +175,10 @@ Optional property: 2000mW, while on a 10'' tablet is around 4500mW. +- linux,hwmon: Register the thermal zone in hwmon subsystems + Type: boolean (requires CONFIG_THERMAL_HWMON). + Size: one cell + Note: The delay properties are bound to the maximum dT/dt (temperature derivative over time) in two situations for a thermal zone: (i) - when passive cooling is activated (polling-delay-passive); and @@ -556,6 +560,8 @@ thermal-zones { sustainable-power = <2500>; + linux,hwmon; + trips { /* Trips are based on resulting linear equation */ cpu_trip: cpu-trip { diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index d04ec3b9e5ff..037ef6bb9420 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -994,8 +994,11 @@ int __init of_parse_thermal_zones(void) goto exit_free; } - /* No hwmon because there might be hwmon drivers registering */ - tzp->no_hwmon = true; + /** + * No hwmon by default because there might be hwmon drivers + * registering + */ + tzp->no_hwmon = !of_property_read_bool(child, "linux,hwmon"); if (!of_property_read_u32(child, "sustainable-power", &prop)) tzp->sustainable_power = prop;
Introduce an optional property called "linux,hwmon", which enable registration in hwmon subsystems. Cc: Zhang Rui <rui.zhang@intel.com> Cc: Eduardo Valentin <edubezval@gmail.com> Signed-off-by: Michael Tatarinov <kukabu@gmail.com> --- Documentation/devicetree/bindings/thermal/thermal.txt | 6 ++++++ drivers/thermal/of-thermal.c | 7 +++++-- 2 files changed, 11 insertions(+), 2 deletions(-)