From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bartlomiej Zolnierkiewicz Subject: Re: [PATCH v2 01/17] thermal: add thermal_zone_set_mode() helper Date: Tue, 6 Nov 2018 17:11:23 +0100 Message-ID: References: <1539791563-5959-1-git-send-email-b.zolnierkie@samsung.com> <1539791563-5959-2-git-send-email-b.zolnierkie@samsung.com> <1541491911.2124.91.camel@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: <1541491911.2124.91.camel@intel.com> Sender: linux-kernel-owner@vger.kernel.org To: Zhang Rui , Eduardo Valentin Cc: Amit kucheria , Eric Anholt , Stefan Wahren , Markus Mayer , bcm-kernel-feedback-list@broadcom.com, Heiko Stuebner , Thierry Reding , Jonathan Hunter , Keerthy , Masahiro Yamada , Jun Nie , Baoyou Xie , Shawn Guo , linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org List-Id: linux-pm@vger.kernel.org On 11/06/2018 09:11 AM, Zhang Rui wrote: > On δΈ‰, 2018-10-17 at 17:52 +0200, Bartlomiej Zolnierkiewicz wrote: >> In order to remove the code duplication and prepare for further >> changes: >> >> * Add thermal_zone_set_mode() helper. Then update core code and >> drivers to use it. >> >> There should be no functional changes caused by this patch. >> >> Signed-off-by: Bartlomiej Zolnierkiewicz >> --- >> drivers/thermal/hisi_thermal.c | 14 ++------------ >> drivers/thermal/of-thermal.c | 3 ++- >> drivers/thermal/rockchip_thermal.c | 26 +++++++++----------------- >> drivers/thermal/thermal_helpers.c | 14 ++++++++++++++ >> drivers/thermal/thermal_sysfs.c | 8 +++++--- >> include/linux/thermal.h | 5 +++++ >> 6 files changed, 37 insertions(+), 33 deletions(-) >> >> >> >> diff --git a/drivers/thermal/thermal_helpers.c >> b/drivers/thermal/thermal_helpers.c >> index 2ba756a..b18cee2 100644 >> --- a/drivers/thermal/thermal_helpers.c >> +++ b/drivers/thermal/thermal_helpers.c >> @@ -224,3 +224,17 @@ int thermal_zone_get_offset(struct >> thermal_zone_device *tz) >> return 0; >> } >> EXPORT_SYMBOL_GPL(thermal_zone_get_offset); >> + >> +/** >> + * thermal_zone_set_mode() - sets mode of thermal zone device >> + * @tz: a valid pointer to a struct thermal_zone_device >> + * @mode: mode to be set >> + * >> + * Return: On success returns 0, an error code otherwise. >> + */ >> +int thermal_zone_set_mode(struct thermal_zone_device *tz, >> + enum thermal_device_mode mode) >> +{ >> + return tz->ops->set_mode(tz, mode); > > better to check tz->ops->set_mode first. I think that it should be added incrementally later when needed. We don't do 'defensive coding' in the kernel and in this patchset thermal_zone_set_mode() is not used in any place which has not used ->set_mode directly previously (actually patches #1-4 should not cause any functionality changes as noted in patch descriptions). > thanks, > rui >> +} >> +EXPORT_SYMBOL_GPL(thermal_zone_set_mode); >> diff --git a/drivers/thermal/thermal_sysfs.c >> b/drivers/thermal/thermal_sysfs.c >> index 2241cea..2e9e762 100644 >> --- a/drivers/thermal/thermal_sysfs.c >> +++ b/drivers/thermal/thermal_sysfs.c >> @@ -69,17 +69,19 @@ >> { >> struct thermal_zone_device *tz = to_thermal_zone(dev); >> int result; >> + enum thermal_device_mode mode; >> >> if (!tz->ops->set_mode) >> return -EPERM; >> >> if (!strncmp(buf, "enabled", sizeof("enabled") - 1)) >> - result = tz->ops->set_mode(tz, >> THERMAL_DEVICE_ENABLED); >> + mode = THERMAL_DEVICE_ENABLED; >> else if (!strncmp(buf, "disabled", sizeof("disabled") - 1)) >> - result = tz->ops->set_mode(tz, >> THERMAL_DEVICE_DISABLED); >> + mode = THERMAL_DEVICE_DISABLED; >> else >> - result = -EINVAL; >> + return -EINVAL; >> >> + result = thermal_zone_set_mode(tz, mode); >> if (result) >> return result; >> >> diff --git a/include/linux/thermal.h b/include/linux/thermal.h >> index 5f4705f..9d21fd1 100644 >> --- a/include/linux/thermal.h >> +++ b/include/linux/thermal.h >> @@ -452,6 +452,8 @@ struct thermal_cooling_device * >> int thermal_zone_get_temp(struct thermal_zone_device *tz, int >> *temp); >> int thermal_zone_get_slope(struct thermal_zone_device *tz); >> int thermal_zone_get_offset(struct thermal_zone_device *tz); >> +int thermal_zone_set_mode(struct thermal_zone_device *tz, >> + enum thermal_device_mode mode); >> >> int get_tz_trend(struct thermal_zone_device *, int); >> struct thermal_instance *get_thermal_instance(struct >> thermal_zone_device *, >> @@ -518,6 +520,9 @@ static inline int thermal_zone_get_slope( >> static inline int thermal_zone_get_offset( >> struct thermal_zone_device *tz) >> { return -ENODEV; } >> +static inline int thermal_zone_set_mode( >> + struct thermal_zone_device *tz, enum >> thermal_device_mode mode) >> +{ return -ENODEV; } >> static inline int get_tz_trend(struct thermal_zone_device *tz, int >> trip) >> { return -ENODEV; } >> static inline struct thermal_instance * Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics