diff mbox series

[v2,6/7] clk: mediatek: Export required symbols to compile clk drivers as module

Message ID 20220518111652.223727-7-angelogioacchino.delregno@collabora.com (mailing list archive)
State New, archived
Headers show
Series MediaTek Helio X10 MT6795 - Clock drivers | expand

Commit Message

AngeloGioacchino Del Regno May 18, 2022, 11:16 a.m. UTC
In order to compile the clock drivers for various MediaTek SoCs as
modules, it is necessary to export a few functions from the MediaTek
specific clocks (and reset) libraries.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/clk/mediatek/clk-apmixed.c | 1 +
 drivers/clk/mediatek/clk-cpumux.c  | 2 ++
 drivers/clk/mediatek/clk-mtk.c     | 2 ++
 drivers/clk/mediatek/reset.c       | 1 +
 4 files changed, 6 insertions(+)

Comments

Miles Chen May 19, 2022, 4:41 a.m. UTC | #1
Hi Angelo,

>In order to compile the clock drivers for various MediaTek SoCs as
>modules, it is necessary to export a few functions from the MediaTek
>specific clocks (and reset) libraries.
>
>Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>---
> drivers/clk/mediatek/clk-apmixed.c | 1 +
> drivers/clk/mediatek/clk-cpumux.c  | 2 ++
> drivers/clk/mediatek/clk-mtk.c     | 2 ++
> drivers/clk/mediatek/reset.c       | 1 +
> 4 files changed, 6 insertions(+)
>
>diff --git a/drivers/clk/mediatek/clk-apmixed.c b/drivers/clk/mediatek/clk-apmixed.c
>index 6b0ab0a346e8..f126da693a7f 100644
>--- a/drivers/clk/mediatek/clk-apmixed.c
>+++ b/drivers/clk/mediatek/clk-apmixed.c
>@@ -98,5 +98,6 @@ struct clk_hw *mtk_clk_register_ref2usb_tx(const char *name,
> 
> 	return &tx->hw;
> }
>+EXPORT_SYMBOL_GPL(mtk_clk_register_ref2usb_tx);
> 
> MODULE_LICENSE("GPL");
>diff --git a/drivers/clk/mediatek/clk-cpumux.c b/drivers/clk/mediatek/clk-cpumux.c
>index 2b5d48591738..25618eff6f2a 100644
>--- a/drivers/clk/mediatek/clk-cpumux.c
>+++ b/drivers/clk/mediatek/clk-cpumux.c
>@@ -150,6 +150,7 @@ int mtk_clk_register_cpumuxes(struct device_node *node,
> 
> 	return PTR_ERR(hw);
> }
>+EXPORT_SYMBOL_GPL(mtk_clk_register_cpumuxes);
> 
> void mtk_clk_unregister_cpumuxes(const struct mtk_composite *clks, int num,
> 				 struct clk_hw_onecell_data *clk_data)
>@@ -166,5 +167,6 @@ void mtk_clk_unregister_cpumuxes(const struct mtk_composite *clks, int num,
> 		clk_data->hws[mux->id] = ERR_PTR(-ENOENT);
> 	}
> }
>+EXPORT_SYMBOL_GPL(mtk_clk_unregister_cpumuxes);
> 
> MODULE_LICENSE("GPL");
>diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
>index 05a188c62119..41e60a7e8ff9 100644
>--- a/drivers/clk/mediatek/clk-mtk.c
>+++ b/drivers/clk/mediatek/clk-mtk.c
>@@ -459,6 +459,7 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
> 	mtk_free_clk_data(clk_data);
> 	return r;
> }
>+EXPORT_SYMBOL_GPL(mtk_clk_simple_probe);
> 
> int mtk_clk_simple_remove(struct platform_device *pdev)
> {
>@@ -472,5 +473,6 @@ int mtk_clk_simple_remove(struct platform_device *pdev)
> 
> 	return 0;
> }
>+EXPORT_SYMBOL_GPL(mtk_clk_simple_remove);

Thanks, I need this too. I am preparing a patch to use mtk_clk_simple_remove/mtk_clk_simple_probe
for MT6779 clks first and maybe I can apply this to all MediaTek clk drivers.

Reviewed-by: Miles Chen <miles.chen@mediatek.com> 

thanks,
Miles
> 
> MODULE_LICENSE("GPL");
>diff --git a/drivers/clk/mediatek/reset.c b/drivers/clk/mediatek/reset.c
>index 179505549a7c..290ceda84ce4 100644
>--- a/drivers/clk/mediatek/reset.c
>+++ b/drivers/clk/mediatek/reset.c
>@@ -228,5 +228,6 @@ int mtk_register_reset_controller_with_dev(struct device *dev,
> 
> 	return 0;
> }
>+EXPORT_SYMBOL_GPL(mtk_register_reset_controller_with_dev);
> 
> MODULE_LICENSE("GPL");
>-- 
>2.35.1
>
>
AngeloGioacchino Del Regno May 19, 2022, 8:05 a.m. UTC | #2
Il 19/05/22 06:41, Miles Chen ha scritto:
> 
> Hi Angelo,
> 
>> In order to compile the clock drivers for various MediaTek SoCs as
>> modules, it is necessary to export a few functions from the MediaTek
>> specific clocks (and reset) libraries.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>> drivers/clk/mediatek/clk-apmixed.c | 1 +
>> drivers/clk/mediatek/clk-cpumux.c  | 2 ++
>> drivers/clk/mediatek/clk-mtk.c     | 2 ++
>> drivers/clk/mediatek/reset.c       | 1 +
>> 4 files changed, 6 insertions(+)
>>
>> diff --git a/drivers/clk/mediatek/clk-apmixed.c b/drivers/clk/mediatek/clk-apmixed.c
>> index 6b0ab0a346e8..f126da693a7f 100644
>> --- a/drivers/clk/mediatek/clk-apmixed.c
>> +++ b/drivers/clk/mediatek/clk-apmixed.c
>> @@ -98,5 +98,6 @@ struct clk_hw *mtk_clk_register_ref2usb_tx(const char *name,
>>
>> 	return &tx->hw;
>> }
>> +EXPORT_SYMBOL_GPL(mtk_clk_register_ref2usb_tx);
>>
>> MODULE_LICENSE("GPL");
>> diff --git a/drivers/clk/mediatek/clk-cpumux.c b/drivers/clk/mediatek/clk-cpumux.c
>> index 2b5d48591738..25618eff6f2a 100644
>> --- a/drivers/clk/mediatek/clk-cpumux.c
>> +++ b/drivers/clk/mediatek/clk-cpumux.c
>> @@ -150,6 +150,7 @@ int mtk_clk_register_cpumuxes(struct device_node *node,
>>
>> 	return PTR_ERR(hw);
>> }
>> +EXPORT_SYMBOL_GPL(mtk_clk_register_cpumuxes);
>>
>> void mtk_clk_unregister_cpumuxes(const struct mtk_composite *clks, int num,
>> 				 struct clk_hw_onecell_data *clk_data)
>> @@ -166,5 +167,6 @@ void mtk_clk_unregister_cpumuxes(const struct mtk_composite *clks, int num,
>> 		clk_data->hws[mux->id] = ERR_PTR(-ENOENT);
>> 	}
>> }
>> +EXPORT_SYMBOL_GPL(mtk_clk_unregister_cpumuxes);
>>
>> MODULE_LICENSE("GPL");
>> diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
>> index 05a188c62119..41e60a7e8ff9 100644
>> --- a/drivers/clk/mediatek/clk-mtk.c
>> +++ b/drivers/clk/mediatek/clk-mtk.c
>> @@ -459,6 +459,7 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
>> 	mtk_free_clk_data(clk_data);
>> 	return r;
>> }
>> +EXPORT_SYMBOL_GPL(mtk_clk_simple_probe);
>>
>> int mtk_clk_simple_remove(struct platform_device *pdev)
>> {
>> @@ -472,5 +473,6 @@ int mtk_clk_simple_remove(struct platform_device *pdev)
>>
>> 	return 0;
>> }
>> +EXPORT_SYMBOL_GPL(mtk_clk_simple_remove);
> 
> Thanks, I need this too. I am preparing a patch to use mtk_clk_simple_remove/mtk_clk_simple_probe
> for MT6779 clks first and maybe I can apply this to all MediaTek clk drivers.
> 
> Reviewed-by: Miles Chen <miles.chen@mediatek.com>

Hello Miles,

thanks for telling me, because my next step would have been exactly what
you're doing, for all MediaTek clk drivers... otherwise we'd be doing
redundant work going afterwards.

Regards,
Angelo

> 
> thanks,
> Miles
>>
>> MODULE_LICENSE("GPL");
>> diff --git a/drivers/clk/mediatek/reset.c b/drivers/clk/mediatek/reset.c
>> index 179505549a7c..290ceda84ce4 100644
>> --- a/drivers/clk/mediatek/reset.c
>> +++ b/drivers/clk/mediatek/reset.c
>> @@ -228,5 +228,6 @@ int mtk_register_reset_controller_with_dev(struct device *dev,
>>
>> 	return 0;
>> }
>> +EXPORT_SYMBOL_GPL(mtk_register_reset_controller_with_dev);
>>
>> MODULE_LICENSE("GPL");
>> -- 
>> 2.35.1
>>
>>
Chen-Yu Tsai May 19, 2022, 8:15 a.m. UTC | #3
On Thu, May 19, 2022 at 4:05 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 19/05/22 06:41, Miles Chen ha scritto:
> >
> > Hi Angelo,
> >
> >> In order to compile the clock drivers for various MediaTek SoCs as
> >> modules, it is necessary to export a few functions from the MediaTek
> >> specific clocks (and reset) libraries.
> >>
> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >> ---
> >> drivers/clk/mediatek/clk-apmixed.c | 1 +
> >> drivers/clk/mediatek/clk-cpumux.c  | 2 ++
> >> drivers/clk/mediatek/clk-mtk.c     | 2 ++
> >> drivers/clk/mediatek/reset.c       | 1 +
> >> 4 files changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/clk/mediatek/clk-apmixed.c b/drivers/clk/mediatek/clk-apmixed.c
> >> index 6b0ab0a346e8..f126da693a7f 100644
> >> --- a/drivers/clk/mediatek/clk-apmixed.c
> >> +++ b/drivers/clk/mediatek/clk-apmixed.c
> >> @@ -98,5 +98,6 @@ struct clk_hw *mtk_clk_register_ref2usb_tx(const char *name,
> >>
> >>      return &tx->hw;
> >> }
> >> +EXPORT_SYMBOL_GPL(mtk_clk_register_ref2usb_tx);
> >>
> >> MODULE_LICENSE("GPL");
> >> diff --git a/drivers/clk/mediatek/clk-cpumux.c b/drivers/clk/mediatek/clk-cpumux.c
> >> index 2b5d48591738..25618eff6f2a 100644
> >> --- a/drivers/clk/mediatek/clk-cpumux.c
> >> +++ b/drivers/clk/mediatek/clk-cpumux.c
> >> @@ -150,6 +150,7 @@ int mtk_clk_register_cpumuxes(struct device_node *node,
> >>
> >>      return PTR_ERR(hw);
> >> }
> >> +EXPORT_SYMBOL_GPL(mtk_clk_register_cpumuxes);
> >>
> >> void mtk_clk_unregister_cpumuxes(const struct mtk_composite *clks, int num,
> >>                               struct clk_hw_onecell_data *clk_data)
> >> @@ -166,5 +167,6 @@ void mtk_clk_unregister_cpumuxes(const struct mtk_composite *clks, int num,
> >>              clk_data->hws[mux->id] = ERR_PTR(-ENOENT);
> >>      }
> >> }
> >> +EXPORT_SYMBOL_GPL(mtk_clk_unregister_cpumuxes);
> >>
> >> MODULE_LICENSE("GPL");
> >> diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
> >> index 05a188c62119..41e60a7e8ff9 100644
> >> --- a/drivers/clk/mediatek/clk-mtk.c
> >> +++ b/drivers/clk/mediatek/clk-mtk.c
> >> @@ -459,6 +459,7 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
> >>      mtk_free_clk_data(clk_data);
> >>      return r;
> >> }
> >> +EXPORT_SYMBOL_GPL(mtk_clk_simple_probe);
> >>
> >> int mtk_clk_simple_remove(struct platform_device *pdev)
> >> {
> >> @@ -472,5 +473,6 @@ int mtk_clk_simple_remove(struct platform_device *pdev)
> >>
> >>      return 0;
> >> }
> >> +EXPORT_SYMBOL_GPL(mtk_clk_simple_remove);
> >
> > Thanks, I need this too. I am preparing a patch to use mtk_clk_simple_remove/mtk_clk_simple_probe
> > for MT6779 clks first and maybe I can apply this to all MediaTek clk drivers.
> >
> > Reviewed-by: Miles Chen <miles.chen@mediatek.com>
>
> Hello Miles,
>
> thanks for telling me, because my next step would have been exactly what
> you're doing, for all MediaTek clk drivers... otherwise we'd be doing
> redundant work going afterwards.

Should we consider using symbol namespaces (EXPORT_SYMBOL_NS)?

ChenYu

> Regards,
> Angelo
>
> >
> > thanks,
> > Miles
> >>
> >> MODULE_LICENSE("GPL");
> >> diff --git a/drivers/clk/mediatek/reset.c b/drivers/clk/mediatek/reset.c
> >> index 179505549a7c..290ceda84ce4 100644
> >> --- a/drivers/clk/mediatek/reset.c
> >> +++ b/drivers/clk/mediatek/reset.c
> >> @@ -228,5 +228,6 @@ int mtk_register_reset_controller_with_dev(struct device *dev,
> >>
> >>      return 0;
> >> }
> >> +EXPORT_SYMBOL_GPL(mtk_register_reset_controller_with_dev);
> >>
> >> MODULE_LICENSE("GPL");
> >> --
> >> 2.35.1
> >>
> >>
>
AngeloGioacchino Del Regno May 19, 2022, 8:26 a.m. UTC | #4
Il 19/05/22 10:15, Chen-Yu Tsai ha scritto:
> On Thu, May 19, 2022 at 4:05 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 19/05/22 06:41, Miles Chen ha scritto:
>>>
>>> Hi Angelo,
>>>
>>>> In order to compile the clock drivers for various MediaTek SoCs as
>>>> modules, it is necessary to export a few functions from the MediaTek
>>>> specific clocks (and reset) libraries.
>>>>
>>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>>> ---
>>>> drivers/clk/mediatek/clk-apmixed.c | 1 +
>>>> drivers/clk/mediatek/clk-cpumux.c  | 2 ++
>>>> drivers/clk/mediatek/clk-mtk.c     | 2 ++
>>>> drivers/clk/mediatek/reset.c       | 1 +
>>>> 4 files changed, 6 insertions(+)
>>>>
>>>> diff --git a/drivers/clk/mediatek/clk-apmixed.c b/drivers/clk/mediatek/clk-apmixed.c
>>>> index 6b0ab0a346e8..f126da693a7f 100644
>>>> --- a/drivers/clk/mediatek/clk-apmixed.c
>>>> +++ b/drivers/clk/mediatek/clk-apmixed.c
>>>> @@ -98,5 +98,6 @@ struct clk_hw *mtk_clk_register_ref2usb_tx(const char *name,
>>>>
>>>>       return &tx->hw;
>>>> }
>>>> +EXPORT_SYMBOL_GPL(mtk_clk_register_ref2usb_tx);
>>>>
>>>> MODULE_LICENSE("GPL");
>>>> diff --git a/drivers/clk/mediatek/clk-cpumux.c b/drivers/clk/mediatek/clk-cpumux.c
>>>> index 2b5d48591738..25618eff6f2a 100644
>>>> --- a/drivers/clk/mediatek/clk-cpumux.c
>>>> +++ b/drivers/clk/mediatek/clk-cpumux.c
>>>> @@ -150,6 +150,7 @@ int mtk_clk_register_cpumuxes(struct device_node *node,
>>>>
>>>>       return PTR_ERR(hw);
>>>> }
>>>> +EXPORT_SYMBOL_GPL(mtk_clk_register_cpumuxes);
>>>>
>>>> void mtk_clk_unregister_cpumuxes(const struct mtk_composite *clks, int num,
>>>>                                struct clk_hw_onecell_data *clk_data)
>>>> @@ -166,5 +167,6 @@ void mtk_clk_unregister_cpumuxes(const struct mtk_composite *clks, int num,
>>>>               clk_data->hws[mux->id] = ERR_PTR(-ENOENT);
>>>>       }
>>>> }
>>>> +EXPORT_SYMBOL_GPL(mtk_clk_unregister_cpumuxes);
>>>>
>>>> MODULE_LICENSE("GPL");
>>>> diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
>>>> index 05a188c62119..41e60a7e8ff9 100644
>>>> --- a/drivers/clk/mediatek/clk-mtk.c
>>>> +++ b/drivers/clk/mediatek/clk-mtk.c
>>>> @@ -459,6 +459,7 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
>>>>       mtk_free_clk_data(clk_data);
>>>>       return r;
>>>> }
>>>> +EXPORT_SYMBOL_GPL(mtk_clk_simple_probe);
>>>>
>>>> int mtk_clk_simple_remove(struct platform_device *pdev)
>>>> {
>>>> @@ -472,5 +473,6 @@ int mtk_clk_simple_remove(struct platform_device *pdev)
>>>>
>>>>       return 0;
>>>> }
>>>> +EXPORT_SYMBOL_GPL(mtk_clk_simple_remove);
>>>
>>> Thanks, I need this too. I am preparing a patch to use mtk_clk_simple_remove/mtk_clk_simple_probe
>>> for MT6779 clks first and maybe I can apply this to all MediaTek clk drivers.
>>>
>>> Reviewed-by: Miles Chen <miles.chen@mediatek.com>
>>
>> Hello Miles,
>>
>> thanks for telling me, because my next step would have been exactly what
>> you're doing, for all MediaTek clk drivers... otherwise we'd be doing
>> redundant work going afterwards.
> 
> Should we consider using symbol namespaces (EXPORT_SYMBOL_NS)?
> 

I don't think we should... I don't know if any module in the common clock
framework is doing that, but if we want some symbol namespace separation,
we would want that "at least" on the entire MediaTek framework, right? :-)

In that case, we can simply keep using EXPORT_SYMBOL_GPL() and change the
Makefile in this directory to add:

	ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE=COMMON_CLK_MEDIATEK

...but that's surely out of scope for this specific patch series.

What do you think?

Cheers,
Angelo

> ChenYu
> 
>> Regards,
>> Angelo
>>
>>>
>>> thanks,
>>> Miles
>>>>
>>>> MODULE_LICENSE("GPL");
>>>> diff --git a/drivers/clk/mediatek/reset.c b/drivers/clk/mediatek/reset.c
>>>> index 179505549a7c..290ceda84ce4 100644
>>>> --- a/drivers/clk/mediatek/reset.c
>>>> +++ b/drivers/clk/mediatek/reset.c
>>>> @@ -228,5 +228,6 @@ int mtk_register_reset_controller_with_dev(struct device *dev,
>>>>
>>>>       return 0;
>>>> }
>>>> +EXPORT_SYMBOL_GPL(mtk_register_reset_controller_with_dev);
>>>>
>>>> MODULE_LICENSE("GPL");
>>>> --
>>>> 2.35.1
>>>>
>>>>
>>
Chen-Yu Tsai May 19, 2022, 8:45 a.m. UTC | #5
On Thu, May 19, 2022 at 4:26 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
> Il 19/05/22 10:15, Chen-Yu Tsai ha scritto:
> > On Thu, May 19, 2022 at 4:05 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> wrote:
> >>
> >> Il 19/05/22 06:41, Miles Chen ha scritto:
> >>>
> >>> Hi Angelo,
> >>>
> >>>> In order to compile the clock drivers for various MediaTek SoCs as
> >>>> modules, it is necessary to export a few functions from the MediaTek
> >>>> specific clocks (and reset) libraries.
> >>>>
> >>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >>>> ---
> >>>> drivers/clk/mediatek/clk-apmixed.c | 1 +
> >>>> drivers/clk/mediatek/clk-cpumux.c  | 2 ++
> >>>> drivers/clk/mediatek/clk-mtk.c     | 2 ++
> >>>> drivers/clk/mediatek/reset.c       | 1 +
> >>>> 4 files changed, 6 insertions(+)
> >>>>
> >>>> diff --git a/drivers/clk/mediatek/clk-apmixed.c b/drivers/clk/mediatek/clk-apmixed.c
> >>>> index 6b0ab0a346e8..f126da693a7f 100644
> >>>> --- a/drivers/clk/mediatek/clk-apmixed.c
> >>>> +++ b/drivers/clk/mediatek/clk-apmixed.c
> >>>> @@ -98,5 +98,6 @@ struct clk_hw *mtk_clk_register_ref2usb_tx(const char *name,
> >>>>
> >>>>       return &tx->hw;
> >>>> }
> >>>> +EXPORT_SYMBOL_GPL(mtk_clk_register_ref2usb_tx);
> >>>>
> >>>> MODULE_LICENSE("GPL");
> >>>> diff --git a/drivers/clk/mediatek/clk-cpumux.c b/drivers/clk/mediatek/clk-cpumux.c
> >>>> index 2b5d48591738..25618eff6f2a 100644
> >>>> --- a/drivers/clk/mediatek/clk-cpumux.c
> >>>> +++ b/drivers/clk/mediatek/clk-cpumux.c
> >>>> @@ -150,6 +150,7 @@ int mtk_clk_register_cpumuxes(struct device_node *node,
> >>>>
> >>>>       return PTR_ERR(hw);
> >>>> }
> >>>> +EXPORT_SYMBOL_GPL(mtk_clk_register_cpumuxes);
> >>>>
> >>>> void mtk_clk_unregister_cpumuxes(const struct mtk_composite *clks, int num,
> >>>>                                struct clk_hw_onecell_data *clk_data)
> >>>> @@ -166,5 +167,6 @@ void mtk_clk_unregister_cpumuxes(const struct mtk_composite *clks, int num,
> >>>>               clk_data->hws[mux->id] = ERR_PTR(-ENOENT);
> >>>>       }
> >>>> }
> >>>> +EXPORT_SYMBOL_GPL(mtk_clk_unregister_cpumuxes);
> >>>>
> >>>> MODULE_LICENSE("GPL");
> >>>> diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
> >>>> index 05a188c62119..41e60a7e8ff9 100644
> >>>> --- a/drivers/clk/mediatek/clk-mtk.c
> >>>> +++ b/drivers/clk/mediatek/clk-mtk.c
> >>>> @@ -459,6 +459,7 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
> >>>>       mtk_free_clk_data(clk_data);
> >>>>       return r;
> >>>> }
> >>>> +EXPORT_SYMBOL_GPL(mtk_clk_simple_probe);
> >>>>
> >>>> int mtk_clk_simple_remove(struct platform_device *pdev)
> >>>> {
> >>>> @@ -472,5 +473,6 @@ int mtk_clk_simple_remove(struct platform_device *pdev)
> >>>>
> >>>>       return 0;
> >>>> }
> >>>> +EXPORT_SYMBOL_GPL(mtk_clk_simple_remove);
> >>>
> >>> Thanks, I need this too. I am preparing a patch to use mtk_clk_simple_remove/mtk_clk_simple_probe
> >>> for MT6779 clks first and maybe I can apply this to all MediaTek clk drivers.
> >>>
> >>> Reviewed-by: Miles Chen <miles.chen@mediatek.com>
> >>
> >> Hello Miles,
> >>
> >> thanks for telling me, because my next step would have been exactly what
> >> you're doing, for all MediaTek clk drivers... otherwise we'd be doing
> >> redundant work going afterwards.
> >
> > Should we consider using symbol namespaces (EXPORT_SYMBOL_NS)?
> >
>
> I don't think we should... I don't know if any module in the common clock
> framework is doing that, but if we want some symbol namespace separation,
> we would want that "at least" on the entire MediaTek framework, right? :-)

The sunxi-ng clk driver recently started doing this. See:

    http://git.kernel.org/torvalds/c/551b62b1e4cb64d3b42da0fbfdcd26a5fcd684be

And it's being done for all kinds of common driver libraries.

I agree that it would be done for the entire MediaTek framework.

> In that case, we can simply keep using EXPORT_SYMBOL_GPL() and change the
> Makefile in this directory to add:
>
>         ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE=COMMON_CLK_MEDIATEK

Oh, I didn't know of this trick. Nice. :D

I think we still need to add MODULE_IMPORT_NS() statements, right?

> ...but that's surely out of scope for this specific patch series.
>
> What do you think?

It's definitely out of scope, but nice to have, to reduce the size of the
default symbol table, and limit the usage of the symbols the driver exports.

Regards
ChenYu
AngeloGioacchino Del Regno May 19, 2022, 8:53 a.m. UTC | #6
Il 19/05/22 10:45, Chen-Yu Tsai ha scritto:
> On Thu, May 19, 2022 at 4:26 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>> Il 19/05/22 10:15, Chen-Yu Tsai ha scritto:
>>> On Thu, May 19, 2022 at 4:05 PM AngeloGioacchino Del Regno
>>> <angelogioacchino.delregno@collabora.com> wrote:
>>>>
>>>> Il 19/05/22 06:41, Miles Chen ha scritto:
>>>>>
>>>>> Hi Angelo,
>>>>>
>>>>>> In order to compile the clock drivers for various MediaTek SoCs as
>>>>>> modules, it is necessary to export a few functions from the MediaTek
>>>>>> specific clocks (and reset) libraries.
>>>>>>
>>>>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>>>>> ---
>>>>>> drivers/clk/mediatek/clk-apmixed.c | 1 +
>>>>>> drivers/clk/mediatek/clk-cpumux.c  | 2 ++
>>>>>> drivers/clk/mediatek/clk-mtk.c     | 2 ++
>>>>>> drivers/clk/mediatek/reset.c       | 1 +
>>>>>> 4 files changed, 6 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/clk/mediatek/clk-apmixed.c b/drivers/clk/mediatek/clk-apmixed.c
>>>>>> index 6b0ab0a346e8..f126da693a7f 100644
>>>>>> --- a/drivers/clk/mediatek/clk-apmixed.c
>>>>>> +++ b/drivers/clk/mediatek/clk-apmixed.c
>>>>>> @@ -98,5 +98,6 @@ struct clk_hw *mtk_clk_register_ref2usb_tx(const char *name,
>>>>>>
>>>>>>        return &tx->hw;
>>>>>> }
>>>>>> +EXPORT_SYMBOL_GPL(mtk_clk_register_ref2usb_tx);
>>>>>>
>>>>>> MODULE_LICENSE("GPL");
>>>>>> diff --git a/drivers/clk/mediatek/clk-cpumux.c b/drivers/clk/mediatek/clk-cpumux.c
>>>>>> index 2b5d48591738..25618eff6f2a 100644
>>>>>> --- a/drivers/clk/mediatek/clk-cpumux.c
>>>>>> +++ b/drivers/clk/mediatek/clk-cpumux.c
>>>>>> @@ -150,6 +150,7 @@ int mtk_clk_register_cpumuxes(struct device_node *node,
>>>>>>
>>>>>>        return PTR_ERR(hw);
>>>>>> }
>>>>>> +EXPORT_SYMBOL_GPL(mtk_clk_register_cpumuxes);
>>>>>>
>>>>>> void mtk_clk_unregister_cpumuxes(const struct mtk_composite *clks, int num,
>>>>>>                                 struct clk_hw_onecell_data *clk_data)
>>>>>> @@ -166,5 +167,6 @@ void mtk_clk_unregister_cpumuxes(const struct mtk_composite *clks, int num,
>>>>>>                clk_data->hws[mux->id] = ERR_PTR(-ENOENT);
>>>>>>        }
>>>>>> }
>>>>>> +EXPORT_SYMBOL_GPL(mtk_clk_unregister_cpumuxes);
>>>>>>
>>>>>> MODULE_LICENSE("GPL");
>>>>>> diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
>>>>>> index 05a188c62119..41e60a7e8ff9 100644
>>>>>> --- a/drivers/clk/mediatek/clk-mtk.c
>>>>>> +++ b/drivers/clk/mediatek/clk-mtk.c
>>>>>> @@ -459,6 +459,7 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
>>>>>>        mtk_free_clk_data(clk_data);
>>>>>>        return r;
>>>>>> }
>>>>>> +EXPORT_SYMBOL_GPL(mtk_clk_simple_probe);
>>>>>>
>>>>>> int mtk_clk_simple_remove(struct platform_device *pdev)
>>>>>> {
>>>>>> @@ -472,5 +473,6 @@ int mtk_clk_simple_remove(struct platform_device *pdev)
>>>>>>
>>>>>>        return 0;
>>>>>> }
>>>>>> +EXPORT_SYMBOL_GPL(mtk_clk_simple_remove);
>>>>>
>>>>> Thanks, I need this too. I am preparing a patch to use mtk_clk_simple_remove/mtk_clk_simple_probe
>>>>> for MT6779 clks first and maybe I can apply this to all MediaTek clk drivers.
>>>>>
>>>>> Reviewed-by: Miles Chen <miles.chen@mediatek.com>
>>>>
>>>> Hello Miles,
>>>>
>>>> thanks for telling me, because my next step would have been exactly what
>>>> you're doing, for all MediaTek clk drivers... otherwise we'd be doing
>>>> redundant work going afterwards.
>>>
>>> Should we consider using symbol namespaces (EXPORT_SYMBOL_NS)?
>>>
>>
>> I don't think we should... I don't know if any module in the common clock
>> framework is doing that, but if we want some symbol namespace separation,
>> we would want that "at least" on the entire MediaTek framework, right? :-)
> 
> The sunxi-ng clk driver recently started doing this. See:
> 
>      http://git.kernel.org/torvalds/c/551b62b1e4cb64d3b42da0fbfdcd26a5fcd684be

That's good. And...that's one of the examples for which using "a trick" is
shorter and enhances maintainability!

> 
> And it's being done for all kinds of common driver libraries.
> 
> I agree that it would be done for the entire MediaTek framework.
> 
>> In that case, we can simply keep using EXPORT_SYMBOL_GPL() and change the
>> Makefile in this directory to add:
>>
>>          ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE=COMMON_CLK_MEDIATEK
> 
> Oh, I didn't know of this trick. Nice. :D
> 
> I think we still need to add MODULE_IMPORT_NS() statements, right?
> 

I haven't experimented with the IMPORT, but I believe if that's relative to
files in the same Makefile, we won't need to add that... let's make some
experiments :-)

>> ...but that's surely out of scope for this specific patch series.
>>
>> What do you think?
> 
> It's definitely out of scope, but nice to have, to reduce the size of the
> default symbol table, and limit the usage of the symbols the driver exports.
> 

Agreed.

> Regards
> ChenYu
diff mbox series

Patch

diff --git a/drivers/clk/mediatek/clk-apmixed.c b/drivers/clk/mediatek/clk-apmixed.c
index 6b0ab0a346e8..f126da693a7f 100644
--- a/drivers/clk/mediatek/clk-apmixed.c
+++ b/drivers/clk/mediatek/clk-apmixed.c
@@ -98,5 +98,6 @@  struct clk_hw *mtk_clk_register_ref2usb_tx(const char *name,
 
 	return &tx->hw;
 }
+EXPORT_SYMBOL_GPL(mtk_clk_register_ref2usb_tx);
 
 MODULE_LICENSE("GPL");
diff --git a/drivers/clk/mediatek/clk-cpumux.c b/drivers/clk/mediatek/clk-cpumux.c
index 2b5d48591738..25618eff6f2a 100644
--- a/drivers/clk/mediatek/clk-cpumux.c
+++ b/drivers/clk/mediatek/clk-cpumux.c
@@ -150,6 +150,7 @@  int mtk_clk_register_cpumuxes(struct device_node *node,
 
 	return PTR_ERR(hw);
 }
+EXPORT_SYMBOL_GPL(mtk_clk_register_cpumuxes);
 
 void mtk_clk_unregister_cpumuxes(const struct mtk_composite *clks, int num,
 				 struct clk_hw_onecell_data *clk_data)
@@ -166,5 +167,6 @@  void mtk_clk_unregister_cpumuxes(const struct mtk_composite *clks, int num,
 		clk_data->hws[mux->id] = ERR_PTR(-ENOENT);
 	}
 }
+EXPORT_SYMBOL_GPL(mtk_clk_unregister_cpumuxes);
 
 MODULE_LICENSE("GPL");
diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 05a188c62119..41e60a7e8ff9 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -459,6 +459,7 @@  int mtk_clk_simple_probe(struct platform_device *pdev)
 	mtk_free_clk_data(clk_data);
 	return r;
 }
+EXPORT_SYMBOL_GPL(mtk_clk_simple_probe);
 
 int mtk_clk_simple_remove(struct platform_device *pdev)
 {
@@ -472,5 +473,6 @@  int mtk_clk_simple_remove(struct platform_device *pdev)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(mtk_clk_simple_remove);
 
 MODULE_LICENSE("GPL");
diff --git a/drivers/clk/mediatek/reset.c b/drivers/clk/mediatek/reset.c
index 179505549a7c..290ceda84ce4 100644
--- a/drivers/clk/mediatek/reset.c
+++ b/drivers/clk/mediatek/reset.c
@@ -228,5 +228,6 @@  int mtk_register_reset_controller_with_dev(struct device *dev,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(mtk_register_reset_controller_with_dev);
 
 MODULE_LICENSE("GPL");