linux-kernel - Re: [PATCH v2 05/15] drm/bridge: tc358767: Drop custom tc_write()/tc

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:   Fri, 22 Mar 2019 10:45:56 -0700
From:   Andrey Smirnov <andrew.smirnov@...il.com>
To:     Tomi Valkeinen <tomi.valkeinen@...com>
Cc:     dri-devel@...ts.freedesktop.org,
        Archit Taneja <architt@...eaurora.org>,
        Andrzej Hajda <a.hajda@...sung.com>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Andrey Gusakov <andrey.gusakov@...entembedded.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Chris Healy <cphealy@...il.com>,
        Lucas Stach <l.stach@...gutronix.de>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 05/15] drm/bridge: tc358767: Drop custom
 tc_write()/tc_read() accessors

On Fri, Mar 22, 2019 at 3:29 AM Tomi Valkeinen <tomi.valkeinen@...com> wrote:
>
> On 22/03/2019 05:28, Andrey Smirnov wrote:
> > A very unfortunate aspect of tc_write()/tc_read() macro helpers is
> > that they capture quite a bit of context around them and thus require
> > the caller to have magic variables 'ret' and 'tc' as well as label
> > 'err'. That makes a number of code paths rather counterintuitive and
> > somewhat clunky, for example tc_stream_clock_calc() ends up being like
> > this:
> >
> >       int ret;
> >
> >       tc_write(DP0_VIDMNGEN1, 32768);
> >
> >       return 0;
> > err:
> >       return ret;
> >
> > which is rather surprising when you read the code for the first
> > time. Since those helpers arguably aren't really saving that much code
> > and there's no way of fixing them without making them too verbose to
> > be worth it change the driver code to not use them at all.
>
> I fully agree with this patch and thought about the same thing during my
> work.
>
> However, the timing of this patch is not too good, as this one will
> totally conflict with any other patch for tc358767, and my series is
> still evolving.
>

The reason I rebased this series on top of yours is because I think
mine should go after yours gets accepted and lands in the tree. I am
more than happy to wait for your series to mature. This submission was
done mostly for the sake of discussion, looping original authors in as
well as making you aware of its existence.

> We need to figure out how to combine this series and mine, but I think
> either this patch should be dropped for now, and reapplied after the
> other patches have stabilized, or I think preferably, this one could be
> rebased on top of 5.1-rc1, and used as a base for all other tc358767 work.
>

I think waiting for your work to be done before proceeding to apply
this series, should solve this problem.

Thanks,
Andrey Smirnov