Thread (18 messages) 18 messages, 5 authors, 2015-07-01

Re: [PATCH V3 1/4] mfd: da9062: DA9062 MFD core driver

From: Lee Jones <hidden>
Date: 2015-06-11 08:57:12
Also in: linux-input, linux-rtc, linux-watchdog, lkml

quoted
quoted
quoted
diff --git a/include/linux/mfd/da9062/registers.h
b/include/linux/mfd/da9062/registers.h
quoted
new file mode 100644
index 0000000..d07c2bc
--- /dev/null
+++ b/include/linux/mfd/da9062/registers.h
[...]
quoted
quoted
+/*
+ * Registers
+ */
Really? ;)
quoted
+#define DA9062AA_PAGE_CON		0x000
+#define DA9062AA_STATUS_A		0x001
+#define DA9062AA_STATUS_B		0x002
[...]
quoted
quoted
+
+/*
+ * Bit fields
+ */
+
+/* DA9062AA_PAGE_CON = 0x000 */
+#define DA9062AA_PAGE_SHIFT		0
+#define DA9062AA_PAGE_MASK		(0x3f << 0)
+#define DA9062AA_WRITE_MODE_SHIFT	6
+#define DA9062AA_WRITE_MODE_MASK	(0x01 << 6)
For 1 << X, you should use BIT(X).
For the two comments above "Registers" and "Bit fields" and the (1<<x)
definitions ...

The whole of this file is automatically generated by our hardware designers
I would prefer it if the register definitions and bit fields are not altered using
the #define BIT(nr) (1UL<<(nr)) macro and the comments removed because
we have scripts that can be used to check this file automatically.

Also if the register map is ever updated, then it will be easier for me to diff
the new delivered register and bit field definitions with the old one.

My preference would be not to change this header file.

[...]
 
If these last two things are a problem can you please let me know.
I'm still not particularly happy with this.  Can yo speak to your H/W
guys and get them to change their scripts to output sensible header
files?

To be honest, it's probably not a blocker for acceptance, but if someone
writes a patch next week to change all of the (0x01 << X) lines to
start using the BIT() macro, I will accept it.  Better to influenced
your guys so you are not overly inconvenienced.

FWIW, when upstreaming code, the excuse "someone else wrote it", has
never been a good one to use on the lists.   Believe me, I've
tried. ;)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help