linux-kernel - Re: [PATCHv2 2/5] clk: samsung: exynos5410: Organize register offset constants

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: <53DA3B45.4080501@gmail.com>
Date:	Thu, 31 Jul 2014 14:49:09 +0200
From:	Tomasz Figa <tomasz.figa@...il.com>
To:	Humberto Silva Naves <hsnaves@...il.com>,
	linux-samsung-soc@...r.kernel.org
CC:	devicetree@...r.kernel.org,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kukjin Kim <kgene.kim@...sung.com>,
	Thomas Abraham <ta.omasab@...il.com>,
	linux-doc@...r.kernel.org, Tomasz Figa <t.figa@...sung.com>,
	Randy Dunlap <rdunlap@...radead.org>,
	linux-kernel@...r.kernel.org, Andreas Farber <afaerber@...e.de>,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCHv2 2/5] clk: samsung: exynos5410: Organize register offset
 constants

Hi Humberto,

Please see my comments inline.

On 31.07.2014 13:22, Humberto Silva Naves wrote:
> The different register groups (SRC, DIV, PLL, GATE, etc) are
> now separated by a blank line, and within the same group, the
> definitions are ordered by address. This is done to reduce the
> chances of potential conflicts when adding new entries, and
> to improve the readability of code. While at it, replaced some
> spaces with tabs to keep consistency.

I'm not sure whether this change really improves anything.

It might seem plausible to have the registers grouped by their purpose,
however remaining drivers have them directly listed in order of their
addresses to match the order they are mentioned in documentation. For
consistency, I'd prefer only one convention to be used across all
Samsung clock drivers, so they would have to be changed as well. But
IMHO this is a material for a separate clean-up series, while this one
should be limited to functional improvements.

In fact, this driver is kind of exception as it has PLL register
definitions separated, which I probably missed in review.

Best regards,
Tomasz
--
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/