Nouveau - Apr 2021 - [PATCH v3 03/20] drm/dp: Move i2c init to drm_dp_aux_init, add __must_check and fini

On Mon, Apr 19, 2021 at 06:55:05PM -0400, Lyude Paul
wrote:
> When moving around drm_dp_aux_register() calls, it turned out we
> accidentally managed to cause issues with the Tegra driver due to the fact
> the Tegra driver would attempt to retrieve a reference to the AUX
channel's
> i2c adapter - which wouldn't be initialized until drm_dp_aux_register()
is
> called.
> 
> This doesn't actually make a whole ton of sense, as it's not
unexpected for
> a driver to need to be able to use an AUX adapter before it's been
> registered. Likewise-it's not unexpected for a driver to try using the
i2c
> adapter for said AUX channel before it's been registered as well. In
fact,
> the current documentation for drm_dp_aux_init() even seems to imply that
> drm_dp_aux_init() is supposed to be handling i2c adapter creation for this
> precise reason - not drm_dp_aux_register().
> 
> Since the i2c adapter doesn't need to be linked to the DRM device in
any
> way, we can just fix this problem by moving i2c adapter creation out of
> drm_dp_aux_register() and into drm_dp_aux_init(). Additionally, since this
> means that drm_dp_aux_init() can fail we go ahead and add a __must_check
> attribute to it so that drivers don't ignore its return status on
failures.
> And finally, we add a drm_dp_aux_fini() and hook it up in all DRM drivers
> across the kernel to take care of cleaning up the i2c adapter once it's
no
> longer needed.
> 
> This should also fix the regressions noted in the Tegra driver.
The init vs. register split is intentional. Registering the thing
and allowing userspace access to it before the rest of the driver
is ready isn't particularly great. For a while now we've tried to
move towards an architecture where the driver is fully initialzied
before anything gets exposed to userspace.

-- 
Ville Syrj?l?
Intel

On Tue, 2021-04-20 at 02:16 +0300, Ville Syrj?l? wrote:
> 
> The init vs. register split is intentional. Registering the thing
> and allowing userspace access to it before the rest of the driver
> is ready isn't particularly great. For a while now we've tried to
> move towards an architecture where the driver is fully initialzied
> before anything gets exposed to userspace.
Yeah-thank you for pointing this out. Thierry - do you think there's an
alternate solution we could go with in Tegra to fix the get_device() issue
that wouldn't require us trying to expose the i2c adapter early?
> 
-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat