linux-kernel - Re: [PATCH 3/3] [regmap] [RESEND] Add "no-bus" option for regmap API

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]

Message-ID: <20130113231836.GJ5041@opensource.wolfsonmicro.com>
Date:	Mon, 14 Jan 2013 08:18:37 +0900
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	Andrey Smirnov <andrew.smirnov@...il.com>
Cc:	andrey.smirnov@...vergeddevices.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] [regmap] [RESEND] Add "no-bus" option for regmap API

On Sat, Jan 12, 2013 at 12:54:14PM -0800, Andrey Smirnov wrote:

> +	bool cache_registers;
> +

I'm afraid I don't quite understand this...

> -	if (!map->cache_bypass && map->format.format_write) {
> +	if (!map->cache_bypass && map->cache_registers) {
>  		ret = regcache_write(map, reg, val);

...I think it's mostly to service this check here, but looking at the
code I can't quite think why the code is doing what it's doing - I
suspect we should just remove the check for format_write() here.  I
think it was there to support potential bulk writes from the cache code
(which we don't do yet) but we're not actually doing those and it's not
clear that this should be doing that anyway.

>  	int ret;
> +	void *context = (map->bus) ? map : map->bus_context;
> +

Can you please make this a static inline regmap_map_get_context() or
something?  The same thing appears in quite a few places and the terery
operator isn't great at the best of times.

Otherwise this looks good, thanks a lot for doing this.

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)