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  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 12:29:02 +0200
From:   Tomi Valkeinen <tomi.valkeinen@...com>
To:     Andrey Smirnov <andrew.smirnov@...il.com>,
        <dri-devel@...ts.freedesktop.org>
CC:     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@...r.kernel.org>
Subject: Re: [PATCH v2 05/15] drm/bridge: tc358767: Drop custom
 tc_write()/tc_read() accessors

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.

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.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Powered by blists - more mailing lists