Thread (56 messages) 56 messages, 6 authors, 2021-11-22

Re: [PATCH v3 09/16] reset: starfive-jh7100: Add StarFive JH7100 reset driver

From: Andy Shevchenko <hidden>
Date: 2021-11-02 19:43:30
Also in: linux-clk, linux-devicetree, linux-gpio, linux-riscv, lkml

+Cc: Yury (bitmap expert)

On Tue, Nov 2, 2021 at 6:50 PM Emil Renner Berthing [off-list ref] wrote:
Add a driver for the StarFive JH7100 reset controller.
...
+#define BIT_MASK32(x) BIT((x) % 32)
Possible namespace collision.

...
+/*
+ * the registers work like a 32bit bitmap, so writing a 1 to the m'th bit of
+ * the n'th ASSERT register asserts line 32n + m, and writing a 0 deasserts the
+ * same line.
+ * most reset lines have their status inverted so a 0 in the STATUS register
+ * means the line is asserted and a 1 means it's deasserted. a few lines don't
+ * though, so store the expected value of the status registers when all lines
+ * are asserted.
+ */
Besides missing capitalization, if it sounds like bitmap, use bitmap.
I have checked DT definitions and it seems you don't even need the
BIT_MASK() macro,
+static const u32 jh7100_reset_asserted[4] = {
+       /* STATUS0 register */
+       BIT_MASK32(JH7100_RST_U74) |
+       BIT_MASK32(JH7100_RST_VP6_DRESET) |
+       BIT_MASK32(JH7100_RST_VP6_BRESET),
+       /* STATUS1 register */
+       BIT_MASK32(JH7100_RST_HIFI4_DRESET) |
+       BIT_MASK32(JH7100_RST_HIFI4_BRESET),
+       /* STATUS2 register */
+       BIT_MASK32(JH7100_RST_E24),
+       /* STATUS3 register */
+       0,
+};
Yury, do we have any clever (clean) way to initialize a bitmap with
particular bits so that it will be a constant from the beginning? If
no, any suggestion what we can provide to such users?

...
+       dev_dbg(rcdev->dev, "reset(%lu)\n", id);
These debug messages are useless since one should use ftrace facility instead,

-- 
With Best Regards,
Andy Shevchenko
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help