Message ID | 1403126355-16236-1-git-send-email-swarren@wwwdotorg.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Wed, Jun 18, 2014 at 03:19:15PM -0600, Stephen Warren wrote: > From: Stephen Warren <swarren@nvidia.com> > > When tegra-drm.ko is built as a module, these MODULE_DEVICE_TABLEs allow > the module to be auto-loaded since the module will match the devices > instantiated from device tree. I vaguely remember doing something like this a while back and getting a bunch of link-time errors. But I assume that you've tested this, so I must be remembering wrongly. One comment below: > diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c > index 3396f9f6a9f7..a1f9b06a75d5 100644 > --- a/drivers/gpu/drm/tegra/drm.c > +++ b/drivers/gpu/drm/tegra/drm.c > @@ -694,6 +694,7 @@ static const struct of_device_id host1x_drm_subdevs[] = { > { .compatible = "nvidia,tegra124-hdmi", }, > { /* sentinel */ } > }; > +MODULE_DEVICE_TABLE(of, host1x_drm_subdevs); I don't think this one is really required. It's a union of all other compatible values and only used to delay the DRM subsystem driver registration until all needed subdevices have been probed. Thierry
On 06/18/2014 03:51 PM, Thierry Reding wrote: > On Wed, Jun 18, 2014 at 03:19:15PM -0600, Stephen Warren wrote: >> From: Stephen Warren <swarren@nvidia.com> >> >> When tegra-drm.ko is built as a module, these MODULE_DEVICE_TABLEs allow >> the module to be auto-loaded since the module will match the devices >> instantiated from device tree. > > I vaguely remember doing something like this a while back and getting a > bunch of link-time errors. But I assume that you've tested this, so I > must be remembering wrongly. Were the problems due to: a) Simply building the tegradrm driver as modules. I vaguely recall some runtime issues with tegradrm as a module, but I'm not sure about build issues. I don't think this patch could make this any worse. b) Building as modules works, but adding MODULE_DEVICE_TABLE broke that. This seems unlikely since *many* module in the kernel have a MODULE_DEVICE_TABLE... Certainly, with this patch applied, building tegradrm as a module in next-20140611 works out just fine, and the code runs fine too. Building tegra_defconfig (which has tegradrm builtin) on Linus' master with this patch applied also works out fine. I'll post v2 with the issue you mentioned addressed.
On Wed, Jun 18, 2014 at 04:21:18PM -0600, Stephen Warren wrote: > On 06/18/2014 03:51 PM, Thierry Reding wrote: > > On Wed, Jun 18, 2014 at 03:19:15PM -0600, Stephen Warren wrote: > >> From: Stephen Warren <swarren@nvidia.com> > >> > >> When tegra-drm.ko is built as a module, these MODULE_DEVICE_TABLEs allow > >> the module to be auto-loaded since the module will match the devices > >> instantiated from device tree. > > > > I vaguely remember doing something like this a while back and getting a > > bunch of link-time errors. But I assume that you've tested this, so I > > must be remembering wrongly. > > Were the problems due to: > > a) Simply building the tegradrm driver as modules. > > I vaguely recall some runtime issues with tegradrm as a module, but I'm > not sure about build issues. I don't think this patch could make this > any worse. > > b) Building as modules works, but adding MODULE_DEVICE_TABLE broke that. I think it was this variant. Although adding MODULE_DEVICE_TABLE also broke building the driver as builtin module. I think the issue was that the linker was complaining about some symbol being defined multiple times. But admittedly this was a long time ago, so I'm not sure that my memory is entirely accurate. > This seems unlikely since *many* module in the kernel have a > MODULE_DEVICE_TABLE... > > Certainly, with this patch applied, building tegradrm as a module in > next-20140611 works out just fine, and the code runs fine too. Building > tegra_defconfig (which has tegradrm builtin) on Linus' master with this > patch applied also works out fine. Okay, sounds good then. I'll do some build testing to see if I can reproduce the errors, otherwise this looks good to me. Thierry
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index ef40381f3909..48c3bc460eef 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -1303,6 +1303,7 @@ static const struct of_device_id tegra_dc_of_match[] = { /* sentinel */ } }; +MODULE_DEVICE_TABLE(of, tegra_dc_of_match); static int tegra_dc_parse_dt(struct tegra_dc *dc) { diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c index 3f132e356e9c..708f783ead47 100644 --- a/drivers/gpu/drm/tegra/dpaux.c +++ b/drivers/gpu/drm/tegra/dpaux.c @@ -382,6 +382,7 @@ static const struct of_device_id tegra_dpaux_of_match[] = { { .compatible = "nvidia,tegra124-dpaux", }, { }, }; +MODULE_DEVICE_TABLE(of, tegra_dpaux_of_match); struct platform_driver tegra_dpaux_driver = { .driver = { diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index 3396f9f6a9f7..a1f9b06a75d5 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -694,6 +694,7 @@ static const struct of_device_id host1x_drm_subdevs[] = { { .compatible = "nvidia,tegra124-hdmi", }, { /* sentinel */ } }; +MODULE_DEVICE_TABLE(of, host1x_drm_subdevs); static struct host1x_driver host1x_drm_driver = { .name = "drm", diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c index bd56f2affa78..97c409f10456 100644 --- a/drivers/gpu/drm/tegra/dsi.c +++ b/drivers/gpu/drm/tegra/dsi.c @@ -982,6 +982,7 @@ static const struct of_device_id tegra_dsi_of_match[] = { { .compatible = "nvidia,tegra114-dsi", }, { }, }; +MODULE_DEVICE_TABLE(of, tegra_dsi_of_match); struct platform_driver tegra_dsi_driver = { .driver = { diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c index 7c53941f2a9e..02cd3e37a6ec 100644 --- a/drivers/gpu/drm/tegra/gr2d.c +++ b/drivers/gpu/drm/tegra/gr2d.c @@ -121,6 +121,7 @@ static const struct of_device_id gr2d_match[] = { { .compatible = "nvidia,tegra20-gr2d" }, { }, }; +MODULE_DEVICE_TABLE(of, gr2d_match); static const u32 gr2d_addr_regs[] = { GR2D_UA_BASE_ADDR, diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c index 30f5ba9bd6d0..2bea2b2d204e 100644 --- a/drivers/gpu/drm/tegra/gr3d.c +++ b/drivers/gpu/drm/tegra/gr3d.c @@ -130,6 +130,7 @@ static const struct of_device_id tegra_gr3d_match[] = { { .compatible = "nvidia,tegra20-gr3d" }, { } }; +MODULE_DEVICE_TABLE(of, tegra_gr3d_match); static const u32 gr3d_addr_regs[] = { GR3D_IDX_ATTRIBUTE( 0), diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c index ba067bb767e3..ffe26547328d 100644 --- a/drivers/gpu/drm/tegra/hdmi.c +++ b/drivers/gpu/drm/tegra/hdmi.c @@ -1450,6 +1450,7 @@ static const struct of_device_id tegra_hdmi_of_match[] = { { .compatible = "nvidia,tegra20-hdmi", .data = &tegra20_hdmi_config }, { }, }; +MODULE_DEVICE_TABLE(of, tegra_hdmi_of_match); static int tegra_hdmi_probe(struct platform_device *pdev) { diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c index 27c979b50111..061a5c501124 100644 --- a/drivers/gpu/drm/tegra/sor.c +++ b/drivers/gpu/drm/tegra/sor.c @@ -1455,6 +1455,7 @@ static const struct of_device_id tegra_sor_of_match[] = { { .compatible = "nvidia,tegra124-sor", }, { }, }; +MODULE_DEVICE_TABLE(of, tegra_sor_of_match); struct platform_driver tegra_sor_driver = { .driver = {