2023-12-15 20:03:11

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH v1 2/6] thermal: netlink: Pass pointers to thermal_notify_tz_trip_change()

From: Rafael J. Wysocki <[email protected]>

Instead of requiring the caller of thermal_notify_tz_trip_change() to
provide specific values needed to populate struct param in it, make it
extract those values from objects passed to it by the caller via const
pointers.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/thermal/thermal_netlink.c | 12 +++++++-----
drivers/thermal/thermal_netlink.h | 8 ++++----
drivers/thermal/thermal_trip.c | 8 ++------
3 files changed, 13 insertions(+), 15 deletions(-)

Index: linux-pm/drivers/thermal/thermal_netlink.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_netlink.c
+++ linux-pm/drivers/thermal/thermal_netlink.c
@@ -361,12 +361,14 @@ int thermal_notify_tz_trip_delete(int tz
return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_DELETE, &p);
}

-int thermal_notify_tz_trip_change(int tz_id, int trip_id, int trip_type,
- int trip_temp, int trip_hyst)
+int thermal_notify_tz_trip_change(const struct thermal_zone_device *tz,
+ const struct thermal_trip *trip)
{
- struct param p = { .tz_id = tz_id, .trip_id = trip_id,
- .trip_type = trip_type, .trip_temp = trip_temp,
- .trip_hyst = trip_hyst };
+ struct param p = { .tz_id = tz->id,
+ .trip_id = thermal_zone_trip_id(tz, trip),
+ .trip_type = trip->type,
+ .trip_temp = trip->temperature,
+ .trip_hyst = trip->hysteresis };

return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_CHANGE, &p);
}
Index: linux-pm/drivers/thermal/thermal_netlink.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_netlink.h
+++ linux-pm/drivers/thermal/thermal_netlink.h
@@ -23,8 +23,8 @@ int thermal_notify_tz_trip_up(int tz_id,
int thermal_notify_tz_trip_delete(int tz_id, int id);
int thermal_notify_tz_trip_add(int tz_id, int id, int type,
int temp, int hyst);
-int thermal_notify_tz_trip_change(int tz_id, int id, int type,
- int temp, int hyst);
+int thermal_notify_tz_trip_change(const struct thermal_zone_device *tz,
+ const struct thermal_trip *trip);
int thermal_notify_cdev_state_update(int cdev_id, int state);
int thermal_notify_cdev_add(int cdev_id, const char *name, int max_state);
int thermal_notify_cdev_delete(int cdev_id);
@@ -79,8 +79,8 @@ static inline int thermal_notify_tz_trip
return 0;
}

-static inline int thermal_notify_tz_trip_change(int tz_id, int id, int type,
- int temp, int hyst)
+static inline int thermal_notify_tz_trip_change(const struct thermal_zone_device *tz,
+ const struct thermal_trip *trip)
{
return 0;
}
Index: linux-pm/drivers/thermal/thermal_trip.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_trip.c
+++ linux-pm/drivers/thermal/thermal_trip.c
@@ -155,9 +155,7 @@ int thermal_zone_trip_id(const struct th
void thermal_zone_trip_updated(struct thermal_zone_device *tz,
const struct thermal_trip *trip)
{
- thermal_notify_tz_trip_change(tz->id, thermal_zone_trip_id(tz, trip),
- trip->type, trip->temperature,
- trip->hysteresis);
+ thermal_notify_tz_trip_change(tz, trip);
__thermal_zone_device_update(tz, THERMAL_TRIP_CHANGED);
}

@@ -168,8 +166,6 @@ void thermal_zone_set_trip_temp(struct t
return;

trip->temperature = temp;
- thermal_notify_tz_trip_change(tz->id, thermal_zone_trip_id(tz, trip),
- trip->type, trip->temperature,
- trip->hysteresis);
+ thermal_notify_tz_trip_change(tz, trip);
}
EXPORT_SYMBOL_GPL(thermal_zone_set_trip_temp);





2024-01-03 19:57:17

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH v1 2/6] thermal: netlink: Pass pointers to thermal_notify_tz_trip_change()



On 12/15/23 19:56, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Instead of requiring the caller of thermal_notify_tz_trip_change() to
> provide specific values needed to populate struct param in it, make it
> extract those values from objects passed to it by the caller via const
> pointers.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/thermal/thermal_netlink.c | 12 +++++++-----
> drivers/thermal/thermal_netlink.h | 8 ++++----
> drivers/thermal/thermal_trip.c | 8 ++------
> 3 files changed, 13 insertions(+), 15 deletions(-)
>
> Index: linux-pm/drivers/thermal/thermal_netlink.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_netlink.c
> +++ linux-pm/drivers/thermal/thermal_netlink.c
> @@ -361,12 +361,14 @@ int thermal_notify_tz_trip_delete(int tz
> return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_DELETE, &p);
> }
>
> -int thermal_notify_tz_trip_change(int tz_id, int trip_id, int trip_type,
> - int trip_temp, int trip_hyst)
> +int thermal_notify_tz_trip_change(const struct thermal_zone_device *tz,
> + const struct thermal_trip *trip)
> {
> - struct param p = { .tz_id = tz_id, .trip_id = trip_id,
> - .trip_type = trip_type, .trip_temp = trip_temp,
> - .trip_hyst = trip_hyst };
> + struct param p = { .tz_id = tz->id,
> + .trip_id = thermal_zone_trip_id(tz, trip),
> + .trip_type = trip->type,
> + .trip_temp = trip->temperature,
> + .trip_hyst = trip->hysteresis };
>
> return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_CHANGE, &p);
> }
> Index: linux-pm/drivers/thermal/thermal_netlink.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_netlink.h
> +++ linux-pm/drivers/thermal/thermal_netlink.h
> @@ -23,8 +23,8 @@ int thermal_notify_tz_trip_up(int tz_id,
> int thermal_notify_tz_trip_delete(int tz_id, int id);
> int thermal_notify_tz_trip_add(int tz_id, int id, int type,
> int temp, int hyst);
> -int thermal_notify_tz_trip_change(int tz_id, int id, int type,
> - int temp, int hyst);
> +int thermal_notify_tz_trip_change(const struct thermal_zone_device *tz,
> + const struct thermal_trip *trip);
> int thermal_notify_cdev_state_update(int cdev_id, int state);
> int thermal_notify_cdev_add(int cdev_id, const char *name, int max_state);
> int thermal_notify_cdev_delete(int cdev_id);
> @@ -79,8 +79,8 @@ static inline int thermal_notify_tz_trip
> return 0;
> }
>
> -static inline int thermal_notify_tz_trip_change(int tz_id, int id, int type,
> - int temp, int hyst)
> +static inline int thermal_notify_tz_trip_change(const struct thermal_zone_device *tz,

I was wondering if it's not too long line, but I can see it's common
in that header file. Shouldn't we break such lines like:

static inline
int thermal_notify_tz_trip_change(const struct thermal_zone_device *tz,
const struct thermal_trip *trip)

?
Although, it would apply to all other long lines in that header, so not
particularly to this $subject.

> + const struct thermal_trip *trip)
> {
> return 0;
> }
> Index: linux-pm/drivers/thermal/thermal_trip.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_trip.c
> +++ linux-pm/drivers/thermal/thermal_trip.c
> @@ -155,9 +155,7 @@ int thermal_zone_trip_id(const struct th
> void thermal_zone_trip_updated(struct thermal_zone_device *tz,
> const struct thermal_trip *trip)
> {
> - thermal_notify_tz_trip_change(tz->id, thermal_zone_trip_id(tz, trip),
> - trip->type, trip->temperature,
> - trip->hysteresis);
> + thermal_notify_tz_trip_change(tz, trip);
> __thermal_zone_device_update(tz, THERMAL_TRIP_CHANGED);
> }
>
> @@ -168,8 +166,6 @@ void thermal_zone_set_trip_temp(struct t
> return;
>
> trip->temperature = temp;
> - thermal_notify_tz_trip_change(tz->id, thermal_zone_trip_id(tz, trip),
> - trip->type, trip->temperature,
> - trip->hysteresis);
> + thermal_notify_tz_trip_change(tz, trip);
> }
> EXPORT_SYMBOL_GPL(thermal_zone_set_trip_temp);
>
>
>

Other than the comment above, LGTM.

Reviewed-by: Lukasz Luba <[email protected]>

2024-01-03 20:10:28

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v1 2/6] thermal: netlink: Pass pointers to thermal_notify_tz_trip_change()

On Wed, Jan 3, 2024 at 8:57 PM Lukasz Luba <[email protected]> wrote:
>
>
>
> On 12/15/23 19:56, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > Instead of requiring the caller of thermal_notify_tz_trip_change() to
> > provide specific values needed to populate struct param in it, make it
> > extract those values from objects passed to it by the caller via const
> > pointers.
> >
> > No intentional functional impact.
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> > drivers/thermal/thermal_netlink.c | 12 +++++++-----
> > drivers/thermal/thermal_netlink.h | 8 ++++----
> > drivers/thermal/thermal_trip.c | 8 ++------
> > 3 files changed, 13 insertions(+), 15 deletions(-)
> >
> > Index: linux-pm/drivers/thermal/thermal_netlink.c
> > ===================================================================
> > --- linux-pm.orig/drivers/thermal/thermal_netlink.c
> > +++ linux-pm/drivers/thermal/thermal_netlink.c
> > @@ -361,12 +361,14 @@ int thermal_notify_tz_trip_delete(int tz
> > return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_DELETE, &p);
> > }
> >
> > -int thermal_notify_tz_trip_change(int tz_id, int trip_id, int trip_type,
> > - int trip_temp, int trip_hyst)
> > +int thermal_notify_tz_trip_change(const struct thermal_zone_device *tz,
> > + const struct thermal_trip *trip)
> > {
> > - struct param p = { .tz_id = tz_id, .trip_id = trip_id,
> > - .trip_type = trip_type, .trip_temp = trip_temp,
> > - .trip_hyst = trip_hyst };
> > + struct param p = { .tz_id = tz->id,
> > + .trip_id = thermal_zone_trip_id(tz, trip),
> > + .trip_type = trip->type,
> > + .trip_temp = trip->temperature,
> > + .trip_hyst = trip->hysteresis };
> >
> > return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_TRIP_CHANGE, &p);
> > }
> > Index: linux-pm/drivers/thermal/thermal_netlink.h
> > ===================================================================
> > --- linux-pm.orig/drivers/thermal/thermal_netlink.h
> > +++ linux-pm/drivers/thermal/thermal_netlink.h
> > @@ -23,8 +23,8 @@ int thermal_notify_tz_trip_up(int tz_id,
> > int thermal_notify_tz_trip_delete(int tz_id, int id);
> > int thermal_notify_tz_trip_add(int tz_id, int id, int type,
> > int temp, int hyst);
> > -int thermal_notify_tz_trip_change(int tz_id, int id, int type,
> > - int temp, int hyst);
> > +int thermal_notify_tz_trip_change(const struct thermal_zone_device *tz,
> > + const struct thermal_trip *trip);
> > int thermal_notify_cdev_state_update(int cdev_id, int state);
> > int thermal_notify_cdev_add(int cdev_id, const char *name, int max_state);
> > int thermal_notify_cdev_delete(int cdev_id);
> > @@ -79,8 +79,8 @@ static inline int thermal_notify_tz_trip
> > return 0;
> > }
> >
> > -static inline int thermal_notify_tz_trip_change(int tz_id, int id, int type,
> > - int temp, int hyst)
> > +static inline int thermal_notify_tz_trip_change(const struct thermal_zone_device *tz,
>
> I was wondering if it's not too long line, but I can see it's common
> in that header file. Shouldn't we break such lines like:
>
> static inline
> int thermal_notify_tz_trip_change(const struct thermal_zone_device *tz,
> const struct thermal_trip *trip)
>
> ?

IMO the additional line break doesn't really make it easier to read.

And note that it is OK now to have code lines longer than 80 chars (as
long as they are not longer than 100 chars IIRC), although it is still
recommended (but not required any more) to observe the "old" 80 chars
length limit.

> Although, it would apply to all other long lines in that header, so not
> particularly to this $subject.

Well, right.

> > + const struct thermal_trip *trip)
> > {
> > return 0;
> > }
> > Index: linux-pm/drivers/thermal/thermal_trip.c
> > ===================================================================
> > --- linux-pm.orig/drivers/thermal/thermal_trip.c
> > +++ linux-pm/drivers/thermal/thermal_trip.c
> > @@ -155,9 +155,7 @@ int thermal_zone_trip_id(const struct th
> > void thermal_zone_trip_updated(struct thermal_zone_device *tz,
> > const struct thermal_trip *trip)
> > {
> > - thermal_notify_tz_trip_change(tz->id, thermal_zone_trip_id(tz, trip),
> > - trip->type, trip->temperature,
> > - trip->hysteresis);
> > + thermal_notify_tz_trip_change(tz, trip);
> > __thermal_zone_device_update(tz, THERMAL_TRIP_CHANGED);
> > }
> >
> > @@ -168,8 +166,6 @@ void thermal_zone_set_trip_temp(struct t
> > return;
> >
> > trip->temperature = temp;
> > - thermal_notify_tz_trip_change(tz->id, thermal_zone_trip_id(tz, trip),
> > - trip->type, trip->temperature,
> > - trip->hysteresis);
> > + thermal_notify_tz_trip_change(tz, trip);
> > }
> > EXPORT_SYMBOL_GPL(thermal_zone_set_trip_temp);
> >
> >
> >
>
> Other than the comment above, LGTM.
>
> Reviewed-by: Lukasz Luba <[email protected]>

Thanks!

2024-01-09 11:12:04

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH v1 2/6] thermal: netlink: Pass pointers to thermal_notify_tz_trip_change()

On 15/12/2023 20:56, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Instead of requiring the caller of thermal_notify_tz_trip_change() to
> provide specific values needed to populate struct param in it, make it
> extract those values from objects passed to it by the caller via const
> pointers.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>


Reviewed-by: Daniel Lezcano <[email protected]>

--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog