| Message ID | 20220703183059.4133659-6-daniel.lezcano@linexp.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | thermal OF rework | expand |
Hi Daniel, (+Todd and Wei on CC) On 7/3/22 19:30, Daniel Lezcano wrote: > Different functions are exporting the symbols but are actually only > used by the thermal framework internals. Remove these EXPORT_SYMBOLS. > > Cc: Alexandre Bailon <abailon@baylibre.com> > Cc: Kevin Hilman <khilman@baylibre.com> > Cc; Eduardo Valentin <eduval@amazon.com> > Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org> > --- > drivers/thermal/thermal_helpers.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c > index 3edd047e144f..f4c1e87ef040 100644 > --- a/drivers/thermal/thermal_helpers.c > +++ b/drivers/thermal/thermal_helpers.c > @@ -39,7 +39,6 @@ int get_tz_trend(struct thermal_zone_device *tz, int trip) > > return trend; > } > -EXPORT_SYMBOL(get_tz_trend); > > struct thermal_instance * > get_thermal_instance(struct thermal_zone_device *tz, > @@ -228,7 +227,6 @@ void thermal_cdev_update(struct thermal_cooling_device *cdev) > } > mutex_unlock(&cdev->lock); > } > -EXPORT_SYMBOL(thermal_cdev_update); I wouldn't remove that export. I can see in my Pixel6 modules dir, that it's called in 7 places. I assume that in Android world this is common use. Regards, Lukasz
On 04/07/2022 09:35, Lukasz Luba wrote: > Hi Daniel, > > (+Todd and Wei on CC) > > > On 7/3/22 19:30, Daniel Lezcano wrote: [ ... ] >> } >> -EXPORT_SYMBOL(get_tz_trend); [ ... ] >> } >> -EXPORT_SYMBOL(thermal_cdev_update); > > I wouldn't remove that export. I can see in my Pixel6 modules dir, that > it's called in 7 places. > > I assume that in Android world this is common use. It is not possible to do changes taking into consideration out of tree code. Moreover there is logically no good reason to use the thermal_cdev_update() function from outside of the thermal core code.
On 7/4/22 22:14, Daniel Lezcano wrote: > On 04/07/2022 09:35, Lukasz Luba wrote: >> Hi Daniel, >> >> (+Todd and Wei on CC) >> >> >> On 7/3/22 19:30, Daniel Lezcano wrote: > > [ ... ] > >>> } >>> -EXPORT_SYMBOL(get_tz_trend); > > [ ... ] > >>> } >>> -EXPORT_SYMBOL(thermal_cdev_update); >> >> I wouldn't remove that export. I can see in my Pixel6 modules dir, that >> it's called in 7 places. >> >> I assume that in Android world this is common use. > > It is not possible to do changes taking into consideration out of tree > code. Moreover there is logically no good reason to use the > thermal_cdev_update() function from outside of the thermal core code. > I see your point which is 'upstream'. On the other hand the mostly deployed kernel is in Android devices and that brings a lot to the community. This symbol might also be used by other distros which might have modules for some accelerators, which also support tricky cooling. I would keep it as is...
On Tue, Jul 5, 2022 at 9:30 AM Lukasz Luba <lukasz.luba@arm.com> wrote: > > > > On 7/4/22 22:14, Daniel Lezcano wrote: > > On 04/07/2022 09:35, Lukasz Luba wrote: > >> Hi Daniel, > >> > >> (+Todd and Wei on CC) > >> > >> > >> On 7/3/22 19:30, Daniel Lezcano wrote: > > > > [ ... ] > > > >>> } > >>> -EXPORT_SYMBOL(get_tz_trend); > > > > [ ... ] > > > >>> } > >>> -EXPORT_SYMBOL(thermal_cdev_update); > >> > >> I wouldn't remove that export. I can see in my Pixel6 modules dir, that > >> it's called in 7 places. > >> > >> I assume that in Android world this is common use. > > > > It is not possible to do changes taking into consideration out of tree > > code. Moreover there is logically no good reason to use the > > thermal_cdev_update() function from outside of the thermal core code. > > > > I see your point which is 'upstream'. On the other hand the mostly > deployed kernel is in Android devices and that brings a lot to the > community. > > This symbol might also be used by other distros which might have > modules for some accelerators, which also support tricky cooling. > > I would keep it as is... I think that the long-term goal is to reduce differences between the mainline kernel and Android. From this angle, it would be good if Android was aware that the mainline did stuff especially for them and making them carry an extra patch would go a long way towards that purpose.
On 7/5/22 15:20, Rafael J. Wysocki wrote: > On Tue, Jul 5, 2022 at 9:30 AM Lukasz Luba <lukasz.luba@arm.com> wrote: >> >> >> >> On 7/4/22 22:14, Daniel Lezcano wrote: >>> On 04/07/2022 09:35, Lukasz Luba wrote: >>>> Hi Daniel, >>>> >>>> (+Todd and Wei on CC) >>>> >>>> >>>> On 7/3/22 19:30, Daniel Lezcano wrote: >>> >>> [ ... ] >>> >>>>> } >>>>> -EXPORT_SYMBOL(get_tz_trend); >>> >>> [ ... ] >>> >>>>> } >>>>> -EXPORT_SYMBOL(thermal_cdev_update); >>>> >>>> I wouldn't remove that export. I can see in my Pixel6 modules dir, that >>>> it's called in 7 places. >>>> >>>> I assume that in Android world this is common use. >>> >>> It is not possible to do changes taking into consideration out of tree >>> code. Moreover there is logically no good reason to use the >>> thermal_cdev_update() function from outside of the thermal core code. >>> >> >> I see your point which is 'upstream'. On the other hand the mostly >> deployed kernel is in Android devices and that brings a lot to the >> community. >> >> This symbol might also be used by other distros which might have >> modules for some accelerators, which also support tricky cooling. >> >> I would keep it as is... > > I think that the long-term goal is to reduce differences between the > mainline kernel and Android. From this angle, it would be good if > Android was aware that the mainline did stuff especially for them and > making them carry an extra patch would go a long way towards that > purpose. It's hard to judge sometimes especially on those small bits. I've just pointed out and shared the info that this symbol is used. What you will do with this it's up to you. You and Daniel are the maintainers of this subsystems and have long-term plans for it. Todd and Wei are on CC, so they will know about this change. My job finishes here.
On Mon, Jul 4, 2022 at 2:14 PM Daniel Lezcano <daniel.lezcano@linexp.org> wrote: > > On 04/07/2022 09:35, Lukasz Luba wrote: > > Hi Daniel, > > > > (+Todd and Wei on CC) > > > > > > On 7/3/22 19:30, Daniel Lezcano wrote: > > [ ... ] > > >> } > >> -EXPORT_SYMBOL(get_tz_trend); > > [ ... ] > > >> } > >> -EXPORT_SYMBOL(thermal_cdev_update); > > > > I wouldn't remove that export. I can see in my Pixel6 modules dir, that > > it's called in 7 places. > > > > I assume that in Android world this is common use. > > It is not possible to do changes taking into consideration out of tree > code. Moreover there is logically no good reason to use the > thermal_cdev_update() function from outside of the thermal core code. > I agree. It is totally appropriate for the export to be removed for these functions if the exports are only for out of tree code. If they are needed for Android, they can be carried in the Android kernel trees.
diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c index 3edd047e144f..f4c1e87ef040 100644 --- a/drivers/thermal/thermal_helpers.c +++ b/drivers/thermal/thermal_helpers.c @@ -39,7 +39,6 @@ int get_tz_trend(struct thermal_zone_device *tz, int trip) return trend; } -EXPORT_SYMBOL(get_tz_trend); struct thermal_instance * get_thermal_instance(struct thermal_zone_device *tz, @@ -228,7 +227,6 @@ void thermal_cdev_update(struct thermal_cooling_device *cdev) } mutex_unlock(&cdev->lock); } -EXPORT_SYMBOL(thermal_cdev_update); /** * thermal_zone_get_slope - return the slope attribute of the thermal zone
Different functions are exporting the symbols but are actually only used by the thermal framework internals. Remove these EXPORT_SYMBOLS. Cc: Alexandre Bailon <abailon@baylibre.com> Cc: Kevin Hilman <khilman@baylibre.com> Cc; Eduardo Valentin <eduval@amazon.com> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org> --- drivers/thermal/thermal_helpers.c | 2 -- 1 file changed, 2 deletions(-)