Re: [BUG] New Kernel Bugs
On Nov 13, 2007 12:15 PM, Andrew Morton <[EMAIL PROTECTED]> wrote: > On Mon, 12 Nov 2007 22:42:32 -0800 "Natalie Protasevich" <[EMAIL PROTECTED]> > wrote: > > > This is the listing of the open bugs that are relatively new, around > > 2.6.22 and up. They are vaguely classified by specific area. > > (not a full list, there are more :) [...] > > IDE/SATA= [...] > > DVD-RAM umount and disk free bug > > http://bugzilla.kernel.org/show_bug.cgi?id=9265 > > Kernel: 2.6.15 (asked to try current kernel) > > No response from developers Bug was filled under IO/Storage-Other so is it assigned to <[EMAIL PROTECTED]>. Could be a FS problem as well but it is the best to wait for confirmation with 2.6.23 before proceeding further... - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.22-git: known regressions with patches
Hi, On Thursday 19 July 2007, Michal Piotrowski wrote: > Unclassified > > Subject : disk error loop (panic?) ide_do_rw_disk-bad > References : http://lkml.org/lkml/2007/7/17/467 > Last known good : ? > Submitter : Giacomo Catenazzi <[EMAIL PROTECTED]> > Caused-By : ? > Handled-By : Jens Axboe <[EMAIL PROTECTED]> > Linus Torvalds <[EMAIL PROTECTED]> > Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]> > Patch : http://lkml.org/lkml/2007/7/19/26 > Status : patch available Additional patch is an improvement but the regression itself has already been fixed in the mainline (by reverting problematic changes). Thanks, Bart - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2/3] 2.6.23-rc1: known regressions v2
On Friday 27 July 2007, Michal Piotrowski wrote: > IDE > > Subject : ide problems: 2.6.22-git17 working, 2.6.23-rc1* is not > References : http://lkml.org/lkml/2007/7/27/298 > Last known good : ? > Submitter : dth <[EMAIL PROTECTED]> > Caused-By : ? > Handled-By : ? > Status : unknown No IDE changes after 2.6.22-git17. Despite this I will try to follow this bugreport. Thanks, Bart - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/11] au1100fb: remove a bogus dma_free_nonconsistent call
On Tuesday, June 20, 2017 11:10:45 AM Christoph Hellwig wrote: > On Mon, Jun 19, 2017 at 01:15:04PM +0200, Bartlomiej Zolnierkiewicz wrote: > > On Friday, June 16, 2017 09:17:08 AM Christoph Hellwig wrote: > > > au1100fb is using managed dma allocations, so it doesn't need to > > > explicitly free the dma memory in the error path (and if it did > > > it would have to use the managed version). > > > > > > Signed-off-by: Christoph Hellwig > > > > Acked-by: Bartlomiej Zolnierkiewicz > > Are you fine with picking this up through the new dma-mapping tree? Yes, this is why Ack-ed it. If you prefer me to merge it through fbdev for 4.13 please let me know. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Re: [PATCH v3 11/14] nubus: Rename struct nubus_dev
On Tuesday, December 05, 2017 01:20:02 AM Finn Thain wrote: > It is misleading to call a functional resource a "device". In adopting > the Linux Driver Model, the struct device will be embedded in struct > nubus_board. That will compound the terminlogy problem because drivers > will bind with boards, not with functional resources. Avoid this by > renaming struct nubus_dev as struct nubus_rsrc. "Functional resource" > is the vendor's terminology so this helps avoid confusion. > > Cc: Bartlomiej Zolnierkiewicz > Tested-by: Stan Johnson > Signed-off-by: Finn Thain Acked-by: Bartlomiej Zolnierkiewicz for fbdev part > --- > drivers/net/ethernet/8390/mac8390.c | 26 > drivers/net/ethernet/natsemi/macsonic.c | 22 +++ > drivers/nubus/nubus.c | 105 > > drivers/nubus/proc.c| 15 ++--- > drivers/video/fbdev/macfb.c | 2 +- > include/linux/nubus.h | 30 + > 6 files changed, 98 insertions(+), 102 deletions(-) Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Re: [PATCH v3 12/14] nubus: Adopt standard linked list implementation
On Tuesday, December 05, 2017 01:20:02 AM Finn Thain wrote: > This increases code re-use and improves readability. > > Cc: Bartlomiej Zolnierkiewicz > Signed-off-by: Finn Thain > Tested-by: Stan Johnson Acked-by: Bartlomiej Zolnierkiewicz for fbdev part > --- > drivers/net/ethernet/8390/mac8390.c | 7 +++-- > drivers/net/ethernet/cirrus/mac89x0.c | 6 +++-- > drivers/net/ethernet/natsemi/macsonic.c | 8 +++--- > drivers/nubus/nubus.c | 45 > - > drivers/nubus/proc.c| 11 +++- > drivers/video/fbdev/macfb.c | 8 +++--- > include/linux/nubus.h | 15 +-- > 7 files changed, 40 insertions(+), 60 deletions(-) Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Re: [PATCH 03/11] au1100fb: remove a bogus dma_free_nonconsistent call
On Friday, June 16, 2017 09:17:08 AM Christoph Hellwig wrote: > au1100fb is using managed dma allocations, so it doesn't need to > explicitly free the dma memory in the error path (and if it did > it would have to use the managed version). > > Signed-off-by: Christoph Hellwig Acked-by: Bartlomiej Zolnierkiewicz Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Re: [PATCH v4 01/11] acpi: thermal: Fix error handling in the register function
On 5/28/20 9:20 PM, Andrzej Pietrasiewicz wrote:
> The acpi_thermal_register_thermal_zone() is missing any error handling.
> This needs to be fixed.
>
> Signed-off-by: Andrzej Pietrasiewicz
Reviewed-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> ---
> drivers/acpi/thermal.c | 20
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 19067a5e5293..6de8066ca1e7 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -901,23 +901,35 @@ static int acpi_thermal_register_thermal_zone(struct
> acpi_thermal *tz)
> result = sysfs_create_link(&tz->device->dev.kobj,
> &tz->thermal_zone->device.kobj,
> "thermal_zone");
> if (result)
> - return result;
> + goto unregister_tzd;
>
> result = sysfs_create_link(&tz->thermal_zone->device.kobj,
> &tz->device->dev.kobj, "device");
> if (result)
> - return result;
> + goto remove_tz_link;
>
> status = acpi_bus_attach_private_data(tz->device->handle,
> tz->thermal_zone);
> - if (ACPI_FAILURE(status))
> - return -ENODEV;
> + if (ACPI_FAILURE(status)) {
> + result = -ENODEV;
> + goto remove_dev_link;
> + }
>
> tz->tz_enabled = 1;
>
> dev_info(&tz->device->dev, "registered as thermal_zone%d\n",
>tz->thermal_zone->id);
> +
> return 0;
> +
> +remove_dev_link:
> + sysfs_remove_link(&tz->thermal_zone->device.kobj, "device");
> +remove_tz_link:
> + sysfs_remove_link(&tz->device->dev.kobj, "thermal_zone");
> +unregister_tzd:
> + thermal_zone_device_unregister(tz->thermal_zone);
> +
> + return result;
> }
>
> static void acpi_thermal_unregister_thermal_zone(struct acpi_thermal *tz)
>
Re: [PATCH v4 02/11] thermal: Store thermal mode in a dedicated enum
On 5/28/20 9:20 PM, Andrzej Pietrasiewicz wrote:
> Prepare for storing mode in struct thermal_zone_device.
>
> Signed-off-by: Andrzej Pietrasiewicz
Reviewed-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> ---
> drivers/acpi/thermal.c| 27 +--
> drivers/platform/x86/acerhdf.c| 8 --
> .../intel/int340x_thermal/int3400_thermal.c | 18 +
> 3 files changed, 25 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 6de8066ca1e7..fb46070c66d8 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -172,7 +172,7 @@ struct acpi_thermal {
> struct acpi_thermal_trips trips;
> struct acpi_handle_list devices;
> struct thermal_zone_device *thermal_zone;
> - int tz_enabled;
> + enum thermal_device_mode mode;
> int kelvin_offset; /* in millidegrees */
> struct work_struct thermal_check_work;
> };
> @@ -500,7 +500,7 @@ static void acpi_thermal_check(void *data)
> {
> struct acpi_thermal *tz = data;
>
> - if (!tz->tz_enabled)
> + if (tz->mode != THERMAL_DEVICE_ENABLED)
> return;
>
> thermal_zone_device_update(tz->thermal_zone,
> @@ -534,8 +534,7 @@ static int thermal_get_mode(struct thermal_zone_device
> *thermal,
> if (!tz)
> return -EINVAL;
>
> - *mode = tz->tz_enabled ? THERMAL_DEVICE_ENABLED :
> - THERMAL_DEVICE_DISABLED;
> + *mode = tz->mode;
>
> return 0;
> }
> @@ -544,27 +543,25 @@ static int thermal_set_mode(struct thermal_zone_device
> *thermal,
> enum thermal_device_mode mode)
> {
> struct acpi_thermal *tz = thermal->devdata;
> - int enable;
>
> if (!tz)
> return -EINVAL;
>
> + if (mode != THERMAL_DEVICE_DISABLED &&
> + mode != THERMAL_DEVICE_ENABLED)
> + return -EINVAL;
> /*
>* enable/disable thermal management from ACPI thermal driver
>*/
> - if (mode == THERMAL_DEVICE_ENABLED)
> - enable = 1;
> - else if (mode == THERMAL_DEVICE_DISABLED) {
> - enable = 0;
> + if (mode == THERMAL_DEVICE_DISABLED)
> pr_warn("thermal zone will be disabled\n");
> - } else
> - return -EINVAL;
>
> - if (enable != tz->tz_enabled) {
> - tz->tz_enabled = enable;
> + if (mode != tz->mode) {
> + tz->mode = mode;
> ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> "%s kernel ACPI thermal control\n",
> - tz->tz_enabled ? "Enable" : "Disable"));
> + tz->mode == THERMAL_DEVICE_ENABLED ?
> + "Enable" : "Disable"));
> acpi_thermal_check(tz);
> }
> return 0;
> @@ -915,7 +912,7 @@ static int acpi_thermal_register_thermal_zone(struct
> acpi_thermal *tz)
> goto remove_dev_link;
> }
>
> - tz->tz_enabled = 1;
> + tz->mode = THERMAL_DEVICE_ENABLED;
>
> dev_info(&tz->device->dev, "registered as thermal_zone%d\n",
>tz->thermal_zone->id);
> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
> index 8cc86f4e3ac1..830a8b060e74 100644
> --- a/drivers/platform/x86/acerhdf.c
> +++ b/drivers/platform/x86/acerhdf.c
> @@ -68,6 +68,7 @@ static int kernelmode = 1;
> #else
> static int kernelmode;
> #endif
> +static enum thermal_device_mode thermal_mode;
>
> static unsigned int interval = 10;
> static unsigned int fanon = 6;
> @@ -397,6 +398,7 @@ static inline void acerhdf_revert_to_bios_mode(void)
> {
> acerhdf_change_fanstate(ACERHDF_FAN_AUTO);
> kernelmode = 0;
> + thermal_mode = THERMAL_DEVICE_DISABLED;
> if (thz_dev)
> thz_dev->polling_delay = 0;
> pr_notice("kernel mode fan control OFF\n");
> @@ -404,6 +406,7 @@ static inline void acerhdf_revert_to_bios_mode(void)
> static inline void acerhdf_enable_kernelmode(void)
> {
> kernelmode = 1;
> + thermal_mode = THERMAL_DEVICE_ENABLED;
>
> thz_dev->polling_delay = interval*1000;
> thermal_zone_device_update(thz_dev, THERMAL_EVENT_UNSPECIFIED);
> @@ -416,8 +419,7 @@ static int acerhdf_get_mode(struct thermal_zone_device
> *thermal,
> if (verbose)
> pr_not
Re: [PATCH v4 03/11] thermal: Add current mode to thermal zone device
On 5/28/20 9:20 PM, Andrzej Pietrasiewicz wrote:
> Prepare for changing the place where the mode is stored: now it is in
> drivers, which might or might not implement get_mode()/set_mode() methods.
> A lot of cleanup can be done thanks to storing it in struct tzd. The
> get_mode() methods will become redundant.
>
> Signed-off-by: Andrzej Pietrasiewicz
Reviewed-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> ---
> include/linux/thermal.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 216185bb3014..5f91d7f04512 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -128,6 +128,7 @@ struct thermal_cooling_device {
> * @trip_temp_attrs: attributes for trip points for sysfs: trip temperature
> * @trip_type_attrs: attributes for trip points for sysfs: trip type
> * @trip_hyst_attrs: attributes for trip points for sysfs: trip hysteresis
> + * @mode:current mode of this thermal zone
> * @devdata: private pointer for device private data
> * @trips: number of trip points the thermal zone supports
> * @trips_disabled; bitmap for disabled trips
> @@ -170,6 +171,7 @@ struct thermal_zone_device {
> struct thermal_attr *trip_temp_attrs;
> struct thermal_attr *trip_type_attrs;
> struct thermal_attr *trip_hyst_attrs;
> + enum thermal_device_mode mode;
> void *devdata;
> int trips;
> unsigned long trips_disabled; /* bitmap for disabled trips */
>
Re: [PATCH v4 04/11] thermal: Store device mode in struct thermal_zone_device
On 5/28/20 9:20 PM, Andrzej Pietrasiewicz wrote:
> Prepare for eliminating get_mode().
>
> Signed-off-by: Andrzej Pietrasiewicz
Reviewed-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> ---
> drivers/acpi/thermal.c| 18 ++--
> .../ethernet/mellanox/mlxsw/core_thermal.c| 21 +++
> drivers/platform/x86/acerhdf.c| 15 ++---
> drivers/thermal/da9062-thermal.c | 6 ++
> drivers/thermal/imx_thermal.c | 17 +++
> .../intel/int340x_thermal/int3400_thermal.c | 12 +++
> .../thermal/intel/intel_quark_dts_thermal.c | 16 +++---
> drivers/thermal/thermal_of.c | 10 +++--
> 8 files changed, 44 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index fb46070c66d8..4ba273f49d87 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -172,7 +172,6 @@ struct acpi_thermal {
> struct acpi_thermal_trips trips;
> struct acpi_handle_list devices;
> struct thermal_zone_device *thermal_zone;
> - enum thermal_device_mode mode;
> int kelvin_offset; /* in millidegrees */
> struct work_struct thermal_check_work;
> };
> @@ -500,7 +499,7 @@ static void acpi_thermal_check(void *data)
> {
> struct acpi_thermal *tz = data;
>
> - if (tz->mode != THERMAL_DEVICE_ENABLED)
> + if (tz->thermal_zone->mode != THERMAL_DEVICE_ENABLED)
> return;
>
> thermal_zone_device_update(tz->thermal_zone,
> @@ -529,12 +528,7 @@ static int thermal_get_temp(struct thermal_zone_device
> *thermal, int *temp)
> static int thermal_get_mode(struct thermal_zone_device *thermal,
> enum thermal_device_mode *mode)
> {
> - struct acpi_thermal *tz = thermal->devdata;
> -
> - if (!tz)
> - return -EINVAL;
> -
> - *mode = tz->mode;
> + *mode = thermal->mode;
>
> return 0;
> }
> @@ -556,11 +550,11 @@ static int thermal_set_mode(struct thermal_zone_device
> *thermal,
> if (mode == THERMAL_DEVICE_DISABLED)
> pr_warn("thermal zone will be disabled\n");
>
> - if (mode != tz->mode) {
> - tz->mode = mode;
> + if (mode != tz->thermal_zone->mode) {
> + tz->thermal_zone->mode = mode;
> ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> "%s kernel ACPI thermal control\n",
> - tz->mode == THERMAL_DEVICE_ENABLED ?
> + tz->thermal_zone->mode == THERMAL_DEVICE_ENABLED ?
> "Enable" : "Disable"));
> acpi_thermal_check(tz);
> }
> @@ -912,7 +906,7 @@ static int acpi_thermal_register_thermal_zone(struct
> acpi_thermal *tz)
> goto remove_dev_link;
> }
>
> - tz->mode = THERMAL_DEVICE_ENABLED;
> + tz->thermal_zone->mode = THERMAL_DEVICE_ENABLED;
>
> dev_info(&tz->device->dev, "registered as thermal_zone%d\n",
>tz->thermal_zone->id);
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> index ce0a6837daa3..aa082e8a0b13 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> @@ -98,7 +98,6 @@ struct mlxsw_thermal_module {
> struct mlxsw_thermal *parent;
> struct thermal_zone_device *tzdev;
> struct mlxsw_thermal_trip trips[MLXSW_THERMAL_NUM_TRIPS];
> - enum thermal_device_mode mode;
> int module; /* Module or gearbox number */
> };
>
> @@ -110,7 +109,6 @@ struct mlxsw_thermal {
> struct thermal_cooling_device *cdevs[MLXSW_MFCR_PWMS_MAX];
> u8 cooling_levels[MLXSW_THERMAL_MAX_STATE + 1];
> struct mlxsw_thermal_trip trips[MLXSW_THERMAL_NUM_TRIPS];
> - enum thermal_device_mode mode;
> struct mlxsw_thermal_module *tz_module_arr;
> u8 tz_module_num;
> struct mlxsw_thermal_module *tz_gearbox_arr;
> @@ -280,9 +278,7 @@ static int mlxsw_thermal_unbind(struct
> thermal_zone_device *tzdev,
> static int mlxsw_thermal_get_mode(struct thermal_zone_device *tzdev,
> enum thermal_device_mode *mode)
> {
> - struct mlxsw_thermal *thermal = tzdev->devdata;
> -
> - *mode = thermal->mode;
> + *mode = tzdev->mode;
>
> return 0;
Re: [PATCH v4 05/11] thermal: remove get_mode() operation of drivers
On 5/28/20 9:20 PM, Andrzej Pietrasiewicz wrote:
> get_mode() is now redundant, as the state is stored in struct
> thermal_zone_device.
>
> Consequently the "mode" attribute in sysfs can always be visible, because
> it is always possible to get the mode from struct tzd.
>
> Signed-off-by: Andrzej Pietrasiewicz
The side-effect of splitting the v3 is that some devices will be
marked in sysfs as disabled (however they are in reality enabled)
after applying this patch (it gets fixed by patch #08).
I think that we can live with that so:
Reviewed-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> ---
> drivers/acpi/thermal.c| 9 --
> .../ethernet/mellanox/mlxsw/core_thermal.c| 19
> drivers/platform/x86/acerhdf.c| 12
> drivers/thermal/da9062-thermal.c | 8 -
> drivers/thermal/imx_thermal.c | 9 --
> .../intel/int340x_thermal/int3400_thermal.c | 9 --
> .../thermal/intel/intel_quark_dts_thermal.c | 8 -
> drivers/thermal/thermal_core.c| 7 +
> drivers/thermal/thermal_of.c | 9 --
> drivers/thermal/thermal_sysfs.c | 30 ++-
> include/linux/thermal.h | 2 --
> 11 files changed, 3 insertions(+), 119 deletions(-)
>
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 4ba273f49d87..592be97c4456 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -525,14 +525,6 @@ static int thermal_get_temp(struct thermal_zone_device
> *thermal, int *temp)
> return 0;
> }
>
> -static int thermal_get_mode(struct thermal_zone_device *thermal,
> - enum thermal_device_mode *mode)
> -{
> - *mode = thermal->mode;
> -
> - return 0;
> -}
> -
> static int thermal_set_mode(struct thermal_zone_device *thermal,
> enum thermal_device_mode mode)
> {
> @@ -847,7 +839,6 @@ static struct thermal_zone_device_ops
> acpi_thermal_zone_ops = {
> .bind = acpi_thermal_bind_cooling_device,
> .unbind = acpi_thermal_unbind_cooling_device,
> .get_temp = thermal_get_temp,
> - .get_mode = thermal_get_mode,
> .set_mode = thermal_set_mode,
> .get_trip_type = thermal_get_trip_type,
> .get_trip_temp = thermal_get_trip_temp,
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> index aa082e8a0b13..6e26678ac312 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> @@ -275,14 +275,6 @@ static int mlxsw_thermal_unbind(struct
> thermal_zone_device *tzdev,
> return 0;
> }
>
> -static int mlxsw_thermal_get_mode(struct thermal_zone_device *tzdev,
> - enum thermal_device_mode *mode)
> -{
> - *mode = tzdev->mode;
> -
> - return 0;
> -}
> -
> static int mlxsw_thermal_set_mode(struct thermal_zone_device *tzdev,
> enum thermal_device_mode mode)
> {
> @@ -403,7 +395,6 @@ static int mlxsw_thermal_trend_get(struct
> thermal_zone_device *tzdev,
> static struct thermal_zone_device_ops mlxsw_thermal_ops = {
> .bind = mlxsw_thermal_bind,
> .unbind = mlxsw_thermal_unbind,
> - .get_mode = mlxsw_thermal_get_mode,
> .set_mode = mlxsw_thermal_set_mode,
> .get_temp = mlxsw_thermal_get_temp,
> .get_trip_type = mlxsw_thermal_get_trip_type,
> @@ -462,14 +453,6 @@ static int mlxsw_thermal_module_unbind(struct
> thermal_zone_device *tzdev,
> return err;
> }
>
> -static int mlxsw_thermal_module_mode_get(struct thermal_zone_device *tzdev,
> - enum thermal_device_mode *mode)
> -{
> - *mode = tzdev->mode;
> -
> - return 0;
> -}
> -
> static int mlxsw_thermal_module_mode_set(struct thermal_zone_device *tzdev,
>enum thermal_device_mode mode)
> {
> @@ -591,7 +574,6 @@ mlxsw_thermal_module_trip_hyst_set(struct
> thermal_zone_device *tzdev, int trip,
> static struct thermal_zone_device_ops mlxsw_thermal_module_ops = {
> .bind = mlxsw_thermal_module_bind,
> .unbind = mlxsw_thermal_module_unbind,
> - .get_mode = mlxsw_thermal_module_mode_get,
> .set_mode = mlxsw_thermal_module_mode_set,
> .get_temp = mlxsw_thermal_module_temp_get,
> .get_trip_type = mlxsw_thermal_module_trip_type_get
Re: [PATCH v4 06/11] thermal: Add mode helpers
On 5/28/20 9:20 PM, Andrzej Pietrasiewicz wrote:
> Prepare for making the drivers not access tzd's private members.
>
> Signed-off-by: Andrzej Pietrasiewicz
Reviewed-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> ---
> drivers/thermal/thermal_core.c | 53 ++
> include/linux/thermal.h| 13 +
> 2 files changed, 66 insertions(+)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 14d3b1b94c4f..f2a5c5ee3455 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -459,6 +459,59 @@ static void thermal_zone_device_reset(struct
> thermal_zone_device *tz)
> thermal_zone_device_init(tz);
> }
>
> +int thermal_zone_device_set_mode(struct thermal_zone_device *tz,
> + enum thermal_device_mode mode)
> +{
> + int ret = 0;
> +
> + mutex_lock(&tz->lock);
> +
> + /* do nothing if mode isn't changing */
> + if (mode == tz->mode) {
> + mutex_unlock(&tz->lock);
> +
> + return ret;
> + }
> +
> + if (tz->ops->set_mode)
> + ret = tz->ops->set_mode(tz, mode);
> +
> + if (!ret)
> + tz->mode = mode;
> +
> + mutex_unlock(&tz->lock);
> +
> + thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> +
> + return ret;
> +}
> +
> +int thermal_zone_device_enable(struct thermal_zone_device *tz)
> +{
> + return thermal_zone_device_set_mode(tz, THERMAL_DEVICE_ENABLED);
> +}
> +EXPORT_SYMBOL(thermal_zone_device_enable);
> +
> +int thermal_zone_device_disable(struct thermal_zone_device *tz)
> +{
> + return thermal_zone_device_set_mode(tz, THERMAL_DEVICE_DISABLED);
> +}
> +EXPORT_SYMBOL(thermal_zone_device_disable);
> +
> +int thermal_zone_device_is_enabled(struct thermal_zone_device *tz)
> +{
> + enum thermal_device_mode mode;
> +
> + mutex_lock(&tz->lock);
> +
> + mode = tz->mode;
> +
> + mutex_unlock(&tz->lock);
> +
> + return mode == THERMAL_DEVICE_ENABLED;
> +}
> +EXPORT_SYMBOL(thermal_zone_device_is_enabled);
> +
> void thermal_zone_device_update(struct thermal_zone_device *tz,
> enum thermal_notify_event event)
> {
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index a808f6fa2777..df013c39ba9b 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -416,6 +416,9 @@ int thermal_zone_get_offset(struct thermal_zone_device
> *tz);
>
> void thermal_cdev_update(struct thermal_cooling_device *);
> void thermal_notify_framework(struct thermal_zone_device *, int);
> +int thermal_zone_device_enable(struct thermal_zone_device *tz);
> +int thermal_zone_device_disable(struct thermal_zone_device *tz);
> +int thermal_zone_device_is_enabled(struct thermal_zone_device *tz);
> #else
> static inline struct thermal_zone_device *thermal_zone_device_register(
> const char *type, int trips, int mask, void *devdata,
> @@ -463,6 +466,16 @@ static inline void thermal_cdev_update(struct
> thermal_cooling_device *cdev)
> static inline void thermal_notify_framework(struct thermal_zone_device *tz,
> int trip)
> { }
> +
> +static inline int thermal_zone_device_enable(struct thermal_zone_device *tz)
> +{ return -ENODEV; }
> +
> +static inline int thermal_zone_device_disable(struct thermal_zone_device *tz)
> +{ return -ENODEV; }
> +
> +static inline int
> +thermal_zone_device_is_enabled(struct thermal_zone_device *tz)
> +{ return -ENODEV; }
> #endif /* CONFIG_THERMAL */
>
> #endif /* __THERMAL_H__ */
>
Re: [PATCH v4 07/11] thermal: Use mode helpers in drivers
On 5/28/20 9:20 PM, Andrzej Pietrasiewicz wrote:
> Use thermal_zone_device_{en|dis}able() and thermal_zone_device_is_enabled().
>
> Consequently, all set_mode() implementations in drivers:
>
> - can stop modifying tzd's "mode" member,
> - shall stop taking tzd's lock, as it is taken in the helpers
> - shall stop calling thermal_zone_device_update() as it is called in the
> helpers
> - can assume they are called when the mode truly changes, so checks to
> verify that can be dropped
>
> Not providing set_mode() by a driver no longer prevents the core from
> being able to set tzd's mode, so the relevant check in mode_store() is
> removed.
>
> Other comments:
>
> - acpi/thermal.c: tz->thermal_zone->mode will be updated only after we
> return from set_mode(), so use function parameter in thermal_set_mode()
> instead, no need to call acpi_thermal_check() in set_mode()
> - thermal/imx_thermal.c: regmap writes and mode assignment are done in
> thermal_zone_device_{en|dis}able() and set_mode() callback
> - thermal/intel/intel_quark_dts_thermal.c: soc_dts_{en|dis}able() are a
> part of set_mode() callback, so they don't need to modify tzd->mode, and
> don't need to fall back to the opposite mode if unsuccessful, as the return
> value will be propagated to thermal_zone_device_{en|dis}able() and
> ultimately tzd's member will not be changed in thermal_zone_device_set_mode().
> - thermal/of-thermal.c: no need to set zone->mode to DISABLED in
> of_parse_thermal_zones() as a tzd is kzalloc'ed so mode is DISABLED anyway
>
> Signed-off-by: Andrzej Pietrasiewicz
> ---
> drivers/acpi/thermal.c| 21 ++-
> .../ethernet/mellanox/mlxsw/core_thermal.c| 37 +--
> drivers/platform/x86/acerhdf.c| 17 +
> drivers/thermal/da9062-thermal.c | 6 ++-
> drivers/thermal/hisi_thermal.c| 6 ++-
> drivers/thermal/imx_thermal.c | 33 +++--
> .../intel/int340x_thermal/int3400_thermal.c | 5 +--
> .../thermal/intel/intel_quark_dts_thermal.c | 18 ++---
> drivers/thermal/rockchip_thermal.c| 6 ++-
> drivers/thermal/sprd_thermal.c| 6 ++-
> drivers/thermal/thermal_core.c| 2 +-
> drivers/thermal/thermal_of.c | 10 +
> drivers/thermal/thermal_sysfs.c | 11 ++
> 13 files changed, 80 insertions(+), 98 deletions(-)
[...]
> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
> index 32c5fe16b7f7..3efe749dc5a0 100644
> --- a/drivers/platform/x86/acerhdf.c
> +++ b/drivers/platform/x86/acerhdf.c
> @@ -397,19 +397,16 @@ static inline void acerhdf_revert_to_bios_mode(void)
> {
> acerhdf_change_fanstate(ACERHDF_FAN_AUTO);
> kernelmode = 0;
> - if (thz_dev) {
> - thz_dev->mode = THERMAL_DEVICE_DISABLED;
> + if (thz_dev)
> thz_dev->polling_delay = 0;
> - }
> +
> pr_notice("kernel mode fan control OFF\n");
> }
> static inline void acerhdf_enable_kernelmode(void)
> {
> kernelmode = 1;
> - thz_dev->mode = THERMAL_DEVICE_ENABLED;
>
> thz_dev->polling_delay = interval*1000;
> - thermal_zone_device_update(thz_dev, THERMAL_EVENT_UNSPECIFIED);
> pr_notice("kernel mode fan control ON\n");
> }
>
> @@ -723,6 +720,8 @@ static void acerhdf_unregister_platform(void)
>
> static int __init acerhdf_register_thermal(void)
> {
> + int ret;
> +
> cl_dev = thermal_cooling_device_register("acerhdf-fan", NULL,
>&acerhdf_cooling_ops);
>
> @@ -736,8 +735,12 @@ static int __init acerhdf_register_thermal(void)
> if (IS_ERR(thz_dev))
> return -EINVAL;
>
> - thz_dev->mode = kernelmode ?
> - THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED;
> + if (kernelmode)
> + ret = thermal_zone_device_enable(thz_dev);
> + else
> + ret = thermal_zone_device_disable(thz_dev);
> + if (ret)
Cleanup on error seems to be missing here.
> + return ret;
>
> if (strcmp(thz_dev->governor->name,
> acerhdf_zone_params.governor_name)) {
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Re: [PATCH v4 08/11] thermal: Explicitly enable non-changing thermal zone devices
On 5/28/20 9:20 PM, Andrzej Pietrasiewicz wrote:
> Some thermal zone devices never change their state, so they should be
> always enabled.
>
> Signed-off-by: Andrzej Pietrasiewicz
> ---
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c | 8
> drivers/net/wireless/intel/iwlwifi/mvm/tt.c| 9 -
> drivers/platform/x86/intel_mid_thermal.c | 6 ++
> drivers/power/supply/power_supply_core.c | 9 +++--
> drivers/thermal/armada_thermal.c | 6 ++
> drivers/thermal/dove_thermal.c | 6 ++
> .../thermal/intel/int340x_thermal/int3400_thermal.c| 5 +
> .../intel/int340x_thermal/int340x_thermal_zone.c | 5 +
> drivers/thermal/intel/intel_pch_thermal.c | 5 +
> drivers/thermal/intel/intel_soc_dts_iosf.c | 3 +++
> drivers/thermal/intel/x86_pkg_temp_thermal.c | 6 ++
> drivers/thermal/kirkwood_thermal.c | 7 +++
> drivers/thermal/rcar_thermal.c | 9 -
> drivers/thermal/spear_thermal.c| 7 +++
> drivers/thermal/st/st_thermal.c| 5 +
> drivers/thermal/thermal_of.c | 10 +-
> 16 files changed, 101 insertions(+), 5 deletions(-)
[...]
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index 3c0397a29b8c..8e8c9af7e5f4 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -485,6 +485,10 @@ static int int3400_thermal_probe(struct platform_device
> *pdev)
> goto free_art_trt;
> }
>
> + result = thermal_zone_device_enable(priv->thermal);
I'm not sure about correctness of this addition.
This driver contains ->set_mode but doesn't call it on initialization
(in v3 it was using THERMAL_DEVICE_DISABLED as .initial_mode parameter).
> + if (result)
> + goto free_tzd;
> +
> priv->rel_misc_dev_res = acpi_thermal_rel_misc_device_add(
> priv->adev->handle);
>
> @@ -518,6 +522,7 @@ static int int3400_thermal_probe(struct platform_device
> *pdev)
> free_rel_misc:
> if (!priv->rel_misc_dev_res)
> acpi_thermal_rel_misc_device_remove(priv->adev->handle);
> +free_tzd:
> thermal_zone_device_unregister(priv->thermal);
> free_art_trt:
> kfree(priv->trts);
[...]
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index 43a516a35d64..011fd7f0a01e 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -1066,7 +1066,7 @@ int __init of_parse_thermal_zones(void)
> for_each_available_child_of_node(np, child) {
> struct thermal_zone_device *zone;
> struct thermal_zone_params *tzp;
> - int i, mask = 0;
> + int i, ret, mask = 0;
> u32 prop;
>
> tz = thermal_of_build_thermal_zone(child);
> @@ -1113,6 +1113,14 @@ int __init of_parse_thermal_zones(void)
> of_thermal_free_zone(tz);
> /* attempting to build remaining zones still */
> }
> + ret = thermal_zone_device_enable(zone);
This doesn't seem correct as it is done too early and
there is already proper thermal_zone_device_enable() call
in thermal_zone_of_sensor_register().
> + if (ret) {
> + thermal_zone_device_unregister(zone);
> + pr_err("Failed to enable thermal zone\n");
> + kfree(tzp);
> + kfree(ops);
> + of_thermal_free_zone(tz);
> + }
> }
> of_node_put(np);
>
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Re: [PATCH v4 09/11] thermal: core: Stop polling DISABLED thermal devices
On 5/28/20 9:20 PM, Andrzej Pietrasiewicz wrote:
> Polling DISABLED devices is not desired, as all such "disabled" devices
> are meant to be handled by userspace. This patch introduces and uses
> should_stop_polling() to decide whether the device should be polled or not.
>
> Signed-off-by: Andrzej Pietrasiewicz
Reviewed-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> ---
> drivers/thermal/thermal_core.c | 16 ++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 14baf0288759..e9c0b990e4a9 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -301,13 +301,22 @@ static void thermal_zone_device_set_polling(struct
> thermal_zone_device *tz,
> cancel_delayed_work(&tz->poll_queue);
> }
>
> +static inline bool should_stop_polling(struct thermal_zone_device *tz)
> +{
> + return !thermal_zone_device_is_enabled(tz);
> +}
> +
> static void monitor_thermal_zone(struct thermal_zone_device *tz)
> {
> + bool stop;
> +
> + stop = should_stop_polling(tz);
> +
> mutex_lock(&tz->lock);
>
> - if (tz->passive)
> + if (!stop && tz->passive)
> thermal_zone_device_set_polling(tz, tz->passive_delay);
> - else if (tz->polling_delay)
> + else if (!stop && tz->polling_delay)
> thermal_zone_device_set_polling(tz, tz->polling_delay);
> else
> thermal_zone_device_set_polling(tz, 0);
> @@ -517,6 +526,9 @@ void thermal_zone_device_update(struct
> thermal_zone_device *tz,
> {
> int count;
>
> + if (should_stop_polling(tz))
> + return;
> +
> if (atomic_read(&in_suspend))
> return;
>
>
Re: [PATCH v4 10/11] thermal: Simplify or eliminate unnecessary set_mode() methods
On 5/28/20 9:20 PM, Andrzej Pietrasiewicz wrote:
> Setting polling_delay is now done at thermal_core level (by not polling
> DISABLED devices), so no need to repeat this code.
>
> int340x: Checking for an impossible enum value is unnecessary.
> acpi/thermal: It only prints debug messages.
>
> Signed-off-by: Andrzej Pietrasiewicz
Reviewed-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> ---
> drivers/acpi/thermal.c| 26
> .../ethernet/mellanox/mlxsw/core_thermal.c| 30 ---
> drivers/platform/x86/acerhdf.c| 3 --
> drivers/thermal/imx_thermal.c | 6
> .../intel/int340x_thermal/int3400_thermal.c | 4 ---
> drivers/thermal/thermal_of.c | 18 ---
> 6 files changed, 87 deletions(-)
>
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 52b6cda1bcc3..29a2b73fe035 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -525,31 +525,6 @@ static int thermal_get_temp(struct thermal_zone_device
> *thermal, int *temp)
> return 0;
> }
>
> -static int thermal_set_mode(struct thermal_zone_device *thermal,
> - enum thermal_device_mode mode)
> -{
> - struct acpi_thermal *tz = thermal->devdata;
> -
> - if (!tz)
> - return -EINVAL;
> -
> - if (mode != THERMAL_DEVICE_DISABLED &&
> - mode != THERMAL_DEVICE_ENABLED)
> - return -EINVAL;
> - /*
> - * enable/disable thermal management from ACPI thermal driver
> - */
> - if (mode == THERMAL_DEVICE_DISABLED)
> - pr_warn("thermal zone will be disabled\n");
> -
> - ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> - "%s kernel ACPI thermal control\n",
> - mode == THERMAL_DEVICE_ENABLED ?
> - "Enable" : "Disable"));
> -
> - return 0;
> -}
> -
> static int thermal_get_trip_type(struct thermal_zone_device *thermal,
>int trip, enum thermal_trip_type *type)
> {
> @@ -836,7 +811,6 @@ static struct thermal_zone_device_ops
> acpi_thermal_zone_ops = {
> .bind = acpi_thermal_bind_cooling_device,
> .unbind = acpi_thermal_unbind_cooling_device,
> .get_temp = thermal_get_temp,
> - .set_mode = thermal_set_mode,
> .get_trip_type = thermal_get_trip_type,
> .get_trip_temp = thermal_get_trip_temp,
> .get_crit_temp = thermal_get_crit_temp,
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> index e1d800be8bb4..c7f334383912 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> @@ -275,19 +275,6 @@ static int mlxsw_thermal_unbind(struct
> thermal_zone_device *tzdev,
> return 0;
> }
>
> -static int mlxsw_thermal_set_mode(struct thermal_zone_device *tzdev,
> - enum thermal_device_mode mode)
> -{
> - struct mlxsw_thermal *thermal = tzdev->devdata;
> -
> - if (mode == THERMAL_DEVICE_ENABLED)
> - tzdev->polling_delay = thermal->polling_delay;
> - else
> - tzdev->polling_delay = 0;
> -
> - return 0;
> -}
> -
> static int mlxsw_thermal_get_temp(struct thermal_zone_device *tzdev,
> int *p_temp)
> {
> @@ -388,7 +375,6 @@ static int mlxsw_thermal_trend_get(struct
> thermal_zone_device *tzdev,
> static struct thermal_zone_device_ops mlxsw_thermal_ops = {
> .bind = mlxsw_thermal_bind,
> .unbind = mlxsw_thermal_unbind,
> - .set_mode = mlxsw_thermal_set_mode,
> .get_temp = mlxsw_thermal_get_temp,
> .get_trip_type = mlxsw_thermal_get_trip_type,
> .get_trip_temp = mlxsw_thermal_get_trip_temp,
> @@ -446,20 +432,6 @@ static int mlxsw_thermal_module_unbind(struct
> thermal_zone_device *tzdev,
> return err;
> }
>
> -static int mlxsw_thermal_module_mode_set(struct thermal_zone_device *tzdev,
> - enum thermal_device_mode mode)
> -{
> - struct mlxsw_thermal_module *tz = tzdev->devdata;
> - struct mlxsw_thermal *thermal = tz->parent;
> -
> - if (mode == THERMAL_DEVICE_ENABLED)
> - tzdev->polling_delay = thermal->polling_delay;
> - else
> - tzdev->polling_delay = 0;
> -
> - return 0;
> -}
> -
> static int mlxsw_thermal_module_temp_get(struct thermal_zon
Re: [PATCH v4 11/11] thermal: Rename set_mode() to change_mode()
On 5/28/20 9:20 PM, Andrzej Pietrasiewicz wrote:
> set_mode() is only called when tzd's mode is about to change. Actual
> setting is performed in thermal_core, in thermal_zone_device_set_mode().
> The meaning of set_mode() callback is actually to notify the driver about
> the mode being changed and giving the driver a chance to oppose such
> change.
>
> To better reflect the purpose of the method rename it to change_mode()
>
> Signed-off-by: Andrzej Pietrasiewicz
Reviewed-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> ---
> drivers/platform/x86/acerhdf.c | 6 +++---
> drivers/thermal/imx_thermal.c | 8
> drivers/thermal/intel/int340x_thermal/int3400_thermal.c | 6 +++---
> drivers/thermal/intel/intel_quark_dts_thermal.c | 6 +++---
> drivers/thermal/thermal_core.c | 4 ++--
> include/linux/thermal.h | 2 +-
> 6 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
> index d33a70af0869..63b562e06d5c 100644
> --- a/drivers/platform/x86/acerhdf.c
> +++ b/drivers/platform/x86/acerhdf.c
> @@ -413,8 +413,8 @@ static inline void acerhdf_enable_kernelmode(void)
> * the temperature and the fan.
> * disabled: the BIOS takes control of the fan.
> */
> -static int acerhdf_set_mode(struct thermal_zone_device *thermal,
> - enum thermal_device_mode mode)
> +static int acerhdf_change_mode(struct thermal_zone_device *thermal,
> +enum thermal_device_mode mode)
> {
> if (mode == THERMAL_DEVICE_DISABLED && kernelmode)
> acerhdf_revert_to_bios_mode();
> @@ -473,7 +473,7 @@ static struct thermal_zone_device_ops acerhdf_dev_ops = {
> .bind = acerhdf_bind,
> .unbind = acerhdf_unbind,
> .get_temp = acerhdf_get_ec_temp,
> - .set_mode = acerhdf_set_mode,
> + .change_mode = acerhdf_change_mode,
> .get_trip_type = acerhdf_get_trip_type,
> .get_trip_hyst = acerhdf_get_trip_hyst,
> .get_trip_temp = acerhdf_get_trip_temp,
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index a02398118d88..9700ae39feb7 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -330,8 +330,8 @@ static int imx_get_temp(struct thermal_zone_device *tz,
> int *temp)
> return 0;
> }
>
> -static int imx_set_mode(struct thermal_zone_device *tz,
> - enum thermal_device_mode mode)
> +static int imx_change_mode(struct thermal_zone_device *tz,
> +enum thermal_device_mode mode)
> {
> struct imx_thermal_data *data = tz->devdata;
> struct regmap *map = data->tempmon;
> @@ -447,7 +447,7 @@ static struct thermal_zone_device_ops imx_tz_ops = {
> .bind = imx_bind,
> .unbind = imx_unbind,
> .get_temp = imx_get_temp,
> - .set_mode = imx_set_mode,
> + .change_mode = imx_change_mode,
> .get_trip_type = imx_get_trip_type,
> .get_trip_temp = imx_get_trip_temp,
> .get_crit_temp = imx_get_crit_temp,
> @@ -860,7 +860,7 @@ static int __maybe_unused imx_thermal_suspend(struct
> device *dev)
>* Need to disable thermal sensor, otherwise, when thermal core
>* try to get temperature before thermal sensor resume, a wrong
>* temperature will be read as the thermal sensor is powered
> - * down. This is done in set_mode() operation called from
> + * down. This is done in change_mode() operation called from
>* thermal_zone_device_disable()
>*/
> ret = thermal_zone_device_disable(data->tz);
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index 9af862ab9f65..58870d215471 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -377,8 +377,8 @@ static int int3400_thermal_get_temp(struct
> thermal_zone_device *thermal,
> return 0;
> }
>
> -static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
> - enum thermal_device_mode mode)
> +static int int3400_thermal_change_mode(struct thermal_zone_device *thermal,
> +enum thermal_device_mode mode)
> {
> struct int3400_thermal_priv *priv = thermal->devdata;
> int result = 0;
> @@ -399,7 +399,7 @@ static int int3400_thermal_set_mode(struct
Re: [RFC v3 1/2] thermal: core: Let thermal zone device's mode be stored in its struct
t rid of the specific call done inside and then > remove the ops. > > The initial_mode approach looks hackish, I suggest to make the default > the thermal zone disabled after creating and then explicitly enable it. > Note that is what do a lot of drivers already. > > Hopefully, these changes are git-bisect safe. > > Does it make sense ? Besides the requirement to split the patch it seems that the above list contains a lot of problematic areas with the existing thermal code yet to be addressed.. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Re: [PATCH v3 1/2] thermal: core: Let thermal zone device's mode be stored in its struct
On 4/23/20 6:57 PM, Andrzej Pietrasiewicz wrote:
> Thermal zone devices' mode is stored in individual drivers. This patch
> changes it so that mode is stored in struct thermal_zone_device instead.
>
> As a result all driver-specific variables storing the mode are not needed
> and are removed. Consequently, the get_mode() implementations have nothing
> to operate on and need to be removed, too.
>
> Some thermal framework functions are introduced:
>
> thermal_zone_device_get_mode()
> thermal_zone_device_set_mode()
> thermal_zone_device_enable()
> thermal_zone_device_disable()
>
> thermal_zone_device_get_mode() and its "set" counterpart take tzd's lock
> and the "set" calls driver's set_mode() if provided, so the latter must
> not take this lock again. At the end of the "set"
> thermal_zone_device_update() is called so drivers don't need to repeat this
> invocation in their specific set_mode() implementations.
>
> struct thermal_zone_params gains a new member called initial_mode, which
> is used to set tzd's mode at registration time and if tzp is not provided
> to thermal_zone_device_register() then it is assumed that the initial
> mode is THERMAL_DEVICE_ENABLED.
>
> The sysfs "mode" attribute is always exposed from now on, because all
> thermal zone devices now have their get_mode() implemented at the generic
> level and it is always available. Exposing "mode" doesn't hurt the drivers
> which don't provide their own set_mode(), because writing to "mode" will
> result in -EPERM, as expected.
>
> Signed-off-by: Andrzej Pietrasiewicz
Reviewed-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> ---
> drivers/acpi/thermal.c| 35 ++---
> .../ethernet/mellanox/mlxsw/core_thermal.c| 42
> drivers/platform/x86/acerhdf.c| 17 ++-
> drivers/thermal/da9062-thermal.c | 11 -
> drivers/thermal/hisi_thermal.c| 6 ++-
> drivers/thermal/imx_thermal.c | 36 ++
> .../intel/int340x_thermal/int3400_thermal.c | 31 ++--
> .../int340x_thermal/int340x_thermal_zone.c| 1 +
> .../thermal/intel/intel_quark_dts_thermal.c | 30 +++-
> drivers/thermal/intel/x86_pkg_temp_thermal.c | 1 +
> drivers/thermal/of-thermal.c | 24 +
> drivers/thermal/rockchip_thermal.c| 6 ++-
> drivers/thermal/sprd_thermal.c| 6 ++-
> drivers/thermal/thermal_core.c| 49 +++
> drivers/thermal/thermal_core.h| 3 ++
> drivers/thermal/thermal_sysfs.c | 29 ++-
> include/linux/thermal.h | 22 -
> 17 files changed, 105 insertions(+), 244 deletions(-)
>
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 19067a5e5293..d1f352ed5241 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -172,7 +172,6 @@ struct acpi_thermal {
> struct acpi_thermal_trips trips;
> struct acpi_handle_list devices;
> struct thermal_zone_device *thermal_zone;
> - int tz_enabled;
> int kelvin_offset; /* in millidegrees */
> struct work_struct thermal_check_work;
> };
> @@ -500,7 +499,7 @@ static void acpi_thermal_check(void *data)
> {
> struct acpi_thermal *tz = data;
>
> - if (!tz->tz_enabled)
> + if (tz->thermal_zone->mode != THERMAL_DEVICE_ENABLED)
> return;
>
> thermal_zone_device_update(tz->thermal_zone,
> @@ -526,25 +525,10 @@ static int thermal_get_temp(struct thermal_zone_device
> *thermal, int *temp)
> return 0;
> }
>
> -static int thermal_get_mode(struct thermal_zone_device *thermal,
> - enum thermal_device_mode *mode)
> -{
> - struct acpi_thermal *tz = thermal->devdata;
> -
> - if (!tz)
> - return -EINVAL;
> -
> - *mode = tz->tz_enabled ? THERMAL_DEVICE_ENABLED :
> - THERMAL_DEVICE_DISABLED;
> -
> - return 0;
> -}
> -
> static int thermal_set_mode(struct thermal_zone_device *thermal,
> enum thermal_device_mode mode)
> {
> struct acpi_thermal *tz = thermal->devdata;
> - int enable;
>
> if (!tz)
> return -EINVAL;
> @@ -552,20 +536,14 @@ static int thermal_set_mode(struct thermal_zone_device
> *thermal,
> /*
>* enable/disable thermal management from ACPI thermal driver
&g
Re: [PATCH v7 07/11] thermal: Use mode helpers in drivers
On 6/29/20 2:29 PM, Andrzej Pietrasiewicz wrote:
> Use thermal_zone_device_{en|dis}able() and thermal_zone_device_is_enabled().
>
> Consequently, all set_mode() implementations in drivers:
>
> - can stop modifying tzd's "mode" member,
> - shall stop taking tzd's lock, as it is taken in the helpers
> - shall stop calling thermal_zone_device_update() as it is called in the
> helpers
> - can assume they are called when the mode truly changes, so checks to
> verify that can be dropped
>
> Not providing set_mode() by a driver no longer prevents the core from
> being able to set tzd's mode, so the relevant check in mode_store() is
> removed.
>
> Other comments:
>
> - acpi/thermal.c: tz->thermal_zone->mode will be updated only after we
> return from set_mode(), so use function parameter in thermal_set_mode()
> instead, no need to call acpi_thermal_check() in set_mode()
> - thermal/imx_thermal.c: regmap writes and mode assignment are done in
> thermal_zone_device_{en|dis}able() and set_mode() callback
> - thermal/intel/intel_quark_dts_thermal.c: soc_dts_{en|dis}able() are a
> part of set_mode() callback, so they don't need to modify tzd->mode, and
> don't need to fall back to the opposite mode if unsuccessful, as the return
> value will be propagated to thermal_zone_device_{en|dis}able() and
> ultimately tzd's member will not be changed in thermal_zone_device_set_mode().
> - thermal/of-thermal.c: no need to set zone->mode to DISABLED in
> of_parse_thermal_zones() as a tzd is kzalloc'ed so mode is DISABLED anyway
>
> Signed-off-by: Andrzej Pietrasiewicz
> [for acerhdf]
> Acked-by: Peter Kaestle
Reviewed-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> ---
> drivers/acpi/thermal.c| 21 ++-
> .../ethernet/mellanox/mlxsw/core_thermal.c| 37 +--
> drivers/platform/x86/acerhdf.c| 17 +
> drivers/thermal/da9062-thermal.c | 6 ++-
> drivers/thermal/hisi_thermal.c| 6 ++-
> drivers/thermal/imx_thermal.c | 33 +++--
> .../intel/int340x_thermal/int3400_thermal.c | 5 +--
> .../thermal/intel/intel_quark_dts_thermal.c | 18 ++---
> drivers/thermal/rockchip_thermal.c| 6 ++-
> drivers/thermal/sprd_thermal.c| 6 ++-
> drivers/thermal/thermal_core.c| 2 +-
> drivers/thermal/thermal_of.c | 10 +
> drivers/thermal/thermal_sysfs.c | 11 ++
> 13 files changed, 80 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 592be97c4456..52b6cda1bcc3 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -499,7 +499,7 @@ static void acpi_thermal_check(void *data)
> {
> struct acpi_thermal *tz = data;
>
> - if (tz->thermal_zone->mode != THERMAL_DEVICE_ENABLED)
> + if (!thermal_zone_device_is_enabled(tz->thermal_zone))
> return;
>
> thermal_zone_device_update(tz->thermal_zone,
> @@ -542,14 +542,11 @@ static int thermal_set_mode(struct thermal_zone_device
> *thermal,
> if (mode == THERMAL_DEVICE_DISABLED)
> pr_warn("thermal zone will be disabled\n");
>
> - if (mode != tz->thermal_zone->mode) {
> - tz->thermal_zone->mode = mode;
> - ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> - "%s kernel ACPI thermal control\n",
> - tz->thermal_zone->mode == THERMAL_DEVICE_ENABLED ?
> - "Enable" : "Disable"));
> - acpi_thermal_check(tz);
> - }
> + ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> + "%s kernel ACPI thermal control\n",
> + mode == THERMAL_DEVICE_ENABLED ?
> + "Enable" : "Disable"));
> +
> return 0;
> }
>
> @@ -897,13 +894,17 @@ static int acpi_thermal_register_thermal_zone(struct
> acpi_thermal *tz)
> goto remove_dev_link;
> }
>
> - tz->thermal_zone->mode = THERMAL_DEVICE_ENABLED;
> + result = thermal_zone_device_enable(tz->thermal_zone);
> + if (result)
> + goto acpi_bus_detach;
>
> dev_info(&tz->device->dev, "registered as thermal_zone%d\n",
>tz->thermal_zone->id);
>
> return 0;
>
> +acpi_bus_detach:
> + acpi_bus_detach_private_data(tz->device->handle);
> remove_dev_link:
> sysfs_rem
Re: [PATCH v7 08/11] thermal: Explicitly enable non-changing thermal zone devices
On 6/29/20 2:29 PM, Andrzej Pietrasiewicz wrote:
> Some thermal zone devices never change their state, so they should be
> always enabled.
>
> Signed-off-by: Andrzej Pietrasiewicz
Reviewed-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> ---
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c | 8
> drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 9 -
> drivers/platform/x86/intel_mid_thermal.c | 6 ++
> drivers/power/supply/power_supply_core.c | 9 +++--
> drivers/thermal/armada_thermal.c | 6 ++
> drivers/thermal/dove_thermal.c | 6 ++
> .../thermal/intel/int340x_thermal/int340x_thermal_zone.c | 5 +
> drivers/thermal/intel/intel_pch_thermal.c| 5 +
> drivers/thermal/intel/intel_soc_dts_iosf.c | 3 +++
> drivers/thermal/intel/x86_pkg_temp_thermal.c | 6 ++
> drivers/thermal/kirkwood_thermal.c | 7 +++
> drivers/thermal/rcar_thermal.c | 9 -
> drivers/thermal/spear_thermal.c | 7 +++
> drivers/thermal/st/st_thermal.c | 5 +
> 14 files changed, 87 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
> b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
> index 3de8a5e83b6c..e3510e9b21f3 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
> @@ -92,6 +92,14 @@ int cxgb4_thermal_init(struct adapter *adap)
> ch_thermal->tzdev = NULL;
> return ret;
> }
> +
> + ret = thermal_zone_device_enable(ch_thermal->tzdev);
> + if (ret) {
> + dev_err(adap->pdev_dev, "Failed to enable thermal zone\n");
> + thermal_zone_device_unregister(adap->ch_thermal.tzdev);
> + return ret;
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
> b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
> index 418e59b7c671..0c95663bf9ed 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
> @@ -733,7 +733,7 @@ static struct thermal_zone_device_ops tzone_ops = {
>
> static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
> {
> - int i;
> + int i, ret;
> char name[16];
> static atomic_t counter = ATOMIC_INIT(0);
>
> @@ -759,6 +759,13 @@ static void iwl_mvm_thermal_zone_register(struct iwl_mvm
> *mvm)
> return;
> }
>
> + ret = thermal_zone_device_enable(mvm->tz_device.tzone);
> + if (ret) {
> + IWL_DEBUG_TEMP(mvm, "Failed to enable thermal zone\n");
> + thermal_zone_device_unregister(mvm->tz_device.tzone);
> + return;
> + }
> +
> /* 0 is a valid temperature,
>* so initialize the array with S16_MIN which invalid temperature
>*/
> diff --git a/drivers/platform/x86/intel_mid_thermal.c
> b/drivers/platform/x86/intel_mid_thermal.c
> index f402e2e74a38..f12f4e7bd971 100644
> --- a/drivers/platform/x86/intel_mid_thermal.c
> +++ b/drivers/platform/x86/intel_mid_thermal.c
> @@ -493,6 +493,12 @@ static int mid_thermal_probe(struct platform_device
> *pdev)
> ret = PTR_ERR(pinfo->tzd[i]);
> goto err;
> }
> + ret = thermal_zone_device_enable(pinfo->tzd[i]);
> + if (ret) {
> + kfree(td_info);
> + thermal_zone_device_unregister(pinfo->tzd[i]);
> + goto err;
> + }
> }
>
> pinfo->pdev = pdev;
> diff --git a/drivers/power/supply/power_supply_core.c
> b/drivers/power/supply/power_supply_core.c
> index 02b37fe6061c..90e56736d479 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -939,7 +939,7 @@ static struct thermal_zone_device_ops psy_tzd_ops = {
>
> static int psy_register_thermal(struct power_supply *psy)
> {
> - int i;
> + int i, ret;
>
> if (psy->desc->no_thermal)
> return 0;
> @@ -949,7 +949,12 @@ static int psy_register_thermal(struct power_supply *psy)
> if (psy->desc->properties[i] == POWER_SUPPLY_PROP_TEMP) {
> psy->tzd = thermal_zone_device_register(psy->desc->name,
>
