linux-kernel - Re: [PATCH] drm: tda998x: mali-dp: hdlcd: refactor connector registration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 24 Oct 2016 22:24:42 +0200
From:   Daniel Vetter <daniel@...ll.ch>
To:     Brian Starkey <brian.starkey@....com>
Cc:     Russell King <rmk+kernel@...linux.org.uk>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm: tda998x: mali-dp: hdlcd: refactor connector registration

On Mon, Oct 24, 2016 at 4:52 PM, Brian Starkey <brian.starkey@....com> wrote:
>>
>>> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
>>> b/drivers/gpu/drm/i2c/tda998x_drv.c
>>> index f4315bc..6e6fca2 100644
>>> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
>>> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
>>> @@ -1369,7 +1369,6 @@ const struct drm_connector_helper_funcs
>>> tda998x_connector_helper_funcs = {
>>>
>>>  static void tda998x_connector_destroy(struct drm_connector *connector)
>>>  {
>>> -       drm_connector_unregister(connector);
>>>         drm_connector_cleanup(connector);
>>>  }
>>>
>>> @@ -1441,16 +1440,10 @@ static int tda998x_bind(struct device *dev,
>>> struct device *master, void *data)
>>>         if (ret)
>>>                 goto err_connector;
>>>
>>> -       ret = drm_connector_register(&priv->connector);
>>> -       if (ret)
>>> -               goto err_sysfs;
>>> -
>>
>>
>> Instead of smashing all these patches into one, what about checking here
>> for midlayer driver set with:
>>
>>         /* register here for drivers still using midlayer load/unload */
>>         if (dev->driver->load)
>>                 drm_connector_register(connector),
>>
>> Similar in other places. That way we wouldn't need to switch the world in
>> one patch.
>
>
> I don't think that helps. If we do that in isolation (first), then
> mali-dp and hdlcd won't get their connectors registered because their
> bind order is:
>
>         drm_dev_register();
>         component_bind_all();
>
> If we change the mali-dp/hdlcd bind order first, then tda998x will
> explode on drm_connector_register() until it's patched to remove that.
>
> As I mentioned in my mail to Russell, the only way I can see to avoid
> patching all three drivers in one go is:
>  1) Add (probably open-coded) drm_connector_register_all() to the end
>     of bind in hdlcd and mali-dp
>  2) Patch tda998x to remove drm_connector_register()
>  3) Reorder hdlcd/mali-dp bind and remove the connector registration
>     added in 1)
>
> We can do that, but it's extra churn for the same result, and none of
> the 5 patches will really make sense in isolation anyway.

I thought there's also armada to take care of, which this patch would
break? Maybe even another driver, so the hack would still be useful
for those other drivers. And it would have been useful if malidp/hdlcd
wouldn't have started out with the wrong init ordering ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch