From: Arnd Bergmann <[email protected]>
Linking an object file into multiple modules is not supported
and causes a W=1 warning:
scripts/Makefile.build:236: drivers/platform/x86/intel/int3472/Makefile: common.o is added to multiple modules: intel_skl_int3472_discrete intel_skl_int3472_tps68470
Split out the common part here into a separate module to make it
more reliable.
Fixes: a2f9fbc247ee ("platform/x86: int3472: Split into 2 drivers")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/platform/x86/intel/int3472/Makefile | 9 ++++++---
drivers/platform/x86/intel/int3472/common.c | 7 +++++++
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/intel/int3472/Makefile b/drivers/platform/x86/intel/int3472/Makefile
index 9f16cb514397..a8aba07bf1dc 100644
--- a/drivers/platform/x86/intel/int3472/Makefile
+++ b/drivers/platform/x86/intel/int3472/Makefile
@@ -1,4 +1,7 @@
obj-$(CONFIG_INTEL_SKL_INT3472) += intel_skl_int3472_discrete.o \
- intel_skl_int3472_tps68470.o
-intel_skl_int3472_discrete-y := discrete.o clk_and_regulator.o led.o common.o
-intel_skl_int3472_tps68470-y := tps68470.o tps68470_board_data.o common.o
+ intel_skl_int3472_tps68470.o \
+ intel_skl_int3472_common.o
+intel_skl_int3472_discrete-y := discrete.o clk_and_regulator.o led.o
+intel_skl_int3472_tps68470-y := tps68470.o tps68470_board_data.o
+
+intel_skl_int3472_common-y += common.o
diff --git a/drivers/platform/x86/intel/int3472/common.c b/drivers/platform/x86/intel/int3472/common.c
index 9db2bb0bbba4..8e4a782b2c35 100644
--- a/drivers/platform/x86/intel/int3472/common.c
+++ b/drivers/platform/x86/intel/int3472/common.c
@@ -29,6 +29,7 @@ union acpi_object *skl_int3472_get_acpi_buffer(struct acpi_device *adev, char *i
return obj;
}
+EXPORT_SYMBOL_GPL(skl_int3472_get_acpi_buffer);
int skl_int3472_fill_cldb(struct acpi_device *adev, struct int3472_cldb *cldb)
{
@@ -52,6 +53,7 @@ int skl_int3472_fill_cldb(struct acpi_device *adev, struct int3472_cldb *cldb)
kfree(obj);
return ret;
}
+EXPORT_SYMBOL_GPL(skl_int3472_fill_cldb);
/* sensor_adev_ret may be NULL, name_ret must not be NULL */
int skl_int3472_get_sensor_adev_and_name(struct device *dev,
@@ -80,3 +82,8 @@ int skl_int3472_get_sensor_adev_and_name(struct device *dev,
return ret;
}
+EXPORT_SYMBOL_GPL(skl_int3472_get_sensor_adev_and_name);
+
+MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI Device Driver library");
+MODULE_AUTHOR("Daniel Scally <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
2.39.2
On Wed, May 29, 2024 at 12:50 PM Arnd Bergmann <[email protected]> wrote:
>
> From: Arnd Bergmann <[email protected]>
>
> Linking an object file into multiple modules is not supported
> and causes a W=1 warning:
>
> scripts/Makefile.build:236: drivers/platform/x86/intel/int3472/Makefile: common.o is added to multiple modules: intel_skl_int3472_discrete intel_skl_int3472_tps68470
>
> Split out the common part here into a separate module to make it
> more reliable.
..
> obj-$(CONFIG_INTEL_SKL_INT3472) += intel_skl_int3472_discrete.o \
> - intel_skl_int3472_tps68470.o
> + intel_skl_int3472_tps68470.o \
> + intel_skl_int3472_common.o
A nit: Can this be put above instead?
..
> +EXPORT_SYMBOL_GPL(skl_int3472_get_sensor_adev_and_name);
Are these namespaced?
--
With Best Regards,
Andy Shevchenko
On Wed, May 29, 2024, at 15:41, Andy Shevchenko wrote:
> On Wed, May 29, 2024 at 12:50 PM Arnd Bergmann <[email protected]> wrote:
>>
>> From: Arnd Bergmann <[email protected]>
>>
>> Linking an object file into multiple modules is not supported
>> and causes a W=1 warning:
>>
>> scripts/Makefile.build:236: drivers/platform/x86/intel/int3472/Makefile: common.o is added to multiple modules: intel_skl_int3472_discrete intel_skl_int3472_tps68470
>>
>> Split out the common part here into a separate module to make it
>> more reliable.
>
> ...
>
>> obj-$(CONFIG_INTEL_SKL_INT3472) += intel_skl_int3472_discrete.o \
>> - intel_skl_int3472_tps68470o
>
>> + intel_skl_int3472_tps68470o \
>> + intel_skl_int3472_common.o
>
> A nit: Can this be put above instead?
I've changed it like this now, is that what you meant?
obj-$(CONFIG_INTEL_SKL_INT3472) += intel_skl_int3472_common.o \
intel_skl_int3472_discrete.o \
intel_skl_int3472_tps68470.o \
intel_skl_int3472_common-y += common.o
intel_skl_int3472_discrete-y := discrete.o clk_and_regulator.o led.o
intel_skl_int3472_tps68470-y := tps68470.o tps68470_board_data.o
> ...
>
>> +EXPORT_SYMBOL_GPL(skl_int3472_get_sensor_adev_and_name);
>
> Are these namespaced?
No, is there any advantage to making them namespaced?
It's only a few symbols and they have proper prefixes.
Arnd
On Wed, May 29, 2024 at 5:14 PM Arnd Bergmann <[email protected]> wrote:
> On Wed, May 29, 2024, at 15:41, Andy Shevchenko wrote:
> > On Wed, May 29, 2024 at 12:50 PM Arnd Bergmann <[email protected]> wrote:
..
> >> obj-$(CONFIG_INTEL_SKL_INT3472) += intel_skl_int3472_discrete.o \
> >> - intel_skl_int3472_tps68470.o
> >
> >> + intel_skl_int3472_tps68470.o \
> >> + intel_skl_int3472_common.o
> >
> > A nit: Can this be put above instead?
>
> I've changed it like this now, is that what you meant?
>
>
> obj-$(CONFIG_INTEL_SKL_INT3472) += intel_skl_int3472_common.o \
> intel_skl_int3472_discrete.o \
> intel_skl_int3472_tps68470.o \
Without the last trailing \, but yes.
> intel_skl_int3472_common-y += common.o
> intel_skl_int3472_discrete-y := discrete.o clk_and_regulator.o led.o
> intel_skl_int3472_tps68470-y := tps68470.o tps68470_board_data.o
..
> >> +EXPORT_SYMBOL_GPL(skl_int3472_get_sensor_adev_and_name);
> >
> > Are these namespaced?
>
> No, is there any advantage to making them namespaced?
Yes, to clean up the global namespace.
> It's only a few symbols and they have proper prefixes.
It's different from the exported namespace.
The function prefixes are needed due to C language, as we can't have
two functions named the same. The export OTOH is what used for linking
modules and if there is no need to have it exported globally, if, for
example, compiling in this one.
So, can we move to the exported namespace at the same time?
--
With Best Regards,
Andy Shevchenko
On Wed, May 29, 2024 at 5:48 PM Arnd Bergmann <[email protected]> wrote:
>
> On Wed, May 29, 2024, at 16:28, Andy Shevchenko wrote:
> > On Wed, May 29, 2024 at 5:14 PM Arnd Bergmann <[email protected]> wrote:
> >> On Wed, May 29, 2024, at 15:41, Andy Shevchenko wrote:
> >> > On Wed, May 29, 2024 at 12:50 PM Arnd Bergmann <[email protected]>
> > It's different from the exported namespace.
> > The function prefixes are needed due to C language, as we can't have
> > two functions named the same. The export OTOH is what used for linking
> > modules and if there is no need to have it exported globally, if, for
> > example, compiling in this one.
> >
> > So, can we move to the exported namespace at the same time?
>
> Maybe you can come up with a patch then?
Yes, why not.
> I have no idea
> which namespace to use here, seeing that there are already
> six differnet namespaces in use in drivers/platform/x86/intel/
> but none of them seem to be a good fit for this one.
>
> Are you asking to just define another namespace here?
Yes.
> How would I define what the rules about using this namespace
> are, and where are they documented?
Currently we use a common sense, like a pattern:
SUBSYSTEM_DRIVER
or so.
In this case INTEL_INT3472 is good enough as it's unique enough to not
collide with anything else in Intel's world (okay, I hope that we
learnt our mistakes in the past and won't issue same ACPI ID for
different devices).
--
With Best Regards,
Andy Shevchenko
On Wed, May 29, 2024, at 16:28, Andy Shevchenko wrote:
> On Wed, May 29, 2024 at 5:14 PM Arnd Bergmann <[email protected]> wrote:
>> On Wed, May 29, 2024, at 15:41, Andy Shevchenko wrote:
>> > On Wed, May 29, 2024 at 12:50 PM Arnd Bergmann <arnd@kernelorg>
> It's different from the exported namespace.
> The function prefixes are needed due to C language, as we can't have
> two functions named the same. The export OTOH is what used for linking
> modules and if there is no need to have it exported globally, if, for
> example, compiling in this one.
>
> So, can we move to the exported namespace at the same time?
Maybe you can come up with a patch then? I have no idea
which namespace to use here, seeing that there are already
six differnet namespaces in use in drivers/platform/x86/intel/
but none of them seem to be a good fit for this one.
Are you asking to just define another namespace here?
How would I define what the rules about using this namespace
are, and where are they documented?
Arnd