[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <063D6719AE5E284EB5DD2968C1650D6D5F4F06BF@AcuExch.aculab.com>
Date: Tue, 5 Jul 2016 10:56:13 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Jakub Kicinski' <jakub.kicinski@...ronome.com>,
"kvalo@...eaurora.org" <kvalo@...eaurora.org>,
"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"hannes@...essinduktion.org" <hannes@...essinduktion.org>,
"nbd@....name" <nbd@....name>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCHv3 wl-drv-next 1/2] add basic register-field manipulation
macros
From: Jakub Kicinski
> Sent: 01 July 2016 22:27
>
> C bitfields are problematic and best avoided. Developers
> interacting with hardware registers find themselves searching
> for easy-to-use alternatives. Common approach is to define
> structures or sets of macros containing mask and shift pair.
> Operations on the register are then performed as follows:
>
> field = (reg >> shift) & mask;
>
> reg &= ~(mask << shift);
> reg |= (field & mask) << shift;
>
> Defining shift and mask separately is tedious. Ivo van Doorn
> came up with an idea of computing them at compilation time
> based on a single shifted mask (later refined by Felix) which
> can be used like this:
>
> #define REG_FIELD 0x000ff000
>
> field = FIELD_GET(REG_FIELD, reg);
>
> reg &= ~REG_FIELD;
> reg |= FIELD_PUT(REG_FIELD, field);
My problem with these sort of 'helpers' is that they make it much harder
to read code unless you happen to know exactly what the helpers do.
Unexpected issues (like values being sign extended) can be hard to spot.
A lot of the time you can make things simpler by not doing the shifts
(ie define shifted constants).
David
Powered by blists - more mailing lists