diff mbox series

[v3,05/12] thermal/core: Remove unneeded EXPORT_SYMBOLS

Message ID 20220703183059.4133659-6-daniel.lezcano@linexp.org
State Superseded
Headers show
Series thermal OF rework | expand

Commit Message

Daniel Lezcano July 3, 2022, 6:30 p.m. UTC
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(-)

Comments

Lukasz Luba July 4, 2022, 7:35 a.m. UTC | #1
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
Daniel Lezcano July 4, 2022, 9:14 p.m. UTC | #2
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.
Lukasz Luba July 5, 2022, 7:30 a.m. UTC | #3
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...
Rafael J. Wysocki July 5, 2022, 2:20 p.m. UTC | #4
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.
Lukasz Luba July 5, 2022, 2:47 p.m. UTC | #5
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.
Todd Kjos July 5, 2022, 4:26 p.m. UTC | #6
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 mbox series

Patch

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