2014-01-19 05:55:16

by SeongJae Park

[permalink] [raw]
Subject: [PATCH] clk: export __clk_get_hw for re-use in others

Following build comes while modprobe process:
> ERROR: "__clk_get_hw" [drivers/clk/clk-max77686.ko] undefined!
> make[2]: *** [__modpost] Error 1
> make[1]: *** [modules] Error 2

Export the symbol to fix it and for other part's usecase.

Signed-off-by: SeongJae Park <[email protected]>
---
drivers/clk/clk.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 2b38dc9..3883fba 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -575,6 +575,7 @@ struct clk_hw *__clk_get_hw(struct clk *clk)
{
return !clk ? NULL : clk->hw;
}
+EXPORT_SYMBOL_GPL(__clk_get_hw);

u8 __clk_get_num_parents(struct clk *clk)
{
--
1.8.3.2


2014-01-19 17:36:39

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] clk: export __clk_get_hw for re-use in others

On Sun, Jan 19, 2014 at 02:55:07PM +0900, SeongJae Park wrote:
> Following build comes while modprobe process:
> > ERROR: "__clk_get_hw" [drivers/clk/clk-max77686.ko] undefined!
> > make[2]: *** [__modpost] Error 1
> > make[1]: *** [modules] Error 2
>
> Export the symbol to fix it and for other part's usecase.
>
> Signed-off-by: SeongJae Park <[email protected]>
> ---
> drivers/clk/clk.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 2b38dc9..3883fba 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -575,6 +575,7 @@ struct clk_hw *__clk_get_hw(struct clk *clk)
> {
> return !clk ? NULL : clk->hw;
> }
> +EXPORT_SYMBOL_GPL(__clk_get_hw);

__ functions should usually only be for "internal" use, why does this
get exported to modules? Why not just put it in a .h file?

greg k-h

2014-01-20 07:47:25

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH] clk: export __clk_get_hw for re-use in others

On Sun, Jan 19, 2014 at 9:37 AM, Greg KH <[email protected]> wrote:
> On Sun, Jan 19, 2014 at 02:55:07PM +0900, SeongJae Park wrote:
>> Following build comes while modprobe process:
>> > ERROR: "__clk_get_hw" [drivers/clk/clk-max77686.ko] undefined!
>> > make[2]: *** [__modpost] Error 1
>> > make[1]: *** [modules] Error 2
>>
>> Export the symbol to fix it and for other part's usecase.
>>
>> Signed-off-by: SeongJae Park <[email protected]>
>> ---
>> drivers/clk/clk.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index 2b38dc9..3883fba 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -575,6 +575,7 @@ struct clk_hw *__clk_get_hw(struct clk *clk)
>> {
>> return !clk ? NULL : clk->hw;
>> }
>> +EXPORT_SYMBOL_GPL(__clk_get_hw);
>
> __ functions should usually only be for "internal" use, why does this
> get exported to modules? Why not just put it in a .h file?

It was originally used only within the clock core but it is sensible
for hardware-specific clock drivers to use this as well. I plan to
audit all of the double-underscore functions in
include/linux/clk-provider.h for 3.15.

Regards,
Mike

>
> greg k-h

2014-01-20 08:07:48

by SeongJae Park

[permalink] [raw]
Subject: Re: [PATCH] clk: export __clk_get_hw for re-use in others

On Mon, Jan 20, 2014 at 4:47 PM, Mike Turquette <[email protected]> wrote:
> On Sun, Jan 19, 2014 at 9:37 AM, Greg KH <[email protected]> wrote:
>> On Sun, Jan 19, 2014 at 02:55:07PM +0900, SeongJae Park wrote:
>>> Following build comes while modprobe process:
>>> > ERROR: "__clk_get_hw" [drivers/clk/clk-max77686.ko] undefined!
>>> > make[2]: *** [__modpost] Error 1
>>> > make[1]: *** [modules] Error 2
>>>
>>> Export the symbol to fix it and for other part's usecase.
>>>
>>> Signed-off-by: SeongJae Park <[email protected]>
>>> ---
>>> drivers/clk/clk.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>>> index 2b38dc9..3883fba 100644
>>> --- a/drivers/clk/clk.c
>>> +++ b/drivers/clk/clk.c
>>> @@ -575,6 +575,7 @@ struct clk_hw *__clk_get_hw(struct clk *clk)
>>> {
>>> return !clk ? NULL : clk->hw;
>>> }
>>> +EXPORT_SYMBOL_GPL(__clk_get_hw);
>>
>> __ functions should usually only be for "internal" use, why does this
>> get exported to modules? Why not just put it in a .h file?
>
> It was originally used only within the clock core but it is sensible
> for hardware-specific clock drivers to use this as well. I plan to
> audit all of the double-underscore functions in
> include/linux/clk-provider.h for 3.15.
>
> Regards,
> Mike
>
Thank you very much for answering about it, Mike.

I agree Greg's indication and think Mike's explanation is reasonable.

So, I think it would be better to just export the symbol now
because it would be easier for future functions renaming and
similar issues were solved in this way in past:
https://lkml.org/lkml/2013/4/15/50

Or, maybe I can change the client code of __clk_get_hw to not use the function.

What do you think would be better to fix this build error? Or, do you
have better idea?
I will respect your opinion.

Thanks and Regards.
SeongJae Park.

>>
>> greg k-h

2014-01-22 03:06:20

by SeongJae Park

[permalink] [raw]
Subject: Re: [PATCH] clk: export __clk_get_hw for re-use in others

Dear Greg, Mike,

May I ask your answer or other opinion, please?

On Mon, Jan 20, 2014 at 5:07 PM, SeongJae Park <[email protected]> wrote:
> On Mon, Jan 20, 2014 at 4:47 PM, Mike Turquette <[email protected]> wrote:
>> On Sun, Jan 19, 2014 at 9:37 AM, Greg KH <[email protected]> wrote:
>>> On Sun, Jan 19, 2014 at 02:55:07PM +0900, SeongJae Park wrote:
>>>> Following build comes while modprobe process:
>>>> > ERROR: "__clk_get_hw" [drivers/clk/clk-max77686.ko] undefined!
>>>> > make[2]: *** [__modpost] Error 1
>>>> > make[1]: *** [modules] Error 2
>>>>
>>>> Export the symbol to fix it and for other part's usecase.
>>>>
>>>> Signed-off-by: SeongJae Park <[email protected]>
>>>> ---
>>>> drivers/clk/clk.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>>>> index 2b38dc9..3883fba 100644
>>>> --- a/drivers/clk/clk.c
>>>> +++ b/drivers/clk/clk.c
>>>> @@ -575,6 +575,7 @@ struct clk_hw *__clk_get_hw(struct clk *clk)
>>>> {
>>>> return !clk ? NULL : clk->hw;
>>>> }
>>>> +EXPORT_SYMBOL_GPL(__clk_get_hw);
>>>
>>> __ functions should usually only be for "internal" use, why does this
>>> get exported to modules? Why not just put it in a .h file?
>>
>> It was originally used only within the clock core but it is sensible
>> for hardware-specific clock drivers to use this as well. I plan to
>> audit all of the double-underscore functions in
>> include/linux/clk-provider.h for 3.15.
>>
>> Regards,
>> Mike
>>
> Thank you very much for answering about it, Mike.
>
> I agree Greg's indication and think Mike's explanation is reasonable.
>
> So, I think it would be better to just export the symbol now
> because it would be easier for future functions renaming and
> similar issues were solved in this way in past:
> https://lkml.org/lkml/2013/4/15/50
>
> Or, maybe I can change the client code of __clk_get_hw to not use the function.
>
> What do you think would be better to fix this build error? Or, do you
> have better idea?
> I will respect your opinion.
>
> Thanks and Regards.
> SeongJae Park.
>
>>>
>>> greg k-h

2014-01-22 04:58:56

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] clk: export __clk_get_hw for re-use in others

On Wed, Jan 22, 2014 at 12:05:57PM +0900, SeongJae Park wrote:
> Dear Greg, Mike,
>
> May I ask your answer or other opinion, please?

It's the middle of the merge window, it's not time for new development,
or much time for free-time for me, sorry. Feel free to fix it the best
way you know how.

greg k-h

2014-01-22 05:23:56

by SeongJae Park

[permalink] [raw]
Subject: Re: [PATCH] clk: export __clk_get_hw for re-use in others

On Wed, Jan 22, 2014 at 1:59 PM, Greg KH <[email protected]> wrote:
> On Wed, Jan 22, 2014 at 12:05:57PM +0900, SeongJae Park wrote:
>> Dear Greg, Mike,
>>
>> May I ask your answer or other opinion, please?
>
> It's the middle of the merge window, it's not time for new development,
> or much time for free-time for me, sorry. Feel free to fix it the best
> way you know how.

Oops, I've forgot about the merge window. Thank you very much for your
kind answer.
Sorry if I bothered you while you're in busy time.
Because the build problem is not a big deal because it exists only in
-next tree,
I will wait until merge window be closed and then fix it again if it
still exist.

SeongJae Park.
>
> greg k-h

2014-01-22 17:59:34

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH] clk: export __clk_get_hw for re-use in others

On 01/21/14 21:23, SeongJae Park wrote:
> On Wed, Jan 22, 2014 at 1:59 PM, Greg KH <[email protected]> wrote:
>> On Wed, Jan 22, 2014 at 12:05:57PM +0900, SeongJae Park wrote:
>>> Dear Greg, Mike,
>>>
>>> May I ask your answer or other opinion, please?
>> It's the middle of the merge window, it's not time for new development,
>> or much time for free-time for me, sorry. Feel free to fix it the best
>> way you know how.
> Oops, I've forgot about the merge window. Thank you very much for your
> kind answer.
> Sorry if I bothered you while you're in busy time.
> Because the build problem is not a big deal because it exists only in
> -next tree,
> I will wait until merge window be closed and then fix it again if it
> still exist.
>

I've already sent a patch that exports this and other clock provider
functions. Please use this one:

https://patchwork.kernel.org/patch/3507921/

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2014-01-22 18:11:27

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH] clk: export __clk_get_hw for re-use in others

On Wed, Jan 22, 2014 at 9:59 AM, Stephen Boyd <[email protected]> wrote:
> On 01/21/14 21:23, SeongJae Park wrote:
>> On Wed, Jan 22, 2014 at 1:59 PM, Greg KH <[email protected]> wrote:
>>> On Wed, Jan 22, 2014 at 12:05:57PM +0900, SeongJae Park wrote:
>>>> Dear Greg, Mike,
>>>>
>>>> May I ask your answer or other opinion, please?
>>> It's the middle of the merge window, it's not time for new development,
>>> or much time for free-time for me, sorry. Feel free to fix it the best
>>> way you know how.
>> Oops, I've forgot about the merge window. Thank you very much for your
>> kind answer.
>> Sorry if I bothered you while you're in busy time.
>> Because the build problem is not a big deal because it exists only in
>> -next tree,
>> I will wait until merge window be closed and then fix it again if it
>> still exist.
>>
>
> I've already sent a patch that exports this and other clock provider
> functions. Please use this one:
>
> https://patchwork.kernel.org/patch/3507921/

I'm going to take Stephen's patch into a fixes branch and send it as
part of a pull request. Maybe -rc1 or -rc2 at the latest.

Thanks all.

Regards,
Mike

>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation
>

2014-01-23 02:05:38

by SeongJae Park

[permalink] [raw]
Subject: Re: [PATCH] clk: export __clk_get_hw for re-use in others

On Thu, Jan 23, 2014 at 3:11 AM, Mike Turquette <[email protected]> wrote:
> On Wed, Jan 22, 2014 at 9:59 AM, Stephen Boyd <[email protected]> wrote:
>> On 01/21/14 21:23, SeongJae Park wrote:
>>> On Wed, Jan 22, 2014 at 1:59 PM, Greg KH <[email protected]> wrote:
>>>> On Wed, Jan 22, 2014 at 12:05:57PM +0900, SeongJae Park wrote:
>>>>> Dear Greg, Mike,
>>>>>
>>>>> May I ask your answer or other opinion, please?
>>>> It's the middle of the merge window, it's not time for new development,
>>>> or much time for free-time for me, sorry. Feel free to fix it the best
>>>> way you know how.
>>> Oops, I've forgot about the merge window. Thank you very much for your
>>> kind answer.
>>> Sorry if I bothered you while you're in busy time.
>>> Because the build problem is not a big deal because it exists only in
>>> -next tree,
>>> I will wait until merge window be closed and then fix it again if it
>>> still exist.
>>>
>>
>> I've already sent a patch that exports this and other clock provider
>> functions. Please use this one:
>>
>> https://patchwork.kernel.org/patch/3507921/
>
> I'm going to take Stephen's patch into a fixes branch and send it as
> part of a pull request. Maybe -rc1 or -rc2 at the latest.

Got it. Thank you for let me know :)

>
> Thanks all.
>
> Regards,
> Mike
>
>>
>> --
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> hosted by The Linux Foundation
>>

2014-01-29 07:29:14

by David Rientjes

[permalink] [raw]
Subject: Re: [PATCH] clk: export __clk_get_hw for re-use in others

On Wed, 22 Jan 2014, SeongJae Park wrote:

> Oops, I've forgot about the merge window. Thank you very much for your
> kind answer.
> Sorry if I bothered you while you're in busy time.
> Because the build problem is not a big deal because it exists only in
> -next tree,

This problem exists in Linus's tree, not only in -next.

2014-01-29 08:14:12

by SeongJae Park

[permalink] [raw]
Subject: Re: [PATCH] clk: export __clk_get_hw for re-use in others

Hi,

On Wed, Jan 29, 2014 at 4:29 PM, David Rientjes <[email protected]> wrote:
> On Wed, 22 Jan 2014, SeongJae Park wrote:
>
>> Oops, I've forgot about the merge window. Thank you very much for your
>> kind answer.
>> Sorry if I bothered you while you're in busy time.
>> Because the build problem is not a big deal because it exists only in
>> -next tree,
>
> This problem exists in Linus's tree, not only in -next.

Yes, it looks like the problem caused commit is in Linus's tree
now(Maybe between this merge window).

But, because similar and better patch was
submitted(https://patchwork.kernel.org/patch/3507921/)
before mine by Stephen and Mike said he will merge the patch during rc1 or rc2,
looks like there is nothing we can rather than just waiting rc1 or rc2.

If there is anything I am thinking or doing wrong, please let me know.

Thanks and Regards,
SeongJae Park