linux-kernel - Re: [RFC PATCH arm: initial TI-Nspire support]

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: <CACRpkdaFS=ZM6dTtD3ziBTfVJzwOU2x8gfyYutogkYL8LEo8qQ@mail.gmail.com>
Date:	Tue, 9 Apr 2013 13:58:45 +0200
From:	Linus Walleij <linus.walleij@...aro.org>
To:	Daniel Tang <dt.tangr@...il.com>
Cc:	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Russell King - ARM Linux <linux@....linux.org.uk>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	fabian@...ter-vogt.de, Lionel Debroux <lionel_debroux@...oo.fr>
Subject: Re: [RFC PATCH arm: initial TI-Nspire support]

On Tue, Apr 9, 2013 at 1:39 PM, Daniel Tang <dt.tangr@...il.com> wrote:

>>> +union reg_clk_speed {
>>> +       unsigned long raw;
>>> +       struct {
>>> +               unsigned long __padding0:1;
>>> +               unsigned long base_cpu_ratio:7;
>>> +               unsigned long is_base_27mhz:1;
>>> +               unsigned long __padding1:3;
>>> +               unsigned long cpu_ahb_ratio:3;
>>> +               unsigned long __padding2:1;
>>> +               unsigned long base_val:5;
>>> +       } val;
>>> +};
>>
>> Usually to try to fit a struct over a register range is not such a good
>> idea in Linux.
>>
>> Instead define abstract representations of what you want to do
>> (remove everything named "padding" above, use proper data types instead
>> of these unsigned longs and that complex union) then use offsets to
>> registers and remap the base offset in memory.
>>
>> It makes for simpler debugging and ability to properly use read|write[lwb]
>> macros.
>
> The structure is actually a bitfield. We'd readl() the raw unsigned long into the 'raw' field and then access the data via the 'val' structure.
>
> Should we be using bitmasks and bitshifting to get at those values instead?

I personally think that is better because it avoids complex structures
and strange helper functions.

When it really shows it's ugly face is when you get a second generation
of the hardware that have some other bits in slightly different places,
and you have to create another struct to map a small difference instead
of being able to just tweak the code slightly.

>> Not only should this be done from devicetree, but exactly which
>> synaptics driver are you using with this?
>>
>> I don't think there is one in the kernel tree yet.
>>
>
> It's this one here http://lxr.free-electrons.com/source/drivers/input/mouse/synaptics_i2c.c

Aha I thought it was one of those modern RMI4 things.
OK then...

> Also, how would you like us to submit updates? Should we continue posting updated patches as replies to this thread?

Just repost it somehow, include relevant people on To/Cc. No need
to use the same thread I think?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/