This patch fixes multiple definition error like below when building it as moudle with device tree support.
drivers/gpu/drm/exynos/exynos_drm_g2d.o: In function `.LANCHOR1': exynos_drm_g2d.c:(.rodata+0x6c): multiple definition of `__mod_of_device_table' drivers/gpu/drm/exynos/exynos_drm_fimd.o:exynos_drm_fimd.c:(.rodata+0x144): first defined here
Signed-off-by: Inki Dae inki.dae@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_g2d.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 746b282..1e02d13 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -117,7 +117,7 @@ static const struct of_device_id fimd_driver_dt_match[] = { .data = &exynos5_fimd_driver_data }, {}, }; -MODULE_DEVICE_TABLE(of, fimd_driver_dt_match); +MODULE_DEVICE_TABLE(of_fimd, fimd_driver_dt_match); #endif
static inline struct fimd_driver_data *drm_fimd_get_driver_data( diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c index 47a493c..6a01ff1 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c @@ -1525,7 +1525,7 @@ static const struct of_device_id exynos_g2d_match[] = { { .compatible = "samsung,exynos5250-g2d" }, {}, }; -MODULE_DEVICE_TABLE(of, exynos_g2d_match); +MODULE_DEVICE_TABLE(of_g2d, exynos_g2d_match); #endif
struct platform_driver g2d_driver = {
Hi Inki,
On Friday 26 of April 2013 14:03:10 Inki Dae wrote:
This patch fixes multiple definition error like below when building it as moudle with device tree support.
drivers/gpu/drm/exynos/exynos_drm_g2d.o: In function `.LANCHOR1': exynos_drm_g2d.c:(.rodata+0x6c): multiple definition of `__mod_of_device_table' drivers/gpu/drm/exynos/exynos_drm_fimd.o:exynos_drm_fimd.c:(.rodata+0x1 44): first defined here
Signed-off-by: Inki Dae inki.dae@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_g2d.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 746b282..1e02d13 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -117,7 +117,7 @@ static const struct of_device_id fimd_driver_dt_match[] = { .data = &exynos5_fimd_driver_data }, {}, }; -MODULE_DEVICE_TABLE(of, fimd_driver_dt_match); +MODULE_DEVICE_TABLE(of_fimd, fimd_driver_dt_match);
I wonder if this change wouldn't break the purpose of having MODULE_DEVICE_TABLE at all.
As far as I remember, this is used to create a symbol with well known name that userspace tools can use to identify what devices are handled in this module. For example
MODULE_DEVICE_TABLE(of, fimd_driver_dt_match);
results in creation of __mod_of_device_table symbol, of which tools, such as depmod are aware and can build a list of supported devices.
Your change will result in creation of __mod_of_fimd_device_table, which is unknown and won't be of any use.
By the way, looking at the definition of MODULE_DEVICE_TABLE, which is
139 #define MODULE_DEVICE_TABLE(type,name) \ 140 MODULE_GENERIC_TABLE(type##_device,name)
and then MODULE_GENERIC_TABLE
85 #ifdef MODULE 86 #define MODULE_GENERIC_TABLE(gtype,name) \ 87 extern const struct gtype##_id __mod_##gtype##_table \ 88 __attribute__ ((unused, alias(__stringify(name)))) 89 90 #else /* !MODULE */ 91 #define MODULE_GENERIC_TABLE(gtype,name) 92 #endif
it seems like the exact line that will be generated after your change, will be
extern const struct of_fimd_device_id __mod_of_fimd_device_table __attribute__ ((unused, alias(__stringify(name))));
which seems wrong, because of_fimd_device_id is not a correct struct type.
Best regards, Tomasz
Hi Tomasz,
2013/4/26 Tomasz Figa tomasz.figa@gmail.com
Hi Inki,
On Friday 26 of April 2013 14:03:10 Inki Dae wrote:
This patch fixes multiple definition error like below when building it as moudle with device tree support.
drivers/gpu/drm/exynos/exynos_drm_g2d.o: In function `.LANCHOR1': exynos_drm_g2d.c:(.rodata+0x6c): multiple definition of `__mod_of_device_table' drivers/gpu/drm/exynos/exynos_drm_fimd.o:exynos_drm_fimd.c:(.rodata+0x1 44): first defined here
Signed-off-by: Inki Dae inki.dae@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_g2d.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 746b282..1e02d13 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -117,7 +117,7 @@ static const struct of_device_id fimd_driver_dt_match[] = { .data = &exynos5_fimd_driver_data }, {}, }; -MODULE_DEVICE_TABLE(of, fimd_driver_dt_match); +MODULE_DEVICE_TABLE(of_fimd, fimd_driver_dt_match);
I wonder if this change wouldn't break the purpose of having MODULE_DEVICE_TABLE at all.
As far as I remember, this is used to create a symbol with well known name that userspace tools can use to identify what devices are handled in this module. For example
MODULE_DEVICE_TABLE(of, fimd_driver_dt_match);
results in creation of __mod_of_device_table symbol, of which tools, such as depmod are aware and can build a list of supported devices.
Your change will result in creation of __mod_of_fimd_device_table, which is unknown and won't be of any use.
By the way, looking at the definition of MODULE_DEVICE_TABLE, which is
139 #define MODULE_DEVICE_TABLE(type,name) \ 140 MODULE_GENERIC_TABLE(type##_device,name)
and then MODULE_GENERIC_TABLE
85 #ifdef MODULE 86 #define MODULE_GENERIC_TABLE(gtype,name) \ 87 extern const struct gtype##_id __mod_##gtype##_table \ 88 __attribute__ ((unused, alias(__stringify(name)))) 89 90 #else /* !MODULE */ 91 #define MODULE_GENERIC_TABLE(gtype,name) 92 #endif
it seems like the exact line that will be generated after your change, will be
extern const struct of_fimd_device_id __mod_of_fimd_device_table __attribute__ ((unused, alias(__stringify(name))));
Exactly right. it's my mistake. But now it seems that __mode_of_device_table is multi defined at fimd and g2d side so there still is module build error. :(
Thanks, Inki Dae
which seems wrong, because of_fimd_device_id is not a correct struct type.
Best regards, Tomasz
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On 04/26/2013 10:20 AM, Inki Dae wrote:
Exactly right. it's my mistake. But now it seems that __mode_of_device_table is multi defined at fimd and g2d side so there still is module build error. :(
Since all drivers seem to be linked into single a single module, you likely need to create a separate table of struct of_device_id just for the purpose of MODULE_DEVICE_TABLE(of, ...). This table would contain 'compatible' strings for all devices. Or choose of_device_id for just one device and define MODULE_DEVICE_TABLE() for it in some common place, e.g. exynos_drm_drv.c. I believe all devices should be listed though.
Thanks, Sylwester
On Friday 26 of April 2013 11:48:50 Sylwester Nawrocki wrote:
On 04/26/2013 10:20 AM, Inki Dae wrote:
Exactly right. it's my mistake. But now it seems that __mode_of_device_table is multi defined at fimd and g2d side so there still is module build error. :(
Since all drivers seem to be linked into single a single module, you likely need to create a separate table of struct of_device_id just for the purpose of MODULE_DEVICE_TABLE(of, ...). This table would contain 'compatible' strings for all devices. Or choose of_device_id for just one device and define MODULE_DEVICE_TABLE() for it in some common place, e.g. exynos_drm_drv.c. I believe all devices should be listed though.
IMHO, the most proper solution would be to split the module into parent exynos_drm module and per-device submodules, which would depend on the parent module.
This way you would be able to load dynamically any submodule you want, without recompiling the modules.
Best regards, Tomasz
On 04/26/2013 09:42 PM, Tomasz Figa wrote:
On Friday 26 of April 2013 11:48:50 Sylwester Nawrocki wrote:
On 04/26/2013 10:20 AM, Inki Dae wrote:
Exactly right. it's my mistake. But now it seems that __mode_of_device_table is multi defined at fimd and g2d side so there still is module build error. :(
Since all drivers seem to be linked into single a single module, you likely need to create a separate table of struct of_device_id just for the purpose of MODULE_DEVICE_TABLE(of, ...). This table would contain 'compatible' strings for all devices. Or choose of_device_id for just one device and define MODULE_DEVICE_TABLE() for it in some common place, e.g. exynos_drm_drv.c. I believe all devices should be listed though.
IMHO, the most proper solution would be to split the module into parent exynos_drm module and per-device submodules, which would depend on the parent module.
This way you would be able to load dynamically any submodule you want, without recompiling the modules.
Yes, this is how it would have been in a perfect world. Probably something worth to consider for the future, it likely implies a lot of work.
OTOH if there is one device for which the driver will always be present in the main module it should be enough to make a single entry MODULE_DEVICE_TABLE including its compatible string to ensure the driver is properly loaded, shouldn't it ?
2013/4/27 Sylwester Nawrocki sylvester.nawrocki@gmail.com
On 04/26/2013 09:42 PM, Tomasz Figa wrote:
On Friday 26 of April 2013 11:48:50 Sylwester Nawrocki wrote:
On 04/26/2013 10:20 AM, Inki Dae wrote:
Exactly right. it's my mistake. But now it seems that __mode_of_device_table is multi defined at fimd and g2d side so there still is module build error. :(
Since all drivers seem to be linked into single a single module, you likely need to create a separate table of struct of_device_id just for the purpose of MODULE_DEVICE_TABLE(of, ...). This table would contain 'compatible' strings for all devices. Or choose of_device_id for just one device and define MODULE_DEVICE_TABLE() for it in some common place, e.g. exynos_drm_drv.c. I believe all devices should be listed though.
IMHO, the most proper solution would be to split the module into parent exynos_drm module and per-device submodules, which would depend on the parent module.
This way you would be able to load dynamically any submodule you want, without recompiling the modules.
Yes, this is how it would have been in a perfect world. Probably something worth to consider for the future, it likely implies a lot of work.
For now, exynos drm framework has a problem to module support with device tree. And for this, going back to previous style might be a right way but this also still has the probing issue. Frankly spreaking, I don't really want back to previous style because that way not only still has a probing issue but also makes exynos drm framework so complicated(there are many things to consider every time framework updating). So I'd like to resolve this issue as is first.
OTOH if there is one device for which the driver will always be present
in the main module it should be enough to make a single entry MODULE_DEVICE_TABLE including its compatible string to ensure the driver is properly loaded, shouldn't it ?
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/**majordomo-info.htmlhttp://vger.kernel.org/majordomo-info.html
2013/4/27 Tomasz Figa tomasz.figa@gmail.com
On Friday 26 of April 2013 11:48:50 Sylwester Nawrocki wrote:
On 04/26/2013 10:20 AM, Inki Dae wrote:
Exactly right. it's my mistake. But now it seems that __mode_of_device_table is multi defined at fimd and g2d side so there still is module build error. :(
Since all drivers seem to be linked into single a single module, you likely need to create a separate table of struct of_device_id just for the purpose of MODULE_DEVICE_TABLE(of, ...). This table would contain 'compatible' strings for all devices. Or choose of_device_id for just one device and define MODULE_DEVICE_TABLE() for it in some common place, e.g. exynos_drm_drv.c. I believe all devices should be listed though.
IMHO, the most proper solution would be to split the module into parent exynos_drm module and per-device submodules, which would depend on the parent module.
This way had already been used a long time ago and there was some probing issues so we made exynos drm framework to be one module.
This way you would be able to load dynamically any submodule you want,
without recompiling the modules.
Best regards, Tomasz
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Inki Dae -
Sylwester Nawrocki -
Sylwester Nawrocki -
Tomasz Figa