Messages in this thread | | | From | Andy Shevchenko <> | Date | Tue, 2 Nov 2021 21:42:30 +0200 | Subject | Re: [PATCH v3 09/16] reset: starfive-jh7100: Add StarFive JH7100 reset driver |
| |
+Cc: Yury (bitmap expert)
On Tue, Nov 2, 2021 at 6:50 PM Emil Renner Berthing <kernel@esmil.dk> 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
| |