linux-kernel - Re: [PATCH] drm/i2c: tda998x: don't register the connector

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:   Thu, 22 Sep 2016 14:38:45 +0100
From:   Brian Starkey <brian.starkey@....com>
To:     Daniel Vetter <daniel@...ll.ch>
Cc:     Sean Paul <seanpaul@...omium.org>,
        Russell King - ARM Linux <linux@...linux.org.uk>,
        Liviu Dudau <Liviu.Dudau@....com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH] drm/i2c: tda998x: don't register the connector

On Thu, Sep 22, 2016 at 01:28:37PM +0200, Daniel Vetter wrote:
>On Thu, Sep 22, 2016 at 1:22 PM, Sean Paul <seanpaul@...omium.org> wrote:
>> On Thu, Sep 22, 2016 at 3:51 AM, Russell King - ARM Linux
>> <linux@...linux.org.uk> wrote:
>>> On Thu, Sep 22, 2016 at 11:39:18AM +0100, Brian Starkey wrote:
>>>> Actually, could you please hold off picking this up? We need to make
>>>> changes in mali-dp and hdlcd or this will mess up their registration.
>>>> I will send those patches later today, but better if this all goes in
>>>> together (whenever that ends up being).
>>>
>>> Sorry, but I'm annoyed with this - the impression being given was that
>>> I was holding up this patch by not testing it on Armada, and I brought
>>> up the issue about registration at the beginning of this.
>>>

Sorry, this was poor on my part. There's no-one who will care about
Mali-DP, but I had forgotten that this would also break HDLCD.

>>> Now we're _just_ finding out that there are drivers where removing the
>>> connector registration in tda998x causes them to break?  It's a bit
>>> late to be checking your own drivers when you've been chasing me...
>>>

It's not like I didn't check our drivers, more that I should have sent
a full series for all three drivers together in the first place.

However, without patching all three drivers in the same commit, there
would always be some breakage. HDLCD and Mali-DP call
drm_dev_register() before binding the components - this was needed to
work with tda998x, which needed the device to be already registered
before its bind callback runs.

It's more proper to call drm_dev_register() as the very last thing
(i.e. after component_bind_all()) to avoid races with userspace - but
I couldn't do that without this change in tda998x first.

>>> Sorry, but it sounds like we're not ready to make this change - and as
>>> it's the very last day that changes will appear in linux-next prior to
>>> the merge window (assuming Linus releases 4.8 on Sunday), I'd suggest
>>> holding off until after the merge window is over, so we can get some
>>> testing with these other two drivers with this change in place.
>>>
>>
>> sigh. I just pushed my queue to drm-misc, which included this patch.
>> Sounds like I should revert?
>
>I thought I looked at the entire situation and since we register
>(since recently) all connectors in drm_dev_register() there shouldn't
>be an issue for any other driver. Imo no need to revert anything here
>- until someone complains with a bug report and proves me wrong ;-)

Yeah... this does indeed break HDLCD and Mali-DP. They register the
device, then bind tda998x which init's its connector. The connector
ends up in the connector_list, but not the idr.

I don't know how widely HDLCD on Juno is used (for anyone to notice).

-Brian

>-Daniel
>-- 
>Daniel Vetter
>Software Engineer, Intel Corporation
>+41 (0) 79 365 57 48 - http://blog.ffwll.ch
>